mikebronner / laravel-model-caching

Eloquent model-caching made easy.
MIT License
2.25k stars 212 forks source link

Dynamic local scopes in `with` clauses always returns same results, even if scope is different. #291

Open hyperized opened 5 years ago

hyperized commented 5 years ago

Describe the bug I'm not sure if this is a feature request or bug. I hope you don't mind I put it in bugs.

We're currently using Dynamic Local Query scopes (https://laravel.com/docs/5.8/eloquent#local-scopes -> Dynamic scopes). However it seems that the cache key does not take the dynamic aspect into account, making it rather .. static.

I could imagine that the cache key generation method can check for additional parameters when generating the key as a possible solution?

Eloquent Query

public function scopeOfLesson($query, Lesson $lesson)
{
return $query->where('lesson_id', $lesson->id);
}
return Student::
where('id', 1)
->with(['progress' => static function (HasMany $model) use ($lesson) {
$model->ofLesson($lesson);
}])
->get();

The results is that when given two different $lessons that the cache results will return the result of the first requested $lesson.

Stack Trace n/a

Environment

Additional context Thank you for making this package, we use it in every Laravel project to speed things up :heart:!

mikebronner commented 5 years ago

@hyperized Thanks for reporting this! I will take a look at the implementation and add a test-case to and hopefully find a way to fix it. :)

mikebronner commented 5 years ago

@hyperized I have been able to reproduce this issue and am trying to figure out a fix. It's complicated because this is a scope inside a relationship query. If the scope were applied to the main model directly, it would work as expected.

As a work-around for the time-being, run queries with dynamic scopes in with clauses like this:

return Student::
    disableModelCaching()
    ->where('id', 1)
    ->with(['progress' => static function (HasMany $model) use ($lesson) {
        $model->ofLesson($lesson);
    }])
    ->get();
mikebronner commented 5 years ago

Been looking at this for a few hours now and am having a hard time coming to a solution. Any help would be appreciated, if anyone knows how we could analyze the eagerloads that are closures.

eliasjtg commented 2 years ago

Same issue here, any solution?

mikebronner commented 2 years ago

Not yet ... if someone is willing to create a PR for this, that would be greatly appreciated. Short of that, even if someone just submits a PR with the test case, that would be great.