feature #31454 [Messenger] remove send_and_handle which can be achieved with SyncTransport (Tobion)

This PR was merged into the 4.3 branch.

Discussion
----------

[Messenger] remove send_and_handle which can be achieved with SyncTransport

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | no
| New feature?  | yes/no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | yes     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets |
| License       | MIT
| Doc PR        | https://github.com/symfony/symfony-docs/issues/11236

The send_and_handle option is pretty awkward and we don't need it anymore because the same thing can be achieved with the SyncTransport from #30759

So the following example from the doc in https://symfony.com/doc/current/messenger.html#routing
```yaml
framework:
    messenger:
        routing:
            'My\Message\ThatIsGoingToBeSentAndHandledLocally':
                 senders: [amqp]
                 send_and_handle: true
```
is the same as
```yaml
framework:
    messenger:
        routing:
            'My\Message\ThatIsGoingToBeSentAndHandledLocally':
                 senders: [amqp, sync]
```

https://github.com/symfony/symfony/pull/31401#pullrequestreview-235396370

Commits
-------

4552b7f5b2 [Messenger] remove send_and_handle option which can be achieved with SyncTransport

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

@@ -22,6 +22,8 @@ CHANGELOG
    `framework.messenger.default_serializer`, which holds the string service
    id and `framework.messenger.symfony_serializer`, which configures the
    options if you're using Symfony's serializer.
+ * [BC Break] Removed the `framework.messenger.routing.send_and_handle` configuration.
+   Instead of setting it to true, configure a `SyncTransport` and route messages to it.
  * Added information about deprecated aliases in `debug:autowiring` 
  * Added php ini session options `sid_length` and `sid_bits_per_character` 
    to the `session` section of the configuration

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -1125,7 +1125,6 @@ class Configuration implements ConfigurationInterface
                                         if (!\is_int($k)) {
                                             $newConfig[$k] = [
                                                 'senders' => $v['senders'] ?? (\is_array($v) ? array_values($v) : [$v]),
-                                                'send_and_handle' => $v['send_and_handle'] ?? false,
                                             ];
                                         } else {
                                             $newConfig[$v['message-class']]['senders'] = array_map(
@@ -1134,7 +1133,6 @@ class Configuration implements ConfigurationInterface
                                                 },
                                                 array_values($v['sender'])
                                             );
-                                            $newConfig[$v['message-class']]['send-and-handle'] = $v['send-and-handle'] ?? false;
                                         }
                                     }
 
@@ -1147,7 +1145,6 @@ class Configuration implements ConfigurationInterface
                                         ->requiresAtLeastOneElement()
                                         ->prototype('scalar')->end()
                                     ->end()
-                                    ->booleanNode('send_and_handle')->defaultFalse()->end()
                                 ->end()
                             ->end()
                         ->end()

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -1745,7 +1745,6 @@ class FrameworkExtension extends Extension
         }
 
         $messageToSendersMapping = [];
-        $messagesToSendAndHandle = [];
         foreach ($config['routing'] as $message => $messageConfiguration) {
             if ('*' !== $message && !class_exists($message) && !interface_exists($message, false)) {
                 throw new LogicException(sprintf('Invalid Messenger routing configuration: class or interface "%s" not found.', $message));
@@ -1759,7 +1758,6 @@ class FrameworkExtension extends Extension
             }
 
             $messageToSendersMapping[$message] = $messageConfiguration['senders'];
-            $messagesToSendAndHandle[$message] = $messageConfiguration['send_and_handle'];
         }
 
         $senderReferences = [];
@@ -1770,7 +1768,6 @@ class FrameworkExtension extends Extension
         $container->getDefinition('messenger.senders_locator')
             ->replaceArgument(0, $messageToSendersMapping)
             ->replaceArgument(1, ServiceLocatorTagPass::register($container, $senderReferences))
-            ->replaceArgument(2, $messagesToSendAndHandle)
         ;
 
         $container->getDefinition('messenger.retry_strategy_locator')

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

@@ -11,7 +11,6 @@
         <service id="messenger.senders_locator" class="Symfony\Component\Messenger\Transport\Sender\SendersLocator">
             <argument type="collection" /> <!-- Per message senders map -->
             <argument /> <!-- senders locator -->
