diff --git a/src/Stache/Stores/BasicStore.php b/src/Stache/Stores/BasicStore.php index 560faf35e36..06b858271f2 100644 --- a/src/Stache/Stores/BasicStore.php +++ b/src/Stache/Stores/BasicStore.php @@ -35,7 +35,19 @@ public function getItem($key) return null; } - $item = $this->makeItemFromFile($path, File::get($path)); + $contents = File::get($path); + + // A file a fresh listing shows was removed after its path was recorded is + // treated as gone, rather than rebuilt from no contents as a new, empty item + if ($contents === null) { + $listing = @scandir(dirname($path)); + + if ($listing !== false && ! in_array(basename($path), $listing, true)) { + return null; + } + } + + $item = $this->makeItemFromFile($path, $contents); $this->cacheItem($item); diff --git a/src/Stache/Stores/Store.php b/src/Stache/Stores/Store.php index ee31e4236a9..446432c2897 100644 --- a/src/Stache/Stores/Store.php +++ b/src/Stache/Stores/Store.php @@ -295,7 +295,19 @@ protected function handleDeletedItem($item, $key) protected function getItemFromModifiedPath($path) { - return $this->makeItemFromFile($path, File::get($path)); + $contents = File::get($path); + + // A file a fresh listing shows was removed after the traversal stat'ed it is + // skipped, rather than rebuilt from no contents as a new, empty item + if ($contents === null) { + $listing = @scandir(dirname($path)); + + if ($listing !== false && ! in_array(basename($path), $listing, true)) { + return null; + } + } + + return $this->makeItemFromFile($path, $contents); } public function paths() @@ -313,12 +325,24 @@ public function paths() $files = Traverser::filter([$this, 'getItemFilter'])->traverse($this); $fileItems = $files->map(function ($timestamp, $path) { + $contents = File::get($path); + + // A file a fresh listing shows was removed after the traversal stat'ed it is + // skipped, rather than rebuilt from no contents as a new, empty item + if ($contents === null) { + $listing = @scandir(dirname($path)); + + if ($listing !== false && ! in_array(basename($path), $listing, true)) { + return null; + } + } + return [ - 'item' => $item = $this->makeItemFromFile($path, File::get($path)), + 'item' => $item = $this->makeItemFromFile($path, $contents), 'key' => $this->getItemKey($item), 'path' => $path, ]; - }); + })->filter(); $items = $fileItems->reject(function ($item) { try { diff --git a/src/Stache/Stores/TaxonomyTermsStore.php b/src/Stache/Stores/TaxonomyTermsStore.php index c70a87eca58..9f08db72f7d 100644 --- a/src/Stache/Stores/TaxonomyTermsStore.php +++ b/src/Stache/Stores/TaxonomyTermsStore.php @@ -94,7 +94,19 @@ public function getItem($key) [$site, $slug] = explode('::', $key); if ($path = $this->getPath($key)) { - $term = $this->makeItemFromFile($path, File::get($path)); + $contents = File::get($path); + + // A file a fresh listing shows was removed after its path was recorded is + // treated as gone, rather than rebuilt from no contents as a new, empty item + if ($contents === null) { + $listing = @scandir(dirname($path)); + + if ($listing !== false && ! in_array(basename($path), $listing, true)) { + return null; + } + } + + $term = $this->makeItemFromFile($path, $contents); } else { $term = Term::make($slug) ->taxonomy($this->childKey()) @@ -189,7 +201,19 @@ public function paths() $files = Traverser::filter([$this, 'getItemFilter'])->traverse($this); $paths = $files->mapWithKeys(function ($timestamp, $path) { - $term = $this->makeItemFromFile($path, File::get($path)); + $contents = File::get($path); + + // A file a fresh listing shows was removed after its path was recorded is + // treated as gone, rather than rebuilt from no contents as a new, empty item + if ($contents === null) { + $listing = @scandir(dirname($path)); + + if ($listing !== false && ! in_array(basename($path), $listing, true)) { + return []; + } + } + + $term = $this->makeItemFromFile($path, $contents); return $term->localizations()->flatMap(function ($localization, $locale) use ($path) { $this->cacheItem($localization); @@ -259,6 +283,7 @@ public function delete($term) protected function getItemFromModifiedPath($path) { - return parent::getItemFromModifiedPath($path)->localizations()->all(); + // The parent skips a file that vanished before its read, and returns null for it + return parent::getItemFromModifiedPath($path)?->localizations()->all() ?? []; } } diff --git a/src/Stache/Traverser.php b/src/Stache/Traverser.php index 9ef7ae90f5c..2a22040452b 100644 --- a/src/Stache/Traverser.php +++ b/src/Stache/Traverser.php @@ -29,7 +29,20 @@ public function traverse($store) continue; } - $paths[Path::tidy($file->getPathname())] = $file->getMTime(); + try { + $mtime = $file->getMTime(); + } catch (\RuntimeException $e) { + // Skip the file only when a fresh listing shows it was removed after it was listed + $listing = @scandir($file->getPath()); + + if ($listing === false || in_array($file->getFilename(), $listing, true)) { + throw $e; + } + + continue; + } + + $paths[Path::tidy($file->getPathname())] = $mtime; } // Sort by path, then (stably) by timestamp, so files with the same timestamp are diff --git a/tests/Stache/BasicStoreTest.php b/tests/Stache/BasicStoreTest.php index abaabbe526f..8878fbfb8aa 100644 --- a/tests/Stache/BasicStoreTest.php +++ b/tests/Stache/BasicStoreTest.php @@ -2,9 +2,11 @@ namespace Tests\Stache; +use Facades\Statamic\Stache\Traverser; use Illuminate\Filesystem\Filesystem; use Illuminate\Support\Facades\Cache; use PHPUnit\Framework\Attributes\Test; +use Statamic\Facades\File; use Statamic\Facades\Path; use Statamic\Facades\YAML; use Statamic\Stache\Stores\BasicStore; @@ -99,6 +101,40 @@ public function clearing_a_cold_store_does_not_read_any_files() $this->assertEquals(0, $this->store->madeItemCount); } + #[Test] + public function it_does_not_rebuild_an_item_whose_file_was_removed_after_its_path_was_recorded() + { + file_put_contents($this->tempDir.'/foo.yaml', ''); + $this->store->paths(); + + // The path is recorded, but the item has not been built. + $this->store->forgetItem('foo'); + $store = $this->store; + + unlink($this->tempDir.'/foo.yaml'); + + // A file removed between the existence check and the read. + File::partialMock()->shouldReceive('exists')->with($this->tempDir.'/foo.yaml')->andReturnTrue(); + + $this->assertNull($store->getItem('foo')); + } + + #[Test] + public function paths_skip_a_file_removed_after_the_traversal_stat() + { + file_put_contents($this->tempDir.'/foo.yaml', ''); + file_put_contents($this->tempDir.'/bar.yaml', ''); + + Traverser::shouldReceive('filter')->andReturnSelf(); + Traverser::shouldReceive('traverse')->andReturn(collect([ + $this->tempDir.'/bar.yaml' => 1234567890, + $this->tempDir.'/gone.yaml' => 1234567891, + $this->tempDir.'/foo.yaml' => 1234567892, + ])); + + $this->assertEqualsCanonicalizing(['foo', 'bar'], $this->store->paths()->keys()->all()); + } + #[Test] public function rebuilding_paths_caches_file_timestamps_for_the_watcher() { diff --git a/tests/Stache/TraverserTest.php b/tests/Stache/TraverserTest.php index 1c3a1dcacb3..bca57674dca 100644 --- a/tests/Stache/TraverserTest.php +++ b/tests/Stache/TraverserTest.php @@ -194,4 +194,28 @@ public function a_traversal_started_by_a_filter_does_not_replace_the_running_tra $this->assertCount(3, StacheFacade::store('entries')->store('blog')->paths()); } + + #[Test] + public function it_skips_a_file_removed_after_it_was_listed() + { + touch($this->tempDir.'/one.txt', 1234567891); + touch($this->tempDir.'/two.txt', 1234567892); + + $store = Mockery::mock(); + $store->shouldReceive('directory')->andReturn($this->tempDir); + + // Removing the file as the filter sees it is the same as a concurrent delete landing + // between the Finder listing it and the traverser reading its modified time. + $this->traverser = $this->traverser->filter(function ($file) { + if ($file->getFilename() === 'one.txt') { + unlink($file->getPathname()); + } + + return true; + }); + + $files = $this->traverser->traverse($store); + + $this->assertSame([Path::tidy($this->tempDir.'/two.txt')], $files->keys()->all()); + } }