diff --git a/CHANGELOG.md b/CHANGELOG.md index 50a4298..7a8cc03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ### Changes +* [#94](https://github.com/cleverage/flysystem-process-bundle/issues/94) `ListContentTask` (`flysystem_list_content`): the listing is read item by item (the Flysystem generator is iterated lazily) instead of being loaded in an array, which used about 26 MB more memory for 100,000 files. The output and its order are unchanged; `getFilteredFilesystemContents()` and `$fsContent` of this (final since 3.3) class now are a `\Generator`. Update documentation, add tests. * [#92](https://github.com/cleverage/flysystem-process-bundle/issues/92) Documentation: the storages are configured with the format of league/flysystem-bundle 3.5 and later (the adapter being the key of its options), the `adapter` / `options` format being deprecated since flysystem-bundle 3.5; the former format is mentioned for an earlier version. v3.4 diff --git a/docs/reference/tasks/list_content_task.md b/docs/reference/tasks/list_content_task.md index 0499325..db0b0b5 100644 --- a/docs/reference/tasks/list_content_task.md +++ b/docs/reference/tasks/list_content_task.md @@ -30,6 +30,10 @@ with `property: path` to get the path, as expected by [FileFetchTask](file_fetch When the storage is empty, or when no item matches `file_pattern`, no output is produced and the task is skipped. +The listing is read item by item while the task iterates, it is not loaded in memory. Each execution of the task (for each +input) lists the storage again. Be careful if a downstream task changes the listed directory (e.g. removing the files) +while the iteration is in progress: with an adapter listing lazily, the iteration may then skip some items. + Options ------- diff --git a/src/Task/ListContentTask.php b/src/Task/ListContentTask.php index ba843cc..17be3b1 100644 --- a/src/Task/ListContentTask.php +++ b/src/Task/ListContentTask.php @@ -29,9 +29,11 @@ class ListContentTask extends AbstractConfigurableTask implements IterableTaskInterface { /** - * @var list<\League\Flysystem\StorageAttributes>|null + * The listing being iterated, read lazily (a huge storage is not loaded in memory). + * + * @var \Generator|null */ - protected ?array $fsContent = null; + protected ?\Generator $fsContent = null; /** * @param ServiceLocator $storages @@ -54,7 +56,7 @@ protected function configureOptions(OptionsResolver $resolver): void */ public function execute(ProcessState $state): void { - if (null === $this->fsContent || null === key($this->fsContent)) { + if (!$this->fsContent instanceof \Generator || !$this->fsContent->valid()) { /** @var string $filesystemOption */ $filesystemOption = $this->getOption($state, 'filesystem'); $filesystem = $this->storages->get($filesystemOption); @@ -64,39 +66,36 @@ public function execute(ProcessState $state): void $this->fsContent = $this->getFilteredFilesystemContents($filesystem, $patternOption); } - if (null === key($this->fsContent)) { + if (!$this->fsContent->valid()) { $state->setSkipped(true); $this->fsContent = null; } else { - $state->setOutput(current($this->fsContent)); + $state->setOutput($this->fsContent->current()); } } public function next(ProcessState $state): bool { - if (!\is_array($this->fsContent)) { + if (!$this->fsContent instanceof \Generator) { return false; } - next($this->fsContent); + $this->fsContent->next(); - return null !== key($this->fsContent); + return $this->fsContent->valid(); } /** - * @return list<\League\Flysystem\StorageAttributes> + * @return \Generator * * @throws FilesystemException */ - protected function getFilteredFilesystemContents(FilesystemOperator $filesystem, ?string $pattern = null): array + protected function getFilteredFilesystemContents(FilesystemOperator $filesystem, ?string $pattern = null): \Generator { - $results = []; foreach ($filesystem->listContents('') as $item) { if (null === $pattern || preg_match($pattern, $item->path())) { - $results[] = $item; + yield $item; } } - - return $results; } } diff --git a/tests/Task/ListContentTaskTest.php b/tests/Task/ListContentTaskTest.php index e13c52f..3c0b317 100644 --- a/tests/Task/ListContentTaskTest.php +++ b/tests/Task/ListContentTaskTest.php @@ -15,8 +15,12 @@ use CleverAge\FlysystemProcessBundle\Task\ListContentTask; use CleverAge\ProcessBundle\Model\ProcessState; +use League\Flysystem\DirectoryListing; +use League\Flysystem\FileAttributes; +use League\Flysystem\FilesystemOperator; use League\Flysystem\StorageAttributes; use PHPUnit\Framework\Attributes\CoversClass; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\OptionsResolver\Exception\MissingOptionsException; #[CoversClass(ListContentTask::class)] @@ -57,6 +61,46 @@ public function testEachExecutionListsTheStorageAgain(): void self::assertSame(2, $this->listings['source']); } + /** + * The listing is read item by item, not loaded in an array: only the items already output (and the one being + * output) are read from the listing. + */ + public function testListingIsReadLazily(): void + { + $read = 0; + $listing = new DirectoryListing((static function () use (&$read): \Generator { + foreach (['a.txt', 'b.txt', 'c.txt'] as $path) { + ++$read; + yield new FileAttributes($path); + } + })()); + $filesystem = $this->createStub(FilesystemOperator::class); + $filesystem->method('listContents')->willReturn($listing); + $task = new ListContentTask(new ServiceLocator(['source' => static fn (): FilesystemOperator => $filesystem])); + $state = $this->createState(ListContentTask::class, ['filesystem' => 'source']); + $task->initialize($state); + + $state->reset(true); + $task->execute($state); + self::assertSame(1, $read); + self::assertTrue($task->next($state)); + $task->execute($state); + self::assertSame(2, $read); + self::assertTrue($task->next($state)); + $task->execute($state); + self::assertFalse($task->next($state)); + self::assertSame(3, $read); + } + + public function testNoItemMatchingThePattern(): void + { + $this->createFiles('source', ['a.txt' => 'a']); + [$task, $state] = $this->createTask(['file_pattern' => '/\.csv$/']); + + self::assertSame([], $this->iterate($task, $state, null)); + self::assertFalse($task->next($state)); + } + public function testNextBeforeExecute(): void { [$task, $state] = $this->createTask([]);