From 973661c0115cb3b4a4083498bdf35eadd1807a93 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Thu, 29 Oct 2026 10:12:25 +0000 Subject: [PATCH 1/3] key graphql response cache under a generation so concurrent requests can't escape invalidation --- src/GraphQL/ResponseCache/DefaultCache.php | 17 +++++- tests/Feature/GraphQL/RequestCacheTest.php | 69 +++++++++++++++++++++- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/GraphQL/ResponseCache/DefaultCache.php b/src/GraphQL/ResponseCache/DefaultCache.php index 14a99190933..35f695a4aa6 100644 --- a/src/GraphQL/ResponseCache/DefaultCache.php +++ b/src/GraphQL/ResponseCache/DefaultCache.php @@ -6,11 +6,14 @@ use Illuminate\Http\Request; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Str; use Statamic\Contracts\GraphQL\ResponseCache; use Statamic\Events\Event; class DefaultCache implements ResponseCache { + private ?string $generation = null; + public function get(Request $request) { $cached = Cache::get($this->getCacheKey($request)); @@ -66,11 +69,23 @@ protected function getCacheKey(Request $request) $query = $request->input('query'); $vars = $request->input('variables'); - return 'gql-cache:'.md5($query).'_'.md5(json_encode($vars)); + return 'gql-cache:'.$this->getGeneration().':'.md5($query).'_'.md5(json_encode($vars)); + } + + private function getGeneration(): string + { + return $this->generation ??= Cache::rememberForever($this->getGenerationKey(), fn () => Str::random()); + } + + private function getGenerationKey(): string + { + return 'gql-cache:generation'; } public function handleInvalidationEvent(Event $event) { + Cache::forget($this->getGenerationKey()); + $this->getTrackedResponses()->each(function ($key) { Cache::forget($key); }); diff --git a/tests/Feature/GraphQL/RequestCacheTest.php b/tests/Feature/GraphQL/RequestCacheTest.php index 97f6c2927ff..b3bb400829a 100644 --- a/tests/Feature/GraphQL/RequestCacheTest.php +++ b/tests/Feature/GraphQL/RequestCacheTest.php @@ -3,9 +3,13 @@ namespace Tests\Feature\GraphQL; use Facades\Tests\Factories\EntryFactory; +use Illuminate\Cache\Events\WritingKey; +use Illuminate\Http\JsonResponse; +use Illuminate\Http\Request; use Illuminate\Support\Carbon; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Event; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; @@ -13,6 +17,7 @@ use Statamic\Facades\GraphQL; use Statamic\Facades\Token; use Statamic\GraphQL\Queries\PingQuery; +use Statamic\GraphQL\ResponseCache\DefaultCache; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; @@ -80,8 +85,10 @@ public function it_caches_endpoint_using_configured_expiry() config(['statamic.graphql.cache.expiry' => 13]); app()->instance('request-tracking', $requests = Collection::make()); + Cache::forever('gql-cache:generation', 'abc'); + // md5('{one}')_md5(json_encode(['foo' => 'bar'])) - $key = 'gql-cache:0737a8fd85124abab1445165f4753936_9bb58f26192e4ba00f01e2e7b136bbd8'; + $key = 'gql-cache:abc:0737a8fd85124abab1445165f4753936_9bb58f26192e4ba00f01e2e7b136bbd8'; Carbon::setTestNow(now()); $this->assertFalse(Cache::has($key)); @@ -176,6 +183,62 @@ public function it_busts_whole_cache_when_content_is_saved() $this->assertEquals('test', Cache::get('completely-unrelated-thing')); } + /** + * @see https://github.com/statamic/cms/issues/15523 + */ + #[Test] + public function it_busts_responses_cached_concurrently_when_content_is_saved() + { + $entry = EntryFactory::collection('test')->create(); + + $requestOne = Request::create('/graphql', 'POST', ['query' => '{one}']); + $requestTwo = Request::create('/graphql', 'POST', ['query' => '{two}']); + + $workerOne = new DefaultCache; + $workerTwo = new DefaultCache; + + $workerOne->get($requestOne); + $workerTwo->get($requestTwo); + + $workerTwoIsWriting = false; + + Event::listen(WritingKey::class, function () use (&$workerTwoIsWriting, $workerTwo, $requestTwo) { + if ($workerTwoIsWriting) { + return; + } + + $workerTwoIsWriting = true; + $workerTwo->put($requestTwo, new JsonResponse(['data' => ['two' => 'two']])); + }); + + $workerOne->put($requestOne, new JsonResponse(['data' => ['one' => 'one']])); + + $entry->save(); + + $this->assertNull((new DefaultCache)->get($requestOne)); + $this->assertNull((new DefaultCache)->get($requestTwo)); + } + + /** + * @see https://github.com/statamic/cms/issues/15523 + */ + #[Test] + public function it_doesnt_cache_a_stale_response_when_content_is_saved_mid_request() + { + $entry = EntryFactory::collection('test')->create(); + + $request = Request::create('/graphql', 'POST', ['query' => '{one}']); + + $worker = new DefaultCache; + $worker->get($request); + + $entry->save(); + + $worker->put($request, new JsonResponse(['data' => ['one' => 'one']])); + + $this->assertNull((new DefaultCache)->get($request)); + } + #[Test] public function it_ignores_configured_events() { @@ -213,11 +276,13 @@ public function it_ignores_configured_events() #[Test] public function it_caches_response_data_as_primitives() { + Cache::forever('gql-cache:generation', 'abc'); + app()->instance('request-tracking', $requests = Collection::make()); $this->post('/graphql', ['query' => '{one}']); - $key = 'gql-cache:'.md5('{one}').'_'.md5(json_encode(null)); + $key = 'gql-cache:abc:'.md5('{one}').'_'.md5(json_encode(null)); $cached = Cache::get($key); $this->assertIsArray($cached); From 98b10d7a86dc06fc257a261cbad2c6177797babe Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 30 Oct 2026 16:02:35 +1100 Subject: [PATCH 2/3] key rest api response cache under a generation so concurrent requests can't escape invalidation The generation is memoized per request rather than per instance, since the REST cacher is resolved once per app by ApiCacheManager and would otherwise hold a stale generation in long-lived processes. Co-Authored-By: Claude Opus 5.5 --- src/API/Cachers/DefaultCacher.php | 20 +++- tests/API/CacherTest.php | 148 +++++++++++++++++++++--------- 2 files changed, 125 insertions(+), 43 deletions(-) diff --git a/src/API/Cachers/DefaultCacher.php b/src/API/Cachers/DefaultCacher.php index 2679738ad83..4959df394de 100644 --- a/src/API/Cachers/DefaultCacher.php +++ b/src/API/Cachers/DefaultCacher.php @@ -5,11 +5,15 @@ use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Str; use Statamic\API\AbstractCacher; use Statamic\Events\Event; +use WeakMap; class DefaultCacher extends AbstractCacher { + private ?WeakMap $generations = null; + /** * {@inheritdoc} */ @@ -45,6 +49,8 @@ public function put(Request $request, JsonResponse $response) */ public function handleInvalidationEvent(Event $event) { + Cache::forget($this->getGenerationKey()); + $this->getTrackedResponses()->each(function ($key) { Cache::forget($key); }); @@ -106,6 +112,18 @@ protected function getCacheKey(Request $request) $key = md5(str_replace($domain, '', $fullUrl)); - return $this->normalizeKey($key); + return $this->normalizeKey($this->getGeneration($request).':'.$key); + } + + private function getGeneration(Request $request): string + { + $this->generations ??= new WeakMap; + + return $this->generations[$request] ??= Cache::rememberForever($this->getGenerationKey(), fn () => Str::random()); + } + + private function getGenerationKey(): string + { + return $this->normalizeKey('generation'); } } diff --git a/tests/API/CacherTest.php b/tests/API/CacherTest.php index 40fe3bfe7fd..4955b8a681e 100644 --- a/tests/API/CacherTest.php +++ b/tests/API/CacherTest.php @@ -3,14 +3,17 @@ namespace Tests\API; use Facades\Tests\Factories\EntryFactory; +use Illuminate\Cache\Events\WritingKey; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Event as EventFacade; use Orchestra\Testbench\Attributes\DefineEnvironment; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Statamic\API\AbstractCacher; +use Statamic\API\Cachers\DefaultCacher; use Statamic\Events\EntrySaved; use Statamic\Events\Event; use Statamic\Facades; @@ -50,8 +53,7 @@ public function it_caches_endpoint_using_default_cacher() ['id' => 'apple'], ]]); - $hash = md5($endpoint); - $cacheKey = "api-cache:$hash"; + $cacheKey = $this->cacheKey($endpoint); $this->assertTrue(Cache::has($cacheKey)); $this->assertEquals([$cacheKey], Cache::get('api-cache:tracked-responses')); @@ -83,8 +85,7 @@ public function it_bypasses_cache_when_using_a_valid_token($endpoint, $headers) ['id' => 'apple'], ]]); - $hash = md5($endpoint); - $this->assertFalse(Cache::has("api-cache:$hash")); + $this->assertFalse(Cache::has($this->cacheKey($endpoint))); $this->assertNull(Cache::get('api-cache:tracked-responses')); } @@ -104,8 +105,7 @@ public function it_doesnt_bypass_cache_when_using_an_invalid_token($endpoint, $h ['id' => 'apple'], ]]); - $hash = md5($endpoint); - $this->assertTrue(Cache::has($cacheKey = "api-cache:$hash")); + $this->assertTrue(Cache::has($cacheKey = $this->cacheKey($endpoint))); $this->assertEquals([$cacheKey], Cache::get('api-cache:tracked-responses')); } @@ -126,13 +126,13 @@ public function it_caches_endpoint_using_configured_expiry() $this->makeEntry('apple')->save(); $endpoint = '/api/collections/articles/entries'; - $hash = md5($endpoint); - $cacheKey = "api-cache:$hash"; Carbon::setTestNow(now()); $this->get($endpoint)->assertOk(); + $cacheKey = $this->cacheKey($endpoint); + $this->assertTrue(Cache::has($cacheKey)); Carbon::setTestNow(now()->addMinutes(14)); @@ -151,8 +151,7 @@ public function it_caches_endpoint_with_query_params() ['id' => 'apple'], ]]); - $hash = md5($endpoint); - $cacheKey = "api-cache:$hash"; + $cacheKey = $this->cacheKey($endpoint); $this->assertTrue(Cache::has($cacheKey)); $this->assertEquals([$cacheKey], Cache::get('api-cache:tracked-responses')); @@ -175,15 +174,12 @@ public function it_caches_multiple_endpoints() ['id' => 'apple'], ]]); - $hashOne = md5($endpointOne); - $hashTwo = md5($endpointTwo); - $this->assertTrue(Cache::has("api-cache:$hashOne")); - $this->assertTrue(Cache::has("api-cache:$hashTwo")); + $keyOne = $this->cacheKey($endpointOne); + $keyTwo = $this->cacheKey($endpointTwo); + $this->assertTrue(Cache::has($keyOne)); + $this->assertTrue(Cache::has($keyTwo)); - $cachedResponses = [ - "api-cache:$hashOne", - "api-cache:$hashTwo", - ]; + $cachedResponses = [$keyOne, $keyTwo]; $this->assertEquals($cachedResponses, Cache::get('api-cache:tracked-responses')); } @@ -206,16 +202,17 @@ public function it_busts_whole_cache_when_content_is_saved() ['id' => 'apple'], ]]); - $hashOne = md5($endpointOne); - $hashTwo = md5($endpointTwo); - $this->assertTrue(Cache::has("api-cache:$hashOne")); - $this->assertTrue(Cache::has("api-cache:$hashTwo")); + $keyOne = $this->cacheKey($endpointOne); + $keyTwo = $this->cacheKey($endpointTwo); + $this->assertTrue(Cache::has($keyOne)); + $this->assertTrue(Cache::has($keyTwo)); $this->assertCount(2, Cache::get('api-cache:tracked-responses')); $entry->save(); - $this->assertFalse(Cache::has("api-cache:$hashOne")); - $this->assertFalse(Cache::has("api-cache:$hashTwo")); + $this->assertFalse(Cache::has($keyOne)); + $this->assertFalse(Cache::has($keyTwo)); + $this->assertFalse(Cache::has('api-cache:generation')); $this->assertFalse(Cache::has('api-cache:tracked-responses')); } @@ -236,20 +233,91 @@ public function it_busts_whole_cache_when_unrelated_content_is_saved() ['id' => 'apple'], ]]); - $hashOne = md5($endpointOne); - $hashTwo = md5($endpointTwo); + $keyOne = $this->cacheKey($endpointOne); + $keyTwo = $this->cacheKey($endpointTwo); - $this->assertTrue(Cache::has("api-cache:$hashOne")); - $this->assertTrue(Cache::has("api-cache:$hashTwo")); + $this->assertTrue(Cache::has($keyOne)); + $this->assertTrue(Cache::has($keyTwo)); $this->assertCount(2, Cache::get('api-cache:tracked-responses')); Facades\Form::make('contact')->save(); - $this->assertFalse(Cache::has("api-cache:$hashOne")); - $this->assertFalse(Cache::has("api-cache:$hashTwo")); + $this->assertFalse(Cache::has($keyOne)); + $this->assertFalse(Cache::has($keyTwo)); + $this->assertFalse(Cache::has('api-cache:generation')); $this->assertFalse(Cache::has('api-cache:tracked-responses')); } + /** + * @see https://github.com/statamic/cms/issues/15523 + */ + #[Test] + public function it_busts_responses_cached_concurrently_when_content_is_saved() + { + $entry = $this->makeEntry('apple'); + $entry->save(); + + $requestOne = Request::create('/api/collections/articles/entries?query=one'); + $requestTwo = Request::create('/api/collections/articles/entries?query=two'); + + $workerOne = new DefaultCacher(['expiry' => 60]); + $workerTwo = new DefaultCacher(['expiry' => 60]); + + $workerOne->get($requestOne); + $workerTwo->get($requestTwo); + + $workerTwoIsWriting = false; + + EventFacade::listen(WritingKey::class, function () use (&$workerTwoIsWriting, $workerTwo, $requestTwo) { + if ($workerTwoIsWriting) { + return; + } + + $workerTwoIsWriting = true; + $workerTwo->put($requestTwo, new JsonResponse(['data' => 'two'])); + }); + + $workerOne->put($requestOne, new JsonResponse(['data' => 'one'])); + + $entry->save(); + + $this->assertNull((new DefaultCacher(['expiry' => 60]))->get($requestOne)); + $this->assertNull((new DefaultCacher(['expiry' => 60]))->get($requestTwo)); + } + + /** + * @see https://github.com/statamic/cms/issues/15523 + */ + #[Test] + public function it_doesnt_cache_a_stale_response_when_content_is_saved_mid_request() + { + $entry = $this->makeEntry('apple'); + $entry->save(); + + $request = Request::create('/api/collections/articles/entries'); + + $cacher = new DefaultCacher(['expiry' => 60]); + $cacher->get($request); + + $cacher->handleInvalidationEvent(new EntrySaved($entry)); + + $cacher->put($request, new JsonResponse(['data' => 'stale'])); + + $this->assertNull($cacher->get(Request::create('/api/collections/articles/entries'))); + } + + #[Test] + public function a_long_lived_cacher_uses_the_current_generation_for_each_request() + { + $cacher = new DefaultCacher(['expiry' => 60]); + + $cacher->put(Request::create('/api/collections/articles/entries'), new JsonResponse(['data' => 'old'])); + + Cache::forget('api-cache:generation'); + + $this->assertNull($cacher->get(Request::create('/api/collections/articles/entries'))); + } + #[Test] public function it_can_disable_default_cacher_by_setting_false_on_parent_cache_config() { @@ -263,10 +331,7 @@ public function it_can_disable_default_cacher_by_setting_false_on_parent_cache_c ['id' => 'apple'], ]]); - $hash = md5($endpoint); - $cacheKey = "api-cache:$hash"; - - $this->assertFalse(Cache::has($cacheKey)); + $this->assertFalse(Cache::has($this->cacheKey($endpoint))); $this->assertFalse(Cache::has('api-cache:tracked-responses')); } @@ -283,10 +348,7 @@ public function it_can_disable_default_cacher_by_setting_false_on_child_class_co ['id' => 'apple'], ]]); - $hash = md5($endpoint); - $cacheKey = "api-cache:$hash"; - - $this->assertFalse(Cache::has($cacheKey)); + $this->assertFalse(Cache::has($this->cacheKey($endpoint))); $this->assertFalse(Cache::has('api-cache:tracked-responses')); } @@ -304,10 +366,7 @@ public function it_can_use_custom_cacher() ['id' => 'apple'], ]]); - $hash = md5($endpoint); - $cacheKey = "api-cache:$hash"; - - $this->assertFalse(Cache::has($cacheKey)); + $this->assertFalse(Cache::has($this->cacheKey($endpoint))); $this->assertFalse(Cache::has('api-cache:tracked-responses')); $this->assertTrue(Cache::has('custom-cache')); @@ -320,6 +379,11 @@ public function it_can_use_custom_cacher() $this->assertFalse(Cache::has('custom-cache')); } + private function cacheKey($endpoint) + { + return 'api-cache:'.Cache::get('api-cache:generation').':'.md5($endpoint); + } + protected function setCustomExpiry($app) { $app->config->set('statamic.api.cache.expiry', 13); From 3babef189877bb4008cd0791d12da8856ea1bab9 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 30 Oct 2026 16:14:34 +1100 Subject: [PATCH 3/3] memoize graphql response cache generation per request for consistency with the rest api cacher This is purely for consistency, not necessity. The GraphQL cache is bound with bind() so each resolution is already a fresh instance, but the REST cacher is held for the app's lifetime by ApiCacheManager and has to memoize per request. Using the same approach in both avoids confusion about why they differ. Co-Authored-By: Claude Opus 5.5 --- src/GraphQL/ResponseCache/DefaultCache.php | 11 +++++++---- tests/Feature/GraphQL/RequestCacheTest.php | 12 ++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/GraphQL/ResponseCache/DefaultCache.php b/src/GraphQL/ResponseCache/DefaultCache.php index 35f695a4aa6..cad70f4e6cd 100644 --- a/src/GraphQL/ResponseCache/DefaultCache.php +++ b/src/GraphQL/ResponseCache/DefaultCache.php @@ -9,10 +9,11 @@ use Illuminate\Support\Str; use Statamic\Contracts\GraphQL\ResponseCache; use Statamic\Events\Event; +use WeakMap; class DefaultCache implements ResponseCache { - private ?string $generation = null; + private ?WeakMap $generations = null; public function get(Request $request) { @@ -69,12 +70,14 @@ protected function getCacheKey(Request $request) $query = $request->input('query'); $vars = $request->input('variables'); - return 'gql-cache:'.$this->getGeneration().':'.md5($query).'_'.md5(json_encode($vars)); + return 'gql-cache:'.$this->getGeneration($request).':'.md5($query).'_'.md5(json_encode($vars)); } - private function getGeneration(): string + private function getGeneration(Request $request): string { - return $this->generation ??= Cache::rememberForever($this->getGenerationKey(), fn () => Str::random()); + $this->generations ??= new WeakMap; + + return $this->generations[$request] ??= Cache::rememberForever($this->getGenerationKey(), fn () => Str::random()); } private function getGenerationKey(): string diff --git a/tests/Feature/GraphQL/RequestCacheTest.php b/tests/Feature/GraphQL/RequestCacheTest.php index b3bb400829a..61003dfb9a2 100644 --- a/tests/Feature/GraphQL/RequestCacheTest.php +++ b/tests/Feature/GraphQL/RequestCacheTest.php @@ -239,6 +239,18 @@ public function it_doesnt_cache_a_stale_response_when_content_is_saved_mid_reque $this->assertNull((new DefaultCache)->get($request)); } + #[Test] + public function a_long_lived_cache_uses_the_current_generation_for_each_request() + { + $cache = new DefaultCache; + + $cache->put(Request::create('/graphql', 'POST', ['query' => '{one}']), new JsonResponse(['data' => ['one' => 'old']])); + + Cache::forget('gql-cache:generation'); + + $this->assertNull($cache->get(Request::create('/graphql', 'POST', ['query' => '{one}']))); + } + #[Test] public function it_ignores_configured_events() {