[Messenger] Added documentation about DispatchAfterCurrentBusMiddleware by Nyholm · Pull Request #10015 · symfony/symfony-docs

Nyholm · 2018-07-04T14:43:22Z

Documentation to PR: symfony/symfony#27844

javiereguiluz · 2018-07-04T14:50:20Z

@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 · 2018-07-04T14:55:33Z

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.

HeahDude · 2018-07-05T08:45:53Z

messenger/message-recorder.rst

+
+    .. code-block:: yaml
+
+        # config/packages/workflow.yaml


- workflow.yaml + messenger.yaml

HeahDude · 2018-07-05T08:46:17Z

messenger/message-recorder.rst

+Record Events Produced by a Handler
+===================================
+
+In a example application there is a command (a CQRS message) named ``CreateUser``.


"In an example"

HeahDude · 2018-07-05T08:47:18Z

messenger/message-recorder.rst

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


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

?

HeahDude · 2018-07-05T08:47:58Z

messenger/message-recorder.rst

+
+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``.


"a UserCreatedEvent"

Are you sure?

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

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.

HeahDude · 2018-07-05T08:48:45Z

messenger/message-recorder.rst

+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


- That would mean + That means

HeahDude · 2018-07-05T08:50:34Z

messenger/message-recorder.rst

+                            - 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']


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

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

messenger/message-recorder.rst

HeahDude · 2018-07-05T08:53:09Z

messenger/message-recorder.rst

+            $this->em = $em;
+        }
+
+        public function __invoke(CreateUser $command)


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

use App\Entity\User; use App\Messenger\Command\CreateUser; use App\Messenger\Event\UserCreatedEvent;

?

sroze · 2018-08-26T15:51:00Z

messenger/message-recorder.rst

+Record Events Produced by a Handler
+===================================
+
+In an example application there is a command (a CQRS message) named ``CreateUser``.


- 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``.

?

sroze · 2018-08-26T15:51:20Z

messenger/message-recorder.rst

+
+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``.


UserCreated event?

sroze · 2018-08-26T15:53:50Z

messenger/message-recorder.rst

+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``


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.

Nice. Thank you

sroze · 2018-08-26T15:54:07Z

messenger/message-recorder.rst

+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


Same, I'd highlight the solution:

**Solution:**

sroze · 2018-08-26T15:55:16Z

messenger/message-recorder.rst

+.. index::
+    single: Messenger; Record messages
+
+Record Events Produced by a Handler


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"?

sroze · 2018-08-26T15:56:56Z

messenger/message-recorder.rst

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


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

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

Nyholm · 2018-08-26T16:23:45Z

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

arnolanglade

It would be good to add a rich model User with a named constructor signUp that will recored a SignedUpUser event.

arnolanglade · 2018-09-09T18:13:58Z

messenger/message-recorder.rst

+Events Recorder: Handle Events After CommandHandler Is Done
+===========================================================
+
+Let's take the example of an application that has a command (a CQRS message) named


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

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.

arnolanglade · 2018-09-09T18:26:58Z

messenger/message-recorder.rst

+===========================================================
+
+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


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

messenger/message-recorder.rst

arnolanglade · 2018-09-09T18:34:34Z

messenger/message-recorder.rst

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


This event should be produced by the user model.

magnusnordlander · 2018-12-08T12:25:33Z

messenger/message-recorder.rst

+Events Recorder: Handle Events After CommandHandler Is Done
+===========================================================
+
+Let's take the example of an application that has a command (a CQRS message) named


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.

magnusnordlander · 2018-12-08T12:28:33Z

messenger/message-recorder.rst

+``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``


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

gubler · 2019-02-24T14:03:14Z

The current PR symfony/symfony#28849 replaces the RecordsMessages middleware and recorder with a HandleMessageInNewTransaction 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 · 2019-02-24T15:03:09Z

Thank you @gubler Could you open a PR to my fork?

gubler · 2019-02-24T15:15:54Z

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

messenger/message-recorder.rst

Nyholm · 2019-03-17T10:14:04Z

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

Nyholm · 2019-03-17T10:45:56Z

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.

…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#10015 This is a replacement for #27844. We achieve the same goals without introducing the new concept of "recorder". ```php class 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 · 2019-03-20T13:42:28Z

Thank you @Nyholm!

Nyholm · 2019-04-08T17:48:04Z

@sroze could you add your review?

The current PR is for HandleMessageInNewTransaction middleware instead of the original RecordsMessages middleware. This documentation covers the new middleware.

