diff --git a/src/Filesystem/Directory.php b/src/Filesystem/Directory.php index 8ce9162..5f95450 100644 --- a/src/Filesystem/Directory.php +++ b/src/Filesystem/Directory.php @@ -38,6 +38,13 @@ public function create(string $path, int $mode = 0777): bool public function remove(string $directory): bool { $directory = realpath($directory); + + // Nothing to remove: the directory does not exist (any more) or is a file. + // Both would make the iterator below throw, which is fatal in a destructor. + if ($directory === false || !is_dir($directory)) { + return false; + } + $iterator = new \RecursiveIteratorIterator( new \RecursiveDirectoryIterator($directory, \FilesystemIterator::SKIP_DOTS), \RecursiveIteratorIterator::CHILD_FIRST diff --git a/tests/Unit/Command/Extension/UploadExtensionVersionCommandTest.php b/tests/Unit/Command/Extension/UploadExtensionVersionCommandTest.php index 1e90327..4b83fbf 100644 --- a/tests/Unit/Command/Extension/UploadExtensionVersionCommandTest.php +++ b/tests/Unit/Command/Extension/UploadExtensionVersionCommandTest.php @@ -110,6 +110,26 @@ public function transactionDirectoryIsRemovedAfterwards(): void self::assertDirectoryDoesNotExist($this->workingDirectory . '/tailor-version-upload'); } + #[Test] + public function transactionDirectoryRemovalSurvivesAnAlreadyRemovedDirectory(): void + { + $command = $this->command(); + $tester = $this->apiTester($command, self::jsonResponse([], 201)); + $tester->execute($this->uploadArguments()); + + // Something removed the transaction directory before the destructor could + $transactionPath = $this->workingDirectory . '/tailor-version-upload'; + foreach ((array)glob($transactionPath . '/*') as $file) { + unlink((string)$file); + } + rmdir($transactionPath); + + unset($command, $tester); + gc_collect_cycles(); + + self::assertDirectoryDoesNotExist($transactionPath); + } + #[Test] public function failingRequestReturnsFailure(): void { diff --git a/tests/Unit/Filesystem/DirectoryTest.php b/tests/Unit/Filesystem/DirectoryTest.php new file mode 100644 index 0000000..a828723 --- /dev/null +++ b/tests/Unit/Filesystem/DirectoryTest.php @@ -0,0 +1,63 @@ +temporaryDirectory = sys_get_temp_dir() . '/tailor-test-' . bin2hex(random_bytes(8)); + mkdir($this->temporaryDirectory, 0777, true); + } + + protected function tearDown(): void + { + if (is_dir($this->temporaryDirectory)) { + (new Directory())->remove($this->temporaryDirectory); + } + } + + #[Test] + public function directoryIsRemovedWithItsContent(): void + { + mkdir($this->temporaryDirectory . '/nested/deeper', 0777, true); + file_put_contents($this->temporaryDirectory . '/artefact.zip', 'content'); + file_put_contents($this->temporaryDirectory . '/nested/deeper/file.txt', 'content'); + + self::assertTrue((new Directory())->remove($this->temporaryDirectory)); + self::assertDirectoryDoesNotExist($this->temporaryDirectory); + } + + #[Test] + public function removingANonExistingDirectoryReturnsFalse(): void + { + self::assertFalse((new Directory())->remove($this->temporaryDirectory . '/never-created')); + } + + #[Test] + public function removingAFileReturnsFalse(): void + { + $file = $this->temporaryDirectory . '/artefact.zip'; + file_put_contents($file, 'content'); + + self::assertFalse((new Directory())->remove($file)); + self::assertFileExists($file); + } +}