andersao / andersao/l5-repository
CacheableRepository all() used in conjunction with whereHas() returns the same cached objects
- Dominant language
- PHP
- Stars
- 4.2k
- Forks
- 880
- PR merge metrics
- No merged PRs in 30d
Description
I'm using the following code to filter the repository to just return records for an associated user_id:
```
return $this->whereHas($relation, function ($query) use ($userId) {
$query->where('users.id', $userId);
})->all();
```
It looks like what happens is https://github.com/andersao/l5-repository/blob/master/src/Prettus/Repository/Traits/CacheableRepository.php#L206 or specifically `func_get_args()` is empty in this instance as I'm not passing anything in.
It looks like this is somewhat mitigated with @elliottmb's comment: https://github.com/andersao/l5-repository/issues/78#issuecomment-166059698.
Changing the call to getCacheKey() to look something like the following, it seems to work as expected.
```
$args = func_get_args();
if ($this->model instanceof \Illuminate\Database\Eloquent\Builder) {
$query = $this->model->getQuery();
$args = array_merge(
$args,
[
$query->getBindings(),
$query->toSql()
]
);
}
$key = $this->getCacheKey('all', $args);
```
It's looking like this works for my workflow in rudimentary tests but I don't totally know the repercussions of this possibly impacting others. My gut is because the args are something like `select * from users` versus always being empty that at worst it may possibly cache the same underlying results multiple times but that's something I can live with. My use case heavily involves RequestCriteria but that doesn't seem affected by this change at all.
I don't know if this is something I should submit a PR for or if this will help someone else. It's possible there's a better approach to what I'm doing that doesn't require modifications to this package that I'm just not aware of.
Contributor guide
Assessment
This issue has not been assessed yet.