messenger/message-recorder.rst

Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>

OskarStark · 2019-04-08T20:15:04Z

messenger/message-recorder.rst

+.. index::
+    single: Messenger; Record messages; Transaction messages
+
+Transactional Messages: Handle Events After CommandHandler is Done


@javiereguiluz s/After/after/ ?

sroze

Except these few details, it looks good to me 👍

messenger/message-recorder.rst

Nyholm · 2019-04-28T07:14:27Z

Thank you for the review. I've updated accordingly

weaverryan · 2019-05-24T13:59:35Z

Thank you @Nyholm and @gubler!

Nyholm · 2019-05-24T14:22:56Z

Thank you for merging.

…dleware entry (weaverryan) This PR was merged into the 4.3 branch. Discussion ---------- [Messenger] Tweaks for DispatchAfterCurrentBusMiddleware entry Just some proofreading changes for symfony#10015! Commits ------- 599d10b minor reorg of new DispatchAfterCurrentBusMiddleware

carsonbot added Status: Needs Review Messenger labels Jul 4, 2018

This was referenced Jul 4, 2018

[Messenger] Show how to configure a doctrine transaction middleware #10013

Closed

[Messenger] Adding support for record messages symfony/symfony#27844

Closed

HeahDude reviewed Jul 5, 2018

View reviewed changes

HeahDude added this to the 4.2 milestone Jul 5, 2018

sroze reviewed Aug 26, 2018

View reviewed changes

arnolanglade reviewed Sep 9, 2018

View reviewed changes

Nyholm mentioned this pull request Oct 13, 2018

[Messenger] Support for handling messages after current bus is finished symfony/symfony#28849

Merged

magnusnordlander reviewed Dec 8, 2018

View reviewed changes

Nyholm mentioned this pull request Feb 24, 2019

Explain HandleMessageInNewTransaction middleware Nyholm/symfony-docs#4

Merged

ogizanagi reviewed Feb 24, 2019

View reviewed changes

messenger/message-recorder.rst Outdated Show resolved Hide resolved

weaverryan reviewed Mar 13, 2019

View reviewed changes

messenger/message-recorder.rst Outdated Show resolved Hide resolved

messenger/message-recorder.rst Show resolved Hide resolved

messenger/message-recorder.rst Outdated Show resolved Hide resolved

messenger/message-recorder.rst Show resolved Hide resolved

Nyholm and others added 6 commits April 8, 2019 20:46

Added documentation about message recorder

8107340

According to feedback

66fc600

a user

091040f

Added fixes

0a3ccf6

Explain HandleMessageInNewTransaction middleware

b5921bf

The current PR is for HandleMessageInNewTransaction middleware instead of the original RecordsMessages middleware. This documentation covers the new middleware.

Minor updates

aba0edf

Added PHP and XML config

9b589ff

Nyholm force-pushed the message-recorder branch from 3e1453e to 9b589ff Compare April 8, 2019 18:53

Nyholm changed the title ~~[Messenger] Added documentation about message recorder~~ [Messenger] Added documentation about DispatchAfterCurrentBusMiddleware Apr 8, 2019

OskarStark reviewed Apr 8, 2019

View reviewed changes

OskarStark and others added 5 commits April 8, 2019 21:37

Update messenger/message-recorder.rst

0e1e0dd

Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>

Update messenger/message-recorder.rst

aa4096f

Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>

Update messenger/message-recorder.rst

636de55

Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>

Update messenger/message-recorder.rst

157d3f0

Co-Authored-By: Nyholm <tobias.nyholm@gmail.com>

updates according to feedback

6b71ef8

OskarStark reviewed Apr 8, 2019

View reviewed changes

sroze approved these changes Apr 22, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Apr 22, 2019

minor updates according to feedback

45245df

weaverryan mentioned this pull request May 24, 2019

[Messenger] Tweaks for DispatchAfterCurrentBusMiddleware entry #11597

Merged

weaverryan closed this in 5e1033a May 24, 2019

Nyholm deleted the message-recorder branch May 24, 2019 14:23

Uh oh!

Conversation

Nyholm commented Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

javiereguiluz commented Jul 4, 2018

Uh oh!

Nyholm commented Jul 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HeahDude Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nyholm commented Aug 26, 2018

Uh oh!

arnolanglade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gubler commented Feb 24, 2019

Uh oh!

Nyholm commented Feb 24, 2019

Uh oh!

gubler commented Feb 24, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nyholm commented Jul 4, 2018 •

edited

Loading

HeahDude Jul 5, 2018 •

edited

Loading