diff --git a/CHANGELOG.md b/CHANGELOG.md index 40fa8da5..7d3c7a73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,13 +15,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Deploy this version with a new `CacheVersionModule` version (an app that binds its own cache adapter rather than this package's `Storage*Module`s must clear its pools instead): entries written by earlier versions may hold per-user content, and an earlier version reading this version's donut cache shows raw `[le2:...]` placeholders during a rolling deploy. - A `#[Cacheable]`/`#[CacheableResponse]` response whose own code sets `Cache-Control` to `private` or `no-store` is no longer stored server-side; its header is kept and CDN headers are removed. A value declared by `#[HttpCache]`/`#[NoHttpCache]` is still stored. +- Upgrading changes the Ray.Aop-woven class name - and so the ETag (`crc32($ro::class . ...)`) - for a `#[CacheableResponse]` class or method that also declares an `#[Embed]`, because it is now woven with a new interceptor (#226); a page with no `#[Embed]` keeps its stock woven class and ETag, and a `#[DonutCache]` page never carries an ETag at all, so neither is affected. - `ResourceDonut::FORMAT` is no longer public; use `ResourceDonut::placeholder()`. `ResourceDonut::refresh()` takes an additional `UncacheableMarkInterface` argument. - Requires `koriym/semantic-logger` `^0.11`. Written logs use its `log` document shape, and diagnostic `kind` is now `code`. +- Requires `bear/resource` `^1.22.4`. - `KeepMutationsAndFailures` also keeps a session cut at the record limit (`semantic_logger_truncated`): a `command` past the limit leaves no other trace. ### Fixed - `LogFileWriter`: two sessions flushed in the same microsecond no longer overwrite each other. The timestamped filename now carries a random suffix, so both survive (#221). - A `#[DonutCache]` page embedding an `#[Embed]` hole whose URI carried two or more query parameters leaked a raw `[le:...]` placeholder to the client and could throw `ParameterException` on the next request - fixed by moving new placeholders to a `[le2:...]` marker (#232). +- An embedded child of a `#[DonutCache]`/`#[CacheableResponse]` page is now reused until its query or links change, instead of running again when rendered as a hole after already being read in `onGet()`. Every such execution that was read but never rendered as a hole now tags the stored template and the live response's own Surrogate-Key/ETag, so purging any one of them refreshes the page instead of leaving it stale (#226). +- `ResourceBodyEvaluator` no longer crashes materializing an embedded child whose `jsonSerialize()` does not answer the memoized `ResourceObject` (the shape `BEAR\Async\AsyncRequest` overrides it to): such a child is now left unmaterialized instead of asserting. Async support itself remains out of scope. ## [1.17.0] - 2026-09-17 diff --git a/composer.json b/composer.json index 85fdd57b..4b03019a 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ ], "require": { "php": "^8.2", - "bear/resource": "^1.16.1", + "bear/resource": "^1.22.4", "bear/sunday": "^1.5", "koriym/semantic-logger": "^0.11", "psr/cache": "^1.0 || ^2.0 || ^3.0", diff --git a/src/DonutCacheModule.php b/src/DonutCacheModule.php index b615af2e..3d54e38e 100644 --- a/src/DonutCacheModule.php +++ b/src/DonutCacheModule.php @@ -8,6 +8,7 @@ use BEAR\RepositoryModule\Annotation\CacheableResponse; use BEAR\RepositoryModule\Annotation\DonutCache; use BEAR\RepositoryModule\Annotation\RefreshCache; +use BEAR\Resource\Annotation\Embed; use Override; use Ray\Aop\AbstractMatcher; use Ray\Aop\MatcherInterface; @@ -52,6 +53,8 @@ protected function configure(): void $this->bind(RepositoryLoggerInterface::class)->to(RepositoryLogger::class)->in(Scope::SINGLETON); $this->bind(PurgerInterface::class)->to(NullPurger::class); $this->bind(UriTagInterface::class)->to(UriTag::class); + // One registry per request (see DonutChildExecutions); issue #226 + $this->bind(DonutChildExecutionsInterface::class)->to(DonutChildExecutions::class)->in(Scope::SINGLETON); $this->installAopClassModule(); $this->installAopMethodModule(); } @@ -76,6 +79,28 @@ private function installAopClassModule(): void self::commandMethods($this->matcher), [DonutCommandInterceptor::class], ); + + // Plain bindInterceptor, not bindPriorityInterceptor: this one must run INSIDE + // EmbedInterceptor (see EmbedOnceInterceptor's own docblock for why Ray.Aop's + // onion ordering guarantees that regardless of install order), where a priority + // interceptor never would. Also requires #[Embed]: a class with none (Page\Index + // is this shape) has nothing for EmbedOnceInterceptor to do, so it is left out of + // the pointcut set entirely rather than woven in as a no-op - keeping both the + // Ray.Aop-generated class name (and so EtagSetter's crc32 input) and the per-call + // reflection cost unaffected for every page that embeds nothing. Still a + // LogicalAndMatcher, not a bare annotatedWith(), so it stays in onionOrderMatch()'s + // leftover (install-order-independent) bucket. + $this->bindInterceptor( + $this->matcher->logicalOr( + $this->matcher->annotatedWith(CacheableResponse::class), + $this->matcher->annotatedWith(DonutCache::class), + ), + $this->matcher->logicalAnd( + $this->matcher->startsWith('onGet'), + $this->matcher->annotatedWith(Embed::class), + ), + [EmbedOnceInterceptor::class], + ); } /** @@ -103,15 +128,7 @@ private function installAopMethodModule(): void // 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), - ), - ), - ); + $readNotDeclared = $this->readNotDeclaredMatcher(); $writeNotDeclared = $this->matcher->logicalNot( $this->matcher->logicalOr( $this->matcher->annotatedWith(CacheableResponse::class), @@ -127,6 +144,21 @@ private function installAopMethodModule(): void [DonutCacheInterceptor::class], ); + // Same plain-bindInterceptor reasoning as installAopClassModule()'s own + // EmbedOnceInterceptor binding (#[Embed] required too), for a class whose + // #[CacheableResponse] sits on onGet() itself rather than on the class + // (BlogPostingCache is this fixture). + $this->bindInterceptor( + $readNotDeclared, + $this->matcher->logicalAnd( + $this->matcher->annotatedWith(CacheableResponse::class), + $this->matcher->logicalAnd( + $this->matcher->startsWith('onGet'), + $this->matcher->annotatedWith(Embed::class), + ), + ), + [EmbedOnceInterceptor::class], + ); $this->bindInterceptor( $writeNotDeclared, $this->matcher->logicalAnd( @@ -141,4 +173,23 @@ private function installAopMethodModule(): void [DonutCommandInterceptor::class], ); } + + /** + * Classes governed by neither #[CacheableResponse], #[Cacheable] nor #[DonutCache] at + * the class level - shared by installAopMethodModule()'s read bindings so the two stay + * in lockstep: a method-level #[CacheableResponse] read is exactly where + * EmbedOnceInterceptor must run too. + */ + private function readNotDeclaredMatcher(): AbstractMatcher + { + return $this->matcher->logicalNot( + $this->matcher->logicalOr( + $this->matcher->annotatedWith(CacheableResponse::class), + $this->matcher->logicalOr( + $this->matcher->annotatedWith(Cacheable::class), + $this->matcher->annotatedWith(DonutCache::class), + ), + ), + ); + } } diff --git a/src/DonutChildExecutions.php b/src/DonutChildExecutions.php new file mode 100644 index 00000000..b769a0d5 --- /dev/null +++ b/src/DonutChildExecutions.php @@ -0,0 +1,54 @@ +> */ + private WeakMap $byPage; + + public function __construct() + { + // A fresh WeakMap is empty, so its template parameters are whatever the property + // declares them as - never actually checked against `object`/`mixed`. + /** @psalm-suppress PropertyTypeCoercion */ + $this->byPage = new WeakMap(); + } + + #[Override] + public function register(ResourceObject $page, OnceRequest $once): void + { + $existing = $this->registered($page); + $existing[] = $once; + $this->byPage[$page] = $existing; + } + + #[Override] + public function take(ResourceObject $page): array + { + $registered = $this->registered($page); + unset($this->byPage[$page]); + + return $registered; + } + + /** @return list */ + private function registered(ResourceObject $page): array + { + if (! isset($this->byPage[$page])) { + return []; + } + + /** @var list $registered */ + $registered = $this->byPage[$page]; + + return $registered; + } +} diff --git a/src/DonutChildExecutionsInterface.php b/src/DonutChildExecutionsInterface.php new file mode 100644 index 00000000..187bee05 --- /dev/null +++ b/src/DonutChildExecutionsInterface.php @@ -0,0 +1,41 @@ +body` (or overwrites the slot with a + * plain value pulled out of it) and `ResourceDonut::create()`'s body walk can no longer find + * it. `DonutRepository` reads the list back once the page's donut template has been built - + * late enough that every wrapper's `OnceRequest::generations()` is final - to tag the + * template with whichever generations executed but were never rendered as a hole. + * + * Bound `Scope::SINGLETON`, one instance per DI container (per request in the usual + * BEAR.Sunday bootstrap) - the same lifetime `UncacheableMark` already relies on. Keyed by + * object identity in a `WeakMap`, not by URI: two sibling pages embedding the same child URI + * must not share a slot, and an entry a `#[DonutCache]` write never reaches (a non-200 + * response, a cache-store failure) is simply never read back, not leaked - `WeakMap` drops it + * with the page object itself. + */ +interface DonutChildExecutionsInterface +{ + /** Record that `$once` was created for one of `$page`'s own `#[Embed]`s */ + public function register(ResourceObject $page, OnceRequest $once): void; + + /** + * Return and forget every wrapper registered for `$page` + * + * A page read exactly once: the second call for the same `$page` returns an empty list, + * so a write path that (for whatever reason) reads twice cannot double-tag. + * + * @return list + */ + public function take(ResourceObject $page): array; +} diff --git a/src/DonutRepository.php b/src/DonutRepository.php index b48002e0..35ace451 100644 --- a/src/DonutRepository.php +++ b/src/DonutRepository.php @@ -24,6 +24,8 @@ use Koriym\SemanticLogger\SemanticLoggerInterface; use Override; +use function array_key_exists; +use function array_merge; use function array_unique; use function array_values; use function explode; @@ -31,6 +33,11 @@ final readonly class DonutRepository implements DonutRepositoryInterface { + /** + * One dependency per collaborator, all constructor-injected. + * + * @SuppressWarnings("PHPMD.ExcessiveParameterList") + */ public function __construct( private QueryRepositoryInterface $queryRepository, private DonutHeaderSetter $donutHeaderSetter, @@ -41,6 +48,7 @@ public function __construct( private DonutRendererInterface $renderer, private UnstorableGateInterface $gate, private UncacheableMarkInterface $mark, + private DonutChildExecutionsInterface $childExecutions, private UriTagInterface $uriTag = new UriTag(), ) { } @@ -146,8 +154,21 @@ private function doPutStatic(ResourceObject $ro, int|null $ttl, int|null $sMaxAg $keys = new SurrogateKeys($ro->uri); $keys->addTag($ro); $declaredKeys = $this->getHeaderKeys($ro); - $templateKeys = $this->templateKeys($ro, $declaredKeys); - $donut = ResourceDonut::create($ro, $this->renderer, $keys, $sMaxAge, true, $this->mark)->withStorageState($ttl, $templateKeys); + // create() must run before unrenderedChildren(): it is what actually runs onGet()'s + // embeds and the page's own (string) $ro template render, so an OnceRequest's own + // generations() (issue #226) is not final until it returns. + $donut = ResourceDonut::create($ro, $this->renderer, $keys, $sMaxAge, true, $this->mark); + $unrenderedChildren = $this->unrenderedChildren($ro); + // The live response's own Surrogate-Key carries every unrendered child's tag too + // (issue #226 review): without this, purging such a child's tag invalidates the + // stored template but leaves the saved ETag/view entry - tagged from $keys alone - + // reachable, so a purge after this write still serves a cache_hit. + foreach ($unrenderedChildren as $childRo) { + $keys->addTag($childRo); + } + + $templateKeys = $this->templateKeys($ro, $declaredKeys, $this->childTags($unrenderedChildren)); + $donut = $donut->withStorageState($ttl, $templateKeys); $donut->render($ro, $this->renderer); // Checked against the still-unmutated header, before setHeaders() below runs // headerSetter/setCdnCacheControl - the same point QueryRepository checks at (issue @@ -195,8 +216,20 @@ private function doPutDonut(ResourceObject $ro, int|null $donutTtl): void $this->logger->event(new PutDonutContext((string) $ro->uri, $donutTtl, null)); $keys = new SurrogateKeys($ro->uri); $declaredKeys = $this->getHeaderKeys($ro); - $templateKeys = $this->templateKeys($ro, $declaredKeys); - $donut = ResourceDonut::create($ro, $this->renderer, $keys, $donutTtl, false, $this->mark)->withStorageState($donutTtl, $templateKeys); + // create() must run before unrenderedChildren(): see doPutStatic()'s own comment above. + $donut = ResourceDonut::create($ro, $this->renderer, $keys, $donutTtl, false, $this->mark); + $unrenderedChildren = $this->unrenderedChildren($ro); + // Same reasoning as doPutStatic()'s own loop: this page's live CDN Surrogate-Key + // header (set below via $keys->setSurrogateHeader()) must carry an unrendered + // child's tag too, or a CDN-level purge by that tag cannot reach this response - + // even though a #[DonutCache] page never gets a server-side ETag/view entry of its + // own (only the template, tagged via $templateKeys below, is ever saved here). + foreach ($unrenderedChildren as $childRo) { + $keys->addTag($childRo); + } + + $templateKeys = $this->templateKeys($ro, $declaredKeys, $this->childTags($unrenderedChildren)); + $donut = $donut->withStorageState($donutTtl, $templateKeys); $donut->render($ro, $this->renderer); $templateUnstorable = $this->gate->reasonExcludingHoleMarks($ro); $responseUnstorable = $this->gate->reason($ro); @@ -221,17 +254,77 @@ private function doPutDonut(ResourceObject $ro, int|null $donutTtl): void /** * The tags a donut template is stored under: its own URI tag (what `purge($uri)` invalidates - - * an untagged template is reachable by no `invalidateTags()` call, issue #185), plus whatever - * the resource declared. The children's tags are deliberately absent: a child's write must drop - * the content and leave the shell to be recomposed. + * an untagged template is reachable by no `invalidateTags()` call, issue #185), whatever the + * resource declared, plus (issue #226) whatever extra tags the caller found. The latter already + * excludes a child rendered as a hole - deliberately kept out so that child's own write drops + * only the rendered content and leaves the shell to be recomposed, not the whole template. * * @param list $declaredKeys + * @param list $extraChildTags + * + * @return list + */ + private function templateKeys(ResourceObject $ro, array $declaredKeys, array $extraChildTags = []): array + { + return array_values(array_unique([($this->uriTag)($ro->uri), ...$declaredKeys, ...$extraChildTags])); + } + + /** + * This page's own embedded children that executed (any generation - see + * `OnceRequest::generations()`) but were never substituted into the template as a hole + * (issue #226) - read once from whatever `EmbedOnceInterceptor` registered for $ro, + * regardless of whether a child is still reachable through $ro->body by now (onGet() + * may have unset() it, or overwritten the slot with a plain value pulled out of it). + * + * A child re-executed mid-`onGet()` (a `withQuery()`/`addQuery()`/`link*()` call) + * answers more than one generation here: every one of them ran and none but possibly + * the last could ever have become a hole, so every one but a rendered last needs its own + * tag - purging an EARLIER generation's tag must invalidate the page precisely as much + * as purging the latest's. + * + * Never runs a child to find this out: `OnceRequest::generations()` answers purely from + * each wrapper's own memo, populated only by whatever already read it. + * + * `take()` is destructive (the registry answers empty on a second call for the same + * $ro), so this reads it exactly once and the caller must reuse the result for every + * purpose - both the live response's `SurrogateKeys` and the template's own tags. + * + * @return list + */ + private function unrenderedChildren(ResourceObject $ro): array + { + $children = []; + foreach ($this->childExecutions->take($ro) as $once) { + foreach ($once->generations() as $generation) { + if ($generation['rendered']) { + continue; + } + + $children[] = $generation['resourceObject']; + } + } + + return $children; + } + + /** + * Flatten a list of children into their own URI tags and declared Surrogate-Keys + * + * @param list $children * * @return list */ - private function templateKeys(ResourceObject $ro, array $declaredKeys): array + private function childTags(array $children): array { - return array_values(array_unique([($this->uriTag)($ro->uri), ...$declaredKeys])); + $tags = []; + foreach ($children as $childRo) { + $tags[] = ($this->uriTag)($childRo->uri); + if (array_key_exists(Header::SURROGATE_KEY, $childRo->headers)) { + $tags = array_merge($tags, explode(' ', $childRo->headers[Header::SURROGATE_KEY])); + } + } + + return array_values(array_unique($tags)); } /** diff --git a/src/DonutRequest.php b/src/DonutRequest.php index 104b4cc2..be838867 100644 --- a/src/DonutRequest.php +++ b/src/DonutRequest.php @@ -33,17 +33,32 @@ public function __toString(): string // string before the hole runs, usable both as enterHole()'s key and, unchanged, // for the placeholder/view mapping that ResourceDonut::refresh() later re-fetches // by (issue #222: a query-less key here would also let two sibling holes on the - // same path but different query args collide on one owner slot). + // same path but different query args collide on one owner slot). Verified equal to + // the executed object's own URI below: __invoke()'s first statement does the exact + // same sync from the exact same $this->query, so the two can never disagree. $uri = $this->request->toUri(); $this->mark->enterHole($uri); try { - $view = (string) ($this->request)(); + // The single execution this hole's content and tags both come from - not a + // second, fresh call through $this->request (which for a plain, unwrapped + // AbstractRequest would double-execute a non-idempotent child, and for an + // OnceRequest would still be correct, but there is no reason to prefer a + // second call over the result already in hand). + $executed = ($this->request)(); + $view = (string) $executed; } finally { $this->mark->leaveHole($uri); } - $this->etags->addTag($this->request->resourceObject); + $this->etags->addTag($executed); $this->donutStorage->setView($uri, $view); + if ($this->request instanceof OnceRequest) { + // Excludes this child from DonutRepository's issue #226 "executed but not + // rendered" sweep: a rendered hole's tag already landed on $this->etags above, + // and (by design, see DonutRepository::templateKeys()) a rendered hole's tag + // stays out of the template's own storage tags either way. + $this->request->markRendered(); + } return ResourceDonut::placeholder($uri); } diff --git a/src/EmbedOnceInterceptor.php b/src/EmbedOnceInterceptor.php new file mode 100644 index 00000000..b5695b2b --- /dev/null +++ b/src/EmbedOnceInterceptor.php @@ -0,0 +1,127 @@ +body` - this one needs to run after. Ray.Aop's own onion ordering gives + * that for free rather than by binding order: `EmbedInterceptor`'s method matcher is a bare + * `annotatedWith(Embed::class)`, which `Ray\Aop\MethodMatch::onionOrderMatch()` binds + * synchronously while walking the method's attributes in declared order - strictly before it + * returns the leftover (including every matcher built with `logicalAnd()`/`logicalOr()`, this + * one's method matcher among them) to be bound in whatever order they were registered. A + * method carrying `#[Embed(...)]` therefore always gets `EmbedInterceptor` ahead of this one in + * its interceptor chain, regardless of whether `DonutCacheModule` or `EmbedResourceModule` is + * installed first - verified for both orders in `EmbedOnceInterceptorInstallOrderTest`. + * + * The `declaredEmbedRels()` check is the fallback for everything that guarantee does not + * cover - a third-party AOP container with different ordering rules, or a future Ray.Aop + * release that changes them: if a declared `#[Embed]`'s `rel` is still missing from the body, + * swapping would either skip it (leaving the double-execution/#226 gap open for that embed) + * or, if run before `EmbedInterceptor` instead of after, race it. Either way nothing here can + * safely proceed, so it warns and leaves the body exactly as it found it - `onGet()` and the + * eventual donut render still work, just without once-only semantics for that one request. + */ +final class EmbedOnceInterceptor implements MethodInterceptor +{ + public function __construct( + private readonly DonutChildExecutionsInterface $childExecutions, + ) { + } + + #[Override] + public function invoke(MethodInvocation $invocation) + { + $ro = $invocation->getThis(); + assert($ro instanceof ResourceObject); + $this->swap($ro, $invocation); + + return $invocation->proceed(); + } + + /** @param MethodInvocation $invocation */ + private function swap(ResourceObject $ro, MethodInvocation $invocation): void + { + $body = $ro->body; + foreach ($this->declaredEmbedRels($invocation) as $rel) { + if (is_array($body) && array_key_exists($rel, $body)) { + continue; + } + + // The real too-early state: EmbedInterceptor::prepareBody() is what turns a + // fresh ResourceObject's null body into [] - if this runs first, $body is + // still null here, never an array missing just one key. + trigger_error(sprintf( + '%s: #[Embed(rel: "%s")] on %s::onGet() was not in the body yet - ' . + 'EmbedOnceInterceptor ran before EmbedInterceptor (check module install order); skipping the swap for this request', + self::class, + $rel, + $ro::class, + ), E_USER_WARNING); + + return; + } + + if (! is_array($body)) { + return; + } + + /** @psalm-suppress MixedAssignment $child */ + foreach ($body as $rel => $child) { + if (! $child instanceof Request) { + continue; + } + + $once = new OnceRequest($child); + $body[$rel] = $once; + $this->childExecutions->register($ro, $once); + } + + $ro->body = $body; + } + + /** + * The `#[Embed]` relations declared on the invoked method, `_self` excluded - a self-link + * embed (`EmbedInterceptor::linkSelf()`) never reaches `$ro->body`, so its absence there + * is not a sign anything ran out of order. + * + * @param MethodInvocation $invocation + * + * @return list + */ + private function declaredEmbedRels(MethodInvocation $invocation): array + { + $rels = []; + foreach ($invocation->getMethod()->getAnnotations() as $annotation) { + if (! $annotation instanceof Embed || $annotation->rel === '_self') { + continue; + } + + $rels[] = $annotation->rel; + } + + return $rels; + } +} diff --git a/src/OnceRequest.php b/src/OnceRequest.php new file mode 100644 index 00000000..a3137948 --- /dev/null +++ b/src/OnceRequest.php @@ -0,0 +1,257 @@ +body[$rel]` for every + * `#[Embed]`. Left as-is, that `Request` can be executed twice for one page render: once + * when the page's own `onGet()` reads a value out of it (`offsetGet()`/`__get()`, which goes + * through `AbstractRequest`'s private memo), and once more when `DonutRequest::__toString()` + * renders it as a hole (`($this->request)()`, which calls `__invoke()` directly and so + * bypasses that memo - see `DonutRequest`). For a non-idempotent child (a counter, a random + * value) the two runs disagree, and the donut template ends up holding a different value + * than whatever the page's own body already used - the MISS render and the stored render + * diverge. + * + * `EmbedOnceInterceptor` swaps every embedded `Request` for one of these before `onGet()` + * runs, so every access path - `onGet()` reading the body, and the later donut hole render - + * shares the one execution. `__invoke()` is the only path that actually calls the inner + * `Request`; every other `AbstractRequest` accessor (`__toString()`, `offsetGet()`, `__get()`, + * `getIterator()`, `jsonSerialize()`) already funnels through `__invoke()` via + * `AbstractRequest`'s own private memo, and callers that invoke `($wrapper)()` directly - + * `DonutRequest` among them - hit this class's own memo instead of the inner `Request`'s, + * closing exactly the gap described above. + * + * `withQuery()`/`addQuery()`/`link*()` discard the memo: a child whose query or links the + * page explicitly changes mid-`onGet()` is expected to run again under the new request, not + * silently answer the previous one. Each such re-execution is a new *generation* - and + * `$executions` keeps every one of them, not only the current one: `onGet()` reading a + * child, re-querying it, and reading it again (never leaving either result as a hole) must + * tag both generations' own URIs/surrogate keys for `DonutRepository` (issue #226), not just + * the last - purging the FIRST generation's tag must invalidate the page just as much as + * purging the second's. + * + * `AbstractRequest` itself never gets this treatment and is never asked to: a plain + * `$resource->get()`/`newRequest()` call in application code, or a `#[Cacheable]` resource + * read outside a donut page, keeps running every time it is invoked, exactly as before. + * + * @psalm-suppress PropertyNotSetInConstructor `$invoker`/`$linker` are AbstractRequest's own + * construction-only properties, used only by its default `__invoke()` - overridden here + * to always delegate to the inner `Request` instead, so neither is ever read. + * @psalm-type Execution = array{resourceObject: ResourceObject, rendered: bool} + */ +final class OnceRequest extends AbstractRequest +{ + private ResourceObject|null $executed = null; + + /** @var list every generation's result, oldest first */ + private array $generations = []; + + /** @var array parallel to $generations - whether DonutRequest rendered that one as a hole */ + private array $rendered = []; + + public function __construct(private readonly Request $request) + { + $this->resourceObject = $request->resourceObject; + $this->uri = $request->uri; + $this->method = $request->method; + $this->query = $request->query; + $this->links = $request->links; + $this->in = $request->in; + } + + /** + * {@inheritDoc} + * + * The single execution point for the current generation: runs the inner `Request` the + * first time this is called (directly, or through `AbstractRequest`'s own memo via + * `__toString()`/`offsetGet()`/etc.) and returns that same result on every later call, + * until a query or link change starts a new generation. + */ + #[Override] + public function __invoke(array|null $query = null): ResourceObject + { + if ($query !== null) { + $this->request->addQuery($query); + $this->resync(); + } + + if ($this->executed === null) { + $this->executed = ($this->request)(); + $this->generations[] = $this->executed; + $this->rendered[] = false; + } + + return $this->executed; + } + + /** + * {@inheritDoc} + * + * Mirrors `Request::__get()`'s own DSL exactly. `eager`/`lazy` is genuinely new here - + * `AbstractRequest`'s default `__get()` has no such case and would instead try to read + * a non-existent `eager`/`lazy` property off the memoized `ResourceObject`. The `code`/ + * `headers`/`body` branch is a deliberate, explicit match for `Request`'s own contract + * rather than a correctness fix: `AbstractRequest`'s default already funnels every + * unknown property through its own private memo's `invoke()`, which calls this class's + * overridden `__invoke()` just the same, so it already answers from the one execution. + */ + #[Override] + public function __get(string $name): mixed + { + if ($name === 'eager' || $name === 'lazy') { + $this->in = $name; + $this->request->in = $name; + + return $this; + } + + if (in_array($name, ['code', 'headers', 'body'], true)) { + return ($this)()->{$name}; + } + + throw new OutOfRangeException($name); + } + + /** {@inheritDoc} */ + #[Override] + public function withQuery(array $query): RequestInterface + { + $this->request->withQuery($query); + $this->resync(); + + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function addQuery(array $query): RequestInterface + { + $this->request->addQuery($query); + $this->resync(); + + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function toUri(): string + { + return $this->request->toUri(); + } + + /** {@inheritDoc} */ + #[Override] + public function toUriWithMethod(): string + { + return $this->request->toUriWithMethod(); + } + + /** {@inheritDoc} */ + #[Override] + public function linkSelf(string $linkKey): RequestInterface + { + $this->request->linkSelf($linkKey); + $this->resync(); + + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function linkNew(string $linkKey): RequestInterface + { + $this->request->linkNew($linkKey); + $this->resync(); + + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function linkCrawl(string $linkKey): RequestInterface + { + $this->request->linkCrawl($linkKey); + $this->resync(); + + return $this; + } + + /** Whether the current generation's inner `Request` has run yet */ + public function hasExecuted(): bool + { + return $this->executed !== null; + } + + /** + * Record that the current generation's execution was substituted into the donut + * template as a hole + * + * Called by `DonutRequest::__toString()` once it has rendered this generation - so that + * `DonutRepository` (issue #226) does not also tag it as a body-only, non-hole read: the + * hole already carries its own tag (`DonutRequest` adds it directly), and a rendered + * hole's tag is deliberately kept out of the donut template's own storage tags + * (`DonutRepository::templateKeys()`) so that the child's own write drops the rendered + * content and leaves the shell to be recomposed, not the whole template. Only ever the + * LATEST generation can be a hole - the template at render time holds whatever is + * currently in `$ro->body[$rel]`, which a superseded earlier generation no longer is. + * + * `DonutRequest` only ever calls this right after invoking this request, so there is + * always at least one generation to mark by the time this runs. + */ + public function markRendered(): void + { + $this->rendered[count($this->rendered) - 1] = true; + } + + /** + * Every generation this request has run across every withQuery()/addQuery()/link*() + * call, oldest first, paired with whether `DonutRequest` rendered it as a hole + * + * Read once by `DonutRepository` (issue #226) to tag every generation that executed but + * was never rendered as a hole - not only the current one: `onGet()` re-querying an + * embed mid-request (see class docblock) leaves an earlier generation's own URI/ + * surrogate key just as reachable through `purge()` as the latest's. + * + * @return list + */ + public function generations(): array + { + $result = []; + foreach ($this->generations as $i => $resourceObject) { + $result[] = ['resourceObject' => $resourceObject, 'rendered' => $this->rendered[$i]]; + } + + return $result; + } + + /** Discard the current generation's memo and mirror the inner `Request`'s query/links after they changed */ + private function resync(): void + { + $this->query = $this->request->query; + $this->links = $this->request->links; + $this->executed = null; + // $this->generations/$this->rendered are NOT cleared here - issue #226 needs every + // generation's own tag, not only the one still current after this change. + // AbstractRequest's own private invoke() short-circuits on $result !== null, + // bypassing our overridden __invoke() entirely for every accessor that goes + // through it (offsetGet(), __get(), getIterator(), __toString(), jsonSerialize()) - + // left stale here, a withQuery()/addQuery()/link*() call after such a read would + // keep answering the pre-change ResourceObject forever. + $this->result = null; + } +} diff --git a/src/ResourceBodyEvaluator.php b/src/ResourceBodyEvaluator.php index a961de23..4bc9ebac 100644 --- a/src/ResourceBodyEvaluator.php +++ b/src/ResourceBodyEvaluator.php @@ -74,11 +74,21 @@ private function copyForStore(mixed $item): mixed * The copy is the storage boundary: the memoized object stays reachable through the * live request and the pool may hold it by reference, while the recursion above * rewrites body in place. + * + * AbstractRequest's own contract is that jsonSerialize() answers that memoized + * ResourceObject - but a subclass may override it to mean something else (BEAR\Async\ + * AsyncRequest decodes an already-flushed JSON view into an array instead, to support + * PendingRequests' own parallel-execution protocol). Materializing such a request would + * need that package's own flush semantics, which is out of scope here, so it is left as + * the live request rather than forced through asResourceObject() and crashed on. */ - private function materialize(RequestInterface $request): ResourceObject + private function materialize(RequestInterface $request): mixed { if ($request instanceof AbstractRequest) { - return clone $this->asResourceObject($request->jsonSerialize()); + /** @psalm-suppress MixedAssignment $memo - mixed when jsonSerialize() answers something other than the memo */ + $memo = $request->jsonSerialize(); + + return $memo instanceof ResourceObject ? clone $memo : $request; } return $this->asResourceObject($request()); diff --git a/tests/EmbedOnceInterceptorInstallOrderTest.php b/tests/EmbedOnceInterceptorInstallOrderTest.php new file mode 100644 index 00000000..3d7fa85f --- /dev/null +++ b/tests/EmbedOnceInterceptorInstallOrderTest.php @@ -0,0 +1,104 @@ +assertOnceSemanticsHold($module); + } + + public function testReversedOrderResourceModuleInstalledBeforeQueryRepositoryModule(): void + { + $module = new class extends AbstractModule { + protected function configure(): void + { + $this->install(new ResourceModule('FakeVendor\HelloWorld')); + $this->install(new QueryRepositoryModule()); + } + }; + $this->assertOnceSemanticsHold($module); + } + + private function assertOnceSemanticsHold(AbstractModule $module): void + { + $module->override(new class extends AbstractModule { + protected function configure(): void + { + $this->bind(AdapterInterface::class)->annotatedWith(ResourceObjectPool::class)->to(ArrayAdapter::class); + $this->bind(AdapterInterface::class)->annotatedWith(EtagPool::class)->to(ArrayAdapter::class); + $this->bind(SemanticLoggerInterface::class)->annotatedWith(CacheLog::class)->to(NullSemanticLogger::class); + } + }); + $module->override(new TwigModule([dirname(__DIR__) . '/tests/Fake/fake-app/var/templates'])); + + $warnings = []; + set_error_handler(static function (int $errno, string $message) use (&$warnings): bool { + if ($errno === E_USER_WARNING) { + $warnings[] = $message; + } + + return true; + }); + try { + $injector = new Injector($module, __DIR__ . '/tmp'); + $resource = $injector->getInstance(ResourceInterface::class); + $ro = $resource->get(self::COUNTER_DONUT); + } finally { + restore_error_handler(); + } + + $this->assertSame([], $warnings, 'EmbedOnceInterceptor must never warn that it ran before EmbedInterceptor, in either install order'); + $this->assertSame(1, Counter::$calls, 'the embed must still execute exactly once, in either install order'); + $view = (string) $ro->view; + $this->assertStringContainsString('read:1', $view); + $this->assertStringContainsString('call:1', $view); + } +} diff --git a/tests/EmbedOnceInterceptorWarningTest.php b/tests/EmbedOnceInterceptorWarningTest.php new file mode 100644 index 00000000..41b527cd --- /dev/null +++ b/tests/EmbedOnceInterceptorWarningTest.php @@ -0,0 +1,154 @@ +body = null; + + $invocation = new class ($ro) implements MethodInvocation { + public function __construct(private readonly ResourceObject $ro) + { + } + + public function proceed(): mixed + { + return $this->ro; + } + + public function getThis(): ResourceObject + { + return $this->ro; + } + + public function getArguments(): ArrayObject + { + return new ArrayObject(); + } + + public function getNamedArguments(): ArrayObject + { + return new ArrayObject(); + } + + public function getMethod(): ReflectionMethod + { + return new ReflectionMethod(CounterDonut::class, 'onGet'); + } + }; + + $warnings = []; + set_error_handler(static function (int $errno, string $message) use (&$warnings): bool { + if ($errno === E_USER_WARNING) { + $warnings[] = $message; + } + + return true; + }); + try { + $interceptor = new EmbedOnceInterceptor(new DonutChildExecutions()); + $result = $interceptor->invoke($invocation); + } finally { + restore_error_handler(); + } + + $this->assertSame($ro, $result, 'proceed() still runs - the request is served, just without once-only semantics'); + $this->assertNull($ro->body, 'nothing is swapped (or even initialized) when this runs before EmbedInterceptor'); + $this->assertCount(1, $warnings); + $this->assertTrue( + str_contains($warnings[0], 'counter') && str_contains($warnings[0], 'EmbedOnceInterceptor ran before EmbedInterceptor'), + sprintf('unexpected warning message: %s', $warnings[0]), + ); + } + + public function testSelfLinkOnlyEmbedLeavesANullBodyAlone(): void + { + // _self never reaches $ro->body (EmbedInterceptor::linkSelf()), so + // declaredEmbedRels() is empty for this class and the declared-rel loop never + // runs - the one real shape that reaches swap()'s is_array($body) fallback + // instead, with body still null if nothing else has touched it yet. + $ro = new SelfLinkOnlyDonut(); + $ro->body = null; + + $invocation = new class ($ro) implements MethodInvocation { + public function __construct(private readonly ResourceObject $ro) + { + } + + public function proceed(): mixed + { + return $this->ro; + } + + public function getThis(): ResourceObject + { + return $this->ro; + } + + public function getArguments(): ArrayObject + { + return new ArrayObject(); + } + + public function getNamedArguments(): ArrayObject + { + return new ArrayObject(); + } + + public function getMethod(): ReflectionMethod + { + return new ReflectionMethod(SelfLinkOnlyDonut::class, 'onGet'); + } + }; + + $warnings = []; + set_error_handler(static function (int $errno, string $message) use (&$warnings): bool { + if ($errno === E_USER_WARNING) { + $warnings[] = $message; + } + + return true; + }); + try { + $interceptor = new EmbedOnceInterceptor(new DonutChildExecutions()); + $result = $interceptor->invoke($invocation); + } finally { + restore_error_handler(); + } + + $this->assertSame($ro, $result); + $this->assertNull($ro->body, 'nothing to swap - a self-link embed has no body entry to begin with'); + $this->assertSame([], $warnings, 'no declared rel is ever missing, since _self never counts as one'); + } +} diff --git a/tests/EmbeddedChildMaterializationTest.php b/tests/EmbeddedChildMaterializationTest.php index 0aca8267..29a1e0d1 100644 --- a/tests/EmbeddedChildMaterializationTest.php +++ b/tests/EmbeddedChildMaterializationTest.php @@ -12,6 +12,7 @@ use FakeVendor\HelloWorld\Resource\Page\Dep\CountingChild; use PHPUnit\Framework\TestCase; use Ray\Di\Injector; +use ReflectionMethod; use function assert; use function is_array; @@ -151,6 +152,37 @@ public function testRequestWithoutMemoIsMaterializedByRunningIt(): void $this->assertSame(1, $request->invoked); } + /** + * An AbstractRequest whose jsonSerialize() does not answer a memoized ResourceObject + * (BEAR\Async\AsyncRequest's own shape, see ResourceBodyEvaluator::materialize()) is + * left as the live request instead of crashing: a blind assert($memo instanceof + * ResourceObject) would raise AssertionError here (zend.assertions=1 in this suite), + * and even with assertions off, `clone` on the decoded array would be a fatal error. + * + * `AbstractRequest::jsonSerialize()` is declared `: ResourceObject` in older bear/resource + * releases (this project's own lowest-supported one among them) and `: mixed` in newer + * ones - a fixture overriding it to answer an array is only loadable under the latter, so + * this skips rather than fatal-erroring PHP's own class-declaration check under the former + * (`FakeAsyncLikeRequest` is only autoloaded past this guard). + */ + public function testAsyncLikeRequestIsLeftUnmaterializedInsteadOfCrashing(): void + { + $returnType = (new ReflectionMethod(AbstractRequest::class, 'jsonSerialize'))->getReturnType(); + if ((string) $returnType !== 'mixed') { + self::markTestSkipped('this bear/resource release declares AbstractRequest::jsonSerialize(): ' . $returnType . ' - a request answering non-ResourceObject data from it cannot be constructed here'); + } + + $ro = new class extends ResourceObject { + }; + $ro->uri = new Uri('app://self/async-like'); + $request = new FakeAsyncLikeRequest($ro); + + $body = (new ResourceBodyEvaluator())(['child' => $request]); + + assert(is_array($body)); + $this->assertSame($request, $body['child'], 'not materialized - the live request is kept, not wrapped into a ResourceObject'); + } + /** * A ResourceObject sitting in the body is copied before its own body is rewritten * diff --git a/tests/Fake/FakeAsyncLikeRequest.php b/tests/Fake/FakeAsyncLikeRequest.php new file mode 100644 index 00000000..ead90abc --- /dev/null +++ b/tests/Fake/FakeAsyncLikeRequest.php @@ -0,0 +1,95 @@ + 'from-pending-requests']; + } + + /** + * Never exercised by ResourceBodyEvaluator (it reads jsonSerialize() only) - stubbed + * out just to satisfy AbstractRequest's abstract contract for a concrete subclass. + */ + #[Override] + public function withQuery(array $query): RequestInterface + { + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function addQuery(array $query): RequestInterface + { + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function toUri(): string + { + return (string) $this->uri; + } + + /** {@inheritDoc} */ + #[Override] + public function toUriWithMethod(): string + { + return $this->toUri(); + } + + /** {@inheritDoc} */ + #[Override] + public function linkSelf(string $linkKey): RequestInterface + { + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function linkNew(string $linkKey): RequestInterface + { + return $this; + } + + /** {@inheritDoc} */ + #[Override] + public function linkCrawl(string $linkKey): RequestInterface + { + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/App/Counter.php b/tests/Fake/fake-app/src/Resource/App/Counter.php new file mode 100644 index 00000000..a0b8229e --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/App/Counter.php @@ -0,0 +1,29 @@ +get()` and + * every embedded read runs this, the same as any other call. Used to prove that + * `OnceRequest`'s once-only semantics apply only when `EmbedOnceInterceptor` actually wraps + * an embed, never to a direct call. + */ +class Counter extends ResourceObject +{ + public static int $calls = 0; + + public function onGet(string|null $tag = null): static + { + self::$calls++; + $this->body = [ + 'call' => self::$calls, + 'tag' => $tag, + ]; + + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Html/BodyOnlyChildCacheableResponse.php b/tests/Fake/fake-app/src/Resource/Page/Html/BodyOnlyChildCacheableResponse.php new file mode 100644 index 00000000..7b11964a --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Html/BodyOnlyChildCacheableResponse.php @@ -0,0 +1,27 @@ +body['description'] = $this->body['category']['comment']; + unset($this->body['category']); + + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Html/BodyOnlyChildDonut.php b/tests/Fake/fake-app/src/Resource/Page/Html/BodyOnlyChildDonut.php new file mode 100644 index 00000000..1a1f2502 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Html/BodyOnlyChildDonut.php @@ -0,0 +1,31 @@ +body` no longer holds any trace it ever existed. + * - `unused` is declared but never touched at all - onGet() never reads it, the template + * never renders it - proving it contributes no tag and, more importantly, never runs + * ("never execute a child just to tag it"). + */ +#[DonutCache] +class BodyOnlyChildDonut extends ResourceObject +{ + #[Embed(rel: 'category', src: 'page://self/html/comment')] + #[Embed(rel: 'unused', src: 'app://self/counter')] + public function onGet(): static + { + $this->body['description'] = $this->body['category']['comment']; + unset($this->body['category']); + + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Html/CounterCacheMethodLevel.php b/tests/Fake/fake-app/src/Resource/Page/Html/CounterCacheMethodLevel.php new file mode 100644 index 00000000..95c58530 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Html/CounterCacheMethodLevel.php @@ -0,0 +1,29 @@ + */ + public $body = ['label' => 'counter-cache-method-level']; + + #[CacheableResponse] + #[Embed(rel: 'counter', src: 'app://self/counter')] + public function onGet(): static + { + $this->body['read'] = $this->body['counter']['call']; + + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Html/CounterDonut.php b/tests/Fake/fake-app/src/Resource/Page/Html/CounterDonut.php new file mode 100644 index 00000000..24dd4095 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Html/CounterDonut.php @@ -0,0 +1,32 @@ +|null */ + public $body = ['label' => 'counter-donut']; + + #[Embed(rel: 'counter', src: 'app://self/counter')] + public function onGet(): static + { + $this->body['read'] = $this->body['counter']['call']; + + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Html/RequeryDonut.php b/tests/Fake/fake-app/src/Resource/Page/Html/RequeryDonut.php new file mode 100644 index 00000000..6da53096 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Html/RequeryDonut.php @@ -0,0 +1,37 @@ +body['item']['call']; + $this->body['item']->withQuery(['tag' => 'requeried']); + $second = $this->body['item']['call']; + $secondTag = $this->body['item']['tag']; + unset($this->body['item']); + + $this->body['first'] = $first; + $this->body['second'] = $second; + $this->body['secondTag'] = $secondTag; + + return $this; + } +} diff --git a/tests/Fake/fake-app/src/Resource/Page/Html/SelfLinkOnlyDonut.php b/tests/Fake/fake-app/src/Resource/Page/Html/SelfLinkOnlyDonut.php new file mode 100644 index 00000000..cdcc2111 --- /dev/null +++ b/tests/Fake/fake-app/src/Resource/Page/Html/SelfLinkOnlyDonut.php @@ -0,0 +1,24 @@ +body` at all. declaredEmbedRels() filters + * `_self` out, so this is the one real shape where EmbedOnceInterceptor::swap() reaches + * its is_array($body) fallback (as opposed to the declared-rel loop) with a genuinely + * null body if nothing else has run yet. + */ +#[DonutCache] +class SelfLinkOnlyDonut extends ResourceObject +{ + #[Embed(rel: '_self', src: 'page://self/html/self-link-only-donut')] + public function onGet(): static + { + return $this; + } +} diff --git a/tests/Fake/fake-app/var/templates/App/Counter.html.twig b/tests/Fake/fake-app/var/templates/App/Counter.html.twig new file mode 100644 index 00000000..5208b858 --- /dev/null +++ b/tests/Fake/fake-app/var/templates/App/Counter.html.twig @@ -0,0 +1 @@ +call:{{ call }} tag:{{ tag }} \ No newline at end of file diff --git a/tests/Fake/fake-app/var/templates/Page/Html/BodyOnlyChildCacheableResponse.html.twig b/tests/Fake/fake-app/var/templates/Page/Html/BodyOnlyChildCacheableResponse.html.twig new file mode 100644 index 00000000..8342b6ba --- /dev/null +++ b/tests/Fake/fake-app/var/templates/Page/Html/BodyOnlyChildCacheableResponse.html.twig @@ -0,0 +1 @@ +desc:{{ description }} \ No newline at end of file diff --git a/tests/Fake/fake-app/var/templates/Page/Html/BodyOnlyChildDonut.html.twig b/tests/Fake/fake-app/var/templates/Page/Html/BodyOnlyChildDonut.html.twig new file mode 100644 index 00000000..8342b6ba --- /dev/null +++ b/tests/Fake/fake-app/var/templates/Page/Html/BodyOnlyChildDonut.html.twig @@ -0,0 +1 @@ +desc:{{ description }} \ No newline at end of file diff --git a/tests/Fake/fake-app/var/templates/Page/Html/CounterCacheMethodLevel.html.twig b/tests/Fake/fake-app/var/templates/Page/Html/CounterCacheMethodLevel.html.twig new file mode 100644 index 00000000..af007d9a --- /dev/null +++ b/tests/Fake/fake-app/var/templates/Page/Html/CounterCacheMethodLevel.html.twig @@ -0,0 +1 @@ +read:{{ read }}{{ counter }} \ No newline at end of file diff --git a/tests/Fake/fake-app/var/templates/Page/Html/CounterDonut.html.twig b/tests/Fake/fake-app/var/templates/Page/Html/CounterDonut.html.twig new file mode 100644 index 00000000..af007d9a --- /dev/null +++ b/tests/Fake/fake-app/var/templates/Page/Html/CounterDonut.html.twig @@ -0,0 +1 @@ +read:{{ read }}{{ counter }} \ No newline at end of file diff --git a/tests/Fake/fake-app/var/templates/Page/Html/RequeryDonut.html.twig b/tests/Fake/fake-app/var/templates/Page/Html/RequeryDonut.html.twig new file mode 100644 index 00000000..d16cb3a1 --- /dev/null +++ b/tests/Fake/fake-app/var/templates/Page/Html/RequeryDonut.html.twig @@ -0,0 +1 @@ +first:{{ first }} second:{{ second }} secondTag:{{ secondTag }} \ No newline at end of file diff --git a/tests/OnceRequestBehaviorTest.php b/tests/OnceRequestBehaviorTest.php new file mode 100644 index 00000000..52efd27e --- /dev/null +++ b/tests/OnceRequestBehaviorTest.php @@ -0,0 +1,248 @@ +override(new TwigModule([dirname(__DIR__) . '/tests/Fake/fake-app/var/templates'])); + $injector = new Injector($module, __DIR__ . '/tmp'); + $this->resource = $injector->getInstance(ResourceInterface::class); + $this->queryRepository = $injector->getInstance(QueryRepositoryInterface::class); + $this->logger = $injector->getInstance(SemanticLoggerInterface::class, CacheLog::class); + } + + public function testEmbeddedChildExecutesOnceAcrossBodyReadAndHoleRender(): void + { + $ro = $this->resource->get(self::COUNTER_DONUT); + $this->flushAndValidate($this->logger); + + $this->assertSame(1, Counter::$calls, 'read in onGet() and rendered as a hole: one execution total'); + $view = (string) $ro->view; + $this->assertStringContainsString('read:1', $view); + $this->assertStringContainsString('call:1', $view, 'the hole was substituted from the very same execution onGet() already read'); + } + + public function testWithQueryAfterAReadReExecutesThroughOffsetGet(): void + { + $ro = $this->resource->get(self::REQUERY_DONUT); + $this->flushAndValidate($this->logger); + + // Both reads went through offsetGet(), never a direct ($request)() call - if + // OnceRequest::resync() left AbstractRequest's own private $result memo stale, + // both would read 1 and $calls would stay 1. + $this->assertSame(2, Counter::$calls, 'withQuery() after a read must force a second, real execution'); + $view = (string) $ro->view; + $this->assertStringContainsString('first:1', $view); + $this->assertStringContainsString('second:2', $view); + $this->assertStringContainsString('secondTag:requeried', $view, 'the second read reflects the new query, not a stale memo of the first'); + } + + public function testEveryGenerationTagsTheStoredTemplateAndPurgingTheFirstOneInvalidatesThePage(): void + { + $this->resource->get(self::REQUERY_DONUT); + $this->flushAndValidate($this->logger); + + $this->resource->get(self::REQUERY_DONUT); + $warm = $this->flushAndValidate($this->logger); + $warmClose = self::scopeCloseOf($warm, 'get', self::REQUERY_DONUT); + $this->assertNotNull($warmClose); + $this->assertSame('cache_hit', $warmClose[0], 'the stored template is warm before the purge'); + + // Only the FIRST generation's tag (the pre-withQuery() counter, no query) - never + // the second's (tag=requeried). Issue #226 review: OnceRequest discarded its own + // execution history on resync(), so only the LAST generation's tag ever reached + // the stored template - purging an earlier generation's tag left the page serving + // a stale cache_hit. + $this->queryRepository->purge(new Uri(self::COUNTER)); + $this->flushAndValidate($this->logger); + + $this->resource->get(self::REQUERY_DONUT); + $afterPurge = $this->flushAndValidate($this->logger); + $closeAfterPurge = self::scopeCloseOf($afterPurge, 'get', self::REQUERY_DONUT); + $this->assertNotNull($closeAfterPurge); + $this->assertSame('cache_miss', $closeAfterPurge[0], "purging the FIRST generation's tag alone must invalidate the page - every generation is tagged, not only the last"); + } + + public function testExplicitResourceCallAlwaysExecutes(): void + { + $this->resource->get(self::COUNTER); + $this->resource->get(self::COUNTER); + $this->resource->get(self::COUNTER); + + $this->assertSame(3, Counter::$calls, 'a direct, non-embedded call is never wrapped - every call is a real roll of the dice'); + } + + public function testCacheableResourceOutsideDonutPageIsUnaffected(): void + { + $this->resource->get(self::CHILD); + $miss = $this->flushAndValidate($this->logger); + $missClose = self::scopeCloseOf($miss, 'get', self::CHILD); + $this->assertNotNull($missClose); + $this->assertSame('cache_miss', $missClose[0]); + + $this->resource->get(self::CHILD); + $hit = $this->flushAndValidate($this->logger); + $hitClose = self::scopeCloseOf($hit, 'get', self::CHILD); + $this->assertNotNull($hitClose); + $this->assertSame('cache_hit', $hitClose[0], '#[Cacheable] outside any donut page caches exactly as before - untouched by EmbedOnceInterceptor'); + } + + /** + * The doPutStatic()/#[CacheableResponse] counterpart of + * testPurgingTheBodyOnlyChildInvalidatesThePage(): that test proves the #[DonutCache] + * TEMPLATE is tagged; this one proves the live response's own stored VIEW/ETag - saved + * through a separate SurrogateKeys instance on this path (issue #226 review) - is too. + */ + public function testPurgingTheBodyOnlyChildInvalidatesTheCacheableResponseViewToo(): void + { + $this->resource->get(self::BODY_ONLY_CHILD_CACHEABLE_RESPONSE); + $this->flushAndValidate($this->logger); + + $this->resource->get(self::BODY_ONLY_CHILD_CACHEABLE_RESPONSE); + $warm = $this->flushAndValidate($this->logger); + $warmClose = self::scopeCloseOf($warm, 'get', self::BODY_ONLY_CHILD_CACHEABLE_RESPONSE); + $this->assertNotNull($warmClose); + $this->assertSame('cache_hit', $warmClose[0], 'the stored view/ETag is warm before the purge'); + + $this->queryRepository->purge(new Uri(self::CHILD)); + $this->flushAndValidate($this->logger); + + $this->resource->get(self::BODY_ONLY_CHILD_CACHEABLE_RESPONSE); + $tree = $this->flushAndValidate($this->logger); + $close = self::scopeCloseOf($tree, 'get', self::BODY_ONLY_CHILD_CACHEABLE_RESPONSE); + $this->assertNotNull($close); + $this->assertSame('cache_miss', $close[0], "purging the body-only child's tag must invalidate the live response's own stored view/ETag too, not only a #[DonutCache] template"); + } + + public function testBodyOnlyChildTagsTheStoredTemplateAndNeverTouchedEmbedDoesNot(): void + { + $this->resource->get(self::BODY_ONLY_CHILD_DONUT); + $tree = $this->flushAndValidate($this->logger); + + $this->assertSame(0, Counter::$calls, "the 'unused' embed is never read and never rendered - it must never run just to be tagged"); + + $saveDonut = self::eventContextsJsonOf($tree, 'save_donut', self::BODY_ONLY_CHILD_DONUT); + $this->assertCount(1, $saveDonut); + $this->assertStringContainsString('"' . self::CHILD_URI_TAG . '"', $saveDonut[0], "the body-only child's own URI tag lands on the stored template (issue #226)"); + $this->assertStringContainsString('"' . self::CHILD_SURROGATE_KEY . '"', $saveDonut[0], "the body-only child's own declared Surrogate-Key lands on the stored template too"); + $this->assertStringNotContainsString('"' . self::COUNTER_URI_TAG . '"', $saveDonut[0], 'the never-touched embed contributes no tag'); + } + + /** + * The #[DonutCache] path's own live-response counterpart: no server-side ETag/view is + * ever saved for this path (doPutDonut() never calls saveEtag()/saveView()), but the + * CDN-facing Surrogate-Key header on the response itself (issue #226 review) must still + * carry the body-only child's tag, or a CDN-level purge by that tag cannot reach it. + */ + public function testDonutCachePathAlsoPutsTheBodyOnlyChildTagOnTheLiveSurrogateKeyHeader(): void + { + $ro = $this->resource->get(self::BODY_ONLY_CHILD_DONUT); + $this->flushAndValidate($this->logger); + + $surrogateKey = $ro->headers[Header::SURROGATE_KEY] ?? ''; + $this->assertStringContainsString(self::CHILD_URI_TAG, $surrogateKey, "the live #[DonutCache] response's own Surrogate-Key header must carry the body-only child's tag too"); + } + + public function testPurgingTheBodyOnlyChildInvalidatesThePage(): void + { + $this->resource->get(self::BODY_ONLY_CHILD_DONUT); + $this->flushAndValidate($this->logger); + + $this->resource->get(self::BODY_ONLY_CHILD_DONUT); + $warm = $this->flushAndValidate($this->logger); + $warmClose = self::scopeCloseOf($warm, 'get', self::BODY_ONLY_CHILD_DONUT); + $this->assertNotNull($warmClose); + $this->assertSame('cache_hit', $warmClose[0], 'the stored template is warm before the purge'); + + $this->queryRepository->purge(new Uri(self::CHILD)); + $this->flushAndValidate($this->logger); + + $this->resource->get(self::BODY_ONLY_CHILD_DONUT); + $tree = $this->flushAndValidate($this->logger); + $close = self::scopeCloseOf($tree, 'get', self::BODY_ONLY_CHILD_DONUT); + $this->assertNotNull($close); + $this->assertSame('cache_miss', $close[0], "purging the body-only child's tag must invalidate the page's own stored template, not just leave it to a silent refresh"); + } + + public function testMissAndHitServeByteIdenticalViewForAnIdempotentChild(): void + { + $missRo = $this->resource->get(self::BLOG_POSTING_DONUT); + $this->flushAndValidate($this->logger); + $missView = (string) $missRo->view; + $this->assertStringContainsString('comment01', $missView); + + $hitRo = $this->resource->get(self::BLOG_POSTING_DONUT); + $hit = $this->flushAndValidate($this->logger); + $hitClose = self::scopeCloseOf($hit, 'get', self::BLOG_POSTING_DONUT); + $this->assertNotNull($hitClose); + $this->assertSame('cache_hit', $hitClose[0], 'this read must come from the stored donut template, not a fresh render'); + + $this->assertSame( + $missView, + (string) $hitRo->view, + 'the MISS render and the HIT-refresh of an idempotent child must be byte-identical - the whole point of "MISS equals the render without #[DonutCache], HIT re-fetches only holes"', + ); + } + + public function testMethodLevelCacheableResponseEmbedExecutesOnceToo(): void + { + // DonutCacheModule::installAopMethodModule() binds EmbedOnceInterceptor a second + // time, for a class whose #[CacheableResponse] sits on onGet() itself (like + // BlogPostingCache) rather than on the class - this is the fixture proving THAT + // binding is actually reached, not merely registered at configure() time: a dead + // binding here would leave 'counter' as a plain Request, double-executing it the + // same way CounterDonut (class-level) would without EmbedOnceInterceptor at all. + $ro = $this->resource->get(self::COUNTER_CACHE_METHOD_LEVEL); + $this->flushAndValidate($this->logger); + + $this->assertSame(1, Counter::$calls); + $view = (string) $ro->view; + $this->assertStringContainsString('read:1', $view); + $this->assertStringContainsString('call:1', $view); + } +} diff --git a/tests/OnceRequestUnitTest.php b/tests/OnceRequestUnitTest.php new file mode 100644 index 00000000..fc565793 --- /dev/null +++ b/tests/OnceRequestUnitTest.php @@ -0,0 +1,177 @@ +resource = $injector->getInstance(ResourceInterface::class); + } + + private function newCounterRequest(): Request + { + $request = $this->resource->newRequest(Method::GET, 'app://self/counter'); + self::assertInstanceOf(Request::class, $request); + + return $request; + } + + public function testAddQueryDiscardsTheMemoAndReExecutes(): void + { + $once = new OnceRequest($this->newCounterRequest()); + $first = $once(); + $this->assertTrue($once->hasExecuted()); + + $once->addQuery(['tag' => 'added']); + $this->assertFalse($once->hasExecuted(), 'addQuery() must discard the memo, same as withQuery()'); + + $second = $once(); + $this->assertSame(2, Counter::$calls); + $this->assertNotSame($first, $second); + assert(is_array($second->body)); + $this->assertSame('added', $second->body['tag']); + } + + public function testInvokeWithAQueryArgumentMergesAndReExecutes(): void + { + $once = new OnceRequest($this->newCounterRequest()); + $once(); + $this->assertSame(1, Counter::$calls); + + $second = $once(['tag' => 'merged']); + $this->assertSame(2, Counter::$calls, '__invoke($query) must force a second execution under the merged query'); + assert(is_array($second->body)); + $this->assertSame('merged', $second->body['tag']); + } + + public function testToUriWithMethodDelegatesToTheInnerRequest(): void + { + $inner = $this->newCounterRequest(); + $once = new OnceRequest($inner); + + $this->assertSame($inner->toUriWithMethod(), $once->toUriWithMethod()); + } + + public function testLinkSelfDelegatesAndDiscardsTheMemo(): void + { + $once = new OnceRequest($this->newCounterRequest()); + $once(); + $this->assertTrue($once->hasExecuted()); + + $once->linkSelf('someLink'); + $this->assertFalse($once->hasExecuted(), 'a link change must discard the memo too - the next invocation is a linked one'); + $this->assertNotSame([], $once->links); + } + + public function testLinkNewDelegatesAndDiscardsTheMemo(): void + { + $once = new OnceRequest($this->newCounterRequest()); + $once(); + + $once->linkNew('someLink'); + $this->assertFalse($once->hasExecuted()); + $this->assertNotSame([], $once->links); + } + + public function testLinkCrawlDelegatesAndDiscardsTheMemo(): void + { + $once = new OnceRequest($this->newCounterRequest()); + $once(); + + $once->linkCrawl('someLink'); + $this->assertFalse($once->hasExecuted()); + $this->assertNotSame([], $once->links); + } + + public function testGetCodeReadsFromTheMemoizedExecutionWithoutReExecuting(): void + { + $once = new OnceRequest($this->newCounterRequest()); + + $code = $once->code; + $this->assertSame(1, Counter::$calls, '__get() must run through this class\'s own memo, not AbstractRequest\'s separate one'); + + $this->assertSame($code, $once->code); + $this->assertSame(1, Counter::$calls, 'a second __get() for the same generation must not execute again'); + } + + public function testGetBodyMatchesTheSameExecutionCodeCameFrom(): void + { + $once = new OnceRequest($this->newCounterRequest()); + + $body = $once->body; + assert(is_array($body)); + $this->assertSame(1, $body['call'], 'the body must come from the one execution __get(\'code\') above would also have read'); + } + + public function testGetHeadersAlsoReadsFromTheSameMemoizedExecution(): void + { + $once = new OnceRequest($this->newCounterRequest()); + + $executed = $once(); + $this->assertSame(1, Counter::$calls); + + $this->assertSame($executed->headers, $once->headers, '__get(\'headers\') must answer the same execution __invoke() already memoized, not a fresh one'); + $this->assertSame(1, Counter::$calls, 'reading headers after an already-memoized execution must not run a second one'); + } + + public function testGetEagerSetsInOnBothWrapperAndInnerRequestAndReturnsItself(): void + { + $inner = $this->newCounterRequest(); + $once = new OnceRequest($inner); + + $result = $once->eager; + + $this->assertSame($once, $result, 'the eager/lazy DSL returns $this for chaining, same as Request::__get()'); + $this->assertSame('eager', $once->in); + $this->assertSame('eager', $inner->in, 'must forward to the inner request too, not only set the wrapper\'s own $in'); + } + + public function testGetLazySetsInOnBothWrapperAndInnerRequest(): void + { + $inner = $this->newCounterRequest(); + $once = new OnceRequest($inner); + + $flippedToEager = $once->eager; // flip away from the default 'lazy' first, so this assertion is not a no-op + $flippedToLazy = $flippedToEager->lazy; + + $this->assertSame($once, $flippedToLazy, 'the DSL keeps returning $this across the chain'); + $this->assertSame('lazy', $once->in); + $this->assertSame('lazy', $inner->in); + } + + public function testGetUnknownPropertyThrowsOutOfRangeException(): void + { + $once = new OnceRequest($this->newCounterRequest()); + + $this->expectException(OutOfRangeException::class); + $once->notAKnownProperty; // @phpstan-ignore-line + } +}