[Messenger] Add the Envelope in the documentation #9757

sroze · 2018-05-10T15:05:24Z

Because we've merged this beautiful PR :)

weaverryan

I made some minor tweaks (please check to make sure I didn't change anything important - was just meant to be language tweaks. And, 2 questions

weaverryan · 2018-05-10T15:32:42Z

components/messenger.rst

+    {
+        public function handle($message, callable $next)
+        {
+            // $message here is an `Envelope` object


is it always an Envelope? Or maybe it could be an Envolope?

It's always an Envelope if you implements the interface.

Note: that's true considering our bus implementation which will always inject the message wrapped in an Envelope. But we currently also use Envelope::wrap in envelope aware middleware and most tests are calling handle on envelope aware middleware with a message directly, not an envelope.
We could remove Envelope::wrap and fix tests of these middleware, but IMHO it's still another hint symfony/symfony#27322 is needed for clarity & better design.

weaverryan · 2018-05-10T15:34:22Z

components/messenger.rst

+
+            return $next(
+                $message->with(new AnotherEnvelopeItem(/* ... */))
+            );


Is this the main use-case for an envelop-aware middleware? Or would we more commonly be reading some configuration from the envelope?

Both are good use cases tbh

I think reading would be way more common and should be showcased instead. I would put a note about being extra cautious about altering the envelope or its message as it'll be completely transparent. Even if there are legit use-cases.

About altering/replacing the message, the tactician docs says:

You don’t have to pass the same command to $next that came in! If you’re working with a legacy system and want to convert or modify the incoming commands, you’re free to do so. Just think carefully!

Also showing an example where we read the envelope items would make the purpose of those items more understandable.

ogizanagi

I as about to (try to) work on this. Thanks for taking care of it ❤️

Should we update the schema in order to represent that receivers get a message wrapped into an envelope and senders send an Envelope too?
Adding an Envelope entry below it would be helpful too (with a link to this new section).

ogizanagi · 2018-05-11T10:53:27Z

components/messenger.rst

+        ]))
+    );
+
+Instead of dealing directly with the message in the handlers or middleware, you


handlers will always get the message (and I think it should stay as is as the end of the pipe), never the Envelope, so I think mentioning handlers here is misleading.

ogizanagi · 2018-05-11T10:57:27Z

components/messenger.rst

+
+            return $next(
+                $message->with(new AnotherEnvelopeItem(/* ... */))
+            );


I think reading would be way more common and should be showcased instead. I would put a note about being extra cautious about altering the envelope or its message as it'll be completely transparent. Even if there are legit use-cases.

About altering/replacing the message, the tactician docs says:

You don’t have to pass the same command to $next that came in! If you’re working with a legacy system and want to convert or modify the incoming commands, you’re free to do so. Just think carefully!

Also showing an example where we read the envelope items would make the purpose of those items more understandable.

ogizanagi · 2018-05-11T11:04:04Z

components/messenger.rst

-It will wrap the received messages into ``ReceivedMessage`` objects and the
-``SendMessageMiddleware`` middleware will know it should not route these
-messages again to a transport.
+It will add a ``ReceivedMessage`` configuration to the message envelopes and the


- configuration + marker item

?

ogizanagi · 2018-05-11T11:12:27Z

components/messenger.rst

@@ -188,6 +235,6 @@ Receiver and Sender on the same bus

 To allow us to receive and send messages on the same bus and prevent an infinite
 loop, the message bus is equipped with the ``WrapIntoReceivedMessage`` middleware.


Actually, the WrapIntoReceivedMessage is neither a middleware, neither it is used in Symfony core. That's the Worker that call the receive method with a $handleradding the ReceivedMessage marker to the envelope.

Hence I wondered multiple times if it was really intended? Should we remove the WrapIntoReceivedMessage decorator? Should we decorate in the core any created receiver? Should the worker only dispatch the received envelope to the bus, or keep adding ReceivedMessage?

ogizanagi · 2018-05-11T11:21:57Z

components/messenger.rst

+            'groups' => ['my_serialization_groups'],
+        ]))
+    );
+


We could mention others built-in configurations available (just ValidationConfiguration for now) here or in another section.

Just did 👍

sroze

Updated :)

sroze · 2018-06-03T14:50:24Z

components/messenger.rst

+            'groups' => ['my_serialization_groups'],
+        ]))
+    );
+


Just did 👍

sroze · 2018-06-03T14:53:02Z

@weaverryan can you have another look please?

weaverryan · 2018-06-07T12:47:01Z

components/messenger.rst

