NEXT-24720 - Fix mailer integration

pweyck · Jan 13, 2023 · 2951e25 · 2951e25
1 parent 5af8aa8
commit 2951e25
Show file tree

Hide file tree

Showing 12 changed files with 213 additions and 41 deletions.
diff --git a/changelog/_unreleased/2022-11-23-use-env-local-for-web-installer.md b/changelog/_unreleased/2022-11-23-use-env-local-for-web-installer.md
@@ -5,7 +5,17 @@ issue: NEXT-24091
 
 # Core
 
-* Renamed `SHOPWARE_ES_HOSTS` to `OPENSEARCH_URL` to use more generic environment variable name used by cloud providers.
+* Added support for multiple mailers defined in Symfony framework configuration
+
+___
+
+# Upgrade Information
+
+## Change of environment variables
+
+* Renamed following environment variables to use more generic environment variable name used by cloud providers:
+  * `SHOPWARE_ES_HOSTS` to `OPENSEARCH_URL`
+  * `MAILER_URL` to `MAILER_DSN`
 
 You can change this variable back in your installation using a `config/packages/elasticsearch.yaml` with
 

diff --git a/devenv.nix b/devenv.nix
@@ -89,5 +89,5 @@
   env.APP_SECRET = lib.mkDefault "devsecret";
   env.CYPRESS_baseUrl = lib.mkDefault "http://localhost:8000";
   env.DATABASE_URL = lib.mkDefault "mysql://root@localhost:3306/shopware";
-  env.MAILER_URL = lib.mkDefault "smtp://localhost:1025";
+  env.MAILER_DSN = lib.mkDefault "smtp://localhost:1025";
 }
diff --git a/src/Core/Content/Content.php b/src/Core/Content/Content.php
@@ -2,6 +2,7 @@
 
 namespace Shopware\Core\Content;
 
+use Shopware\Core\Content\Mail\MailerConfigurationCompilerPass;
 use Shopware\Core\Framework\Bundle;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -42,5 +43,7 @@ public function build(ContainerBuilder $container): void
         if ($container->getParameter('kernel.environment') === 'test') {
             $loader->load('services_test.xml');
         }
+
+        $container->addCompilerPass(new MailerConfigurationCompilerPass());
     }
 }
diff --git a/src/Core/Content/DependencyInjection/mail_template.xml b/src/Core/Content/DependencyInjection/mail_template.xml
@@ -83,11 +83,6 @@
             <argument type="service" id="document.repository"/>
         </service>
 
-        <service id="mailer.default_transport" class="Symfony\Component\Mailer\Transport\TransportInterface">
-            <argument type="string">%env(MAILER_URL)%</argument>
-            <factory service="Shopware\Core\Content\Mail\Service\MailerTransportLoader" method="fromString"/>
-        </service>
-
         <service id="Shopware\Core\Content\MailTemplate\Service\AttachmentLoader">
             <argument type="service" id="document.repository"/>
             <argument type="service" id="Shopware\Core\Checkout\Document\Service\DocumentGenerator"/>

diff --git a/src/Core/Content/Mail/MailerConfigurationCompilerPass.php b/src/Core/Content/Mail/MailerConfigurationCompilerPass.php
@@ -0,0 +1,29 @@
+<?php declare(strict_types=1);
+
+namespace Shopware\Core\Content\Mail;
+
+use Shopware\Core\Content\Mail\Service\MailerTransportLoader;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Reference;
+
+/**
+ * @package core
+ *
+ * @interal
+ */
+class MailerConfigurationCompilerPass implements CompilerPassInterface
+{
+    public function process(ContainerBuilder $container): void
+    {
+        $container->getDefinition('mailer.default_transport')->setFactory([
+            new Reference(MailerTransportLoader::class),
+            'fromString',
+        ]);
+
+        $container->getDefinition('mailer.transports')->setFactory([
+            new Reference(MailerTransportLoader::class),
+            'fromStrings',
+        ]);
+    }
+}
diff --git a/src/Core/Content/Mail/Service/MailerTransportLoader.php b/src/Core/Content/Mail/Service/MailerTransportLoader.php
@@ -2,13 +2,15 @@
 
 namespace Shopware\Core\Content\Mail\Service;
 
+use Doctrine\DBAL\Exception\DriverException;
 use League\Flysystem\FilesystemOperator;
 use Shopware\Core\Framework\DataAbstractionLayer\EntityRepository;
 use Shopware\Core\System\SystemConfig\SystemConfigService;
 use Symfony\Component\Mailer\Transport;
 use Symfony\Component\Mailer\Transport\Dsn;
 use Symfony\Component\Mailer\Transport\SendmailTransport;
 use Symfony\Component\Mailer\Transport\TransportInterface;
