-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger] Uses a messenger serializer, not an individual encoder/decoder #28405
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
Conversation
268e650
to
29d887f
Compare
@@ -62,7 +62,6 @@ | |||
|
|||
<service id="messenger.transport.amqp.factory" class="Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory"> | |||
<argument type="service" id="messenger.transport.encoder" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably need to be changed to be messenger.transport.serializer
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
/** | ||
* @author Samuel Roze <samuel.roze@gmail.com> | ||
*/ | ||
interface SerializerInterface extends DecoderInterface, EncoderInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we need to keep the decoder and encoder interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed them 👍
95134f2
to
b2ebd79
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with minor comment
|
||
/** | ||
* Encodes an envelope content (message & items) to a common format understandable by transports. | ||
* The encoded array should only contain scalar and arrays. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scalars and arrays
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sroze Can you add an entry in the component CHANGELOG? And perhaps a note in UPGRADE file?
b2ebd79
to
5b93f5f
Compare
I've added changes on both CHANGELOG (Messenger & FWB) + the UPGRADE-4.2 👍 |
Thank you @sroze. |
…dual encoder/decoder (sroze) This PR was merged into the 4.2-dev branch. Discussion ---------- [Messenger] Uses a messenger serializer, not an individual encoder/decoder | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | ø | License | MIT | Doc PR | Will come Makes the component-based even simpler. **Before** ```php $encoderDecoder = Serializer::create(); $middleware = [new SendMessageMiddleware(new SenderLocator([ Message::class => new AmqpTransport($encoderDecoder, $encoderDecoder, $connection), ]))]; ``` **After** ```php $middleware = [new SendMessageMiddleware(new SenderLocator([ Message::class => new AmqpTransport(Serializer::create(), $connection), ]))]; ``` Commits ------- 5b93f5f Uses a messenger serializer, not an individual encoder/decoder
…erializer default id (ogizanagi) This PR was merged into the 4.2 branch. Discussion ---------- [FrameworkBundle][Messenger] Restore check for messenger serializer default id | Q | A | ------------- | --- | Branch? | 4.2 <!-- see below --> | Bug fix? | yes | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | https://symfony-devs.slack.com/archives/C9PQ75TV3/p1543590611003500 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | N/A From Slack: > @adamquaile [4:10 PM] > So, I just updated to 4.2 today, and got this message: > > The default Messenger serializer cannot be enabled as the Serializer support is not available. Try enabling it or running "composer require symfony/serializer-pack" > > In the docs it's stated: > > In order to use Symfony's built-in AMQP transport, you will need the Serializer Component. Ensure that it is installed with: > >But I haven't yet configured AMQP - I'm using my own transport. Should I be getting this exception? --- This check was removed in #28405, but is actually still necessary to not fail as soon as you can install the Messenger component without the Serializer one installed. Commits ------- 1cf17c0 [FrameworkBundle][Messenger] Restore check for messenger serializer default id
Makes the component-based even simpler.
Before
After