From c85b964a9a288fbb38e98506e0f18f61dfc8f667 Mon Sep 17 00:00:00 2001 From: Alexis Thinardon Date: Fri, 14 Dec 2018 15:53:51 +0100 Subject: [PATCH 1/2] Fix directory deletion --- spec/Gaufrette/Adapter/LocalSpec.php | 5 ++ src/Gaufrette/Adapter/Local.php | 56 +++++++++++-- .../Functional/Adapter/LocalTest.php | 83 +++++++++---------- .../Functional/Adapter/SafeLocalTest.php | 7 +- .../Functional/FileStream/LocalTest.php | 11 ++- .../Functional/LocalDirectoryDeletor.php | 31 ------- 6 files changed, 109 insertions(+), 84 deletions(-) delete mode 100644 tests/Gaufrette/Functional/LocalDirectoryDeletor.php diff --git a/spec/Gaufrette/Adapter/LocalSpec.php b/spec/Gaufrette/Adapter/LocalSpec.php index cf68ea5e8..7f6a34c20 100644 --- a/spec/Gaufrette/Adapter/LocalSpec.php +++ b/spec/Gaufrette/Adapter/LocalSpec.php @@ -79,6 +79,11 @@ function it_deletes_file() $this->delete('filename1')->shouldReturn(false); } + function it_deletes_dir() + { + $this->delete('dir')->shouldReturn(true); + } + function it_checks_if_given_key_is_directory() { $this->isDirectory('dir')->shouldReturn(true); diff --git a/src/Gaufrette/Adapter/Local.php b/src/Gaufrette/Adapter/Local.php index b25a725b7..bab1f4dc1 100644 --- a/src/Gaufrette/Adapter/Local.php +++ b/src/Gaufrette/Adapter/Local.php @@ -53,6 +53,10 @@ public function __construct($directory, $create = false, $mode = 0777) */ public function read($key) { + if ($this->isDirectory($key)) { + return false; + } + return file_get_contents($this->computePath($key)); } @@ -141,17 +145,18 @@ public function mtime($key) /** * {@inheritdoc} * - * @throws \OutOfBoundsException If the computed path is out of the directory - * @throws \InvalidArgumentException if the directory already exists - * @throws \RuntimeException if the directory could not be created + * Can also delete a directory recursively when the given $key matches a + * directory. */ public function delete($key) { if ($this->isDirectory($key)) { - return rmdir($this->computePath($key)); + return $this->deleteDirectory($this->computePath($key)); + } elseif ($this->exists($key)) { + return unlink($this->computePath($key)); } - return unlink($this->computePath($key)); + return false; } /** @@ -309,4 +314,45 @@ protected function createDirectory($directory) throw new \RuntimeException(sprintf('The directory \'%s\' could not be created.', $directory)); } } + + /** + * @param string The directory's path to delete + * + * @throws \InvalidArgumentException When attempting to delete the root + * directory of this adapter. + * + * @return bool Wheter the operation succeeded or not + */ + private function deleteDirectory($directory) + { + if ($this->directory === $directory) { + throw new \InvalidArgumentException( + sprintf('Impossible to delete the root directory of this Local adapter ("%s").', $directory) + ); + } + + $status = true; + + if (file_exists($directory)) { + $iterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator( + $directory, + \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::UNIX_PATHS + ), + \RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ($iterator as $item) { + if ($item->isDir()) { + $status = $status && rmdir(strval($item)); + } else { + $status = $status && unlink(strval($item)); + } + } + + $status = $status && rmdir($directory); + } + + return $status; + } } diff --git a/tests/Gaufrette/Functional/Adapter/LocalTest.php b/tests/Gaufrette/Functional/Adapter/LocalTest.php index 7dbd9d27b..4958fdefb 100644 --- a/tests/Gaufrette/Functional/Adapter/LocalTest.php +++ b/tests/Gaufrette/Functional/Adapter/LocalTest.php @@ -4,7 +4,6 @@ use Gaufrette\Filesystem; use Gaufrette\Adapter\Local; -use Gaufrette\Functional\LocalDirectoryDeletor; class LocalTest extends FunctionalTestCase { @@ -23,9 +22,15 @@ public function setUp() public function tearDown() { + $adapter = $this->filesystem->getAdapter(); + + foreach ($this->filesystem->keys() as $key) { + $adapter->delete($key); + } + $this->filesystem = null; - LocalDirectoryDeletor::deleteDirectory($this->directory); + rmdir($this->directory); } /** @@ -51,11 +56,12 @@ public function shouldWorkWithSyslink() @unlink($linkname); symlink($dirname, $linkname); - $this->filesystem = new Filesystem(new Local($linkname)); - $this->filesystem->write('test.txt', 'abc 123'); + $fs = new Filesystem(new Local($linkname)); + $fs->write('test.txt', 'abc 123'); + + $this->assertSame('abc 123', $fs->read('test.txt')); + $fs->delete('test.txt'); - $this->assertSame('abc 123', $this->filesystem->read('test.txt')); - $this->filesystem->delete('test.txt'); @unlink($linkname); @rmdir($dirname); } @@ -67,13 +73,6 @@ public function shouldWorkWithSyslink() */ public function shouldListingOnlyGivenDirectory() { - $dirname = sprintf( - '%s/localDir', - $this->directory - ); - @mkdir($dirname); - - $this->filesystem = new Filesystem(new Local($this->directory)); $this->filesystem->write('aaa.txt', 'some content'); $this->filesystem->write('localDir/test.txt', 'some content'); @@ -89,10 +88,6 @@ public function shouldListingOnlyGivenDirectory() $this->assertCount(2, $dirs['keys']); $this->assertEquals('aaa.txt', $dirs['keys'][0]); $this->assertEquals('localDir/test.txt', $dirs['keys'][1]); - - @unlink($dirname.DIRECTORY_SEPARATOR.'test.txt'); - @unlink($this->directory.DIRECTORY_SEPARATOR.'aaa.txt'); - @rmdir($dirname); } /** @@ -102,29 +97,15 @@ public function shouldListingOnlyGivenDirectory() */ public function shouldListingAllKeys() { - $dirname = sprintf( - '%s/localDir', - $this->directory - ); - @mkdir($dirname); - - $this->filesystem = new Filesystem(new Local($this->directory)); $this->filesystem->write('aaa.txt', 'some content'); $this->filesystem->write('localDir/dir1/dir2/dir3/test.txt', 'some content'); $keys = $this->filesystem->keys(); $dirs = $this->filesystem->listKeys(); + $this->assertCount(6, $keys); $this->assertCount(4, $dirs['dirs']); $this->assertEquals('localDir/dir1/dir2/dir3/test.txt', $dirs['keys'][1]); - - foreach ($dirs['keys'] as $item) { - @unlink($item); - } - $reversed = array_reverse($dirs['dirs']); - foreach ($reversed as $item) { - @rmdir($item); - } } /** @@ -133,15 +114,6 @@ public function shouldListingAllKeys() */ public function shouldBeAbleToClearCache() { - $dirname = sprintf( - '%s/adapters/bbb', - dirname(__DIR__) - ); - - @mkdir($dirname); - - $this->filesystem = new Filesystem(new Local($dirname)); - $this->filesystem->get('test.txt', true); $this->filesystem->write('test.txt', '123', true); @@ -161,9 +133,32 @@ public function shouldBeAbleToClearCache() $fsRegister = $fsReflection->getProperty('fileRegister'); $fsRegister->setAccessible(true); $this->assertCount(0, $fsRegister->getValue($this->filesystem)); + } - $this->filesystem->delete('test.txt'); - $this->filesystem->delete('test2.txt'); - @rmdir($dirname); + /** + * @test + * @group functional + */ + public function shouldDeleteDirectory() + { + $path = $this->directory . DIRECTORY_SEPARATOR . 'delete-me.d'; + mkdir($path); + + $this->assertTrue(is_dir($path)); + + $this->assertTrue($this->filesystem->getAdapter()->delete('delete-me.d')); + + $this->assertFalse(is_dir($path)); + } + + /** + * @test + * @group functional + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Impossible to delete the root directory of this Local adapter + */ + public function shouldNotDeleteTheAdapterRootDirectory() + { + $this->filesystem->getAdapter()->delete('/'); } } diff --git a/tests/Gaufrette/Functional/Adapter/SafeLocalTest.php b/tests/Gaufrette/Functional/Adapter/SafeLocalTest.php index 257cabce5..efe299e8e 100644 --- a/tests/Gaufrette/Functional/Adapter/SafeLocalTest.php +++ b/tests/Gaufrette/Functional/Adapter/SafeLocalTest.php @@ -4,7 +4,6 @@ use Gaufrette\Filesystem; use Gaufrette\Adapter\SafeLocal; -use Gaufrette\Functional\LocalDirectoryDeletor; class SafeLocalTest extends FunctionalTestCase { @@ -19,9 +18,13 @@ public function setUp() public function tearDown() { + foreach ($this->filesystem->keys() as $key) { + $this->filesystem->delete($key); + } + $this->filesystem = null; - LocalDirectoryDeletor::deleteDirectory($this->getDirectory()); + rmdir($this->getDirectory()); } private function getDirectory() diff --git a/tests/Gaufrette/Functional/FileStream/LocalTest.php b/tests/Gaufrette/Functional/FileStream/LocalTest.php index 4837bedc7..c05c7fb0d 100644 --- a/tests/Gaufrette/Functional/FileStream/LocalTest.php +++ b/tests/Gaufrette/Functional/FileStream/LocalTest.php @@ -4,7 +4,6 @@ use Gaufrette\Filesystem; use Gaufrette\Adapter\Local as LocalAdapter; -use Gaufrette\Functional\LocalDirectoryDeletor; class LocalTest extends FunctionalTestCase { @@ -35,7 +34,15 @@ public function testDirectoryChmod() public function tearDown() { - LocalDirectoryDeletor::deleteDirectory($this->directory); + $adapter = $this->filesystem->getAdapter(); + + foreach ($this->filesystem->keys() as $key) { + $adapter->delete($key); + } + + $this->filesystem = null; + + rmdir($this->directory); } /** diff --git a/tests/Gaufrette/Functional/LocalDirectoryDeletor.php b/tests/Gaufrette/Functional/LocalDirectoryDeletor.php deleted file mode 100644 index 5974bbe71..000000000 --- a/tests/Gaufrette/Functional/LocalDirectoryDeletor.php +++ /dev/null @@ -1,31 +0,0 @@ -isDir()) { - rmdir(strval($item)); - } else { - unlink(strval($item)); - } - } - } - } -} From 8b23f732055411885520017e7688042ef368bb1b Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Wed, 5 Jun 2019 09:40:10 +0200 Subject: [PATCH 2/2] add doc about Local adapter directory deletion feature --- doc/adapters/local.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/adapters/local.md b/doc/adapters/local.md index af742e17d..820eeb292 100644 --- a/doc/adapters/local.md +++ b/doc/adapters/local.md @@ -22,3 +22,17 @@ use Gaufrette\Adapter\Local as LocalAdapter; $adapter = new LocalAdapter('/var/media', true, 0750); $filesystem = new Filesystem($adapter); ``` + +## Delete a directory + +Directory deletion is a feature only available on the `Local` adapter. It is +not supported by the `FilesysteInterface` which aims to only deal with objects. + +Following the above statement, you have to explicitely retrieve the adapter +in order to delete a directory : + +```php +$filesystem->getAdapter()->delete($dirKey); +``` + +Note that you can't delete the root directory of the Local adapter.