From 3ac6bf9f2403a9db4a39a82cc3927415500b65e0 Mon Sep 17 00:00:00 2001
From: Ryan Weaver <ryan@thatsquality.com>
Date: Sun, 24 Mar 2019 22:13:23 -0400
Subject: [PATCH] Avoid dispatching SendMessageToTransportsEvent on redeliver

This purpose of this event is to be a hook when a message is sent to a transport.
If that message is redelivered later, that's not the purpose of this hook (there
are Worker events for that) and could cause problems if the user unknowingly
tries to modify the Envelope in some way, not thinking about how this might
be a redelivery message.
---
 .../Event/SendMessageToTransportsEvent.php    |  2 ++
 .../Middleware/SendMessageMiddleware.php      | 19 ++++++------
 .../Middleware/SendMessageMiddlewareTest.php  | 29 ++++++++++++++++++-
 3 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/Symfony/Component/Messenger/Event/SendMessageToTransportsEvent.php b/src/Symfony/Component/Messenger/Event/SendMessageToTransportsEvent.php
index a54622e83c..e0f3a07137 100644
--- a/src/Symfony/Component/Messenger/Event/SendMessageToTransportsEvent.php
+++ b/src/Symfony/Component/Messenger/Event/SendMessageToTransportsEvent.php
@@ -20,6 +20,8 @@ use Symfony\Component\Messenger\Envelope;
  * The event is *only* dispatched if the message will actually
  * be sent to at least one transport. If the message is sent
  * to multiple transports, the message is dispatched only one time.
+ * This message is only dispatched the first time a message
+ * is sent to a transport, not also if it is retried.
  *
  * @author Ryan Weaver <ryan@symfonycasts.com>
  */
diff --git a/src/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php b/src/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php
index e389da910c..b4b50f9e89 100644
--- a/src/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php
+++ b/src/Symfony/Component/Messenger/Middleware/SendMessageMiddleware.php
@@ -62,20 +62,21 @@ class SendMessageMiddleware implements MiddlewareInterface
                 /** @var RedeliveryStamp|null $redeliveryStamp */
                 $redeliveryStamp = $envelope->last(RedeliveryStamp::class);
 
-                $senders = \iterator_to_array($this->sendersLocator->getSenders($envelope, $handle));
-
-                if (null !== $this->eventDispatcher && \count($senders) > 0) {
-                    $event = new SendMessageToTransportsEvent($envelope);
-                    $this->eventDispatcher->dispatch($event);
-                    $envelope = $event->getEnvelope();
-                }
-
-                foreach ($senders as $alias => $sender) {
+                // dispatch event unless this is a redelivery
+                $shouldDispatchEvent = null === $redeliveryStamp;
+                foreach ($this->sendersLocator->getSenders($envelope, $handle) as $alias => $sender) {
                     // on redelivery, only deliver to the given sender
                     if (null !== $redeliveryStamp && !$redeliveryStamp->shouldRedeliverToSender(\get_class($sender), $alias)) {
                         continue;
                     }
 
+                    if (null !== $this->eventDispatcher && $shouldDispatchEvent) {
+                        $event = new SendMessageToTransportsEvent($envelope);
+                        $this->eventDispatcher->dispatch($event);
+                        $envelope = $event->getEnvelope();
+                        $shouldDispatchEvent = false;
+                    }
+
                     $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)));
                 }
diff --git a/src/Symfony/Component/Messenger/Tests/Middleware/SendMessageMiddlewareTest.php b/src/Symfony/Component/Messenger/Tests/Middleware/SendMessageMiddlewareTest.php
index 8237838a3a..0a23a0dbba 100644
--- a/src/Symfony/Component/Messenger/Tests/Middleware/SendMessageMiddlewareTest.php
+++ b/src/Symfony/Component/Messenger/Tests/Middleware/SendMessageMiddlewareTest.php
@@ -205,7 +205,7 @@ class SendMessageMiddlewareTest extends MiddlewareTestCase
         $this->assertNull($envelope->last(SentStamp::class), 'it does not add sent stamp for received messages');
     }
 
-    public function testItDispatchesTheEventOnceTime()
+    public function testItDispatchesTheEventOneTime()
     {
         $envelope = new Envelope(new DummyMessage('original envelope'));
 
@@ -224,4 +224,31 @@ class SendMessageMiddlewareTest extends MiddlewareTestCase
 
         $middleware->handle($envelope, $this->getStackMock(false));
     }
+
+    public function testItDoesNotDispatchWithNoSenders()
+    {
+        $envelope = new Envelope(new DummyMessage('original envelope'));
+
+        $dispatcher = $this->createMock(EventDispatcherInterface::class);
+        $dispatcher->expects($this->never())->method('dispatch');
+
+        $middleware = new SendMessageMiddleware(new SendersLocator([]), $dispatcher);
+
+        $middleware->handle($envelope, $this->getStackMock());
+    }
+
+    public function testItDoesNotDispatchOnRetry()
+    {
+        $envelope = new Envelope(new DummyMessage('original envelope'));
+        $envelope = $envelope->with(new RedeliveryStamp(3, 'foo_sender'));
+
+        $dispatcher = $this->createMock(EventDispatcherInterface::class);
+        $dispatcher->expects($this->never())->method('dispatch');
+
+        $sender = $this->getMockBuilder(SenderInterface::class)->getMock();
+
+        $middleware = new SendMessageMiddleware(new SendersLocator([DummyMessage::class => [$sender]]), $dispatcher);
+
+        $middleware->handle($envelope, $this->getStackMock(false));
+    }
 }