+        {
+            // $message here is an `Envelope` object, because this middleware
+            // implements the EnvelopeAwareInterface interface. Otherwise,
+            // it would be the "original" message.


I think we should remove the last sentence. I think it's obvious that, if you do NOT implement ``EnvelopeAwareInterface`, that you receive the normal message. So, this sentence made me wonder if you were actually saying something different.

weaverryan · 2018-06-07T12:50:12Z

components/messenger.rst

+            // implements the EnvelopeAwareInterface interface. Otherwise,
+            // it would be the "original" message.
+
+            if (null !== $message->get(ReceivedMessage::class)) {


Is this get() method something that lives on Envelope? Isn't there always just one message inside an Envelope? If so, why do we pass an argument here? Or, could you also pass something like $message->get(SerializerConfiguration::class) to get those types of things out?

Also, suppose some similar code: if $message->get(MyMessage::class) is null, does it mean that this message is NOT originally a MyMessage isntance, right? I mean, it's equivalent to !$message instanceof MyMessage in a middleware without EnvelopAwareInterface?

Some inline comments with some of these answers might be all we need :)

Also, is it a real use-case to look need to know whether or not a message was just received? If you're using an async transport and you want to do something for a specific message type, is it important that you make sure this processing is done only when your message has been "received"? If so, why? And if so, how could I both perform my action only when the message is "received" AND only check the class of the original message (e.g. I need to do something when my MyMessage class is "received"? Or is that totally not the correct pattern.

if $message->get(MyMessage::class) is null, does it mean that this message is NOT originally a MyMessage isntance, right?

There is mis interpretation here. The ReceivedMessage class is not an example here, it's an actual envelope item within the component has is a marker that represents that the message has just been received. It's needed to have the router middleware ignoring them (otherwise we would have a receive -> send) loop.

Or, could you also pass something like $message->get(SerializerConfiguration::class) to get those types of things out?

That's not also, that's the only thing you can do :)

how could I both perform my action only when the message is "received" AND only check the class of the original message

if (nul !== $message->get(ReceivedMessage::class) && $message->getMessage() instanceof YourOwnMessageClass) { // Your logic... }

Might be clearer if we rename $message to $envelope?

javiereguiluz · 2018-06-09T14:11:40Z

components/messenger.rst

@@ -86,6 +86,65 @@ that will do the required processing for your message::
       }
    }

+Envelope


The title of this section and the introduction is not very clear to me. Here is a reword proposal:

Before:

Envelope -------- The notion of an envelope is a concept that helps add context around the messages. An envelope is a message and a set of data. From a user's perspective, this allows you to set some configuration around the message. For example, to set the serialization groups used when the message goes through the transport layer, wrap your message in an ``Envelope`` and add some ``SerializerConfiguration``::

After:

Adding Metadata to Messages (Envelopes) --------------------------------------- If you need to add metadata or some configuration to a message, wrap it with the :class:`Symfony\\Component\\Messenger\\Envelope` class. For example, to set the serialization groups used when the message goes through the transport layer, use the ``SerializerConfiguration`` envelope::

Much better 👍

javiereguiluz · 2018-06-27T07:57:19Z

Ping to @sroze. Will you have some time to fix the final issues of this PR? Thanks!

sroze · 2018-06-30T11:00:13Z

components/messenger.rst

@@ -86,6 +86,65 @@ that will do the required processing for your message::
       }
    }

+Envelope


Much better 👍

sroze · 2018-06-30T11:03:59Z

components/messenger.rst

+
+    class MyOwnMiddleware implements MiddlewareInterface, EnvelopeAwareInterface
+    {
+        public function handle($message, callable $next)


Actually, maybe we can rename $message to $envelope in this example? PHP allows us to do so (i.e. variable names not to match with the name in the interface). It might be clearer.

Yes. Please rename this.

sroze · 2018-06-30T11:06:46Z

components/messenger.rst

+            // implements the EnvelopeAwareInterface interface. Otherwise,
+            // it would be the "original" message.
+
+            if (null !== $message->get(ReceivedMessage::class)) {


if $message->get(MyMessage::class) is null, does it mean that this message is NOT originally a MyMessage isntance, right?

There is mis interpretation here. The ReceivedMessage class is not an example here, it's an actual envelope item within the component has is a marker that represents that the message has just been received. It's needed to have the router middleware ignoring them (otherwise we would have a receive -> send) loop.

Or, could you also pass something like $message->get(SerializerConfiguration::class) to get those types of things out?

That's not also, that's the only thing you can do :)

how could I both perform my action only when the message is "received" AND only check the class of the original message

if (nul !== $message->get(ReceivedMessage::class) && $message->getMessage() instanceof YourOwnMessageClass) { // Your logic... }

Might be clearer if we rename $message to $envelope?

sroze · 2018-06-30T11:09:46Z

@javiereguiluz pong :)

Nyholm

Good. Just that minor change renaming the variable

weaverryan · 2018-07-12T12:38:38Z

Thank you @sroze! Sorry for the delay! I did rename the variable to $envelope as suggested. Cheers!

…e, weaverryan) This PR was merged into the 4.1 branch. Discussion ---------- [Messenger] Add the Envelope in the documentation Because we've merged this beautiful PR :) Commits ------- a87d727 Renaming variable 16c7385 Merge branch '4.1' into messenger-envelope dd8b368 Reword the header regarding the envelope 357fe25 Add message envelope in the documentation

* 4.1: Simplified the "Release Process" page [Workflow] Fix typo documentation Fix mismatched list items typos in sample code [symfony#9988] fix minor typos [symfony#10099] update XML and PHP config examples [symfony#9969] fix a minor typo [symfony#10022] fix a minor typo preUpdate Event Listener On Uploaded Imagery [symfony#9757] fix rst syntax [symfony#10124] fix XML attribute name [symfony#10062] fix the code block [PHPUnitBridge] Explain how to show stack traces Fix docs on trusted hosts opcode optimizations

carsonbot added Status: Needs Review Messenger Status: Needs Work and removed Status: Needs Review labels May 10, 2018

weaverryan requested changes May 10, 2018

View reviewed changes

ogizanagi reviewed May 11, 2018

View reviewed changes

ogizanagi mentioned this pull request May 11, 2018

[Messenger] Add the transports documentation #9756

Closed

ogizanagi reviewed May 11, 2018

View reviewed changes

ogizanagi mentioned this pull request May 22, 2018

[Messenger] Envelope as first class citizen in middleware too symfony/symfony#27322

Closed

carsonbot added Status: Needs Review and removed Status: Needs Work labels Jun 3, 2018

sroze commented Jun 3, 2018

View reviewed changes

components/messenger.rst

'groups' => ['my_serialization_groups'],

]))

);

Copy link

Contributor Author

sroze Jun 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did 👍

sroze force-pushed the messenger-envelope branch from f57138f to c9a625e Compare June 3, 2018 14:52

javiereguiluz self-assigned this Jun 7, 2018

weaverryan reviewed Jun 7, 2018

View reviewed changes

sroze mentioned this pull request Jun 8, 2018

Publish Kafka message to the right topic sroze/messenger-enqueue-transport#9

Closed

javiereguiluz reviewed Jun 9, 2018

View reviewed changes

javiereguiluz added Status: Needs Work and removed Status: Needs Review labels Jun 27, 2018

sroze changed the base branch from master to 4.1 June 30, 2018 10:59

sroze commented Jun 30, 2018

View reviewed changes

carsonbot added Status: Needs Review and removed Status: Needs Work labels Jun 30, 2018

sroze added 2 commits June 30, 2018 12:08

Add message envelope in the documentation

357fe25

Reword the header regarding the envelope

dd8b368

sroze force-pushed the messenger-envelope branch from 2ed4789 to dd8b368 Compare June 30, 2018 11:09

ogizanagi mentioned this pull request Jul 4, 2018

[Messenger] Envelope-aware middleware is never called with a message symfony/symfony#27841

Merged

Merge branch '4.1' into messenger-envelope

16c7385

Nyholm approved these changes Jul 12, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jul 12, 2018

Renaming variable

a87d727

weaverryan merged commit a87d727 into symfony:4.1 Jul 12, 2018

xabbuh added a commit that referenced this pull request Aug 6, 2018

[#9757] fix rst syntax

8f727dc

Guikingone pushed a commit to Guikingone/symfony-docs that referenced this pull request Feb 12, 2019

[symfony#9757] fix rst syntax

9e485a8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] Add the Envelope in the documentation #9757

[Messenger] Add the Envelope in the documentation #9757

sroze commented May 10, 2018

weaverryan left a comment

weaverryan May 10, 2018

sroze May 10, 2018

ogizanagi May 22, 2018 •

edited

Loading

weaverryan May 10, 2018

sroze May 10, 2018

ogizanagi May 11, 2018

ogizanagi left a comment

ogizanagi May 11, 2018

ogizanagi May 11, 2018

ogizanagi May 11, 2018

ogizanagi May 11, 2018

ogizanagi May 11, 2018

sroze Jun 3, 2018

sroze left a comment

sroze Jun 3, 2018

sroze commented Jun 3, 2018

weaverryan Jun 7, 2018

weaverryan Jun 7, 2018

weaverryan Jun 7, 2018

sroze Jun 30, 2018 •

edited

Loading

javiereguiluz Jun 9, 2018

sroze Jun 30, 2018

javiereguiluz commented Jun 27, 2018

sroze Jun 30, 2018

sroze Jun 30, 2018

Nyholm Jul 12, 2018

sroze Jun 30, 2018 •

edited

Loading

sroze commented Jun 30, 2018

Nyholm left a comment

weaverryan commented Jul 12, 2018

		@@ -188,6 +235,6 @@ Receiver and Sender on the same bus

		To allow us to receive and send messages on the same bus and prevent an infinite
		loop, the message bus is equipped with the ``WrapIntoReceivedMessage`` middleware.

[Messenger] Add the Envelope in the documentation #9757

[Messenger] Add the Envelope in the documentation #9757

Conversation

sroze commented May 10, 2018

weaverryan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi May 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sroze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sroze commented Jun 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sroze Jun 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javiereguiluz commented Jun 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sroze Jun 30, 2018 • edited Loading

Choose a reason for hiding this comment

sroze commented Jun 30, 2018

Nyholm left a comment

Choose a reason for hiding this comment

weaverryan commented Jul 12, 2018

ogizanagi May 22, 2018 •

edited

Loading

sroze Jun 30, 2018 •

edited

Loading

sroze Jun 30, 2018 •

edited

Loading