Resolve embed dependency before HAL rendering strips Request from body

src/DevEtagSetter.php

@@ -5,19 +5,13 @@
 namespace BEAR\QueryRepository;
 
 use BEAR\RepositoryModule\Annotation\HttpCache;
-use BEAR\Resource\Request;
 use BEAR\Resource\ResourceObject;
 use Override;
 
 use function gmdate;
 
 final readonly class DevEtagSetter implements EtagSetterInterface
 {
-    public function __construct(
-        private CacheDependencyInterface $cacheDeperency,
-    ) {
-    }
-
     /**
      * {@inheritDoc}
      */
@@ -30,17 +24,5 @@ public function __invoke(ResourceObject $ro, int|null $time = null, HttpCache|nu
         // Usually, the ETag is a hash of the resource view or body.
         $ro->headers[Header::ETAG] = $uriEtag;
         $ro->headers[Header::LAST_MODIFIED] = gmdate(Header::RFC7231, 0);
-        $this->setCacheDependency($ro);
-    }
-
-    /** @codeCoverageIgnore */
-    private function setCacheDependency(ResourceObject $ro): void
-    {
-        /** @var mixed $body */
-        foreach ((array) $ro->body as $body) {
-            if ($body instanceof Request && isset($body->resourceObject->headers[Header::ETAG])) {
-                $this->cacheDeperency->depends($ro, $body->resourceObject);
-            }
-        }
     }
 }

src/EtagSetter.php

@@ -5,7 +5,6 @@
 namespace BEAR\QueryRepository;
 
 use BEAR\RepositoryModule\Annotation\HttpCache;
-use BEAR\Resource\Request;
 use BEAR\Resource\ResourceObject;
 use Override;
 
@@ -19,11 +18,6 @@
 
 final readonly class EtagSetter implements EtagSetterInterface
 {
-    public function __construct(
-        private CacheDependencyInterface $cacheDeperency,
-    ) {
-    }
-
     /**
      * {@inheritDoc}
      */
@@ -38,7 +32,6 @@ public function __invoke(ResourceObject $ro, int|null $time = null, HttpCache|nu
         $etag =  $this->getEtag($ro, $httpCache);
         $ro->headers[Header::ETAG] = $etag;
         $ro->headers[Header::LAST_MODIFIED] = gmdate(Header::RFC7231, $time);
-        $this->setCacheDependency($ro);
     }
 
     public function getEtagByPartialBody(HttpCache $httpCacche, ResourceObject $ro): string
@@ -72,14 +65,4 @@ private function getEtag(ResourceObject $ro, HttpCache|null $httpCache = null):
 
         return (string) crc32($ro::class . $etag . (string) $ro->uri);
     }
-
-    private function setCacheDependency(ResourceObject $ro): void
-    {
-        /** @var mixed $body */
-        foreach ((array) $ro->body as $body) {
-            if ($body instanceof Request && isset($body->resourceObject->headers[Header::ETAG])) {
-                $this->cacheDeperency->depends($ro, $body->resourceObject);
-            }
-        }
-    }
 }

src/QueryRepository.php

@@ -7,6 +7,7 @@
 use BEAR\QueryRepository\Exception\ExpireAtKeyNotExists;
 use BEAR\RepositoryModule\Annotation\Cacheable;
 use BEAR\RepositoryModule\Annotation\HttpCache;
+use BEAR\Resource\AbstractRequest;
 use BEAR\Resource\AbstractUri;
 use BEAR\Resource\ResourceObject;
 use Override;
@@ -24,6 +25,7 @@ public function __construct(
         private HeaderSetter $headerSetter,
         private ResourceStorageInterface $storage,
         private Expiry $expiry,
+        private CacheDependencyInterface $cacheDependency,
     ) {
     }
 
@@ -35,6 +37,10 @@ public function put(ResourceObject $ro)
     {
         $this->logger->log('put-query-repository', ['uri' => (string) $ro->uri]);
         $this->storage->deleteEtag($ro->uri);
+        if ($ro->code === 200) {
+            $this->setCacheDependency($ro);
+        }
+
         $ro->toString();
         $cacheable = $this->getCacheableAnnotation($ro);
         $httpCache = $this->getHttpCacheAnnotation($ro);
@@ -53,6 +59,32 @@ public function put(ResourceObject $ro)
         return $this->storage->saveValue($ro, $ttl);
     }
 
