Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Messenger] Add the Envelope in the documentation#9757

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
weaverryan merged 4 commits intosymfony:4.1fromsroze:messenger-envelope
Jul 12, 2018

Conversation

@sroze
Copy link
Contributor

Because we've merged this beautiful PR :)

ragboyjr reacted with thumbs up emoji
Copy link
Member

@weaverryanweaverryan left a comment

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

It's always anEnvelope if you implements the interface.

Copy link
Contributor

@ogizanagiogizanagiMay 22, 2018
edited
Loading

Choose a reason for hiding this comment

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

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


return $next(
$message->with(new AnotherEnvelopeItem(/* ... */))
);
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Both are good use cases tbh

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@ogizanagiogizanagi left a comment

Choose a reason for hiding this comment

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

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 anEnvelope entry below it would be helpful too (with a link to this new section).

]))
);

Instead of dealing directly with the message in the handlers or middleware, you
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


return $next(
$message->with(new AnotherEnvelopeItem(/* ... */))
);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

It willwrap the received messages into``ReceivedMessage``objects and the
``SendMessageMiddleware`` middleware will know it should not route these
messagesagain to a transport.
It willadd a``ReceivedMessage``configuration to the message envelopes and the
Copy link
Contributor

Choose a reason for hiding this comment

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

- configuration+ marker item

?

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, theWrapIntoReceivedMessage is neither a middleware, neither it is used in Symfony core. That's theWorker that call thereceive method with a$handleradding theReceivedMessage marker to the envelope.

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Just did 👍

Copy link
ContributorAuthor

@srozesroze left a comment

Choose a reason for hiding this comment

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

Updated :)

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

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Just did 👍

@sroze
Copy link
ContributorAuthor

@weaverryan can you have another look please?

@javiereguiluzjaviereguiluz self-assigned thisJun 7, 2018
{
// $message here is an `Envelope` object, because this middleware
// implements the EnvelopeAwareInterface interface. Otherwise,
// it would be the "original" message.
Copy link
Member

Choose a reason for hiding this comment

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

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.

// implements the EnvelopeAwareInterface interface. Otherwise,
// it would be the "original" message.

if (null !== $message->get(ReceivedMessage::class)) {
Copy link
Member

Choose a reason for hiding this comment

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

Is thisget() method something that lives onEnvelope? 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 aMyMessage isntance, right? I mean, it's equivalent to!$message instanceof MyMessage in a middleware withoutEnvelopAwareInterface?

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

Copy link
Member

Choose a reason for hiding this comment

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

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 myMyMessage class is "received"? Or is that totally not the correct pattern.

Copy link
ContributorAuthor

@srozesrozeJun 30, 2018
edited
Loading

Choose a reason for hiding this comment

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

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. TheReceivedMessage 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 notalso, 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 werename$message to$envelope?

}
}

Envelope
Copy link
Member

Choose a reason for hiding this comment

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

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 themessages. An envelope is a message and a set of data. From a user's perspective, thisallows you to set some configuration around the message. For example, to set the serializationgroups used when the message goes through the transport layer, wrap your messagein 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 theserialization groups used when the message goes through the transport layer, usethe ``SerializerConfiguration`` envelope::

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Much better 👍

@javiereguiluz
Copy link
Member

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

@srozesroze changed the base branch frommaster to4.1June 30, 2018 10:59
}
}

Envelope
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Much better 👍


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

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. Please rename this.

// implements the EnvelopeAwareInterface interface. Otherwise,
// it would be the "original" message.

if (null !== $message->get(ReceivedMessage::class)) {
Copy link
ContributorAuthor

@srozesrozeJun 30, 2018
edited
Loading

Choose a reason for hiding this comment

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

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. TheReceivedMessage 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 notalso, 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 werename$message to$envelope?

@sroze
Copy link
ContributorAuthor

@javiereguiluz pong :)

Copy link
Member

@NyholmNyholm left a comment

Choose a reason for hiding this comment

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

Good. Just that minor change renaming the variable

@weaverryan
Copy link
Member

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

ogizanagi reacted with hooray emoji

@weaverryanweaverryan merged commita87d727 intosymfony:4.1Jul 12, 2018
weaverryan added a commit that referenced this pull requestJul 12, 2018
…e, weaverryan)This PR was merged into the 4.1 branch.Discussion----------[Messenger] Add the Envelope in the documentationBecause we've merged this beautiful PR :)Commits-------a87d727 Renaming variable16c7385 Merge branch '4.1' into messenger-envelopedd8b368 Reword the header regarding the envelope357fe25 Add message envelope in the documentation
xabbuh added a commit that referenced this pull requestAug 6, 2018
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull requestAug 8, 2018
* 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
Guikingone pushed a commit to Guikingone/symfony-docs that referenced this pull requestFeb 12, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz left review comments

@weaverryanweaverryanweaverryan requested changes

+2 more reviewers

@ogizanagiogizanagiogizanagi left review comments

@NyholmNyholmNyholm approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@javiereguiluzjaviereguiluz

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sroze@javiereguiluz@weaverryan@Nyholm@ogizanagi@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp