Bind command methods on the donut path to the command interceptor

CHANGELOG.md

@@ -72,6 +72,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Donut caching could not handle a non-200 response, in two ways (#206). The save gate skips 4xx and above, so a `301` or a `204` is stored, but `saveView()` required a validator that `EtagSetter` issues for 200 only: under `#[CacheableResponse]` every such request died on that assertion (a `TypeError` converted to a 400 with `zend.assertions=-1`), and a `204` leaving its body null died earlier still, in `ResourceDonut::create()`. Under `#[DonutCache]` nothing threw and the page answered 200 with the `Location` header attached from the second request on, because neither `DonutRepository::get()` nor `ResourceDonut::refresh()` restored the code the entry was saved with. The validator is now saved only when the response has one, `ResourceDonut` carries the code through the refresh (entries serialized before it leave the response's own code alone), and a null body is composed as an empty one. Behaviour change: a `#[CacheableResponse]` or `#[DonutCache]` page that answers 2xx or 3xx is now served with that status instead of crashing or degrading to 200 - a redirect that was reachable only once now persists until its cache entry is invalidated, so such a page needs the surrogate keys that invalidate it.
 - `onPost` on a `#[Cacheable]` class ran with no interceptor at all (#212): `CommandInterceptor` was bound to `onPut`/`onPatch`/`onDelete`, and `RefreshInterceptor` only to classes that are not `#[Cacheable]`. A POST - the method a form submits - left the representation it had just made stale being served, and a `#[Refresh]`/`#[Purge]` written on it was dropped with no error and no log event.
+- A write on the donut path could be answered from the cache without running: `#[RefreshCache]`, and a method-level `#[CacheableResponse]` on a command method, bound the query-side `DonutCacheInterceptor`, which returns the stored representation and never calls `proceed()`. Once the page was cached, `onPut`/`onPost`/`onDelete` did nothing and answered 200 with the cached page instead of their own response; both now bind `DonutCommandInterceptor`, which runs the write and then purges and refreshes. `DonutCacheModule` also missed `onPost` in its class-level write matcher, the same gap #212 fixed on the value-cache side. `tests/WeavingMatrixTest.php` asserts that a write runs for every declaration shape, and that it announces the change where a declaration names one - a shape whose declaration sits on `onGet` alone still needs `#[Purge]`/`#[Refresh]` on the write, which no matcher can supply.
 - `DevEtagSetter` and `MobileEtagSetter` set a validator regardless of the response code, where `EtagSetter` has always skipped a non-200. With a stored `301` now reaching them through the donut refresh, an `If-None-Match` match would have let `ConditionalResponse::isModified()` answer `304` in place of the redirect; both now skip a non-200. `CdnCacheControlHeaderSetterInterface` is likewise called for 200 only, so a stored non-200 is no longer handed the Fastly/Akamai default shared-cache lifetime of a year.
 - A client-chosen `If-None-Match` token containing a PSR-6 reserved character (`{}()/\@:`) reached the ETag pool as a cache key and threw, turning a request header into a 500 that logged like a pool outage. Such a token can never have been issued by this server, so `EntityTags` drops it and the request is answered in full; `*` is likewise dropped (RFC 9110 §13.1.2 gives it existence semantics this package does not implement).
 - An embedded child was materialized twice per parent store: the storage re-invoked the request instead of reusing the execution `setCacheDependency()` and the renderer already paid for, so a `type: "view"` entry could hold a body and a view from two different runs of a non-idempotent child.

src/DonutCacheModule.php

@@ -4,10 +4,13 @@
 
 namespace BEAR\QueryRepository;
 
+use BEAR\RepositoryModule\Annotation\Cacheable;
 use BEAR\RepositoryModule\Annotation\CacheableResponse;
 use BEAR\RepositoryModule\Annotation\DonutCache;
 use BEAR\RepositoryModule\Annotation\RefreshCache;
 use Override;
+use Ray\Aop\AbstractMatcher;
+use Ray\Aop\MatcherInterface;
 use Ray\Di\AbstractModule;
 use Ray\Di\Scope;
 
@@ -68,28 +71,72 @@ private function installAopClassModule(): void
 
         $this->bindInterceptor(
             $this->matcher->annotatedWith(CacheableResponse::class),
-            $this->matcher->logicalOr(
-                $this->matcher->startsWith('onPut'),
-                $this->matcher->logicalOr(
-                    $this->matcher->startsWith('onPatch'),
-                    $this->matcher->startsWith('onDelete'),
+            self::commandMethods($this->matcher),
+            [DonutCommandInterceptor::class],
+        );
+    }
+
+    /**
+     * onPost writes too: a POST that changes state must refresh the donut it invalidates
+     *
+     * @see \BEAR\QueryRepository\CacheableModule::installAopModule() same set on the value-cache side
+     */
+    private static function commandMethods(MatcherInterface $matcher): AbstractMatcher
+    {
+        return $matcher->logicalOr(
+            $matcher->startsWith('onPut'),
+            $matcher->logicalOr(
+                $matcher->startsWith('onPost'),
+                $matcher->logicalOr(
+                    $matcher->startsWith('onPatch'),
+                    $matcher->startsWith('onDelete'),
                 ),
             ),
-            [DonutCommandInterceptor::class],
         );
     }
 
     private function installAopMethodModule(): void
     {
+        // Ray.Aop merges overlapping bindings without deduplicating, so a class whose own
+        // declaration already governs the method is excluded. The two sets differ because
+        // #[DonutCache] governs onGet only: excluding it from the write bindings too would
+        // leave a #[RefreshCache] write on such a class with no interceptor at all.
+        $readNotDeclared = $this->matcher->logicalNot(
+            $this->matcher->logicalOr(
+                $this->matcher->annotatedWith(CacheableResponse::class),
+                $this->matcher->logicalOr(
+                    $this->matcher->annotatedWith(Cacheable::class),
+                    $this->matcher->annotatedWith(DonutCache::class),
+                ),
+            ),
+        );
+        $writeNotDeclared = $this->matcher->logicalNot(
+            $this->matcher->logicalOr(
+                $this->matcher->annotatedWith(CacheableResponse::class),
+                $this->matcher->annotatedWith(Cacheable::class),
+            ),
+        );
         $this->bindInterceptor(
-            $this->matcher->any(),
-            $this->matcher->annotatedWith(CacheableResponse::class),
+            $readNotDeclared,
+            $this->matcher->logicalAnd(
+                $this->matcher->annotatedWith(CacheableResponse::class),
+                $this->matcher->startsWith('onGet'),
+            ),
             [DonutCacheInterceptor::class],
         );
+
         $this->bindInterceptor(
-            $this->matcher->any(),
+            $writeNotDeclared,
+            $this->matcher->logicalAnd(
+                $this->matcher->annotatedWith(CacheableResponse::class),
+                self::commandMethods($this->matcher),
+            ),
+            [DonutCommandInterceptor::class],
+        );
+        $this->bindInterceptor(
+            $writeNotDeclared,
             $this->matcher->annotatedWith(RefreshCache::class),
-            [DonutCacheInterceptor::class],
+            [DonutCommandInterceptor::class],
         );
     }
 }

tests/DonutCommandInterceptorTest.php

@@ -177,6 +177,6 @@ public function testCacheableResponse(): void
         assert(is_array($ro->bindings['onDelete']));
         assert(isset($ro->bindings['onGet'][0]));
         assert(isset($ro->bindings['onDelete'][0]));
-        $this->assertInstanceOf(DonutCacheInterceptor::class, $ro->bindings['onDelete'][0]);
+        $this->assertInstanceOf(DonutCommandInterceptor::class, $ro->bindings['onDelete'][0]);
     }
 }

