[Messenger] Add a new Messenger component #24411

sroze · 2017-10-03T16:39:23Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#24326
License	MIT
Doc PR	symfony/symfony-docs#9437

As discussed in #24326. This PR is to help going forward with the discussions of having a Message component.

Resources

What	Where
Documentation	In the PR
Demo	In `sroze/symfony-demo:message-component-demo`
php-enqueue adapter	1. Source: In `sroze/enqueue-bridge` (to be moved as `symfony/enqueue-bridge` I guess) 2. Demo: In `sroze/symfony-demo:message-component-demo-with-enqueue`
Swarrot adapter	Outdated adapter, waiting for stabilization 1. Source: In `sroze/swarrot-bridge` (to be moved as `symfony/swarrot-bridge` I guess) 2. Demo: In `sroze/symfony-demo:message-component-demo-with-swarrot`
HTTP adapter	Outdated adapter, waiting for stabilization 1. Source: In `sroze/message-http-adapter` 2. Demo: In `sroze/symfony-demo:message-component-demo-with-http-adapter`
Web profiler integration	In the pull-request

Important points

Tests are not in the PR as they were written in PhpSpec & Behat. If we decide to go forward with this approach, I'll translate them to PHPUnit.
The aim is not to solve all the message/queuing problems but provide a good, simple and extensible message bus for developers.
The communication with the actual AMQP/API brokers is down to the adapters for now. Not sure if we need to ship some by default or not 🤔

I guess that this would replace #23842 & #23315.

Changes from the proposals

Based on the comments, a few changes have been made from the proposal.

MessageProducers have been renamed to MessageSenders
MessageConsumers have been renamed to MessageReceivers

mvrhov · 2017-10-06T13:06:40Z

src/Symfony/Component/Message/MessageBus.php

