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/src/GraphQL/ResponseCache/DefaultCache.php b/src/GraphQL/ResponseCache/DefaultCache.php index 14a99190933..cad70f4e6cd 100644 --- a/src/GraphQL/ResponseCache/DefaultCache.php +++ b/src/GraphQL/ResponseCache/DefaultCache.php @@ -6,11 +6,15 @@ 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; +use WeakMap; class DefaultCache implements ResponseCache { + private ?WeakMap $generations = null; + public function get(Request $request) { $cached = Cache::get($this->getCacheKey($request)); @@ -66,11 +70,25 @@ 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($request).':'.md5($query).'_'.md5(json_encode($vars)); + } + + 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 'gql-cache:generation'; } public function handleInvalidationEvent(Event $event) { + Cache::forget($this->getGenerationKey()); + $this->getTrackedResponses()->each(function ($key) { Cache::forget($key); }); 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); diff --git a/tests/Feature/GraphQL/RequestCacheTest.php b/tests/Feature/GraphQL/RequestCacheTest.php index 97f6c2927ff..61003dfb9a2 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,74 @@ 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 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() { @@ -213,11 +288,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);