Skip to content

Commit

Permalink
Merge pull request #1808 from tinect/feat/testMoveAndCopyTasks
Browse files Browse the repository at this point in the history
feat: add dedicated test cases to ensure copy and move methods to always overwrite target
  • Loading branch information
frankdejonge authored Aug 17, 2024
2 parents 0ee937a + f650a7b commit 2c8083f
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 20 deletions.
52 changes: 52 additions & 0 deletions src/AdapterTestUtilities/FilesystemAdapterTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions src/FilesystemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/GridFS/GridFSAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,14 @@ public function listContents(string $path, bool $deep): iterable

public function move(string $source, string $destination, Config $config): void
{
if ($source === $destination) {
return;
}

if ($this->fileExists($destination)) {
$this->delete($destination);
}

try {
$result = $this->bucket->getFilesCollection()->updateMany(
['filename' => $this->prefixer->prefixPath($source)],
Expand Down
1 change: 0 additions & 1 deletion src/GridFS/GridFSAdapterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use League\Flysystem\UnableToWriteFile;
use MongoDB\Client;
use MongoDB\Database;
use MongoDB\Driver\ReadPreference;
use function getenv;

/**
Expand Down
8 changes: 5 additions & 3 deletions src/InMemory/InMemoryFilesystemAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 0 additions & 12 deletions src/InMemory/InMemoryFilesystemAdapterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Local/LocalFilesystemAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
4 changes: 4 additions & 0 deletions src/PhpseclibV3/SftpAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ public function move(string $source, string $destination, Config $config): void
throw UnableToMoveFile::fromLocationTo($source, $destination, $exception);
}

if ($sourceLocation === $destinationLocation) {
return;
}

if ($connection->rename($sourceLocation, $destinationLocation)) {
return;
}
Expand Down
8 changes: 8 additions & 0 deletions src/WebDAV/WebDAVAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
7 changes: 7 additions & 0 deletions src/ZipArchive/ZipArchiveAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 2c8083f

Please sign in to comment.