-            <argument type="collection" /> <!-- Messages to send and handle -->
         </service>
         <service id="messenger.middleware.send_message" class="Symfony\Component\Messenger\Middleware\SendMessageMiddleware">
             <tag name="monolog.logger" channel="messenger" />

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -434,7 +434,6 @@
             <xsd:element name="sender" type="messenger_routing_sender" />
         </xsd:choice>
         <xsd:attribute name="message-class" type="xsd:string" use="required"/>
-        <xsd:attribute name="send-and-handle" type="xsd:boolean" default="false"/>
     </xsd:complexType>
 
     <xsd:complexType name="messenger_routing_sender">

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php

@@ -10,7 +10,6 @@ $container->loadFromExtension('framework', [
             'Symfony\Component\Messenger\Tests\Fixtures\DummyMessage' => ['amqp', 'audit'],
             'Symfony\Component\Messenger\Tests\Fixtures\SecondMessage' => [
                 'senders' => ['amqp', 'audit'],
-                'send_and_handle' => true,
             ],
             '*' => 'amqp',
         ],

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml

@@ -13,7 +13,7 @@
                 <framework:sender service="amqp" />
                 <framework:sender service="audit" />
             </framework:routing>
-            <framework:routing message-class="Symfony\Component\Messenger\Tests\Fixtures\SecondMessage" send-and-handle="true">
+            <framework:routing message-class="Symfony\Component\Messenger\Tests\Fixtures\SecondMessage">
                 <framework:sender service="amqp" />
                 <framework:sender service="audit" />
             </framework:routing>

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml

@@ -7,7 +7,6 @@ framework:
             'Symfony\Component\Messenger\Tests\Fixtures\DummyMessage': [amqp, audit]
             'Symfony\Component\Messenger\Tests\Fixtures\SecondMessage':
                 senders: [amqp, audit]
-                send_and_handle: true
             '*': amqp
         transports:
             amqp: 'amqp://localhost/%2f/messages'

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

@@ -40,7 +40,6 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpClient\ScopingHttpClient;
 use Symfony\Component\HttpKernel\DependencyInjection\LoggerPass;
 use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage;
-use Symfony\Component\Messenger\Tests\Fixtures\SecondMessage;
 use Symfony\Component\Messenger\Transport\TransportFactory;
 use Symfony\Component\PropertyAccess\PropertyAccessor;
 use Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader;
@@ -715,13 +714,6 @@ abstract class FrameworkExtensionTest extends TestCase
         $container = $this->createContainerFromFile('messenger_routing');
         $senderLocatorDefinition = $container->getDefinition('messenger.senders_locator');
 
-        $messageToSendAndHandleMapping = [
-            DummyMessage::class => false,
-            SecondMessage::class => true,
-            '*' => false,
-        ];
-
-        $this->assertSame($messageToSendAndHandleMapping, $senderLocatorDefinition->getArgument(2));
         $sendersMapping = $senderLocatorDefinition->getArgument(0);
         $this->assertEquals([
             'amqp',

src/Symfony/Component/Messenger/CHANGELOG.md

@@ -6,6 +6,7 @@ CHANGELOG
 
  * [BC BREAK] `SendersLocatorInterface` has an additional method:
    `getSenderByAlias()`.
+ * Removed argument `?bool &$handle = false` from `SendersLocatorInterface::getSenders`
  * A new `ListableReceiverInterface` was added, which a receiver
    can implement (when applicable) to enable listing and fetching
    individual messages by id (used in the new "Failed Messages" commands).

src/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php

@@ -52,7 +52,6 @@ class SendMessageMiddleware implements MiddlewareInterface
             'class' => \get_class($envelope->getMessage()),
         ];
 
-        $handle = false;
         $sender = null;
 
         try {
@@ -65,7 +64,7 @@ class SendMessageMiddleware implements MiddlewareInterface
 
                 // dispatch event unless this is a redelivery
                 $shouldDispatchEvent = null === $redeliveryStamp;
-                foreach ($this->getSenders($envelope, $handle, $redeliveryStamp) as $alias => $sender) {
+                foreach ($this->getSenders($envelope, $redeliveryStamp) as $alias => $sender) {
                     if (null !== $this->eventDispatcher && $shouldDispatchEvent) {
                         $event = new SendMessageToTransportsEvent($envelope);
                         $this->eventDispatcher->dispatch($event);
@@ -76,14 +75,9 @@ class SendMessageMiddleware implements MiddlewareInterface
                     $this->logger->info('Sending message "{class}" with "{sender}"', $context + ['sender' => \get_class($sender)]);
                     $envelope = $sender->send($envelope->with(new SentStamp(\get_class($sender), \is_string($alias) ? $alias : null)));
                 }
-
-                // on a redelivery, only send back to queue: never call local handlers
-                if (null !== $redeliveryStamp) {
-                    $handle = false;
-                }
             }
 
-            if (null === $sender || $handle) {
+            if (null === $sender) {
                 return $stack->next()->handle($envelope, $stack);
             }
         } catch (\Throwable $e) {
@@ -100,7 +94,7 @@ class SendMessageMiddleware implements MiddlewareInterface
     /**
      * * @return iterable|SenderInterface[]
      */
-    private function getSenders(Envelope $envelope, &$handle, ?RedeliveryStamp $redeliveryStamp): iterable
+    private function getSenders(Envelope $envelope, ?RedeliveryStamp $redeliveryStamp): iterable
     {
         if (null !== $redeliveryStamp) {
             return [
@@ -108,6 +102,6 @@ class SendMessageMiddleware implements MiddlewareInterface
             ];
         }
 
-        return $this->sendersLocator->getSenders($envelope, $handle);
+        return $this->sendersLocator->getSenders($envelope);
     }
 }

src/Symfony/Component/Messenger/Tests/Middleware/SendMessageMiddlewareTest.php

@@ -92,11 +92,7 @@ class SendMessageMiddlewareTest extends MiddlewareTestCase
 
         $sendersLocator = $this->createSendersLocator(
             [DummyMessage::class => ['foo', 'bar']],
-            ['foo' => $sender, 'bar' => $sender2],
-            [
-                // normally, this class sends and handles (but not on retry)
-                DummyMessage::class => true,
-            ]
+            ['foo' => $sender, 'bar' => $sender2]
         );
 
         $middleware = new SendMessageMiddleware($sendersLocator);
@@ -126,68 +122,46 @@ class SendMessageMiddlewareTest extends MiddlewareTestCase
         $middleware->handle($envelope, $this->getStackMock(false));
     }
 
-    public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageClass()
-    {
-        $message = new DummyMessage('Hey');
-        $envelope = new Envelope($message);
-        $sender = $this->getMockBuilder(SenderInterface::class)->getMock();
-
-        $sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
-            DummyMessage::class => true,
-        ]);
-        $middleware = new SendMessageMiddleware($sendersLocator);
-
-        $sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);
-
-        $middleware->handle($envelope, $this->getStackMock());
-    }
-
-    public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageParentClass()
+    public function testItSendsTheMessageBasedOnTheMessageParentClass()
     {
         $message = new ChildDummyMessage('Hey');
         $envelope = new Envelope($message);
         $sender = $this->getMockBuilder(SenderInterface::class)->getMock();
 
-        $sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
-            DummyMessage::class => true,
-        ]);
+        $sendersLocator = $this->createSendersLocator([DummyMessage::class => ['foo_sender']], ['foo_sender' => $sender]);
         $middleware = new SendMessageMiddleware($sendersLocator);
 
         $sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);
 
