Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add dedicated test cases to ensure copy and move methods to always overwrite target #1808

Merged
merged 7 commits into from
Aug 17, 2024
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
Loading