#228 - Uploaded files: only delete the files written for the execution, keep them while a failed message will be retried

CHANGELOG.md

@@ -2,6 +2,7 @@ Latest
 ------
 
 ## Fixes
+* [#228](https://github.com/cleverage/ui-process-bundle/issues/228) `ProcessExecuteHandler`, `ProcessExecuteController` (`cleverage_ui_process_http_execute`): only the files written by the launch form or the HTTP API for the execution are deleted (#209), flagged by the new `deleteInput` argument of `ProcessExecuteMessage` (`false` by default): a file of the upload directory given by its path (HTTP API, text input of the launch form, schedule, default input) is no longer deleted. A message failing in a Messenger worker keeps its file while it will be retried or when it is sent to a failure transport (`messenger:failed:retry`), new internal `UploadedFileSubscriber`. The file uploaded with a request rejected by the HTTP API (422) is deleted. The messages queued by 3.4 have no flag: their file is no longer deleted by the worker, it may need a manual cleanup of the upload directory (`cleverage_ui_process.upload_directory`). Update documentation, add tests.
 * [#229](https://github.com/cleverage/ui-process-bundle/issues/229) Login throttling of the `main` firewall (#207, #222): the requests of the HTTP API (`HttpProcessExecutionAuthenticator`) are no longer counted, on every firewall with a login throttling (`LoginThrottlingPass` decorates its limiter, the firewall settings still apply to the login form): failed logins blocked a valid API token, and invalid tokens blocked the login form of the IP address. `login_throttling: false` in the application `main` firewall disables it (it was enabled again by the bundle). With `framework.rate_limiter: false`, an explicit error tells how to fix the configuration instead of "Parent definition "limiter" does not exist". `HttpProcessExecutionAuthenticator::onAuthenticationFailure()` answers the message key of the exception, instead of an empty message when there were too many attempts. Update documentation, add tests.
 
 v3.4

config/services/event_subscriber.yaml

@@ -9,3 +9,12 @@ services:
             - '@cleverage_ui_process.monolog_handler.doctrine_process'
             - '@cleverage_ui_process.manager.process_execution'
             - '@event_dispatcher'
+
+    cleverage_ui_process.event_subscriber.uploaded_file:
+        class: CleverAge\UiProcessBundle\EventSubscriber\UploadedFileSubscriber
+        public: false
+        tags:
+            - { name: 'kernel.event_subscriber' }
+        arguments:
+            - '@cleverage_ui_process.manager.uploaded_file'
+            # Failure transports: set by the UploadedFileSubscriberPass

config/services/message.yaml

@@ -17,5 +17,6 @@ services:
             - '@cleverage_process.manager.process'
             - '@cleverage_ui_process.monolog_handler.process'
             - '@cleverage_ui_process.manager.uploaded_file'
+            - '@cleverage_ui_process.event_subscriber.uploaded_file'
         tags: [messenger.message_handler]
 

docs/reference/01-bundle_configuration.md

@@ -97,7 +97,7 @@ Container parameters
 | Parameter                               | Default                                   | Description |
 |-----------------------------------------|-------------------------------------------|-------------|
 | `cleverage_ui_process.upload_directory` | `%kernel.project_dir%/var/storage/uploads` | Directory where files uploaded from the launch form or the [HTTP API](06-http_api.md) are stored before being passed as process input. |
-| `cleverage_ui_process.delete_uploaded_files` | `true` | Delete the uploaded file once the process has been executed (successfully or not), by the Messenger handler or by the HTTP API when the process is not queued. Only the files of `cleverage_ui_process.upload_directory` are deleted. Set it to `false` to keep them, e.g. when a process stores the path of its input. |
+| `cleverage_ui_process.delete_uploaded_files` | `true` | Delete the uploaded file once the process has been executed (successfully or not), by the Messenger handler or by the HTTP API when the process is not queued; the file of a request rejected by the HTTP API is also deleted. Only the files written by the bundle for the execution are deleted, never a file given by its path, even in `cleverage_ui_process.upload_directory`. A message failing in a Messenger worker keeps its file while it will be retried, or when it is sent to a failure transport (see [messenger](08-messenger.md#uploaded-files)). Set it to `false` to keep them, e.g. when a process stores the path of its input. |
 | `upload_directory`                      | `%cleverage_ui_process.upload_directory%` | Deprecated since 3.3, removed in 4.0: use `cleverage_ui_process.upload_directory`. When your application still sets it, its value is used, with a deprecation. If your application has its own `upload_directory` parameter (e.g. for another bundle), also set `cleverage_ui_process.upload_directory`: the bundle then leaves `upload_directory` alone, without deprecation. |
 
 It can be overridden in your application:

docs/reference/02-process_ui_options.md

@@ -86,7 +86,8 @@ The launch form (`/process/launch?process=<process_code>`) contains:
 When `entrypoint_type` is `file`, the uploaded file is saved to the `cleverage_ui_process.upload_directory` directory (see
 [bundle configuration](01-bundle_configuration.md#container-parameters)) with a random name keeping the original
 extension, and its absolute path is passed as process input. The file is deleted once the process has been executed,
-unless `cleverage_ui_process.delete_uploaded_files` is `false`.
+unless `cleverage_ui_process.delete_uploaded_files` is `false`. When `entrypoint_type` is `text`, a file path given as
+input is never deleted.
 
 Default values
 --------------

docs/reference/06-http_api.md

@@ -34,7 +34,7 @@ Parameters can be sent either as a JSON body, or as form data (`application/x-ww
 | Parameter | Type                      | Required | Default | Description                                                                                                                                                           |
 |-----------|---------------------------|:--------:|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `code`    | `string`                  |    **X** |         | Code of the process. It must exist and be public.                                                                                                                    |
-| `input`   | `string` or file          |          | `null`  | Process input. With form data, it can be an uploaded file: the file is saved in the `cleverage_ui_process.upload_directory` directory (see [bundle configuration](01-bundle_configuration.md#container-parameters)) and its path is passed as input; the file is deleted once the process has been executed, unless `cleverage_ui_process.delete_uploaded_files` is `false`. |
+| `input`   | `string` or file          |          | `null`  | Process input. With form data, it can be an uploaded file: the file is saved in the `cleverage_ui_process.upload_directory` directory (see [bundle configuration](01-bundle_configuration.md#container-parameters)) and its path is passed as input; the file is deleted once the process has been executed, or when the request is rejected (422), unless `cleverage_ui_process.delete_uploaded_files` is `false`. A file given by its path is never deleted. |
 | `context` | `object` or JSON `string` |          | `{}`    | Process context, as key/value pairs. A JSON string must decode to an object or an array (`422` otherwise). With form data, send one field per value: `context[key]=value`. |
 | `queue`   | `bool`                    |          | `true`  | `true`: the process is queued to the `execute_process` transport (see [messenger](08-messenger.md)). `false`: the process is executed during the HTTP request.      |
 

docs/reference/08-messenger.md

@@ -10,7 +10,7 @@ Messages
 
 | Message                                                | Transport         | Handler                                                                                                         |
 |--------------------------------------------------------|-------------------|-----------------------------------------------------------------------------------------------------------------|
-| `CleverAge\UiProcessBundle\Message\ProcessExecuteMessage` (`code`, `input`, `context`) | `execute_process` | Executes the process with the process manager, like `cleverage:process:execute`.                                |
+| `CleverAge\UiProcessBundle\Message\ProcessExecuteMessage` (`code`, `input`, `context`, `deleteInput`) | `execute_process` | Executes the process with the process manager, like `cleverage:process:execute`.                                |
 | `CleverAge\UiProcessBundle\Message\CronProcessMessage` (`processSchedule`)             | `scheduler_cron`  | Reads the schedule again, and dispatches a `ProcessExecuteMessage` with its current input and context, unless it has been disabled or deleted. |
 
 You can dispatch a `ProcessExecuteMessage` yourself to queue a process from your code:
@@ -27,6 +27,22 @@ public function importProducts(string $filePath): void
 }
 ```
 
+Uploaded files
+--------------
+
+The launch form and the HTTP API set `deleteInput` (`false` by default) when the input is the file they have just
+written in the upload directory: the file is deleted once the process has been executed, successfully or not (see the
+`cleverage_ui_process.delete_uploaded_files` [parameter](01-bundle_configuration.md#container-parameters)). In a
+worker, the file of a failed message is kept while the message will be retried (retry strategy of the transport), or
+when it is sent to a failure transport, so that `messenger:failed:retry` executes it with its file. It is deleted once
+the message no longer fails, or when it fails again from the failure transport and Symfony drops it (not sent again to
+the failure transport it has been received from). A file of a message removed from the failure transport
+(`messenger:failed:remove`) is kept. With Symfony 6.4.0 to 6.4.4, which sends a message failing again from the failure
+transport back to it, its file is deleted anyway.
+
+The messages queued by the version 3.4 of the bundle have no `deleteInput`: their file is not deleted, you may have to
+remove it from the upload directory.
+
 Transports
 ----------
 

src/CleverAgeUiProcessBundle.php

@@ -16,6 +16,7 @@
 use CleverAge\UiProcessBundle\DependencyInjection\Compiler\CheckDoctrineLazyGhostObjectsPass;
 use CleverAge\UiProcessBundle\DependencyInjection\Compiler\LoginThrottlingPass;
 use CleverAge\UiProcessBundle\DependencyInjection\Compiler\UploadDirectoryParameterPass;
+use CleverAge\UiProcessBundle\DependencyInjection\Compiler\UploadedFileSubscriberPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 
@@ -38,5 +39,6 @@ public function build(ContainerBuilder $container): void
         $container->addCompilerPass(new CheckDoctrineLazyGhostObjectsPass());
         $container->addCompilerPass(new UploadDirectoryParameterPass());
         $container->addCompilerPass(new LoginThrottlingPass());
+        $container->addCompilerPass(new UploadedFileSubscriberPass());
     }
 }

src/Controller/Admin/Process/LaunchAction.php

@@ -113,16 +113,19 @@ public function __invoke(
         $form->handleRequest($this->requestStack->getMainRequest());
         if ($form->isSubmitted() && $form->isValid()) {
             $input = $form->get('input')->getData();
+            // Only the file written here is deleted once executed, never a file given by its path (text input)
+            $deleteInput = false;
             if ($input instanceof UploadedFile) {
                 $filename = \sprintf('%s/%s.%s', $uploadDirectory, Uuid::v4(), $input->getClientOriginalExtension());
                 (new Filesystem())->dumpFile($filename, $input->getContent());
                 $input = $filename;
+                $deleteInput = true;
             }
             /** @var string $processCode */
             $processCode = $form->getConfig()->getOption('process_code');
             /** @var array<string, mixed> $context */
             $context = $form->get('context')->getData();
-            $this->dispatch($processCode, $input, $context);
+            $this->dispatch($processCode, $input, $context, $deleteInput);
             $this->addFlash(
                 'success',
                 'Process has been added to queue. It will start as soon as possible'
@@ -143,15 +146,16 @@ public function __invoke(
     /**
      * @param array<string, mixed> $context
      */
-    protected function dispatch(string $processCode, mixed $input = null, array $context = []): void
+    protected function dispatch(string $processCode, mixed $input = null, array $context = [], bool $deleteInput = false): void
     {
         $message = new ProcessExecuteMessage(
             $processCode,
             $input,
             array_merge(
                 ['execution_user' => $this->getUser()?->getEmail()],
                 $context
-            )
+            ),
+            $deleteInput
         );
         $this->messageBus->dispatch($message);
     }

src/Controller/ProcessExecuteController.php

@@ -16,11 +16,13 @@
 use CleverAge\ProcessBundle\Manager\ProcessManagerInterface;
 use CleverAge\UiProcessBundle\Entity\ProcessExecution;
 use CleverAge\UiProcessBundle\Http\Model\HttpProcessExecution;
+use CleverAge\UiProcessBundle\Http\ValueResolver\HttpProcessExecuteValueResolver;
 use CleverAge\UiProcessBundle\Manager\ProcessExecutionManager;
 use CleverAge\UiProcessBundle\Manager\UploadedFileManager;
 use CleverAge\UiProcessBundle\Message\ProcessExecuteMessage;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Attribute\ValueResolver;
 use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException;
@@ -47,13 +49,18 @@ public function __construct(
 
     public function __invoke(
         #[ValueResolver('http_process_execution')] HttpProcessExecution $httpProcessExecution,
+        Request $request,
     ): JsonResponse {
+        // Only the file written by the value resolver is deleted, never a file given by its path
+        $uploadedInput = $request->attributes->get(HttpProcessExecuteValueResolver::UPLOADED_INPUT_ATTRIBUTE);
+        $deleteInput = \is_string($uploadedInput) && $uploadedInput === $httpProcessExecution->input;
         $violations = $this->validator->validate($httpProcessExecution);
         if ($violations->count() > 0) {
             $violationsMessages = [];
             foreach ($violations as $violation) {
                 $violationsMessages[] = $violation->getMessage();
             }
+            $this->uploadedFileManager?->deleteInput($uploadedInput);
             throw new UnprocessableEntityHttpException(implode('. ', $violationsMessages));
         }
         /** @var array<string, mixed> $context */
@@ -65,7 +72,8 @@ public function __invoke(
                 new ProcessExecuteMessage(
                     $httpProcessExecution->code ?? '',
                     $httpProcessExecution->input,
-                    $context
+                    $context,
+                    $deleteInput
                 )
             );
 
@@ -87,7 +95,9 @@ public function __invoke(
 
             return new JsonResponse($message, Response::HTTP_INTERNAL_SERVER_ERROR);
         } finally {
-            $this->uploadedFileManager?->deleteInput($httpProcessExecution->input);
+            if ($deleteInput) {
+                $this->uploadedFileManager?->deleteInput($httpProcessExecution->input);
+            }
         }
 
         return new JsonResponse('Process has been proceed well.');

src/DependencyInjection/Compiler/UploadedFileSubscriberPass.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the CleverAge/UiProcessBundle 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\UiProcessBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * Gives the failure transports of the application to the UploadedFileSubscriber: the uploaded input of a message sent
+ * to a failure transport is kept, the message can be retried with messenger:failed:retry. Symfony only defines the
+ * SendFailedMessageToFailureTransportListener when a failure transport is configured.
+ *
+ * @internal
+ */
+final readonly class UploadedFileSubscriberPass implements CompilerPassInterface
+{
+    public const SUBSCRIBER = 'cleverage_ui_process.event_subscriber.uploaded_file';
+    public const FAILURE_LISTENER = 'messenger.failure.send_failed_message_to_failure_transport_listener';
+
+    public function process(ContainerBuilder $container): void
+    {
+        if (!$container->hasDefinition(self::SUBSCRIBER) || !$container->hasDefinition(self::FAILURE_LISTENER)) {
+            return;
+        }
+
+        $failureListenerArguments = $container->getDefinition(self::FAILURE_LISTENER)->getArguments();
+        $subscriber = $container->getDefinition(self::SUBSCRIBER)->setArgument(1, $failureListenerArguments[0] ?? null);
+        // Failure transport names by transport name: since Symfony 6.4.34 / 7.4.6 / 8.0.6
+        if (\array_key_exists(2, $failureListenerArguments)) {
+            $subscriber->setArgument(2, $failureListenerArguments[2]);
+        }
+    }
+}