From 2951e25b0f923907bfa40ba9ac2cdb124dfc23f3 Mon Sep 17 00:00:00 2001 From: Soner Sayakci Date: Tue, 10 Jan 2023 16:50:51 +0100 Subject: [PATCH] NEXT-24720 - Fix mailer integration --- ...2-11-23-use-env-local-for-web-installer.md | 12 +++- devenv.nix | 2 +- src/Core/Content/Content.php | 3 + .../DependencyInjection/mail_template.xml | 5 -- .../Mail/MailerConfigurationCompilerPass.php | 29 ++++++++ .../Mail/Service/MailerTransportLoader.php | 71 +++++++++++++------ .../Resources/config/packages/framework.yaml | 4 +- .../System/Command/SystemSetupCommand.php | 4 +- tests/migration/phpunit.xml | 2 +- .../MailerConfigurationCompilerPassTest.php | 49 +++++++++++++ .../Service/MailerTransportLoaderTest.php | 71 +++++++++++++++++-- .../System/Command/SystemSetupCommandTest.php | 2 +- 12 files changed, 213 insertions(+), 41 deletions(-) create mode 100644 src/Core/Content/Mail/MailerConfigurationCompilerPass.php create mode 100644 tests/unit/php/Core/Content/Mail/MailerConfigurationCompilerPassTest.php 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 index 0d8e6880c7a..cafb5e844b8 100644 --- 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 index 9e8e2a8804e..6cf063b49cc 100644 --- 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 index 38dec365fcd..84180853097 100644 --- 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 index d2e830bb273..caf4659fe70 100644 --- a/src/Core/Content/DependencyInjection/mail_template.xml +++ b/src/Core/Content/DependencyInjection/mail_template.xml @@ -83,11 +83,6 @@ - - %env(MAILER_URL)% - - - diff --git a/src/Core/Content/Mail/MailerConfigurationCompilerPass.php b/src/Core/Content/Mail/MailerConfigurationCompilerPass.php new file mode 100644 index 00000000000..36d4c5323a8 --- /dev/null +++ b/src/Core/Content/Mail/MailerConfigurationCompilerPass.php @@ -0,0 +1,29 @@ +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 index 59e4ff9c0eb..e84ec06b64c 100644 --- a/src/Core/Content/Mail/Service/MailerTransportLoader.php +++ b/src/Core/Content/Mail/Service/MailerTransportLoader.php @@ -2,6 +2,7 @@ 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; @@ -9,6 +10,7 @@ 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 $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 index 6d640542255..b50ee6cf158 100644 --- 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 index 3d23da2b2d6..37ca8d88780 100644 --- 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 index e0a065af2c6..46c1c126002 100644 --- a/tests/migration/phpunit.xml +++ b/tests/migration/phpunit.xml @@ -23,7 +23,7 @@ - + diff --git a/tests/unit/php/Core/Content/Mail/MailerConfigurationCompilerPassTest.php b/tests/unit/php/Core/Content/Mail/MailerConfigurationCompilerPassTest.php new file mode 100644 index 00000000000..51742b161cd --- /dev/null +++ b/tests/unit/php/Core/Content/Mail/MailerConfigurationCompilerPassTest.php @@ -0,0 +1,49 @@ +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 index 23dbae34d4f..44683594791 100644 --- 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,6 +149,68 @@ 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 */ @@ -155,13 +218,11 @@ 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()); } diff --git a/tests/unit/php/Core/Maintenance/System/Command/SystemSetupCommandTest.php b/tests/unit/php/Core/Maintenance/System/Command/SystemSetupCommandTest.php index 7b8da649214..14fa6001073 100644 --- a/tests/unit/php/Core/Maintenance/System/Command/SystemSetupCommandTest.php +++ b/tests/unit/php/Core/Maintenance/System/Command/SystemSetupCommandTest.php @@ -72,7 +72,7 @@ public function testEnvFileGeneration(): void 'SHOPWARE_HTTP_DEFAULT_TTL' => '7200', 'SHOPWARE_CDN_STRATEGY_DEFAULT' => 'id', 'BLUE_GREEN_DEPLOYMENT' => '1', - 'MAILER_URL' => 'smtp://localhost:25', + 'MAILER_DSN' => 'smtp://localhost:25', 'COMPOSER_HOME' => __DIR__, ], $env); }