tests/Fake/fake-app/src/Resource/Page/Mx/CANone.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Mx;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\RepositoryModule\Annotation\CacheableResponse;
+use BEAR\RepositoryModule\Annotation\DonutCache;
+use BEAR\RepositoryModule\Annotation\Purge;
+use BEAR\RepositoryModule\Annotation\RefreshCache;
+use BEAR\Resource\ResourceObject;
+
+/** Weaving matrix fixture: CANone */
+#[Cacheable]
+class CANone extends ResourceObject
+{
+    /** Number of times a write body really ran */
+    public static int $ran = 0;
+
+    public function onGet(int $id = 0): static
+    {
+        $this->body = ['v' => $id];
+
+        return $this;
+    }
+
+    public function onPut(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    public function onPost(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    public function onDelete(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+}

tests/Fake/fake-app/src/Resource/Page/Mx/CAPurge.php

@@ -0,0 +1,53 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Mx;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\RepositoryModule\Annotation\CacheableResponse;
+use BEAR\RepositoryModule\Annotation\DonutCache;
+use BEAR\RepositoryModule\Annotation\Purge;
+use BEAR\RepositoryModule\Annotation\RefreshCache;
+use BEAR\Resource\ResourceObject;
+
+/** Weaving matrix fixture: CAPurge */
+#[Cacheable]
+class CAPurge extends ResourceObject
+{
+    /** Number of times a write body really ran */
+    public static int $ran = 0;
+
+    public function onGet(int $id = 0): static
+    {
+        $this->body = ['v' => $id];
+
+        return $this;
+    }
+
+    #[Purge(uri: 'page://self/mx/CAPurge?id=1')]
+    public function onPut(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    #[Purge(uri: 'page://self/mx/CAPurge?id=1')]
+    public function onPost(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    #[Purge(uri: 'page://self/mx/CAPurge?id=1')]
+    public function onDelete(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+}

tests/Fake/fake-app/src/Resource/Page/Mx/CARefresh.php

@@ -0,0 +1,53 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Mx;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\RepositoryModule\Annotation\CacheableResponse;
+use BEAR\RepositoryModule\Annotation\DonutCache;
+use BEAR\RepositoryModule\Annotation\Purge;
+use BEAR\RepositoryModule\Annotation\RefreshCache;
+use BEAR\Resource\ResourceObject;
+
+/** Weaving matrix fixture: CARefresh */
+#[Cacheable]
+class CARefresh extends ResourceObject
+{
+    /** Number of times a write body really ran */
+    public static int $ran = 0;
+
+    public function onGet(int $id = 0): static
+    {
+        $this->body = ['v' => $id];
+
+        return $this;
+    }
+
+    #[RefreshCache]
+    public function onPut(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    #[RefreshCache]
+    public function onPost(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    #[RefreshCache]
+    public function onDelete(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+}

tests/Fake/fake-app/src/Resource/Page/Mx/CRBoth.php

@@ -0,0 +1,46 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Mx;
+
+use BEAR\RepositoryModule\Annotation\CacheableResponse;
+use BEAR\Resource\ResourceObject;
+
+/** Weaving matrix fixture: the attribute declared at both levels */
+#[CacheableResponse]
+class CRBoth extends ResourceObject
+{
+    /** Number of times a write body really ran */
+    public static int $ran = 0;
+
+    #[CacheableResponse]
+    public function onGet(int $id = 0): static
+    {
+        $this->body = ['v' => $id];
+
+        return $this;
+    }
+
+    public function onPut(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    public function onPost(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    public function onDelete(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+}

tests/Fake/fake-app/src/Resource/Page/Mx/CRNone.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Mx;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\RepositoryModule\Annotation\CacheableResponse;
+use BEAR\RepositoryModule\Annotation\DonutCache;
+use BEAR\RepositoryModule\Annotation\Purge;
+use BEAR\RepositoryModule\Annotation\RefreshCache;
+use BEAR\Resource\ResourceObject;
+
+/** Weaving matrix fixture: CRNone */
+#[CacheableResponse]
+class CRNone extends ResourceObject
+{
+    /** Number of times a write body really ran */
+    public static int $ran = 0;
+
+    public function onGet(int $id = 0): static
+    {
+        $this->body = ['v' => $id];
+
+        return $this;
+    }
+
+    public function onPut(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    public function onPost(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+    public function onDelete(int $id = 0): static
+    {
+        self::$ran++;
+        $this->code = 204;
+
+        return $this;
+    }
+
+}