From 0b4c776d9be4ab7435d176dadc6534f9274c597c Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Thu, 22 Oct 2026 17:17:17 +0200 Subject: [PATCH 1/2] #75 - FileFetchTask: fail at initialization with an explicit message for an unknown storage, and with an explicit message when the task is executed after a failed initialization instead of an uninitialized property error or the storages of a previous initialization. Update documentation, add tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 +++ docs/reference/tasks/file_fetch_task.md | 4 +++- src/Task/FileFetchTask.php | 32 ++++++++++++++++++++----- tests/Task/FileFetchTaskTest.php | 31 ++++++++++++++++++++++-- 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb54c41..6222801 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ Latest ------ +### Fixes +* [#75](https://github.com/cleverage/flysystem-process-bundle/issues/75) `FileFetchTask` (`flysystem_file_fetch`): an unknown storage fails at initialization with an explicit message (`Unknown storage "…" (option "…")`, with the known storages) instead of `Service "…" not found`, When the task is executed after a failed initialization (process-bundle v5 goes on), it fails with an explicit message instead of `Typed property … must not be accessed before initialization`, and no longer reuses the storages of a previous initialization. Update documentation, add tests. + v3.3 ------ diff --git a/docs/reference/tasks/file_fetch_task.md b/docs/reference/tasks/file_fetch_task.md index e3f2340..6f24c03 100644 --- a/docs/reference/tasks/file_fetch_task.md +++ b/docs/reference/tasks/file_fetch_task.md @@ -98,7 +98,9 @@ Notes ----- * Both storages are resolved when the task is initialized, at the start of the process: an unknown storage name makes - the process fail before any task is executed. + the initialization fail (`Unknown storage "…"`, logged at the `critical` level). With process-bundle v5, the process + goes on and fails when the task is first executed (deprecated, the process will fail before executing any task in + process-bundle v6). * `file_pattern` is only tested on the files located at the root of `source_filesystem` (the listing is not recursive and directories are ignored). The pattern is tested on the path relative to the storage root: to target a sub-directory, configure a dedicated storage whose root is this directory. diff --git a/src/Task/FileFetchTask.php b/src/Task/FileFetchTask.php index 7190e76..b20692d 100644 --- a/src/Task/FileFetchTask.php +++ b/src/Task/FileFetchTask.php @@ -19,6 +19,7 @@ use CleverAge\ProcessBundle\Model\ProcessState; use League\Flysystem\FilesystemException; use League\Flysystem\FilesystemOperator; +use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -50,19 +51,19 @@ public function __construct(protected readonly ServiceLocator $storages) /** * @throws \InvalidArgumentException + * @throws UnexpectedValueException */ #[\Override] public function initialize(ProcessState $state): void { + // The storages of a previous initialization must not be used if this one fails + unset($this->sourceFS, $this->destinationFS); + // Configure options parent::initialize($state); - /** @var string $sourceFilesystemOption */ - $sourceFilesystemOption = $this->getOption($state, 'source_filesystem'); - $this->sourceFS = $this->storages->get($sourceFilesystemOption); - /** @var string $destinationFilesystemOption */ - $destinationFilesystemOption = $this->getOption($state, 'destination_filesystem'); - $this->destinationFS = $this->storages->get($destinationFilesystemOption); + $this->sourceFS = $this->getStorage($state, 'source_filesystem'); + $this->destinationFS = $this->getStorage($state, 'destination_filesystem'); } /** @@ -72,6 +73,11 @@ public function initialize(ProcessState $state): void */ public function execute(ProcessState $state): void { + // With process-bundle v5, a task is executed even if its initialization has failed (the error is logged) + if (!isset($this->sourceFS, $this->destinationFS)) { + throw new UnexpectedValueException('The storages of the task are not available: its initialization has failed, see the previous error'); + } + // The files are listed once per input, when its iteration starts $this->matchingFiles ??= $this->findMatchingFiles($state); @@ -104,6 +110,20 @@ public function next(ProcessState $state): bool return false; } + /** + * @throws UnexpectedValueException + */ + protected function getStorage(ProcessState $state, string $option): FilesystemOperator + { + /** @var string $storage */ + $storage = $this->getOption($state, $option); + try { + return $this->storages->get($storage); + } catch (ServiceNotFoundException $e) { + throw new UnexpectedValueException(\sprintf('Unknown storage "%s" (option "%s"), known storages: "%s"', $storage, $option, implode('", "', array_keys($this->storages->getProvidedServices()))), 0, $e); + } + } + /** * @return list * diff --git a/tests/Task/FileFetchTaskTest.php b/tests/Task/FileFetchTaskTest.php index 86bf533..7c78ba4 100644 --- a/tests/Task/FileFetchTaskTest.php +++ b/tests/Task/FileFetchTaskTest.php @@ -13,6 +13,7 @@ namespace CleverAge\FlysystemProcessBundle\Tests\Task; +use CleverAge\FlysystemProcessBundle\Exception\UnexpectedValueException; use CleverAge\FlysystemProcessBundle\Task\FileFetchTask; use CleverAge\ProcessBundle\Model\ProcessState; use League\Flysystem\FilesystemOperator; @@ -64,6 +65,32 @@ public function testWriteFailureKeepsSourceAndFails(): void $task->execute($state); } + public function testUnknownStorageFailsAtInitialization(): void + { + $task = $this->createTask(); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Unknown storage "unknown" (option "destination_filesystem"), known storages: "source", "destination"'); + $task->initialize($this->createState('unknown')); + } + + public function testTaskFailsExplicitlyWhenExecutedAfterAFailedInitialization(): void + { + $task = $this->createTask(); + $task->initialize($this->createState()); + try { + $task->initialize($this->createState('unknown')); + self::fail('The initialization should have failed'); + } catch (UnexpectedValueException) { + } + + // The storages of the previous initialization are not used + $this->destination->expects($this->never())->method('writeStream'); + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('The storages of the task are not available: its initialization has failed, see the previous error'); + $task->execute($this->createState()); + } + private function createTask(): FileFetchTask { /** @var ServiceLocator $storages */ @@ -75,12 +102,12 @@ private function createTask(): FileFetchTask return new FileFetchTask($storages); } - private function createState(): ProcessState&MockObject + private function createState(string $destination = 'destination'): ProcessState&MockObject { $state = $this->createMock(ProcessState::class); $state->method('getContextualizedOptions')->willReturn([ 'source_filesystem' => 'source', - 'destination_filesystem' => 'destination', + 'destination_filesystem' => $destination, 'remove_source' => true, ]); $state->method('getInput')->willReturn('file.txt'); From 44da2a4bbb2908f4dcee98ad846978f069c79448 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Thu, 22 Oct 2026 17:19:44 +0200 Subject: [PATCH 2/2] #75 - Fix tests Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 +- src/Task/FileFetchTask.php | 3 --- tests/Task/FileFetchTaskStorageTest.php | 4 +++- tests/Task/FileFetchTaskTest.php | 7 +++---- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6222801..6d63a7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Latest ------ ### Fixes -* [#75](https://github.com/cleverage/flysystem-process-bundle/issues/75) `FileFetchTask` (`flysystem_file_fetch`): an unknown storage fails at initialization with an explicit message (`Unknown storage "…" (option "…")`, with the known storages) instead of `Service "…" not found`, When the task is executed after a failed initialization (process-bundle v5 goes on), it fails with an explicit message instead of `Typed property … must not be accessed before initialization`, and no longer reuses the storages of a previous initialization. Update documentation, add tests. +* [#75](https://github.com/cleverage/flysystem-process-bundle/issues/75) `FileFetchTask` (`flysystem_file_fetch`): an unknown storage fails at initialization with an explicit message (`Unknown storage "…" (option "…")`, with the known storages) instead of `Service "…" not found`, When the task is executed after a failed initialization (process-bundle v5 goes on), it fails with an explicit message instead of `Typed property … must not be accessed before initialization`. Update documentation, add tests. v3.3 ------ diff --git a/src/Task/FileFetchTask.php b/src/Task/FileFetchTask.php index b20692d..a198f47 100644 --- a/src/Task/FileFetchTask.php +++ b/src/Task/FileFetchTask.php @@ -56,9 +56,6 @@ public function __construct(protected readonly ServiceLocator $storages) #[\Override] public function initialize(ProcessState $state): void { - // The storages of a previous initialization must not be used if this one fails - unset($this->sourceFS, $this->destinationFS); - // Configure options parent::initialize($state); diff --git a/tests/Task/FileFetchTaskStorageTest.php b/tests/Task/FileFetchTaskStorageTest.php index 316ae59..7349352 100644 --- a/tests/Task/FileFetchTaskStorageTest.php +++ b/tests/Task/FileFetchTaskStorageTest.php @@ -13,6 +13,7 @@ namespace CleverAge\FlysystemProcessBundle\Tests\Task; +use CleverAge\FlysystemProcessBundle\Exception\UnexpectedValueException; use CleverAge\FlysystemProcessBundle\Task\FileFetchTask; use CleverAge\ProcessBundle\Model\ProcessState; use PHPUnit\Framework\Attributes\CoversClass; @@ -143,7 +144,8 @@ public function testNextBeforeExecute(): void public function testUnknownStorage(): void { - $this->expectException(\Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException::class); + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Unknown storage "unknown" (option "source_filesystem")'); $this->createTask(['source_filesystem' => 'unknown']); } diff --git a/tests/Task/FileFetchTaskTest.php b/tests/Task/FileFetchTaskTest.php index 7c78ba4..0bd010f 100644 --- a/tests/Task/FileFetchTaskTest.php +++ b/tests/Task/FileFetchTaskTest.php @@ -76,19 +76,18 @@ public function testUnknownStorageFailsAtInitialization(): void public function testTaskFailsExplicitlyWhenExecutedAfterAFailedInitialization(): void { + $state = $this->createState('unknown'); $task = $this->createTask(); - $task->initialize($this->createState()); try { - $task->initialize($this->createState('unknown')); + $task->initialize($state); self::fail('The initialization should have failed'); } catch (UnexpectedValueException) { } - // The storages of the previous initialization are not used $this->destination->expects($this->never())->method('writeStream'); $this->expectException(UnexpectedValueException::class); $this->expectExceptionMessage('The storages of the task are not available: its initialization has failed, see the previous error'); - $task->execute($this->createState()); + $task->execute($state); } private function createTask(): FileFetchTask