diff --git a/src/DevEtagSetter.php b/src/DevEtagSetter.php index 8e601ec2..5971aa2d 100644 --- a/src/DevEtagSetter.php +++ b/src/DevEtagSetter.php @@ -5,7 +5,6 @@ namespace BEAR\QueryRepository; use BEAR\RepositoryModule\Annotation\HttpCache; -use BEAR\Resource\Request; use BEAR\Resource\ResourceObject; use Override; @@ -13,11 +12,6 @@ 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); - } - } } } diff --git a/src/EtagSetter.php b/src/EtagSetter.php index 3abc2640..01b3778f 100644 --- a/src/EtagSetter.php +++ b/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); - } - } - } } diff --git a/src/QueryRepository.php b/src/QueryRepository.php index 0f75d158..ae298353 100644 --- a/src/QueryRepository.php +++ b/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} */ diff --git a/tests/CacheDependencyTest.php b/tests/CacheDependencyTest.php index c1772b54..dafd400a 100644 --- a/tests/CacheDependencyTest.php +++ b/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] ?? ''); + } } diff --git a/tests/EtagSetterTest.php b/tests/EtagSetterTest.php index 34462ea4..4820bc03 100644 --- a/tests/EtagSetterTest.php +++ b/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'; diff --git a/tests/Fake/fake-app/src/Resource/Page/Dep/NonCacheableChild.php b/tests/Fake/fake-app/src/Resource/Page/Dep/NonCacheableChild.php new file mode 100644 index 00000000..cef86ec5 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Dep/NonCacheableChild.php @@ -0,0 +1,15 @@ + 1]; + + public function onGet() + { + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Dep/ParentOfNonCacheable.php b/tests/Fake/fake-app/src/Resource/Page/Dep/ParentOfNonCacheable.php new file mode 100644 index 00000000..45b6bf77 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Dep/ParentOfNonCacheable.php @@ -0,0 +1,19 @@ + 1]; + + #[Embed(rel: 'child', src: '/dep/non-cacheable-child')] + public function onGet() + { + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Hal/Child.php b/tests/Fake/fake-app/src/Resource/Page/Hal/Child.php new file mode 100644 index 00000000..733e9060 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Hal/Child.php @@ -0,0 +1,19 @@ + 'hal', + ]; + + public function onGet() + { + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Hal/ParentResource.php b/tests/Fake/fake-app/src/Resource/Page/Hal/ParentResource.php new file mode 100644 index 00000000..8a8fd958 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Hal/ParentResource.php @@ -0,0 +1,21 @@ + 'hal', + ]; + + #[Embed(rel: 'child', src: '/hal/child')] + public function onGet() + { + return $this; + } +} diff --git a/tests/ResourceRepositoryTest.php b/tests/ResourceRepositoryTest.php index a121cf4f..334a4a81 100644 --- a/tests/ResourceRepositoryTest.php +++ b/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); }