diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bc30dbe..e461090d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Deploy this version with a new `CacheVersionModule` version (an app that binds its own cache adapter rather than this package's `Storage*Module`s must clear its pools instead): entries written by earlier versions may hold per-user content, and an earlier version reading this version's donut cache shows raw `[le2:...]` placeholders during a rolling deploy. - A `#[Cacheable]`/`#[CacheableResponse]`/`#[DonutCache]` response whose own code, including a `$headers` property default, sets `Cache-Control` to `private` or `no-store` is not stored server-side. To keep server-side caching with a client-facing `no-store`, declare `#[HttpCache(noStore: true)]` instead. +- Writing such an unstorable response invalidates the pools and purges the CDN only when an earlier write left an entry for its URI, not on every GET. - `ResourceDonut::FORMAT` is no longer public; use `ResourceDonut::placeholder()`. - Requires `koriym/semantic-logger` `^0.11`. Written logs use its `log` document shape, and diagnostic `kind` is now `code`. - Requires `bear/resource` `^1.22.4`. diff --git a/src/DonutRepository.php b/src/DonutRepository.php index fe15dbdb..7964f68d 100644 --- a/src/DonutRepository.php +++ b/src/DonutRepository.php @@ -156,8 +156,7 @@ private function doPutStatic(ResourceObject $ro, int|null $ttl, int|null $sMaxAg $unstorable = $this->declaredCacheControl->reason($ro); $surrogateKeys = $this->setHeaders($keys, $ro, $sMaxAge); $this->logCdnHeaders($ro); - $this->logger->event(new PreWriteCleanupContext((string) $ro->uri)); - $this->resourceStorage->invalidateTags([($this->uriTag)($ro->uri)]); + $this->cleanUpBeforeWrite($ro, $unstorable); if ($unstorable !== null) { $this->logger->event(new PutSkippedContext((string) $ro->uri, $unstorable)); @@ -186,8 +185,7 @@ private function doPutDonut(ResourceObject $ro, int|null $donutTtl): void } $this->logCdnHeaders($ro); - $this->logger->event(new PreWriteCleanupContext((string) $ro->uri)); - $this->resourceStorage->invalidateTags([($this->uriTag)($ro->uri)]); + $this->cleanUpBeforeWrite($ro, $unstorable); if ($unstorable !== null) { $this->logger->event(new PutSkippedContext((string) $ro->uri, $unstorable)); @@ -197,6 +195,16 @@ private function doPutDonut(ResourceObject $ro, int|null $donutTtl): void $this->resourceStorage->saveDonut($ro->uri, $donut, $donutTtl, $templateKeys); } + private function cleanUpBeforeWrite(ResourceObject $ro, string|null $unstorable): void + { + if ($unstorable !== null && $this->resourceStorage->get($ro->uri) === null && $this->resourceStorage->getDonut($ro->uri) === null) { + return; + } + + $this->logger->event(new PreWriteCleanupContext((string) $ro->uri)); + $this->resourceStorage->invalidateTags([($this->uriTag)($ro->uri)]); + } + /** * Register body children the page's onGet() already ran as dependencies, before the template keys read the header * diff --git a/src/QueryRepository.php b/src/QueryRepository.php index 2624688c..ffd4af23 100644 --- a/src/QueryRepository.php +++ b/src/QueryRepository.php @@ -69,11 +69,9 @@ public function put(ResourceObject $ro) private function doPut(ResourceObject $ro): bool { - // The writer knows its own purpose: this deleteEtag clears the entry about to be - // rewritten below. The marker records that, so readers never have to infer - // cleanup-vs-invalidation from tag correlation. - $this->logger->event(new PreWriteCleanupContext((string) $ro->uri)); - $this->storage->deleteEtag($ro->uri); + // Before headerSetter mutates the header + $unstorable = $this->declaredCacheControl->reason($ro); + $this->cleanUpBeforeWrite($ro, $unstorable); if ($ro->code === 200) { $this->setCacheDependency($ro); } @@ -93,10 +91,6 @@ private function doPut(ResourceObject $ro): bool $this->logger->event($this->cachePolicy($ro, $cacheable, $ttl)); } - // Checked against the still-unmutated header, before headerSetter runs the identical - // check - a dynamically set Cache-Control trips this; #[HttpCache]'s own value is exempt. - $unstorable = $this->declaredCacheControl->reason($ro); - ($this->headerSetter)($ro, $ttl, $httpCache); if ($unstorable !== null) { @@ -118,6 +112,19 @@ private function doPut(ResourceObject $ro): bool return $this->storage->saveValue($ro, $ttl); } + private function cleanUpBeforeWrite(ResourceObject $ro, string|null $unstorable): void + { + if ($unstorable !== null && $this->storage->get($ro->uri) === null) { + return; + } + + // The writer knows its own purpose: this deleteEtag clears the entry about to be + // rewritten below. The marker records that, so readers never have to infer + // cleanup-vs-invalidation from tag correlation. + $this->logger->event(new PreWriteCleanupContext((string) $ro->uri)); + $this->storage->deleteEtag($ro->uri); + } + /** * The declaration that decided the lifetime, as declared. * diff --git a/tests/DonutRepositoryTest.php b/tests/DonutRepositoryTest.php index 11f8798d..ae78e210 100644 --- a/tests/DonutRepositoryTest.php +++ b/tests/DonutRepositoryTest.php @@ -12,6 +12,7 @@ use BEAR\Resource\Uri; use Koriym\SemanticLogger\SemanticLoggerInterface; use Madapaja\TwigModule\TwigModule; +use Override; use PHPUnit\Framework\TestCase; use Ray\Di\AbstractModule; use Ray\Di\Injector; @@ -57,11 +58,24 @@ public function setUp(): void parent::setUp(); } - private function getInjector(): Injector + private function getInjector(PurgerInterface|null $purger = null): Injector { $namespace = 'FakeVendor\HelloWorld'; $module = new FakeEtagPoolModule(ModuleFactory::getInstance($namespace)); $module->override(new TwigModule([dirname(__DIR__) . '/tests/Fake/fake-app/var/templates'])); + if ($purger !== null) { + $module->override(new class ($purger) extends AbstractModule { + public function __construct(private readonly PurgerInterface $purger) + { + } + + #[Override] + protected function configure(): void + { + $this->bind(PurgerInterface::class)->toInstance($this->purger); + } + }); + } return new Injector($module, __DIR__ . '/tmp'); } @@ -186,6 +200,41 @@ public function testDonutCacheTemplateSkipsSaveWhenThePageWritesPrivateIntoItsOw $this->assertNotNull(self::eventContextJsonOf($tree2, 'cache_miss')); } + public function testRepeatedUnstorableDonutGetDoesNotPurge(): void + { + foreach (['page://self/html/unstorable-donut', 'page://self/html/unstorable-donut-cache-template'] as $uri) { + $purger = new FakeCountingPurger(); + $resource = $this->getInjector($purger)->getInstance(ResourceInterface::class); + + for ($i = 0; $i < 3; $i++) { + $resource->get($uri); + } + + $this->assertSame([], $purger->tags, $uri); + } + } + + public function testUnstorableDonutWriteStillDropsAnEarlierEntry(): void + { + $purger = new FakeCountingPurger(); + $injector = $this->getInjector($purger); + $resource = $injector->getInstance(ResourceInterface::class); + $storage = $injector->getInstance(ResourceStorageInterface::class); + $donutRepository = $injector->getInstance(DonutRepositoryInterface::class); + $uri = new Uri('page://self/html/unstorable-donut?unstorable=0'); + + $ro = $resource->get((string) $uri); + $this->assertNotNull($storage->get($uri)); + $this->assertNotNull($storage->getDonut($uri)); + $purger->reset(); + + $ro->headers[Header::CACHE_CONTROL] = 'private'; + $donutRepository->putStatic($ro); + $this->assertNull($storage->get($uri)); + $this->assertNull($storage->getDonut($uri)); + $this->assertNotSame([], $purger->tags); + } + public function testDonutCacheWithAMultiParamEmbedRendersAndRefreshesBothQueryParams(): void { // End-to-end through the real #[Embed] pipeline (URI-template expansion, not a diff --git a/tests/QueryRepositoryTest.php b/tests/QueryRepositoryTest.php index 558f33c6..e8ed2322 100644 --- a/tests/QueryRepositoryTest.php +++ b/tests/QueryRepositoryTest.php @@ -17,6 +17,7 @@ use FakeVendor\HelloWorld\Resource\App\User\Profile; use FakeVendor\HelloWorld\Resource\Page\None; use Koriym\SemanticLogger\SemanticLoggerInterface; +use Override; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemPoolInterface; use Ray\Di\AbstractModule; @@ -311,6 +312,53 @@ public function testRuntimePrivateResponseSkipsStoreAndOrdinaryResponsesStillSto $this->assertNotContains('cache_hit', self::collectTypes($tree2)); } + public function testRepeatedUnstorableGetDoesNotPurge(): void + { + $purger = new FakeCountingPurger(); + $resource = $this->injectorWithPurger($purger)->getInstance(ResourceInterface::class); + + for ($i = 0; $i < 3; $i++) { + $resource->get('app://self/unstorable-value'); + } + + $this->assertSame([], $purger->tags); + } + + public function testUnstorablePutStillDropsAnEarlierEntry(): void + { + $purger = new FakeCountingPurger(); + $injector = $this->injectorWithPurger($purger); + $resource = $injector->getInstance(ResourceInterface::class); + $repository = $injector->getInstance(QueryRepositoryInterface::class); + $uri = 'app://self/unstorable-value?unstorable=0'; + $ro = $resource->get($uri); + $this->assertNotNull($repository->get(new Uri($uri))); + $purger->reset(); + + $ro->headers[Header::CACHE_CONTROL] = 'private'; + $this->assertFalse($repository->put($ro)); + $this->assertNull($repository->get(new Uri($uri))); + $this->assertNotSame([], $purger->tags); + } + + private function injectorWithPurger(PurgerInterface $purger): Injector + { + $module = new FakeEtagPoolModule(ModuleFactory::getInstance('FakeVendor\HelloWorld')); + $module->override(new class ($purger) extends AbstractModule { + public function __construct(private readonly PurgerInterface $purger) + { + } + + #[Override] + protected function configure(): void + { + $this->bind(PurgerInterface::class)->toInstance($this->purger); + } + }); + + return new Injector($module, __DIR__ . '/tmp'); + } + public function testCacheableWithNoHttpCacheStillStores(): void { // #[NoHttpCache] declares its own Cache-Control (private, no-store, no-cache,