Do not invalidate and purge on every GET of an unstorable response

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`.

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
      *

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.
      *

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

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,