Pre-release fixes for 1.18.0

CHANGELOG.md

@@ -8,15 +8,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
-- `UncacheableMarkInterface`/`UncacheableMark`: a request-scoped service a producer of a per-user value (a CSRF token issuer, etc.) calls to mark the current response unstorable - issue #222. A `#[Cacheable]` or `#[CacheableResponse]` write that is marked, or whose `Cache-Control` already says `no-store`/`private`, is skipped (`put_skipped{reason: unstorable}`), and the response gets `Cache-Control: private, no-store` plus `Pragma: no-cache` and no CDN lifetime header instead. `$module->override(new DevUnstorableGateModule())` turns the skip into a thrown `UnstorableResponseDetected` during development - `install()` does not work here, since `QueryRepositoryModule` already binds `UnstorableGateInterface` and `install()` never replaces a binding the installer already holds. Existing cache entries written before this change may hold per-user content - purge on deploy. `UncacheableMark` is bound `Scope::SINGLETON` for the request's one container; a long-running worker (Swoole, RoadRunner) reusing a container across requests must rebind it per request, the same way it already must for `ServerContextInterface`.
+- `UncacheableMarkInterface::mark($reason)`: code that produces a per-user value (a CSRF token, etc.) marks the current response unstorable; it is then not cached (`put_skipped{reason: unstorable}`) and is sent with `Cache-Control: private, no-store` and `Pragma: no-cache` and no CDN headers (#222). A mark also skips every `#[Cacheable]`/`#[CacheableResponse]` write that happens later in the same request, including an unrelated sibling's (#234).
+- `UncacheableMark` is request-scoped: a long-running worker (Swoole, RoadRunner) that reuses one container across requests must rebind it per request, as it already must for `ServerContextInterface`.
+- `DevUnstorableGateModule`: `$module->override(new DevUnstorableGateModule())` turns such a skip into a thrown `Exception\UnstorableResponseDetected` during development (`install()` has no effect here).
 
 ### 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]` response whose own code sets `Cache-Control` to `private` or `no-store` is no longer stored server-side; its header is kept and CDN headers are removed. A value declared by `#[HttpCache]`/`#[NoHttpCache]` is still stored.
+- `ResourceDonut::FORMAT` is no longer public; use `ResourceDonut::placeholder()`. `ResourceDonut::refresh()` takes an additional `UncacheableMarkInterface` argument.
 - Requires `koriym/semantic-logger` `^0.11`. Written logs use its `log` document shape, and diagnostic `kind` is now `code`.
 - `KeepMutationsAndFailures` also keeps a session cut at the record limit (`semantic_logger_truncated`): a `command` past the limit leaves no other trace.
 
 ### Fixed
 - `LogFileWriter`: two sessions flushed in the same microsecond no longer overwrite each other. The timestamped filename now carries a random suffix, so both survive (#221).
-- A `#[DonutCache]` page embedding an `#[Embed]` hole whose URI carries two or more query parameters leaked a raw `[le:...]` placeholder to the client on the first request and could throw `BEAR\Resource\Exception\ParameterException` on the next - fixed (#232) by moving new placeholders to a `[le2:...]` marker. No action needed for existing entries: an old-form `[le:...]` entry keeps working exactly as before; only an entry already corrupted by this bug before upgrading should be purged or left to expire.
+- A `#[DonutCache]` page embedding an `#[Embed]` hole whose URI carried two or more query parameters leaked a raw `[le:...]` placeholder to the client and could throw `ParameterException` on the next request - fixed by moving new placeholders to a `[le2:...]` marker (#232).
 
 ## [1.17.0] - 2026-09-17
 

README.md

@@ -17,7 +17,7 @@
 - **CDN Integration**: Seamlessly integrates with modern CDNs (e.g., Fastly, Akamai) for shared cache management.
 - **Conditional Requests with ETag Support**: Reduces network overhead by leveraging `ETag` and `304 Not Modified` responses.
 - **Distributed Cache Support**: Works with server-side caches (e.g., Redis, APC), shared caches (e.g., CDNs), and client-side caches.
-- **Unstorable-Response Guard**: A producer of a per-user value (a CSRF token, etc.) can mark the current response unstorable via `UncacheableMarkInterface`, so it is never written to the shared cache ([#222](https://github.com/bearsunday/BEAR.QueryRepository/issues/222)); the mark is per-request state and must be rebound each request under a long-running worker (Swoole, RoadRunner). This guard only stops *new* writes — deploying it does not clean cache entries already stored before the upgrade, which may hold per-user content; purge them as part of the deploy.
+- **Unstorable-Response Guard**: A producer of a per-user value (a CSRF token, etc.) can mark the current response unstorable via `UncacheableMarkInterface`, so it is never written to the shared cache ([#222](https://github.com/bearsunday/BEAR.QueryRepository/issues/222)); the mark is per-request state and must be rebound each request under a long-running worker (Swoole, RoadRunner). This guard only stops *new* writes — deploy a new `CacheVersionModule` version alongside it so this release starts from a fresh cache namespace: entries written by earlier versions may hold per-user content, and an earlier version reading this version's donut cache would otherwise show raw placeholders during a rolling deploy. `CacheVersionModule` only namespaces pools built through this package's own `Storage*Module`s or a `#[CacheNamespace]`-aware adapter; an app binding its own cache adapter directly must clear those pools instead.
 
 ## Documentation
 

src/DonutRepository.php

@@ -263,7 +263,7 @@ private function refreshDonut(ResourceObject $ro): ResourceObject|null
 
         $this->logger->event(new CacheHitContext('donut'));
         $this->logger->event(new RefreshDonutContext((string) $ro->uri));
-        $donut->refresh($this->resource, $ro);
+        $donut->refresh($this->resource, $ro, $this->mark);
         $surrogateKeys = $ro->headers[Header::SURROGATE_KEY] ?? '';
 
         // refresh() restores the stored headers before returning, discarding anything a
@@ -278,12 +278,14 @@ private function refreshDonut(ResourceObject $ro): ResourceObject|null
             // cached and the page is never stored as a rendered view, so there is no
             // page-level entry to save after the refresh - and this package never hands
             // the CDN a lifetime header of its own for this kind, mirroring doPutDonut's
-            // MISS-path exactly: only overwrite the restored headers with Cache-Control:
-            // private, no-store when a hole marks itself on THIS recomposition, never add
-            // a lifetime header that was never there (issue #222 - a hole marking itself
-            // must still be caught on a later, recomposed hit, not just the one that first
-            // wrote the template; but a storable template-only hit must stay exactly as
-            // storable as the MISS that created it, no new CDN lifetime invented here).
+            // MISS-path exactly: only touch the restored headers (HeaderSetter overwrites
+            // Cache-Control/Pragma when a mark fired, keeps a hand-set private/no-store
+            // header as-is otherwise - issue #222) when a hole marks itself on THIS
+            // recomposition, never add a lifetime header that was never there (a hole
+            // marking itself must still be caught on a later, recomposed hit, not just the
+            // one that first wrote the template; but a storable template-only hit must
+            // stay exactly as storable as the MISS that created it, no new CDN lifetime
+            // invented here).
             if ($unstorable !== null) {
                 ($this->donutHeaderSetter)($ro, null, null);
             }
@@ -297,8 +299,9 @@ private function refreshDonut(ResourceObject $ro): ResourceObject|null
         // When the recomposed content is byte-identical to the recorded one, carry over
         // the original Last-Modified instead of advancing it to the recomposition time
         $lastModified = $unstorable === null ? $donut->getUnchangedLastModified((string) $ro->view) : null;
-        // Always run, unstorable or not: this is what replaces the restored, stale-but-
-        // "storable" headers with Cache-Control: private, no-store on an unstorable response.
+        // Always run, unstorable or not: this is what strips the restored, stale-but-
+        // "storable" CDN headers from an unstorable response and - when a mark fired -
+        // also replaces Cache-Control/Pragma (HeaderSetter, issue #222).
         ($this->donutHeaderSetter)($ro, $donut->ttl, null, $lastModified);
         $this->donutHeaderSetter->setCdnCacheControl($ro, $donut->ttl);
         $this->logCdnHeaders($ro);

src/HeaderSetter.php

@@ -17,15 +17,12 @@
 final readonly class HeaderSetter
 {
     /**
-     * Every header unset on an unstorable response (issue #222): the CDN-facing ones this
-     * package's setters manage, across every flavor (default, Fastly, Akamai), plus
-     * Cache-Control/Pragma themselves - so one the application set directly in onGet(),
-     * bypassing the setters, does not survive the refusal under any header-name casing (RFC
-     * 7230 §3.2: $ro->headers is a plain PHP array, not an HTTP-aware collection).
+     * The CDN-facing headers unset on an unstorable response (issue #222), across every
+     * flavor (default, Fastly, Akamai) - unset unconditionally, whether the gate tripped on
+     * a mark or on a Cache-Control the response set itself, since this package never hands a
+     * shared cache a lifetime for a response it is itself refusing to store.
      */
-    private const STRIPPED_HEADERS = [
-        Header::CACHE_CONTROL,
-        Header::PRAGMA,
+    private const CDN_FACING_HEADERS = [
         Header::CDN_CACHE_CONTROL,
         FastlyCacheControlHeaderSetter::CDN_CACHE_CONTROL_HEADER,
         AkamaiCacheControlHeaderSetter::CDN_CACHE_CONTROL_HEADER,
@@ -36,23 +33,34 @@
     public function __construct(
         private EtagSetterInterface $etagSetter,
         private UnstorableGateInterface $gate,
+        private UncacheableMarkInterface $mark,
     ) {
     }
 
     /**
      * Set the validator and Cache-Control max-age, or - when the gate says this response is
-     * unstorable for this request (issue #222) - strip every CDN-facing header and replace
-     * Cache-Control/Pragma with ones that tell every cache, shared or private, not to keep it.
-     * A Cache-Control value #[HttpCache] or #[NoHttpCache] itself declared is exempt from the
-     * gate's no-store/private check: #[Cacheable] combined with #[HttpCache(noStore: true)]
-     * still stores server-side while telling the client not to.
+     * unstorable for this request (issue #222) - strip every CDN-facing header under any
+     * casing (RFC 7230 §3.2: $ro->headers is a plain PHP array, not an HTTP-aware collection).
+     *
+     * A producer that called UncacheableMarkInterface::mark() gets its Cache-Control/Pragma
+     * replaced too, with ones that tell every cache, shared or private, not to keep the
+     * response - the response was never meant to carry a cache directive of its own. A
+     * response refused only because it already set its own Cache-Control to `private`/
+     * `no-store` - no mark - keeps exactly the header the developer wrote: the gate decides
+     * whether THIS package stores it server-side, not what the client is told. A
+     * Cache-Control value #[HttpCache] or #[NoHttpCache] itself declared is exempt from the
+     * gate's no-store/private check either way: #[Cacheable] combined with
+     * #[HttpCache(noStore: true)] still stores server-side while telling the client not to.
      */
     public function __invoke(ResourceObject $ro, int|null $cacheControlMaxAge, HttpCache|null $httpCache, int|null $time = null): void
     {
         if ($this->gate->reason($ro) !== null) {
-            $this->stripHeaders($ro);
-            $ro->headers[Header::CACHE_CONTROL] = 'private, no-store';
-            $ro->headers[Header::PRAGMA] = 'no-cache';
+            $this->stripHeaders($ro, self::CDN_FACING_HEADERS);
+            if ($this->mark->isMarked()) {
+                $this->stripHeaders($ro, [Header::CACHE_CONTROL, Header::PRAGMA]);
+                $ro->headers[Header::CACHE_CONTROL] = 'private, no-store';
+                $ro->headers[Header::PRAGMA] = 'no-cache';
+            }
 
             return;
         }
@@ -64,9 +72,10 @@ public function __invoke(ResourceObject $ro, int|null $cacheControlMaxAge, HttpC
         }
     }
 
-    private function stripHeaders(ResourceObject $ro): void
+    /** @param list<string> $canonicalNames */
+    private function stripHeaders(ResourceObject $ro, array $canonicalNames): void
     {
-        foreach (self::STRIPPED_HEADERS as $canonical) {
+        foreach ($canonicalNames as $canonical) {
             foreach (array_keys($ro->headers) as $name) {
                 if (strtolower($name) === strtolower($canonical)) {
                     unset($ro->headers[$name]);

src/ResourceDonut.php

@@ -73,15 +73,27 @@ public function __construct(
     ) {
     }
 
-    public function refresh(ResourceInterface $resource, ResourceObject $ro): ResourceObject
+    public function refresh(ResourceInterface $resource, ResourceObject $ro, UncacheableMarkInterface $mark): ResourceObject
     {
         $etags = new SurrogateKeys($ro->uri);
-        $refreshView =  preg_replace_callback(self::URI_REGEX, static function (array $matches) use ($resource, $etags): string {
+        $refreshView =  preg_replace_callback(self::URI_REGEX, static function (array $matches) use ($resource, $etags, $mark): string {
             // Group 1 is "2" for a new `le2` match (its URI, group 2, is rawurlencode()'d),
             // or "" for a legacy `le` match (group 2 is its URI, used verbatim).
             $uri = $matches[1] === '2' ? rawurldecode($matches[2]) : $matches[2];
-            $ro = $resource->get($uri);
-            $ro->toString();
+            // Scoped the same way DonutRequest::__toString() scopes the MISS path: a mark
+            // made while this hole re-runs (onGet() or its own template render) must be
+            // attributed to the hole's own owner slot, not TOP_LEVEL - otherwise an
+            // unrelated sibling hole marking itself here is misread as "the page itself
+            // marked", and a nested #[DonutCache] hole's OWN template write is wrongly
+            // refused by reasonExcludingHoleMarks() for a mark that was never its own.
+            $mark->enterHole($uri);
+            try {
+                $ro = $resource->get($uri);
+                $ro->toString();
+            } finally {
+                $mark->leaveHole($uri);
+            }
+
             if (array_key_exists(Header::SURROGATE_KEY, $ro->headers)) {
                 $etags->addTag($ro);
             }

src/UncacheableMarkInterface.php

@@ -30,6 +30,17 @@
  * slot; anything else - including two sibling holes on the same path with different query
  * arguments - gets its own.
  *
+ * Known limitation, not a regression: isMarked() answers "marked anywhere in this request, by
+ * any owner" - every check except a `#[DonutCache]` write's own template uses it, including
+ * `QueryRepository::doPut()` and `DonutRepository::doPutStatic()` for an entirely unrelated
+ * `#[Cacheable]` or `#[CacheableResponse]` resource embedded elsewhere in the same request
+ * tree. A sibling hole that marks itself therefore also refuses every OTHER `#[Cacheable]`/
+ * `#[CacheableResponse]` write in that request, not just its own - because
+ * `QueryRepositoryInterface`/`DonutRepositoryInterface` cannot see the resource call tree
+ * `UncacheableMark`'s owner stack is built from, only "was anything marked". Scoping those
+ * writes' own checks the same way `reasonExcludingHoleMarks()` scopes a donut template
+ * write is deliberately left for a later release (#234).
+ *
  * @see https://github.com/bearsunday/BEAR.QueryRepository/issues/222
  */
 interface UncacheableMarkInterface

src/UnstorableGateInterface.php

@@ -19,6 +19,10 @@
  * directive, not a value a method or a render produced: `#[Cacheable]` combined with
  * `#[HttpCache(noStore: true)]` still stores server-side while telling the client not to.
  *
+ * reason() reads `UncacheableMarkInterface::isMarked()`, "marked anywhere in this request" -
+ * see that interface's docblock for the resulting, deliberate limitation: a mark made by one
+ * `#[DonutCache]` hole also refuses an entirely unrelated sibling resource's own write (#234).
+ *
  * @see https://github.com/bearsunday/BEAR.QueryRepository/issues/222
  */
 interface UnstorableGateInterface

tests/DonutCacheTest.php

@@ -20,6 +20,7 @@
 class DonutCacheTest extends TestCase
 {
     private ResourceInterface $resource;
+    private UncacheableMarkInterface $mark;
 
     protected function setUp(): void
     {
@@ -29,6 +30,7 @@ protected function setUp(): void
         $module->override(new TwigModule([$path]));
         $injector = new Injector($module, __DIR__ . '/tmp');
         $this->resource = $injector->getInstance(ResourceInterface::class);
+        $this->mark = new UncacheableMark();
 
         parent::setUp();
     }
@@ -38,7 +40,7 @@ public function testGetState(): void
         $headers = ['Content-Type' => 'application/xml;'];
         $donut = new ResourceDonut('cmt=[le:page://self/html/comment]', $headers, null, true);
         $blog = $this->resource->get('page://self/html/blog-posting');
-        $ro = $donut->refresh($this->resource, $blog);
+        $ro = $donut->refresh($this->resource, $blog, $this->mark);
         $this->assertInstanceOf(ResourceObject::class, $ro);
         $this->assertSame('cmt=comment01', $ro->view);
         $this->assertSame($headers['Content-Type'], $ro->headers['Content-Type']);
@@ -179,7 +181,7 @@ public function testDonutReplaysTheStatusCodeItWasStoredWith(): void
         $donut = new ResourceDonut('tmpl', [], null, true, null, null, null, null, 301);
         $ro = $this->resource->get('page://self/html/comment');
 
-        $donut->refresh($this->resource, $ro);
+        $donut->refresh($this->resource, $ro, $this->mark);
 
         $this->assertSame(301, $ro->code);
     }
@@ -192,7 +194,7 @@ public function testLegacyDonutPayloadLeavesTheStatusCodeAlone(): void
         $ro = $this->resource->get('page://self/html/comment');
         $ro->code = 301;
 
-        $this->legacyDonut()->refresh($this->resource, $ro);
+        $this->legacyDonut()->refresh($this->resource, $ro, $this->mark);
 
         $this->assertSame(301, $ro->code);
     }