l5-repository icon indicating copy to clipboard operation
l5-repository copied to clipboard

CacheableRepository all() used in conjunction with whereHas() returns the same cached objects

Open w0rd-driven opened this issue 6 years ago • 1 comments

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.

w0rd-driven avatar Apr 05 '18 17:04 w0rd-driven

Same problem here. As a workaround on one of my repositories using this package, I had to turn off caching for it. It's strange that, after more than 2 years, this behavior wasn't fixed (maybe I'm missing something or my approach isn't completely correct). Any suggestions? @andersao

Thanks

eleftrik avatar Nov 29 '20 13:11 eleftrik