Cache wrapper not updating model-scoped features correctly

bradietilley commented 10 months ago

Pennant Version

1.5.1

Laravel Version

10.39.0

PHP Version

8.2.8

Database Driver & Version

MySQL v8.0.33 for macos 13.3 on arm64 (Homebrew)

Description

The caching facility in the Decorator class appears to be incorrectly pushing feature flags to the cache instead of updating an existing cache entry, when a Model scope is provided when the Model is a different instance of the same record.

For example User::first() !== User::first() because the objects are different, albeit the same record.

This is pretty much what's happening in the cache - the same record won't match because the object is different.

Steps To Reproduce

Repo: https://github.com/bradietilley/pennant-caching

git clone git@github.com:bradietilley/pennant-caching.git
cd pennant-caching
composer install
cp .env.example .env
# <setup database details in .env>
artisan test

I added some comments in tests/Unit/PennantTest.php that explains the issue. In essence, $item['scope'] === $scope is failing but (($item['scope'] === $scope) || ($scope instanceof \Illuminate\Database\Eloquent\Model && $scope->is($item['scope']))) works. Of course, this logic is repeated in more than one place so it'd be worth keeping it DRY.

bradietilley commented 10 months ago

Side note:

Alternatively I could add FeatureFlaggable interface to my model and then add:

    /**
     * Cast the object to a feature scope identifier for the given driver.
     */
    public function toFeatureIdentifier(string $driver): mixed
    {
        return $this->getTable().'_'.$this->getKey();
    }

... but seems weird to add this boilerplate to every project we use pennant in 🤷

timacdonald commented 10 months ago

Thanks again for the reproduction repository on this one.

I've opened a PR to fix this issue: https://github.com/laravel/pennant/pull/83

I'll close this and we can track it in the PR.

laravel / pennant