Global scopes do not work with cachable models

[scottgrayson]

Issue

I would like to have a cache for users.index queries because they take a while. I want to ->disableCache() on all other queries. I tried a global scope that disables cache but found that global scopes do not work with models that use the Cachable trait.

This does not work:

• neither the enabled nor nocache scopes work here
• enabled works if I remove the Cachable trait

    use Cachable;

    public static function boot()
    {
        parent::boot();

        static::addGlobalScope('enabled', function (CachedBuilder $builder) {
            // dd('hi');
            $builder->where('enabled', 1);
        });

        static::addGlobalScope('nocache', function (CachedBuilder $builder) {
            $builder->disableCache();
        });
    }

• I get no errors, but results always use cache.
• I never get the dd('hi'); output if i uncomment it (shows that the callback given to addGlobalScope is never applied)

Environment

Laravel Version: 5.6.10
Laravel Model Caching Package Version: 0.2.52
PHP Version: 7.1.11
Valet Version: 2.0.7
Operating System & Version: MacOS 10.12.6

Stack Trace

N\A

[mikebronner]

Hi @scottgrayson, thanks for reporting this use-case. I don't think this package is a good fit for you, if you only want to cache a single query in your entire app. You would be better off writing the cache functionality manually.

However, if you are using this on all other models, but only wish to have special treatment on a single model, my suggestion would be similar to above: do not make that model Cachable, but instead manually cache that one query as needed.

I will look into why the scope would not be working and see if there is a fix for that.

[mikebronner]

PS: have you tried using a local scope on the model?

[scottgrayson]

However, if you are using this on all other models, but only wish to have special treatment on a single model, my suggestion would be similar to above: do not make that model Cachable, but instead manually cache that one query as needed.

This describes my situation. I'm working on caching it manually now, but if I could use your package for users as well, it would be a cleaner solution for me.

have you tried using a local scope on the model?

I have tried a local scope and it does work.

    public function scopeNocache($query)
    {
        return $query->disableCache();
    }

this is not very useful though. is there a different way you thought of using a local scope?

[mikebronner]

Hmm, I see. It will take some time for me to evaluate and research this, as we have Easter weekend coming up, and I will be out of town. I hope to get to it within the next week or so. I will post back here when I have an update. :)

[sgtaziz]

I also noticed this issue when using the Clockwork package. However in this case, it actually throws an error instead of silently failing. The error is thrown when any model with the Cachable trait is used. Below is the stack trace:

[2018-04-09 05:25:56] local.ERROR: Call to a member function addGlobalScope() on null {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Call to a member function addGlobalScope() on null at /var/www/laraveldev/vendor/itsgoingd/clockwork/Clockwork/DataSource/EloquentDataSource.php:52)
[stacktrace]
#0 /var/www/laraveldev/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php(357): Clockwork\\DataSource\\EloquentDataSource->Clockwork\\DataSource\\{closure}(NULL, Array)
#1 /var/www/laraveldev/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php(209): Illuminate\\Events\\Dispatcher->Illuminate\\Events\\{closure}('eloquent.booted...', Array)
#2 /var/www/laraveldev/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php(182): Illuminate\\Events\\Dispatcher->dispatch('eloquent.booted...', Array, false)

Help with this would be much appreciated, I'm a big fan of this package 😄

[jcsoriano]

Hi, any plans on making Cachable work with global scopes soon? :) The reason I'm asking is I did a search on which functionality uses global scopes and found two pretty common use cases:

1. Models with SoftDeletes (vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasGlobalScopes.php)

and
2. Scout Searchable models (vendor/laravel/scout/src/Searchable.php)

So I'm concerned that if I add Cachable, soft-deleted models will still appear in results or scout searches won't work well.

Thank you very much for this package!

[mycarrysun]

Everyone active on this issue - I am currently investigating the global scope scopeDisableCache() and I don't think that the title on this issue is accurate.

It seems the way that the cache key is formed is already taking any scopes that have been applied into account. So global scopes will be caught.

The problem you are facing here is only limited to the scopeDisableCache() function. I believe it's because of this line. When changing to:

$this->isCachable = false;

it seems to fix the problem.

@mikebronner Can you confirm this will not break other functionality? I will create a PR and you can close if it breaks something I don't know about.

[mikebronner]

@mycarrysun Thanks for looking into this!!! :)
I have not looked at this issue for quite some time. If you can create a PR, I will write some tests to see if it works, as well as see if it breaks current functionality.

[mikebronner]

@scottgrayson @jcsoriano @sgtaziz Can you test if this is now fixed in the latest release, 0.4.12 or newer? I believe this is working now. If you are still having issues, please re-open.