From c36e43113a4f0608acfd217f86b3a3e27c7a4f71 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Fri, 2 Oct 2026 11:43:44 +0200 Subject: [PATCH] fix(task) #37 #38 #39 #40 #41 FileFetchTask lists once and processes each input, ignore_missing on input files, file named 0, RemoveFileTask missing files and lists; #33 add missing tests Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 10 + composer.json | 1 + docs/reference/tasks/file_fetch_task.md | 34 ++-- docs/reference/tasks/remove_file_task.md | 15 +- docs/troubleshooting.md | 2 +- src/Task/FileFetchTask.php | 67 ++++--- src/Task/RemoveFileTask.php | 25 ++- tests/CleverAgeFlysystemProcessBundleTest.php | 30 +++ ...CleverAgeFlysystemProcessExtensionTest.php | 82 ++++++++ tests/Task/FileFetchTaskStorageTest.php | 179 ++++++++++++++++++ tests/Task/FileFetchTaskTest.php | 1 + tests/Task/ListContentTaskTest.php | 103 ++++++++++ tests/Task/RemoveFileTaskTest.php | 131 +++++++++++++ tests/Task/StorageTestCase.php | 141 ++++++++++++++ 14 files changed, 760 insertions(+), 61 deletions(-) create mode 100644 tests/CleverAgeFlysystemProcessBundleTest.php create mode 100644 tests/DependencyInjection/CleverAgeFlysystemProcessExtensionTest.php create mode 100644 tests/Task/FileFetchTaskStorageTest.php create mode 100644 tests/Task/ListContentTaskTest.php create mode 100644 tests/Task/RemoveFileTaskTest.php create mode 100644 tests/Task/StorageTestCase.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 8266b5e..1aa86c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,16 @@ Latest ------ +### Changes +* [#33](https://github.com/cleverage/flysystem-process-bundle/issues/33) Add missing tests: FileFetchTask, ListContentTask and RemoveFileTask on local storages, bundle and DI extension. + +### Fixes +* [#37](https://github.com/cleverage/flysystem-process-bundle/issues/37) Fix FileFetchTask: list the source storage once per input (it was listed again before each file). Update documentation, add tests. +* [#38](https://github.com/cleverage/flysystem-process-bundle/issues/38) Fix FileFetchTask: process each input (with `file_pattern`, only the first input copied the files; a path received twice was skipped). Update documentation, add tests. +* [#39](https://github.com/cleverage/flysystem-process-bundle/issues/39) Fix FileFetchTask: apply `ignore_missing` to the files given as input (a missing file threw `UnableToReadFile`). Update documentation, add tests. +* [#40](https://github.com/cleverage/flysystem-process-bundle/issues/40) Fix FileFetchTask and RemoveFileTask: a file named `0` is no longer handled as no file. Add tests. +* [#41](https://github.com/cleverage/flysystem-process-bundle/issues/41) Fix RemoveFileTask: a missing file is logged as not found (it was logged as deleted), accept a list of paths, fix the deletion failure message. Update documentation, add tests. + v3.1 ------ diff --git a/composer.json b/composer.json index 9657071..6ca0d63 100644 --- a/composer.json +++ b/composer.json @@ -66,6 +66,7 @@ "phpunit/phpunit": "^10.5|^11|^12|^13", "rector/rector": "*", "roave/security-advisories": "dev-latest", + "symfony/filesystem": "^6.4|^7.4|^8", "symfony/test-pack": "^1.1" }, "config": { diff --git a/docs/reference/tasks/file_fetch_task.md b/docs/reference/tasks/file_fetch_task.md index 61dfa9f..ca05c04 100644 --- a/docs/reference/tasks/file_fetch_task.md +++ b/docs/reference/tasks/file_fetch_task.md @@ -30,19 +30,19 @@ Possible outputs `string`: for each copied file, its path relative to the storage root (the same path is used in the `source_filesystem` and in the `destination_filesystem`). -When no file matches `file_pattern` (and `ignore_missing` is `true`), or when every received file has already been -copied during this process execution, no output is produced and the task is skipped. +When no file matches `file_pattern`, or when no file given as input exists (and `ignore_missing` is `true`), no +output is produced and the task is skipped. Options ------- -| Code | Type | Required | Default | Description | -|--------------------------|----------------|:--------:|---------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `source_filesystem` | `string` | **X** | | Name of the Flysystem storage to read files from, as configured under `flysystem.storages` (see [configuration](../../index.md#configuration)) | -| `destination_filesystem` | `string` | **X** | | Name of the Flysystem storage to write files to, as configured under `flysystem.storages` | -| `file_pattern` | `string\|null` | | `null` | Regular expression (see [preg_match](https://www.php.net/manual/en/function.preg-match.php)) tested on the path of each file at the root of `source_filesystem`. If `null` (or empty), the file path(s) are taken from the input | -| `remove_source` | `bool` | | `false` | Delete the file from `source_filesystem` after the copy (move instead of copy) | -| `ignore_missing` | `bool` | | `true` | Only used with `file_pattern`: if `false`, throw an `\UnexpectedValueException` (`File(s) not found in source filesystem`) when no file matches the pattern. If `true`, the task is skipped | +| Code | Type | Required | Default | Description | +|--------------------------|----------------|:--------:|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `source_filesystem` | `string` | **X** | | Name of the Flysystem storage to read files from, as configured under `flysystem.storages` (see [configuration](../../index.md#configuration)) | +| `destination_filesystem` | `string` | **X** | | Name of the Flysystem storage to write files to, as configured under `flysystem.storages` | +| `file_pattern` | `string\|null` | | `null` | Regular expression (see [preg_match](https://www.php.net/manual/en/function.preg-match.php)) tested on the path of each file at the root of `source_filesystem`. If `null` (or empty), the file path(s) are taken from the input | +| `remove_source` | `bool` | | `false` | Delete the file from `source_filesystem` after the copy (move instead of copy) | +| `ignore_missing` | `bool` | | `true` | If `false`, throw an `\UnexpectedValueException` when no file matches `file_pattern` (`File(s) not found in source filesystem`) or when a file given as input does not exist (`File not found in source filesystem`). If `true`, missing files are skipped | Examples -------- @@ -101,14 +101,14 @@ Notes * `file_pattern` is only tested on the files located at the root of `source_filesystem` (the listing is not recursive and directories are ignored). The pattern is tested on the path relative to the storage root: to target a sub-directory, configure a dedicated storage whose root is this directory. -* The source storage is listed again before each iteration: files added to the source during the iteration are also - copied. With an SFTP storage and long-running downstream tasks, see - [SFTP stale connection](../../troubleshooting.md). -* A given file is copied only once per process execution: already copied paths are remembered, so receiving the same - path again (or a new input while using `file_pattern`) skips the task. -* The file is written to `destination_filesystem` with the same path, overwriting any existing file. A file given as - input that does not exist in `source_filesystem` throws a `League\Flysystem\UnableToReadFile` exception, whatever - the value of `ignore_missing`. +* With `file_pattern`, the source storage is listed once per input, when the iteration starts: files added to the + source during the iteration are copied by the next execution of the task. With an SFTP storage and long-running + downstream tasks, see [SFTP stale connection](../../troubleshooting.md). +* Each input is processed: the files are copied for each input received by the task (e.g. after an iterable task), + even if they have already been copied during this process execution. A path given several times in the same input + is copied once. +* The file is written to `destination_filesystem` with the same path, overwriting any existing file. The existence of + the files given as input is checked in `source_filesystem` (see `ignore_missing`). * A failure while writing to `destination_filesystem` throws a `League\Flysystem\FilesystemException` (e.g. `UnableToWriteFile`): the task's `error_strategy` applies, and with `remove_source: true` the source file is kept. * To read a copied file with a core task (e.g. diff --git a/docs/reference/tasks/remove_file_task.md b/docs/reference/tasks/remove_file_task.md index c57aedb..be9af89 100644 --- a/docs/reference/tasks/remove_file_task.md +++ b/docs/reference/tasks/remove_file_task.md @@ -12,11 +12,10 @@ Accepted inputs --------------- * When `file_pattern` is set, the input is ignored (but the deletion is run again each time the task is executed). -* Otherwise, `string`: path of the file to delete, relative to the root of the `filesystem` storage. An empty input - throws an `\UnexpectedValueException` (`No pattern neither input provided for the Task`). A list of paths is not - supported: iterate over it first (e.g. with - [InputIteratorTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/input_iterator_task.md)). - A `StorageAttributes` output of [ListContentTask](list_content_task.md) must be converted to its `path` first. +* Otherwise, `string|array`: path, or list of paths, of the file(s) to delete, relative to the root of the + `filesystem` storage. An empty input throws an `\UnexpectedValueException` (`No pattern neither input provided for + the Task`). A `StorageAttributes` output of [ListContentTask](list_content_task.md) must be converted to its `path` + first. Possible outputs ---------------- @@ -65,8 +64,8 @@ Notes * `file_pattern` is only tested on the files located at the root of the storage (the listing is not recursive and directories are ignored). * Deletion errors do not stop the process: each deleted file is logged with the `info` level (`Deleted input file`), - and a deletion failure (`League\Flysystem\FilesystemException`) is logged with the `warning` level, with the file - path in the log context. Deleting a file that does not exist is not an error for most adapters (e.g. `local`, - `sftp`): it is logged as deleted. + and a deletion failure (`League\Flysystem\FilesystemException`) is logged with the `warning` level + (`Failed to delete input file`), with the file path in the log context. A file given as input that does not exist + (or is a directory) is not deleted and is logged with the `warning` level (`Input file not found`). * The storage is resolved on each execution: an unknown storage name makes the task fail when it is executed. * See the [Remote cleanup](../../cookbooks/remote_cleanup.md) cookbook. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 4f8ad85..9670d71 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -16,7 +16,7 @@ Connection closed prematurely Typically the first SFTP task succeeds (a file is fetched/listed), then the process spends a long time doing other work (large CSV import, thousands of API calls, …), and a subsequent SFTP task — -often the archiving step, or the next iteration of an iterable task that re-lists the source — blows +often the archiving step, or the listing of the source for the next input of an iterable task — blows up with the error above. ### Cause diff --git a/src/Task/FileFetchTask.php b/src/Task/FileFetchTask.php index ff4d4f6..fc950a4 100644 --- a/src/Task/FileFetchTask.php +++ b/src/Task/FileFetchTask.php @@ -32,9 +32,11 @@ class FileFetchTask extends AbstractConfigurableTask implements IterableTaskInte protected FilesystemOperator $destinationFS; /** - * @var array + * Files of the current input, null when no iteration is in progress. + * + * @var list|null */ - protected array $matchingFiles = []; + protected ?array $matchingFiles = null; /** * @param ServiceLocator $storages @@ -67,10 +69,12 @@ public function initialize(ProcessState $state): void */ public function execute(ProcessState $state): void { - $this->findMatchingFiles($state); + // The files are listed once per input, when its iteration starts + $this->matchingFiles ??= $this->findMatchingFiles($state); $file = current($this->matchingFiles); - if (!$file) { + if (false === $file) { + $this->matchingFiles = null; $state->setSkipped(true); return; @@ -82,55 +86,62 @@ public function execute(ProcessState $state): void $state->setOutput($file); } - /** - * @throws \UnexpectedValueException - * @throws \InvalidArgumentException - * @throws FilesystemException - */ public function next(ProcessState $state): bool { - $this->findMatchingFiles($state); + if (null === $this->matchingFiles) { + return false; + } + if (false !== next($this->matchingFiles)) { + return true; + } + + // End of the iteration: the next input lists its files again + $this->matchingFiles = null; - return false !== next($this->matchingFiles); + return false; } /** + * @return list + * * @throws \UnexpectedValueException * @throws \InvalidArgumentException * @throws FilesystemException */ - protected function findMatchingFiles(ProcessState $state): void + protected function findMatchingFiles(ProcessState $state): array { + /** @var bool $ignoreMissing */ + $ignoreMissing = $this->getOption($state, 'ignore_missing'); + $matchingFiles = []; + /** @var ?string $filePattern */ $filePattern = $this->getOption($state, 'file_pattern'); - if ($filePattern) { + if (null !== $filePattern && '' !== $filePattern) { foreach ($this->sourceFS->listContents('/') as $file) { - if ('file' === $file->type() - && preg_match($filePattern, $file->path()) - && !\in_array($file->path(), $this->matchingFiles, true) - ) { - $this->matchingFiles[] = $file->path(); + if ('file' === $file->type() && preg_match($filePattern, $file->path())) { + $matchingFiles[] = $file->path(); } } } else { - /** @var array|string|null $input */ $input = $state->getInput(); - if (!$input) { + if (null === $input || '' === $input || [] === $input) { throw new \UnexpectedValueException('No pattern neither input provided for the Task'); } - if (\is_array($input)) { - foreach ($input as $file) { - if (!\in_array($file, $this->matchingFiles, true)) { - $this->matchingFiles[] = $file; - } + /** @var list $files */ + $files = \is_array($input) ? array_values($input) : [$input]; + foreach (array_unique($files) as $file) { + if ($this->sourceFS->fileExists($file)) { + $matchingFiles[] = $file; + } elseif (!$ignoreMissing) { + throw new \UnexpectedValueException("File {$file} not found in source filesystem"); } - } elseif (!\in_array($input, $this->matchingFiles, true)) { - $this->matchingFiles[] = $input; } } - if ([] === $this->matchingFiles && !$this->getOption($state, 'ignore_missing')) { + if ([] === $matchingFiles && !$ignoreMissing) { throw new \UnexpectedValueException('File(s) not found in source filesystem'); } + + return $matchingFiles; } /** diff --git a/src/Task/RemoveFileTask.php b/src/Task/RemoveFileTask.php index 9140544..d03f9af 100644 --- a/src/Task/RemoveFileTask.php +++ b/src/Task/RemoveFileTask.php @@ -52,26 +52,37 @@ public function execute(ProcessState $state): void /** @var ?string $filePattern */ $filePattern = $this->getOption($state, 'file_pattern'); - if ($filePattern) { + if (null !== $filePattern && '' !== $filePattern) { foreach ($this->filesystem->listContents('/') as $file) { if ('file' === $file->type() && preg_match($filePattern, $file->path())) { - $this->deleteFile($file->path()); + $this->deleteFile($file->path(), false); } } } else { - /** @var ?string $input */ $input = $state->getInput(); - if (!$input) { + if (null === $input || '' === $input || [] === $input) { throw new \UnexpectedValueException('No pattern neither input provided for the Task'); } - $this->deleteFile($input); + /** @var list $files */ + $files = \is_array($input) ? array_values($input) : [$input]; + foreach ($files as $file) { + $this->deleteFile($file, true); + } } } - private function deleteFile(string $filePath): void + /** + * @param bool $checkExists Most adapters (e.g. local, sftp) do not fail when deleting a missing file + */ + private function deleteFile(string $filePath, bool $checkExists): void { try { + if ($checkExists && !$this->filesystem->fileExists($filePath)) { + $this->logger->warning('Input file not found', ['file' => $filePath]); + + return; + } $this->filesystem->delete($filePath); $result = true; } catch (FilesystemException) { @@ -81,7 +92,7 @@ private function deleteFile(string $filePath): void if ($result) { $this->logger->info('Deleted input file', ['file' => $filePath]); } else { - $this->logger->warning('Failed to deleted input file', ['file' => $filePath]); + $this->logger->warning('Failed to delete input file', ['file' => $filePath]); } } } diff --git a/tests/CleverAgeFlysystemProcessBundleTest.php b/tests/CleverAgeFlysystemProcessBundleTest.php new file mode 100644 index 0000000..d01c606 --- /dev/null +++ b/tests/CleverAgeFlysystemProcessBundleTest.php @@ -0,0 +1,30 @@ +getPath(); + + self::assertSame(\dirname(__DIR__), $path); + self::assertDirectoryExists($path.'/config/services'); + } +} diff --git a/tests/DependencyInjection/CleverAgeFlysystemProcessExtensionTest.php b/tests/DependencyInjection/CleverAgeFlysystemProcessExtensionTest.php new file mode 100644 index 0000000..08691d2 --- /dev/null +++ b/tests/DependencyInjection/CleverAgeFlysystemProcessExtensionTest.php @@ -0,0 +1,82 @@ + + */ + public static function provideTasks(): iterable + { + yield 'file_fetch' => ['cleverage_flysystem_process.task.file_fetch', FileFetchTask::class]; + yield 'list_content' => ['cleverage_flysystem_process.task.list_content', ListContentTask::class]; + yield 'remove_file' => ['cleverage_flysystem_process.task.remove_file', RemoveFileTask::class]; + } + + /** + * @param class-string $class + */ + #[DataProvider('provideTasks')] + public function testTaskIsRegistered(string $id, string $class): void + { + $container = new ContainerBuilder(); + (new CleverAgeFlysystemProcessExtension())->load([], $container); + + $definition = $container->getDefinition($id); + self::assertSame($class, $definition->getClass()); + // Tasks are stateful: each process execution must get its own instance + self::assertFalse($definition->isShared()); + + // The Flysystem storages, indexed by name + $storages = array_values(array_filter( + $definition->getArguments(), + static fn (mixed $argument): bool => $argument instanceof ServiceLocatorArgument + )); + self::assertCount(1, $storages); + $taggedIterator = $storages[0]->getTaggedIteratorArgument(); + self::assertInstanceOf(TaggedIteratorArgument::class, $taggedIterator); + self::assertSame('flysystem.storage', $taggedIterator->getTag()); + self::assertSame('storage', $taggedIterator->getIndexAttribute()); + + // Referenced as '@' in process configurations + $alias = $container->getAlias($class); + self::assertSame($id, (string) $alias); + self::assertTrue($alias->isPublic()); + } + + public function testEveryTaskIsTested(): void + { + $container = new ContainerBuilder(); + (new CleverAgeFlysystemProcessExtension())->load([], $container); + + $ids = array_filter( + array_keys($container->getDefinitions()), + static fn (string $id): bool => str_starts_with($id, 'cleverage_flysystem_process.task.') + ); + self::assertEqualsCanonicalizing(array_column(iterator_to_array(self::provideTasks()), 0), array_values($ids)); + } +} diff --git a/tests/Task/FileFetchTaskStorageTest.php b/tests/Task/FileFetchTaskStorageTest.php new file mode 100644 index 0000000..316ae59 --- /dev/null +++ b/tests/Task/FileFetchTaskStorageTest.php @@ -0,0 +1,179 @@ +createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b', 'c.csv' => 'c', 'dir.txt/d.txt' => 'd']); + [$task, $state] = $this->createTask(['file_pattern' => '/\.txt$/']); + + $outputs = $this->iterate($task, $state, null); + sort($outputs); + + // Directories are ignored + self::assertSame(['a.txt', 'b.txt'], $outputs); + self::assertSame(['a.txt', 'b.txt'], $this->getFiles('destination')); + self::assertSame('a', file_get_contents($this->dir.'/destination/a.txt')); + // Source kept by default + self::assertSame(['a.txt', 'b.txt', 'c.csv', 'dir.txt'], $this->getFiles('source')); + } + + public function testSourceIsListedOncePerInput(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b', 'c.txt' => 'c']); + [$task, $state] = $this->createTask(['file_pattern' => '/\.txt$/']); + + self::assertCount(3, $this->iterate($task, $state, null)); + self::assertSame(1, $this->listings['source']); + } + + public function testEachInputCopiesTheMatchingFiles(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b']); + [$task, $state] = $this->createTask(['file_pattern' => '/\.txt$/']); + + self::assertCount(2, $this->iterate($task, $state, 'first')); + self::assertCount(2, $this->iterate($task, $state, 'second')); + self::assertSame(2, $this->listings['source']); + } + + public function testFilesFromInput(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b', '0' => 'zero']); + [$task, $state] = $this->createTask([]); + + self::assertSame(['a.txt'], $this->iterate($task, $state, 'a.txt')); + self::assertSame(['b.txt', '0'], $this->iterate($task, $state, ['b.txt', '0', 'b.txt'])); + // A path received twice is copied again + self::assertSame(['a.txt'], $this->iterate($task, $state, 'a.txt')); + self::assertSame(['0'], $this->iterate($task, $state, '0')); + self::assertSame(['0', 'a.txt', 'b.txt'], $this->getFiles('destination')); + } + + public function testRemoveSource(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b']); + [$task, $state] = $this->createTask(['remove_source' => true]); + + self::assertSame(['a.txt'], $this->iterate($task, $state, 'a.txt')); + self::assertSame(['b.txt'], $this->getFiles('source')); + self::assertSame(['a.txt'], $this->getFiles('destination')); + } + + public function testExistingDestinationFileIsOverwritten(): void + { + $this->createFiles('source', ['a.txt' => 'new']); + $this->createFiles('destination', ['a.txt' => 'old']); + [$task, $state] = $this->createTask([]); + + $this->iterate($task, $state, 'a.txt'); + + self::assertSame('new', file_get_contents($this->dir.'/destination/a.txt')); + } + + public function testMissingInputFileIsIgnored(): void + { + $this->createFiles('source', ['a.txt' => 'a']); + [$task, $state] = $this->createTask([]); + + self::assertSame(['a.txt'], $this->iterate($task, $state, ['missing.txt', 'a.txt'])); + self::assertSame([], $this->iterate($task, $state, 'missing.txt')); + } + + public function testMissingInputFileFails(): void + { + [$task, $state] = $this->createTask(['ignore_missing' => false]); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('File missing.txt not found in source filesystem'); + $this->iterate($task, $state, 'missing.txt'); + } + + public function testNoMatchingFileIsIgnored(): void + { + [$task, $state] = $this->createTask(['file_pattern' => '/\.txt$/']); + + self::assertSame([], $this->iterate($task, $state, null)); + } + + public function testNoMatchingFileFails(): void + { + [$task, $state] = $this->createTask(['file_pattern' => '/\.txt$/', 'ignore_missing' => false]); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('File(s) not found in source filesystem'); + $this->iterate($task, $state, null); + } + + public function testNoPatternNorInput(): void + { + [$task, $state] = $this->createTask([]); + + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('No pattern neither input provided for the Task'); + $this->iterate($task, $state, ''); + } + + public function testNextBeforeExecute(): void + { + [$task, $state] = $this->createTask([]); + + self::assertFalse($task->next($state)); + } + + public function testUnknownStorage(): void + { + $this->expectException(\Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException::class); + $this->createTask(['source_filesystem' => 'unknown']); + } + + public function testRequiredOptions(): void + { + $this->expectException(MissingOptionsException::class); + $task = new FileFetchTask($this->createStorages()); + $task->initialize($this->createState(FileFetchTask::class, [])); + } + + public function testInvalidOptionType(): void + { + $this->expectException(InvalidOptionsException::class); + $this->createTask(['remove_source' => 'yes']); + } + + /** + * @param array $options + * + * @return array{FileFetchTask, ProcessState} + */ + private function createTask(array $options): array + { + $state = $this->createState(FileFetchTask::class, $options + [ + 'source_filesystem' => 'source', + 'destination_filesystem' => 'destination', + ]); + $task = new FileFetchTask($this->createStorages()); + $task->initialize($state); + + return [$task, $state]; + } +} diff --git a/tests/Task/FileFetchTaskTest.php b/tests/Task/FileFetchTaskTest.php index 40b9ab2..86bf533 100644 --- a/tests/Task/FileFetchTaskTest.php +++ b/tests/Task/FileFetchTaskTest.php @@ -32,6 +32,7 @@ protected function setUp(): void { $this->source = $this->createMock(FilesystemOperator::class); $this->source->method('readStream')->willReturnCallback(static fn () => fopen('php://memory', 'r')); + $this->source->method('fileExists')->willReturn(true); $this->destination = $this->createMock(FilesystemOperator::class); } diff --git a/tests/Task/ListContentTaskTest.php b/tests/Task/ListContentTaskTest.php new file mode 100644 index 0000000..e13c52f --- /dev/null +++ b/tests/Task/ListContentTaskTest.php @@ -0,0 +1,103 @@ +createFiles('source', ['a.txt' => 'a', 'b.csv' => 'b', 'dir/c.txt' => 'c']); + [$task, $state] = $this->createTask([]); + + // Files and directories, at the root of the storage only + self::assertSame(['a.txt:file', 'b.csv:file', 'dir:dir'], $this->paths($this->iterate($task, $state, null))); + } + + public function testFilePattern(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.csv' => 'b', 'dir/c.txt' => 'c']); + [$task, $state] = $this->createTask(['file_pattern' => '/\.txt$/']); + + self::assertSame(['a.txt:file'], $this->paths($this->iterate($task, $state, null))); + } + + public function testEmptyStorage(): void + { + [$task, $state] = $this->createTask([]); + + self::assertSame([], $this->iterate($task, $state, null)); + } + + public function testEachExecutionListsTheStorageAgain(): void + { + $this->createFiles('source', ['a.txt' => 'a']); + [$task, $state] = $this->createTask([]); + + self::assertSame(['a.txt:file'], $this->paths($this->iterate($task, $state, null))); + $this->createFiles('source', ['b.txt' => 'b']); + self::assertSame(['a.txt:file', 'b.txt:file'], $this->paths($this->iterate($task, $state, null))); + self::assertSame(2, $this->listings['source']); + } + + public function testNextBeforeExecute(): void + { + [$task, $state] = $this->createTask([]); + + self::assertFalse($task->next($state)); + } + + public function testRequiredOptions(): void + { + $this->expectException(MissingOptionsException::class); + $task = new ListContentTask($this->createStorages()); + $task->initialize($this->createState(ListContentTask::class, [])); + } + + /** + * @param list $items + * + * @return list + */ + private function paths(array $items): array + { + $paths = array_map( + static fn (mixed $item): string => $item instanceof StorageAttributes ? "{$item->path()}:{$item->type()}" : '', + $items + ); + sort($paths); + + return $paths; + } + + /** + * @param array $options + * + * @return array{ListContentTask, ProcessState} + */ + private function createTask(array $options): array + { + $state = $this->createState(ListContentTask::class, $options + ['filesystem' => 'source']); + $task = new ListContentTask($this->createStorages()); + $task->initialize($state); + + return [$task, $state]; + } +} diff --git a/tests/Task/RemoveFileTaskTest.php b/tests/Task/RemoveFileTaskTest.php new file mode 100644 index 0000000..acaa0b1 --- /dev/null +++ b/tests/Task/RemoveFileTaskTest.php @@ -0,0 +1,131 @@ + */ + private array $logs = []; + + public function testRemoveInputFile(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b']); + + $this->execute([], 'a.txt'); + + self::assertSame(['b.txt'], $this->getFiles('source')); + self::assertSame(['info: Deleted input file a.txt'], $this->logs); + } + + public function testRemoveListOfFiles(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b', '0' => 'zero', 'c.txt' => 'c']); + + $this->execute([], ['a.txt', 'b.txt', '0']); + + self::assertSame(['c.txt'], $this->getFiles('source')); + } + + public function testMissingFile(): void + { + $this->createFiles('source', ['dir/a.txt' => 'a']); + + $this->execute([], ['missing.txt', 'dir']); + + self::assertSame(['warning: Input file not found missing.txt', 'warning: Input file not found dir'], $this->logs); + self::assertSame(['dir'], $this->getFiles('source')); + } + + public function testFilePattern(): void + { + $this->createFiles('source', ['a.txt' => 'a', 'b.txt' => 'b', 'c.csv' => 'c', 'dir.txt/d.txt' => 'd']); + + // The input is ignored + $this->execute(['file_pattern' => '/\.txt$/'], 'c.csv'); + + self::assertSame(['c.csv', 'dir.txt'], $this->getFiles('source')); + self::assertCount(2, $this->logs); + } + + public function testDeletionFailureIsLogged(): void + { + $filesystem = $this->createStub(FilesystemOperator::class); + $filesystem->method('fileExists')->willReturn(true); + $filesystem->method('delete')->willThrowException(UnableToDeleteFile::atLocation('a.txt')); + /** @var ServiceLocator $storages */ + $storages = new ServiceLocator(['source' => static fn (): FilesystemOperator => $filesystem]); + + $this->execute([], 'a.txt', $storages); + + self::assertSame(['warning: Failed to delete input file a.txt'], $this->logs); + } + + public function testNoPatternNorInput(): void + { + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('No pattern neither input provided for the Task'); + $this->execute([], null); + } + + public function testRequiredOptions(): void + { + $this->expectException(MissingOptionsException::class); + $task = new RemoveFileTask($this->createLogger(), $this->createStorages()); + $task->initialize($this->createState(RemoveFileTask::class, [])); + } + + /** + * @param array $options + * @param ServiceLocator|null $storages + */ + private function execute(array $options, mixed $input, ?ServiceLocator $storages = null): void + { + $state = $this->createState(RemoveFileTask::class, $options + ['filesystem' => 'source']); + $task = new RemoveFileTask($this->createLogger(), $storages ?? $this->createStorages()); + $task->initialize($state); + $state->setInput($input); + $task->execute($state); + } + + private function createLogger(): AbstractLogger + { + $onLog = function (string $log): void { + $this->logs[] = $log; + }; + + return new class($onLog) extends AbstractLogger { + public function __construct(private readonly \Closure $onLog) + { + } + + /** + * @param array $context + */ + public function log($level, string|\Stringable $message, array $context = []): void + { + $file = $context['file'] ?? ''; + ($this->onLog)(\sprintf('%s: %s %s', \is_scalar($level) ? $level : '', $message, \is_string($file) ? $file : '')); + } + }; + } +} diff --git a/tests/Task/StorageTestCase.php b/tests/Task/StorageTestCase.php new file mode 100644 index 0000000..55e368f --- /dev/null +++ b/tests/Task/StorageTestCase.php @@ -0,0 +1,141 @@ + Number of listings of each storage */ + protected array $listings = []; + + protected function setUp(): void + { + $this->dir = sys_get_temp_dir().'/'.uniqid('flysystem_task_test_', true); + mkdir($this->dir.'/source', 0o777, true); + mkdir($this->dir.'/destination', 0o777, true); + } + + protected function tearDown(): void + { + (new SymfonyFilesystem())->remove($this->dir); + } + + /** + * @param array $files Contents indexed by path (a numeric path such as "0" is an int key) + */ + protected function createFiles(string $storage, array $files): void + { + foreach ($files as $path => $content) { + (new SymfonyFilesystem())->dumpFile("{$this->dir}/{$storage}/{$path}", $content); + } + } + + /** + * @return list + */ + protected function getFiles(string $storage): array + { + $files = array_values(array_diff(scandir("{$this->dir}/{$storage}") ?: [], ['.', '..'])); + sort($files); + + return $files; + } + + /** + * @return ServiceLocator + */ + protected function createStorages(): ServiceLocator + { + $factories = []; + foreach (['source', 'destination'] as $name) { + $listings = &$this->listings; + $listings[$name] = 0; + $adapter = new class("{$this->dir}/{$name}", $listings[$name]) extends LocalFilesystemAdapter { + public function __construct(string $location, private int &$listings) + { + parent::__construct($location); + } + + #[\Override] + public function listContents(string $path, bool $deep): iterable + { + ++$this->listings; + + return parent::listContents($path, $deep); + } + }; + $filesystem = new Filesystem($adapter); + $factories[$name] = static fn (): FilesystemOperator => $filesystem; + } + + /** @var ServiceLocator $storages */ + $storages = new ServiceLocator($factories); + + return $storages; + } + + /** + * @param array $options + */ + protected function createState(string $class, array $options): ProcessState + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('task', $class, $options)); + + return $state; + } + + /** + * Execute the task for one input, then iterate until next() returns false, as the process manager does. + * + * @return list + */ + protected function iterate(AbstractConfigurableTask&IterableTaskInterface $task, ProcessState $state, mixed $input): array + { + $outputs = []; + $state->reset(true); + $state->setInput($input); + do { + $state->reset(false); + $task->execute($state); + if ($state->isSkipped()) { + break; + } + $outputs[] = $state->getOutput(); + } while ($task->next($state)); + + return $outputs; + } +}