#208 - Fix FolderBrowserTask, InputFolderBrowserTask, CsvReaderTask and LineReaderTask (and their Input* variants)

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.

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.

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.

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
 ----------------

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
 ----------------

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
 ----------------

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.

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

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

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
     {

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

tests/Task/File/Csv/CsvReaderTaskTest.php

@@ -0,0 +1,130 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Tests\Task\File\Csv;
+
+use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
+use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
+use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
+use CleverAge\ProcessBundle\Filesystem\CsvFile;
+use CleverAge\ProcessBundle\Filesystem\CsvResource;
+use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
+use CleverAge\ProcessBundle\Model\ProcessHistory;
+use CleverAge\ProcessBundle\Model\ProcessState;
+use CleverAge\ProcessBundle\Task\File\Csv\AbstractCsvResourceTask;
+use CleverAge\ProcessBundle\Task\File\Csv\AbstractCsvTask;
+use CleverAge\ProcessBundle\Task\File\Csv\CsvReaderTask;
+use CleverAge\ProcessBundle\Task\File\Csv\InputCsvReaderTask;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Component\Filesystem\Filesystem;
+
+#[\PHPUnit\Framework\Attributes\CoversClass(CsvReaderTask::class)]
+#[\PHPUnit\Framework\Attributes\CoversClass(InputCsvReaderTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(AbstractCsvTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(AbstractCsvResourceTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(CsvFile::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(CsvResource::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(AbstractConfigurableTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ContextualOptionResolver::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)]
+class CsvReaderTaskTest extends TestCase
+{
+    private string $tmpDir;
+
+    protected function setUp(): void
+    {
+        $this->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<mixed>
+     */
+    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;
+    }
+}

tests/Task/File/FolderBrowserTaskTest.php

@@ -0,0 +1,130 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ProcessBundle package.
+ *
+ * Copyright (c) Clever-Age
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace CleverAge\ProcessBundle\Tests\Task\File;
+
+use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
+use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
+use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
+use CleverAge\ProcessBundle\Model\AbstractConfigurableTask;
+use CleverAge\ProcessBundle\Model\ProcessHistory;
+use CleverAge\ProcessBundle\Model\ProcessState;
+use CleverAge\ProcessBundle\Task\File\FolderBrowserTask;
+use CleverAge\ProcessBundle\Task\File\InputFolderBrowserTask;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Component\Filesystem\Filesystem;
+
+#[\PHPUnit\Framework\Attributes\CoversClass(FolderBrowserTask::class)]
+#[\PHPUnit\Framework\Attributes\CoversClass(InputFolderBrowserTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(AbstractConfigurableTask::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ContextualOptionResolver::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)]
+#[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)]
+class FolderBrowserTaskTest extends TestCase
+{
+    private string $tmpDir;
+
+    protected function setUp(): void
+    {
+        $this->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<mixed>
+     */
+    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;
+    }
+}