Merge pull request #233 from cleverage/232

#232 - Fix edge cases looping forever or failing with a TypeError

Author: njoubert-cleverage

CHANGELOG.md

@@ -25,6 +25,7 @@ Latest
 * [#221](https://github.com/cleverage/process-bundle/issues/221) Fix CsvSplitterTask: each produced file contains `max_lines` data lines (instead of `max_lines - 2`), no infinite loop with `max_lines` <= 2 (`max_lines` must now be an integer greater than 0), no header-only file at the end. Update documentation, add tests.
 * [#223](https://github.com/cleverage/process-bundle/issues/223) Fix CounterTask: the final count is outputted once, as `flush()` may be called several times. Document that `flush()` implementations must be idempotent. Update documentation, add tests.
 * [#228](https://github.com/cleverage/process-bundle/issues/228) Add the missing `setAllowedTypes()` on HashTransformer (`raw_output`), SimpleBatchTask (`batch_count`), ConditionTrait (`empty`, `not_empty`), NormalizerTask / SerializerTask / DeserializerTask (`context`) and ObjectUpdaterTask (`property_path`): a wrong type is now reported when the options are resolved. Add tests.
+* [#232](https://github.com/cleverage/process-bundle/issues/232) Fix edge cases looping forever or failing with a `TypeError`: validate FileSplitterTask `max_lines` (greater than 0, also when given as input), catch any `\Throwable` in PropertySetterTask, type SlugifyTransformer options, explicit exceptions in InputFolderBrowserTask (no folder path) and `XmlFile::write()` (`saveXML()` failure). 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.

docs/reference/tasks/file_splitter_task.md

@@ -25,10 +25,10 @@ next `max_lines` lines of the source file (the last file may contain fewer lines
 Options
 -------
 
-| Code        | Type     | Required | Default | Description                               |
-|-------------|----------|:--------:|---------|-------------------------------------------|
-| `file_path` | `string` |  **X**   |         | Path of the file to split                 |
-| `max_lines` | `int`    |          | `1000`  | Maximum number of lines per produced file |
+| Code        | Type     | Required | Default | Description                                                       |
+|-------------|----------|:--------:|---------|-------------------------------------------------------------------|
+| `file_path` | `string` |  **X**   |         | Path of the file to split                                         |
+| `max_lines` | `int`    |          | `1000`  | Maximum number of lines per produced file, must be greater than 0 |
 
 Examples
 --------
@@ -48,7 +48,7 @@ read_chunk:
 Notes
 -----
 
-* Values given as input are merged after option resolution, so they are not validated.
+* Values given as input (`file_path`, `max_lines`) are validated like the options; other input keys are ignored.
 * Every line of the source file is kept, in order, including empty lines. Line content is preserved, but each line
   break (`\n` or `\r\n`) is written as `PHP_EOL`, and a missing line break on the last line is added.
 * An empty source file produces no output (the task is skipped).

docs/reference/tasks/input_folder_browser_task.md

@@ -19,7 +19,8 @@ directory.
 
 The folder path is released once its files have all been iterated, so each input (the same path again or a different
 one) is browsed from the start. Receiving a different folder path while an iteration is in progress throws a
-`\LogicException`.
+`\LogicException`, and an empty input (`null`, `''`) when no folder is being browsed throws an
+`\UnexpectedValueException`.
 
 Possible outputs
 ----------------

docs/reference/tasks/property_setter_task.md

@@ -21,7 +21,8 @@ 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)
+If a value cannot be set (including a `\TypeError` of the property accessor, e.g. on a scalar input), 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.
 
 Options

docs/reference/tasks/xml_writer_task.md

@@ -43,3 +43,4 @@ Notes
 -----
 
 * The file is opened on each execution: with the default `wb` mode, each input overwrites the file.
+* If the XML content cannot be generated (`\DOMDocument::saveXML()` fails) or written, a `\RuntimeException` is thrown.

docs/reference/transformers/slugify_transformer.md

@@ -55,6 +55,6 @@ slugify:
 Notes
 -----
 
-An invalid `transliterator` identifier (rejected by `\Transliterator::create()`) raises an
-`InvalidOptionsException` when the options are resolved, i.e. when the transformer is configured, not on the first
-transformed value.
+An invalid `transliterator` identifier (rejected by `\Transliterator::create()`), or an option that is not a string,
+raises an `InvalidOptionsException` when the options are resolved, i.e. when the transformer is configured, not on the
+first transformed value.

src/Filesystem/XmlFile.php

@@ -67,6 +67,9 @@ public function read(): \DOMDocument
     public function write(\DOMDocument $dom): void
     {
         $content = $dom->saveXML();
+        if (false === $content) {
+            throw new \RuntimeException('Could not generate the XML content');
+        }
         $result = $this->file->fwrite($content);
 
         if (false === $result) {

src/Task/File/FileSplitterTask.php

@@ -98,6 +98,7 @@ protected function configureOptions(OptionsResolver $resolver): void
             'max_lines' => 1000,
         ]);
         $resolver->setAllowedTypes('max_lines', ['int']);
+        $resolver->setAllowedValues('max_lines', static fn (int $value): bool => $value >= 1);
     }
 
     /**
@@ -115,7 +116,11 @@ protected function getMergedOptions(ProcessState $state): array
         }
         // @var array<mixed> $input
 
-        return array_merge($options, $input);
+        // Options given as input are validated like the task options
+        $resolver = new OptionsResolver();
+        $this->configureOptions($resolver);
+
+        return $resolver->resolve(array_merge($options, array_intersect_key($input, $options)));
     }
 
     private function stripLineBreak(string $line): string

src/Task/File/InputFolderBrowserTask.php

@@ -73,6 +73,9 @@ protected function getOptions(ProcessState $state): array
             $this->folderPath = $folderPath;
         }
 
+        if (null === $this->folderPath) {
+            throw new \UnexpectedValueException('No folder path given as input');
+        }
         if (!is_dir($this->folderPath)) {
             throw new InvalidConfigurationException("Folder path does not exists or is not a folder: '{$this->folderPath}'");
         }

src/Task/PropertySetterTask.php

@@ -37,7 +37,7 @@ public function execute(ProcessState $state): void
         foreach ($options['values'] as $key => $value) {
             try {
                 $this->accessor->setValue($input, $key, $value);
-            } catch (\Exception $e) {
+            } catch (\Throwable $e) {
                 $state->addErrorContextValue('property', $key);
                 $state->addErrorContextValue('value', $value);
                 $state->setException($e);