From 5bd44a1ab6dc2fa173994139fb24610ff03f7efb Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Wed, 30 Sep 2026 11:22:10 +0200 Subject: [PATCH] fix(task) #35 FileFetchTask no longer ignores write failures on the destination storage: the error is thrown and, with remove_source, the source file is no longer deleted Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 + docs/reference/tasks/file_fetch_task.md | 2 + src/Task/FileFetchTask.php | 14 ++-- tests/Task/FileFetchTaskTest.php | 89 +++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 tests/Task/FileFetchTaskTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e4c7de..d4b879f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ Latest * [#30](https://github.com/cleverage/flysystem-process-bundle/issues/30) Update quality stack: use Rector `withComposerBased()` sets (removed `SYMFONY_64` / `PHPUNIT_100` sets), declare used Symfony packages and PHPUnit range in composer.json, apply quality tools fixes * [#32](https://github.com/cleverage/flysystem-process-bundle/issues/32) Add missing documentations: harmonize and complete reference pages for every Task (renamed to snake_case), add SFTP import, SFTP export and remote cleanup cookbooks. Harmonize and fix existing documentation. +### Fixes +* [#35](https://github.com/cleverage/flysystem-process-bundle/issues/35) FileFetchTask no longer ignores write failures on the destination storage: the error is thrown (the error strategy applies) and, with `remove_source`, the source file is no longer deleted + v3.0 ------ diff --git a/docs/reference/tasks/file_fetch_task.md b/docs/reference/tasks/file_fetch_task.md index 504d45c..61dfa9f 100644 --- a/docs/reference/tasks/file_fetch_task.md +++ b/docs/reference/tasks/file_fetch_task.md @@ -109,6 +109,8 @@ Notes * The file is written to `destination_filesystem` with the same path, overwriting any existing file. A file given as input that does not exist in `source_filesystem` throws a `League\Flysystem\UnableToReadFile` exception, whatever the value of `ignore_missing`. +* A failure while writing to `destination_filesystem` throws a `League\Flysystem\FilesystemException` (e.g. + `UnableToWriteFile`): the task's `error_strategy` applies, and with `remove_source: true` the source file is kept. * To read a copied file with a core task (e.g. [InputCsvReaderTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/input_csv_reader_task.md)), prefix the output with the directory of the local destination storage (e.g. with the `base_path` option). diff --git a/src/Task/FileFetchTask.php b/src/Task/FileFetchTask.php index d79f55e..ff4d4f6 100644 --- a/src/Task/FileFetchTask.php +++ b/src/Task/FileFetchTask.php @@ -141,22 +141,20 @@ protected function doFileCopy(ProcessState $state, string $filename, bool $remov { $buffer = $this->sourceFS->readStream($filename); + // A write failure is not caught: the task fails (the error strategy applies) and the source is kept try { $this->destinationFS->writeStream($filename, $buffer); - $result = true; - } catch (FilesystemException) { - $result = false; - } - - if (\is_resource($buffer)) { - fclose($buffer); + } finally { + if (\is_resource($buffer)) { + fclose($buffer); + } } if ($removeSource) { $this->sourceFS->delete($filename); } - return $result ? $filename : null; + return $filename; } protected function configureOptions(OptionsResolver $resolver): void diff --git a/tests/Task/FileFetchTaskTest.php b/tests/Task/FileFetchTaskTest.php new file mode 100644 index 0000000..40b9ab2 --- /dev/null +++ b/tests/Task/FileFetchTaskTest.php @@ -0,0 +1,89 @@ +source = $this->createMock(FilesystemOperator::class); + $this->source->method('readStream')->willReturnCallback(static fn () => fopen('php://memory', 'r')); + $this->destination = $this->createMock(FilesystemOperator::class); + } + + public function testCopyWithRemoveSource(): void + { + $this->destination->expects($this->once())->method('writeStream')->with('file.txt'); + $this->source->expects($this->once())->method('delete')->with('file.txt'); + + $state = $this->createState(); + $state->expects($this->once())->method('setOutput')->with('file.txt'); + + $task = $this->createTask(); + $task->initialize($state); + $task->execute($state); + } + + public function testWriteFailureKeepsSourceAndFails(): void + { + $this->destination->expects($this->once())->method('writeStream')->willThrowException(UnableToWriteFile::atLocation('file.txt', 'Is a directory')); + $this->source->expects($this->never())->method('delete'); + + $state = $this->createState(); + $state->expects($this->never())->method('setOutput'); + + $task = $this->createTask(); + $task->initialize($state); + + $this->expectException(UnableToWriteFile::class); + $task->execute($state); + } + + private function createTask(): FileFetchTask + { + /** @var ServiceLocator $storages */ + $storages = new ServiceLocator([ + 'source' => fn (): FilesystemOperator => $this->source, + 'destination' => fn (): FilesystemOperator => $this->destination, + ]); + + return new FileFetchTask($storages); + } + + private function createState(): ProcessState&MockObject + { + $state = $this->createMock(ProcessState::class); + $state->method('getContextualizedOptions')->willReturn([ + 'source_filesystem' => 'source', + 'destination_filesystem' => 'destination', + 'remove_source' => true, + ]); + $state->method('getInput')->willReturn('file.txt'); + + return $state; + } +}