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] Added documentation about DispatchAfterCurrentBusMiddleware#10015

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

Closed
Nyholm wants to merge13 commits intosymfony:masterfromNyholm:message-recorder

Conversation

@Nyholm
Copy link
Member

@NyholmNyholm commentedJul 4, 2018•
edited
Loading

Documentation to PR:symfony/symfony#27844

Screenshot 2019-03-17 at 11 43 23
Screenshot 2019-03-17 at 11 43 34

Screenshot 2019-03-17 at 11 43 39

@javiereguiluz
Copy link
Member

@Nyholm thanks for this contribution! Lately in the Symfony Docs we're trying to not add new articles when they are too short. So, do you think we could move this article to a section inside an existing article? If there is no article where we can put this ... or if such article is already super long, we could consider creating this new article. Thanks!

@Nyholm
Copy link
MemberAuthor

Thank you. I agree.

I did not want my PR's to build on each other. At the same time, the message recorder has not been merged yet (Im not sure that it will).

#10013 is blocking. When#10013 is merged I will put this text into that article.


.. code-block:: yaml

# config/packages/workflow.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

- workflow.yaml+ messenger.yaml

Record Events Produced by a Handler
===================================

In a example application there is a command (a CQRS message) named ``CreateUser``.
Copy link
Contributor

Choose a reason for hiding this comment

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

"In an example"

===================================

In a example application there is a command (a CQRS message) named ``CreateUser``.
That command is handled by the ``CreateUserHandler``. The command handler creates
Copy link
Contributor

Choose a reason for hiding this comment

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

- by the ``CreateUserHandler``. The command handler creates+ by the ``CreateUserHandler`` which creates

?


In a example application there is a command (a CQRS message) named ``CreateUser``.
That command is handled by the ``CreateUserHandler``. The command handler creates
a ``User`` object, stores that object to a database and dispatches an ``UserCreatedEvent``.
Copy link
Contributor

Choose a reason for hiding this comment

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

"a UserCreatedEvent"

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Are you sure?

Copy link
Contributor

Choose a reason for hiding this comment

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

Almost :) I'm not native. But you wrote "a User" just before. Seehttps://english.stackexchange.com/questions/105116/is-it-a-user-or-an-user

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I would say that "a User" was a typo.

I read your source. I did not know that. Thank you. I've updated the PR.

There are many subscribers to the ``UserCreatedEvent``, one subscriber may send
a welcome email to the new user. Since we are using the ``DoctrineTransactionMiddleware``
we wrap all database queries in one database transaction and rollback that transaction
if an exception is thrown. That would mean that if an exception is thrown when sending
Copy link
Contributor

@HeahDudeHeahDudeJul 5, 2018•
edited
Loading

Choose a reason for hiding this comment

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

- That would mean+ That means

- messenger.middleware.validation
- messenger.middleware.handles_recorded_messages: ['@messenger.bus.event']
# Doctrine transaction must be after handles_recorded_messages middleware
- app.doctrine_transaction_middleware: ['default']
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using an FQCN forapp.doctrine_transaction_middleware? or may we explicit the definition somewhere. If possible I'd rather use the class name.

sroze reacted with thumbs up emoji
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I have an idea that this will be on the same page as#10013 where we useapp.doctrine_transaction_middleware.

$this->em = $em;
}

public function __invoke(CreateUser $command)
Copy link
Contributor

Choose a reason for hiding this comment

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

We're missing use statements for domain classes, what about:

useApp\Entity\User;useApp\Messenger\Command\CreateUser;useApp\Messenger\Event\UserCreatedEvent;

?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Thank you

@HeahDudeHeahDude added this to the4.2 milestoneJul 5, 2018
Record Events Produced by a Handler
===================================

In an example application there is a command (a CQRS message) named ``CreateUser``.
Copy link
Contributor

Choose a reason for hiding this comment

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

- In an example application there is a command (a CQRS message) named ``CreateUser``.+ Let's take the example of an application that has a command (a CQRS message) named ``CreateUser``.

?


