mikebronner / laravel-model-caching

Eloquent model-caching made easy.
MIT License
2.23k stars 212 forks source link

Retrieved Observer not firing from CachedBuilder #399

Open paularmstrong80 opened 3 years ago

paularmstrong80 commented 3 years ago

Hi Mike,

First, thank you for the great package, really appreciate your work!

I came across this issue when attempting to hook into the model's retrieved event.

The retrieved event only fires if the result hasn't been cached. Once the result is in cache, it no longer fires.

To work around the issue I had to do the following:

  1. Replaced newEloquentBuilder in base model to return extended GeneaLabs\LaravelModelCaching\CachedBuilder if model is cachable.
namespace App\Models;

use Illuminate\Database\Eloquent\Model;
use GeneaLabs\LaravelModelCaching\Traits\Cachable;
use App\Database\Eloquent\Builder\OverrideCachedBuilder;
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;

abstract class BaseModel extends Model
{
    use Cachable;

    public function newEloquentBuilder($query)
    {
        if (! $this->isCachable()) {
            $this->isCachable = false;

            return new EloquentBuilder($query);
        }

        return new OverrideCachedBuilder($query); // call custom extended CachedBuilder
    }

}
  1. Override CachedBuilder cachedValue method with a modified version that fires the retrieved model event.
namespace App\Database\Eloquent\Builder;

use GeneaLabs\LaravelModelCaching\CachedBuilder;

class OverrideCachedBuilder extends CachedBuilder
{
    public function cachedValue(array $arguments, string $cacheKey)
    {
        $method = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2)[1]['function'];
        $cacheTags = $this->makeCacheTags();
        $hashedCacheKey = sha1($cacheKey);
        $result = $this->retrieveCachedValue(
            $arguments,
            $cacheKey,
            $cacheTags,
            $hashedCacheKey,
            $method
        );

        // start - my hack to fire model retrieved event
        if ($resultValue = data_get($result, 'value')) {
            try {
                if (is_subclass_of($resultValue, 'Illuminate\Database\Eloquent\Model')) {
                    $resultValue->fireModelEvent('retrieved', false);
                }
            } catch (\Throwable $th) {
                //throw $th;
            }
        }
        // end - my hack to fire model retrieved event

        return $this->preventHashCollision(
            $result,
            $arguments,
            $cacheKey,
            $cacheTags,
            $hashedCacheKey,
            $method
        );
    }
}

My solution is not pretty, but works. I had troubles doing it other ways because of the debug_backtrace in the cacheValue event.

If there's a way to improve this solution, and/or get it into core, that would be amazing.

Kind Regards, Paul

Originally posted by @paularmstrong80 in https://github.com/GeneaLabs/laravel-model-caching/issues/171#issuecomment-819961169

mikebronner commented 3 years ago

Hi @paularmstrong80, thanks for moving this over to a new issue.

I'm going to have to ponder this a bit, but I can see where you're coming from.

sdbruder commented 2 years ago

Coming from this exact same issue (getting retrieved event for cached model instances, too). This will be fixed, ie, we will get retrieved for all models, cached or not? if Yes, there is a way to differentiate which one is cached or not cached? Another option is to get another event like retrievedFromCache or something like that in case of model instances which came from cache.

paularmstrong80 commented 2 years ago

There is a event Illuminate\Cache\Events\CacheHit which might be useful to you