fix(EmailManager): Removed EmailManager and ContactFormManager direct…

… usage. Use their *Factory service to avoid sharing service data between async requests.
roadiz · Jul 5, 2024 · 80e25e5 · 80e25e5
1 parent f5289b8
commit 80e25e5
Show file tree

Hide file tree

Showing 25 changed files with 321 additions and 213 deletions.
diff --git a/lib/RoadizCompatBundle/src/Aliases.php b/lib/RoadizCompatBundle/src/Aliases.php
@@ -238,8 +238,6 @@ public static function getAliases(): array
             \RZ\Roadiz\CoreBundle\ListManager\Paginator::class => \RZ\Roadiz\Core\ListManagers\Paginator::class,
             \RZ\Roadiz\CoreBundle\ListManager\QueryBuilderListManager::class => \RZ\Roadiz\Core\ListManagers\QueryBuilderListManager::class,
             \RZ\Roadiz\CoreBundle\ListManager\TagListManager::class => \RZ\Roadiz\Core\ListManagers\TagListManager::class,
-            \RZ\Roadiz\CoreBundle\Mailer\ContactFormManager::class => \RZ\Roadiz\Utils\ContactFormManager::class,
-            \RZ\Roadiz\CoreBundle\Mailer\EmailManager::class => \RZ\Roadiz\Utils\EmailManager::class,
             \RZ\Roadiz\CoreBundle\Node\NodeDuplicator::class => \RZ\Roadiz\Utils\Node\NodeDuplicator::class,
             \RZ\Roadiz\CoreBundle\Node\NodeFactory::class => \RZ\Roadiz\Utils\Node\NodeFactory::class,
             \RZ\Roadiz\CoreBundle\Node\NodeMover::class => \RZ\Roadiz\Utils\Node\NodeMover::class,

diff --git a/lib/RoadizCompatBundle/src/Controller/Controller.php b/lib/RoadizCompatBundle/src/Controller/Controller.php
@@ -24,8 +24,6 @@
 use RZ\Roadiz\CoreBundle\Form\Error\FormErrorSerializer;
 use RZ\Roadiz\CoreBundle\ListManager\EntityListManager;
 use RZ\Roadiz\CoreBundle\ListManager\EntityListManagerInterface;
-use RZ\Roadiz\CoreBundle\Mailer\ContactFormManager;
-use RZ\Roadiz\CoreBundle\Mailer\EmailManager;
 use RZ\Roadiz\CoreBundle\Node\NodeFactory;
 use RZ\Roadiz\CoreBundle\Preview\PreviewResolverInterface;
 use RZ\Roadiz\CoreBundle\Repository\TranslationRepository;
@@ -37,6 +35,7 @@
 use RZ\Roadiz\Documents\UrlGenerators\DocumentUrlGeneratorInterface;
 use RZ\Roadiz\OpenId\OAuth2LinkGenerator;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Cmf\Component\Routing\RouteObjectInterface;
 use Symfony\Component\Form\Extension\Core\Type\FormType;
 use Symfony\Component\Form\FormBuilderInterface;
@@ -51,7 +50,6 @@
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
-use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
@@ -87,9 +85,7 @@ public static function getSubscribedServices(): array
             'translator' => TranslatorInterface::class,
             'urlGenerator' => UrlGeneratorInterface::class,
             UrlGeneratorInterface::class => UrlGeneratorInterface::class,
-            ContactFormManager::class => ContactFormManager::class,
             DocumentUrlGeneratorInterface::class => DocumentUrlGeneratorInterface::class,
-            EmailManager::class => EmailManager::class,
             Environment::class => Environment::class,
             FormErrorSerializer::class => FormErrorSerializer::class,
             LoggerInterface::class => LoggerInterface::class,

diff --git a/lib/RoadizCoreBundle/config/services.yaml b/lib/RoadizCoreBundle/config/services.yaml
@@ -347,6 +347,18 @@ services:
     RZ\Roadiz\CoreBundle\Mailer\EmailManager:
         # Recreate manager for each usage
         shared: false
+        deprecated:
+            message: 'The "%service_id%" service is deprecated and will be removed in Roadiz 3.0. Use RZ\Roadiz\CoreBundle\Mailer\EmailManagerFactory instead.'
+            package: roadiz/core-bundle
+            version: '2.3.18'
+
+    RZ\Roadiz\CoreBundle\Mailer\ContactFormManager:
+        # Recreate manager for each usage
+        shared: false
+        deprecated:
+            message: 'The "%service_id%" service is deprecated and will be removed in Roadiz 3.0. Use RZ\Roadiz\CoreBundle\Mailer\ContactFormManagerFactory instead.'
+            package: roadiz/core-bundle
+            version: '2.3.18'
 
     RZ\Roadiz\CoreBundle\Doctrine\EventSubscriber\:
         resource: '../src/Doctrine/EventSubscriber'