+    private function setCacheDependency(ResourceObject $ro): void
+    {
+        if (isset($ro->headers[Header::SURROGATE_KEY])) {
+            return;
+        }
+
+        /** @var mixed $body */
+        foreach ((array) $ro->body as $body) {
+            if (! ($body instanceof AbstractRequest)) {
+                continue;
+            }
+
+            // Materialize the child while HAL still has the Request in body.
+            // AbstractRequest::__toString() memoizes the inner result, so
+            // repeated casts here and in the HAL renderer share one
+            // invocation regardless of the request implementation's
+            // execution strategy.
+            (string) $body;
+            if (! isset($body->resourceObject->headers[Header::ETAG])) {
+                continue;
+            }
+
+            $this->cacheDependency->depends($ro, $body->resourceObject);
+        }
+    }
+
     /**
      * {@inheritDoc}
      */

tests/CacheDependencyTest.php

@@ -4,6 +4,7 @@
 
 namespace BEAR\QueryRepository;
 
+use BEAR\Resource\Module\HalModule;
 use BEAR\Resource\ResourceInterface;
 use BEAR\Resource\Uri;
 use PHPUnit\Framework\TestCase;
@@ -127,4 +128,39 @@ public function testMultipleParentsDependOnSameChild(): void
         $this->assertFalse($this->storage->hasEtag($etagA));
         $this->assertFalse($this->storage->hasEtag($etagB));
     }
+
+    /**
+     * Non-Cacheable child has no ETag header, so setCacheDependency must
+     * continue past it without registering a (parent, child) dependency. The
+     * parent's cached state still gets a Surrogate-Key (its own URI tag),
+     * but the child's URI tag must NOT appear in it.
+     */
+    public function testNonCacheableChildDoesNotContributeSurrogateKey(): void
+    {
+        $this->resource->get('page://self/dep/parent-of-non-cacheable');
+
+        $parent = $this->repository->get(new Uri('page://self/dep/parent-of-non-cacheable'));
+        $this->assertInstanceOf(ResourceState::class, $parent);
+
+        $childTag = (new UriTag())(new Uri('page://self/dep/non-cacheable-child'));
+        $surrogateKey = $parent->headers[Header::SURROGATE_KEY] ?? '';
+        $this->assertStringNotContainsString($childTag, $surrogateKey);
+    }
+
+    public function testHalEmbeddedChildAddsChildSurrogateKeyToParent(): void
+    {
+        $module = ModuleFactory::getInstance('FakeVendor\HelloWorld');
+        $module->override(new HalModule());
+        $injector = new Injector(new FakeEtagPoolModule($module), __DIR__ . '/tmp');
+        $resource = $injector->getInstance(ResourceInterface::class);
+        $repository = $injector->getInstance(QueryRepositoryInterface::class);
+
+        $resource->get('page://self/hal/parent-resource');
+
+        $parent = $repository->get(new Uri('page://self/hal/parent-resource'));
+        $this->assertInstanceOf(ResourceState::class, $parent);
+        $childTag = (new UriTag())(new Uri('page://self/hal/child'));
+
+        $this->assertStringContainsString($childTag, $parent->headers[Header::SURROGATE_KEY] ?? '');
+    }
 }

tests/EtagSetterTest.php

