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

Cannot use non-eloquent objects as scope #97

Closed
jkatruska opened this issue Mar 29, 2024 · 3 comments
Closed

Cannot use non-eloquent objects as scope #97

jkatruska opened this issue Mar 29, 2024 · 3 comments

Comments

@jkatruska
Copy link

jkatruska commented Mar 29, 2024

Pennant Version

1.7.0

Laravel Version

10.15.0

PHP Version

8.1.23

Database Driver & Version

No response

Description

Problem

I'm currently using laravel without eloquent, and I'm trying to use simple user object as scope. Data to this object are loaded from auth API as I'm using service oriented architecture . When I try to use that object as scope I'm getting an exception Unable to serialize the feature scope to a string. You should implement the FeatureScopeable contract., so I've implemented this interface to my object but basically return type of that method must be null|int|string|Model or I'll still get said exception from \Laravel\Pennant\FeatureManager::serializeScope. So I had to change my implementation to return just userId . But now my feature resolvers are getting just that userId as scope which means I have to again get user from my auth service based on its id.

Expected behaviour

I want it to work like it works with eloquent models. So you can pass object as scope and it will get serialized to string for purpose of serialization scope for storage.

Proposed solution

At first I thought that we can add new arm to match in serializeScope() something like this
$scope instanceof \Laravel\Pennant\Contracts\FeatureScopeable => $scope->toFeatureIdentifier() but then I've realised that this method may not return string and looking further into the code it could break some things. So as an alternative to this we could create new interface to serve this purpose to create identifier for serialization. Then you can implement it at any object and you can keep your object as scope and serialization would also work.

Steps To Reproduce

Create any POPO object and try to use it as a scope, at first you will get exception to use FeatureScopeable and when you will implement it you will still be getting exceptions if your return value won't be null|int|string|Model

@driesvints
Copy link
Member

Right now this requires Eloquent, sorry. We don't have plans to change this right now. You can always attempt a PR to change this and depending on the amount of code involved and complexity we can see how it goes.

@timacdonald
Copy link
Member

@jkatruska, you may use anything as scope, eloquent or not.

The value returned from the toFeatureIdentifier method should uniquely identify the scope in some serialized way.

For example, lets say I have the following user object...

final readonly class User
{
    public function __construct(
        public int $id,
        public string $name,
        public string $email,
    ) {
        //
    }
}

Notice that the class does not extend eloquent. I can use this object with Pennant manually...

$user = new User(
    id: 432,
    name: 'Tim',
    email: 'tim@laravel.com'
);

/* ... */

// use the integer based ID...

Feature::for($user->id)->active('new-api');

Notice I use the ID here and not the email, as the email address may change and would break the connection between the stored value and the user.

If I want to integrate with Pennant directly, as you are trying to do, I would implement the FeatureScopeable interface...

final readonly class User implements FeatureScopeable
{
    public function __construct(
        public int $id,
        public string $name,
        public string $email,
    ) {
        //
    }

    public function toFeatureIdentifier(string $driver): int
    {
        return $this->id;
    }
}

Then the object may be used with Pennant like so:

- Feature::for($user->id)->active('new-api');
+ Feature::for($user)->active('new-api');

If you want to make it easier to identify scopes in the database or make sure it does not conflict with other integer based scopes, you can prepend a value to the id, which is probably what I would do:

final readonly class User implements FeatureScopeable
{
    public function __construct(
        public int $id,
        public string $name,
        public string $email,
    ) {
        //
    }

    public function toFeatureIdentifier(string $driver): string
    {
        return "user:{$this->id}";
    }
}

Hope that helps.

@jkatruska
Copy link
Author

Hi @timacdonald,

as I mentioned previously I'm aware of this, but this would result in that my feature resolver will get int or string as input parameter and I would have to fetch user again so I can call user methods e.g. $user->isInBeta() to determine if user can access some feature.

On the other hand if you pass Eloquent object as a scope you will get that same object in your resolver.

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

Successfully merging a pull request may close this issue.

3 participants