From c7f3471221da2c062f536c76cd8c04013598ed3d Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Mon, 28 Sep 2026 12:04:17 +0200 Subject: [PATCH] fix(task) #203 Fix `ProcessState::addErrorContextValue()`: accept any value type, so that iterable tasks on an empty iterable are skipped and PropertySetterTask reports the original exception instead of a `TypeError`. Update documentation, add tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/03-custom_tasks.md | 4 +- docs/reference/tasks/property_setter_task.md | 4 +- docs/reference/tasks/split_join_line_task.md | 3 +- docs/reference/tasks/yaml_reader_task.md | 2 +- src/Model/ProcessState.php | 2 +- src/Task/AbstractIterableOutputTask.php | 3 +- tests/Model/ProcessStateTest.php | 68 ++++++++++ tests/Task/AbstractIterableOutputTaskTest.php | 116 ++++++++++++++++++ tests/Task/PropertySetterTaskTest.php | 85 +++++++++++++ 10 files changed, 278 insertions(+), 10 deletions(-) create mode 100644 tests/Model/ProcessStateTest.php create mode 100644 tests/Task/AbstractIterableOutputTaskTest.php create mode 100644 tests/Task/PropertySetterTaskTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e918fb94..ae84f2ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Latest * [#192](https://github.com/cleverage/process-bundle/issues/192) Fix CommandRunnerTask: only pass the `options` option to `Process::setOptions()`, support string `commandline` through `Process::fromShellCommandline()`, validate option types. Update documentation, add tests. * [#194](https://github.com/cleverage/process-bundle/issues/194) Fix ProcessLauncherTask: the `process_options` normalizer returned an array despite its scalar return type, so the task always failed with a `TypeError`. Update documentation, add tests. * [#189](https://github.com/cleverage/process-bundle/issues/189) Fix EventDispatcherTask: dispatch the event under the `event_name` option (regression since v4.0). `event_name` is now optional: when `null` (default), the event is dispatched under its class name. Update documentation, add tests. +* [#203](https://github.com/cleverage/process-bundle/issues/203) Fix `ProcessState::addErrorContextValue()`: accept any value type, so that iterable tasks on an empty iterable are skipped and PropertySetterTask reports the original exception instead of a `TypeError`. Update documentation, add tests. ## Deprecated * [#189](https://github.com/cleverage/process-bundle/issues/189) EventDispatcherTask: when `event_name` is set, listening to `CleverAge\ProcessBundle\Event\EventDispatcherTaskEvent` is deprecated (the event is still dispatched under its class name, with an `E_USER_DEPRECATED` error, if it has listeners). Listen to the configured `event_name` instead: the BC layer will be removed in v6.0. diff --git a/docs/03-custom_tasks.md b/docs/03-custom_tasks.md index 11b72502..cefbcda4 100644 --- a/docs/03-custom_tasks.md +++ b/docs/03-custom_tasks.md @@ -82,8 +82,8 @@ Sometimes, when you execute a task, you need to change how the process continues `error_strategy` is then applied (throwing an exception from `execute` has the same effect) * `ProcessState::setErrorOutput($value)`: send a value to the error branch of your workflow (the tasks listed in `error_outputs`) -* `ProcessState::addErrorContextValue($key, $value)` / `removeErrorContext($key)`: add information to the log record - written when an error occurs +* `ProcessState::addErrorContextValue($key, $value)` / `removeErrorContext($key)`: add information (any value) to the + log record written when an error occurs ## Options diff --git a/docs/reference/tasks/property_setter_task.md b/docs/reference/tasks/property_setter_task.md index 10d88144..028379b2 100644 --- a/docs/reference/tasks/property_setter_task.md +++ b/docs/reference/tasks/property_setter_task.md @@ -22,9 +22,7 @@ Possible outputs The input, with the configured values set. If a value cannot be set, the exception is set on the state (with `property` and `value` added to the error context) -and handled according to the task `error_strategy`; the remaining values are not set. Note that only `string`, `int` -and `array` values can be added to the error context: for other value types (`bool`, `float`, `null`, objects), a -`\TypeError` is raised instead of the original exception (it is still handled according to `error_strategy`). +and handled according to the task `error_strategy`; the remaining values are not set. Options ------- diff --git a/docs/reference/tasks/split_join_line_task.md b/docs/reference/tasks/split_join_line_task.md index ab5e5d2b..712c62cf 100644 --- a/docs/reference/tasks/split_join_line_task.md +++ b/docs/reference/tasks/split_join_line_task.md @@ -19,7 +19,8 @@ Possible outputs ---------------- `array`: one line per split value, iterated in the order of `split_columns`. Each line contains all the original columns -except the `split_columns`, plus the `join_column` holding the split value (as a string). +except the `split_columns`, plus the `join_column` holding the split value (as a string). If no line is produced (empty +`split_columns`), the task is skipped. Options ------- diff --git a/docs/reference/tasks/yaml_reader_task.md b/docs/reference/tasks/yaml_reader_task.md index cab89566..c08f4e21 100644 --- a/docs/reference/tasks/yaml_reader_task.md +++ b/docs/reference/tasks/yaml_reader_task.md @@ -43,5 +43,5 @@ Notes ----- * The root of the file must be a mapping or a sequence, otherwise an `\InvalidArgumentException` is thrown (e.g. for an - empty file). An empty root mapping or sequence (`{}` or `[]`) raises a `\TypeError`. + empty file). An empty root mapping or sequence (`{}` or `[]`) produces no output: the task is skipped. * The current root key is added to the error context of the process as `iterator_key`. diff --git a/src/Model/ProcessState.php b/src/Model/ProcessState.php index b6ec5c9d..c9a5cef7 100644 --- a/src/Model/ProcessState.php +++ b/src/Model/ProcessState.php @@ -201,7 +201,7 @@ public function setErrorContext(array $errorContext): void $this->errorContext = $errorContext; } - public function addErrorContextValue(string|int $key, string|int|array $value): void + public function addErrorContextValue(string|int $key, mixed $value): void { $this->errorContext[$key] = $value; } diff --git a/src/Task/AbstractIterableOutputTask.php b/src/Task/AbstractIterableOutputTask.php index 8e8ccb7a..e586f062 100644 --- a/src/Task/AbstractIterableOutputTask.php +++ b/src/Task/AbstractIterableOutputTask.php @@ -30,9 +30,8 @@ public function execute(ProcessState $state): void { $this->handleIteratorFromInput($state); - $state->addErrorContextValue('iterator_key', $this->iterator->key()); - if ($this->iterator->valid()) { + $state->addErrorContextValue('iterator_key', $this->iterator->key()); $state->setOutput($this->iterator->current()); } else { $state->setSkipped(true); diff --git a/tests/Model/ProcessStateTest.php b/tests/Model/ProcessStateTest.php new file mode 100644 index 00000000..1453e2f5 --- /dev/null +++ b/tests/Model/ProcessStateTest.php @@ -0,0 +1,68 @@ + + */ + public static function errorContextValueProvider(): iterable + { + yield 'string' => ['foo']; + yield 'int' => [42]; + yield 'array' => [['foo' => 'bar']]; + yield 'bool' => [true]; + yield 'float' => [1.5]; + yield 'null' => [null]; + yield 'object' => [new \stdClass()]; + } + + #[DataProvider('errorContextValueProvider')] + public function testAddErrorContextValueAcceptsAnyValue(mixed $value): void + { + $state = $this->createState(); + + $state->addErrorContextValue('key', $value); + + self::assertSame(['key' => $value], $state->getErrorContext()); + } + + public function testRemoveErrorContext(): void + { + $state = $this->createState(); + $state->addErrorContextValue('kept', 'foo'); + $state->addErrorContextValue(0, null); + + $state->removeErrorContext(0); + + self::assertSame(['kept' => 'foo'], $state->getErrorContext()); + } + + private function createState(): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + + return new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + } +} diff --git a/tests/Task/AbstractIterableOutputTaskTest.php b/tests/Task/AbstractIterableOutputTaskTest.php new file mode 100644 index 00000000..583d217d --- /dev/null +++ b/tests/Task/AbstractIterableOutputTaskTest.php @@ -0,0 +1,116 @@ +createState(['output' => []]); + $task->initialize($state); + + $task->execute($state); + + self::assertTrue($state->isSkipped()); + self::assertNull($state->getException()); + self::assertSame([], $state->getErrorContext()); + self::assertFalse($task->next($state)); + } + + public function testEmptyInputIteratorIsSkipped(): void + { + $task = new InputIteratorTask(); + $state = $this->createState([], []); + $task->initialize($state); + + $task->execute($state); + + self::assertTrue($state->isSkipped()); + self::assertSame([], $state->getErrorContext()); + self::assertFalse($task->next($state)); + } + + public function testIterationSetsIteratorKeyInErrorContext(): void + { + $task = new InputIteratorTask(); + $state = $this->createState([], ['a' => 'foo', 'b' => 'bar']); + $task->initialize($state); + + $outputs = []; + $keys = []; + do { + $state->setSkipped(false); + $task->execute($state); + $outputs[] = $state->getOutput(); + $keys[] = $state->getErrorContext()['iterator_key'] ?? null; + } while ($task->next($state)); + + self::assertSame(['foo', 'bar'], $outputs); + self::assertSame(['a', 'b'], $keys); + self::assertSame([], $state->getErrorContext()); + + // A new input starts a new iteration cycle; an empty one is skipped + $state->setInput([]); + $task->execute($state); + self::assertTrue($state->isSkipped()); + self::assertSame([], $state->getErrorContext()); + } + + public function testSplitJoinLineWithoutSplitColumnIsSkipped(): void + { + $task = new SplitJoinLineTask(); + $state = $this->createState(['split_columns' => [], 'join_column' => 'value'], ['name' => 'Item1']); + $task->initialize($state); + + $task->execute($state); + + self::assertTrue($state->isSkipped()); + self::assertSame([], $state->getErrorContext()); + } + + private function createState(array $options, mixed $input = null): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('iterate', AbstractIterableOutputTask::class, $options)); + $state->setInput($input); + + return $state; + } +} diff --git a/tests/Task/PropertySetterTaskTest.php b/tests/Task/PropertySetterTaskTest.php new file mode 100644 index 00000000..d528d2d7 --- /dev/null +++ b/tests/Task/PropertySetterTaskTest.php @@ -0,0 +1,85 @@ +execute(['[status]' => 'imported', '[enabled]' => true], ['name' => 'Foo']); + + self::assertNull($state->getException()); + self::assertSame(['name' => 'Foo', 'status' => 'imported', 'enabled' => true], $state->getOutput()); + } + + /** + * @return iterable + */ + public static function valueProvider(): iterable + { + yield 'string' => ['foo']; + yield 'int' => [42]; + yield 'array' => [['foo']]; + yield 'bool' => [true]; + yield 'float' => [1.5]; + yield 'null' => [null]; + } + + #[DataProvider('valueProvider')] + public function testFailureKeepsOriginalExceptionWithErrorContext(mixed $value): void + { + // A property path (not an index) cannot be written to an array + $state = $this->execute(['enabled' => $value], ['name' => 'Foo']); + + self::assertInstanceOf(NoSuchPropertyException::class, $state->getException()); + self::assertSame(['property' => 'enabled', 'value' => $value], $state->getErrorContext()); + self::assertNull($state->getOutput()); + } + + private function execute(array $values, mixed $input): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('set', PropertySetterTask::class, ['values' => $values])); + $state->setInput($input); + + $task = new PropertySetterTask(new NullLogger(), PropertyAccess::createPropertyAccessor()); + $task->initialize($state); + $task->execute($state); + + return $state; + } +}