Skip to content

Conversation

thyseus
Copy link

@thyseus thyseus commented Mar 16, 2021

Why ?

I have the following model:

class DemoUnscoped { public $table = 'demo_unscoped'; public function scopeFooBar($query)	{ $query->from('demo_scoped'); return $query;	} }

Now, assuming my table demo_unscoped has 666 entries, and demo_scoped has 999 entries, prior to this patch i get the following results:

DemoUnscoped::count(); 666 DemoUnscoped::FooBar()->count(); 666 // <----------- WRONG (has cached the value but should not!) 

With this fix applied i get:

DemoUnscoped::count(); 666 DemoUnscoped::FooBar()->count(); 999 // <----------- Correct ! 
@mikebronner
Copy link
Owner

Hi @thyseus, thanks for submitting this. Unfortunately the tests aren't passing. Could you take a look into that? Be sure to run the tests locally on your machine and make sure they all pass. I'll try to take a look at it when I have some free time. Thanks! :)

@thyseus
Copy link
Author

thyseus commented Mar 16, 2021

Hey, @mikebronner - thanks a lot for your quick reply !

I already begun digging into this issue. It seems like i need to adjust the $key fixtures in (every??) test so the tests are using the new source. Unfortunately i can not grasp where the "table/from" part is inside the key:

$key = sha1("genealabs:laravel-model-caching:testing:{$this->testingSqlitePath}testing.sqlite:book-store:genealabslaravelmodelcachingcachedbelongstomany-book_store.book_id_=_{$bookId}"); $tags = [ "genealabs:laravel-model-caching:testing:{$this->testingSqlitePath}testing.sqlite:genealabslaravelmodelcachingtestsfixturesstore",	];
  • which part in $key does represent the tablename ?

Could you give me a little advice ? You know the code much better than me :)

and by the way: i have never seen a such thorougly tested module in my whole life. really good work !!!

@mikebronner
Copy link
Owner

Ah, yes!!! It's annoying and tedious :) Unfortunately your change seems that it could be affecting all cache keys. Let me take a look as to why they are changing. Most tests should not really change, unless from() is not set in the query.
I also need to add PHP 8.0 to the test matrix.

@thyseus
Copy link
Author

thyseus commented Mar 16, 2021

Thanks a very lot ! Your module is awesome. Let me know if i can be of any help...

@mikebronner mikebronner merged commit ba18569 into mikebronner:master Mar 16, 2021
@mikebronner
Copy link
Owner

@thyseus try release 0.11.2 and let me know how it works for you. :) Thanks for submitting this PR! :)

@thyseus thyseus deleted the caching-key-query-from-instead-model-table branch March 16, 2021 15:14
@thyseus
Copy link
Author

thyseus commented Mar 16, 2021

@mikebronner just did run composer update, geneolabs/laravel-model-caching got updated and everything is working as expected ! Thanks again a lot for this really quick reaction.

@mikebronner
Copy link
Owner

@thyseus That's awesome! :) Super glad you are enjoying this package.

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

2 participants