Criteria & Cache ? about l5-repository HOT 11 CLOSED

And in a similar fashion to @DarKDinDoN, I'm now commenting with little to no time having passed between comments.

I've found a possible work around for this whole mess without doing anything too fancy. By giving up some code reuse, I've modified the standard CacheableRepository trait's all() method, and soon other methods, to simply inject the SQL string and bindings into the $args input of the getCacheKey() method.

    public function all($columns = array('*'))
    {
        if (!$this->allowedCache('all') || $this->isSkippedCache()) {
            return parent::all($columns);
        }

        $this->applyCriteria();
        $this->applyScope();

        $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);

        $minutes = $this->getCacheMinutes();
        $value = $this->getCacheRepository()->remember($key, $minutes, function () use ($columns) {
            if ($this->model instanceof \Illuminate\Database\Eloquent\Builder) {
                $results = $this->model->get($columns);
            } else {
                $results = $this->model->all($columns);
            }

            return $this->parserResult($results);
        });

        $this->resetModel();

        return $value;
    }

from l5-repository.

For the record, I added a getByManyCriteria method that does the exact same thing as the getByCriteria method but accepts an array as parameter.

from l5-repository.

Sorry for the second comment in no time.
I actually deleted the getByManyCriteria and changed the getCacheKey() method and serialize the criteria along with the other parameters :

public function getCacheKey($method, $args = null)
    {
        $request = app('Illuminate\Http\Request');
        $args    = serialize($args);
        $criteria = serialize($this->getCriteria()); // Serialize the criteria
        $key     = sprintf('%s@%s-%s',
            get_called_class(),
            $method,
            md5($args.$criteria.$request->fullUrl()) // Add the criteria to the hash
        );

        CacheKeys::putKey(get_called_class(), $key);

        return $key;
    }

from l5-repository.

This is indeed a problem, parameters of criteria aren't included in the cache. UserCriteria(1) will cache records, the next time UserCriteria(2) will return those same records.

Code of DarKDinDoN seems to fix it

from l5-repository.

Actually, It won't work if you use a closure in the criteria.

e.g : WhereHasCriteria with a $q->wereHas(...);

No solution yet.

from l5-repository.

I've been running into this issue myself.

I haven't 100% solved it to my liking, but what I've done is use @DarKDinDoN's tweak. I've paired that with implementing the Serializable Interface in any Criteria that uses a closure. In the serialize method, I use SuperClosure\Serializer to serialize the closure.

It appears to work in most cases, but still run into some failings.

One such failing is if you use the RequestCriteria.

from l5-repository.

I've been very busy in recent months with my work projects, which eventually pulling away of my personal projects, so it could not provide the necessary support to you, I am more relaxed now and I'm checking all the issues in order to work on fixes and new implementations, apologize to everyone.

from l5-repository.

That's good to hear, but it's quite alright. Life has a way of making us busy and no apology is needed.

I've gone about modifying the cache key generation method to take into consideration the underlying query and bindings of a statement. I've improved the way I do it since my comment above, but the general idea is the same.

Essentially apply the criteria and scope so that the underlying query instance is fully built up, and then serialize the query string and the bindings array before concatenation of them alongside the $args and $request serialization before passing them into the md5 function.

Would you be interested in a pull request or do you have another idea for solving this issue? I'll admit my solution is eloquent dependent and doesn't take advantage of the inherited parent implementation beyond the first if statement.

from l5-repository.

@elliottmb Could you make a pull request, or create a gist with modified code ?

Thanks.

from l5-repository.

@elliottmb news on this one?

from l5-repository.

I'd also love to see a solution so that the criteria is cached

from l5-repository.