Skip to content

Commit

Permalink
Merge branch 'next-39778/auto-imported-from-github' into 'trunk'
Browse files Browse the repository at this point in the history
NEXT-39778 - Improve mail sender using private fs over mq

See merge request shopware/6/product/platform!15384
  • Loading branch information
Gaitholabi committed Dec 10, 2024
2 parents 0480f49 + 8b4f001 commit e573cbb
Show file tree
Hide file tree
Showing 10 changed files with 436 additions and 75 deletions.
10 changes: 10 additions & 0 deletions changelog/_unreleased/2024-11-14-improve-mail-sender-fs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
title: Improve mail sender by using the private file system over message queue
issue: NEXT-00000
author: Benjamin Wittwer
author_email: [email protected]
author_github: akf-bw
---
# Core
* Deprecated the `envelope` parameter in `Shopware\Core\Content\Mail\Service\AbstractMailSender::send`
* Changed `\Shopware\Core\Content\Mail\Service\MailSender` to write the serialized mail to the private file system and dispatch a `\Shopware\Core\Content\Mail\Message\SendMailMessage` to the message bus instead of directly sending the mail to the Symfony mailer
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1155,11 +1155,6 @@ parameters:
count: 1
path: src/Core/Content/Mail/Service/AbstractMailService.php

-
message: "#^Throwing new exceptions within classes are not allowed\\. Please use domain exception pattern\\. See https\\://github\\.com/shopware/platform/blob/v6\\.4\\.20\\.0/adr/2022\\-02\\-24\\-domain\\-exceptions\\.md$#"
count: 1
path: src/Core/Content/Mail/Service/MailSender.php

-
message: "#^Throwing new exceptions within classes are not allowed\\. Please use domain exception pattern\\. See https\\://github\\.com/shopware/platform/blob/v6\\.4\\.20\\.0/adr/2022\\-02\\-24\\-domain\\-exceptions\\.md$#"
count: 1
Expand Down
33 changes: 21 additions & 12 deletions src/Core/Content/DependencyInjection/mail_template.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<!-- Template Entities -->
<service id="Shopware\Core\Content\MailTemplate\MailTemplateDefinition">
<tag name="shopware.entity.definition" entity="mail_template"/>
</service>
Expand All @@ -24,14 +25,13 @@
<tag name="shopware.entity.definition"/>
</service>

<service id="Shopware\Core\Content\Mail\Service\MailSender" public="true">
<argument type="service" id="mailer.mailer"/>
<argument type="service" id="Shopware\Core\System\SystemConfig\SystemConfigService"/>
<argument>%shopware.mail.max_body_length%</argument>
<!-- Header Footer Entities -->
<service id="Shopware\Core\Content\MailTemplate\Aggregate\MailHeaderFooter\MailHeaderFooterDefinition">
<tag name="shopware.entity.definition"/>
</service>

<service id="Shopware\Core\Content\Mail\Service\MailFactory" public="true">
<argument type="service" id="validator"/>
<service id="Shopware\Core\Content\MailTemplate\Aggregate\MailHeaderFooterTranslation\MailHeaderFooterTranslationDefinition">
<tag name="shopware.entity.definition"/>
</service>

<!-- Controller -->
Expand All @@ -43,16 +43,25 @@
</call>
</service>

<!-- Header Footer Entities -->
<service id="Shopware\Core\Content\MailTemplate\Aggregate\MailHeaderFooter\MailHeaderFooterDefinition">
<tag name="shopware.entity.definition"/>
<!-- Sending -->
<service id="Shopware\Core\Content\Mail\Message\SendMailHandler">
<argument type="service" id="mailer.transports"/>
<argument type="service" id="shopware.filesystem.private"/>
<argument type="service" id="logger"/>
<tag name="messenger.message_handler"/>
</service>

<service id="Shopware\Core\Content\MailTemplate\Aggregate\MailHeaderFooterTranslation\MailHeaderFooterTranslationDefinition">
<tag name="shopware.entity.definition"/>
<service id="Shopware\Core\Content\Mail\Service\MailSender" public="true">
<argument type="service" id="messenger.default_bus"/>
<argument type="service" id="shopware.filesystem.private"/>
<argument type="service" id="Shopware\Core\System\SystemConfig\SystemConfigService"/>
<argument>%shopware.mail.max_body_length%</argument>
</service>

<service id="Shopware\Core\Content\Mail\Service\MailFactory" public="true">
<argument type="service" id="validator"/>
</service>

