Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update model to implement a trait to set mongo functionality #2120

Closed
michaeljennings opened this issue Oct 13, 2020 · 4 comments · Fixed by #2580
Closed

Update model to implement a trait to set mongo functionality #2120

michaeljennings opened this issue Oct 13, 2020 · 4 comments · Fixed by #2580

Comments

@michaeljennings
Copy link

In a project I'm working on we use laravel passport for our authentication. Up until now we've been using the designmynight/laravel-mongodb-passport package to update the passport models to work with mongo. This package adds copies of each of the passport models but extends the mongodb base model instead.

We've just updated to version 10 of passport and you can no longer do this as a lot of the classes now type hint the default passport models, this means you now have to extend the passport models.

For example you could override the default token model easily by doing Passport::useTokenModel(CustomTokenModel::class), but in the Laravel\Passport\TokenRepository class it type hints the default token model in save method.

/**
 * Store the given token instance.
 *
 * @param  \Laravel\Passport\Token  $token
 * @return void
 */
public function save(Token $token)
{
    $token->save();
}

This leaves us in a situation where we either have to go through and make custom versions of every passport class that has a type hint in, or to extend every passport model and copy the contents of the mongodb model into it.

I'm proposing to change the mongo db model so it implements a trait to set the mongo specific functionality.

The model would then end up looking something like this:

namespace Jenssegers\Mongodb\Eloquent;

use Illuminate\Database\Eloquent\Model as BaseModel;

abstract class Model extends BaseModel
{
    use Moloquent;

    /**
     * The primary key for the model.
     * @var string
     */
    protected $primaryKey = '_id';

    /**
     * The primary key type.
     * @var string
     */
    protected $keyType = 'string';
}

The Moloquent trait would then have all of the current methods and mongo specific properties from the current model.

This would allow us to extend models from other projects more easily and then just implement the Moloquent trait to make it work with mongo. So our token class could then become:

class Token extends \Laravel\Passport\Token
{
    use Moloquent;
}

I've tried cloning the project and implementing the trait instead and all of the tests still pass.

I'm happy to submit a merge request to show how the trait would work but let me know your thoughts.

@divine
Copy link
Contributor

divine commented Oct 13, 2020

Hello,

Sounds good for me.

@Smolevich what do you think?

Thanks!

@Smolevich
Copy link
Contributor

@divine i am agree with this case

@GromNaN
Copy link
Member

GromNaN commented Sep 4, 2023

Currently, the Model class is a marker for MongoDB models. If a trait is used, an other marker will be necessary.

if (! is_subclass_of($related, MongoDBModel::class)) {

Edit: use class_uses_recursive.

@LukeTowers
Copy link

This would also be useful in order to add support to Winter CMS which also extends the base Laravel model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants