From 4ac9e58c69f71c12cd0b0bfa09e0dc323d45a7c8 Mon Sep 17 00:00:00 2001 From: Nicolas Joubert Date: Wed, 30 Sep 2026 15:23:04 +0200 Subject: [PATCH] fix(task) #15 ZipTask and UnzipTask resolve their options on every execution, so that each input is used Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 + composer.json | 1 + docs/reference/tasks/unzip_task.md | 5 +- docs/reference/tasks/zip_task.md | 6 +- src/Task/UnzipTask.php | 3 +- src/Task/ZipTask.php | 3 +- tests/Task/UnzipTaskTest.php | 104 +++++++++++++++++++++++++++ tests/Task/ZipTaskTest.php | 112 +++++++++++++++++++++++++++++ 8 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 tests/Task/UnzipTaskTest.php create mode 100644 tests/Task/ZipTaskTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index ead438c..a640c37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ Latest ------ +### Fixes +* [#15](https://github.com/cleverage/archive-process-bundle/issues/15) Fix ZipTask and UnzipTask: resolve the options on every execution, so that each input is used (the options of the first input were reused for the following ones). Update documentation, add tests. + v2.1 ------ diff --git a/composer.json b/composer.json index b0e4205..ea24137 100644 --- a/composer.json +++ b/composer.json @@ -54,6 +54,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/unzip_task.md b/docs/reference/tasks/unzip_task.md index 8634abc..213e1a2 100644 --- a/docs/reference/tasks/unzip_task.md +++ b/docs/reference/tasks/unzip_task.md @@ -78,7 +78,6 @@ Notes archive is extracted, existing files with the same name are overwritten and other files already present in the destination directory are kept. * A `\RuntimeException` is thrown if the file cannot be opened as a zip archive. -* Options are resolved (and cached) on the first execution of the task: when the task receives several inputs during - the same process execution (e.g. after an iterable task), the values of the first input are reused for the following - ones. +* Options are resolved on each execution of the task: when the task receives several inputs (e.g. after an iterable + task), each input is extracted with its own `filename` and `destination`. * See the [Import the CSV files of an uploaded archive](../../cookbooks/import_archive.md) cookbook. diff --git a/docs/reference/tasks/zip_task.md b/docs/reference/tasks/zip_task.md index b70ec5f..6f76b48 100644 --- a/docs/reference/tasks/zip_task.md +++ b/docs/reference/tasks/zip_task.md @@ -95,9 +95,9 @@ Notes filesystem root), and the entries keep their full path (without the leading `/`) inside the archive. * Each file must exist and be readable, otherwise an `\UnexpectedValueException` is thrown. Only files can be added: directories are not browsed. -* Options are resolved (and cached) on the first execution of the task: when the task receives several inputs during - the same process execution (e.g. after an iterable task), the values of the first input are reused for the following - ones. To archive a list of files, aggregate them first (e.g. with an +* Options are resolved on each execution of the task: when the task receives several inputs (e.g. after an iterable + task), each input creates its own archive. To archive a list of files in a single archive, aggregate them first (e.g. + with an [AggregateIterableTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/aggregate_iterable_task.md)) and send them all at once in the `files` key. * See the [Export, archive and upload a file](../../cookbooks/export_archive_upload.md) cookbook. diff --git a/src/Task/UnzipTask.php b/src/Task/UnzipTask.php index b80cca7..4a48ca3 100644 --- a/src/Task/UnzipTask.php +++ b/src/Task/UnzipTask.php @@ -68,7 +68,8 @@ protected function configureOptions(OptionsResolver $resolver): void #[\Override] protected function getOptions(ProcessState $state): ?array { - if (null === $this->options && \is_array($state->getInput())) { + // The options depend on the input: resolve them on every execution + if (\is_array($state->getInput())) { $resolver = new OptionsResolver(); $this->configureOptions($resolver); $this->options = $resolver->resolve(array_merge($state->getContextualizedOptions() ?? [], $state->getInput())); diff --git a/src/Task/ZipTask.php b/src/Task/ZipTask.php index cd2c7df..962ad7d 100644 --- a/src/Task/ZipTask.php +++ b/src/Task/ZipTask.php @@ -78,7 +78,8 @@ protected function configureOptions(OptionsResolver $resolver): void #[\Override] protected function getOptions(ProcessState $state): ?array { - if (null === $this->options && \is_array($state->getInput())) { + // The options depend on the input: resolve them on every execution + if (\is_array($state->getInput())) { $resolver = new OptionsResolver(); $this->configureOptions($resolver); $this->options = $resolver->resolve(array_merge($state->getContextualizedOptions() ?? [], $state->getInput())); diff --git a/tests/Task/UnzipTaskTest.php b/tests/Task/UnzipTaskTest.php new file mode 100644 index 0000000..590ddea --- /dev/null +++ b/tests/Task/UnzipTaskTest.php @@ -0,0 +1,104 @@ +dir = sys_get_temp_dir().'/'.uniqid('unzip_task_test_', true); + mkdir($this->dir); + $this->createArchive($this->dir.'/archive1.zip', 'file1.txt'); + $this->createArchive($this->dir.'/archive2.zip', 'file2.txt'); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->dir); + } + + public function testUnzip(): void + { + [$task, $state] = $this->createTask([ + 'filename' => $this->dir.'/archive1.zip', + 'destination' => $this->dir.'/destination', + ]); + + self::assertSame($this->dir.'/destination', $this->execute($task, $state, null)); + self::assertFileExists($this->dir.'/destination/file1.txt'); + } + + public function testEachInputIsUsed(): void + { + [$task, $state] = $this->createTask([]); + + $output1 = $this->execute($task, $state, ['filename' => $this->dir.'/archive1.zip', 'destination' => $this->dir.'/destination1']); + $output2 = $this->execute($task, $state, ['filename' => $this->dir.'/archive2.zip', 'destination' => $this->dir.'/destination2']); + + self::assertSame($this->dir.'/destination1', $output1); + self::assertSame($this->dir.'/destination2', $output2); + self::assertFileExists($this->dir.'/destination1/file1.txt'); + self::assertFileExists($this->dir.'/destination2/file2.txt'); + self::assertFileDoesNotExist($this->dir.'/destination1/file2.txt'); + } + + /** + * @param array $options + * + * @return array{UnzipTask, ProcessState} + */ + private function createTask(array $options): array + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('unzip', UnzipTask::class, $options)); + + $task = new UnzipTask(); + $task->initialize($state); + + return [$task, $state]; + } + + private function execute(UnzipTask $task, ProcessState $state, mixed $input): mixed + { + $state->reset(false); + $state->setInput($input); + $task->execute($state); + + return $state->getOutput(); + } + + private function createArchive(string $filename, string $entry): void + { + $zip = new \ZipArchive(); + self::assertTrue($zip->open($filename, \ZipArchive::CREATE)); + $zip->addFromString($entry, 'content of '.$entry); + $zip->close(); + } +} diff --git a/tests/Task/ZipTaskTest.php b/tests/Task/ZipTaskTest.php new file mode 100644 index 0000000..1f0af0e --- /dev/null +++ b/tests/Task/ZipTaskTest.php @@ -0,0 +1,112 @@ +dir = sys_get_temp_dir().'/'.uniqid('zip_task_test_', true); + mkdir($this->dir); + file_put_contents($this->dir.'/file1.txt', 'file 1'); + file_put_contents($this->dir.'/file2.txt', 'file 2'); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->dir); + } + + public function testZipFiles(): void + { + [$task, $state] = $this->createTask([ + 'filename' => $this->dir.'/archive.zip', + 'files' => [$this->dir.'/file1.txt', 'file2.txt'], + 'files_base_path' => $this->dir, + ]); + + self::assertSame($this->dir.'/archive.zip', $this->execute($task, $state, null)); + self::assertSame(['file1.txt', 'file2.txt'], $this->getEntries($this->dir.'/archive.zip')); + } + + public function testEachInputIsUsed(): void + { + [$task, $state] = $this->createTask(['files_base_path' => $this->dir]); + + $output1 = $this->execute($task, $state, ['filename' => $this->dir.'/archive1.zip', 'files' => 'file1.txt']); + $output2 = $this->execute($task, $state, ['filename' => $this->dir.'/archive2.zip', 'files' => 'file2.txt']); + + self::assertSame($this->dir.'/archive1.zip', $output1); + self::assertSame($this->dir.'/archive2.zip', $output2); + self::assertSame(['file1.txt'], $this->getEntries($this->dir.'/archive1.zip')); + self::assertSame(['file2.txt'], $this->getEntries($this->dir.'/archive2.zip')); + } + + /** + * @param array $options + * + * @return array{ZipTask, ProcessState} + */ + private function createTask(array $options): array + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->setContextualOptionResolver(new ContextualOptionResolver()); + $state->setContext([]); + $state->setTaskConfiguration(new TaskConfiguration('zip', ZipTask::class, $options)); + + $task = new ZipTask(); + $task->initialize($state); + + return [$task, $state]; + } + + private function execute(ZipTask $task, ProcessState $state, mixed $input): mixed + { + $state->reset(false); + $state->setInput($input); + $task->execute($state); + + return $state->getOutput(); + } + + /** + * @return list + */ + private function getEntries(string $filename): array + { + $zip = new \ZipArchive(); + self::assertTrue($zip->open($filename)); + $entries = []; + for ($i = 0; $i < $zip->numFiles; ++$i) { + $entries[] = (string) $zip->getNameIndex($i); + } + $zip->close(); + + return $entries; + } +}