From 8efa86ade4df6607993fd298d247a20e2db3bc7f Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 12 Oct 2026 20:10:29 +0800 Subject: [PATCH 1/2] fix(api-cache): first invalidation changes the list cache key A missing api_query_version key was read as 1 and Cache::increment creates it at 1, so the first invalidation for a table/company left list cache keys unchanged and stale lists were served until a second write. Missing versions now read as 0, and stores whose increment does not create missing keys get the version seeded. The version bump is what actually invalidates list caches: on Laravel 10 with phpredis >= 6.1, RedisTagSet::entries() starts ZSCAN at cursor '0', which phpredis treats as a finished scan, so the tag flush deletes the tag sets but leaves the tagged values in Redis until their TTL. --- src/Support/ApiModelCache.php | 39 ++++++++--- tests/Unit/Support/ApiModelCacheTest.php | 85 ++++++++++++++++++++---- 2 files changed, 102 insertions(+), 22 deletions(-) diff --git a/src/Support/ApiModelCache.php b/src/Support/ApiModelCache.php index f6c3e069..2453592d 100644 --- a/src/Support/ApiModelCache.php +++ b/src/Support/ApiModelCache.php @@ -17,6 +17,13 @@ * - Automatic cache invalidation * - Multi-tenancy support * - Cache tagging for efficient bulk invalidation + * + * List caches are invalidated by a per table/company query version embedded in + * the cache key, not by the tag flush. 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 +96,21 @@ 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}"; + } + /** * Generate a cache key for a single model instance. * @@ -166,9 +180,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 @@ -323,10 +336,15 @@ public static function invalidateModelCache(Model $model, ?string $companyUuid = 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); + // This ensures the cache key changes after invalidation, and it is what + // actually hides stale list caches (see the class docblock). A missing + // version reads as 0 and INCR creates it at 1, so the first invalidation + // also changes the key. Stores whose increment does not create missing + // keys (database, memcached) return false, so the version is seeded. + $versionKey = static::queryVersionKey($model->getTable(), $companyUuid); + if (Cache::increment($versionKey) === false) { + Cache::add($versionKey, 1) || Cache::increment($versionKey); + } // Generate tags for BOTH model and query caches // Model caches: single-record lookups @@ -339,7 +357,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 this removes the tag sets but not the values (see the class + // docblock); the version bump above is what invalidates list 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..be205432 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 @@ -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++; @@ -581,6 +601,47 @@ 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 seeds the query version 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(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); +}); + 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 +655,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 () { @@ -686,7 +747,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', From f4a20cb1a7ad18a2ac5ba99dc19b3a51ee56d671 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Mon, 12 Oct 2026 20:44:13 +0800 Subject: [PATCH 2/2] fix(api-cache): version model and relationship cache keys Single model and relationship caches had no version, so with the Redis tag flush leaving values behind they were served stale until their TTL after a write. Their keys now carry a per table api_model_version that invalidateModelCache() bumps alongside the query version. It is per table because findCached/findByPublicIdCached/loadCached are keyed by id and know no company. The increment-or-seed logic is shared via bumpVersion(). --- src/Support/ApiModelCache.php | 60 +++++++++++++++++------- tests/Unit/Support/ApiModelCacheTest.php | 47 +++++++++++++++---- 2 files changed, 81 insertions(+), 26 deletions(-) diff --git a/src/Support/ApiModelCache.php b/src/Support/ApiModelCache.php index 2453592d..9ce16762 100644 --- a/src/Support/ApiModelCache.php +++ b/src/Support/ApiModelCache.php @@ -18,8 +18,10 @@ * - Multi-tenancy support * - Cache tagging for efficient bulk invalidation * - * List caches are invalidated by a per table/company query version embedded in - * the cache key, not by the tag flush. On Laravel 10 with phpredis >= 6.1, + * 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 @@ -111,6 +113,28 @@ protected static function queryVersionKey(string $table, ?string $companyUuid): 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. * @@ -120,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}"; } /** @@ -130,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}"; } /** @@ -335,16 +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, and it is what - // actually hides stale list caches (see the class docblock). A missing - // version reads as 0 and INCR creates it at 1, so the first invalidation - // also changes the key. Stores whose increment does not create missing - // keys (database, memcached) return false, so the version is seeded. - $versionKey = static::queryVersionKey($model->getTable(), $companyUuid); - if (Cache::increment($versionKey) === false) { - Cache::add($versionKey, 1) || 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 @@ -357,8 +381,8 @@ public static function invalidateModelCache(Model $model, ?string $companyUuid = Cache::tags($modelTags)->flush(); // Flush query-level caches (collections, lists) - // On Redis this removes the tag sets but not the values (see the class - // docblock); the version bump above is what invalidates list caches + // 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 be205432..047067fb 100644 --- a/tests/Unit/Support/ApiModelCacheTest.php +++ b/tests/Unit/Support/ApiModelCacheTest.php @@ -426,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']); }); @@ -520,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; @@ -539,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 () { @@ -588,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']); @@ -624,7 +625,35 @@ public function gc(int $max_lifetime): int|false ->and($store->get('api_query_version:orders:company-1'))->toBe(2); }); -test('api model cache seeds the query version on stores whose increment does not create missing keys', function () { +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; @@ -635,11 +664,13 @@ public function gc(int $max_lifetime): int|false 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); + 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 () { @@ -693,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 () {