From 34dfe2ff816d0c3308e2b6e08313159dcab0bd73 Mon Sep 17 00:00:00 2001 From: edalzell Date: Sun, 25 Oct 2026 16:25:29 -0700 Subject: [PATCH 1/2] Lock the Comb index file across each read-modify-write --- src/Search/Comb/Index.php | 53 +++++++++++++++-------- tests/Search/CombIndexTest.php | 78 ++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 18 deletions(-) diff --git a/src/Search/Comb/Index.php b/src/Search/Comb/Index.php index 9126b35fccb..b436c5a4e0e 100644 --- a/src/Search/Comb/Index.php +++ b/src/Search/Comb/Index.php @@ -2,7 +2,9 @@ namespace Statamic\Search\Comb; +use Closure; use Statamic\Facades\File; +use Statamic\Facades\Stache; use Statamic\Search\Comb\Exceptions\NoQuery; use Statamic\Search\Comb\Exceptions\NoResultsFound; use Statamic\Search\Comb\Exceptions\NotEnoughCharacters; @@ -88,32 +90,36 @@ public function path() public function delete($document) { - try { - $data = $this->data(); - } catch (IndexNotFoundException $e) { - return; - } + $this->locked(function () use ($document) { + try { + $data = $this->data(); + } catch (IndexNotFoundException $e) { + return; + } - $ref = $document->getSearchReference(); + $ref = $document->getSearchReference(); - if (! $data->has($ref)) { - return; - } + if (! $data->has($ref)) { + return; + } - $data->forget($ref); + $data->forget($ref); - $this->save($data); + $this->save($data); + }); } public function insertDocuments(Documents $documents) { - try { - $data = $this->data(); - } catch (IndexNotFoundException $e) { - $data = collect(); - } - - $this->save($documents->union($data)); + $this->locked(function () use ($documents) { + try { + $data = $this->data(); + } catch (IndexNotFoundException $e) { + $data = collect(); + } + + $this->save($documents->union($data)); + }); } public function deleteIndex() @@ -128,6 +134,17 @@ protected function save($documents) app('files')->put($this->path(), $documents->toJson(), lock: true); } + private function locked(Closure $callback) + { + $lock = tap(Stache::lock('search-index-'.$this->name))->acquire(true); + + try { + return $callback(); + } finally { + $lock->release(); + } + } + public function extraAugmentedResultData(Result $result) { return [ diff --git a/tests/Search/CombIndexTest.php b/tests/Search/CombIndexTest.php index fa12fb837ac..b746b065fef 100644 --- a/tests/Search/CombIndexTest.php +++ b/tests/Search/CombIndexTest.php @@ -6,7 +6,10 @@ use Mockery; use PHPUnit\Framework\Attributes\Test; use Statamic\Contracts\Search\Searchable; +use Statamic\Facades\Stache; use Statamic\Search\Comb\Index; +use Statamic\Search\Documents; +use Symfony\Component\Lock\LockInterface; use Tests\TestCase; class CombIndexTest extends TestCase @@ -114,6 +117,81 @@ public function delete_rewrites_the_index_when_the_reference_is_present() ); } + #[Test] + public function inserting_holds_the_index_lock_across_the_read_and_the_write() + { + $path = $this->createIndexFile(['entry::a' => ['title' => 'Foo']]); + $lock = $this->expectIndexLock( + acquired: fn () => $this->assertSame(['entry::a' => ['title' => 'Foo']], json_decode(file_get_contents($path), true)), + released: fn () => $this->assertSame(['entry::b' => ['title' => 'Bar'], 'entry::a' => ['title' => 'Foo']], json_decode(file_get_contents($path), true)), + ); + + $index = $this->getIndex('test', ['path' => $this->tmpDir], null); + + $index->insertDocuments(new Documents(['entry::b' => ['title' => 'Bar']])); + + $lock->shouldHaveReceived('release')->once(); + } + + #[Test] + public function deleting_holds_the_index_lock_across_the_read_and_the_write() + { + $path = $this->createIndexFile(['entry::a' => ['title' => 'Foo'], 'entry::b' => ['title' => 'Bar']]); + $lock = $this->expectIndexLock( + acquired: fn () => $this->assertCount(2, json_decode(file_get_contents($path), true)), + released: fn () => $this->assertSame(['entry::b' => ['title' => 'Bar']], json_decode(file_get_contents($path), true)), + ); + + $index = $this->getIndex('test', ['path' => $this->tmpDir], null); + + $document = Mockery::mock(Searchable::class); + $document->shouldReceive('getSearchReference')->andReturn('entry::a'); + + $index->delete($document); + + $lock->shouldHaveReceived('release')->once(); + } + + #[Test] + public function the_index_lock_is_released_when_the_write_fails() + { + $this->createIndexFile([]); + file_put_contents($this->tmpDir.'/test.json', '{not json'); + $lock = $this->expectIndexLock(); + + $index = $this->getIndex('test', ['path' => $this->tmpDir], null); + + try { + $index->insertDocuments(new Documents(['entry::b' => ['title' => 'Bar']])); + $this->fail('Expected the invalid index file to throw.'); + } catch (\JsonException $e) { + // + } + + $lock->shouldHaveReceived('release')->once(); + } + + private function expectIndexLock(?\Closure $acquired = null, ?\Closure $released = null) + { + $lock = Mockery::spy(LockInterface::class); + $lock->shouldReceive('acquire')->with(true)->once()->andReturnUsing(function () use ($acquired) { + if ($acquired) { + $acquired(); + } + + return true; + }); + $lock->shouldReceive('release')->andReturnUsing(function () use ($released) { + if ($released) { + $released(); + } + }); + + Stache::partialMock()->shouldReceive('lock')->with('search-index-test')->andReturn($lock); + + return $lock; + } + private function createIndexFile(array $data): string { $this->tmpDir = sys_get_temp_dir().'/statamic_comb_test_'.uniqid(); From 2be683998698d3896b8f172ca4aa7096abcc8ea0 Mon Sep 17 00:00:00 2001 From: edalzell Date: Mon, 26 Oct 2026 10:51:47 -0700 Subject: [PATCH 2/2] Use the cache lock for Comb index writes --- src/Search/Comb/Index.php | 10 ++------ tests/Search/CombIndexTest.php | 44 +++++++++------------------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/src/Search/Comb/Index.php b/src/Search/Comb/Index.php index b436c5a4e0e..baeeeb1d4ea 100644 --- a/src/Search/Comb/Index.php +++ b/src/Search/Comb/Index.php @@ -3,8 +3,8 @@ namespace Statamic\Search\Comb; use Closure; +use Illuminate\Support\Facades\Cache; use Statamic\Facades\File; -use Statamic\Facades\Stache; use Statamic\Search\Comb\Exceptions\NoQuery; use Statamic\Search\Comb\Exceptions\NoResultsFound; use Statamic\Search\Comb\Exceptions\NotEnoughCharacters; @@ -136,13 +136,7 @@ protected function save($documents) private function locked(Closure $callback) { - $lock = tap(Stache::lock('search-index-'.$this->name))->acquire(true); - - try { - return $callback(); - } finally { - $lock->release(); - } + return Cache::lock('search-index-'.$this->name, 60)->block(30, $callback); } public function extraAugmentedResultData(Result $result) diff --git a/tests/Search/CombIndexTest.php b/tests/Search/CombIndexTest.php index b746b065fef..e5c23cc8365 100644 --- a/tests/Search/CombIndexTest.php +++ b/tests/Search/CombIndexTest.php @@ -2,14 +2,14 @@ namespace Tests\Search; +use Illuminate\Contracts\Cache\Lock; use Illuminate\Contracts\Filesystem\Filesystem; +use Illuminate\Support\Facades\Cache; use Mockery; use PHPUnit\Framework\Attributes\Test; use Statamic\Contracts\Search\Searchable; -use Statamic\Facades\Stache; use Statamic\Search\Comb\Index; use Statamic\Search\Documents; -use Symfony\Component\Lock\LockInterface; use Tests\TestCase; class CombIndexTest extends TestCase @@ -121,7 +121,7 @@ public function delete_rewrites_the_index_when_the_reference_is_present() public function inserting_holds_the_index_lock_across_the_read_and_the_write() { $path = $this->createIndexFile(['entry::a' => ['title' => 'Foo']]); - $lock = $this->expectIndexLock( + $this->expectIndexLock( acquired: fn () => $this->assertSame(['entry::a' => ['title' => 'Foo']], json_decode(file_get_contents($path), true)), released: fn () => $this->assertSame(['entry::b' => ['title' => 'Bar'], 'entry::a' => ['title' => 'Foo']], json_decode(file_get_contents($path), true)), ); @@ -129,15 +129,13 @@ public function inserting_holds_the_index_lock_across_the_read_and_the_write() $index = $this->getIndex('test', ['path' => $this->tmpDir], null); $index->insertDocuments(new Documents(['entry::b' => ['title' => 'Bar']])); - - $lock->shouldHaveReceived('release')->once(); } #[Test] public function deleting_holds_the_index_lock_across_the_read_and_the_write() { $path = $this->createIndexFile(['entry::a' => ['title' => 'Foo'], 'entry::b' => ['title' => 'Bar']]); - $lock = $this->expectIndexLock( + $this->expectIndexLock( acquired: fn () => $this->assertCount(2, json_decode(file_get_contents($path), true)), released: fn () => $this->assertSame(['entry::b' => ['title' => 'Bar']], json_decode(file_get_contents($path), true)), ); @@ -148,46 +146,26 @@ public function deleting_holds_the_index_lock_across_the_read_and_the_write() $document->shouldReceive('getSearchReference')->andReturn('entry::a'); $index->delete($document); - - $lock->shouldHaveReceived('release')->once(); - } - - #[Test] - public function the_index_lock_is_released_when_the_write_fails() - { - $this->createIndexFile([]); - file_put_contents($this->tmpDir.'/test.json', '{not json'); - $lock = $this->expectIndexLock(); - - $index = $this->getIndex('test', ['path' => $this->tmpDir], null); - - try { - $index->insertDocuments(new Documents(['entry::b' => ['title' => 'Bar']])); - $this->fail('Expected the invalid index file to throw.'); - } catch (\JsonException $e) { - // - } - - $lock->shouldHaveReceived('release')->once(); } private function expectIndexLock(?\Closure $acquired = null, ?\Closure $released = null) { - $lock = Mockery::spy(LockInterface::class); - $lock->shouldReceive('acquire')->with(true)->once()->andReturnUsing(function () use ($acquired) { + $lock = Mockery::mock(Lock::class); + $lock->shouldReceive('block')->withArgs(fn ($seconds, $callback) => $callback instanceof \Closure)->once()->andReturnUsing(function ($seconds, $callback) use ($acquired, $released) { if ($acquired) { $acquired(); } - return true; - }); - $lock->shouldReceive('release')->andReturnUsing(function () use ($released) { + $result = $callback(); + if ($released) { $released(); } + + return $result; }); - Stache::partialMock()->shouldReceive('lock')->with('search-index-test')->andReturn($lock); + Cache::shouldReceive('lock')->with('search-index-test', 60)->once()->andReturn($lock); return $lock; }