@@ -22,7 +22,7 @@ protected function setUp(): void
 
     public function testStatusNotOk(): void
     {
-        $setEtag = new EtagSetter(new CacheDependency(new UriTag()));
+        $setEtag = new EtagSetter();
         $ro = new Code();
         $ro->code = 500;
         $setEtag($ro);
@@ -33,7 +33,7 @@ public function testStatusNotOk(): void
     public function testInvoke(): void
     {
         $ro = $this->resource->get('app://self/user', ['id' => 1]);
-        $setEtag = new EtagSetter(new CacheDependency(new UriTag()));
+        $setEtag = new EtagSetter();
         $time = 0;
         $setEtag($ro, $time);
         $expect = 'Thu, 01 Jan 1970 00:00:00 GMT';

tests/Fake/fake-app/src/Resource/Page/Dep/NonCacheableChild.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Dep;
+
+use BEAR\Resource\ResourceObject;
+
+class NonCacheableChild extends ResourceObject
+{
+    public $body = ['non-cacheable-child' => 1];
+
+    public function onGet()
+    {
+        return $this;
+    }
+}

tests/Fake/fake-app/src/Resource/Page/Dep/ParentOfNonCacheable.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Dep;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\Resource\Annotation\Embed;
+use BEAR\Resource\ResourceObject;
+
+#[Cacheable]
+class ParentOfNonCacheable extends ResourceObject
+{
+    public $body = ['parent-of-non-cacheable' => 1];
+
+    #[Embed(rel: 'child', src: '/dep/non-cacheable-child')]
+    public function onGet()
+    {
+        return $this;
+    }
+}

tests/Fake/fake-app/src/Resource/Page/Hal/Child.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Hal;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\Resource\ResourceObject;
+
+#[Cacheable]
+class Child extends ResourceObject
+{
+    public $body = [
+        'child' => 'hal',
+    ];
+
+    public function onGet()
+    {
+        return $this;
+    }
+}

tests/Fake/fake-app/src/Resource/Page/Hal/ParentResource.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace FakeVendor\HelloWorld\Resource\Page\Hal;
+
+use BEAR\RepositoryModule\Annotation\Cacheable;
+use BEAR\Resource\Annotation\Embed;
+use BEAR\Resource\ResourceObject;
+
+#[Cacheable]
+class ParentResource extends ResourceObject
+{
+    public $body = [
+        'parent' => 'hal',
+    ];
+
+    #[Embed(rel: 'child', src: '/hal/child')]
+    public function onGet()
+    {
+        return $this;
+    }
+}

tests/ResourceRepositoryTest.php

@@ -38,7 +38,7 @@ public function get()
         };
         $this->repository = new Repository(
             new RepositoryLogger(),
-            new HeaderSetter(new EtagSetter(new CacheDependency(new UriTag()))),
+            new HeaderSetter(new EtagSetter()),
             new ResourceStorage(
                 new RepositoryLogger(),
                 new NullPurger(),
@@ -49,6 +49,7 @@ public function get()
                 $tagAwareAdapterProvider,
             ),
             new Expiry(0, 0, 0),
+            new CacheDependency(new UriTag()),
         );
         $this->ro = new Index();
         $this->ro->uri = new Uri('page://self/user');
@@ -69,8 +70,12 @@ public function testPutAndGet(): void
         $this->assertSame($headers['content-type'], $roHeaders['content-type']);
         $this->assertSame($headers['etag'], $roHeaders['etag']);
         $this->assertSame($headers['last-modified'], $roHeaders['last-modified']);
-        $this->assertSame('0', $headers['age']);
+        // Age is `time() - strtotime(Last-Modified)`, so put→get crossing a
+        // second boundary on a slow runner can land on '1'. Either value is
+        // a correct freshly-cached response — what matters is that the
+        // header is present and small.
         $this->assertArrayHasKey('age', $headers);
+        $this->assertContains($headers['age'], ['0', '1']);
         $this->assertSame($state->body, $this->ro->body);
     }
 
@@ -101,7 +106,7 @@ public function get()
         };
         $repository = new Repository(
             new RepositoryLogger(),
-            new HeaderSetter(new EtagSetter(new CacheDependency(new UriTag()))),
+            new HeaderSetter(new EtagSetter()),
             new ResourceStorage(
                 new RepositoryLogger(),
                 new NullPurger(),
@@ -112,6 +117,7 @@ public function get()
                 $tagAwareAdapterProvider,
             ),
             new Expiry(0, 0, 0),
+            new CacheDependency(new UriTag()),
         );
         $this->assertInstanceOf(Repository::class, $repository);
     }