Skip to content

Commit 0beb17a

Browse files
bug symfony#59596 [Mime] use isRendered method to ensure we can avoid rendering an email twice (walva)
This PR was merged into the 6.4 branch. Discussion ---------- [Mime] use `isRendered` method to ensure we can avoid rendering an email twice | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes? Not sure | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Issues | | License | MIT # Description: After upgrading my project from Symfony 5.4 to 6.4, due this [change](https://github.com/symfony/symfony/pull/47075/files), I encountered an issue where asynchronous email sending failed due to serialization problems with `TemplatedEmail`. According to the [Symfony documentation](https://symfony.com/doc/current/mailer.html), when sending an email asynchronously, the email instance must be serializable. While Mailer instances are inherently serializable, `TemplatedEmail `requires that its context be serializable. If the context contains non-serializable variables, such as Doctrine entities, it's recommended to either replace them with serializable variables or render the email before calling `$mailer->send($email)`. To address this, I decorated the Symfony `Mailer` to force the rendering of emails using the `BodyRendererInterface::render(Message $message): void` method. However, this approach led to an exception indicating that the context was empty. The method successfully rendered the email, clearing its context, but the same `TemplatedEmail `object was rendered again later in the execution process, specifically in the [MessageListener](https://github.com/symfony/mailer/blob/6.4/EventListener/MessageListener.php#L72). To prevent this double rendering, I utilized the `TemplatedEmail::isRendered(): bool` method, as seen in the [AbstractTransport](https://github.com/symfony/symfony/blob/7.3/src/Symfony/Component/Mailer/Transport/AbstractTransport.php#L83). This ensures that if an email is already rendered, it won't be rendered again, preserving the context and preventing serialization issues. ## Solution The proposed fix involves checking if the `TemplatedEmail` has already been rendered before attempting to render it again. By using the `isRendered()` method, we can avoid unnecessary re-rendering, which can lead to empty contexts and serialization problems during asynchronous email sending. ## Impact This correction allows classes extending `TemplatedEmail` to be sent asynchronously without encountering errors related to double rendering or context loss due to serialization issues, as outlined in the documentation. ## References [Symfony Mailer Documentation](https://symfony.com/doc/current/mailer.html) [MessageListener Implementation](https://github.com/symfony/mailer/blob/6.4/EventListener/MessageListener.php#L72) [AbstractTransport Implementation](https://github.com/symfony/symfony/blob/7.3/src/Symfony/Component/Mailer/Transport/AbstractTransport.php#L83) This pull request aims to enhance the robustness of asynchronous email handling in Symfony by ensuring that TemplatedEmail instances are not rendered multiple times, thereby maintaining their context and serializability. ## Original message: Following up my comment on https://github.com/symfony/symfony/pull/47075/files. I would recommend using the method TemplatedEmail::isRendered() the same way it is used in [AbstractTransport.php](https://github.com/symfony/symfony/blob/7.3/src/Symfony/Component/Mailer/Transport/AbstractTransport.php#L83). Keeping like this prevents subclass of TemplatedEmail with computed html template to not be rendered twice, such as [NotificationEmail](https://github.com/symfony/symfony/blob/7.3/src/Symfony/Bridge/Twig/Mime/NotificationEmail.php#L202). This change will allow developers to create a subclass of template emails and render them before sending them async, without rendering them twice. Commits ------- 2d37c79 [Mime] use isRendered method to avoid rendering an email twice
2 parents 2537633 + 2d37c79 commit 0beb17a

File tree

2 files changed

+5
-1
lines changed

2 files changed

+5
-1
lines changed

src/Symfony/Bridge/Twig/Mime/BodyRenderer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function render(Message $message): void
4545
return;
4646
}
4747

48-
if (null === $message->getTextTemplate() && null === $message->getHtmlTemplate()) {
48+
if ($message->isRendered()) {
4949
// email has already been rendered
5050
return;
5151
}

src/Symfony/Bridge/Twig/Tests/Mime/BodyRendererTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,14 @@ public function testRenderedOnce()
105105
;
106106
$email->textTemplate('text');
107107

108+
$this->assertFalse($email->isRendered());
108109
$renderer->render($email);
110+
$this->assertTrue($email->isRendered());
111+
109112
$this->assertEquals('Text', $email->getTextBody());
110113

111114
$email->text('reset');
115+
$this->assertTrue($email->isRendered());
112116

113117
$renderer->render($email);
114118
$this->assertEquals('reset', $email->getTextBody());

0 commit comments

Comments
 (0)