From ce1fee96d78bcae5bf725215cf470548d2b7036c Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Mon, 28 Sep 2026 12:04:18 +0200 Subject: [PATCH] fix(task) #208 Fix FolderBrowserTask, InputFolderBrowserTask, CsvReaderTask and LineReaderTask (and their `Input*` variants): reset the state at the end of the iteration, so that a following input is read from its beginning. Update documentation, add tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/reference/tasks/csv_reader_task.md | 2 + docs/reference/tasks/folder_browser_task.md | 2 + docs/reference/tasks/input_csv_reader_task.md | 3 +- .../tasks/input_folder_browser_task.md | 4 +- .../reference/tasks/input_line_reader_task.md | 3 +- docs/reference/tasks/line_reader_task.md | 2 + src/Task/File/Csv/CsvReaderTask.php | 9 +- src/Task/File/FolderBrowserTask.php | 9 +- src/Task/File/InputFolderBrowserTask.php | 12 ++ src/Task/File/LineReaderTask.php | 8 +- tests/Task/File/Csv/CsvReaderTaskTest.php | 130 ++++++++++++++++++ tests/Task/File/FolderBrowserTaskTest.php | 130 ++++++++++++++++++ tests/Task/File/LineReaderTaskTest.php | 113 +++++++++++++++ 14 files changed, 422 insertions(+), 6 deletions(-) create mode 100644 tests/Task/File/Csv/CsvReaderTaskTest.php create mode 100644 tests/Task/File/FolderBrowserTaskTest.php create mode 100644 tests/Task/File/LineReaderTaskTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e918fb94..55120064 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. +* [#208](https://github.com/cleverage/process-bundle/issues/208) Fix FolderBrowserTask, InputFolderBrowserTask, CsvReaderTask and LineReaderTask (and their `Input*` variants): reset the state at the end of the iteration, so that a following input is read from its beginning. 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/reference/tasks/csv_reader_task.md b/docs/reference/tasks/csv_reader_task.md index 14980a6c..b1872987 100644 --- a/docs/reference/tasks/csv_reader_task.md +++ b/docs/reference/tasks/csv_reader_task.md @@ -72,4 +72,6 @@ Notes * Each line must contain exactly as many columns as there are headers, otherwise an `\UnexpectedValueException` is thrown. * A UTF-8 BOM is removed from the first header when headers are read from the file. * `csv_file` and `csv_line` are added to the error context of the process. +* The file is closed once fully read: if the task is executed again (e.g. for a new input), the file is read again + from its beginning. * See also [InputCsvReaderTask](input_csv_reader_task.md) to read a file path given as input. diff --git a/docs/reference/tasks/folder_browser_task.md b/docs/reference/tasks/folder_browser_task.md index 9d318139..106299f9 100644 --- a/docs/reference/tasks/folder_browser_task.md +++ b/docs/reference/tasks/folder_browser_task.md @@ -53,4 +53,6 @@ Notes ----- * `current_file_path` is added to the error context of the process. +* The iteration is reset once all files have been output: if the task is executed again (e.g. for a new input), the + folder is browsed again from the start. * See also [InputFolderBrowserTask](input_folder_browser_task.md) to browse a folder path given as input. diff --git a/docs/reference/tasks/input_csv_reader_task.md b/docs/reference/tasks/input_csv_reader_task.md index f03a8781..67a636e8 100644 --- a/docs/reference/tasks/input_csv_reader_task.md +++ b/docs/reference/tasks/input_csv_reader_task.md @@ -14,7 +14,8 @@ Accepted inputs --------------- `string`: path of the file to read, prefixed by the `base_path` option if set. -When a different path is received, the previous file is dropped and the new one is opened. +The file is closed once fully read, so each input (the same path again or a different one) is read from its +beginning. Possible outputs ---------------- diff --git a/docs/reference/tasks/input_folder_browser_task.md b/docs/reference/tasks/input_folder_browser_task.md index b182f2ef..ac146c93 100644 --- a/docs/reference/tasks/input_folder_browser_task.md +++ b/docs/reference/tasks/input_folder_browser_task.md @@ -17,7 +17,9 @@ Accepted inputs `string`: path of the folder to browse, prefixed by the `base_folder_path` option. It must be an existing readable directory. -Receiving a different folder path before the task has been flushed throws a `\LogicException`. +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`. Possible outputs ---------------- diff --git a/docs/reference/tasks/input_line_reader_task.md b/docs/reference/tasks/input_line_reader_task.md index 97de9d46..a3605beb 100644 --- a/docs/reference/tasks/input_line_reader_task.md +++ b/docs/reference/tasks/input_line_reader_task.md @@ -14,7 +14,8 @@ Accepted inputs --------------- `string`: path of the file to read. An `\UnexpectedValueException` is thrown if the file does not exist or is not -readable. When a different path is received, the previous file is dropped and the new one is opened. +readable. The file is released once fully read, so each input (the same path again or a different one) is read from +its beginning. Possible outputs ---------------- diff --git a/docs/reference/tasks/line_reader_task.md b/docs/reference/tasks/line_reader_task.md index 09c63472..702690b8 100644 --- a/docs/reference/tasks/line_reader_task.md +++ b/docs/reference/tasks/line_reader_task.md @@ -51,4 +51,6 @@ trim: Notes ----- +* The file is released once fully read: if the task is executed again (e.g. for a new input), the file is read again + from its beginning. * See also [InputLineReaderTask](input_line_reader_task.md) to read a file path given as input. diff --git a/src/Task/File/Csv/CsvReaderTask.php b/src/Task/File/Csv/CsvReaderTask.php index 6168388e..f21d593c 100644 --- a/src/Task/File/Csv/CsvReaderTask.php +++ b/src/Task/File/Csv/CsvReaderTask.php @@ -74,7 +74,14 @@ public function next(ProcessState $state): bool $state->removeErrorContext('csv_file'); $state->removeErrorContext('csv_line'); - return !$this->csv->isEndOfFile(); + $endOfFile = $this->csv->isEndOfFile(); + if ($endOfFile) { + // Release the file to allow the following iteration to read it again + $this->csv->close(); + $this->csv = null; + } + + return !$endOfFile; } protected function getHeaders(ProcessState $state, array $options): ?array diff --git a/src/Task/File/FolderBrowserTask.php b/src/Task/File/FolderBrowserTask.php index 6668c3eb..2148fe47 100644 --- a/src/Task/File/FolderBrowserTask.php +++ b/src/Task/File/FolderBrowserTask.php @@ -80,7 +80,14 @@ public function next(ProcessState $state): bool $this->files->next(); $state->removeErrorContext('current_file_path'); - return $this->files->valid(); + if (!$this->files->valid()) { + // Reset the iterator to allow the following iteration + $this->files = null; + + return false; + } + + return true; } protected function configureOptions(OptionsResolver $resolver): void diff --git a/src/Task/File/InputFolderBrowserTask.php b/src/Task/File/InputFolderBrowserTask.php index a0151ae1..1283529a 100644 --- a/src/Task/File/InputFolderBrowserTask.php +++ b/src/Task/File/InputFolderBrowserTask.php @@ -31,6 +31,18 @@ public function flush(ProcessState $state): void $state->setSkipped(true); } + #[\Override] + public function next(ProcessState $state): bool + { + $hasNext = parent::next($state); + if (!$hasNext) { + // Release the folder path to allow the following input to browse another folder + $this->folderPath = null; + } + + return $hasNext; + } + #[\Override] public function initialize(ProcessState $state): void { diff --git a/src/Task/File/LineReaderTask.php b/src/Task/File/LineReaderTask.php index da103ab0..3e7b2a29 100644 --- a/src/Task/File/LineReaderTask.php +++ b/src/Task/File/LineReaderTask.php @@ -59,7 +59,13 @@ public function next(ProcessState $state): bool throw new \LogicException('No File initialized'); } - return !$this->file->eof(); + $endOfFile = $this->file->eof(); + if ($endOfFile) { + // Release the file to allow the following iteration to read it again + $this->file = null; + } + + return !$endOfFile; } protected function configureOptions(OptionsResolver $resolver): void diff --git a/tests/Task/File/Csv/CsvReaderTaskTest.php b/tests/Task/File/Csv/CsvReaderTaskTest.php new file mode 100644 index 00000000..b1bb5a42 --- /dev/null +++ b/tests/Task/File/Csv/CsvReaderTaskTest.php @@ -0,0 +1,130 @@ +tmpDir = sys_get_temp_dir().\DIRECTORY_SEPARATOR.uniqid('csv_reader_test_', true); + $filesystem = new Filesystem(); + $filesystem->dumpFile($this->tmpDir.'/a.csv', "id;name\n1;foo\n2;bar\n"); + $filesystem->dumpFile($this->tmpDir.'/b.csv', "id;name\n3;baz\n"); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->tmpDir); + } + + public function testSameFileIsReadAgainOnSecondExecution(): void + { + $task = new CsvReaderTask(new NullLogger()); + $state = $this->createState(['file_path' => $this->tmpDir.'/a.csv']); + + $expected = [['id' => '1', 'name' => 'foo'], ['id' => '2', 'name' => 'bar']]; + self::assertSame( + [$expected, $expected], + [$this->iterate($task, $state), $this->iterate($task, $state)], + ); + $task->finalize($state); + } + + public function testInputCsvReaderReadsSameFileTwice(): void + { + $task = new InputCsvReaderTask(new NullLogger()); + $state = $this->createState([]); + + $expected = [['id' => '1', 'name' => 'foo'], ['id' => '2', 'name' => 'bar']]; + self::assertSame( + [$expected, $expected], + [$this->iterate($task, $state, $this->tmpDir.'/a.csv'), $this->iterate($task, $state, $this->tmpDir.'/a.csv')], + ); + $task->finalize($state); + } + + public function testInputCsvReaderReadsDifferentFilesSuccessively(): void + { + $task = new InputCsvReaderTask(new NullLogger()); + $state = $this->createState([]); + + self::assertSame( + [['id' => '1', 'name' => 'foo'], ['id' => '2', 'name' => 'bar']], + $this->iterate($task, $state, $this->tmpDir.'/a.csv'), + ); + self::assertSame([['id' => '3', 'name' => 'baz']], $this->iterate($task, $state, $this->tmpDir.'/b.csv')); + $task->finalize($state); + } + + /** + * Mimics the ProcessManager loop over an iterable task and returns the non-skipped outputs. + * + * @return list + */ + private function iterate(CsvReaderTask $task, ProcessState $state, mixed $input = null): array + { + $outputs = []; + $state->setInput($input); + do { + $state->setSkipped(false); + $task->execute($state); + if (!$state->isSkipped()) { + $outputs[] = $state->getOutput(); + } + } while ($task->next($state)); + + return $outputs; + } + + private function createState(array $options): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('read', CsvReaderTask::class, $options)); + + return $state; + } +} diff --git a/tests/Task/File/FolderBrowserTaskTest.php b/tests/Task/File/FolderBrowserTaskTest.php new file mode 100644 index 00000000..caa40369 --- /dev/null +++ b/tests/Task/File/FolderBrowserTaskTest.php @@ -0,0 +1,130 @@ +tmpDir = sys_get_temp_dir().\DIRECTORY_SEPARATOR.uniqid('folder_browser_test_', true); + $filesystem = new Filesystem(); + $filesystem->dumpFile($this->tmpDir.'/dirA/a1.txt', 'a1'); + $filesystem->dumpFile($this->tmpDir.'/dirA/a2.txt', 'a2'); + $filesystem->dumpFile($this->tmpDir.'/dirB/b1.txt', 'b1'); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->tmpDir); + } + + public function testSameFolderIsBrowsedAgainOnSecondExecution(): void + { + $task = new FolderBrowserTask(new NullLogger()); + $state = $this->createState(['folder_path' => $this->tmpDir.'/dirA']); + + $expected = [$this->tmpDir.'/dirA/a1.txt', $this->tmpDir.'/dirA/a2.txt']; + self::assertSame( + [$expected, $expected], + [$this->iterate($task, $state), $this->iterate($task, $state)], + ); + } + + public function testInputFolderBrowserBrowsesSameFolderTwice(): void + { + $task = new InputFolderBrowserTask(new NullLogger()); + $state = $this->createState([]); + + $expected = [$this->tmpDir.'/dirA/a1.txt', $this->tmpDir.'/dirA/a2.txt']; + self::assertSame( + [$expected, $expected], + [$this->iterate($task, $state, $this->tmpDir.'/dirA'), $this->iterate($task, $state, $this->tmpDir.'/dirA')], + ); + } + + public function testInputFolderBrowserBrowsesDifferentFoldersSuccessively(): void + { + $task = new InputFolderBrowserTask(new NullLogger()); + $state = $this->createState([]); + + self::assertSame( + [$this->tmpDir.'/dirA/a1.txt', $this->tmpDir.'/dirA/a2.txt'], + $this->iterate($task, $state, $this->tmpDir.'/dirA'), + ); + self::assertSame([$this->tmpDir.'/dirB/b1.txt'], $this->iterate($task, $state, $this->tmpDir.'/dirB')); + } + + public function testInputFolderBrowserBrowsesAnotherFolderAfterAnEmptyOne(): void + { + (new Filesystem())->mkdir($this->tmpDir.'/empty'); + $task = new InputFolderBrowserTask(new NullLogger()); + $state = $this->createState([]); + + self::assertSame([], $this->iterate($task, $state, $this->tmpDir.'/empty')); + self::assertSame([$this->tmpDir.'/dirB/b1.txt'], $this->iterate($task, $state, $this->tmpDir.'/dirB')); + } + + /** + * Mimics the ProcessManager loop over an iterable task and returns the non-skipped outputs. + * + * @return list + */ + private function iterate(FolderBrowserTask $task, ProcessState $state, mixed $input = null): array + { + $outputs = []; + $state->setInput($input); + do { + $state->setSkipped(false); + $task->execute($state); + if (!$state->isSkipped()) { + $outputs[] = $state->getOutput(); + } + } while ($task->next($state)); + + return $outputs; + } + + private function createState(array $options): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('browse', FolderBrowserTask::class, $options)); + + return $state; + } +} diff --git a/tests/Task/File/LineReaderTaskTest.php b/tests/Task/File/LineReaderTaskTest.php new file mode 100644 index 00000000..df1f0752 --- /dev/null +++ b/tests/Task/File/LineReaderTaskTest.php @@ -0,0 +1,113 @@ +tmpDir = sys_get_temp_dir().\DIRECTORY_SEPARATOR.uniqid('line_reader_test_', true); + $filesystem = new Filesystem(); + $filesystem->dumpFile($this->tmpDir.'/a.txt', "a1\na2\n"); + $filesystem->dumpFile($this->tmpDir.'/b.txt', "b1\n"); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->tmpDir); + } + + public function testSameFileIsReadAgainOnSecondExecution(): void + { + $task = new LineReaderTask(); + $state = $this->createState(['filename' => $this->tmpDir.'/a.txt']); + + self::assertSame( + [["a1\n", "a2\n"], ["a1\n", "a2\n"]], + [$this->iterate($task, $state), $this->iterate($task, $state)], + ); + } + + public function testInputLineReaderReadsSameFileTwice(): void + { + $task = new InputLineReaderTask(); + $state = $this->createState([]); + + self::assertSame( + [["a1\n", "a2\n"], ["a1\n", "a2\n"]], + [$this->iterate($task, $state, $this->tmpDir.'/a.txt'), $this->iterate($task, $state, $this->tmpDir.'/a.txt')], + ); + } + + public function testInputLineReaderReadsDifferentFilesSuccessively(): void + { + $task = new InputLineReaderTask(); + $state = $this->createState([]); + + self::assertSame(["a1\n", "a2\n"], $this->iterate($task, $state, $this->tmpDir.'/a.txt')); + self::assertSame(["b1\n"], $this->iterate($task, $state, $this->tmpDir.'/b.txt')); + } + + /** + * Mimics the ProcessManager loop over an iterable task and returns the non-skipped outputs. + * + * @return list + */ + private function iterate(LineReaderTask $task, ProcessState $state, mixed $input = null): array + { + $outputs = []; + $state->setInput($input); + do { + $state->setSkipped(false); + $task->execute($state); + if (!$state->isSkipped()) { + $outputs[] = $state->getOutput(); + } + } while ($task->next($state)); + + return $outputs; + } + + private function createState(array $options): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('read', LineReaderTask::class, $options)); + + return $state; + } +}