In an example application there is a command (a CQRS message) named ``CreateUser``.
That command is handled by the ``CreateUserHandler`` which creates
a ``User`` object, stores that object to a database and dispatches a ``UserCreatedEvent``.
Copy link
Contributor

Choose a reason for hiding this comment

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

UserCreated event?

That event is also a normal message but is handled by an *event* bus.

There are many subscribers to the ``UserCreatedEvent``, one subscriber may send
a welcome email to the new user. Since we are using the ``DoctrineTransactionMiddleware``
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to split the two and explicit the "problem":

- Since we are using the ``DoctrineTransactionMiddleware`` [...]+ We are using the ``DoctrineTransactionMiddleware`` to wrap all database queries in one database transaction.++ **Problem:** if an exception is thrown when sending the welcome email, then the user will not be created because the ``DoctrineTransactionMiddleware`` will rollback the Doctrine transaction, in which the user has been created.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Nice. Thank you

if an exception is thrown. That means that if an exception is thrown when sending
the welcome email, then the user will not be created.

The solution to this issue is to not dispatch the ``UserCreatedEvent`` in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, I'd highlight the solution:

**Solution:**

.. index::
single: Messenger; Record messages

Record Events Produced by a Handler
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better to find a name that describes the problem than the technical solution. Something like "Event recorder: Tolerating failures while in Doctrine transactions"?

$user = new User($command->getUuid(), $command->getName(), $command->getEmail());
$this->em->persist($user);

