diff --git a/CHANGELOG.md b/CHANGELOG.md index 9027253b..5a32828f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Latest * [#205](https://github.com/cleverage/process-bundle/issues/205) Fix FileSplitterTask: produced files lost the first line, doubled line breaks and ended with an empty file; each file now contains exactly `max_lines` lines of the source. Update documentation, add tests. * [#206](https://github.com/cleverage/process-bundle/issues/206) Fix AdvancedStatCounterTask, IterableBatchTask, ConditionTrait and ColumnAggregatorTask edge cases: log the first counted execution with correct counts, accept a `null` `batch_count` (only flush at the end), allow scalar inputs in conditions (`''` path on the whole value), aggregate `null` column values. Update documentation, add tests. * [#207](https://github.com/cleverage/process-bundle/issues/207) Fix JsonStreamReaderTask / JsonStreamWriterTask: throw an explicit `\UnexpectedValueException` when a line decodes to a scalar, create the missing parent directory when writing. 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; + } +}