+use Symfony\Component\Mailer\Transport\Transports;
 
 /**
  * @internal
@@ -44,15 +46,34 @@ public function __construct(
         $this->documentRepository = $documentRepository;
     }
 
+    /**
+     * @param array<string, string> $dsns
+     */
+    public function fromStrings(array $dsns): Transports
+    {
+        $transports = [];
+        foreach ($dsns as $name => $dsn) {
+            if ($name === 'main') {
+                $transports[$name] = $this->fromString($dsn);
+            } else {
+                $transports[$name] = $this->createTransportUsingDSN($dsn);
+            }
+        }
+
+        return new Transports($transports);
+    }
+
     public function fromString(string $dsn): TransportInterface
     {
-        if (trim($this->configService->getString('core.mailerSettings.emailAgent')) === '') {
-            return new MailerTransportDecorator(
-                $this->envBasedTransport->fromString($dsn),
-                $this->attachmentsBuilder,
-                $this->filesystem,
-                $this->documentRepository
-            );
+        try {
+            $transportConfig = trim($this->configService->getString('core.mailerSettings.emailAgent'));
+
+            if ($transportConfig === '') {
+                return $this->createTransportUsingDSN($dsn);
+            }
+        } catch (DriverException $e) {
+            // We don't have a database connection right now
+            return $this->createTransportUsingDSN($dsn);
         }
 
         return new MailerTransportDecorator(
@@ -63,18 +84,25 @@ public function fromString(string $dsn): TransportInterface
         );
     }
 
+    public function createTransportUsingDSN(string $dsn): MailerTransportDecorator
+    {
+        return new MailerTransportDecorator(
+            $this->envBasedTransport->fromString($dsn),
+            $this->attachmentsBuilder,
+            $this->filesystem,
+            $this->documentRepository
+        );
+    }
+
     private function create(): TransportInterface
     {
         $emailAgent = $this->configService->getString('core.mailerSettings.emailAgent');
 
-        switch ($emailAgent) {
-            case 'smtp':
-                return $this->createSmtpTransport($this->configService);
-            case 'local':
-                return new SendmailTransport($this->getSendMailCommandLineArgument($this->configService));
-            default:
-                throw new \RuntimeException(sprintf('Invalid mail agent given "%s"', $emailAgent));
-        }
+        return match ($emailAgent) {
+            'smtp' => $this->createSmtpTransport($this->configService),
+            'local' => new SendmailTransport($this->getSendMailCommandLineArgument($this->configService)),
+            default => throw new \RuntimeException(sprintf('Invalid mail agent given "%s"', $emailAgent)),
+        };
     }
 
     private function createSmtpTransport(SystemConfigService $configService): TransportInterface
@@ -95,14 +123,11 @@ private function getEncryption(SystemConfigService $configService): ?string
     {
         $encryption = $configService->getString('core.mailerSettings.encryption');
 
-        switch ($encryption) {
-            case 'ssl':
-                return 'ssl';
-            case 'tls':
-                return 'tls';
-            default:
-                return null;
-        }
+        return match ($encryption) {
+            'ssl' => 'ssl',
+            'tls' => 'tls',
+            default => null,
+        };
     }
 
     private function getSendMailCommandLineArgument(SystemConfigService $configService): string

diff --git a/src/Core/Framework/Resources/config/packages/framework.yaml b/src/Core/Framework/Resources/config/packages/framework.yaml
@@ -2,7 +2,7 @@ parameters:
     messenger.default_transport_name: 'v65'
     env(MESSENGER_TRANSPORT_DSN): 'doctrine://default?auto_setup=false'
     env(MESSENGER_TRANSPORT_FAILURE_DSN): 'doctrine://default?queue_name=failed&auto_setup=false'
-    env(MAILER_URL): 'null://null'
+    env(MAILER_DSN): 'null://null'
 
 framework:
     esi: true
@@ -23,7 +23,7 @@ framework:
     http_method_override: true
     mailer:
         message_bus: false
-        dsn: '%env(MAILER_URL)%'
+        dsn: '%env(MAILER_DSN)%'
     php_errors:
         log: true
     cache:

diff --git a/src/Core/Maintenance/System/Command/SystemSetupCommand.php b/src/Core/Maintenance/System/Command/SystemSetupCommand.php
@@ -66,7 +66,7 @@ protected function configure(): void
             ->addOption('http-cache-enabled', null, InputOption::VALUE_OPTIONAL, 'Http-Cache enabled', $this->getDefault('SHOPWARE_HTTP_CACHE_ENABLED', '1'))
             ->addOption('http-cache-ttl', null, InputOption::VALUE_OPTIONAL, 'Http-Cache TTL', $this->getDefault('SHOPWARE_HTTP_DEFAULT_TTL', '7200'))
             ->addOption('cdn-strategy', null, InputOption::VALUE_OPTIONAL, 'CDN Strategy', $this->getDefault('SHOPWARE_CDN_STRATEGY_DEFAULT', 'id'))
-            ->addOption('mailer-url', null, InputOption::VALUE_OPTIONAL, 'Mailer URL', $this->getDefault('MAILER_URL', 'native://default'))
+            ->addOption('mailer-url', null, InputOption::VALUE_OPTIONAL, 'Mailer URL', $this->getDefault('MAILER_DSN', 'native://default'))
             ->addOption('dump-env', null, InputOption::VALUE_NONE, 'Dump the generated .env file in a optimized .env.local.php file, to skip parsing of the .env file on each request');
     }
 
