fix(task) #15 ZipTask and UnzipTask resolve their options on every execution

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

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": {

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.

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.

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()));

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()));

tests/Task/UnzipTaskTest.php

@@ -0,0 +1,104 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ArchiveProcessBundle 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\ArchiveProcessBundle\Tests\Task;
+
+use CleverAge\ArchiveProcessBundle\Task\UnzipTask;
+use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
+use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
+use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
+use CleverAge\ProcessBundle\Model\ProcessHistory;
+use CleverAge\ProcessBundle\Model\ProcessState;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Filesystem\Filesystem;
+
+#[CoversClass(UnzipTask::class)]
+class UnzipTaskTest extends TestCase
+{
+    private string $dir;
+
+    protected function setUp(): void
+    {
+        $this->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<string, mixed> $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();
+    }
+}

tests/Task/ZipTaskTest.php

@@ -0,0 +1,112 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/ArchiveProcessBundle 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\ArchiveProcessBundle\Tests\Task;
+
+use CleverAge\ArchiveProcessBundle\Task\ZipTask;
+use CleverAge\ProcessBundle\Configuration\ProcessConfiguration;
+use CleverAge\ProcessBundle\Configuration\TaskConfiguration;
+use CleverAge\ProcessBundle\Context\ContextualOptionResolver;
+use CleverAge\ProcessBundle\Model\ProcessHistory;
+use CleverAge\ProcessBundle\Model\ProcessState;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Filesystem\Filesystem;
+
+#[CoversClass(ZipTask::class)]
+class ZipTaskTest extends TestCase
+{
+    private string $dir;
+
+    protected function setUp(): void
+    {
+        $this->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<string, mixed> $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<string>
+     */
+    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;
+    }
+}