<!-- Header Footer Repository -->
<service id="Shopware\Core\Content\Mail\Service\MailService">
<argument type="service" id="Shopware\Core\Framework\Validation\DataValidator" />
<argument type="service" id="Shopware\Core\Framework\Adapter\Twig\StringTemplateRenderer" />
Expand Down
71 changes: 71 additions & 0 deletions src/Core/Content/Mail/Message/SendMailHandler.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php declare(strict_types=1);

namespace Shopware\Core\Content\Mail\Message;

use League\Flysystem\FilesystemException;
use League\Flysystem\FilesystemOperator;
use Psr\Log\LoggerInterface;
use Shopware\Core\Framework\Log\Package;
use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
use Symfony\Component\Mailer\Transport\TransportInterface;
use Symfony\Component\Messenger\Attribute\AsMessageHandler;
use Symfony\Component\Mime\Email;

/**
* @internal
*/
#[AsMessageHandler(handles: SendMailMessage::class)]
#[Package('services-settings')]
final class SendMailHandler
{
/**
* @internal
*/
public function __construct(
private readonly TransportInterface $transport,
private readonly FilesystemOperator $filesystem,
private readonly LoggerInterface $logger
) {
}

/**
* @throws TransportExceptionInterface
* @throws FilesystemException
*/
public function __invoke(SendMailMessage $message): void
{
$mailDataPath = $message->mailDataPath;
try {
$mailData = $this->filesystem->read($mailDataPath);
} catch (FilesystemException $e) {
if (!$this->filesystem->fileExists($mailDataPath)) {
$this->logger->error('The mail data file does not exist. Mail could not be sent.', ['mailDataPath' => $mailDataPath, 'exception' => $e->getMessage()]);

return;
}

throw $e;
}

$mail = unserialize($mailData);
if (!is_a($mail, Email::class)) {
$this->logger->error('The mail data file does not contain a valid email object. Mail could not be sent.', ['mailDataPath' => $mailDataPath]);

return;
}

$this->transport->send($mail);
$this->cleanup($message);
}

private function cleanup(SendMailMessage $message): void
{
$mailDataPath = $message->mailDataPath;

try {
$this->filesystem->delete($mailDataPath);
} catch (FilesystemException $e) {
$this->logger->error('Could not delete mail data file after sending mail.', ['mailDataPath' => $mailDataPath, 'exception' => $e->getMessage()]);
}
}
}
20 changes: 20 additions & 0 deletions src/Core/Content/Mail/Message/SendMailMessage.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php declare(strict_types=1);

namespace Shopware\Core\Content\Mail\Message;

use Shopware\Core\Framework\Log\Package;
use Shopware\Core\Framework\MessageQueue\AsyncMessageInterface;

/**
* @codeCoverageIgnore
*/
#[Package('services-settings')]
class SendMailMessage implements AsyncMessageInterface
{
/**
* @internal
*/
public function __construct(public readonly string $mailDataPath)
{
}
}
2 changes: 2 additions & 0 deletions src/Core/Content/Mail/Service/AbstractMailSender.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ abstract class AbstractMailSender
abstract public function getDecorated(): AbstractMailSender;

/**
* @deprecated tag:v6.7.0 - Parameter $envelope will be removed
*
* @throws MailTransportFailedException
*/
abstract public function send(Email $email, ?Envelope $envelope = null): void;
Expand Down
29 changes: 17 additions & 12 deletions src/Core/Content/Mail/Service/MailSender.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,31 @@

namespace Shopware\Core\Content\Mail\Service;

use League\Flysystem\FilesystemOperator;
use Shopware\Core\Content\Mail\MailException;
use Shopware\Core\Content\MailTemplate\Exception\MailTransportFailedException;
use Shopware\Core\Content\Mail\Message\SendMailMessage;
use Shopware\Core\Framework\Feature;
use Shopware\Core\Framework\Log\Package;
use Shopware\Core\Framework\Plugin\Exception\DecorationPatternException;
use Shopware\Core\Framework\Util\Hasher;
use Shopware\Core\System\SystemConfig\SystemConfigService;
use Symfony\Component\Mailer\Envelope;
use Symfony\Component\Mailer\MailerInterface;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\Mime\Email;

#[Package('services-settings')]
class MailSender extends AbstractMailSender
{
public const DISABLE_MAIL_DELIVERY = 'core.mailerSettings.disableDelivery';

private const BASE_FILE_SYSTEM_PATH = 'mail-data/';

/**
* @internal
*/
public function __construct(
private readonly MailerInterface $mailer,
private readonly MessageBusInterface $messageBus,
private readonly FilesystemOperator $filesystem,
private readonly SystemConfigService $configService,
private readonly int $maxContentLength,
) {
Expand All @@ -31,12 +37,11 @@ public function getDecorated(): AbstractMailSender
throw new DecorationPatternException(self::class);
}

/**
* @throws MailTransportFailedException
*/
public function send(Email $email, ?Envelope $envelope = null): void
{
$failedRecipients = [];
if ($envelope) {
Feature::triggerDeprecationOrThrow('v6.7.0.0', 'The parameter $envelope is deprecated and will be removed.');
}

$disabled = $this->configService->get(self::DISABLE_MAIL_DELIVERY);

Expand All @@ -53,10 +58,10 @@ public function send(Email $email, ?Envelope $envelope = null): void
throw MailException::mailBodyTooLong($this->maxContentLength);
}

try {
$this->mailer->send($email, $envelope);
} catch (\Throwable $e) {
throw new MailTransportFailedException($failedRecipients, $e);
}
$mailData = serialize($email);
$mailDataPath = self::BASE_FILE_SYSTEM_PATH . Hasher::hash($mailData);

$this->filesystem->write($mailDataPath, $mailData);
$this->messageBus->dispatch(new SendMailMessage($mailDataPath));
}
}
88 changes: 88 additions & 0 deletions tests/integration/Core/Content/Mail/Service/EmailSenderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<?php declare(strict_types=1);

namespace Shopware\Tests\Integration\Core\Content\Mail\Service;

use League\Flysystem\FilesystemOperator;
use PHPUnit\Framework\TestCase;
use Shopware\Core\Content\Mail\Service\MailFactory;
use Shopware\Core\Content\Mail\Service\MailSender;
use Shopware\Core\Framework\Test\TestCaseBase\KernelLifecycleManager;
use Shopware\Core\Framework\Test\TestCaseBase\KernelTestBehaviour;
use Shopware\Core\Framework\Test\TestCaseBase\QueueTestBehaviour;
use Shopware\Core\Framework\Util\Hasher;
use Symfony\Component\Mailer\Transport\TransportInterface;
use Symfony\Component\Mime\Email;

/**
* @internal
*/
class EmailSenderTest extends TestCase
{
use KernelTestBehaviour;
use QueueTestBehaviour;

public function testSendEmail(): void
{
try {
$this->doRunTest();
} finally {
// test updates container state, reset everything.
KernelLifecycleManager::ensureKernelShutdown();
}
}

private function doRunTest(): void
{
// other tests might have already booted the kernel...
KernelLifecycleManager::ensureKernelShutdown();
$container = static::getContainer();
$transport = $this->createMock(TransportInterface::class);
$container->set('mailer.transports', $transport);
$mailFactory = $container->get(MailFactory::class);
static::assertInstanceOf(MailFactory::class, $mailFactory);
$filesystem = $container->get('shopware.filesystem.private');
static::assertInstanceOf(FilesystemOperator::class, $filesystem);

$subject = 'mail create test';
$sender = ['[email protected]' => 'Sales Channel'];
$recipients = ['[email protected]' => 'Receiver name', '[email protected]' => null];
$contents = ['text/html' => 'Message'];
$attachments = ['test'];

$additionalData = [
'recipientsCc' => '[email protected]',
'recipientsBcc' => [
'[email protected]' => 'bccMailRecipient1',
'[email protected]' => 'bccMailRecipient2',
],
];
$binAttachments = [['content' => 'Content', 'fileName' => 'content.txt', 'mimeType' => 'application/txt']];

$mail = $mailFactory->create(
$subject,
$sender,
$recipients,
$contents,
$attachments,
$additionalData,
$binAttachments
);

$mailSender = $container->get(MailSender::class);
$serializedMail = serialize($mail);
$expectedMailPath = 'mail-data/' . Hasher::hash($serializedMail);
$transport->expects(static::once())
->method('send')
->with(
static::callback(
fn (Email $email) => $email->getSubject() === $mail->getSubject() && $email->getHtmlBody() === $mail->getHtmlBody()
)
);

$mailSender->send($mail);
static::assertSame($serializedMail, $filesystem->read($expectedMailPath));

$this->runWorker();
static::assertFalse($filesystem->fileExists($expectedMailPath));
}
}
Loading

0 comments on commit e573cbb

Please sign in to comment.