+     */
+    public function handle($message)
+    {
+        call_user_func($this->callableForNextMiddleware(0), $message);


return missing

everzet

I'd work on naming a bit, Sam :)

Otherwise - 👍

everzet · 2017-10-09T10:14:57Z

src/Symfony/Component/Message/Asynchronous/Middleware/SendMessageToProducersMiddleware.php

+     */
+    private $producerForMessageResolver;
+
+    public function __construct(ProducerForMessageResolverInterface $producerForMessageResolver)


MessageSenderResolverInterface

everzet · 2017-10-09T10:15:17Z

src/Symfony/Component/Message/Asynchronous/Middleware/SendMessageToProducersMiddleware.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+class SendMessageToProducersMiddleware implements MessageBusMiddlewareInterface


MessageSendingMiddleware

everzet · 2017-10-09T10:17:01Z

src/Symfony/Component/Message/Asynchronous/Middleware/SendMessageToProducersMiddleware.php

+    {
+        if ($message instanceof ConsumedMessage) {
+            $message = $message->getMessage();
+        } elseif (!empty($producers = $this->producerForMessageResolver->getProducersForMessage($message))) {


getProducersForMessage() => getSender()

As I guess we'll rename Producer to Sender I see where you are going. But you replace plural by singular here and that's an interesting place for me to explain why it's plural.

At the beginning it was returning ProducerInterface|null and I had a collection of producer that was dispatching to a set of producers to handle this plural. But there's one use-case that didn't work:

When I dispatch a message "Foo" in the message bus
Then I want the message to be sent to the producer "FooProducer"
And I want the message to be handled by the handler

i.e. I want to send the message to something and I want to call the local handler. This can be useful for audit purposes, duplication, etc... To make this use case happening, you just need to have to use a null handler. (See the note in `README.md)

Therefore, I'd say getProducersForMessage() => getSenders()

everzet · 2017-10-09T10:19:36Z

src/Symfony/Component/Message/Handler/CollectionOfMessageHandlers.php

+ *
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+class CollectionOfMessageHandlers


MessageHandlers or MessageHandlerCollection for consistency with other names

Let's go for MessageHandlerCollection.

everzet · 2017-10-09T10:22:31Z

src/Symfony/Component/Message/MessageBus.php

+ * @author Samuel Roze <samuel.roze@gmail.com>
+ * @author Matthias Noback <matthiasnoback@gmail.com>
+ */
+class MessageBus implements MessageBusInterface


MiddleawareMessageBus :D

if we took that the interface is called MessageBusMiddlewareInterface then it should be MessageBusMiddleware which is more accurate

everzet · 2017-10-09T10:23:04Z

src/Symfony/Component/Message/MessageConsumerInterface.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+interface MessageConsumerInterface


ConsumerInterface

everzet · 2017-10-09T10:23:34Z

src/Symfony/Component/Message/MessageHandlerResolver.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+class MessageHandlerResolver implements MessageHandlerResolverInterface


MappedHandlerResolver

everzet · 2017-10-09T10:23:47Z

src/Symfony/Component/Message/MessageHandlerResolverInterface.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+interface MessageHandlerResolverInterface


HandlerResolverInterface

everzet · 2017-10-09T10:24:02Z

src/Symfony/Component/Message/MessageProducerInterface.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+interface MessageProducerInterface


ProducerInterface

everzet · 2017-10-09T10:24:43Z

src/Symfony/Component/Message/Middleware/CallMessageHandlerMiddleware.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+class CallMessageHandlerMiddleware implements MessageBusMiddlewareInterface


HandlerMiddleware

Koc · 2017-10-09T11:05:37Z

src/Symfony/Bundle/FrameworkBundle/Command/MessageConsumeCommand.php

+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+        /** @var ContainerInterface $container */
+        $container = $this->getApplication()->getKernel()->getContainer();


Can we use service locators instead of container?

@Koc the point here is that the consumer - i.e. the service name - can be given as an argument. Not sure how would the service locator plays with that (as it needs to know the dependencies when constructed AFAIK)

Require tags on consumers

Correct, we can require the receivers (note the name change) to be tagged, so we can use service locators 👍

Actually, I don't see a real point of using service locators in here. It forces us to create a new compiler pass and have another set of tags... while just using the container (as in many other commands in the FrameworkBundle) is the simplest option.

It would be nice to promote the good practice of not injecting the container in our own code. A service locator + autoconfiguration looks better to me, especially because you have a ReceiverInterface.

Auto-configuration would be nice if we also define a generic folder (such as src/MessageReceiver). Without, it feels a bit too much work for not much benefits, as I explained in the previous comment. But obviously, if the general feedback is that we need this, we can add it (even later hehe :))

Fetching the receiver from the container has one big drawback: it must be public. I would say go for an autoconfigured tag + scoped receiver locator

Fair enough, updated to use service locators 👍

arnolanglade · 2017-10-09T15:39:46Z

src/Symfony/Component/Message/MessageBusInterface.php

+     *
+     * @param object $message
+     *
+     * @return mixed


Normally a command bus should return void, right?

And that's the big point here: that's not "just" a command bus but it is a message bus. This can also be used for queries in a CQRS-type architecture.

Note that if you want to enforce such limitation for commands, you can very easily create your own instance of the MessageBus and have a special "noop returns" (or "EnforceCommandsDoNotReturnAnything") middleware that would force return null for example.

Ok, thank for explanations.

arnolanglade · 2017-10-09T15:50:10Z

src/Symfony/Component/Message/MessageConsumerInterface.php

+ */
+interface MessageConsumerInterface
+{
+    public function consume(): \Generator;


If I understand this method creates a collection of messages? So you don't consume messages but you create them, right? Perhaps this is not the good wording. Plus, it is strange to use a verb for a method which builds something it is better to use a word like messages()

I'm really not found of the messages() suggestion but agree the naming can be better. What about FetcherInterface that has a fetch(): \Generator method? Because these things are fetching messages from various sources...

Or FetchedMessage::messages() :) but Fetcher::fetch() is more clear than the previous proposal.

arnolanglade · 2017-10-09T15:53:44Z

src/Symfony/Component/Message/MessageProducerInterface.php

+     *
+     * @param object $message
+     */
+    public function produce($message);


I'm not sure we should enforce this for now and leave the door open for results. i.e. I know that some brokers like Google Pub/Sub can return things like the "message ID" that is quite useful for logging/debugging purposes sometimes - so I can expect to see people logging these.

Or one can pass this directly to the e.g frontend, for it to check periodically if this is already processed. and you don't need to predefine the message id and store it to application db.

arnolanglade · 2017-10-09T16:01:10Z

src/Symfony/Component/Message/README.md

+3. `CallMessageHandlerMiddleware` (call the registered handle)
+
+```php
+$result = $this->get('message_bus')->dispatch(new MyMessage(/* ... */));


I think the bus handles message: dispatch -> handle

I'm not against this change; waiting to see more opinions on this :)

arnolanglade · 2017-10-10T06:56:35Z

src/Symfony/Component/Message/Asynchronous/Routing/ProducerForMessageResolver.php

+            return $this->messageToProducerMapping['*'];
+        }
+
+        return array();


arnolanglade · 2017-10-10T06:57:47Z

src/Symfony/Component/Message/DependencyInjection/MessagePass.php

+        $handlerResolver->replaceArgument(0, $this->findHandlers($container));
+    }
+
+    private function findHandlers(ContainerBuilder $container)


missing return type hint

arnolanglade · 2017-10-10T06:58:35Z

src/Symfony/Component/Message/Exception/ExceptionInterface.php

+ *
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+interface ExceptionInterface


is there any value to add an empty interface?

That's a good question; but it's a constant in every Symfony Component so I didn't want to start questioning this one 😉

Using this interface, allows you to catch all the exceptions that come from this component.

mvrhov · 2017-12-01T11:43:04Z

Now that 4.0 is out the door, can we get some more input from the symfony core team.
Ping @symfony/deciders

javiereguiluz · 2017-12-01T12:01:02Z

src/Symfony/Component/Message/Asynchronous/Routing/ProducerForMessageResolver.php

+    /**
+     * {@inheritdoc}
+     */
+    public function getProducersForMessage($message): array


could this method be refactored as follows?

return $this->messageToProducerMapping[get_class($message)] ?? $this->messageToProducerMapping['*'] ?? array();

Good point 👍

javiereguiluz · 2017-12-01T12:02:37Z

src/Symfony/Component/Message/DependencyInjection/MessagePass.php

+    private $messageBusService;
+    private $middlewareTag;
+
+    public function __construct(


In Symfony, the arguments are always put on the same line, no matter how long they are.

javiereguiluz · 2017-12-01T12:05:30Z

src/Symfony/Component/Message/DependencyInjection/MessagePass.php

+    {
+        $handlersByMessage = array();
+
+        foreach ($container->findTaggedServiceIds($this->handlerTag, true) as $serviceId => $tags) {


Can't we use a SplPriorityQueue here instead of manually dealing with priorities? We use it here for example:

symfony/src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php

Line 28 in bf4b09f

$definitions = new \SplPriorityQueue();

sroze

I'll rename the following things in this pull-request, following a number of valid comments regarding the names.

Producer -> Sender
Consumer -> Receiver

sroze · 2017-12-01T15:38:27Z

src/Symfony/Bundle/FrameworkBundle/Command/MessageConsumeCommand.php

+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+        /** @var ContainerInterface $container */
+        $container = $this->getApplication()->getKernel()->getContainer();


@Koc the point here is that the consumer - i.e. the service name - can be given as an argument. Not sure how would the service locator plays with that (as it needs to know the dependencies when constructed AFAIK)

Koc · 2017-12-02T17:15:34Z

@sroze can we tag all consumers with tag queue.consumer? Even reference tagged services

mvrhov · 2017-12-02T18:10:11Z

message.consumer or message.handler? would be better.

Koc · 2017-12-02T21:22:19Z

It depends on accepted terminology. Maybe even without dot separatorm like already proposed in this PR message_middleware tag.

sroze · 2017-12-03T11:50:58Z

@Koc do you mean the message handlers or message receivers? Following the reviews, I had to rename them. Check out the concepts documentation to clarify.

mvrhov · 2017-12-23T13:59:56Z

I've converted a small project from SimpleBus to Messaging component.
I had to change the following:

namespace from SimpleBus to Symfony
rename the method from handle to dispatch I'm still not fond of having dispatch instead of handle, but I understand the reasoning. However this will have to be explained to people.
change tag from command_handler to message_handler
change namespace from *\Command\* to *\Message\*
inject message_bus instead of command_bus
Use MessageBus instead of CommandBus

I still cannot get rid of the SimpleBus as this is missing the EventRecorder and a EventBus.
More to follow

Taluu · 2017-12-24T11:31:17Z

src/Symfony/Component/Message/Handler/MessageHandlerCollection.php

+    {
+        return array_map(function ($handler) use ($message) {
+            return $handler($message);
+        }, $this->handlers);


wouldn't an array_walk be better ? Don't see why we would need to fetch the return values of each handlers..

As discussed here I think we should have all we need to get the result of handlers, to support queries. Therefore, array_map makes sense here :)

ro0NL

And a 👍 from me :)

ro0NL · 2017-12-24T11:00:43Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/message.xml

+
+        <!-- Bus -->
+        <service id="message_bus" class="Symfony\Component\Message\MessageBus" public="true">
+            <argument type="collection" /> <!-- Middlewares -->


can it be type tagged? :)

Yep, I think so 👍

ro0NL · 2017-12-24T11:01:43Z

src/Symfony/Component/Message/Asynchronous/Middleware/SendMessageMiddleware.php

+class SendMessageMiddleware implements MiddlewareInterface
+{
+    /**
+     * @var SenderLocatorInterface


obvious IMHO

ro0NL · 2017-12-24T11:05:17Z

src/Symfony/Component/Message/Debug/LoggingMiddleware.php

+     */
+    public function handle($message, callable $next)
+    {
+        $this->logger->debug('Starting processing message', array(


... message "'.get_class($message).'"'? (as for logs below).

I don't think we should have that as part of the log message (as it's in the context) but happy to change if more people believe it should be the case.

it would make identifying messages a bit easier (i.e. in the web profiler log panel, which would boldify the message class :))

Only if the message is Starting processing {class} message and we add class as context I believe. Which... makes sense actually 👍

ro0NL · 2017-12-24T11:06:54Z

src/Symfony/Component/Message/Debug/LoggingMiddleware.php

+        try {
+            $result = $next($message);
+        } catch (\Throwable $e) {
+            $this->logger->warning('Something went wrong while processing message', array(


An exception occured while ...?

ro0NL · 2017-12-24T11:09:43Z

src/Symfony/Component/Message/DependencyInjection/MessagePass.php

+
+                try {
+                    $method = $reflection->getMethod('__invoke');
+                } catch (\ReflectionException $e) {


just use hasMethod?

getMethod has to be called nonetheless. As we need the access to the method parameters. Do we really need to call 2 methods instead of just catching the exception in those rare cases.

Do we really need to call 2 methods instead of just catching the exception

For readability / less LoC, i'd say yes. No real issue i guess.. just looks like an unneeded micro-optim.

Note that we should also support having the handles attribute on the tag. In case a handler is handling multiple commands/queries and/or the typehint is not present, we need to be able to specify the handled message.

ro0NL · 2017-12-24T11:31:08Z

src/Symfony/Component/Message/MessageBus.php

+     */
+    public function __construct(array $middlewares = array())
+    {
+        $this->middlewares = $middlewares;


to leverage tagged (which implies iterable type), what about;

$this->middlewares = is_array($middlewares) ? array_values($middlewares) : iterator_to_array($middlewares, false);

or find a lazy way?

Yup, good point. Why would you array_values if array?

to re-index middlewares, ensuring callableForNextMiddleware implem always works. (needs an int $index type btw :))

ro0NL · 2017-12-24T11:36:13Z

src/Symfony/Component/Message/Transport/Serialization/SymfonySerialization.php

+     */
+    private $serializer;
+
+    public function __construct(SerializerInterface $serializer)


, $format = 'json?

ro0NL · 2017-12-24T11:37:38Z

src/Symfony/Component/Message/Transport/Serialization/EncoderInterface.php

+     *
+     * @param object $message
+     *
+     * @return array


What about Transport\EncodedMessage?

What would be the value?

type info :) i.e. not having to rely on such a comment;

* The most common keys of the encoded array are: * - `body` (string) - the message body * - `headers` (string<string>) - a key/value pair of headers

common means required?

But much less flexibility for use-cases we don't know about yet. That's for the same reason that the serializer's context is an array.

I've been back and forth on this.
Serializers are never used in Symfony (message component) code. They are only used by queues to transform a PHP object to array to json. Which means that we should not really care. =)

Except for the SymfonySerialization that ships with the component, any implementation of EncoderInterface and DecoderInterface will only be used inside the concept of a "my-queue-bridge". Which leads to the question, do we need these interfaces?

Serializers are never used in Symfony (message component) code. They are only used by queues to transform a PHP object to array to json.

I've read it this way:

Encoders are never used in Symfony (message component) code. They are only used by adapters to transform a PHP object to a PHP array. Adapters will then transform the PHP array to their own transport layer (can be JSON, can be whatever...)

Which means that we should not really care. =)

We, as users, that's correct, we shouldn't care at all about these technical details when dispatching messages to the bus.

Any implementation of EncoderInterface and DecoderInterface will only be used inside the concept of a "my-queue-bridge".

I don't think so, that's why I've put these interfaces within the component. There are multiple use-cases that are not adapter-specific but might provider encoder/decoders:

When dispatching messages, it's pretty useful to be able to trace them. To get things like OpenTracing or Zipkin to work well, all we need is "just" proper headers on the messages (via AMQP, HTTP, whatever). So a symfony-message-zipkin package could offer encoder/decoders decorators to populate and read these headers, regardless of the transport.

It might be useful for some users/companies to add their own routing specific keys (in body, header or anything else) within the message before it being sent to either of the senders (which could be different adapters). Such encoder/decoder would help them here.

Which leads to the question, do we need these interfaces?

The reasons given above expresses my answer which is: I believe we need them. Also, it reduces the duplication within the various adapters and reduces the overhead of creating one.

ro0NL · 2017-12-24T11:39:26Z

src/Symfony/Component/Message/README.md

+#### Same bus received and sender
+
+To allow us to receive and send messages on the same bus and prevent a loop, the message bus is equipped with the
+`WrappedIntoReceivedMessage` received. It will wraps the received messages into `ReceivedMessage` objects and the 


WrapIntoReceivedMessage + It will wrap the..

Yep, good point 👍

ro0NL · 2017-12-24T11:42:25Z

src/Symfony/Component/Message/Transport/ReceiverInterface.php

+ */
+interface ReceiverInterface
+{
+    public function receive(): \Generator;


iterable?

This has been done then reverted to \Generator for the reasons described in this comment.

mvrhov · 2017-12-24T15:19:27Z

src/Symfony/Component/Message/HandlerLocator.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+class HandlerLocator implements HandlerLocatorInterface


@sroze: HandlerLocator should be able to lazy load the handlers. Now the question is, can I change this class to have ServiceLocator injected into the constructor, or should a new class be created and set as default in framework bundle.

+1 for a PSR-11 implem. (IMHO fits core/component)

@mvrhov did the work in sroze#3 and it has been merged within this PR 👏

sroze · 2017-12-25T11:01:35Z

src/Symfony/Bundle/FrameworkBundle/Command/MessageConsumeCommand.php

+    {
+        $this
+            ->setDefinition(array(
+                new InputArgument('consumer', InputArgument::REQUIRED, 'Name of the consumer'),


We renamed "consumer" to "receiver": this needs to reflect the change as well.

sroze · 2017-12-25T11:02:31Z

src/Symfony/Bundle/FrameworkBundle/Command/MessageConsumeCommand.php

+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+        /** @var ContainerInterface $container */
+        $container = $this->getApplication()->getKernel()->getContainer();


Correct, we can require the receivers (note the name change) to be tagged, so we can use service locators 👍

sroze · 2017-12-25T11:02:57Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/message.xml

+
+        <!-- Bus -->
+        <service id="message_bus" class="Symfony\Component\Message\MessageBus" public="true">
+            <argument type="collection" /> <!-- Middlewares -->


Yep, I think so 👍

sroze · 2017-12-25T11:03:55Z

src/Symfony/Component/Message/Debug/LoggingMiddleware.php

+     */
+    public function handle($message, callable $next)
+    {
+        $this->logger->debug('Starting processing message', array(


Only if the message is Starting processing {class} message and we add class as context I believe. Which... makes sense actually 👍

sroze · 2017-12-25T11:04:47Z

src/Symfony/Component/Message/HandlerLocator.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+class HandlerLocator implements HandlerLocatorInterface


@mvrhov did the work in sroze#3 and it has been merged within this PR 👏

ogizanagi · 2018-03-20T18:28:48Z

I'd also vote for Message for the reasons given by @sroze & @dunglas.

sroze · 2018-03-20T21:34:31Z

After maaany discussions, especially with @fabpot and @nicolas-grekas, the component is going to be called « Messenger ».

mvrhov · 2018-03-21T07:47:46Z

src/Symfony/Component/Message/DependencyInjection/MessagePass.php

+
+        $handlersLocatorMapping = array();
+        foreach ($handlersByMessage as $message => $handler) {
+            $handlersLocatorMapping['handles.'.$message] = $handler;


Isn't this one supposed to be handler just like in ContainerHandlerLocator.php line 33?
I'm curious why the test didn't catch this

pborreli · 2018-03-21T09:34:55Z

@sroze so now you have to rename your conference name at Symfony Live Paris 😄

sroze · 2018-03-21T09:32:13Z

src/Symfony/Bundle/FrameworkBundle/Controller/ControllerTrait.php

+    protected function dispatchMessage($message)
+    {
+        if (!$this->container->has('message_bus')) {
+            throw new \LogicException('The message bus is not enabled in your application. Try running "composer require symfony/message".');


Need to be renamed as well.

sroze · 2018-03-21T09:32:58Z

src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php

@@ -118,6 +119,7 @@ public function build(ContainerBuilder $container)
        $container->addCompilerPass(new ResettableServicePass());
        $container->addCompilerPass(new TestServiceContainerWeakRefPass(), PassConfig::TYPE_BEFORE_REMOVING, -32);
        $container->addCompilerPass(new TestServiceContainerRealRefPass(), PassConfig::TYPE_AFTER_REMOVING);
+        $this->addCompilerPassIfExists($container, MessagePass::class);


To be renamed to MessengerPass

sroze · 2018-03-21T09:33:27Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml

+
+        <!-- Bus -->
+        <service id="message_bus" class="Symfony\Component\Messenger\MessageBus" public="true">
+            <argument type="tagged" tag="message_middleware" /> <!-- Middlewares -->


To keep explicit, I'll therefore rename to message_bus_middleware.

sroze · 2018-03-21T09:33:53Z

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php

@@ -249,6 +250,10 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
                    ),
                ),
            ),
+            'message' => array(


messenger

sroze · 2018-03-21T09:34:38Z

src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocatorInterface.php

+interface SenderLocatorInterface
+{
+    /**
+     * Gets the producer (if applicable) for the given message object.


s/producer/sender

sroze · 2018-03-21T09:35:21Z

src/Symfony/Component/Messenger/Debug/LoggingMiddleware.php

+        try {
+            $result = $next($message);
+        } catch (\Throwable $e) {
+            $this->logger->warning('An exception occurred while processing message {class}', array(


Let's keep the same language: s/processing/handling

sroze · 2018-03-21T09:35:29Z

src/Symfony/Component/Messenger/Debug/LoggingMiddleware.php

+     */
+    public function handle($message, callable $next)
+    {
+        $this->logger->debug('Starting processing message {class}', array(


Let's keep the same language: s/processing/handling

sroze · 2018-03-21T09:35:36Z

src/Symfony/Component/Messenger/Debug/LoggingMiddleware.php

+            throw $e;
+        }
+
+        $this->logger->debug('Finished processing message {class}', array(


Let's keep the same language: s/processing/handling

sroze · 2018-03-21T09:35:49Z

src/Symfony/Component/Messenger/DependencyInjection/MessagePass.php

+/**
+ * @author Samuel Roze <samuel.roze@gmail.com>
+ */
+class MessagePass implements CompilerPassInterface


Should be MessengerPass

sroze · 2018-03-21T09:36:26Z

src/Symfony/Component/Messenger/DependencyInjection/MessagePass.php

+
+        $parameter = $parameters[0];
+        if (null === $parameter->getClass()) {
+            throw new RuntimeException(sprintf('The parameter of `__invoke` function of service "%s" must type hint the Message class it handles.', $serviceId));


Message => message

sroze · 2018-03-21T09:39:25Z

@pborreli I know 😅

ro0NL · 2018-03-21T09:48:08Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml

@@ -69,6 +69,13 @@
            <tag name="console.command" command="debug:event-dispatcher" />
        </service>

+        <service id="console.command.messenger_consume_message" class="Symfony\Bundle\FrameworkBundle\Command\MessengerConsumeMessagesCommand">


s\messenger_consume_message\messenger_consume_messages

GwendolenLynch

So much awesome! Thank you @sroze 🍾

ro0NL · 2018-03-21T10:39:13Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml

+        </service>
+
+        <service id="data_collector.messenger" class="Symfony\Bundle\FrameworkBundle\DataCollector\MessengerDataCollector">
+            <tag name="data_collector" template="@WebProfiler/Collector/messages.html.twig" id="messenger" priority="100" />


s\id="messenger"\id="messages"

this breaks the profiler :)

Changed sir' 👍

sroze · 2018-03-21T23:32:33Z

I have squashed all the commits and added tests for what I believe is really important for now. I'd argue the bits that aren't covered by tests will be covered as long as we keep the policy that every change in this experimental component has to be covered by tests.

Therefore, I'd say it's ready to be merged :)

fabpot · 2018-03-22T07:04:44Z

@sroze Can you add a sentence about the fact that the new component is experimental in the README file?

sroze · 2018-03-22T08:47:14Z

@fabpot just added a message and a link to the documentation 👍 (kept two commits so it's super easy to revert the experimental one before 4.2)

fabpot · 2018-03-23T08:02:05Z

Thank you @sroze.

mcfedr · 2018-03-26T20:56:33Z

This looks really interesting, I look forward to trying it out, I have been developing/using a library that solves a similar problem, but in a slightly different (probably less abstract/extensible) way, so you might interested to have a look, https://github.com/mcfedr/queue-manager-bundle, its been used in production on a number of sites that handle ~50 million messages/month - its less of a by-the-book solution, but thought might interesting for comparision

fre5h · 2018-03-27T10:38:09Z

src/Symfony/Bundle/FrameworkBundle/Controller/ControllerTrait.php

+     *
+     * @param object $message The message to dispatch
+     *
+     * @final


There is an annotation @final but method is not final itself

This PR was squashed before being merged into the 4.1-dev branch (closes #26632). Discussion ---------- [Messenger] Add AMQP adapter | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | ø | License | MIT - [x] Depends on the Messenger component #24411 - [x] Add tests once we are all happy about the structure --- In order to give a great DX for simple needs such as sending messages through an AMQP broker such as RabbitMq, we should ship an AMQP adapter for the Messenger component within Symfony Core. It should be as simple as this proposal. We don't need to handle more specific use-cases nor brokers as other adapters such as the [enqueue adapter](https://github.com/sroze/enqueue-bridge) can also be used. Configuring the adapter is as simple as the following configuration: ```yaml # config/packages/messenger_adapters.yaml framework: messenger: adapter: "%env(MESSENGER_DSN)%" ``` With the given `.env` for example: ``` MESSENGER_DSN=amqp://guest:guest@localhost:5672/%2f/messages ``` Keep in mind that after having configured the adapter, developers have to route their messages to the given adapter. ```yaml # config/packages/messenger_routes.yaml framework: messenger: routing: producer). 'App\Message\Command\CreateNumber': messenger.default_sender ``` --- Additionally, multiple adapters can be created and messages routed to these ones. ```yaml # config/packages/messenger_routes.yaml framework: messenger: adapters: commands: "amqp://guest:guest@localhost:5672/%2f/commands" maintenance: "amqp://guest:guest@localhost:5672/%2f/maintenance" routing: producer). 'App\Message\Command\CreateNumber': messenger.commands_sender 'App\Message\Command\MaintenanceSpecificCommand': messenger.maintenance_sender ``` Commits ------- 798c230 [Messenger] Add AMQP adapter

carsonbot added Status: Needs Review Feature labels Oct 3, 2017

sroze force-pushed the add-message-component branch from 9a45c8f to f342984 Compare October 3, 2017 16:45

nicolas-grekas added this to the 4.1 milestone Oct 3, 2017

sroze force-pushed the add-message-component branch from f342984 to a67efc3 Compare October 4, 2017 07:30

mvrhov reviewed Oct 6, 2017

View reviewed changes

sroze force-pushed the add-message-component branch from a67efc3 to 5fdb3e7 Compare October 6, 2017 13:35

everzet reviewed Oct 9, 2017

View reviewed changes

Koc reviewed Oct 9, 2017

View reviewed changes

arnolanglade reviewed Oct 10, 2017

View reviewed changes

javiereguiluz reviewed Dec 1, 2017

View reviewed changes

sroze commented Dec 1, 2017

View reviewed changes

sroze force-pushed the add-message-component branch from 1202153 to ba18c84 Compare December 1, 2017 16:46

sroze force-pushed the add-message-component branch 3 times, most recently from 3bd29c4 to ac80529 Compare December 24, 2017 10:48

Taluu reviewed Dec 24, 2017

View reviewed changes

ro0NL reviewed Dec 24, 2017

View reviewed changes

mvrhov reviewed Dec 24, 2017

View reviewed changes

sroze commented Dec 25, 2017

View reviewed changes

sroze force-pushed the add-message-component branch 4 times, most recently from 21ae05b to 2824e9a Compare December 25, 2017 12:11

sroze force-pushed the add-message-component branch from def7896 to 1b4be4e Compare March 21, 2018 07:43

mvrhov reviewed Mar 21, 2018

View reviewed changes

sroze changed the title ~~[Message] Add a new Message component~~ [Messenger] Add a new Messenger component Mar 21, 2018

sroze commented Mar 21, 2018

View reviewed changes

ro0NL approved these changes Mar 21, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Mar 21, 2018

GwendolenLynch approved these changes Mar 21, 2018

View reviewed changes

ro0NL reviewed Mar 21, 2018

View reviewed changes

Create the Messenger component

c091d66

sroze force-pushed the add-message-component branch from 86a91ec to c091d66 Compare March 21, 2018 23:26

Make the Messenger component as experimental

117b922

sroze force-pushed the add-message-component branch from c63befd to 117b922 Compare March 22, 2018 08:45

sroze mentioned this pull request Mar 22, 2018

[Messenger] Add AMQP adapter #26632

Merged

2 tasks

fabpot closed this in e157ded Mar 23, 2018

Guite mentioned this pull request Mar 23, 2018

Symfony 4 and 5 Guite/MostGenerator#1068

Closed

fre5h reviewed Mar 27, 2018

View reviewed changes

fabpot mentioned this pull request May 7, 2018

Release v4.1.0-BETA1 #27181

Merged

cmodijk mentioned this pull request Jun 18, 2018

The future of SimpleBus SimpleBus/SimpleBus#37

Open

[Messenger] Add a new Messenger component #24411

[Messenger] Add a new Messenger component #24411

Conversation

sroze commented Oct 3, 2017 • edited Loading

Resources

Important points

Changes from the proposals

Choose a reason for hiding this comment

everzet 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

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

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 Mar 13, 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvrhov commented Dec 1, 2017

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

Koc commented Dec 2, 2017

mvrhov commented Dec 2, 2017

Koc commented Dec 2, 2017

sroze commented Dec 3, 2017

mvrhov commented Dec 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL 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

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 commented Oct 3, 2017 •

edited

Loading

sroze Mar 13, 2018 •

edited

Loading

mvrhov commented Dec 23, 2017 •

edited

Loading