fix(task) #35 FileFetchTask no longer ignores write failures

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
 ------
 

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).

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

tests/Task/FileFetchTaskTest.php

@@ -0,0 +1,89 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/FlysystemProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\FlysystemProcessBundle\Tests\Task;
+
+use CleverAge\FlysystemProcessBundle\Task\FileFetchTask;
+use CleverAge\ProcessBundle\Model\ProcessState;
+use League\Flysystem\FilesystemOperator;
+use League\Flysystem\UnableToWriteFile;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\ServiceLocator;
+
+#[CoversClass(FileFetchTask::class)]
+class FileFetchTaskTest extends TestCase
+{
+    private FilesystemOperator&MockObject $source;
+    private FilesystemOperator&MockObject $destination;
+
+    protected function setUp(): void
+    {
+        $this->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<FilesystemOperator> $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;
+    }
+}