diff --git a/CHANGELOG.md b/CHANGELOG.md index e56e1532..1cb12a37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.16.1] - 2026-06-01 + +### Fixed +- Fix multi-embed cache dependency: a `#[Cacheable]` resource embedding more than one child kept only the last child's dependency, so purging an earlier child failed to invalidate the parent (stale cache). `CacheDependency::depends()` now accumulates child tags instead of overwriting, and the erroneous assertion that a parent had no prior tags has been removed. + ## [1.16.0] - 2026-05-16 ### Fixed diff --git a/src/CacheDependency.php b/src/CacheDependency.php index e1fb9a1b..eb79aba2 100644 --- a/src/CacheDependency.php +++ b/src/CacheDependency.php @@ -7,7 +7,6 @@ use BEAR\Resource\ResourceObject; use Override; -use function assert; use function sprintf; final readonly class CacheDependency implements CacheDependencyInterface @@ -20,14 +19,17 @@ public function __construct( #[Override] public function depends(ResourceObject $from, ResourceObject $to): void { - assert(! isset($from->headers[Header::SURROGATE_KEY])); - - $cacheDependencyTags = ($this->uriTag)($to->uri); + $childTags = ($this->uriTag)($to->uri); if (isset($to->headers[Header::SURROGATE_KEY])) { - $cacheDependencyTags .= sprintf(' %s', $to->headers[Header::SURROGATE_KEY]); + $childTags .= sprintf(' %s', $to->headers[Header::SURROGATE_KEY]); unset($to->headers[Header::SURROGATE_KEY]); } - $from->headers[Header::SURROGATE_KEY] = $cacheDependencyTags; + // Accumulate across every embedded child: a resource that embeds more than one + // child must depend on all of them, not only the last. Overwriting here used to + // silently drop earlier children's dependencies (stale-cache bug). + $from->headers[Header::SURROGATE_KEY] = isset($from->headers[Header::SURROGATE_KEY]) + ? sprintf('%s %s', $from->headers[Header::SURROGATE_KEY], $childTags) + : $childTags; } } diff --git a/tests/ComplexCacheDependencyTest.php b/tests/ComplexCacheDependencyTest.php new file mode 100644 index 00000000..400354ad --- /dev/null +++ b/tests/ComplexCacheDependencyTest.php @@ -0,0 +1,115 @@ +resource = $injector->getInstance(ResourceInterface::class); + $this->repository = $injector->getInstance(QueryRepositoryInterface::class); + + parent::setUp(); + } + + private function isCached(string $name): bool + { + return $this->repository->get(new Uri('page://self/dep/' . $name)) instanceof ResourceState; + } + + public function testDiamondBuildsEveryNode(): void + { + $this->resource->get('page://self/dep/diamond-top'); + + foreach (['diamond-top', 'diamond-left', 'diamond-right', 'diamond-bottom'] as $name) { + $this->assertTrue($this->isCached($name), $name . ' should be cached after the build'); + } + } + + public function testMultiEmbedParentDependsOnEveryChild(): void + { + // Regression: diamond-top embeds BOTH arms. Purging EITHER arm must invalidate top. + // Before the fix, the second embed overwrote the first, so one arm's dependency was lost. + $this->resource->get('page://self/dep/diamond-top'); + $this->repository->purge(new Uri('page://self/dep/diamond-left')); + $this->assertFalse($this->isCached('diamond-top'), 'top must depend on its LEFT child'); + + $this->resource->get('page://self/dep/diamond-top'); // rebuild + $this->repository->purge(new Uri('page://self/dep/diamond-right')); + $this->assertFalse($this->isCached('diamond-top'), 'top must also depend on its RIGHT child'); + } + + public function testPurgingSharedLeafCascadesThroughBothPaths(): void + { + $this->resource->get('page://self/dep/diamond-top'); + + // diamond-bottom is embedded via both arms; purging it invalidates every ancestor. + $this->repository->purge(new Uri('page://self/dep/diamond-bottom')); + + $this->assertFalse($this->isCached('diamond-top'), 'top reaches bottom via both arms'); + $this->assertFalse($this->isCached('diamond-left'), 'left embeds bottom'); + $this->assertFalse($this->isCached('diamond-right'), 'right embeds bottom'); + } + + public function testPurgingOneArmIsPreciseAndDoesNotOverReach(): void + { + $this->resource->get('page://self/dep/diamond-top'); + + // Purge only the left arm. + $this->repository->purge(new Uri('page://self/dep/diamond-left')); + + $this->assertFalse($this->isCached('diamond-top'), 'top embeds left -> invalidated'); + $this->assertTrue($this->isCached('diamond-right'), 'the right arm is independent of left'); + $this->assertTrue($this->isCached('diamond-bottom'), 'the shared leaf survives an arm purge'); + } + + public function testRebuildAfterPurgeRePropagatesDependencies(): void + { + $this->resource->get('page://self/dep/diamond-top'); + $this->repository->purge(new Uri('page://self/dep/diamond-bottom')); + $this->assertFalse($this->isCached('diamond-top'), 'cascade invalidated top'); + + // Rebuild and confirm the dependency graph is restored (not a one-shot). + $this->resource->get('page://self/dep/diamond-top'); + $this->assertTrue($this->isCached('diamond-top')); + $this->repository->purge(new Uri('page://self/dep/diamond-bottom')); + $this->assertFalse($this->isCached('diamond-top'), 'cascade still works after a rebuild'); + } + + public function testUnrelatedResourceIsUnaffectedByDiamondPurges(): void + { + $this->resource->get('page://self/dep/diamond-top'); + $this->resource->get('page://self/dep/child-c'); // unrelated chain + + $this->repository->purge(new Uri('page://self/dep/diamond-bottom')); + + $this->assertTrue($this->isCached('child-c'), 'an unrelated resource must not be touched'); + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondBottom.php b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondBottom.php new file mode 100644 index 00000000..effa9d20 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondBottom.php @@ -0,0 +1,17 @@ + 1]; + + public function onGet() + { + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondLeft.php b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondLeft.php new file mode 100644 index 00000000..184f4de1 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondLeft.php @@ -0,0 +1,19 @@ + 1]; + + #[Embed(rel: 'bottom', src: '/dep/diamond-bottom')] + public function onGet() + { + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondRight.php b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondRight.php new file mode 100644 index 00000000..8923ad7c --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondRight.php @@ -0,0 +1,19 @@ + 1]; + + #[Embed(rel: 'bottom', src: '/dep/diamond-bottom')] + public function onGet() + { + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondTop.php b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondTop.php new file mode 100644 index 00000000..d9fa950f --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Dep/DiamondTop.php @@ -0,0 +1,20 @@ + 1]; + + #[Embed(rel: 'left', src: '/dep/diamond-left')] + #[Embed(rel: 'right', src: '/dep/diamond-right')] + public function onGet() + { + return $this; + } +}