@@ -85,7 +85,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             'SHOPWARE_HTTP_DEFAULT_TTL' => $input->getOption('http-cache-ttl'),
             'SHOPWARE_CDN_STRATEGY_DEFAULT' => $input->getOption('cdn-strategy'),
             'BLUE_GREEN_DEPLOYMENT' => $input->getOption('blue-green'),
-            'MAILER_URL' => $input->getOption('mailer-url'),
+            'MAILER_DSN' => $input->getOption('mailer-url'),
             'COMPOSER_HOME' => $input->getOption('composer-home'),
         ];
 

diff --git a/tests/migration/phpunit.xml b/tests/migration/phpunit.xml
@@ -23,7 +23,7 @@
 
         <env name="SHELL_VERBOSITY" value="-1" />
         <env name="SYMFONY_DEPRECATIONS_HELPER" value="disabled" />
-        <server name="MAILER_URL" value="null://localhost"/>
+        <server name="MAILER_DSN" value="null://localhost"/>
         <server name="HTTPS" value="off"/>
         <!--To see the full stackTrace of a Deprecation set the value to a regex matching the deprecation warning-->
         <!--https://symfony.com/doc/current/components/phpunit_bridge.html#display-the-full-stack-trace-->

diff --git a/tests/unit/php/Core/Content/Mail/MailerConfigurationCompilerPassTest.php b/tests/unit/php/Core/Content/Mail/MailerConfigurationCompilerPassTest.php
@@ -0,0 +1,49 @@
+<?php declare(strict_types=1);
+
+namespace Shopware\Tests\Unit\Core\Content\Mail;
+
+use PHPUnit\Framework\TestCase;
+use Shopware\Core\Content\Mail\MailerConfigurationCompilerPass;
+use Shopware\Core\Content\Mail\Service\MailerTransportLoader;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Reference;
+
+/**
+ * @internal
+ *
+ * @covers \Shopware\Core\Content\Mail\MailerConfigurationCompilerPass
+ */
+class MailerConfigurationCompilerPassTest extends TestCase
+{
+    public function testProcess(): void
+    {
+        $container = new ContainerBuilder();
+
+        $container->setDefinition('mailer.default_transport', new Definition(\ArrayObject::class));
+        $container->setDefinition('mailer.transports', new Definition(\ArrayObject::class));
+
+        $pass = new MailerConfigurationCompilerPass();
+        $pass->process($container);
+
+        $defaultTransport = $container->getDefinition('mailer.default_transport');
+
+        $factory = $defaultTransport->getFactory();
+        static::assertIsArray($factory);
+        static::assertArrayHasKey(0, $factory);
+        static::assertArrayHasKey(1, $factory);
+        static::assertInstanceOf(Reference::class, $factory[0]);
+        static::assertSame(MailerTransportLoader::class, (string) $factory[0]);
+        static::assertSame('fromString', $factory[1]);
+
+        $transports = $container->getDefinition('mailer.transports');
+
+        $factory = $transports->getFactory();
+        static::assertIsArray($factory);
+        static::assertArrayHasKey(0, $factory);
+        static::assertArrayHasKey(1, $factory);
+        static::assertInstanceOf(Reference::class, $factory[0]);
+        static::assertSame(MailerTransportLoader::class, (string) $factory[0]);
+        static::assertSame('fromStrings', $factory[1]);
+    }
+}
diff --git a/tests/unit/php/Core/Content/Mail/Service/MailerTransportLoaderTest.php b/tests/unit/php/Core/Content/Mail/Service/MailerTransportLoaderTest.php
@@ -2,14 +2,15 @@
 
 namespace Shopware\Tests\Unit\Core\Content\Mail\Service;
 
