From c3591d5e002d8746091dc254484949056684b473 Mon Sep 17 00:00:00 2001 From: Andrew Clinton Date: Thu, 23 Jul 2026 23:36:32 -0400 Subject: [PATCH 1/5] Fix lost-update race condition in static cache URL map Concurrent requests could each read the URL-to-file-path map, add their own entry, then clobber each other's write with cache->forever(), since the read-modify-write cycle in cacheDomain()/cacheUrl()/forgetUrl() had no locking. This silently dropped tracked URLs even though their static .html files were written to disk correctly, leaving them permanently un-invalidatable since Statamic no longer knew they existed. Wrap the three critical sections in a new withLock() helper that uses Cache::lock()->block() when the store supports it, falling back to the previous unlocked behavior for stores that don't implement LockProvider. Also catch the resulting LockTimeoutException in the CP's "invalidate URLs" utility action, the one synchronous (non-queued) call site that would otherwise surface it as an uncaught 500. --- .../CP/Utilities/CacheController.php | 7 +- src/StaticCaching/Cachers/AbstractCacher.php | 69 ++++++++++--- tests/StaticCaching/CacherConcurrencyTest.php | 96 +++++++++++++++++++ 3 files changed, 159 insertions(+), 13 deletions(-) create mode 100644 tests/StaticCaching/CacherConcurrencyTest.php diff --git a/src/Http/Controllers/CP/Utilities/CacheController.php b/src/Http/Controllers/CP/Utilities/CacheController.php index e9a42d0df09..f458ef4a2a3 100644 --- a/src/Http/Controllers/CP/Utilities/CacheController.php +++ b/src/Http/Controllers/CP/Utilities/CacheController.php @@ -2,6 +2,7 @@ namespace Statamic\Http\Controllers\CP\Utilities; +use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Http\Request; use Illuminate\Support\Facades\Artisan; use Inertia\Inertia; @@ -133,7 +134,11 @@ public function invalidateStaticUrls(Request $request) ...$prefixedRelativeUrls, ]; - app(Cacher::class)->invalidateUrls($urls); + try { + app(Cacher::class)->invalidateUrls($urls); + } catch (LockTimeoutException $e) { + return back()->withError(__('Could not invalidate URLs because the cache was locked by another process. Please try again.')); + } return back()->withSuccess(__('Invalidated URLs in the Static Cache.')); } diff --git a/src/StaticCaching/Cachers/AbstractCacher.php b/src/StaticCaching/Cachers/AbstractCacher.php index 2071ae3a66d..ce603801b68 100644 --- a/src/StaticCaching/Cachers/AbstractCacher.php +++ b/src/StaticCaching/Cachers/AbstractCacher.php @@ -3,6 +3,7 @@ namespace Statamic\StaticCaching\Cachers; use GuzzleHttp\Psr7\Request as GuzzleRequest; +use Illuminate\Contracts\Cache\LockProvider; use Illuminate\Contracts\Cache\Repository; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -17,6 +18,16 @@ abstract class AbstractCacher implements Cacher { + /** + * Seconds until a held lock auto-expires, as a crash safety net. + */ + protected const LOCK_TIMEOUT = 10; + + /** + * Seconds a caller will wait to acquire a lock before giving up. + */ + protected const LOCK_WAIT = 5; + /** * @var Repository */ @@ -125,13 +136,17 @@ public function getDomains() */ public function cacheDomain($domain = null) { - $domains = $this->getDomains(); + $domain = $domain ?? $this->getBaseUrl(); - if (! $domains->contains($domain = $domain ?? $this->getBaseUrl())) { - $domains->push($domain); - } + $this->withLock($this->normalizeKey('domains:lock'), function () use ($domain) { + $domains = $this->getDomains(); - $this->cache->forever($this->normalizeKey('domains'), $domains->all()); + if (! $domains->contains($domain)) { + $domains->push($domain); + } + + $this->cache->forever($this->normalizeKey('domains'), $domains->all()); + }); } /** @@ -174,13 +189,15 @@ public function cacheUrl($key, $url, $domain = null) $this->cacheDomain($domain); - $urls = $this->getUrls($domain); + $this->withLock($this->getUrlsLockKey($domain), function () use ($key, $url, $domain) { + $urls = $this->getUrls($domain); - $url = Str::removeLeft($url, $domain); + $url = Str::removeLeft($url, $domain); - $urls->put($key, $url); + $urls->put($key, $url); - $this->cache->forever($this->getUrlsCacheKey($domain), $urls->all()); + $this->cache->forever($this->getUrlsCacheKey($domain), $urls->all()); + }); } /** @@ -191,11 +208,39 @@ public function cacheUrl($key, $url, $domain = null) */ public function forgetUrl($key, $domain = null) { - $urls = $this->getUrls($domain); + $this->withLock($this->getUrlsLockKey($domain), function () use ($key, $domain) { + $urls = $this->getUrls($domain); - $urls->forget($key); + $urls->forget($key); + + $this->cache->forever($this->getUrlsCacheKey($domain), $urls->all()); + }); + } - $this->cache->forever($this->getUrlsCacheKey($domain), $urls->all()); + /** + * Run a callback while holding an exclusive lock for the given key, if the + * configured cache store supports locking. Falls back to running the + * callback unprotected if it doesn't, so cache stores without lock + * support (e.g. some third-party addons) don't hard crash. + * + * @return mixed + */ + protected function withLock(string $key, \Closure $callback) + { + if (! $this->cache->getStore() instanceof LockProvider) { + return $callback(); + } + + return $this->cache->lock($key, static::LOCK_TIMEOUT)->block(static::LOCK_WAIT, $callback); + } + + /** + * @param string|null $domain + * @return string + */ + protected function getUrlsLockKey($domain = null) + { + return $this->getUrlsCacheKey($domain).':lock'; } /** diff --git a/tests/StaticCaching/CacherConcurrencyTest.php b/tests/StaticCaching/CacherConcurrencyTest.php new file mode 100644 index 00000000000..2edf2726ecd --- /dev/null +++ b/tests/StaticCaching/CacherConcurrencyTest.php @@ -0,0 +1,96 @@ +cacher($cache, ['base_url' => 'http://example.com']); + + $lockKey = 'static-cache:'.md5('http://example.com').'.urls:lock'; + $externalLock = $cache->lock($lockKey, 10); + $this->assertTrue($externalLock->get()); + + try { + $this->expectException(LockTimeoutException::class); + + $cacher->cacheUrl('one', '/one'); + } finally { + $externalLock->forceRelease(); + } + } + + #[Test] + public function forget_url_and_cache_url_share_the_same_lock_for_a_domain() + { + $cache = app(Repository::class); + $cacher = $this->cacher($cache, ['base_url' => 'http://example.com']); + + $cache->forever('static-cache:'.md5('http://example.com').'.urls', [ + 'one' => '/one', + ]); + + $lockKey = 'static-cache:'.md5('http://example.com').'.urls:lock'; + $externalLock = $cache->lock($lockKey, 10); + $this->assertTrue($externalLock->get()); + + try { + $this->expectException(LockTimeoutException::class); + + $cacher->forgetUrl('one'); + } finally { + $externalLock->forceRelease(); + } + } + + #[Test] + public function cache_domain_has_its_own_independent_lock_from_the_urls_map() + { + $cache = app(Repository::class); + $cacher = $this->cacher($cache, ['base_url' => 'http://example.com']); + + $externalLock = $cache->lock('static-cache:domains:lock', 10); + $this->assertTrue($externalLock->get()); + + try { + $this->expectException(LockTimeoutException::class); + + $cacher->cacheUrl('one', '/one'); + } finally { + $externalLock->forceRelease(); + } + } + + #[Test] + public function it_falls_back_to_unlocked_behavior_when_the_cache_store_does_not_support_locking() + { + $store = Mockery::mock(\Illuminate\Contracts\Cache\Store::class); + + $cache = Mockery::mock(\Illuminate\Contracts\Cache\Repository::class); + $cache->shouldReceive('getStore')->andReturn($store); + $cache->shouldReceive('get')->andReturn([]); + $cache->shouldReceive('forever'); + $cache->shouldNotReceive('lock'); + + $cacher = $this->cacher($cache, ['base_url' => 'http://example.com']); + + $cacher->cacheUrl('one', '/one'); + + $this->assertTrue(true); + } + + private function cacher($cache, $config = []) + { + return Mockery::mock(AbstractCacher::class, [$cache, $config])->makePartial(); + } +} From 66379b7ffea9573489a49c3fbb319c49104b0639 Mon Sep 17 00:00:00 2001 From: Andrew Clinton Date: Fri, 24 Jul 2026 11:25:06 -0400 Subject: [PATCH 2/5] Batch the static-cache urls lock for bulk invalidation Sites with hundreds of cached URLs could turn the CP's synchronous "invalidate URLs" action into a request-timeout hazard: invalidateUrls() looped per URL, and each iteration's forgetUrl() acquired/released the domain's urls lock independently, so up to LOCK_WAIT (5s) could be spent per URL, all serialized against the same lock key. invalidateUrls() now groups URLs by domain and holds a single lock for each domain's whole batch instead of one per URL. withLock() gained a reentrancy guard so forgetUrl() calls made from within an already-locked batch (via invalidateUrl()) don't try to re-acquire the same lock and deadlock against themselves. Co-Authored-By: Claude Sonnet 5 --- src/StaticCaching/Cachers/AbstractCacher.php | 66 ++++++++++-- tests/StaticCaching/CacherConcurrencyTest.php | 100 ++++++++++++++++++ 2 files changed, 158 insertions(+), 8 deletions(-) diff --git a/src/StaticCaching/Cachers/AbstractCacher.php b/src/StaticCaching/Cachers/AbstractCacher.php index ce603801b68..ab8a4c88971 100644 --- a/src/StaticCaching/Cachers/AbstractCacher.php +++ b/src/StaticCaching/Cachers/AbstractCacher.php @@ -38,6 +38,16 @@ abstract class AbstractCacher implements Cacher */ private $config; + /** + * Lock keys currently held by this instance, so nested withLock() calls for + * the same key (e.g. forgetUrl() called from within a batched invalidateUrls() + * loop that already holds the domain's urls lock) don't try to re-acquire a + * lock against themselves and deadlock until LOCK_WAIT expires. + * + * @var array + */ + private $heldLocks = []; + public function __construct(Repository $cache, $config) { $this->cache = $cache; @@ -227,11 +237,23 @@ public function forgetUrl($key, $domain = null) */ protected function withLock(string $key, \Closure $callback) { + if (isset($this->heldLocks[$key])) { + return $callback(); + } + if (! $this->cache->getStore() instanceof LockProvider) { return $callback(); } - return $this->cache->lock($key, static::LOCK_TIMEOUT)->block(static::LOCK_WAIT, $callback); + return $this->cache->lock($key, static::LOCK_TIMEOUT)->block(static::LOCK_WAIT, function () use ($key, $callback) { + $this->heldLocks[$key] = true; + + try { + return $callback(); + } finally { + unset($this->heldLocks[$key]); + } + }); } /** @@ -265,18 +287,46 @@ protected function invalidateWildcardUrl($wildcard) /** * Invalidate multiple URLs. * + * Grouped by domain and run under a single lock per domain, so a large + * batch doesn't acquire and release the domain's urls lock once per URL + * (each acquisition able to block up to LOCK_WAIT) and instead holds it + * once for the whole domain's batch. + * * @param array $urls * @return void */ public function invalidateUrls($urls) { - collect($urls)->each(function ($url) { - if (Str::contains($url, '*')) { - $this->invalidateWildcardUrl($url); - } else { - $this->invalidateUrl(...$this->getPathAndDomain($url)); - } - }); + collect($urls) + ->groupBy(fn ($url) => $this->resolveDomainForInvalidation($url)) + ->each(function ($urlsForDomain, $domain) { + $this->withLock($this->getUrlsLockKey($domain), function () use ($urlsForDomain) { + $urlsForDomain->each(function ($url) { + if (Str::contains($url, '*')) { + $this->invalidateWildcardUrl($url); + } else { + $this->invalidateUrl(...$this->getPathAndDomain($url)); + } + }); + }); + }); + } + + /** + * Resolve the domain an invalidation entry belongs to, the same way + * invalidateUrl()/invalidateWildcardUrl() would internally, so entries can + * be grouped by domain before either is called. + * + * @param string $url + * @return string + */ + protected function resolveDomainForInvalidation($url) + { + $url = Str::contains($url, '*') ? substr($url, 0, -1) : $url; + + [, $domain] = $this->getPathAndDomain($url); + + return $domain; } /** diff --git a/tests/StaticCaching/CacherConcurrencyTest.php b/tests/StaticCaching/CacherConcurrencyTest.php index 2edf2726ecd..313b5276f66 100644 --- a/tests/StaticCaching/CacherConcurrencyTest.php +++ b/tests/StaticCaching/CacherConcurrencyTest.php @@ -4,6 +4,7 @@ use Illuminate\Cache\Repository; use Illuminate\Contracts\Cache\LockTimeoutException; +use Illuminate\Http\Request; use Mockery; use PHPUnit\Framework\Attributes\Test; use Statamic\StaticCaching\Cachers\AbstractCacher; @@ -89,8 +90,107 @@ public function it_falls_back_to_unlocked_behavior_when_the_cache_store_does_not $this->assertTrue(true); } + #[Test] + public function invalidate_urls_only_acquires_the_domain_lock_once_for_a_multi_url_batch() + { + $lockKey = 'static-cache:'.md5('http://example.com').'.urls:lock'; + $urlsKey = 'static-cache:'.md5('http://example.com').'.urls'; + + $store = Mockery::mock(\Illuminate\Contracts\Cache\LockProvider::class, \Illuminate\Contracts\Cache\Store::class); + $lock = Mockery::mock(\Illuminate\Contracts\Cache\Lock::class); + + $cache = Mockery::mock(\Illuminate\Contracts\Cache\Repository::class); + $cache->shouldReceive('getStore')->andReturn($store); + $cache->shouldReceive('get')->with($urlsKey, [])->andReturn([ + 'one' => '/one', + 'two' => '/two', + 'three' => '/three', + ]); + $cache->shouldReceive('forever')->withArgs(fn ($key) => $key === $urlsKey); + + // The whole point of the fix: one lock covers the entire batch, not + // one acquisition per URL. + $cache->shouldReceive('lock') + ->once() + ->withArgs(fn ($key) => $key === $lockKey) + ->andReturn($lock); + $lock->shouldReceive('block')->once()->andReturnUsing(fn ($seconds, $callback) => $callback()); + + $cacher = $this->concreteCacher($cache, ['base_url' => 'http://example.com']); + + $cacher->invalidateUrls(['/one', '/two', '/three']); + } + + #[Test] + public function invalidate_urls_throws_once_for_the_whole_batch_when_the_domain_lock_is_contended() + { + $cache = app(Repository::class); + $cacher = $this->concreteCacher($cache, ['base_url' => 'http://example.com']); + + $urlsKey = 'static-cache:'.md5('http://example.com').'.urls'; + $cache->forever($urlsKey, [ + 'one' => '/one', + 'two' => '/two', + 'three' => '/three', + ]); + + $lockKey = 'static-cache:'.md5('http://example.com').'.urls:lock'; + $externalLock = $cache->lock($lockKey, 10); + $this->assertTrue($externalLock->get()); + + try { + $this->expectException(LockTimeoutException::class); + + $cacher->invalidateUrls(['/one', '/two', '/three']); + } finally { + $externalLock->forceRelease(); + } + + // Nothing should have been removed since the batch never acquired the lock. + $this->assertSame([ + 'one' => '/one', + 'two' => '/two', + 'three' => '/three', + ], $cache->get($urlsKey)); + } + private function cacher($cache, $config = []) { return Mockery::mock(AbstractCacher::class, [$cache, $config])->makePartial(); } + + /** + * A minimal concrete AbstractCacher whose invalidateUrl() mirrors the real + * FileCacher/ApplicationCacher pattern of looking up matching urls map + * entries and calling forgetUrl() per match, so tests can exercise the + * real nested-lock path (invalidateUrls() -> invalidateUrl() -> forgetUrl()). + */ + private function concreteCacher($cache, $config = []) + { + return new class($cache, $config) extends AbstractCacher + { + public function cachePage(Request $request, $content) + { + } + + public function getCachedPage(Request $request) + { + } + + public function flush() + { + } + + public function invalidateUrl($url, $domain = null) + { + $domain = $domain ?? $this->getBaseUrl(); + + $this->getUrls($domain) + ->filter(fn ($value) => $value === $url) + ->each(function ($value, $key) use ($domain) { + $this->forgetUrl($key, $domain); + }); + } + }; + } } From d8167490bb397df6ec105a51ab6f2fb50e470ddc Mon Sep 17 00:00:00 2001 From: Andrew Clinton Date: Mon, 27 Jul 2026 22:43:37 -0400 Subject: [PATCH 3/5] Serve response uncached when urls lock times out during page caching A contended urls lock during cachePage() previously threw out of the middleware, replacing the rendered page with a 503 refresh response. FileCacher also left the written HTML file behind with no urls map entry - served forever but invisible to invalidation. Catch the timeout in both cachers: ApplicationCacher skips storing the response, FileCacher additionally deletes the written file. Co-Authored-By: Claude Fable 5 --- .../Cachers/ApplicationCacher.php | 11 +- src/StaticCaching/Cachers/FileCacher.php | 10 +- tests/StaticCaching/CacherConcurrencyTest.php | 105 ++++++++++++++++++ 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/src/StaticCaching/Cachers/ApplicationCacher.php b/src/StaticCaching/Cachers/ApplicationCacher.php index 92bae1e6747..d4342e6f513 100644 --- a/src/StaticCaching/Cachers/ApplicationCacher.php +++ b/src/StaticCaching/Cachers/ApplicationCacher.php @@ -2,6 +2,7 @@ namespace Statamic\StaticCaching\Cachers; +use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Http\Request; use Illuminate\Routing\Events\ResponsePrepared; use Illuminate\Support\Facades\Event; @@ -38,8 +39,14 @@ public function cachePage(Request $request, $content) // and other URL characters wouldn't work as a cache key. $key = $this->makeHash($url); - // Keep track of the URL and key the response content is about to be stored within. - $this->cacheUrl($key, ...$this->getPathAndDomain($url)); + try { + // Keep track of the URL and key the response content is about to be stored within. + $this->cacheUrl($key, ...$this->getPathAndDomain($url)); + } catch (LockTimeoutException $e) { + // The URL couldn't be recorded in the urls map, so don't store the + // response either. The response will just go out uncached. + return; + } $key = $this->normalizeKey('responses:'.$key); $value = $this->normalizeContent($content); diff --git a/src/StaticCaching/Cachers/FileCacher.php b/src/StaticCaching/Cachers/FileCacher.php index 2b339e40120..e8e6151006c 100644 --- a/src/StaticCaching/Cachers/FileCacher.php +++ b/src/StaticCaching/Cachers/FileCacher.php @@ -2,6 +2,7 @@ namespace Statamic\StaticCaching\Cachers; +use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Contracts\Cache\Repository; use Illuminate\Http\Request; use Illuminate\Support\Facades\Log; @@ -81,7 +82,14 @@ public function cachePage(Request $request, $content) return; } - $this->cacheUrl($this->makeHash($url), ...$this->getPathAndDomain($url)); + try { + $this->cacheUrl($this->makeHash($url), ...$this->getPathAndDomain($url)); + } catch (LockTimeoutException $e) { + // The URL couldn't be recorded in the urls map. Remove the written + // file so it isn't served while being invisible to invalidation, + // and let the response go out uncached. + $this->writer->delete($path); + } } public function preventLoggingRewriteWarning() diff --git a/tests/StaticCaching/CacherConcurrencyTest.php b/tests/StaticCaching/CacherConcurrencyTest.php index 313b5276f66..ed4ccddfb0e 100644 --- a/tests/StaticCaching/CacherConcurrencyTest.php +++ b/tests/StaticCaching/CacherConcurrencyTest.php @@ -5,13 +5,29 @@ use Illuminate\Cache\Repository; use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Http\Request; +use Illuminate\Http\Response; +use Illuminate\Routing\Events\ResponsePrepared; +use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\File; use Mockery; use PHPUnit\Framework\Attributes\Test; use Statamic\StaticCaching\Cachers\AbstractCacher; +use Statamic\StaticCaching\Cachers\ApplicationCacher; +use Statamic\StaticCaching\Cachers\FileCacher; +use Statamic\StaticCaching\Cachers\Writer; +use Statamic\StaticCaching\Middleware\Cache as CacheMiddleware; +use Statamic\StaticCaching\NoCache\Session; use Tests\TestCase; class CacherConcurrencyTest extends TestCase { + public function tearDown(): void + { + File::deleteDirectory($this->cachePath()); + + parent::tearDown(); + } + #[Test] public function cache_url_is_blocked_while_another_process_holds_the_urls_lock_for_the_domain() { @@ -154,6 +170,95 @@ public function invalidate_urls_throws_once_for_the_whole_batch_when_the_domain_ ], $cache->get($urlsKey)); } + #[Test] + public function file_cacher_removes_the_written_file_when_the_urls_lock_is_contended() + { + $cache = app(Repository::class); + $cacher = $this->fileCacher($cache, 'http://example.com'); + + $lockKey = 'static-cache:'.md5('http://example.com').'.urls:lock'; + $externalLock = $cache->lock($lockKey, 10); + $this->assertTrue($externalLock->get()); + + try { + $cacher->cachePage(Request::create('http://example.com/about'), 'hello'); + } finally { + $externalLock->forceRelease(); + } + + // The page couldn't be recorded in the urls map, so the written file + // must be removed. Otherwise it would be served forever but invisible + // to invalidation - the exact orphaned state the locking prevents. + $this->assertFileDoesNotExist($cacher->getFilePath('http://example.com/about')); + $this->assertNull($cache->get('static-cache:'.md5('http://example.com').'.urls')); + } + + #[Test] + public function application_cacher_skips_caching_the_response_when_the_urls_lock_is_contended() + { + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + + $lockKey = 'static-cache:'.md5('http://example.com').'.urls:lock'; + $externalLock = $cache->lock($lockKey, 10); + $this->assertTrue($externalLock->get()); + + try { + $cacher->cachePage($request = Request::create('http://example.com/about'), 'hello'); + } finally { + $externalLock->forceRelease(); + } + + // The response listener should never have been registered, so preparing + // a response must not store the page either. + Event::dispatch(new ResponsePrepared($request, new Response('hello'))); + + $this->assertNull($cache->get('static-cache:responses:'.md5('http://example.com/about'))); + $this->assertNull($cache->get('static-cache:'.md5('http://example.com').'.urls')); + } + + #[Test] + public function middleware_serves_the_rendered_response_uncached_when_the_urls_lock_is_contended() + { + $cache = app(Repository::class); + $cacher = $this->fileCacher($cache, 'http://localhost'); + + $middleware = new CacheMiddleware($cacher, app(Session::class)); + + $lockKey = 'static-cache:'.md5('http://localhost').'.urls:lock'; + $externalLock = $cache->lock($lockKey, 10); + $this->assertTrue($externalLock->get()); + + try { + $response = $middleware->handle( + Request::create('http://localhost/about'), + fn () => new Response('

