Skip the same-URI refresh instead of throwing on a mismatched onPost

CHANGELOG.md

@@ -74,6 +74,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.
+- `onPost` on a `#[Cacheable]` class whose parameters did not cover `onGet`'s required ones threw `UnmatchedQuery`, uncaught (#219, a regression from #214 above): `RefreshSameCommand`'s automatic same-URI refresh runs `MatchQuery` against the write's own URI query, and a create posting to a collection URI routinely lacks a parameter `onGet` needs. The write itself already succeeded; the refresh had nothing to target. The same gap existed on the donut path: `DonutCommandInterceptor::refreshDonutAndState()` called `MatchQuery` with no catch either, so a `#[CacheableResponse]`/`#[DonutCache]` collection `onPost` with the same shape 500ed too. Both interceptors now catch `UnmatchedQuery` for `onPost` specifically and record it as a `cache_error{operation: write}` instead; `RefreshSameCommand` returning rather than throwing also lets `CommandsProvider`'s next command run, so an explicit `#[Refresh]`/`#[Purge]` on the same method still fires. `onPut`/`onPatch`/`onDelete` keep throwing on both interceptors: those act on an entity `onGet` already addresses, so a missing required parameter there is a real signature mismatch (`BehaviorTest::testUnMatchQuery`).
 - 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).

src/CommandInterceptor.php