$this->eventRecorder->record(new UserCreatedEvent($command->getUuid());
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to explain what this does, right? Because it does more than "recording" the event. Actually, this call doespostpone the event dispatching isn't it?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

The EventRecorder is just a storage. There is a middleware that reads from that store later. But yes, I'll write something

@Nyholm
Copy link
MemberAuthor

Thank you for the review. I've updated the PR accordingly

Copy link

@arnolangladearnolanglade left a comment

Choose a reason for hiding this comment

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

It would be good to add a rich modelUser with a named constructorsignUp that will recored aSignedUpUser event.

Events Recorder: Handle Events After CommandHandler Is Done
===========================================================

Let's take the example of an application that has a command (a CQRS message) named

Choose a reason for hiding this comment

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

This is nota CQRS message but message. You suer messages too when you application use the CQS pattern for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, CQRS doesn't necessitate any form of messages. Could "an imperative domain message" be an option? Or maybe a more thorough definition could be useful.

===========================================================

Let's take the example of an application that has a command (a CQRS message) named
``CreateUser``. That command is handled by the ``CreateUserHandler`` which creates

Choose a reason for hiding this comment

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

It could be better to useSignUpUser as command name, it is less focus on CRUD action.

magnusnordlander reacted with thumbs up emoji
$this->em->persist($user);

// "Record" this event to be processed later by "handles_recorded_messages".
$this->eventRecorder->record(new UserCreatedEvent($command->getUuid());

Choose a reason for hiding this comment

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

This event should be produced by the user model.

Koc reacted with thumbs up emoji
Events Recorder: Handle Events After CommandHandler Is Done
===========================================================

Let's take the example of an application that has a command (a CQRS message) named
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, CQRS doesn't necessitate any form of messages. Could "an imperative domain message" be an option? Or maybe a more thorough definition could be useful.

``CreateUserHandler`` but to just "record" the events. The recorded events will
be dispatched after ``DoctrineTransactionMiddleware`` has committed the transaction.

To enable this, you simply just add the ``messenger.middleware.handles_recorded_messages``
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove "simply just", because it's just filler, and not everyone would consider this simple.

@gubler
Copy link
Contributor

The current PRsymfony/symfony#28849 replaces the RecordsMessages middleware and recorder with aHandleMessageInNewTransaction middleware. I have did a rewrite of the documentation but I'm not sure how to request a merge into an existing merge request.

My rewrite is here:https://github.com/gubler/messenger-transaction-test/blob/master/docs/transactional-messages.rst

I tried to take into account previous comments on this request. Feel free to take whatever is useful from my rewrite.

@Nyholm
Copy link
MemberAuthor

Thank you@gubler Could you open a PR to myfork?

@gubler
Copy link
Contributor

Done - though its the first time I've done this, so I apologize if I messed anything up :/

Nyholm and ogizanagi reacted with thumbs up emoji

@Nyholm
Copy link
MemberAuthor

We should not forget this:symfony/symfony#28849 (comment)

@Nyholm
Copy link
MemberAuthor

I did some updates to the docs to keep up-to-date with the symfony PR.

I also added some screenshots to this PR description. I hope they will address any questions about the functionality and to help you (everyone) to help me explain this in the docs.

sroze added a commit to symfony/symfony that referenced this pull requestMar 19, 2019
…t bus is finished (Nyholm)This PR was merged into the 4.3-dev branch.Discussion----------[Messenger] Support for handling messages after current bus is finished| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets || License       | MIT| Doc PR        |symfony/symfony-docs#10015This is a replacement for#27844. We achieve the same goals without introducing the new concept of "recorder".```phpclass CreateUserHandler{    private $em;    private $eventBus;    public function __construct(MessageBus $eventBus, EntityManagerInterface $em)    {        $this->eventBus = $eventBus;        $this->em = $em;    }    public function __invoke(CreateUser $command)    {        $user = new User($command->getUuid(), $command->getName(), $command->getEmail());        $this->em->persist($user);        $message = new UserCreatedEvent($command->getUuid();        $this->eventBus->dispatch((new Envelope($message))->with(new DispatchAfterCurrentBus()));    }}```Note that this `DispatchAfterCurrentBusMiddleware` is added automatically as the first middleware.2019-03-13: I updated the PR description.Commits-------903355f Support for handling messages after current bus is finished
@sroze
Copy link
Contributor

Thank you@Nyholm!

@Nyholm
Copy link
MemberAuthor

@sroze could you add your review?

Nyholmand others added6 commitsApril 8, 2019 20:46
The current PR is for HandleMessageInNewTransaction middleware instead of the original RecordsMessages middleware. This documentation covers the new middleware.
@NyholmNyholm changed the title[Messenger] Added documentation about message recorder[Messenger] Added documentation about DispatchAfterCurrentBusMiddlewareApr 8, 2019
OskarStarkand others added5 commitsApril 8, 2019 21:37
Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>
Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>
Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>
Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>
.. index::
single: Messenger; Record messages; Transaction messages

Transactional Messages: Handle Events After CommandHandler is Done
Copy link
Contributor

Choose a reason for hiding this comment

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

@javiereguiluz s/After/after/ ?

Copy link
Contributor

@srozesroze left a comment

Choose a reason for hiding this comment

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

Except these few details, it looks good to me 👍

@Nyholm
Copy link
MemberAuthor

Thank you for the review. I've updated accordingly

@weaverryan
Copy link
Member

Thank you@Nyholm and@gubler!

@Nyholm
Copy link
MemberAuthor

Thank you for merging.

@NyholmNyholm deleted the message-recorder branchMay 24, 2019 14:23
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull requestMay 28, 2019
…dleware entry (weaverryan)This PR was merged into the 4.3 branch.Discussion----------[Messenger] Tweaks for DispatchAfterCurrentBusMiddleware entryJust some proofreading changes forsymfony#10015!Commits-------599d10b minor reorg of new DispatchAfterCurrentBusMiddleware
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@weaverryanweaverryanweaverryan left review comments

@OskarStarkOskarStarkOskarStark left review comments

+6 more reviewers

@gublergublergubler left review comments

@magnusnordlandermagnusnordlandermagnusnordlander left review comments

@ogizanagiogizanagiogizanagi left review comments

@arnolangladearnolangladearnolanglade left review comments

@HeahDudeHeahDudeHeahDude left review comments

@srozesrozesroze approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.2

Development

Successfully merging this pull request may close these issues.

11 participants

@Nyholm@javiereguiluz@gubler@sroze@weaverryan@magnusnordlander@OskarStark@ogizanagi@arnolanglade@HeahDude@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp