Open eidng8 opened 8 years ago
Hey, really sorry for the delay in getting to this one. Good find.
I seem to recall the cache callback being added to prevent a circular loop that popped up in an earlier version of Laravel. I will try and take a look into this soon, but also happy to review any PRs.
Actually, in my project, I've change the callback to 1-liner
return function() use ($columns) {return parent::get($columns);}
It works for me. But it seems that this repo is ripped off of tests, I'm not sure if this change affects anything unwanted. So I leave it to your decision, and didn't submit a PR.
The setup is phpunit & array cache driver. And I've tapped in to the cache using:
The line below:
After that, I've found that there are two entries in the cache: the
aggregate
&relation
. The main model (record) is missed.While traced a bit, I was led to the line, which traced back to query builder (not eloquent builder)
getCountForPagination()
Then the subsequent call to
forPage()->get()
will not hit the cache. Because the$cacheMinutes
property was set tonull
in the closure returned bygetCacheCallback()
. Which ends up failing the non-null test inget()
.