Skip to content

[Mailer] Pass cloned message and envelope to MessageBus #37324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

FabriZZio
Copy link

Q A
Branch? 5.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets no
License MIT
Doc PR no

Recent changes in symfony/mailer@f62d7eb#diff-bb5be03f312bba64e0ce7d4cbbdbf7a8 (avoiding reuse of same var names when cloning) introduced an issue where, if an EventDispatcher is available, the cloned message is not dispatched to the MessageBus anymore.

Instead, the original message is dispatched, resulting in listener behaviour (e.g. \Symfony\Component\Mailer\EventListener\MessageListener) not affecting the message passed to the MessageBus.

This PR passes the cloned messages back to the original $message variable.

Alternatively, the original commit by @fabpot could be reverted. Changes in this PR however make it more explicit.

@fabpot
Copy link
Member

fabpot commented Aug 16, 2020

This change is the same as reverting my original commit, which would revert the fix. Closing.

@fabpot fabpot closed this Aug 16, 2020
@FabriZZio
Copy link
Author

@fabpot, thank you for your feedback. I agree it will revert your original commit, but I think that commit introduced the actual issue.

The original message gets cloned before passing it to the EventDispatcher (your fix), but the "enriched" message does not get passed to the MessageBus.
This way, listeners like \Symfony\Component\Mailer\EventListener\MessageListener setting the headers and/or message content do not affect the original message, resulting in a RawMessage instance with an empty message body passed to the MessageBus.

Why should the MessageBus receive the original message and not the cloned "enriched" one?

Thanks for trying to help me understand!

@fabpot
Copy link
Member

fabpot commented Aug 17, 2020

The idea is that you always get the event just before sending the email and another one with the queued argument set to true when pushing to the queue. That allows to gather data for tests and the web profiler.

Regarding "enriching", it should be done just before the email is triggered regardless of the fact that the queue is used or not. The added benefit is that we serialize a data object, which is very lightweight (see the PR tests for an example).

fabpot added a commit that referenced this pull request Aug 17, 2020
…pot)

This PR was merged into the 5.2-dev branch.

Discussion
----------

[Serializer][Mime] Fix Mime message serialization

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | yes
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #37414, Fix #37324 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | n/a

Symfony serialization is used by Messenger to serialize Emails. By Email messages are data objects with some logic to prepare emails to be sent. Without configuration, the Symfony Serializer serializes Emails with too many data (and triggers some unneeded validation).

This PR aims to fix the above issue and at the same time makes serialized emails as small as possible and as readable as possible.

Commits
-------

9d869b1 Fix Mime message serialization
hultberg pushed a commit to hultberg/symfony that referenced this pull request Sep 17, 2021
…on (fabpot)

This PR was merged into the 5.2-dev branch.

Discussion
----------

[Serializer][Mime] Fix Mime message serialization

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | yes
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix symfony#37414, Fix symfony#37324 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | n/a

Symfony serialization is used by Messenger to serialize Emails. By Email messages are data objects with some logic to prepare emails to be sent. Without configuration, the Symfony Serializer serializes Emails with too many data (and triggers some unneeded validation).

This PR aims to fix the above issue and at the same time makes serialized emails as small as possible and as readable as possible.

Commits
-------

9d869b1 Fix Mime message serialization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants