From c0fa7312968aa5cc24b284e92be176a896b864d2 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Mon, 9 Nov 2026 21:54:44 +0000 Subject: [PATCH 1/2] Resolve collection sites once per traversal in the entries filter With multisite enabled, CollectionEntriesStore::getItemFilter() looked up the collection (from the cache store) for every file. Resolve the store directory and the collection's sites once per traversal instead, via a new protected Store::traverseFiles() method. Co-Authored-By: Claude Opus 5.5 --- src/Stache/Stores/CollectionEntriesStore.php | 28 +++++++++++++-- src/Stache/Stores/Store.php | 9 +++-- tests/Stache/Stores/EntriesStoreTest.php | 37 ++++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/Stache/Stores/CollectionEntriesStore.php b/src/Stache/Stores/CollectionEntriesStore.php index 09109f073da..d8a8052ac60 100644 --- a/src/Stache/Stores/CollectionEntriesStore.php +++ b/src/Stache/Stores/CollectionEntriesStore.php @@ -25,15 +25,34 @@ class CollectionEntriesStore extends ChildStore { protected $collection; private bool $shouldBlinkEntryUris = true; + private ?string $filterDirectory = null; + private ?array $filterSites = null; protected function collection() { return $this->collection ?? Collection::findByHandle($this->childKey); } + protected function traverseFiles() + { + // The filter runs once per file, so resolve anything that's the same + // for every file up front rather than repeating it for each one. + $this->filterDirectory = Str::finish($this->directory(), '/'); + $this->filterSites = Site::multiEnabled() && ($collection = $this->collection()) + ? $collection->sites()->flip()->all() + : null; + + try { + return parent::traverseFiles(); + } finally { + $this->filterDirectory = null; + $this->filterSites = null; + } + } + public function getItemFilter(SplFileInfo $file) { - $dir = Str::finish($this->directory(), '/'); + $dir = $this->filterDirectory ?? Str::finish($this->directory(), '/'); $relative = Path::tidy($file->getPathname()); if (substr($relative, 0, strlen($dir)) == $dir) { @@ -42,7 +61,12 @@ public function getItemFilter(SplFileInfo $file) if (Site::multiEnabled()) { [$site, $relative] = explode('/', $relative, 2); - if (! $this->collection()->sites()->contains($site)) { + + $allowed = $this->filterSites !== null + ? isset($this->filterSites[$site]) + : $this->collection()->sites()->contains($site); + + if (! $allowed) { return false; } } diff --git a/src/Stache/Stores/Store.php b/src/Stache/Stores/Store.php index ee31e4236a9..53a1d9f78b7 100644 --- a/src/Stache/Stores/Store.php +++ b/src/Stache/Stores/Store.php @@ -194,7 +194,7 @@ public function handleFileChanges() $existing = collect(Stache::cacheStore()->get($cacheKey, [])); // Get the files and timestamps from the filesystem right now. - $files = Traverser::filter([$this, 'getItemFilter'])->traverse($this); + $files = $this->traverseFiles(); // Cache the files and timestamps, ready for comparisons on the next request. // We'll do it now since there are multiple early returns coming up. @@ -283,6 +283,11 @@ public function handleFileChanges() $this->modified = $modified; } + protected function traverseFiles() + { + return Traverser::filter([$this, 'getItemFilter'])->traverse($this); + } + protected function handleModifiedItem($item) { // @@ -310,7 +315,7 @@ public function paths() return $this->paths = collect($paths); } - $files = Traverser::filter([$this, 'getItemFilter'])->traverse($this); + $files = $this->traverseFiles(); $fileItems = $files->map(function ($timestamp, $path) { return [ diff --git a/tests/Stache/Stores/EntriesStoreTest.php b/tests/Stache/Stores/EntriesStoreTest.php index 705a70f3c75..46a85989c3a 100644 --- a/tests/Stache/Stores/EntriesStoreTest.php +++ b/tests/Stache/Stores/EntriesStoreTest.php @@ -3,6 +3,7 @@ namespace Tests\Stache\Stores; use Facades\Statamic\Stache\Traverser; +use Illuminate\Filesystem\Filesystem; use Illuminate\Support\Carbon; use Mockery; use PHPUnit\Framework\Attributes\DataProvider; @@ -35,6 +36,42 @@ public function setUp(): void Stache::store('collections')->directory($this->directory); } + #[Test] + public function it_only_gets_files_for_the_collections_sites_when_multisite_is_enabled() + { + $this->setSites([ + 'en' => ['url' => '/'], + 'fr' => ['url' => '/fr/'], + 'de' => ['url' => '/de/'], + ]); + + $collection = Facades\Collection::make('multisite_pages')->sites(['en', 'fr']); + Facades\Collection::partialMock()->shouldReceive('findByHandle')->with('multisite_pages')->andReturn($collection); + + $dir = Path::tidy(__DIR__.'/tmp-multisite'); + foreach (['en', 'fr', 'de'] as $site) { + @mkdir($dir.'/multisite_pages/'.$site, 0777, true); + touch($dir.'/multisite_pages/'.$site.'/page.md', 1234567890); + } + + $store = (new EntriesStore)->directory($dir)->store('multisite_pages'); + + $expected = [ + $dir.'/multisite_pages/en/page.md', + $dir.'/multisite_pages/fr/page.md', + ]; + + try { + // When the filter is called directly, it resolves the collection's sites for each file. + $this->assertEquals($expected, Traverser::filter([$store, 'getItemFilter'])->traverse($store)->keys()->all()); + + // When the store traverses its own files, it resolves them once up front. + $this->assertEquals($expected, (fn () => $this->traverseFiles())->call($store)->keys()->all()); + } finally { + (new Filesystem)->deleteDirectory($dir); + } + } + #[Test] public function it_gets_nested_files() { From 25433f9bd491deea40bd98b4fb3f5796a5802894 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 11 Nov 2026 13:16:42 -0500 Subject: [PATCH 2/2] Reuse the bulk collection lookup in the entries filter instead of a separate traversal hook Co-Authored-By: Claude Sonnet 5.5 --- src/Stache/Stores/CollectionEntriesStore.php | 28 +------- src/Stache/Stores/Store.php | 9 +-- tests/Stache/Stores/EntriesStoreTest.php | 76 +++++++++++--------- 3 files changed, 47 insertions(+), 66 deletions(-) diff --git a/src/Stache/Stores/CollectionEntriesStore.php b/src/Stache/Stores/CollectionEntriesStore.php index 79ebcffd381..ecf192ad44b 100644 --- a/src/Stache/Stores/CollectionEntriesStore.php +++ b/src/Stache/Stores/CollectionEntriesStore.php @@ -25,8 +25,6 @@ class CollectionEntriesStore extends ChildStore { protected $collection; private bool $shouldBlinkEntryUris = true; - private ?string $filterDirectory = null; - private ?array $filterSites = null; private int $makingItemsInBulk = 0; private array $collectionsForBulk = []; @@ -78,26 +76,9 @@ protected function collection() return $this->collection ?? Collection::findByHandle($this->childKey); } - protected function traverseFiles() - { - // The filter runs once per file, so resolve anything that's the same - // for every file up front rather than repeating it for each one. - $this->filterDirectory = Str::finish($this->directory(), '/'); - $this->filterSites = Site::multiEnabled() && ($collection = $this->collection()) - ? $collection->sites()->flip()->all() - : null; - - try { - return parent::traverseFiles(); - } finally { - $this->filterDirectory = null; - $this->filterSites = null; - } - } - public function getItemFilter(SplFileInfo $file) { - $dir = $this->filterDirectory ?? Str::finish($this->directory(), '/'); + $dir = Str::finish($this->directory(), '/'); $relative = Path::tidy($file->getPathname()); if (substr($relative, 0, strlen($dir)) == $dir) { @@ -106,12 +87,9 @@ public function getItemFilter(SplFileInfo $file) if (Site::multiEnabled()) { [$site, $relative] = explode('/', $relative, 2); + $collection = $this->collection ?? $this->findCollection($this->childKey); - $allowed = $this->filterSites !== null - ? isset($this->filterSites[$site]) - : $this->collection()->sites()->contains($site); - - if (! $allowed) { + if (! $collection->sites()->contains($site)) { return false; } } diff --git a/src/Stache/Stores/Store.php b/src/Stache/Stores/Store.php index 53a1d9f78b7..ee31e4236a9 100644 --- a/src/Stache/Stores/Store.php +++ b/src/Stache/Stores/Store.php @@ -194,7 +194,7 @@ public function handleFileChanges() $existing = collect(Stache::cacheStore()->get($cacheKey, [])); // Get the files and timestamps from the filesystem right now. - $files = $this->traverseFiles(); + $files = Traverser::filter([$this, 'getItemFilter'])->traverse($this); // Cache the files and timestamps, ready for comparisons on the next request. // We'll do it now since there are multiple early returns coming up. @@ -283,11 +283,6 @@ public function handleFileChanges() $this->modified = $modified; } - protected function traverseFiles() - { - return Traverser::filter([$this, 'getItemFilter'])->traverse($this); - } - protected function handleModifiedItem($item) { // @@ -315,7 +310,7 @@ public function paths() return $this->paths = collect($paths); } - $files = $this->traverseFiles(); + $files = Traverser::filter([$this, 'getItemFilter'])->traverse($this); $fileItems = $files->map(function ($timestamp, $path) { return [ diff --git a/tests/Stache/Stores/EntriesStoreTest.php b/tests/Stache/Stores/EntriesStoreTest.php index b4707a15887..99badf63833 100644 --- a/tests/Stache/Stores/EntriesStoreTest.php +++ b/tests/Stache/Stores/EntriesStoreTest.php @@ -37,60 +37,68 @@ public function setUp(): void } #[Test] - public function it_only_gets_files_for_the_collections_sites_when_multisite_is_enabled() + public function it_only_looks_up_the_collection_once_when_building_paths() { - $this->setSites([ - 'en' => ['url' => '/'], - 'fr' => ['url' => '/fr/'], - 'de' => ['url' => '/de/'], - ]); - - $collection = Facades\Collection::make('multisite_pages')->sites(['en', 'fr']); - Facades\Collection::partialMock()->shouldReceive('findByHandle')->with('multisite_pages')->andReturn($collection); - - $dir = Path::tidy(__DIR__.'/tmp-multisite'); - foreach (['en', 'fr', 'de'] as $site) { - @mkdir($dir.'/multisite_pages/'.$site, 0777, true); - touch($dir.'/multisite_pages/'.$site.'/page.md', 1234567890); - } + $collection = Facades\Collection::findByHandle('numeric'); - $store = (new EntriesStore)->directory($dir)->store('multisite_pages'); + $lookups = 0; + Facades\Collection::partialMock() + ->shouldReceive('findByHandle') + ->with('numeric') + ->andReturnUsing(function () use ($collection, &$lookups) { + $lookups++; - $expected = [ - $dir.'/multisite_pages/en/page.md', - $dir.'/multisite_pages/fr/page.md', - ]; + return $collection; + }); - try { - // When the filter is called directly, it resolves the collection's sites for each file. - $this->assertEquals($expected, Traverser::filter([$store, 'getItemFilter'])->traverse($store)->keys()->all()); + $store = $this->parent->store('numeric'); - // When the store traverses its own files, it resolves them once up front. - $this->assertEquals($expected, (fn () => $this->traverseFiles())->call($store)->keys()->all()); - } finally { - (new Filesystem)->deleteDirectory($dir); - } + $this->assertCount(3, $store->paths()); + $this->assertEquals(1, $lookups); } #[Test] - public function it_only_looks_up_the_collection_once_when_building_paths() + public function it_only_looks_up_the_collection_once_when_filtering_files_on_multisite() { - $collection = Facades\Collection::findByHandle('numeric'); + $this->setSites([ + 'en' => ['url' => '/'], + 'fr' => ['url' => '/fr/'], + 'de' => ['url' => '/de/'], + ]); + + $collection = Facades\Collection::make('multisite_pages')->sites(['en', 'fr']); $lookups = 0; Facades\Collection::partialMock() ->shouldReceive('findByHandle') - ->with('numeric') + ->with('multisite_pages') ->andReturnUsing(function () use ($collection, &$lookups) { $lookups++; return $collection; }); - $store = $this->parent->store('numeric'); + $dir = Path::tidy(__DIR__.'/tmp-multisite'); + foreach (['en', 'fr', 'de'] as $site) { + @mkdir($dir.'/multisite_pages/'.$site, 0777, true); + foreach (['one', 'two'] as $slug) { + file_put_contents($dir.'/multisite_pages/'.$site.'/'.$slug.'.md', "---\nid: $site-$slug\n---\n"); + } + } - $this->assertCount(3, $store->paths()); - $this->assertEquals(1, $lookups); + $store = (new EntriesStore)->directory($dir)->store('multisite_pages'); + + try { + $this->assertEquals([ + $dir.'/multisite_pages/en/one.md', + $dir.'/multisite_pages/en/two.md', + $dir.'/multisite_pages/fr/one.md', + $dir.'/multisite_pages/fr/two.md', + ], $store->paths()->values()->sort()->values()->all()); + $this->assertEquals(1, $lookups); + } finally { + (new Filesystem)->deleteDirectory($dir); + } } #[Test]