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 e9a42d0df09..7d7e52bb852 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(__('statamic::messages.cache_utility_static_cache_invalidate_urls_locked')); + } 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..ab324ea8b52 100644 --- a/src/StaticCaching/Cachers/AbstractCacher.php +++ b/src/StaticCaching/Cachers/AbstractCacher.php @@ -3,11 +3,13 @@ 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; 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; @@ -17,6 +19,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 */ @@ -27,6 +39,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 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 + */ + private $heldLocks = []; + public function __construct(Repository $cache, $config) { $this->cache = $cache; @@ -125,13 +147,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()); + }); } /** @@ -155,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')); @@ -174,13 +202,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 +221,69 @@ 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); + + $this->cache->forever($this->getUrlsCacheKey($domain), $urls->all()); + }); + } - $urls->forget($key); + /** + * 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 (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, function () use ($key, $callback) { + $this->heldLocks[$key] = true; + + try { + return $callback(); + } finally { + unset($this->heldLocks[$key]); + } + }); + } + + /** + * @param string|null $domain + * @return string + */ + 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->cache->forever($this->getUrlsCacheKey($domain), $urls->all()); + $this->invalidatePathsForDomain([$url], $domain); } /** @@ -205,16 +293,9 @@ public function forgetUrl($key, $domain = null) */ protected function invalidateWildcardUrl($wildcard) { - // Remove the asterisk - $wildcard = substr($wildcard, 0, -1); - - [$wildcard, $domain] = $this->getPathAndDomain($wildcard); + [, $domain] = $this->getPathAndDomain(substr($wildcard, 0, -1)); - $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); } /** @@ -225,15 +306,93 @@ protected function invalidateWildcardUrl($wildcard) */ 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(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 + * 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; + } + /** * Refresh multiple URLs. * diff --git a/src/StaticCaching/Cachers/ApplicationCacher.php b/src/StaticCaching/Cachers/ApplicationCacher.php index 92bae1e6747..3469730520f 100644 --- a/src/StaticCaching/Cachers/ApplicationCacher.php +++ b/src/StaticCaching/Cachers/ApplicationCacher.php @@ -2,10 +2,10 @@ namespace Statamic\StaticCaching\Cachers; +use Illuminate\Contracts\Cache\LockTimeoutException; 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 @@ -38,8 +38,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); @@ -99,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 2b339e40120..0b6b9af129b 100644 --- a/src/StaticCaching/Cachers/FileCacher.php +++ b/src/StaticCaching/Cachers/FileCacher.php @@ -2,11 +2,11 @@ namespace Statamic\StaticCaching\Cachers; +use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Contracts\Cache\Repository; 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; @@ -81,7 +81,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() @@ -119,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 new file mode 100644 index 00000000000..9e8042277de --- /dev/null +++ b/tests/StaticCaching/CacherConcurrencyTest.php @@ -0,0 +1,380 @@ +cachePath()); + + parent::tearDown(); + } + + #[Test] + public function cache_url_is_blocked_while_another_process_holds_the_urls_lock_for_the_domain() + { + $cache = app(Repository::class); + $cacher = $this->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); + } + + #[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', + ]); + // One lock, one map write for the whole batch - not one per URL. + $cache->shouldReceive('forever')->once()->withArgs(fn ($key) => $key === $urlsKey); + + $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)); + } + + #[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')); + } + + #[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'); + } + + 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(); + } + + /** + * 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 = []) + { + return new class($cache, $config) extends AbstractCacher + { + public function cachePage(Request $request, $content) + { + } + + public function getCachedPage(Request $request) + { + } + + public function flush() + { + } + + protected function cleanupInvalidatedUrls($invalidated, $paths, $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() {