diff --git a/src/Assets/AssetContainer.php b/src/Assets/AssetContainer.php index d4bbbee5db6..3566540f3f3 100644 --- a/src/Assets/AssetContainer.php +++ b/src/Assets/AssetContainer.php @@ -27,7 +27,6 @@ use Statamic\Facades\Search; use Statamic\Facades\Stache; use Statamic\Facades\URL; -use Statamic\Statamic; use Statamic\Support\Arr; use Statamic\Support\Str; use Statamic\Support\Traits\FluentlyGetsAndSets; @@ -345,7 +344,7 @@ public function listContents() public function contents() { - return Blink::onceIf(! Statamic::isWorker(), 'asset-listing-cache-'.$this->handle(), function () { + return Blink::once('asset-listing-cache-'.$this->handle(), function () { return app(AssetContainerContents::class)->container($this); }); } diff --git a/src/Assets/AssetContainerContents.php b/src/Assets/AssetContainerContents.php index fa9b33e4278..9c32f2defba 100644 --- a/src/Assets/AssetContainerContents.php +++ b/src/Assets/AssetContainerContents.php @@ -6,7 +6,6 @@ use Illuminate\Support\Facades\Cache; use League\Flysystem\DirectoryListing; use Statamic\Facades\Stache; -use Statamic\Statamic; use Statamic\Support\Str; class AssetContainerContents @@ -31,7 +30,7 @@ public function container($container) */ public function all() { - if ($this->files && ! Statamic::isWorker()) { + if ($this->files) { return $this->files; } @@ -209,7 +208,7 @@ public function metaFilesIn($folder, $recursive) public function filteredFilesIn($folder, $recursive) { - if (isset($this->filteredFiles[$key = $folder.($recursive ? '-recursive' : '')]) && ! Statamic::isWorker()) { + if (isset($this->filteredFiles[$key = $folder.($recursive ? '-recursive' : '')])) { return $this->filteredFiles[$key]; } @@ -239,7 +238,7 @@ public function filteredFilesIn($folder, $recursive) public function filteredDirectoriesIn($folder, $recursive) { - if (isset($this->filteredDirectories[$key = $folder.($recursive ? '-recursive' : '')]) && ! Statamic::isWorker()) { + if (isset($this->filteredDirectories[$key = $folder.($recursive ? '-recursive' : '')])) { return $this->filteredDirectories[$key]; } @@ -297,10 +296,6 @@ public function add($path) $files = $this->all()->put($path, $metadata); - if (Statamic::isWorker()) { - $this->cacheStore()->put($this->key(), $files, $this->ttl()); - } - $this->filteredFiles = null; $this->filteredDirectories = null; diff --git a/src/Stache/Indexes/Index.php b/src/Stache/Indexes/Index.php index 61182a2e005..f8be968065c 100644 --- a/src/Stache/Indexes/Index.php +++ b/src/Stache/Indexes/Index.php @@ -3,7 +3,6 @@ namespace Statamic\Stache\Indexes; use Statamic\Facades\Stache; -use Statamic\Statamic; abstract class Index { @@ -66,17 +65,12 @@ public function load() } $loadingKey = $this->store->key().'/'.$this->name; - $currentlyLoadingThis = in_array($loadingKey, static::$loadingStack); static::$loadingStack[] = $loadingKey; try { $this->loaded = true; - if (Statamic::isWorker() && ! $currentlyLoadingThis) { - $this->loaded = false; - } - debugbar()->addMessage("Loading index: {$loadingKey}", 'stache'); $this->items = Stache::cacheStore()->get($this->cacheKey()); @@ -163,6 +157,11 @@ public function clear() Stache::cacheStore()->forget($this->cacheKey()); } + public function resetMemoizedState() + { + $this->loaded = false; + } + /** @deprecated */ public static function currentlyLoading() { diff --git a/src/Stache/ServiceProvider.php b/src/Stache/ServiceProvider.php index 6981985207e..b54b619e676 100644 --- a/src/Stache/ServiceProvider.php +++ b/src/Stache/ServiceProvider.php @@ -2,12 +2,15 @@ namespace Statamic\Stache; +use Illuminate\Queue\Events\JobProcessing; +use Illuminate\Support\Facades\Event; use Illuminate\Support\ServiceProvider as LaravelServiceProvider; use Statamic\Assets\QueryBuilder as AssetQueryBuilder; use Statamic\Facades\File; use Statamic\Facades\Site; use Statamic\Stache\Query\EntryQueryBuilder; use Statamic\Stache\Query\SubmissionQueryBuilder; +use Statamic\Statamic; use Symfony\Component\Lock\LockFactory; use Symfony\Component\Lock\Store\FlockStore; @@ -45,6 +48,8 @@ public function boot() $stache->sites(Site::all()->keys()->all()); $this->registerStores($stache); + + $this->resetMemoizedStateBetweenJobs($stache); } private function registerStores($stache) @@ -70,6 +75,19 @@ private function registerStores($stache) $stache->registerStores($stores->all()); } + private function resetMemoizedStateBetweenJobs($stache) + { + Event::listen(JobProcessing::class, function () use ($stache) { + if (! Statamic::isWorker()) { + return; + } + + $stache->stores()->each->resetMemoizedState(); + + $this->app->make('stache.indexes')->each->resetMemoizedState(); + }); + } + private function locks() { if (config('statamic.stache.lock.enabled', true)) { diff --git a/src/Stache/Stores/AggregateStore.php b/src/Stache/Stores/AggregateStore.php index 334c0b63835..df641e96186 100644 --- a/src/Stache/Stores/AggregateStore.php +++ b/src/Stache/Stores/AggregateStore.php @@ -87,6 +87,13 @@ public function warm() $this->discoverStores()->each->warm(); } + public function resetMemoizedState() + { + parent::resetMemoizedState(); + + $this->stores->each->resetMemoizedState(); + } + public function paths() { return $this->discoverStores()->flatMap(function ($store) { diff --git a/src/Stache/Stores/ContainerAssetsStore.php b/src/Stache/Stores/ContainerAssetsStore.php index bec0212f604..a8cc4424106 100644 --- a/src/Stache/Stores/ContainerAssetsStore.php +++ b/src/Stache/Stores/ContainerAssetsStore.php @@ -4,7 +4,6 @@ use Statamic\Facades\AssetContainer; use Statamic\Facades\Stache; -use Statamic\Statamic; use Statamic\Support\Str; class ContainerAssetsStore extends ChildStore @@ -54,7 +53,7 @@ public function getItemsFromFiles() public function paths() { - if ($this->paths && ! Statamic::isWorker()) { + if ($this->paths) { return $this->paths; } diff --git a/src/Stache/Stores/Store.php b/src/Stache/Stores/Store.php index c220b679e1b..b9b067a76a6 100644 --- a/src/Stache/Stores/Store.php +++ b/src/Stache/Stores/Store.php @@ -9,7 +9,6 @@ use Statamic\Stache\Exceptions\DuplicateKeyException; use Statamic\Stache\Indexes; use Statamic\Stache\Indexes\Index; -use Statamic\Statamic; use Statamic\Support\Arr; use Statamic\Support\Str; @@ -299,7 +298,7 @@ public function paths() { $this->handleFileChanges(); - if ($this->paths && ! Statamic::isWorker()) { + if ($this->paths) { return $this->paths; } @@ -378,6 +377,11 @@ public function clearCachedPaths() Stache::cacheStore()->forget($this->pathsCacheKey()); } + public function resetMemoizedState() + { + $this->paths = null; + } + protected function pathsCacheKey() { return "stache::indexes::{$this->key()}::path"; diff --git a/tests/Assets/AssetContainerTest.php b/tests/Assets/AssetContainerTest.php index 955c04bdb04..067a3e4d374 100644 --- a/tests/Assets/AssetContainerTest.php +++ b/tests/Assets/AssetContainerTest.php @@ -9,6 +9,7 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Facade; use Illuminate\Support\Facades\Request; use Illuminate\Support\Facades\Storage; use League\Flysystem\DirectoryAttributes; @@ -581,7 +582,7 @@ public function it_gets_the_files_from_the_cache_only_once() } #[Test] - public function it_gets_the_files_from_the_cache_every_time_if_running_in_a_queue_worker() + public function it_still_only_gets_the_files_from_the_cache_once_per_job_if_running_in_a_queue_worker() { $cacheKey = 'asset-list-contents-test'; @@ -607,6 +608,14 @@ public function it_gets_the_files_from_the_cache_every_time_if_running_in_a_queu $expected = ['one.jpg', 'two.jpg']; $this->assertEquals($expected, $container->files()->all()); $this->assertEquals(1, $cacheHits); + $this->assertEquals($expected, $container->files()->all()); + $this->assertEquals(1, $cacheHits); + + // Laravel's real queue worker daemon loop clears resolved facade instances + // before every job it processes, which resets the Blink-backed + // AssetContainerContents instance behind the `contents()` call. + Facade::clearResolvedInstances(); + $this->assertEquals($expected, $container->files()->all()); $this->assertEquals(2, $cacheHits); } @@ -709,7 +718,7 @@ public function it_gets_the_folders_from_the_cache_and_blink_only_once() } #[Test] - public function it_gets_the_folders_from_the_cache_and_blink_every_time_if_running_in_a_queue_worker() + public function it_still_only_gets_the_folders_from_the_cache_and_blink_once_per_job_if_running_in_a_queue_worker() { $cacheKey = 'asset-list-contents-test'; @@ -735,11 +744,20 @@ public function it_gets_the_folders_from_the_cache_and_blink_every_time_if_runni $this->assertEquals($expected, $container->folders()->all()); $this->assertEquals(1, $cacheHits); $this->assertEquals($expected, $container->folders()->all()); - $this->assertEquals(2, $cacheHits); + $this->assertEquals(1, $cacheHits); + // Still within the same job (resolved facade instances haven't been cleared), + // a freshly newed up container reuses the same Blink-cached contents. $anotherInstanceOfTheContainer = (new AssetContainer)->handle('test')->disk('test'); $this->assertEquals($expected, $anotherInstanceOfTheContainer->folders()->all()); - $this->assertEquals(3, $cacheHits); + $this->assertEquals(1, $cacheHits); + + // Laravel's real queue worker daemon loop clears resolved facade instances + // before every job it processes, which resets the Blink-backed contents cache. + Facade::clearResolvedInstances(); + + $this->assertEquals($expected, $container->folders()->all()); + $this->assertEquals(2, $cacheHits); } #[Test] @@ -757,8 +775,11 @@ public function it_does_not_leak_stale_contents_state_across_calls_when_running_ Request::swap(new FakeArtisanRequest('queue:work')); // First job populates the instance's $metaFiles cache. metaFilesIn() has no - // isWorker() guard, so if contents() reused the same instance across jobs - // the filtered result would stick around and bleed into the next job. + // memoization guard of its own, so if contents() reused the same instance + // across jobs the filtered result would stick around and bleed into the + // next job. In real usage, Laravel's queue worker daemon loop clears + // resolved facade instances before every job it processes, which resets + // the Blink-backed instance behind `contents()` at each job boundary. $this->assertEquals( ['.meta/a.txt.yaml'], $container->contents()->metaFilesIn('/', true)->keys()->all() @@ -772,6 +793,10 @@ public function it_does_not_leak_stale_contents_state_across_calls_when_running_ '.meta/b.txt.yaml' => ['type' => 'file', 'path' => '.meta/b.txt.yaml', 'dirname' => '.meta'], ])); + // Simulate the job boundary: Laravel clears resolved facade instances + // before every job. + Facade::clearResolvedInstances(); + $this->assertEquals( ['.meta/a.txt.yaml', '.meta/b.txt.yaml'], $container->contents()->metaFilesIn('/', true)->keys()->sort()->values()->all() diff --git a/tests/Stache/StoreTest.php b/tests/Stache/StoreTest.php index 419e2d2ddbc..7ce04b16e31 100644 --- a/tests/Stache/StoreTest.php +++ b/tests/Stache/StoreTest.php @@ -64,7 +64,7 @@ public function it_gets_the_paths_from_the_cache_only_once() } #[Test] - public function it_gets_the_paths_from_the_cache_every_time_if_running_in_a_queue_worker() + public function it_still_only_gets_the_paths_from_the_cache_once_per_job_if_running_in_a_queue_worker() { $store = $this->store->directory('/path/to/directory'); $cacheKey = "stache::indexes::{$store->key()}::path"; @@ -83,6 +83,33 @@ public function it_gets_the_paths_from_the_cache_every_time_if_running_in_a_queu $expected = collect(['foo', 'bar']); $this->assertEquals($expected, $store->paths()); $this->assertEquals(1, $cacheHits); + $this->assertEquals($expected, $store->paths()); + $this->assertEquals(1, $cacheHits); + } + + #[Test] + public function it_gets_the_paths_from_the_cache_again_after_memoized_state_is_reset_between_jobs() + { + $store = $this->store->directory('/path/to/directory'); + $cacheKey = "stache::indexes::{$store->key()}::path"; + + Cache::put($cacheKey, ['foo', 'bar']); + + $cacheHits = 0; + Event::listen(CacheHit::class, function ($event) use (&$cacheHits, $cacheKey) { + if ($event->key === $cacheKey) { + $cacheHits++; + } + }); + + Request::swap(new FakeArtisanRequest('queue:listen')); + + $expected = collect(['foo', 'bar']); + $this->assertEquals($expected, $store->paths()); + $this->assertEquals(1, $cacheHits); + + $store->resetMemoizedState(); + $this->assertEquals($expected, $store->paths()); $this->assertEquals(2, $cacheHits); }