diff --git a/lib/RoadizCoreBundle/src/Console/MailerTestCommand.php b/lib/RoadizCoreBundle/src/Console/MailerTestCommand.php
@@ -4,7 +4,7 @@
 
 namespace RZ\Roadiz\CoreBundle\Console;
 
-use RZ\Roadiz\CoreBundle\Mailer\EmailManager;
+use RZ\Roadiz\CoreBundle\Mailer\EmailManagerFactory;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
@@ -16,7 +16,7 @@
 final class MailerTestCommand extends Command
 {
     public function __construct(
-        private readonly EmailManager $emailManager,
+        private readonly EmailManagerFactory $emailManagerFactory,
         ?string $name = null
     ) {
         parent::__construct($name);
@@ -37,7 +37,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $to = Address::create($input->getArgument('email'));
         $from = Address::create($input->getOption('from') ?? '[email protected]');
 
-        $this->emailManager
+        $this->emailManagerFactory->create()
             ->setReceiver($to)
             ->setSender($from)
             // Uses email_sender customizable setting

diff --git a/lib/RoadizCoreBundle/src/Controller/CustomFormController.php b/lib/RoadizCoreBundle/src/Controller/CustomFormController.php
@@ -160,8 +160,7 @@ public function addAction(Request $request, int $customFormId): Response
         );
 
         if ($mixed instanceof Response) {
-            $mixed->prepare($request);
-            return $mixed->send();
+            return $mixed;
         } else {
             return $this->render('@RoadizCore/customForm/customForm.html.twig', $mixed);
         }
@@ -232,6 +231,9 @@ public function prepareAndHandleCustomFormAssignation(
                     throw new \RuntimeException('Answer ID is null');
                 }
 
+                if (null === $emailSender || false === filter_var($answer->getEmail(), FILTER_VALIDATE_EMAIL)) {
+                    $emailSender = $answer->getEmail();
+                }
                 if (null === $emailSender || false === filter_var($emailSender, FILTER_VALIDATE_EMAIL)) {
                     $emailSender = $this->settingsBag->get('email_sender');
                 }

diff --git a/lib/RoadizCoreBundle/src/CustomForm/CustomFormHelper.php b/lib/RoadizCoreBundle/src/CustomForm/CustomFormHelper.php
@@ -97,81 +97,84 @@ public function parseAnswerFormData(
         CustomFormAnswer $answer = null,
         string $ipAddress = ""
     ): CustomFormAnswer {
-        if ($form->isSubmitted() && $form->isValid()) {
+        if (!$form->isSubmitted()) {
+            throw new \InvalidArgumentException('Form must be submitted before begin parsing.');
+        }
+        if (!$form->isValid()) {
+            throw new \InvalidArgumentException('Form must be validated before begin parsing.');
+        }
+
+        /*
+         * Create answer if null.
+         */
+        if (null === $answer) {
+            $answer = new CustomFormAnswer();
+            $answer->setCustomForm($this->customForm);
+            $this->em->persist($answer);
+        }
+        $answer->setSubmittedAt(new \DateTime());
+        $answer->setIp($ipAddress);
+        $documentsUploaded = [];
+
+        /** @var CustomFormField $customFormField */
+        foreach ($this->customForm->getFields() as $customFormField) {
+            $formField = null;
+            $fieldAttr = null;
+
             /*
-             * Create answer if null.
+             * Get data in form groups
              */
-            if (null === $answer) {
-                $answer = new CustomFormAnswer();
-                $answer->setCustomForm($this->customForm);
-                $this->em->persist($answer);
+            if ($customFormField->getGroupName() != '') {
+                $groupCanonical = StringHandler::slugify($customFormField->getGroupName());
+                $formGroup = $form->get($groupCanonical);
+                if ($formGroup->has($customFormField->getName())) {
+                    $formField = $formGroup->get($customFormField->getName());
+                    $fieldAttr = $this->getAttribute($answer, $customFormField);
+                }
+            } else {
+                if ($form->has($customFormField->getName())) {
+                    $formField = $form->get($customFormField->getName());
+                    $fieldAttr = $this->getAttribute($answer, $customFormField);
+                }
             }
-            $answer->setSubmittedAt(new \DateTime());
-            $answer->setIp($ipAddress);
-            $documentsUploaded = [];
-
-            /** @var CustomFormField $customFormField */
-            foreach ($this->customForm->getFields() as $customFormField) {
-                $formField = null;
-                $fieldAttr = null;
 
+            if (null !== $formField) {
+                $data = $formField->getData();
                 /*
-                 * Get data in form groups
-                 */
-                if ($customFormField->getGroupName() != '') {
-                    $groupCanonical = StringHandler::slugify($customFormField->getGroupName());
-                    $formGroup = $form->get($groupCanonical);
-                    if ($formGroup->has($customFormField->getName())) {
-                        $formField = $formGroup->get($customFormField->getName());
-                        $fieldAttr = $this->getAttribute($answer, $customFormField);
-                    }
-                } else {
-                    if ($form->has($customFormField->getName())) {
-                        $formField = $form->get($customFormField->getName());
-                        $fieldAttr = $this->getAttribute($answer, $customFormField);
-                    }
+                * Create attribute if null.
+                */
+                if (null === $fieldAttr) {
+                    $fieldAttr = new CustomFormFieldAttribute();
+                    $fieldAttr->setCustomFormAnswer($answer);
+                    $fieldAttr->setCustomFormField($customFormField);
+                    $this->em->persist($fieldAttr);
                 }
 
-                if (null !== $formField) {
-                    $data = $formField->getData();
-                    /*
-                    * Create attribute if null.
-                    */
-                    if (null === $fieldAttr) {
-                        $fieldAttr = new CustomFormFieldAttribute();
-                        $fieldAttr->setCustomFormAnswer($answer);
-                        $fieldAttr->setCustomFormField($customFormField);
-                        $this->em->persist($fieldAttr);
-                    }
-
-                    if (is_array($data) && isset($data[0]) && $data[0] instanceof UploadedFile) {
-                        /** @var UploadedFile $file */
-                        foreach ($data as $file) {
-                            $documentsUploaded[] = $this->handleUploadedFile($file, $fieldAttr);
-                        }
-                    } elseif ($data instanceof UploadedFile) {
-                        $documentsUploaded[] = $this->handleUploadedFile($data, $fieldAttr);
-                    } else {
-                        $fieldAttr->setValue($this->formValueToString($data));
+                if (is_array($data) && isset($data[0]) && $data[0] instanceof UploadedFile) {
+                    /** @var UploadedFile $file */
+                    foreach ($data as $file) {
+                        $documentsUploaded[] = $this->handleUploadedFile($file, $fieldAttr);
                     }
+                } elseif ($data instanceof UploadedFile) {
+                    $documentsUploaded[] = $this->handleUploadedFile($data, $fieldAttr);
+                } else {
+                    $fieldAttr->setValue($this->formValueToString($data));
                 }
             }
+        }
 
-            $this->em->flush();
+        $this->em->flush();
 
-            // Dispatch event on document uploaded
-            foreach ($documentsUploaded as $documentUploaded) {
-                if ($documentUploaded instanceof DocumentInterface) {
-                    $this->eventDispatcher->dispatch(new DocumentCreatedEvent($documentUploaded));
-                }
+        // Dispatch event on document uploaded
+        foreach ($documentsUploaded as $documentUploaded) {
+            if ($documentUploaded instanceof DocumentInterface) {
+                $this->eventDispatcher->dispatch(new DocumentCreatedEvent($documentUploaded));
             }
-
-            $this->em->refresh($answer);
-
-            return $answer;
         }
 
-        throw new \InvalidArgumentException('Form must be submitted and validated before begin parsing.');
+        $this->em->refresh($answer);
+
+        return $answer;
     }
 
     /**

diff --git a/lib/RoadizCoreBundle/src/CustomForm/Message/Handler/CustomFormAnswerNotifyMessageHandler.php b/lib/RoadizCoreBundle/src/CustomForm/Message/Handler/CustomFormAnswerNotifyMessageHandler.php
@@ -11,7 +11,7 @@
 use RZ\Roadiz\CoreBundle\Bag\Settings;
 use RZ\Roadiz\CoreBundle\CustomForm\Message\CustomFormAnswerNotifyMessage;
 use RZ\Roadiz\CoreBundle\Entity\CustomFormAnswer;
-use RZ\Roadiz\CoreBundle\Mailer\EmailManager;
+use RZ\Roadiz\CoreBundle\Mailer\EmailManagerFactory;
 use RZ\Roadiz\Documents\Models\DocumentInterface;
 use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
 use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
@@ -24,11 +24,11 @@
 final class CustomFormAnswerNotifyMessageHandler implements MessageHandlerInterface
 {
     public function __construct(
-        private ManagerRegistry $managerRegistry,
-        private EmailManager $emailManager,
-        private Settings $settingsBag,
-        private FilesystemOperator $documentsStorage,
-        private LoggerInterface $logger,
+        private readonly ManagerRegistry $managerRegistry,
+        private readonly EmailManagerFactory $emailManagerFactory,
+        private readonly Settings $settingsBag,
+        private readonly FilesystemOperator $documentsStorage,
+        private readonly LoggerInterface $messengerLogger,
     ) {
     }
 
@@ -51,12 +51,6 @@ public function __invoke(CustomFormAnswerNotifyMessage $message): void
             $answer->toArray(false)
         );
 
-        $receiver = array_filter(
-            array_map('trim', explode(',', $answer->getCustomForm()->getEmail() ?? ''))
-        );
-        $receiver = array_map(function (string $email) {
-            return new Address($email);
-        }, $receiver);
         $this->sendAnswer(
             $answer,
             [
@@ -65,60 +59,83 @@ public function __invoke(CustomFormAnswerNotifyMessage $message): void
                 'customForm' => $answer->getCustomForm(),
                 'title' => $message->getTitle(),
                 'requestLocale' => $message->getLocale(),
-            ],
-            $receiver
+            ]
+        );
+    }
+
+    /**
+     * @return Address[]
+     */
+    private function getCustomFormReceivers(CustomFormAnswer $answer): array
+    {
+        $receiver = array_filter(
+            array_map('trim', explode(',', $answer->getCustomForm()->getEmail() ?? ''))
         );
+        return array_map(function (string $email) {
+            return new Address($email);
+        }, $receiver);
     }
 
     /**
      * Send an answer form by Email.
      *
      * @param CustomFormAnswer $answer
      * @param array $assignation
-     * @param string|array|null $receiver
      * @throws TransportExceptionInterface
      * @throws LoaderError
      * @throws RuntimeError
      * @throws SyntaxError
      */
     private function sendAnswer(
         CustomFormAnswer $answer,
-        array $assignation,
-        $receiver
+        array $assignation
     ): void {
         $defaultSender = $this->settingsBag->get('email_sender');
-        $defaultSender = !empty($defaultSender) ? $defaultSender : '[email protected]';
-        $this->emailManager->setAssignation($assignation);
-        $this->emailManager->setEmailTemplate('@RoadizCore/email/forms/answerForm.html.twig');
-        $this->emailManager->setEmailPlainTextTemplate('@RoadizCore/email/forms/answerForm.txt.twig');
-        $this->emailManager->setSubject($assignation['title']);
-        $this->emailManager->setEmailTitle($assignation['title']);
-        $this->emailManager->setSender($defaultSender);
+        $defaultSender = filter_var($defaultSender, FILTER_VALIDATE_EMAIL) ? $defaultSender : '[email protected]';
+        $receivers = $this->getCustomFormReceivers($answer);
+
+        $realSender = filter_var($answer->getEmail(), FILTER_VALIDATE_EMAIL) ? $answer->getEmail() : $defaultSender;
+        $emailManager = $this->emailManagerFactory->create();
+        $emailManager->setAssignation($assignation);
+        $emailManager->setEmailTemplate('@RoadizCore/email/forms/answerForm.html.twig');
+        $emailManager->setEmailPlainTextTemplate('@RoadizCore/email/forms/answerForm.txt.twig');
+        $emailManager->setSubject($assignation['title']);
+        $emailManager->setEmailTitle($assignation['title']);
+        $emailManager->setSender($realSender);
 
         try {
             foreach ($answer->getAnswerFields() as $customFormAnswerAttr) {
                 /** @var DocumentInterface $document */
                 foreach ($customFormAnswerAttr->getDocuments() as $document) {
-                    $this->emailManager->addResource(
+                    $emailManager->addResource(
                         $this->documentsStorage->readStream($document->getMountPath()),
                         $document->getFilename(),
                         $this->documentsStorage->mimeType($document->getMountPath())
                     );
+                    $this->messengerLogger->debug(sprintf(
+                        'Joining document %s to email.',
+                        $document->getFilename()
+                    ));
                 }
             }
         } catch (FilesystemException $exception) {
-            $this->logger->error($exception->getMessage(), [
+            $this->messengerLogger->error($exception->getMessage(), [
                 'entity' => $answer
             ]);
         }
 
-        if (empty($receiver)) {
-            $this->emailManager->setReceiver($defaultSender);
+        if (empty($receivers)) {
+            $emailManager->setReceiver($defaultSender);
         } else {
-            $this->emailManager->setReceiver($receiver);
+            $emailManager->setReceiver($receivers);
         }
 
         // Send the message
-        $this->emailManager->send();
+        $emailManager->send();
+        $this->messengerLogger->debug(sprintf(
+            'CustomForm (%s) answer sent to %s',
+            $answer->getCustomForm()->getName(),
+            $realSender
+        ));
     }
 }