+use Doctrine\DBAL\Exception\DriverException;
 use League\Flysystem\FilesystemOperator;
-use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Shopware\Core\Content\Mail\Service\MailAttachmentsBuilder;
 use Shopware\Core\Content\Mail\Service\MailerTransportDecorator;
 use Shopware\Core\Content\Mail\Service\MailerTransportLoader;
 use Shopware\Core\Framework\DataAbstractionLayer\EntityRepository;
 use Shopware\Core\Framework\Test\TestCaseHelper\ReflectionHelper;
+use Shopware\Core\System\SystemConfig\SystemConfigService;
 use Shopware\Tests\Unit\Common\Stubs\SystemConfigService\ConfigService;
 use Symfony\Component\Mailer\Transport;
 use Symfony\Component\Mailer\Transport\AbstractTransportFactory;
@@ -148,20 +149,80 @@ public function testFactoryInvalidAgent(): void
         $loader->fromString('null://null');
     }
 
+    public function testFactoryNoConnection(): void
+    {
+        $config = $this->createMock(SystemConfigService::class);
+        $config->method('get')->willThrowException(DriverException::typeExists('no connection'));
+
+        $loader = new MailerTransportLoader(
+            $this->getTransportFactory(),
+            $config,
+            $this->createMock(MailAttachmentsBuilder::class),
+            $this->createMock(FilesystemOperator::class),
+            $this->createMock(EntityRepository::class)
+        );
+
+        $mailer = $loader->fromString('null://null');
+
+        static::assertInstanceOf(MailerTransportDecorator::class, $mailer);
+
+        $decorated = ReflectionHelper::getPropertyValue($mailer, 'decorated');
+
+        static::assertInstanceOf(Transport\NullTransport::class, $decorated);
+    }
+
+    public function testLoadMultipleMailers(): void
+    {
+        $loader = new MailerTransportLoader(
+            $this->getTransportFactory(),
+            new ConfigService([
+                'core.mailerSettings.emailAgent' => 'smtp',
+                'core.mailerSettings.host' => 'localhost',
+                'core.mailerSettings.port' => '225',
+                'core.mailerSettings.username' => 'root',
+                'core.mailerSettings.password' => 'root',
+                'core.mailerSettings.encryption' => 'foo',
+                'core.mailerSettings.authenticationMethod' => 'cram-md5',
+            ]),
+            $this->createMock(MailAttachmentsBuilder::class),
+            $this->createMock(FilesystemOperator::class),
+            $this->createMock(EntityRepository::class)
+        );
+
+        $dsns = [
+            'main' => 'null://localhost:25',
+            'fallback' => 'null://localhost:25',
+        ];
+
+        $transports = ReflectionHelper::getPropertyValue($loader->fromStrings($dsns), 'transports');
+        static::assertArrayHasKey('main', $transports);
+        static::assertArrayHasKey('fallback', $transports);
+
+        $mainMailer = $transports['main'];
+        static::assertInstanceOf(MailerTransportDecorator::class, $mainMailer);
+
+        $decorated = ReflectionHelper::getPropertyValue($mainMailer, 'decorated');
+        static::assertInstanceOf(EsmtpTransport::class, $decorated);
+
+        $fallbackMailer = $transports['fallback'];
+        static::assertInstanceOf(MailerTransportDecorator::class, $fallbackMailer);
+
+        $decorated = ReflectionHelper::getPropertyValue($fallbackMailer, 'decorated');
+        static::assertInstanceOf(Transport\NullTransport::class, $decorated);
+    }
+
     /**
      * @return array<string, AbstractTransportFactory>
      */
     private function getFactories(): array
     {
         return [
             'smtp' => new EsmtpTransportFactory(),
+            'null' => new Transport\NullTransportFactory(),
         ];
     }
 
-    /**
-     * @return mixed can't annotate more specific as phpstan does not allow to annotate as MockObject&Transport as Transport is final
-     */
-    private function getTransportFactory(): mixed
+    private function getTransportFactory(): Transport
     {
         return new Transport($this->getFactories());
     }