feature #31454 [Messenger] remove send_and_handle which can be achiev…

…ed 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  | BC breaks? | yes  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | | License | MIT | Doc PR | symfony/symfony-docs#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] ``` symfony/symfony#31401 (review) Commits ------- 4552b7f5b2 [Messenger] remove send_and_handle option which can be achieved with SyncTransport
symfony · May 11, 2019 · 01a1678 · 01a1678
2 parents 9a75b92 + f9ff33a
commit 01a1678
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 66 deletions.
diff --git a/CHANGELOG.md b/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).

diff --git a/Middleware/SendMessageMiddleware.php b/Middleware/SendMessageMiddleware.php
@@ -52,7 +52,6 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
             'class' => \get_class($envelope->getMessage()),
         ];
 
-        $handle = false;
         $sender = null;
 
         try {
@@ -65,7 +64,7 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
 
                 // 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 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
                     $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,14 +94,14 @@ public function handle(Envelope $envelope, StackInterface $stack): Envelope
     /**
      * * @return iterable|SenderInterface[]
      */
-    private function getSenders(Envelope $envelope, &$handle, ?RedeliveryStamp $redeliveryStamp): iterable
+    private function getSenders(Envelope $envelope, ?RedeliveryStamp $redeliveryStamp): iterable
     {
         if (null !== $redeliveryStamp) {
             return [
                 $redeliveryStamp->getSenderClassOrAlias() => $this->sendersLocator->getSenderByAlias($redeliveryStamp->getSenderClassOrAlias()),
             ];
         }
 
-        return $this->sendersLocator->getSenders($envelope, $handle);
+        return $this->sendersLocator->getSenders($envelope);
     }
 }
diff --git a/Tests/Middleware/SendMessageMiddlewareTest.php b/Tests/Middleware/SendMessageMiddlewareTest.php
@@ -92,11 +92,7 @@ public function testItSendsToOnlyOneSenderOnRedelivery()
 
         $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 @@ public function testItSendsTheMessageToAssignedSenderWithPreWrappedMessage()
         $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 @@ public function testItDoesNotDispatchOnRedeliver()
         $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 @@ private function createSendersLocator(array $sendersMap, array $senders, array $
                 return $senders[$id];
             });
 
-        return new SendersLocator($sendersMap, $container, $sendAndHandle);
+        return new SendersLocator($sendersMap, $container);
     }
 }
diff --git a/Transport/Sender/SendersLocator.php b/Transport/Sender/SendersLocator.php
@@ -30,36 +30,30 @@ 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;
 
         if (\is_array($sendersLocator) || null === $sendersLocator) {
             @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 @@ public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable
                     }
                 }
 
-                $handle = $handle ?: $this->sendAndHandle[$type] ?? false;
-
                 continue;
             }
 
@@ -87,11 +79,7 @@ public function getSenders(Envelope $envelope, ?bool &$handle = false): iterable
                     yield $senderAlias => $sender;
                 }
             }
-
-            $handle = $handle ?: $this->sendAndHandle[$type] ?? false;
         }
-
-        $handle = $handle || null === $sender;
     }
 
     public function getSenderByAlias(string $alias): SenderInterface

diff --git a/Transport/Sender/SendersLocatorInterface.php b/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.