diff --git a/CHANGELOG.md b/CHANGELOG.md index b5d99e1..98d6b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,36 +2,7 @@ All notable changes to this project will be documented in this file. -## [Unreleased] - -### Fixed - -- A host app whose `statamic.static_caching.invalidation.rules` is the string - `all` no longer fatals the moment static caching is switched on. Statamic - documents that value and checks for exactly it in - `DefaultInvalidator::invalidate()`, so a stock config hit - `TypeError: GraphInvalidator::__construct(): Argument #2 ($rules) must be of - type ?array, string given` during boot. Any non-array value is now normalised - to `[]`: `all` means flush everything on any save, which is precisely the - behaviour the graph replaces, so it must not reach the parent. -- Globals no longer come back from the cache as `__PHP_Incomplete_Class`. Laravel - unserializes cache payloads against an allow list when - `cache.serializable_classes` holds an array, and Statamic fills that array with - its own classes. Binding `TrackingVariables` over the `Variables` contract meant - the globals store cached items of a class nobody had allowed, so the next method - call on a global set fatalled — on every page, since layouts read globals. The - addon now adds its own cached classes to that allow list, leaving the host's - entries and the unrestricted `null`/`true` settings untouched. -- Custom query scopes now work on the tracking entry and term query builders. - Statamic keys its scope registry on the exact builder class, so a scope a site - registered against `Stache\Query\EntryQueryBuilder` was invisible to the - subclass this addon substitutes, and calling it fatally threw - `BadMethodCallException: Call to undefined method ...::yourScope()`. The - ordering was not a fluke: addons boot inside `$app->booted()`, so a site's own - `boot()` always registers its scopes before this addon rebinds the builder. A - scope registered against any ancestor of a tracking builder now applies to it. - -## [2.0.0] - 2026-08-06 +## [2.0.0] - 2026-09-15 Invalidation is now derived from what pages actually read, instead of from rules describing what they might read. There is no configuration to write. @@ -113,6 +84,52 @@ renders; v2 observes it rather than restating it. ### Fixed +- A save no longer leaves pages stale when it clears a large number of URLs. + Statamic keeps one URL map per domain and rewrites all of it for every URL it + clears, so invalidating n URLs out of a map of m cost O(n*m). On a site with a + few thousand cached URLs that is a few hundred kilobytes rewritten thousands of + times per save, each write taking an exclusive lock that live traffic is + contending for on every uncached render. The queued + `Statamic\StaticCaching\Invalidate` job ran past its timeout and was killed + part-way through the list, so every URL it had not reached yet went on serving + the old page. Nothing said so: the only trace was a `TimeoutExceededException` + in `failed_jobs`, while the editor saw a saved entry that never appeared. The + map is now read once and written once per domain actually touched, whatever the + size of the batch — Statamic's own matching, response deletion and events are + left exactly as they are. The `full` strategy still scans its cache directory + once per URL, which is Statamic's own `FileCacher` and unchanged here. +- Invalidation no longer hands the cacher URLs that are not cached any more. The + graph outlives the cache by design — rows are pruned as they are invalidated, + not when a page falls out — so on a site that has been up for a while a tag + routinely resolves to two or three times as many URLs as the cache holds, and + every one of those costs a lookup that can only miss. They are now filtered out + before the cacher sees them. A cacher whose contents cannot be enumerated still + receives the full set: an empty list there means "unknown", not "nothing is + cached", and narrowing against it would clear nothing at all. +- A host app whose `statamic.static_caching.invalidation.rules` is the string + `all` no longer fatals the moment static caching is switched on. Statamic + documents that value and checks for exactly it in + `DefaultInvalidator::invalidate()`, so a stock config hit + `TypeError: GraphInvalidator::__construct(): Argument #2 ($rules) must be of + type ?array, string given` during boot. Any non-array value is now normalised + to `[]`: `all` means flush everything on any save, which is precisely the + behaviour the graph replaces, so it must not reach the parent. +- Globals no longer come back from the cache as `__PHP_Incomplete_Class`. Laravel + unserializes cache payloads against an allow list when + `cache.serializable_classes` holds an array, and Statamic fills that array with + its own classes. Binding `TrackingVariables` over the `Variables` contract meant + the globals store cached items of a class nobody had allowed, so the next method + call on a global set fatalled — on every page, since layouts read globals. The + addon now adds its own cached classes to that allow list, leaving the host's + entries and the unrestricted `null`/`true` settings untouched. +- Custom query scopes now work on the tracking entry and term query builders. + Statamic keys its scope registry on the exact builder class, so a scope a site + registered against `Stache\Query\EntryQueryBuilder` was invisible to the + subclass this addon substitutes, and calling it fatally threw + `BadMethodCallException: Call to undefined method ...::yourScope()`. The + ordering was not a fluke: addons boot inside `$app->booted()`, so a site's own + `boot()` always registers its scopes before this addon rebinds the builder. A + scope registered against any ancestor of a tracking builder now applies to it. - `Invalidator::refresh()` is honoured. `DefaultInvalidator` flips its `$refreshing` flag before delegating to `invalidate()`, which 1.x overrode without checking, so `statamic.static_caching.background_recache` hard-purged instead of refreshing. diff --git a/README.md b/README.md index 3f87ebd..dc95cd0 100644 --- a/README.md +++ b/README.md @@ -353,9 +353,10 @@ listed so you know where the edges are. ## Good to know -- Invalidation is one indexed lookup plus the deletes. Nothing walks content, which - matters with a single queue worker or `QUEUE_CONNECTION=sync`, where it runs inside - the editor's save request. +- Invalidation is one indexed lookup plus the deletes, and the URL map Statamic keeps + per domain is read and written once per pass rather than once per URL cleared. + Nothing walks content, which matters with a single queue worker or + `QUEUE_CONNECTION=sync`, where it runs inside the editor's save request. - A page with more than 2,000 dependencies is treated as depending on everything. - Globals are not scoped per site, so on a multisite install saving one clears the pages that read it across every site. diff --git a/src/Cachers/BatchesInvalidation.php b/src/Cachers/BatchesInvalidation.php new file mode 100644 index 0000000..244bb58 --- /dev/null +++ b/src/Cachers/BatchesInvalidation.php @@ -0,0 +1,146 @@ + URL map, filled lazily while a batch is in flight. Null whenever + * no batch is running, which is what every other caller sees. + * + * @var array|null + */ + private ?array $bufferedUrls = null; + + /** + * Domains whose buffered map was actually modified. A pass that matches + * nothing must not write the map back. + * + * @var array + */ + private array $dirtyDomains = []; + + /** + * @param array $urls + * @return void + */ + public function invalidateUrls($urls) + { + $this->whileBuffering(fn () => parent::invalidateUrls($urls)); + } + + /** + * Refreshing never writes the map — it reads it to find URLs to warm — but it + * reads it once per URL, so the same buffer removes that too. + * + * @param array $urls + * @return void + */ + public function refreshUrls($urls) + { + $this->whileBuffering(fn () => parent::refreshUrls($urls)); + } + + /** + * @param string|null $domain + * @return Collection + */ + public function getUrls($domain = null) + { + if ($this->bufferedUrls === null) { + return parent::getUrls($domain); + } + + return $this->bufferedUrls[$this->bufferKey($domain)] ??= parent::getUrls($domain); + } + + /** + * @param string $key + * @param string|null $domain + * @return void + */ + public function forgetUrl($key, $domain = null) + { + if ($this->bufferedUrls === null) { + parent::forgetUrl($key, $domain); + + return; + } + + // Collection::forget() mutates, so this updates the buffered map in place + // and the parent's next getUrls() in the same pass sees the removal. + $this->getUrls($domain)->forget($key); + + $this->dirtyDomains[$this->bufferKey($domain)] = true; + } + + private function whileBuffering(callable $callback): void + { + // A nested call is already inside a batch. Letting it run against the open + // buffer keeps one flush at the end instead of writing a half-finished map. + if ($this->bufferedUrls !== null) { + $callback(); + + return; + } + + $this->bufferedUrls = []; + $this->dirtyDomains = []; + + try { + $callback(); + } finally { + $buffered = $this->bufferedUrls; + $dirty = $this->dirtyDomains; + + // Closed before the writes so they, and anything reading the map + // afterwards, take the ordinary unbuffered path. In a finally block + // because a throw part-way through still has to persist the deletions + // already made — the alternative is forgetting them and serving pages + // whose responses are gone. + $this->bufferedUrls = null; + $this->dirtyDomains = []; + + foreach (array_keys($dirty) as $domain) { + $this->cache->forever($this->getUrlsCacheKey($domain), $buffered[$domain]->all()); + } + } + } + + /** + * @param string|null $domain + */ + private function bufferKey($domain): string + { + // Matches how getUrls() and getUrlsCacheKey() resolve a null domain, so a + // pass that mixes null and explicit domains still shares one buffer entry. + return $domain ?: $this->getBaseUrl(); + } +} diff --git a/src/Cachers/TrackingApplicationCacher.php b/src/Cachers/TrackingApplicationCacher.php index eee8794..fc5d13b 100644 --- a/src/Cachers/TrackingApplicationCacher.php +++ b/src/Cachers/TrackingApplicationCacher.php @@ -11,5 +11,6 @@ */ final class TrackingApplicationCacher extends ApplicationCacher { + use BatchesInvalidation; use RecordsDependencies; } diff --git a/src/Cachers/TrackingFileCacher.php b/src/Cachers/TrackingFileCacher.php index 0dfa412..ae10323 100644 --- a/src/Cachers/TrackingFileCacher.php +++ b/src/Cachers/TrackingFileCacher.php @@ -11,5 +11,6 @@ */ final class TrackingFileCacher extends FileCacher { + use BatchesInvalidation; use RecordsDependencies; } diff --git a/src/Invalidation/GraphInvalidator.php b/src/Invalidation/GraphInvalidator.php index 32cffe8..a1bc7e5 100644 --- a/src/Invalidation/GraphInvalidator.php +++ b/src/Invalidation/GraphInvalidator.php @@ -81,14 +81,45 @@ public function refresh($item): void public function invalidate($item): void { $tags = $this->tags->forItem($item); + $cached = $this->cached->all(); $this->clear([ ...$this->getItemUrls($item), - ...$tags === [] ? [] : $this->graph->urlsFor([...$tags, Tag::OVERFLOW]), - ...$this->graph->untracked($this->cached->all()), + ...$tags === [] ? [] : $this->stillCached($this->graph->urlsFor([...$tags, Tag::OVERFLOW]), $cached), + ...$this->graph->untracked($cached), ]); } + /** + * The graph outlives the cache: a URL keeps its row after its cached copy is + * gone, and rows are only pruned as they are invalidated. On a site that has + * been up for a while most of what a tag resolves to is no longer cached — + * often two thirds of it — and every one of those costs the cacher a lookup + * that can only miss. Narrowing here is what makes the remaining work match + * the number of pages that actually have to be cleared. + * + * Deliberately not applied to getItemUrls(): those are Statamic's own, may be + * wildcards, and are not graph rows to begin with. + * + * @param list $urls + * @param list $cached + * @return list + */ + private function stillCached(array $urls, array $cached): array + { + // An empty list is ambiguous. It means "nothing is cached" on a site that + // was just flushed, but it equally means "this cacher cannot be + // enumerated" — CachedUrls gives up on anything that is not an + // AbstractCacher, which includes a host app's own cacher. Narrowing + // against it would then clear nothing at all, the one failure this addon + // exists to prevent. Hand the full set over and let the cacher decide. + if ($cached === []) { + return $urls; + } + + return array_values(array_intersect($urls, $cached)); + } + /** * @param list $urls */ diff --git a/tests/Cachers/BatchesInvalidationTest.php b/tests/Cachers/BatchesInvalidationTest.php new file mode 100644 index 0000000..1ab7759 --- /dev/null +++ b/tests/Cachers/BatchesInvalidationTest.php @@ -0,0 +1,178 @@ +cacher = app(Cacher::class); + $this->domain = $this->cacher->getBaseUrl(); + } + + #[Test] + public function it_writes_the_url_map_once_however_many_urls_are_cleared(): void + { + foreach (range(1, 50) as $i) { + $this->cache("/page-{$i}"); + } + + $writes = $this->urlMapWritesDuring(fn () => $this->cacher->invalidateUrls( + array_map(fn (int $i): string => $this->domain."/page-{$i}", range(1, 40)), + )); + + $this->assertSame(1, $writes); + $this->assertCachedIs(array_map(fn (int $i): string => "/page-{$i}", range(41, 50))); + } + + #[Test] + public function it_does_not_rewrite_the_url_map_when_nothing_matched(): void + { + $this->cache('/kept'); + + $writes = $this->urlMapWritesDuring(fn () => $this->cacher->invalidateUrls([ + $this->domain.'/never-cached', + ])); + + $this->assertSame(0, $writes); + $this->assertCachedIs(['/kept']); + } + + #[Test] + public function it_clears_query_string_variants_in_the_same_pass(): void + { + // The reason a single save can name thousands of URLs: every allowed query + // string is its own cache entry, and a crawled paginated listing produces + // hundreds of them under one path. + $this->cache('/list'); + $this->cache('/list?page=2'); + $this->cache('/list?page=3'); + $this->cache('/other'); + + $writes = $this->urlMapWritesDuring(fn () => $this->cacher->invalidateUrls([ + $this->domain.'/list', + ])); + + $this->assertSame(1, $writes); + $this->assertCachedIs(['/other']); + } + + #[Test] + public function it_writes_one_url_map_per_domain_touched(): void + { + $other = 'https://other.test'; + + $this->cache('/a'); + $this->cache('/b'); + $this->cache('/a', $other); + $this->cache('/b', $other); + + $writes = $this->urlMapWritesDuring(fn () => $this->cacher->invalidateUrls([ + $this->domain.'/a', + $other.'/a', + ])); + + $this->assertSame(2, $writes); + $this->assertCachedIs(['/b']); + $this->assertCachedIs(['/b'], $other); + } + + #[Test] + public function it_still_clears_wildcard_urls(): void + { + // Wildcards go through the parent's own matching, which resolves them to + // individual URLs and forgets each one. Those forgets have to land in the + // same buffer as everything else. + $this->cache('/blog/one'); + $this->cache('/blog/two'); + $this->cache('/other'); + + $writes = $this->urlMapWritesDuring(fn () => $this->cacher->invalidateUrls([ + $this->domain.'/blog/*', + ])); + + $this->assertSame(1, $writes); + $this->assertCachedIs(['/other']); + } + + #[Test] + public function refreshing_reads_the_url_map_without_writing_it(): void + { + Queue::fake(); + + $this->cache('/a'); + $this->cache('/b'); + + $writes = $this->urlMapWritesDuring(fn () => $this->cacher->refreshUrls([ + $this->domain.'/a', + $this->domain.'/b', + ])); + + // A refresh warms pages back up rather than dropping them, so the map is + // unchanged and must not be written at all. + $this->assertSame(0, $writes); + $this->assertCachedIs(['/a', '/b']); + } + + /** + * Writes of a URL map, as opposed to the many small per-response keys that + * a pass legitimately touches. Statamic names them `.urls`. + */ + private function urlMapWritesDuring(callable $callback): int + { + $writes = 0; + + Event::listen(KeyWritten::class, function (KeyWritten $event) use (&$writes): void { + if (str_ends_with($event->key, '.urls')) { + $writes++; + } + }); + + $callback(); + + return $writes; + } + + private function cache(string $path, ?string $domain = null): void + { + $this->cacher->cacheUrl(md5($path), $path, $domain ?? $this->domain); + } + + /** + * @param list $expected + */ + private function assertCachedIs(array $expected, ?string $domain = null): void + { + $this->assertEqualsCanonicalizing( + $expected, + $this->cacher->getUrls($domain ?? $this->domain)->values()->all(), + ); + } +} diff --git a/tests/Invalidation/NarrowsToCachedUrlsTest.php b/tests/Invalidation/NarrowsToCachedUrlsTest.php new file mode 100644 index 0000000..6927621 --- /dev/null +++ b/tests/Invalidation/NarrowsToCachedUrlsTest.php @@ -0,0 +1,92 @@ +article(); + + $cacher = app(Cacher::class); + $domain = $cacher->getBaseUrl(); + + // Cached after the save, so the save's own invalidation cannot clear it. + $cacher->cacheUrl(md5('/still-cached'), '/still-cached', $domain); + + $this->graph->record($domain.'/still-cached', ["entry:{$entry->id()}"]); + $this->graph->record($domain.'/long-gone', ["entry:{$entry->id()}"]); + + $handed = $this->urlsHandedToCacher($entry, new CachedUrls($cacher)); + + $this->assertContains($domain.'/still-cached', $handed); + $this->assertNotContains($domain.'/long-gone', $handed); + } + + #[Test] + public function it_hands_over_everything_when_the_cache_cannot_be_enumerated(): void + { + // CachedUrls gives up on any cacher that is not an AbstractCacher, and an + // empty list is indistinguishable from a freshly flushed site. Reading it + // as "nothing is cached" would clear nothing at all on a host app's own + // cacher — silently serving stale pages, the one failure that matters. + $entry = $this->article(); + + $domain = app(Cacher::class)->getBaseUrl(); + + $this->graph->record($domain.'/long-gone', ["entry:{$entry->id()}"]); + + $handed = $this->urlsHandedToCacher($entry, new CachedUrls(Mockery::mock(Cacher::class))); + + $this->assertContains($domain.'/long-gone', $handed); + } + + private function article(): EntryContract + { + Collection::make('articles')->save(); + + return tap(Entry::make()->collection('articles')->slug('one')->data(['title' => 'One']))->save(); + } + + /** + * Invalidates through a cacher that only records its arguments, so the + * assertion is about the list itself and not about what survived in a cache. + * + * @return list + */ + private function urlsHandedToCacher(EntryContract $entry, CachedUrls $cached): array + { + $handed = []; + + $cacher = Mockery::mock(Cacher::class); + $cacher->shouldReceive('invalidateUrls')->once()->with(Mockery::capture($handed)); + + (new GraphInvalidator($cacher, [], $this->graph, app(TagResolver::class), $cached)) + ->invalidate($entry); + + return $handed; + } +}