Normalize the file uploads in order to support all kinds of upload fo…

…rm fields (#83)
terminal42 · May 22, 2024 · fb1a86b · fb1a86b
1 parent ad424fb
commit fb1a86b
Show file tree

Hide file tree

Showing 10 changed files with 124 additions and 75 deletions.
diff --git a/composer.json b/composer.json
@@ -28,7 +28,7 @@
   "require": {
     "php": "^8.1",
     "contao/core-bundle": "^4.13 || ^5.0.8",
-    "codefog/contao-haste": "^5.0",
+    "codefog/contao-haste": "^5.2",
     "symfony/filesystem": "^5.4 || ^6.0",
     "symfony/var-dumper": "^5.4 || ^6.0"
   },

diff --git a/config/services.php b/config/services.php
@@ -4,6 +4,7 @@
 
 namespace Symfony\Component\DependencyInjection\Loader\Configurator;
 
+use Codefog\HasteBundle\FileUploadNormalizer;
 use Codefog\HasteBundle\UrlParser;
 use Terminal42\MultipageFormsBundle\Controller\FrontendModule\StepsController;
 use Terminal42\MultipageFormsBundle\EventListener\CompileFormFieldsListener;
@@ -57,6 +58,7 @@
         ->args([
             service(FormManagerFactoryInterface::class),
             service('request_stack'),
+            service(FileUploadNormalizer::class),
         ])
     ;
 

diff --git a/src/EventListener/CompileFormFieldsListener.php b/src/EventListener/CompileFormFieldsListener.php
@@ -70,8 +70,8 @@ public function __invoke(array $formFields, string $formId, Form $form): array
         // here! The problem with storing $_FILES across requests is that we would need to move
         // it from its tmp_name as PHP deletes files automatically after the request has
         // finished. We could indeed move them here but if we did at this stage the form fields
-        // themselves would later not be able to move them to their own desired place. So
-        // we cannot store any file information at this stage.
+        // themselves would later not be able to move them to their own desired place. So we
+        // cannot store any file information at this stage.
         if ($_POST) {
             $stepData = $stepData->withOriginalPostData(new ParameterBag($_POST));
             $manager->storeStepData($stepData);

diff --git a/src/EventListener/PrepareFomDataListener.php b/src/EventListener/PrepareFomDataListener.php
@@ -4,6 +4,7 @@
 
 namespace Terminal42\MultipageFormsBundle\EventListener;
 
+use Codefog\HasteBundle\FileUploadNormalizer;
 use Contao\CoreBundle\ContaoCoreBundle;
 use Contao\CoreBundle\DependencyInjection\Attribute\AsHook;
 use Contao\Form;
@@ -18,7 +19,8 @@ class PrepareFomDataListener
 {
     public function __construct(
         private readonly FormManagerFactoryInterface $formManagerFactory,
-        private readonly RequestStack $reqestStack,
+        private readonly RequestStack $requestStack,
+        private readonly FileUploadNormalizer $fileUploadNormalizer,
     ) {
     }
 
@@ -83,12 +85,12 @@ public function __invoke(array &$submitted, array &$labels, array $fields, Form
     private function getUploadedFiles($hook = []): FileParameterBag
     {
         // Contao 5
-        if (0 !== (is_countable($hook) ? \count($hook) : 0)) {
-            return new FileParameterBag($hook);
+        if (\is_array($hook) && [] !== $hook) {
+            return new FileParameterBag($this->fileUploadNormalizer->normalize($hook));
         }
 
         // Contao 4.13
-        $request = $this->reqestStack->getCurrentRequest();
+        $request = $this->requestStack->getCurrentRequest();
 
         if (null === $request) {
             return new FileParameterBag();

diff --git a/src/Step/FileParameterBag.php b/src/Step/FileParameterBag.php
@@ -6,6 +6,15 @@
 
 use Symfony\Component\Filesystem\Filesystem;
 
+/**
+ * This class expects $parameters to be in the format of.
+ *
+ * array<string, array<array{name: string, type: string, tmp_name: string, error:
+ * int, size: int, uploaded: bool, uuid: ?string, stream: ?resource}>>
+ *
+ * as provided by the FileUploadNormalizer service. Meaning that every file upload
+ * can contain multiple files.
+ */
 class FileParameterBag extends ParameterBag
 {
     /**
@@ -16,15 +25,20 @@ class FileParameterBag extends ParameterBag
      */
     public function set(string $name, mixed $value): self
     {
-        if (
-            \is_array($value)
-            && \array_key_exists('tmp_name', $value)
-            && \is_string($value['tmp_name'])
-            && is_uploaded_file($value['tmp_name'])
-        ) {
-            $target = (new Filesystem())->tempnam(sys_get_temp_dir(), 'nc');
-            move_uploaded_file($value['tmp_name'], $target);
-            $value['tmp_name'] = $target;
+        if (!\is_array($value)) {
+            throw new \InvalidArgumentException('$value must be an array normalized by the FileUploadNormalizer service.');
+        }
+
+        foreach ($value as $k => $upload) {
+            if (!\is_array($upload) && !\array_key_exists('tmp_name', $upload)) {
+                throw new \InvalidArgumentException('$value must be an array normalized by the FileUploadNormalizer service.');
+            }
+
+            if (is_uploaded_file($upload['tmp_name'])) {
+                $target = (new Filesystem())->tempnam(sys_get_temp_dir(), 'nc');
+                move_uploaded_file($upload['tmp_name'], $target);
+                $value[$k]['tmp_name'] = $target;
+            }
         }
 
         return parent::set($name, $value);

diff --git a/src/Step/StepData.php b/src/Step/StepData.php
@@ -10,7 +10,7 @@ private function __construct(
         private readonly int $step,
         private ParameterBag $submitted,
         private ParameterBag $labels,
-        private ParameterBag $files,
+        private FileParameterBag $files,
         private ParameterBag $originalPostData,
     ) {
     }
@@ -39,7 +39,7 @@ public function getLabels(): ParameterBag
         return $this->labels;
     }
 
-    public function getFiles(): ParameterBag
+    public function getFiles(): FileParameterBag
     {
         return $this->files;
     }
@@ -73,7 +73,7 @@ public function withSubmitted(ParameterBag $submitted): self
         return $clone;
     }
 
-    public function withFiles(ParameterBag $files): self
+    public function withFiles(FileParameterBag $files): self
     {
         $clone = clone $this;
         $clone->files = $files;
@@ -87,7 +87,7 @@ public static function create(int $step): self
             $step,
             new ParameterBag(),
             new ParameterBag(),
-            new ParameterBag(),
+            new FileParameterBag(),
             new ParameterBag(),
         );
     }

diff --git a/src/Widget/Placeholder.php b/src/Widget/Placeholder.php
@@ -82,7 +82,6 @@ public function generate(): void
     private function generateTokens(): array
     {
         $tokens = [];
-        $fileTokens = [];
         $summaryTokens = [];
 
         /** @var FormManagerFactoryInterface $factory */
@@ -91,60 +90,47 @@ private function generateTokens(): array
         /** @var StringParser $stringParser */
         $stringParser = System::getContainer()->get(StringParser::class);
 
-        /** @var UrlParser $urlParser */
-        $urlParser = System::getContainer()->get(UrlParser::class);
-
         $manager = $factory->forFormId((int) $this->pid);
 
         $stepsCollection = $manager->getDataOfAllSteps();
 
-        foreach ($stepsCollection->getAllSubmitted() as $k => $v) {
-            $stringParser->flatten($v, 'form_'.$k, $tokens);
-            $summaryTokens[$k]['value'] = $tokens['form_'.$k];
+        foreach ($stepsCollection->getAllSubmitted() as $formFieldName => $formFieldValue) {
+            $stringParser->flatten($formFieldValue, 'form_'.$formFieldName, $tokens);
+            $summaryTokens[$formFieldName]['value'] = $tokens['form_'.$formFieldName];
         }
 
-        foreach ($stepsCollection->getAllLabels() as $k => $v) {
-            $stringParser->flatten($v, 'formlabel_'.$k, $tokens);
-            $summaryTokens[$k]['label'] = $tokens['formlabel_'.$k];
+        foreach ($stepsCollection->getAllLabels() as $formFieldName => $formFieldValue) {
+            $stringParser->flatten($formFieldValue, 'formlabel_'.$formFieldName, $tokens);
+            $summaryTokens[$formFieldName]['label'] = $tokens['formlabel_'.$formFieldName];
         }
 
-        foreach ($stepsCollection->getAllFiles() as $k => $v) {
-            try {
-                $file = new File($v['tmp_name']);
-            } catch (FileNotFoundException $e) {
-                continue;
-            }
-
-            if (isset($_GET['summary_download']) && $k === $_GET['summary_download']) {
-                $binaryFileResponse = new BinaryFileResponse($file);
-                $binaryFileResponse->setContentDisposition(
-                    $this->mp_forms_downloadInline ? ResponseHeaderBag::DISPOSITION_INLINE : ResponseHeaderBag::DISPOSITION_ATTACHMENT,
-                    $file->getBasename(),
-                );
-
-                throw new ResponseException($binaryFileResponse);
+        foreach ($stepsCollection->getAllFiles() as $formFieldName => $normalizedFiles) {
+            $html = [];
+
+            foreach ($normalizedFiles as $k => $normalizedFile) {
+                try {
+                    $file = new File($normalizedFile['tmp_name']);
+                } catch (FileNotFoundException $e) {
+                    return [];
+                }
+
+                // Generate the tokens for the index (file 0, 1, 2, ...) and store the HTML per
+                // download for later
+                $tokens = array_merge($tokens, $this->generateFileTokens($file, 'file_'.$formFieldName.'_'.$k));
+                $html[] = $this->generateFileDownloadHtml($file, $this->generateAndHandleDownloadUrl($file, 'file_'.$formFieldName.'_'.$k));
+
+                // If we are at key 0 we also generate one non-indexed token for BC reasons and
+                // easier usage for single upload fields.
+                if (0 === $k) {
+                    $tokens = array_merge($tokens, $this->generateFileTokens($file, 'file_'.$formFieldName));
+                }
             }
 
-            $fileTokens['download_url'] = $urlParser->addQueryString('summary_download='.$k);
-            $fileTokens['extension'] = $file->getExtension();
-            $fileTokens['mime'] = $file->getMimeType();
-            $fileTokens['size'] = $file->getSize();
-
-            foreach ($fileTokens as $kk => $vv) {
-                $stringParser->flatten($vv, 'file_'.$k.'_'.$kk, $tokens);
-            }
-
-            // Generate a general HTML output using the download template
-            $tpl = new FrontendTemplate(empty($this->mp_forms_downloadTemplate) ? 'ce_download' : $this->mp_forms_downloadTemplate);
-            $tpl->link = $file->getBasename($file->getExtension());
-            $tpl->title = StringUtil::specialchars(sprintf($GLOBALS['TL_LANG']['MSC']['download'], $file->getBasename($file->getExtension())));
-            $tpl->href = $fileTokens['download_url'];
-            $tpl->filesize = System::getReadableSize($file->getSize());
-            $tpl->mime = $file->getMimeType();
-            $tpl->extension = $file->getExtension();
-
-            $stringParser->flatten($tpl->parse(), 'file_'.$k, $tokens);
-            $summaryTokens[$k]['value'] = $tokens['file_'.$k];
+            // Generate an HTML token (can contain multiple downloads) and add that as the
+            // default value for the "file_<formfield>" token and our summary for later
+            $htmlToken = implode(' ', $html);
+            $tokens['file_'.$formFieldName] = $htmlToken;
+            $summaryTokens[$formFieldName]['value'] = $htmlToken;
         }
 
         // Add a simple summary token that outputs label plus value for everything that
@@ -162,7 +148,7 @@ private function generateTokens(): array
             }
 
             $summaryToken[] = sprintf('<div data-ff-name="%s" class="label">%s</div>', htmlspecialchars($k), $v['label'] ?? '');
-            $summaryToken[] = sprintf('<div data-ff-name="%s" class="value">%s</div>', htmlspecialchars($k), $v['value'] ?? '');
+            $summaryToken[] = sprintf('<div data-ff-name="%s" class="value">%s</div>', htmlspecialchars($k), $v['value']);
         }
 
         $tokens['mp_forms_summary'] = implode("\n", $summaryToken);
@@ -184,4 +170,46 @@ private function generateTokens(): array
 
         return $tokens;
     }
+
+    private function generateFileTokens(File $file, string $tokenKey): array
+    {
+        $fileTokens = [];
+        $fileTokens[$tokenKey.'_download_url'] = $this->generateAndHandleDownloadUrl($file, $tokenKey);
+        $fileTokens[$tokenKey.'_extension'] = $file->getExtension();
+        $fileTokens[$tokenKey.'_mime'] = $file->getMimeType();
+        $fileTokens[$tokenKey.'_size'] = $file->getSize();
+
+        return $fileTokens;
+    }
+
+    private function generateAndHandleDownloadUrl(File $file, string $key): string
+    {
+        if (isset($_GET['summary_download']) && $key === $_GET['summary_download']) {
+            $binaryFileResponse = new BinaryFileResponse($file);
+            $binaryFileResponse->setContentDisposition(
+                $this->mp_forms_downloadInline ? ResponseHeaderBag::DISPOSITION_INLINE : ResponseHeaderBag::DISPOSITION_ATTACHMENT,
+                $file->getBasename(),
+            );
+
+            throw new ResponseException($binaryFileResponse);
+        }
+
+        $urlParser = System::getContainer()->get(UrlParser::class);
+
+        return $urlParser->addQueryString('summary_download='.$key);
+    }
+
+    private function generateFileDownloadHtml(File $file, string $downloadUrl): string
+    {
+        // Generate a general HTML output using the download template
+        $tpl = new FrontendTemplate(empty($this->mp_forms_downloadTemplate) ? 'ce_download' : $this->mp_forms_downloadTemplate);
+        $tpl->link = $file->getBasename($file->getExtension());
+        $tpl->title = StringUtil::specialchars(sprintf($GLOBALS['TL_LANG']['MSC']['download'], $file->getBasename($file->getExtension())));
+        $tpl->href = $downloadUrl;
+        $tpl->filesize = System::getReadableSize($file->getSize());
+        $tpl->mime = $file->getMimeType();
+        $tpl->extension = $file->getExtension();
+
+        return $tpl->parse();
+    }
 }
diff --git a/tests/EventListener/PrepareFormDataListenerTest.php b/tests/EventListener/PrepareFormDataListenerTest.php
@@ -4,6 +4,7 @@
 
 namespace Terminal42\MultipageFormsBundle\Test\EventListener;
 
+use Codefog\HasteBundle\FileUploadNormalizer;
 use Contao\CoreBundle\Exception\RedirectResponseException;
 use Contao\FormModel;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -33,7 +34,7 @@ public function testDataIsStoredProperlyAndDoesNotAdjustHookParametersIfNotOnLas
             1, // This mocks step=1 (page 2)
         );
 
-        $listener = new PrepareFomDataListener($factory, $this->createMock(RequestStack::class));
+        $listener = new PrepareFomDataListener($factory, $this->createMock(RequestStack::class), $this->createMock(FileUploadNormalizer::class));
 
         $submitted = ['submitted2' => 'foobar', 'mp_form_pageswitch' => 'continue'];
         $labels = [];
@@ -77,7 +78,7 @@ public function testDataIsStoredProperlyAndDoesAdjustHookParametersOnLastStep():
             2, // This mocks step=2 (page 3 - last page)
         );
 
-        $listener = new PrepareFomDataListener($factory, $this->createMock(RequestStack::class));
+        $listener = new PrepareFomDataListener($factory, $this->createMock(RequestStack::class), $this->createMock(FileUploadNormalizer::class));
 
         $submitted = ['submitted3' => 'foobar', 'mp_form_pageswitch' => 'continue'];
         $labels = [];

diff --git a/tests/Step/StepDataCollectionTest.php b/tests/Step/StepDataCollectionTest.php
@@ -25,14 +25,12 @@ public function testGetAll(): void
 
         $this->assertSame($expected, $stepCollection->getAllLabels());
         $this->assertSame($expected, $stepCollection->getAllSubmitted());
-        $this->assertSame($expected, $stepCollection->getAllFiles());
     }
 
     private function createStepData(int $step, ParameterBag $parameters): StepData
     {
         $step = StepData::create($step);
         $step = $step->withSubmitted($parameters);
-        $step = $step->withFiles($parameters);
 
         return $step->withLabels($parameters);
     }

diff --git a/tests/Step/StepDataTest.php b/tests/Step/StepDataTest.php
@@ -5,6 +5,7 @@
 namespace Terminal42\MultipageFormsBundle\Test\Step;
 
 use PHPUnit\Framework\TestCase;
+use Terminal42\MultipageFormsBundle\Step\FileParameterBag;
 use Terminal42\MultipageFormsBundle\Step\ParameterBag;
 use Terminal42\MultipageFormsBundle\Step\StepData;
 
@@ -25,14 +26,17 @@ public function testSubmitted(array $data): void
         $this->assertTrue($parameters->equals($stepData->getSubmitted()));
     }
 
-    /**
-     * @dataProvider parametersDataProvider
-     */
-    public function testFiles(array $data): void
+    public function testFiles(): void
     {
         $stepData = StepData::create(1);
 
-        $parameters = new ParameterBag($data);
+        $parameters = new FileParameterBag([
+            'file_upload' => [
+                [
+                    'tmp_name' => '/tmp/file.tmp',
+                ],
+            ],
+        ]);
 
         $this->assertTrue($stepData->getFiles()->empty());