From 91b6fd0dc8831cdcaf8c1164e7c83efde01fbf0c Mon Sep 17 00:00:00 2001 From: tinect Date: Fri, 9 Aug 2024 23:24:39 +0200 Subject: [PATCH 1/6] feat: add dedicated test cases to ensure copy and move methods to always overwrite target --- .../FilesystemAdapterTestCase.php | 52 +++++++++++++++++++ src/InMemory/InMemoryFilesystemAdapter.php | 8 +-- .../InMemoryFilesystemAdapterTest.php | 12 ----- src/Local/LocalFilesystemAdapter.php | 2 +- src/WebDAV/WebDAVAdapter.php | 8 +++ src/ZipArchive/ZipArchiveAdapter.php | 7 +++ 6 files changed, 73 insertions(+), 16 deletions(-) diff --git a/src/AdapterTestUtilities/FilesystemAdapterTestCase.php b/src/AdapterTestUtilities/FilesystemAdapterTestCase.php index 69d0530a8..25fc93cea 100644 --- a/src/AdapterTestUtilities/FilesystemAdapterTestCase.php +++ b/src/AdapterTestUtilities/FilesystemAdapterTestCase.php @@ -824,6 +824,58 @@ public function copying_a_file_with_collision(): void }); } + /** + * @test + */ + public function moving_a_file_with_collision(): void + { + $this->runScenario(function () { + $adapter = $this->adapter(); + $adapter->write('path.txt', 'new contents', new Config()); + $adapter->write('new-path.txt', 'contents', new Config()); + + $adapter->move('path.txt', 'new-path.txt', new Config()); + + $oldFileExists = $adapter->fileExists('path.txt'); + $this->assertFalse($oldFileExists); + + $contents = $adapter->read('new-path.txt'); + $this->assertEquals('new contents', $contents); + }); + } + + /** + * @test + */ + public function copying_a_file_with_same_destination(): void + { + $this->runScenario(function () { + $adapter = $this->adapter(); + $adapter->write('path.txt', 'new contents', new Config()); + + $adapter->copy('path.txt', 'path.txt', new Config()); + $contents = $adapter->read('path.txt'); + + $this->assertEquals('new contents', $contents); + }); + } + + /** + * @test + */ + public function moving_a_file_with_same_destination(): void + { + $this->runScenario(function () { + $adapter = $this->adapter(); + $adapter->write('path.txt', 'new contents', new Config()); + + $adapter->move('path.txt', 'path.txt', new Config()); + + $contents = $adapter->read('path.txt'); + $this->assertEquals('new contents', $contents); + }); + } + protected function assertFileExistsAtPath(string $path): void { $this->runScenario(function () use ($path) { diff --git a/src/InMemory/InMemoryFilesystemAdapter.php b/src/InMemory/InMemoryFilesystemAdapter.php index d77435ce6..fb5469eba 100644 --- a/src/InMemory/InMemoryFilesystemAdapter.php +++ b/src/InMemory/InMemoryFilesystemAdapter.php @@ -223,12 +223,14 @@ public function move(string $source, string $destination, Config $config): void $sourcePath = $this->preparePath($source); $destinationPath = $this->preparePath($destination); - if ( ! $this->fileExists($source) || $this->fileExists($destination)) { + if ( ! $this->fileExists($source)) { throw UnableToMoveFile::fromLocationTo($source, $destination); } - $this->files[$destinationPath] = $this->files[$sourcePath]; - unset($this->files[$sourcePath]); + if ($sourcePath !== $destinationPath) { + $this->files[$destinationPath] = $this->files[$sourcePath]; + unset($this->files[$sourcePath]); + } if ($visibility = $config->get(Config::OPTION_VISIBILITY)) { $this->setVisibility($destination, $visibility); diff --git a/src/InMemory/InMemoryFilesystemAdapterTest.php b/src/InMemory/InMemoryFilesystemAdapterTest.php index c3ed1b812..dc3bf61ba 100644 --- a/src/InMemory/InMemoryFilesystemAdapterTest.php +++ b/src/InMemory/InMemoryFilesystemAdapterTest.php @@ -190,18 +190,6 @@ public function moving_a_file_successfully(): void $this->assertTrue($adapter->fileExists('new-path.txt')); } - /** - * @test - */ - public function moving_a_file_with_collision(): void - { - $this->expectException(UnableToMoveFile::class); - $adapter = $this->adapter(); - $adapter->write('path.txt', 'contents', new Config()); - $adapter->write('new-path.txt', 'contents', new Config()); - $adapter->move('path.txt', 'new-path.txt', new Config()); - } - /** * @test */ diff --git a/src/Local/LocalFilesystemAdapter.php b/src/Local/LocalFilesystemAdapter.php index 2b2e095dd..aa7f7d65e 100644 --- a/src/Local/LocalFilesystemAdapter.php +++ b/src/Local/LocalFilesystemAdapter.php @@ -271,7 +271,7 @@ public function copy(string $source, string $destination, Config $config): void $this->resolveDirectoryVisibility($config->get(Config::OPTION_DIRECTORY_VISIBILITY)) ); - if ( ! @copy($sourcePath, $destinationPath)) { + if ($sourcePath !== $destinationPath && ! @copy($sourcePath, $destinationPath)) { throw UnableToCopyFile::because(error_get_last()['message'] ?? 'unknown', $source, $destination); } diff --git a/src/WebDAV/WebDAVAdapter.php b/src/WebDAV/WebDAVAdapter.php index cde5a7c39..935972692 100644 --- a/src/WebDAV/WebDAVAdapter.php +++ b/src/WebDAV/WebDAVAdapter.php @@ -332,6 +332,10 @@ private function normalizeObject(array $object): array public function move(string $source, string $destination, Config $config): void { + if ($source === $destination) { + return; + } + if ($this->manualMove) { $this->manualMove($source, $destination); @@ -369,6 +373,10 @@ private function manualMove(string $source, string $destination): void public function copy(string $source, string $destination, Config $config): void { + if ($source === $destination) { + return; + } + if ($this->manualCopy) { $this->manualCopy($source, $destination); diff --git a/src/ZipArchive/ZipArchiveAdapter.php b/src/ZipArchive/ZipArchiveAdapter.php index 712856aef..63f3411ed 100644 --- a/src/ZipArchive/ZipArchiveAdapter.php +++ b/src/ZipArchive/ZipArchiveAdapter.php @@ -340,6 +340,13 @@ public function move(string $source, string $destination, Config $config): void $archive = $this->zipArchiveProvider->createZipArchive(); if ($archive->locateName($this->pathPrefixer->prefixPath($destination)) !== false) { + if ($source === $destination) { + //update the config of the file + $this->copy($source, $destination, $config); + + return; + } + $this->delete($destination); $this->copy($source, $destination, $config); $this->delete($source); From 827c77d1e0243fe3ee64781fedbe372c2b40c0af Mon Sep 17 00:00:00 2001 From: tinect Date: Fri, 9 Aug 2024 23:43:20 +0200 Subject: [PATCH 2/6] fix: add support to move files onto existing paths for PhpseclibV3 --- src/PhpseclibV3/SftpAdapter.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/PhpseclibV3/SftpAdapter.php b/src/PhpseclibV3/SftpAdapter.php index 2f6012751..96780f7ef 100644 --- a/src/PhpseclibV3/SftpAdapter.php +++ b/src/PhpseclibV3/SftpAdapter.php @@ -323,6 +323,14 @@ public function move(string $source, string $destination, Config $config): void throw UnableToMoveFile::fromLocationTo($source, $destination, $exception); } + if ($sourceLocation === $destinationLocation) { + return; + } + + if ($this->fileExists($destination)) { + $this->delete($destination); + } + if ( ! $connection->rename($sourceLocation, $destinationLocation)) { throw UnableToMoveFile::fromLocationTo($source, $destination); } From 648ae2404187e5ad2f672c832ed2a75f88447cff Mon Sep 17 00:00:00 2001 From: tinect Date: Fri, 9 Aug 2024 23:44:51 +0200 Subject: [PATCH 3/6] fix: add support to move files onto same path for GridFSAdapterTest --- src/GridFS/GridFSAdapter.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/GridFS/GridFSAdapter.php b/src/GridFS/GridFSAdapter.php index cff68b188..4a85f87d6 100644 --- a/src/GridFS/GridFSAdapter.php +++ b/src/GridFS/GridFSAdapter.php @@ -345,6 +345,10 @@ public function listContents(string $path, bool $deep): iterable public function move(string $source, string $destination, Config $config): void { + if ($source === $destination) { + return; + } + try { $result = $this->bucket->getFilesCollection()->updateMany( ['filename' => $this->prefixer->prefixPath($source)], From fefad3420a82c3b0beeb9cedc868f862fe713f09 Mon Sep 17 00:00:00 2001 From: tinect Date: Fri, 9 Aug 2024 23:57:05 +0200 Subject: [PATCH 4/6] fix: decouple tests moving and copying fail from memory filesystem by using dedicated configs --- src/FilesystemTest.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/FilesystemTest.php b/src/FilesystemTest.php index 26768e91e..a25590a7e 100644 --- a/src/FilesystemTest.php +++ b/src/FilesystemTest.php @@ -504,9 +504,10 @@ public function publicUrl(string $path, Config $config): string */ public function copying_from_and_to_the_same_location_fails(): void { - $this->expectExceptionObject(UnableToCopyFile::fromLocationTo('from.txt', 'from.txt')); + $this->expectExceptionObject(UnableToCopyFile::sourceAndDestinationAreTheSame('from.txt', 'from.txt')); - $this->filesystem->copy('from.txt', 'from.txt'); + $config = [Config::OPTION_COPY_IDENTICAL_PATH => ResolveIdenticalPathConflict::FAIL]; + $this->filesystem->copy('from.txt', 'from.txt', $config); } /** @@ -516,7 +517,8 @@ public function moving_from_and_to_the_same_location_fails(): void { $this->expectExceptionObject(UnableToMoveFile::fromLocationTo('from.txt', 'from.txt')); - $this->filesystem->move('from.txt', 'from.txt'); + $config = [Config::OPTION_MOVE_IDENTICAL_PATH => ResolveIdenticalPathConflict::FAIL]; + $this->filesystem->move('from.txt', 'from.txt', $config); } /** From bf1ebd1d3bfa5f7e27d0902ac51b5f4127cf0f4c Mon Sep 17 00:00:00 2001 From: tinect Date: Sat, 10 Aug 2024 00:01:01 +0200 Subject: [PATCH 5/6] fix: remove existing destination file before moving ing GridFS --- src/GridFS/GridFSAdapter.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/GridFS/GridFSAdapter.php b/src/GridFS/GridFSAdapter.php index 4a85f87d6..83a103fac 100644 --- a/src/GridFS/GridFSAdapter.php +++ b/src/GridFS/GridFSAdapter.php @@ -349,6 +349,10 @@ public function move(string $source, string $destination, Config $config): void return; } + if ($this->fileExists($destination)) { + $this->delete($destination); + } + try { $result = $this->bucket->getFilesCollection()->updateMany( ['filename' => $this->prefixer->prefixPath($source)], From 526ce03aa38a3177b86fa3297f555e988dbcdc34 Mon Sep 17 00:00:00 2001 From: tinect Date: Sat, 10 Aug 2024 00:03:31 +0200 Subject: [PATCH 6/6] style: remove unused import in GridFSAdapterTest --- src/GridFS/GridFSAdapterTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/GridFS/GridFSAdapterTest.php b/src/GridFS/GridFSAdapterTest.php index b84e2f9de..cf5a97a0a 100644 --- a/src/GridFS/GridFSAdapterTest.php +++ b/src/GridFS/GridFSAdapterTest.php @@ -15,7 +15,6 @@ use League\Flysystem\UnableToWriteFile; use MongoDB\Client; use MongoDB\Database; -use MongoDB\Driver\ReadPreference; use function getenv; /**