diff --git a/src/Support/ApiModelCache.php b/src/Support/ApiModelCache.php index f6c3e069..9ce16762 100644 --- a/src/Support/ApiModelCache.php +++ b/src/Support/ApiModelCache.php @@ -17,6 +17,15 @@ * - Automatic cache invalidation * - Multi-tenancy support * - Cache tagging for efficient bulk invalidation + * + * Caches are invalidated by versions embedded in their keys, not by the tag + * flush: list caches carry a per table/company query version, single model and + * relationship caches a per table model version (their lookups are keyed by id + * and know no company). On Laravel 10 with phpredis >= 6.1, + * RedisTagSet::entries() starts ZSCAN with cursor '0', which phpredis treats as + * an already finished scan, so RedisTaggedCache::flush() deletes only the tag + * sets and leaves every tagged value in Redis until its TTL expires. Bumping the + * version moves readers to new keys and leaves the orphaned entries to expire. */ class ApiModelCache { @@ -89,14 +98,43 @@ public static function generateQueryCacheKey(Model $model, Request $request, arr // Get query version for this table/company // This ensures cache keys change after invalidation - $versionKey = "api_query_version:{$table}:{$companyUuid}"; - $version = Cache::get($versionKey, 1); // Default to version 1 + $version = Cache::get(static::queryVersionKey($table, $companyUuid), 0); // Add Redis hash tag {api_query} for Redis Cluster shard routing // Include version to ensure cache keys change after writes return "{api_query}:{$table}:{$companyPart}:v{$version}:{$paramsHash}"; } + /** + * Cache key holding the query version for a table/company. + */ + protected static function queryVersionKey(string $table, ?string $companyUuid): string + { + return "api_query_version:{$table}:{$companyUuid}"; + } + + /** + * Cache key holding the model and relationship version for a table. + */ + protected static function modelVersionKey(string $table): string + { + return "api_model_version:{$table}"; + } + + /** + * Advance a cache version so keys built from it change. + * + * A missing version reads as 0 and INCR creates it at 1, so the first bump + * also changes the key. Stores whose increment does not create missing keys + * (database, memcached) return false, so the version is seeded instead. + */ + protected static function bumpVersion(string $versionKey): void + { + if (Cache::increment($versionKey) === false) { + Cache::add($versionKey, 1) || Cache::increment($versionKey); + } + } + /** * Generate a cache key for a single model instance. * @@ -106,9 +144,11 @@ public static function generateModelCacheKey(Model $model, $id, array $with = [] { $table = $model->getTable(); $withHash = !empty($with) ? ':' . md5(json_encode($with)) : ''; + $version = Cache::get(static::modelVersionKey($table), 0); // Add Redis hash tag {api_model} for Redis Cluster shard routing - return "{api_model}:{$table}:{$id}{$withHash}"; + // Include version to ensure cache keys change after writes + return "{api_model}:{$table}:v{$version}:{$id}{$withHash}"; } /** @@ -116,11 +156,13 @@ public static function generateModelCacheKey(Model $model, $id, array $with = [] */ public static function generateRelationshipCacheKey(Model $model, string $relationshipName): string { - $table = $model->getTable(); - $id = $model->getKey(); + $table = $model->getTable(); + $id = $model->getKey(); + $version = Cache::get(static::modelVersionKey($table), 0); // Add Redis hash tag {api_relation} for Redis Cluster shard routing - return "{api_relation}:{$table}:{$id}:{$relationshipName}"; + // Shares the model version, since relationships are invalidated with their parent + return "{api_relation}:{$table}:v{$version}:{$id}:{$relationshipName}"; } /** @@ -166,9 +208,8 @@ public static function cacheQueryResult(Model $model, Request $request, \Closure $ttl = $ttl ?? static::LIST_TTL; // FIX #3: Guard against read-after-invalidate in same request - // After invalidation, resetCacheStatus() sets $cacheStatus to null - // But we need a way to detect if invalidation happened in this request - // For now, we'll rely on tag flush working correctly + // After invalidation, resetCacheStatus() sets $cacheStatus to null and the + // query version has moved, so the key generated above is already the new one try { // Initialize cache status and key @@ -322,11 +363,12 @@ public static function invalidateModelCache(Model $model, ?string $companyUuid = // after invalidation within the same request lifecycle static::resetCacheStatus(); - // FIX #4: Increment query version counter - // This ensures the cache key changes after invalidation - $table = $model->getTable(); - $versionKey = "api_query_version:{$table}:{$companyUuid}"; - Cache::increment($versionKey); + // FIX #4: Increment the query and model version counters + // This ensures cache keys change after invalidation, and it is what + // actually hides stale caches (see the class docblock) + $table = $model->getTable(); + static::bumpVersion(static::queryVersionKey($table, $companyUuid)); + static::bumpVersion(static::modelVersionKey($table)); // Generate tags for BOTH model and query caches // Model caches: single-record lookups @@ -339,7 +381,8 @@ public static function invalidateModelCache(Model $model, ?string $companyUuid = Cache::tags($modelTags)->flush(); // Flush query-level caches (collections, lists) - // This is CRITICAL - query caches must be explicitly flushed + // On Redis these flushes remove the tag sets but not the values (see the + // class docblock); the version bumps above are what invalidate caches Cache::tags($queryTags)->flush(); } catch (\Exception $e) { Log::error('Failed to invalidate cache', [ diff --git a/tests/Unit/Support/ApiModelCacheTest.php b/tests/Unit/Support/ApiModelCacheTest.php index 9bfe9689..047067fb 100644 --- a/tests/Unit/Support/ApiModelCacheTest.php +++ b/tests/Unit/Support/ApiModelCacheTest.php @@ -140,12 +140,14 @@ public function flush(): bool class ApiModelCacheTestStore { - public array $values = []; - public array $taggedValues = []; - public array $flushedTags = []; - public array $forgotten = []; - public bool $throwOnTags = false; - public bool $lockReturnsNull = false; + public array $values = []; + public array $taggedValues = []; + public array $flushedTags = []; + public array $forgotten = []; + public bool $throwOnTags = false; + public bool $lockReturnsNull = false; + public bool $flushRemovesValues = true; + public bool $incrementCreatesMissing = true; public function get(string $key, mixed $default = null): mixed { @@ -159,8 +161,23 @@ public function put(string $key, mixed $value): bool return true; } - public function increment(string $key): int + public function add(string $key, mixed $value): bool { + if (array_key_exists($key, $this->values)) { + return false; + } + + $this->values[$key] = $value; + + return true; + } + + public function increment(string $key): int|false + { + if (!$this->incrementCreatesMissing && !array_key_exists($key, $this->values)) { + return false; + } + $this->values[$key] = ($this->values[$key] ?? 0) + 1; return $this->values[$key]; @@ -202,7 +219,10 @@ public function forgetTagged(array $tags, string $key): void public function flushTagged(array $tags): void { - unset($this->taggedValues[$this->tagKey($tags)]); + // Laravel 10 RedisTaggedCache on phpredis >= 6.1 drops the tag sets but not the values + if ($this->flushRemovesValues) { + unset($this->taggedValues[$this->tagKey($tags)]); + } } private function tagKey(array $tags): string @@ -406,9 +426,9 @@ public function gc(int $max_lifetime): int|false expect($first)->toBe($second) ->and($first)->toStartWith('{api_query}:orders:company_company-1:v7:') ->and(ApiModelCache::generateModelCacheKey($model, 'order-1', ['customer', 'payload'])) - ->toBe('{api_model}:orders:order-1:' . md5(json_encode(['customer', 'payload']))) + ->toBe('{api_model}:orders:v0:order-1:' . md5(json_encode(['customer', 'payload']))) ->and(ApiModelCache::generateRelationshipCacheKey($model, 'payload')) - ->toBe('{api_relation}:orders:order-1:payload') + ->toBe('{api_relation}:orders:v0:order-1:payload') ->and(ApiModelCache::generateCacheTags($model, 'company-1', true)) ->toBe(['api_cache', 'api_model:orders', 'api_query:orders', 'company:company-1']); }); @@ -428,7 +448,7 @@ public function gc(int $max_lifetime): int|false expect($first->all())->toBe(['fresh-result']) ->and($calls)->toBe(1) ->and(ApiModelCache::getCacheStatus())->toBe('MISS') - ->and(ApiModelCache::getCacheKey())->toStartWith('{api_query}:orders:company_company-1:v1:'); + ->and(ApiModelCache::getCacheKey())->toStartWith('{api_query}:orders:company_company-1:v0:'); $second = ApiModelCache::cacheQueryResult($model, $request, function () use (&$calls) { $calls++; @@ -500,7 +520,7 @@ public function gc(int $max_lifetime): int|false ->and($secondModel)->toBe(['uuid' => 'order-1']) ->and($modelCalls)->toBe(1) ->and(ApiModelCache::getCacheStatus())->toBe('HIT') - ->and(ApiModelCache::getCacheKey())->toStartWith('{api_model}:orders:order-1:'); + ->and(ApiModelCache::getCacheKey())->toStartWith('{api_model}:orders:v0:order-1:'); ApiModelCache::resetCacheStatus(); $relationshipCalls = 0; @@ -519,7 +539,7 @@ public function gc(int $max_lifetime): int|false ->and($secondRelation)->toBe(['uuid' => 'payload-1']) ->and($relationshipCalls)->toBe(1) ->and(ApiModelCache::getCacheStatus())->toBe('HIT') - ->and(ApiModelCache::getCacheKey())->toBe('{api_relation}:orders:order-1:payload'); + ->and(ApiModelCache::getCacheKey())->toBe('{api_relation}:orders:v0:order-1:payload'); }); test('api model cache bypasses model relationship and invalidation work when disabled', function () { @@ -568,6 +588,7 @@ public function gc(int $max_lifetime): int|false expect(ApiModelCache::getCacheStatus())->toBeNull() ->and(ApiModelCache::getCacheKey())->toBeNull() ->and($store->get('api_query_version:orders:company-1'))->toBe(1) + ->and($store->get('api_model_version:orders'))->toBe(1) ->and($store->flushedTags)->toContain(['api_cache', 'api_model:orders', 'company:company-1']) ->and($store->flushedTags)->toContain(['api_cache', 'api_model:orders', 'api_query:orders', 'company:company-1']); @@ -581,6 +602,77 @@ public function gc(int $max_lifetime): int|false expect($store->flushedTags)->toContain(['company:company-1']); }); +test('api model cache does not serve a list cached before the first invalidation when the tag flush leaves it behind', function () { + $store = api_model_cache_fixture(); + $store->flushRemovesValues = false; + $model = api_model_cache_model(); + $request = api_model_cache_request(['status' => 'active']); + + expect(ApiModelCache::cacheQueryResult($model, $request, fn () => collect(['stale'])))->toEqual(collect(['stale'])); + $keyBeforeInvalidation = ApiModelCache::getCacheKey(); + + ApiModelCache::invalidateModelCache($model, 'company-1'); + + expect(ApiModelCache::cacheQueryResult($model, $request, fn () => collect(['fresh'])))->toEqual(collect(['fresh'])) + ->and(ApiModelCache::getCacheStatus())->toBe('MISS') + ->and(ApiModelCache::getCacheKey())->not->toBe($keyBeforeInvalidation) + ->and($store->get('api_query_version:orders:company-1'))->toBe(1); + + ApiModelCache::invalidateModelCache($model, 'company-1'); + + expect(ApiModelCache::cacheQueryResult($model, $request, fn () => collect(['fresher'])))->toEqual(collect(['fresher'])) + ->and(ApiModelCache::getCacheStatus())->toBe('MISS') + ->and($store->get('api_query_version:orders:company-1'))->toBe(2); +}); + +test('api model cache does not serve model or relationship lookups cached before an invalidation when the tag flush leaves them behind', function () { + $store = api_model_cache_fixture(); + $store->flushRemovesValues = false; + $model = api_model_cache_model(); + + ApiModelCache::cacheModel($model, 'order-1', fn () => ['status' => 'stale'], ['payload']); + ApiModelCache::cacheRelationship($model, 'payload', fn () => ['name' => 'stale']); + $modelKeyBefore = ApiModelCache::generateModelCacheKey($model, 'order-1', ['payload']); + $relationKeyBefore = ApiModelCache::generateRelationshipCacheKey($model, 'payload'); + + ApiModelCache::invalidateModelCache($model, 'company-1'); + + expect(ApiModelCache::cacheModel($model, 'order-1', fn () => ['status' => 'fresh'], ['payload']))->toBe(['status' => 'fresh']) + ->and(ApiModelCache::getCacheStatus())->toBe('MISS') + ->and(ApiModelCache::getCacheKey())->not->toBe($modelKeyBefore) + ->and(ApiModelCache::cacheRelationship($model, 'payload', fn () => ['name' => 'fresh']))->toBe(['name' => 'fresh']) + ->and(ApiModelCache::getCacheStatus())->toBe('MISS') + ->and(ApiModelCache::getCacheKey())->not->toBe($relationKeyBefore) + ->and($store->get('api_model_version:orders'))->toBe(1); + + // Lookups know no company, so a write in any company moves the table's model version + ApiModelCache::invalidateModelCache($model, null); + + expect(ApiModelCache::cacheModel($model, 'order-1', fn () => ['status' => 'fresher'], ['payload']))->toBe(['status' => 'fresher']) + ->and(ApiModelCache::cacheRelationship($model, 'payload', fn () => ['name' => 'fresher']))->toBe(['name' => 'fresher']) + ->and($store->get('api_model_version:orders'))->toBe(2); +}); + +test('api model cache seeds versions on stores whose increment does not create missing keys', function () { + $store = api_model_cache_fixture(); + $store->flushRemovesValues = false; + $store->incrementCreatesMissing = false; + $model = api_model_cache_model(); + $request = api_model_cache_request(['status' => 'active']); + + ApiModelCache::cacheQueryResult($model, $request, fn () => collect(['stale'])); + ApiModelCache::invalidateModelCache($model, 'company-1'); + + expect($store->get('api_query_version:orders:company-1'))->toBe(1) + ->and($store->get('api_model_version:orders'))->toBe(1) + ->and(ApiModelCache::cacheQueryResult($model, $request, fn () => collect(['fresh'])))->toEqual(collect(['fresh'])); + + ApiModelCache::invalidateModelCache($model, 'company-1'); + + expect($store->get('api_query_version:orders:company-1'))->toBe(2) + ->and($store->get('api_model_version:orders'))->toBe(2); +}); + test('api model cache resolves tenant scope from authenticated user and request input fallbacks', function () { api_model_cache_fixture(); $model = api_model_cache_model(); @@ -594,9 +686,9 @@ public function gc(int $max_lifetime): int|false ], null); expect(ApiModelCache::generateQueryCacheKey($model, $userRequest)) - ->toStartWith('{api_query}:orders:company_company-from-user:v1:') + ->toStartWith('{api_query}:orders:company_company-from-user:v0:') ->and(ApiModelCache::generateQueryCacheKey($model, $inputRequest)) - ->toStartWith('{api_query}:orders:company_company-from-input:v1:'); + ->toStartWith('{api_query}:orders:company_company-from-input:v0:'); }); test('api model cache invalidation methods swallow tagged cache backend failures', function () { @@ -632,13 +724,13 @@ public function gc(int $max_lifetime): int|false expect($modelResult)->toBe(['fallback' => true]) ->and(ApiModelCache::getCacheStatus())->toBe('ERROR') - ->and(ApiModelCache::getCacheKey())->toBe('{api_model}:orders:order-1'); + ->and(ApiModelCache::getCacheKey())->toBe('{api_model}:orders:v0:order-1'); $relationshipResult = ApiModelCache::cacheRelationship($model, 'payload', fn () => ['relationship' => true]); expect($relationshipResult)->toBe(['relationship' => true]) ->and(ApiModelCache::getCacheStatus())->toBe('ERROR') - ->and(ApiModelCache::getCacheKey())->toBe('{api_relation}:orders:order-1:payload'); + ->and(ApiModelCache::getCacheKey())->toBe('{api_relation}:orders:v0:order-1:payload'); }); test('api model cache warmup logs failures without leaking exceptions', function () { @@ -686,7 +778,7 @@ public function gc(int $max_lifetime): int|false ->and($second->pluck('uuid')->all())->toBe(['order-2']) ->and($callbackCalls)->toBe(1) ->and(ApiModelCache::getCacheStatus())->toBe('HIT') - ->and(ApiModelCache::getCacheKey())->toStartWith('{api_query}:orders:company_company-1:v1:') + ->and(ApiModelCache::getCacheKey())->toStartWith('{api_query}:orders:company_company-1:v0:') ->and(ApiModelCacheTraitTestModel::$mutatedRequestQueries)->toHaveCount(1) ->and(ApiModelCacheTraitTestModel::$mutatedRequestQueries[0])->toMatchArray([ 'status' => 'active',