@@ -19,7 +19,7 @@
 /**
  * Interceptor for cache invalidation on CQRS commands with #[Purge] or #[Refresh]
  *
- * Automatically bound to all command methods (onPut/onPatch/onDelete) of #[Cacheable] classes.
+ * Automatically bound to all command methods (onPost/onPut/onPatch/onDelete) of #[Cacheable] classes.
  * Processes #[Purge] and #[Refresh] annotations on these methods and executes cache
  * invalidation after successful write operations.
  *

src/DonutCommandInterceptor.php

@@ -5,6 +5,8 @@
 
 namespace BEAR\QueryRepository;
 
+use BEAR\QueryRepository\Exception\UnmatchedQuery;
+use BEAR\QueryRepository\Log\Context\CacheErrorContext;
 use BEAR\QueryRepository\Log\Context\CommandResultContext;
 use BEAR\RepositoryModule\Annotation\CacheLog;
 use BEAR\Resource\Code;
@@ -18,11 +20,12 @@
 use function assert;
 use function call_user_func_array;
 use function is_callable;
+use function str_starts_with;
 
 /**
  * Interceptor for donut cache invalidation on CQRS commands
  *
- * Bound to command methods (onPut/onPatch/onDelete) of classes marked with #[CacheableResponse].
+ * Bound to command methods (onPost/onPut/onPatch/onDelete) of classes marked with #[CacheableResponse].
  * Refreshes donut cache and resource state after successful write operations.
  *
  * @see \BEAR\RepositoryModule\Annotation\CacheableResponse
@@ -53,7 +56,21 @@ public function invoke(MethodInvocation $invocation): ResourceObject
         $openId = $this->logger->open(($this->commandContextFactory)($invocation, 'DonutCommandInterceptor'));
         try {
             if ($ro->code < Code::BAD_REQUEST) {
-                $this->refreshDonutAndState($ro);
+                try {
+                    $this->refreshDonutAndState($ro);
+                } catch (UnmatchedQuery $e) {
+                    // Same shape as RefreshSameCommand::command() on the value-cache side (#219):
+                    // an onPost-bound class that creates rather than addresses an entity routinely
+                    // lacks a parameter onGet requires. The write already ran; there is no entry
+                    // to refresh, so the purge/refresh is skipped and recorded instead of thrown.
+                    // onPut/onPatch/onDelete keep throwing - a mismatch there addresses an entity
+                    // onGet already does, so it is a real signature mismatch, not this case.
+                    if (! str_starts_with($invocation->getMethod()->getName(), 'onPost')) {
+                        throw $e;
+                    }
+
+                    $this->logger->event(new CacheErrorContext((string) $ro->uri, 'write', $e->getMessage(), $e::class));
+                }
             }
         } finally {
             $this->logger->close(new CommandResultContext($ro->code), $openId);

src/RefreshSameCommand.php

@@ -5,27 +5,56 @@
 
 namespace BEAR\QueryRepository;
 
+use BEAR\QueryRepository\Exception\UnmatchedQuery;
+use BEAR\QueryRepository\Log\Context\CacheErrorContext;
+use BEAR\RepositoryModule\Annotation\CacheLog;
 use BEAR\Resource\ResourceObject;
+use Koriym\SemanticLogger\NullSemanticLogger;
+use Koriym\SemanticLogger\SemanticLoggerInterface;
 use Override;
 use Ray\Aop\MethodInvocation;
 use ReflectionException;
 
 use function call_user_func_array;
 use function is_callable;
+use function str_starts_with;
 
 final readonly class RefreshSameCommand implements CommandInterface
 {
     public function __construct(
         private QueryRepositoryInterface $repository,
         private MatchQueryInterface $matchQuery,
+        #[CacheLog]
+        private SemanticLoggerInterface $logger = new NullSemanticLogger(),
     ) {
     }
 
     #[Override]
     public function command(MethodInvocation $invocation, ResourceObject $ro): void
     {
-        unset($invocation);
-        $getQuery = $this->getQuery($ro);
+        try {
+            $getQuery = $this->getQuery($ro);
+        } catch (UnmatchedQuery $e) {
+            // #214 bound this command to onPost too, on classes that create rather than address
+            // an existing entity - onGet's required parameters (an id assigned on write) are
+            // routinely absent from a POST's own query. Pre-#214, onPost reached no command at
+            // all, so this was silently a no-op; skipping here restores that shape instead of a
+            // regression, and an explicit #[Refresh]/#[Purge] on the method still runs next
+            // (CommandsProvider orders this command first, but returning - not throwing - lets
+            // CommandInterceptor's loop reach the next command).
+            //
+            // onPut/onPatch/onDelete keep throwing: those act on an entity onGet already
+            // addresses, so a required parameter missing there is a real signature mismatch, not
+            // this case (BehaviorTest::testUnMatchQuery pins that).
+            if (! str_starts_with($invocation->getMethod()->getName(), 'onPost')) {
+                throw $e;
+            }
+
+            $this->logger->event(new CacheErrorContext((string) $ro->uri, 'write', $e->getMessage(), $e::class));
+
+            return;
+        }
+
         $delUri = clone $ro->uri;
         $delUri->query = $getQuery;
 
@@ -46,6 +75,7 @@ public function command(MethodInvocation $invocation, ResourceObject $ro): void
      * @return array<string, mixed>
      *
      * @throws ReflectionException
+     * @throws UnmatchedQuery
      */
     private function getQuery(ResourceObject $ro): array
     {

tests/DonutUnmatchedQueryRefreshTest.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BEAR\QueryRepository;
+
+use BEAR\QueryRepository\Exception\UnmatchedQuery;
+use BEAR\RepositoryModule\Annotation\CacheLog;
+use BEAR\Resource\ResourceInterface;
+use Koriym\SemanticLogger\SemanticLoggerInterface;
+use Madapaja\TwigModule\TwigModule;
+use PHPUnit\Framework\TestCase;
+use Ray\Di\Injector;
+
+use function dirname;
+
+/**
+ * onPost on a #[CacheableResponse] class whose parameters miss onGet's required ones (#219)
+ *
+ * The same gap as `UnmatchedQueryRefreshTest`, on the donut command path:
+ * `DonutCommandInterceptor::refreshDonutAndState()` calls `MatchQuery` with no catch, inside
+ * an `invoke()` whose `try` is `finally`-only. `DonutCacheModule` binds this interceptor to
+ * `onPost` too (`commandMethods()`), so a `#[CacheableResponse]` collection POST whose
+ * parameters do not cover `onGet`'s required ones threw uncaught here as well.
+ */
+class DonutUnmatchedQueryRefreshTest extends TestCase
+{
+    use SemanticLogTreeTrait;
+
+    private ResourceInterface $resource;
+    private SemanticLoggerInterface $logger;
+
+    protected function setUp(): void
+    {
+        $module = new FakeEtagPoolModule(ModuleFactory::getInstance('FakeVendor\HelloWorld'));
+        $module->override(new TwigModule([dirname(__DIR__) . '/tests/Fake/fake-app/var/templates']));
+        $injector = new Injector($module, __DIR__ . '/tmp');
+        $this->resource = $injector->getInstance(ResourceInterface::class);
+        $this->logger = $injector->getInstance(SemanticLoggerInterface::class, CacheLog::class);
+
+        parent::setUp();
+    }
+
+    public function testPostSkipsTheRefreshInsteadOfThrowing(): void
+    {
+        $ro = $this->resource->post('page://self/html/mismatched-donut-writer', ['title' => 'new']);
+
+        $this->assertSame(200, $ro->code, 'the write itself still succeeds; only the automatic refresh cannot apply');
+    }
+
+    public function testTheSkipIsRecordedAsACacheError(): void
+    {
+        $this->resource->post('page://self/html/mismatched-donut-writer', ['title' => 'new']);
+        $tree = $this->flushAndValidate($this->logger);
+
+        $error = self::eventContextJsonOf($tree, 'cache_error');
+        $this->assertNotNull($error, 'the skip is a recorded event, not a silent one');
+        $this->assertStringContainsString('"operation":"write"', $error);
+        $this->assertStringContainsString('"exceptionClass":"BEAR\\\\QueryRepository\\\\Exception\\\\UnmatchedQuery"', $error);
+    }
+
+    public function testPutStillThrowsForAGenuineMismatch(): void
+    {
+        $this->expectException(UnmatchedQuery::class);
+        $this->resource->put('page://self/html/mismatched-donut-writer', ['title' => 'new']);
+    }
+}

tests/Fake/fake-app/src/Resource/App/MismatchedWriter.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace FakeVendor\HelloWorld\Resource\App;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\RepositoryModule\Annotation\Purge;
+use BEAR\Resource\ResourceObject;
+
+/**
+ * A #[Cacheable] collection whose onPost does not share onGet's required parameter
+ *
+ * onGet needs `id`; onPost creates a new item and never carries one. The automatic
+ * same-URI refresh (RefreshSameCommand) has no entry to target and has to be skipped,
+ * not thrown - the explicit #[Purge] below still has to run.
+ */
+#[Cacheable]
+class MismatchedWriter extends ResourceObject
+{
+    public function onGet(int $id): static
+    {
+        $this->body = ['id' => $id];
+
+        return $this;
+    }
+
+    #[Purge(uri: 'app://self/refresh-dest?id=1')]
+    public function onPost(string $title): static
+    {
+        $this->body = ['title' => $title];
+
+        return $this;
+    }
+}

tests/Fake/fake-app/src/Resource/Page/Html/MismatchedDonutWriter.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Html;
+
+use BEAR\RepositoryModule\Annotation\CacheableResponse;
+use BEAR\Resource\ResourceObject;
+
+/**
+ * A #[CacheableResponse] page whose onPost does not share onGet's required parameter (#219)
+ *
+ * onGet needs `id`; onPost creates a new item and never carries one. DonutCommandInterceptor's
+ * automatic refresh has no entry to target and has to be skipped, not thrown. onPut addresses
+ * the same entity onGet does, so a parameter it omits is a real signature mismatch - that one
+ * still throws (mirrors BehaviorTest::testUnMatchQuery on the value-cache side).
+ */
+#[CacheableResponse]
+class MismatchedDonutWriter extends ResourceObject
+{
+    public function onGet(int $id): static
+    {
+        $this->body = ['id' => $id];
+
+        return $this;
+    }
+
+    public function onPost(string $title): static
+    {
+        $this->body = ['title' => $title];
+
+        return $this;
+    }
+
+    public function onPut(string $title): static
+    {
+        $this->body = ['title' => $title];
+
+        return $this;
+    }
+}

tests/Fake/fake-app/var/templates/Page/Html/MismatchedDonutWriter.html.twig

@@ -0,0 +1 @@
+mismatched:{{ id }}

tests/UnmatchedQueryRefreshTest.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BEAR\QueryRepository;
+
+use BEAR\RepositoryModule\Annotation\CacheLog;
+use BEAR\Resource\ResourceInterface;
+use BEAR\Resource\Uri;
+use FakeVendor\HelloWorld\Resource\App\RefreshDest;
+use Koriym\SemanticLogger\SemanticLoggerInterface;
+use PHPUnit\Framework\TestCase;
+use Ray\Di\Injector;
+
+/**
+ * onPost on a #[Cacheable] class whose parameters miss onGet's required ones (#219)
+ *
+ * #214 bound `CommandInterceptor` to `onPost` too, so `RefreshSameCommand`'s automatic
+ * same-URI refresh now runs on every write - including a create whose parameters do not
+ * cover `onGet`'s required ones. `MatchQuery` throws `UnmatchedQuery` for that case; before
+ * this fix nothing caught it, so the write itself failed instead of just skipping a refresh
+ * it cannot perform.
+ */
+class UnmatchedQueryRefreshTest extends TestCase
+{
+    use SemanticLogTreeTrait;
+
+    private ResourceInterface $resource;
+    private QueryRepositoryInterface $repository;
+    private SemanticLoggerInterface $logger;
+
+    protected function setUp(): void
+    {
+        $injector = new Injector(ModuleFactory::getInstance('FakeVendor\HelloWorld'), __DIR__ . '/tmp');
+        $this->resource = $injector->getInstance(ResourceInterface::class);
+        $this->repository = $injector->getInstance(QueryRepositoryInterface::class);
+        $this->logger = $injector->getInstance(SemanticLoggerInterface::class, CacheLog::class);
+
+        RefreshDest::$id = 0;
+
+        parent::setUp();
+    }
+
+    public function testPostSkipsTheRefreshInsteadOfThrowing(): void
+    {
+        $ro = $this->resource->post('app://self/mismatched-writer', ['title' => 'new']);
+
+        $this->assertSame(200, $ro->code, 'the write itself still succeeds; only the automatic refresh cannot apply');
+    }
+
+    public function testPostStillRunsTheExplicitPurgeWrittenOnIt(): void
+    {
+        $dest = new Uri('app://self/refresh-dest?id=1');
+        $this->repository->put($this->resource->get('app://self/refresh-dest', ['id' => '1']));
+        $this->assertInstanceOf(ResourceState::class, $this->repository->get($dest));
+
+        $this->resource->post('app://self/mismatched-writer', ['title' => 'new']);
+
+        $this->assertNull($this->repository->get($dest), 'CommandsProvider runs RefreshAnnotatedCommand next - the skip must not stop it');
+    }
+
+    public function testTheSkipIsRecordedAsACacheError(): void
+    {
+        $this->resource->post('app://self/mismatched-writer', ['title' => 'new']);
+        $tree = $this->flushAndValidate($this->logger);
+
+        $error = self::eventContextJsonOf($tree, 'cache_error');
+        $this->assertNotNull($error, 'the skip is a recorded event, not a silent one');
+        $this->assertStringContainsString('"operation":"write"', $error);
+        $this->assertStringContainsString('"exceptionClass":"BEAR\\\\QueryRepository\\\\Exception\\\\UnmatchedQuery"', $error);
+    }
+}