Hello

') + ); + } finally { + $externalLock->forceRelease(); + } + + // The page rendered fine and should reach the visitor, not be discarded + // in favor of a 503 refresh response just because it couldn't be cached. + $this->assertSame(200, $response->getStatusCode()); + $this->assertStringContainsString('

Hello

', $response->getContent()); + $this->assertFileDoesNotExist($cacher->getFilePath('http://localhost/about')); + } + + private function cachePath() + { + return storage_path('framework/testing/static-cache-concurrency'); + } + + private function fileCacher($cache, $baseUrl) + { + return new FileCacher(new Writer, $cache, [ + 'base_url' => $baseUrl, + 'path' => $this->cachePath(), + 'locale' => 'en', + ]); + } + private function cacher($cache, $config = []) { return Mockery::mock(AbstractCacher::class, [$cache, $config])->makePartial(); From d8b4e601358835b9a3b5db8ebe5978989c2e15b4 Mon Sep 17 00:00:00 2001 From: Andrew Clinton Date: Tue, 28 Jul 2026 03:32:21 -0400 Subject: [PATCH 4/5] Hold the urls lock for map bookkeeping only during invalidation Invalidation previously held a domain's urls lock for its entire run - file deletes, per-url directory scans, and UrlInvalidated listeners included. A large batch could hold the lock long enough for visitor requests to stall or give up caching, or outlive the lock's expiry entirely, silently reintroducing the lost-update race. Split invalidation into two phases. Under the lock: resolve matching map entries, remove them, and persist the map in a single write. After releasing it: delete files / forget responses and dispatch events. The worst concurrent interleaving is now a map entry whose cached copy was deleted, which self-heals on the next request. Flushing wipes the urls map before deleting stored pages for the same reason. Custom cachers extending AbstractCacher must implement the new cleanupInvalidatedUrls() hook; invalidateUrls() no longer routes through invalidateUrl(). Co-Authored-By: Claude Fable 5 --- src/StaticCaching/Cachers/AbstractCacher.php | 124 +++++++++++++----- .../Cachers/ApplicationCacher.php | 41 +++--- src/StaticCaching/Cachers/FileCacher.php | 48 ++++--- tests/StaticCaching/CacherConcurrencyTest.php | 109 ++++++++++++--- tests/StaticCaching/CacherTest.php | 27 +++- tests/StaticCaching/FileCacherTest.php | 53 ++++++++ 6 files changed, 306 insertions(+), 96 deletions(-) diff --git a/src/StaticCaching/Cachers/AbstractCacher.php b/src/StaticCaching/Cachers/AbstractCacher.php index ab8a4c88971..ab324ea8b52 100644 --- a/src/StaticCaching/Cachers/AbstractCacher.php +++ b/src/StaticCaching/Cachers/AbstractCacher.php @@ -9,6 +9,7 @@ use Illuminate\Http\Request; use Illuminate\Http\Response; use Statamic\Console\Commands\StaticWarmJob; +use Statamic\Events\UrlInvalidated; use Statamic\Facades\Site; use Statamic\Facades\URL; use Statamic\StaticCaching\Cacher; @@ -39,10 +40,10 @@ abstract class AbstractCacher implements Cacher private $config; /** - * Lock keys currently held by this instance, so nested withLock() calls for - * the same key (e.g. forgetUrl() called from within a batched invalidateUrls() - * loop that already holds the domain's urls lock) don't try to re-acquire a - * lock against themselves and deadlock until LOCK_WAIT expires. + * Lock keys currently held by this instance, so nested withLock() calls + * for the same key (e.g. forgetUrl() called from code already holding the + * domain's urls lock) don't try to re-acquire a lock against themselves + * and deadlock until LOCK_WAIT expires. * * @var array */ @@ -180,7 +181,9 @@ public function getUrls($domain = null) public function flushUrls() { $this->getDomains()->each(function ($domain) { - $this->cache->forget($this->getUrlsCacheKey($domain)); + $this->withLock($this->getUrlsLockKey($domain), function () use ($domain) { + $this->cache->forget($this->getUrlsCacheKey($domain)); + }); }); $this->cache->forget($this->normalizeKey('domains')); @@ -265,6 +268,24 @@ protected function getUrlsLockKey($domain = null) return $this->getUrlsCacheKey($domain).':lock'; } + /** + * Invalidate a URL. + * + * @param string $url + * @param string|null $domain + * @return void + */ + public function invalidateUrl($url, $domain = null) + { + // For CLI contexts where Site::current()->url() may return the wrong + // domain causing getUrls() to look under the wrong cache key. + if ($domain === null) { + [$url, $domain] = $this->getPathAndDomain($url); + } + + $this->invalidatePathsForDomain([$url], $domain); + } + /** * Invalidate a wildcard URL. * @@ -272,26 +293,14 @@ protected function getUrlsLockKey($domain = null) */ protected function invalidateWildcardUrl($wildcard) { - // Remove the asterisk - $wildcard = substr($wildcard, 0, -1); + [, $domain] = $this->getPathAndDomain(substr($wildcard, 0, -1)); - [$wildcard, $domain] = $this->getPathAndDomain($wildcard); - - $this->getUrls($domain)->filter(function ($url) use ($wildcard) { - return Str::startsWith($url, $wildcard); - })->each(function ($url) use ($domain) { - $this->invalidateUrl($url, $domain); - }); + $this->invalidatePathsForDomain([$wildcard], $domain); } /** * Invalidate multiple URLs. * - * Grouped by domain and run under a single lock per domain, so a large - * batch doesn't acquire and release the domain's urls lock once per URL - * (each acquisition able to block up to LOCK_WAIT) and instead holds it - * once for the whole domain's batch. - * * @param array $urls * @return void */ @@ -299,19 +308,74 @@ public function invalidateUrls($urls) { collect($urls) ->groupBy(fn ($url) => $this->resolveDomainForInvalidation($url)) - ->each(function ($urlsForDomain, $domain) { - $this->withLock($this->getUrlsLockKey($domain), function () use ($urlsForDomain) { - $urlsForDomain->each(function ($url) { - if (Str::contains($url, '*')) { - $this->invalidateWildcardUrl($url); - } else { - $this->invalidateUrl(...$this->getPathAndDomain($url)); - } - }); - }); - }); + ->each(fn ($urlsForDomain, $domain) => $this->invalidatePathsForDomain($urlsForDomain->all(), $domain)); } + /** + * Invalidate a set of paths (and trailing-* wildcards) for a single domain. + * + * Two phases, so the urls lock is only held for map bookkeeping and never + * for driver cleanup (file deletes, cache forgets) or event listeners: + * + * 1. Under the domain's urls lock: resolve which map entries match, remove + * them, and persist the map in a single write. + * 2. After releasing the lock: driver cleanup and UrlInvalidated events. + * + * A page cached concurrently with phase two can at worst leave a map entry + * whose cached copy was just deleted, which is self-healing: the next + * request re-renders and re-caches under the same key. The reverse - a + * cached copy the map doesn't know about - cannot happen. + * + * @param array $urls + * @param string|null $domain + * @return void + */ + protected function invalidatePathsForDomain($urls, $domain) + { + $paths = collect($urls)->map(function ($url) { + $wildcard = Str::contains($url, '*'); + + [$path] = $this->getPathAndDomain($wildcard ? substr($url, 0, -1) : $url); + + return ['path' => $path, 'wildcard' => $wildcard]; + }); + + $invalidated = $this->withLock($this->getUrlsLockKey($domain), function () use ($paths, $domain) { + $urls = $this->getUrls($domain); + + $invalidated = $urls->filter(fn ($value) => $paths->contains(fn ($path) => $path['wildcard'] + ? Str::startsWith($value, $path['path']) + : ($value === $path['path'] || Str::startsWith($value, $path['path'].'?')))); + + if ($invalidated->isNotEmpty()) { + $this->cache->forever($this->getUrlsCacheKey($domain), $urls->diffKeys($invalidated)->all()); + } + + return $invalidated; + }); + + $this->cleanupInvalidatedUrls($invalidated, $paths, $domain); + + $paths->each(function ($path) use ($invalidated, $domain) { + $path['wildcard'] + ? $invalidated + ->filter(fn ($value) => Str::startsWith($value, $path['path'])) + ->each(fn ($value) => UrlInvalidated::dispatch($value, $domain)) + : UrlInvalidated::dispatch($path['path'], $domain); + }); + } + + /** + * Clean up the driver's stored copies of invalidated pages. Runs after the + * urls lock has been released. + * + * @param \Illuminate\Support\Collection $invalidated The removed entries, keyed by their urls map key. + * @param \Illuminate\Support\Collection $paths The ['path' => string, 'wildcard' => bool] entries the invalidation was requested with. + * @param string|null $domain + * @return void + */ + abstract protected function cleanupInvalidatedUrls($invalidated, $paths, $domain); + /** * Resolve the domain an invalidation entry belongs to, the same way * invalidateUrl()/invalidateWildcardUrl() would internally, so entries can diff --git a/src/StaticCaching/Cachers/ApplicationCacher.php b/src/StaticCaching/Cachers/ApplicationCacher.php index d4342e6f513..3469730520f 100644 --- a/src/StaticCaching/Cachers/ApplicationCacher.php +++ b/src/StaticCaching/Cachers/ApplicationCacher.php @@ -6,7 +6,6 @@ use Illuminate\Http\Request; use Illuminate\Routing\Events\ResponsePrepared; use Illuminate\Support\Facades\Event; -use Statamic\Events\UrlInvalidated; use Statamic\StaticCaching\Page; class ApplicationCacher extends AbstractCacher @@ -106,38 +105,34 @@ private function getFromCache(Request $request) */ public function flush() { - $this->getDomains()->each(function ($domain) { - $this->getUrls($domain)->keys()->each(function ($key) { - $this->cache->forget($this->normalizeKey('responses:'.$key)); + // Capture each domain's response keys and wipe its map under the urls + // lock, so a page cached mid-flush can't end up as a stored response + // the map doesn't know about. The responses are forgotten afterwards. + $keys = $this->getDomains()->flatMap(function ($domain) { + return $this->withLock($this->getUrlsLockKey($domain), function () use ($domain) { + $keys = $this->getUrls($domain)->keys(); + + $this->cache->forget($this->getUrlsCacheKey($domain)); + + return $keys; }); }); - $this->flushUrls(); + $this->cache->forget($this->normalizeKey('domains')); + + $keys->each(fn ($key) => $this->cache->forget($this->normalizeKey('responses:'.$key))); } /** - * Invalidate a URL. + * Forget the stored responses for invalidated urls map entries. * - * @param string $url + * @param \Illuminate\Support\Collection $invalidated + * @param \Illuminate\Support\Collection $paths * @param string|null $domain * @return void */ - public function invalidateUrl($url, $domain = null) + protected function cleanupInvalidatedUrls($invalidated, $paths, $domain) { - // For CLI contexts where Site::current()->url() may return the wrong - // domain causing getUrls() to look under the wrong cache key. - if ($domain === null) { - [$url, $domain] = $this->getPathAndDomain($url); - } - - $this - ->getUrls($domain) - ->filter(fn ($value) => $value === $url || str_starts_with($value, $url.'?')) - ->each(function ($value, $key) use ($domain) { - $this->cache->forget($this->normalizeKey('responses:'.$key)); - $this->forgetUrl($key, $domain); - }); - - UrlInvalidated::dispatch($url, $domain); + $invalidated->keys()->each(fn ($key) => $this->cache->forget($this->normalizeKey('responses:'.$key))); } } diff --git a/src/StaticCaching/Cachers/FileCacher.php b/src/StaticCaching/Cachers/FileCacher.php index e8e6151006c..0b6b9af129b 100644 --- a/src/StaticCaching/Cachers/FileCacher.php +++ b/src/StaticCaching/Cachers/FileCacher.php @@ -7,7 +7,6 @@ use Illuminate\Http\Request; use Illuminate\Support\Facades\Log; use Illuminate\Support\LazyCollection; -use Statamic\Events\UrlInvalidated; use Statamic\Facades\File; use Statamic\Facades\Path; use Statamic\Facades\Site; @@ -127,38 +126,45 @@ public function hasCachedPage(Request $request) */ public function flush() { + // Wipe the urls map before deleting files, so a page cached mid-flush + // can't end up as a file the map doesn't know about. + $this->flushUrls(); + foreach ($this->getCachePaths() as $path) { $this->writer->flush($path); } - - $this->flushUrls(); } /** - * Invalidate a URL. + * Delete the static files for invalidated urls map entries. * - * @param string $url + * @param \Illuminate\Support\Collection $invalidated + * @param \Illuminate\Support\Collection $paths + * @param string|null $domain * @return void */ - public function invalidateUrl($url, $domain = null) + protected function cleanupInvalidatedUrls($invalidated, $paths, $domain) { - $site = optional(Site::findByUrl($domain.$url))->handle(); - - $this - ->getUrls($domain) - ->filter(fn ($value) => $value === $url || str_starts_with($value, $url.'?')) - ->each(function ($value, $key) use ($site, $domain) { - $this->writer->delete($this->getFilePath($domain.$value, $site)); - $this->forgetUrl($key, $domain); - }); + $invalidated->each(function ($value) use ($domain) { + $site = optional(Site::findByUrl($domain.$value))->handle(); - $this->getFiles($site) - ->filter(fn ($file) => str_starts_with($file, $url.'_')) - ->each(function ($file, $path) { - $this->writer->delete($path); - }); + $this->writer->delete($this->getFilePath($domain.$value, $site)); + }); - UrlInvalidated::dispatch($url, $domain); + // Also delete file variants that aren't tracked in the urls map, + // e.g. pagination pages written alongside the base url. + $paths + ->flatMap(fn ($path) => $path['wildcard'] + ? $invalidated->filter(fn ($value) => Str::startsWith($value, $path['path']))->values() + : [$path['path']]) + ->groupBy(fn ($prefix) => optional(Site::findByUrl($domain.$prefix))->handle() ?? '') + ->each(function ($prefixes, $site) { + $this->getFiles($site === '' ? null : $site) + ->filter(fn ($file) => $prefixes->contains(fn ($prefix) => str_starts_with($file, $prefix.'_'))) + ->each(function ($file, $path) { + $this->writer->delete($path); + }); + }); } /** diff --git a/tests/StaticCaching/CacherConcurrencyTest.php b/tests/StaticCaching/CacherConcurrencyTest.php index ed4ccddfb0e..9e8042277de 100644 --- a/tests/StaticCaching/CacherConcurrencyTest.php +++ b/tests/StaticCaching/CacherConcurrencyTest.php @@ -11,6 +11,7 @@ use Illuminate\Support\Facades\File; use Mockery; use PHPUnit\Framework\Attributes\Test; +use Statamic\Events\UrlInvalidated; use Statamic\StaticCaching\Cachers\AbstractCacher; use Statamic\StaticCaching\Cachers\ApplicationCacher; use Statamic\StaticCaching\Cachers\FileCacher; @@ -122,10 +123,9 @@ public function invalidate_urls_only_acquires_the_domain_lock_once_for_a_multi_u 'two' => '/two', 'three' => '/three', ]); - $cache->shouldReceive('forever')->withArgs(fn ($key) => $key === $urlsKey); + // One lock, one map write for the whole batch - not one per URL. + $cache->shouldReceive('forever')->once()->withArgs(fn ($key) => $key === $urlsKey); - // The whole point of the fix: one lock covers the entire batch, not - // one acquisition per URL. $cache->shouldReceive('lock') ->once() ->withArgs(fn ($key) => $key === $lockKey) @@ -245,6 +245,94 @@ public function middleware_serves_the_rendered_response_uncached_when_the_urls_l $this->assertFileDoesNotExist($cacher->getFilePath('http://localhost/about')); } + #[Test] + public function urls_lock_is_released_before_invalidation_cleanup_and_events_run() + { + $cache = app(Repository::class); + $writer = Mockery::spy(Writer::class); + + $cacher = new FileCacher($writer, $cache, [ + 'base_url' => 'http://example.com', + 'path' => $this->cachePath(), + 'locale' => 'en', + ]); + + $cache->forever('static-cache:'.md5('http://example.com').'.urls', ['one' => '/one']); + + $lockKey = 'static-cache:'.md5('http://example.com').'.urls:lock'; + + // If invalidation still held the urls lock while dispatching events, + // this listener wouldn't be able to acquire it - and neither would a + // visitor request trying to cache a page during a long cleanup. + $lockWasFree = null; + Event::listen(UrlInvalidated::class, function () use ($cache, $lockKey, &$lockWasFree) { + $lock = $cache->lock($lockKey, 10); + + if ($lockWasFree = $lock->get()) { + $lock->release(); + } + }); + + $cacher->invalidateUrls(['/one']); + + $this->assertTrue($lockWasFree, 'The urls lock should be released before cleanup and events run.'); + $this->assertSame([], $cacher->getUrls('http://example.com')->all()); + } + + #[Test] + public function file_cacher_flush_wipes_the_urls_map_before_deleting_files() + { + $cache = app(Repository::class); + $urlsKey = 'static-cache:'.md5('http://example.com').'.urls'; + + $cache->forever('static-cache:domains', ['http://example.com']); + $cache->forever($urlsKey, ['one' => '/one']); + + // If files were deleted first, a request re-rendering one of them could + // write a fresh file whose map entry is then wiped - an orphan. Map + // first means the worst case is a map entry without a file, which the + // next request heals by re-caching under the same key. + $writer = Mockery::mock(Writer::class); + $writer->shouldReceive('flush')->once()->andReturnUsing(function () use ($cache, $urlsKey) { + $this->assertNull($cache->get($urlsKey), 'The urls map should be wiped before files are deleted.'); + }); + + $cacher = new FileCacher($writer, $cache, [ + 'base_url' => 'http://example.com', + 'path' => $this->cachePath(), + 'locale' => 'en', + ]); + + $cacher->flush(); + + $this->assertNull($cache->get($urlsKey)); + $this->assertNull($cache->get('static-cache:domains')); + } + + #[Test] + public function application_cacher_flush_wipes_the_urls_map_before_forgetting_responses() + { + $urlsKey = 'static-cache:'.md5('http://example.com').'.urls'; + + $store = Mockery::mock(\Illuminate\Contracts\Cache\LockProvider::class, \Illuminate\Contracts\Cache\Store::class); + $lock = Mockery::mock(\Illuminate\Contracts\Cache\Lock::class); + $lock->shouldReceive('block')->andReturnUsing(fn ($seconds, $callback) => $callback()); + + $cache = Mockery::mock(\Illuminate\Contracts\Cache\Repository::class); + $cache->shouldReceive('getStore')->andReturn($store); + $cache->shouldReceive('lock')->andReturn($lock); + $cache->shouldReceive('get')->with('static-cache:domains', [])->andReturn(['http://example.com']); + $cache->shouldReceive('get')->with($urlsKey, [])->andReturn(['one' => '/one']); + + $cache->shouldReceive('forget')->with($urlsKey)->once()->ordered(); + $cache->shouldReceive('forget')->with('static-cache:domains')->once()->ordered(); + $cache->shouldReceive('forget')->with('static-cache:responses:one')->once()->ordered(); + + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + + $cacher->flush(); + } + private function cachePath() { return storage_path('framework/testing/static-cache-concurrency'); @@ -265,10 +353,8 @@ private function cacher($cache, $config = []) } /** - * A minimal concrete AbstractCacher whose invalidateUrl() mirrors the real - * FileCacher/ApplicationCacher pattern of looking up matching urls map - * entries and calling forgetUrl() per match, so tests can exercise the - * real nested-lock path (invalidateUrls() -> invalidateUrl() -> forgetUrl()). + * A minimal concrete AbstractCacher with no driver-side storage, so tests + * can exercise the shared two-phase invalidation path directly. */ private function concreteCacher($cache, $config = []) { @@ -286,15 +372,8 @@ public function flush() { } - public function invalidateUrl($url, $domain = null) + protected function cleanupInvalidatedUrls($invalidated, $paths, $domain) { - $domain = $domain ?? $this->getBaseUrl(); - - $this->getUrls($domain) - ->filter(fn ($value) => $value === $url) - ->each(function ($value, $key) use ($domain) { - $this->forgetUrl($key, $domain); - }); } }; } diff --git a/tests/StaticCaching/CacherTest.php b/tests/StaticCaching/CacherTest.php index 2c544275030..8ac16b2bb6e 100644 --- a/tests/StaticCaching/CacherTest.php +++ b/tests/StaticCaching/CacherTest.php @@ -4,8 +4,10 @@ use Illuminate\Cache\Repository; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Event; use Mockery; use PHPUnit\Framework\Attributes\Test; +use Statamic\Events\UrlInvalidated; use Statamic\StaticCaching\Cachers\AbstractCacher; use Tests\TestCase; @@ -238,6 +240,8 @@ public function it_asks_the_url_excluder_if_a_url_should_be_excluder() #[Test] public function it_invalidates_urls() { + Event::fake(); + $cache = app(Repository::class); $this->setSites([ @@ -259,13 +263,6 @@ public function it_invalidates_urls() $cacher = $this->cacher(); - $cacher->shouldReceive('invalidateUrl')->once()->with('/', 'http://example.com'); - $cacher->shouldReceive('invalidateUrl')->twice()->with('/one', 'http://example.com'); - $cacher->shouldReceive('invalidateUrl')->once()->with('/two', 'http://example.com'); - $cacher->shouldReceive('invalidateUrl')->once()->with('/three', 'http://example.co.uk'); - $cacher->shouldReceive('invalidateUrl')->times(3)->with('/blog/post', 'http://example.com'); - $cacher->shouldReceive('invalidateUrl')->once()->with('/blog/post', 'http://example.co.uk'); - $cacher->invalidateUrls([ '/', '/one', @@ -277,6 +274,22 @@ public function it_invalidates_urls() 'http://example.com/blog/*', 'http://example.co.uk/blog/*', ]); + + // Matching entries are removed from each domain's urls map. + $this->assertEquals(['blog' => '/blog', 'contact' => '/contact'], $cacher->getUrls('http://example.com')->all()); + $this->assertEquals(['blog' => '/blog', 'contact' => '/contact'], $cacher->getUrls('http://example.co.uk')->all()); + + // An event fires for every requested path on its resolved domain, with + // wildcards expanding to the map entries they matched: '/' (1), + // '/one' and 'one' (2), '/two' (1), '/three' (1), and '/blog/post' + // once per wildcard on its domain (3 + 1). + Event::assertDispatchedTimes(UrlInvalidated::class, 9); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/'); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/one'); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/two'); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.co.uk/three'); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/blog/post'); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.co.uk/blog/post'); } private function cacher($config = []) diff --git a/tests/StaticCaching/FileCacherTest.php b/tests/StaticCaching/FileCacherTest.php index f8992335821..1a99ddca8d0 100644 --- a/tests/StaticCaching/FileCacherTest.php +++ b/tests/StaticCaching/FileCacherTest.php @@ -345,6 +345,59 @@ public function invalidating_a_url_dispatches_event($domain, $expectedUrl) }); } + #[Test] + public function invalidating_multiple_urls_deletes_files_and_removes_entries_for_exact_and_wildcard_urls() + { + Event::fake(); + + $writer = \Mockery::spy(Writer::class); + $cache = app(Repository::class); + $cacher = $this->fileCacher(['base_url' => 'http://example.com'], $writer, $cache); + + // Put it in the container so that the event can resolve it. + $this->instance(Cacher::class, $cacher); + + $cache->forever($this->cacheKey('http://example.com'), [ + 'one' => '/one', + 'oneqs' => '/one?foo=bar', + 'blogone' => '/blog/one', + 'blogtwo' => '/blog/two', + 'other' => '/other', + ]); + + $cacher->invalidateUrls(['/one', '/blog/*']); + + $writer->shouldHaveReceived('delete')->with($cacher->getFilePath('/one'))->once(); + $writer->shouldHaveReceived('delete')->with($cacher->getFilePath('/one?foo=bar'))->once(); + $writer->shouldHaveReceived('delete')->with($cacher->getFilePath('/blog/one'))->once(); + $writer->shouldHaveReceived('delete')->with($cacher->getFilePath('/blog/two'))->once(); + $this->assertEquals(['other' => '/other'], $cacher->getUrls('http://example.com')->all()); + + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/one'); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/blog/one'); + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/blog/two'); + } + + #[Test] + public function invalidating_urls_not_in_the_map_still_dispatches_events() + { + Event::fake(); + + $writer = \Mockery::spy(Writer::class); + $cache = app(Repository::class); + $cacher = $this->fileCacher(['base_url' => 'http://example.com'], $writer, $cache); + + $this->instance(Cacher::class, $cacher); + + $cacher->invalidateUrls(['/missing']); + + $writer->shouldNotHaveReceived('delete'); + + // Listeners like CDN purgers rely on the event firing regardless of + // whether the url was tracked in the local map. + Event::assertDispatched(UrlInvalidated::class, fn ($event) => $event->url === 'http://example.com/missing'); + } + #[Test] public function recaching_a_url_will_trigger_a_recache_job() { From 4abca3278564e9d10d9cedc04481aa5aeefeaf65 Mon Sep 17 00:00:00 2001 From: Andrew Clinton Date: Tue, 28 Jul 2026 13:16:41 -0400 Subject: [PATCH 5/5] Move the lock timeout message into the translation file The lock-contention error added to the CP's "invalidate URLs" utility used the full sentence as its own translation key. Sentence-length strings belong in a lang file, so addons and translators can override them without matching the English copy verbatim. Co-Authored-By: Claude Opus 5 --- lang/en/messages.php | 1 + src/Http/Controllers/CP/Utilities/CacheController.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lang/en/messages.php b/lang/en/messages.php index f6339ba62df..6714acaebcb 100644 --- a/lang/en/messages.php +++ b/lang/en/messages.php @@ -25,6 +25,7 @@ 'cache_utility_image_cache_description' => 'The image cache stores copies of all transformed and resized images.', 'cache_utility_stache_description' => 'The Stache is Statamic\'s content store that functions much like a database. It is generated automatically from the content files.', 'cache_utility_static_cache_description' => 'Static pages bypass Statamic completely and are rendered directly from the server for maximum performance.', + 'cache_utility_static_cache_invalidate_urls_locked' => 'Could not invalidate URLs because the cache was locked by another process. Please try again.', 'choose_entry_localization_deletion_behavior' => 'Choose the action you wish to perform on the localized entries.', 'collection_configure_date_behavior_private' => 'Private - Hidden from listings, URLs return 404', 'collection_configure_date_behavior_public' => 'Public - Always visible', diff --git a/src/Http/Controllers/CP/Utilities/CacheController.php b/src/Http/Controllers/CP/Utilities/CacheController.php index f458ef4a2a3..7d7e52bb852 100644 --- a/src/Http/Controllers/CP/Utilities/CacheController.php +++ b/src/Http/Controllers/CP/Utilities/CacheController.php @@ -137,7 +137,7 @@ public function invalidateStaticUrls(Request $request) try { app(Cacher::class)->invalidateUrls($urls); } catch (LockTimeoutException $e) { - return back()->withError(__('Could not invalidate URLs because the cache was locked by another process. Please try again.')); + return back()->withError(__('statamic::messages.cache_utility_static_cache_invalidate_urls_locked')); } return back()->withSuccess(__('Invalidated URLs in the Static Cache.'));