-        $middleware->handle($envelope, $this->getStackMock());
+        $middleware->handle($envelope, $this->getStackMock(false));
     }
 
-    public function testItAlsoCallsTheNextMiddlewareBasedOnTheMessageInterface()
+    public function testItSendsTheMessageBasedOnTheMessageInterface()
     {
         $message = new DummyMessage('Hey');
         $envelope = new Envelope($message);
         $sender = $this->getMockBuilder(SenderInterface::class)->getMock();
 
-        $sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
-            DummyMessageInterface::class => true,
-        ]);
+        $sendersLocator = $this->createSendersLocator([DummyMessageInterface::class => ['foo_sender']], ['foo_sender' => $sender]);
         $middleware = new SendMessageMiddleware($sendersLocator);
 
         $sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);
 
-        $middleware->handle($envelope, $this->getStackMock());
+        $middleware->handle($envelope, $this->getStackMock(false));
     }
 
-    public function testItAlsoCallsTheNextMiddlewareBasedOnWildcard()
+    public function testItSendsTheMessageBasedOnWildcard()
     {
         $message = new DummyMessage('Hey');
         $envelope = new Envelope($message);
         $sender = $this->getMockBuilder(SenderInterface::class)->getMock();
 
-        $sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender], [
-            '*' => true,
-        ]);
+        $sendersLocator = $this->createSendersLocator(['*' => ['foo_sender']], ['foo_sender' => $sender]);
         $middleware = new SendMessageMiddleware($sendersLocator);
 
         $sender->expects($this->once())->method('send')->with($envelope->with(new SentStamp(\get_class($sender), 'foo_sender')))->willReturn($envelope);
 
