From 7aca6c4a77e377dd1273e5067c21a2c96a8d1806 Mon Sep 17 00:00:00 2001 From: Derrick Austin Date: Fri, 14 Aug 2026 14:12:28 -0500 Subject: [PATCH] Always reset the force-deleting flag after a force delete If delete() throws, forceDelete() left the datasource stuck in force-deleting mode, so every later ordinary delete on that instance ran as a hard delete. The flag now resets in a finally block and the exception propagates. --- src/Halcyon/Datasource/Datasource.php | 12 ++-- tests/Halcyon/DatasourceForceDeleteTest.php | 66 +++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 tests/Halcyon/DatasourceForceDeleteTest.php diff --git a/src/Halcyon/Datasource/Datasource.php b/src/Halcyon/Datasource/Datasource.php index 42bc1640b..b788ca58a 100644 --- a/src/Halcyon/Datasource/Datasource.php +++ b/src/Halcyon/Datasource/Datasource.php @@ -59,11 +59,13 @@ public function forceDelete(string $dirName, string $fileName, string $extension { $this->forceDeleting = true; - $success = $this->delete($dirName, $fileName, $extension); - - $this->forceDeleting = false; - - return $success; + try { + return $this->delete($dirName, $fileName, $extension); + } finally { + // A delete that throws must not leave the datasource in force-deleting mode, + // or every later ordinary delete on this instance becomes a hard delete. + $this->forceDeleting = false; + } } /** diff --git a/tests/Halcyon/DatasourceForceDeleteTest.php b/tests/Halcyon/DatasourceForceDeleteTest.php new file mode 100644 index 000000000..f38f0268e --- /dev/null +++ b/tests/Halcyon/DatasourceForceDeleteTest.php @@ -0,0 +1,66 @@ +getValue($datasource); + } + + public function testForceDeleteResetsTheFlagWhenDeleteThrows() + { + $datasource = new class ('/tmp', new Filesystem) extends FileDatasource + { + public function delete(string $dirName, string $fileName, string $extension): bool + { + throw new RuntimeException('delete failed'); + } + }; + + $caught = null; + + try { + $datasource->forceDelete('pages', 'index', 'htm'); + } + catch (RuntimeException $ex) { + $caught = $ex; + } + + // Asserted outside the catch: PHPUnit's own assertion failures extend RuntimeException, + // so a fail() inside the try would be swallowed by the catch above. + $this->assertInstanceOf(RuntimeException::class, $caught, 'forceDelete() must propagate the failure'); + $this->assertSame('delete failed', $caught->getMessage()); + + $this->assertFalse( + $this->getForceDeleting($datasource), + 'A failed force delete must not leave the datasource in force-deleting mode, or every ' + . 'later ordinary delete on this instance becomes a hard delete.' + ); + } + + public function testForceDeleteReturnsTheDeleteResultAndResetsTheFlag() + { + $datasource = new class ('/tmp', new Filesystem) extends FileDatasource + { + /** + * @var bool The flag value observed while delete() ran. + */ + public $flagDuringDelete = false; + + public function delete(string $dirName, string $fileName, string $extension): bool + { + $this->flagDuringDelete = $this->forceDeleting; + + return true; + } + }; + + $this->assertTrue($datasource->forceDelete('pages', 'index', 'htm')); + $this->assertTrue($datasource->flagDuringDelete, 'delete() should run in force-deleting mode'); + $this->assertFalse($this->getForceDeleting($datasource)); + } +}