-        $middleware->handle($envelope, $this->getStackMock());
+        $middleware->handle($envelope, $this->getStackMock(false));
     }
 
     public function testItCallsTheNextMiddlewareWhenNoSenderForThisMessage()
@@ -267,7 +241,7 @@ class SendMessageMiddlewareTest extends MiddlewareTestCase
         $middleware->handle($envelope, $this->getStackMock(false));
     }
 
-    private function createSendersLocator(array $sendersMap, array $senders, array $sendAndHandle = [])
+    private function createSendersLocator(array $sendersMap, array $senders)
     {
         $container = $this->createMock(ContainerInterface::class);
         $container->expects($this->any())
@@ -281,6 +255,6 @@ class SendMessageMiddlewareTest extends MiddlewareTestCase
                 return $senders[$id];
             });
 
-        return new SendersLocator($sendersMap, $container, $sendAndHandle);
+        return new SendersLocator($sendersMap, $container);
     }
 }

src/Symfony/Component/Messenger/Transport/Sender/SendersLocator.php

@@ -30,14 +30,12 @@ class SendersLocator implements SendersLocatorInterface
     private $sendersMap;
     private $sendersLocator;
     private $useLegacyLookup = false;
-    private $sendAndHandle;
 
     /**
      * @param string[][]         $sendersMap     An array, keyed by "type", set to an array of sender aliases
      * @param ContainerInterface $sendersLocator Locator of senders, keyed by sender alias
-     * @param bool[]             $sendAndHandle
      */
-    public function __construct(array $sendersMap, /*ContainerInterface*/ $sendersLocator = null, array $sendAndHandle = [])
+    public function __construct(array $sendersMap, /*ContainerInterface*/ $sendersLocator = null)
     {
         $this->sendersMap = $sendersMap;
 
@@ -45,21 +43,17 @@ class SendersLocator implements SendersLocatorInterface
             @trigger_error(sprintf('"%s::__construct()" requires a "%s" as 2nd argument. Not doing so is deprecated since Symfony 4.3 and will be required in 5.0.', __CLASS__, ContainerInterface::class), E_USER_DEPRECATED);
             // "%s" requires a "%s" as 2nd argument. Not doing so is deprecated since Symfony 4.3 and will be required in 5.0.'
             $this->sendersLocator = new ServiceLocator([]);
-            $this->sendAndHandle = $sendersLocator;
             $this->useLegacyLookup = true;
         } else {
             $this->sendersLocator = $sendersLocator;
-            $this->sendAndHandle = $sendAndHandle;
         }
     }
 
     /**
      * {@inheritdoc}
      */
-    public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable
+    public function getSenders(Envelope $envelope): iterable
     {
-        $handle = false;
-        $sender = null;
         $seen = [];
 
         foreach (HandlersLocator::listTypes($envelope) as $type) {
@@ -71,8 +65,6 @@ class SendersLocator implements SendersLocatorInterface
                     }
                 }
 
-                $handle = $handle ?: $this->sendAndHandle[$type] ?? false;
-
                 continue;
             }
 
@@ -87,11 +79,7 @@ class SendersLocator implements SendersLocatorInterface
                     yield $senderAlias => $sender;
                 }
             }
-
-            $handle = $handle ?: $this->sendAndHandle[$type] ?? false;
         }
-
-        $handle = $handle || null === $sender;
     }
 
     public function getSenderByAlias(string $alias): SenderInterface

src/Symfony/Component/Messenger/Transport/Sender/SendersLocatorInterface.php

@@ -27,12 +27,9 @@ interface SendersLocatorInterface
     /**
      * Gets the senders for the given message name.
      *
-     * @param bool|null &$handle True after calling the method when the next middleware
-     *                           should also get the message; false otherwise
-     *
      * @return iterable|SenderInterface[] Indexed by sender alias if available
      */
-    public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable;
+    public function getSenders(Envelope $envelope): iterable;
 
     /**
      * Returns a specific sender by its alias.