minor #28908 [Messenger] internal cleanups (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Messenger] internal cleanups

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

From the updated changelog:
 * `MessengerDataCollector::getMessages()` returns an iterable, not just an array anymore
 * `AbstractHandlerLocator` is now internal
 * `HandlerLocatorInterface::resolve()` has been replaced by `getHandler(Envelope $envelope)`
 * `SenderLocatorInterface::getSenderForMessage()` has been replaced by `getSender(Envelope $envelope)`
 * `SenderInterface::send()` returns `void`

+ some internal simplifications

Commits
-------

4a3edd0b37 [Messenger] internal cleanups

Author: sroze

Asynchronous/Middleware/SendMessageMiddleware.php

@@ -44,21 +44,17 @@ public function handle(Envelope $envelope, callable $next): void
             return;
         }
 
-        $sender = $this->senderLocator->getSenderForMessage($envelope->getMessage());
+        $sender = $this->senderLocator->getSender($envelope);
 
         if ($sender) {
             $sender->send($envelope);
 
-            if (!$this->mustSendAndHandle($envelope->getMessage())) {
+            if (!AbstractSenderLocator::getValueFromMessageRouting($this->messagesToSendAndHandleMapping, $envelope)) {
+                // message has no corresponding handler
                 return;
             }
         }
 
         $next($envelope);
     }
-
-    private function mustSendAndHandle($message): bool
-    {
-        return (bool) AbstractSenderLocator::getValueFromMessageRouting($this->messagesToSendAndHandleMapping, $message);
-    }
 }

Asynchronous/Routing/AbstractSenderLocator.php

@@ -11,28 +11,33 @@
 
 namespace Symfony\Component\Messenger\Asynchronous\Routing;
 
+use Symfony\Component\Messenger\Envelope;
+
 /**
  * @author Samuel Roze <[email protected]>
  *
  * @internal
  */
 abstract class AbstractSenderLocator implements SenderLocatorInterface
 {
-    public static function getValueFromMessageRouting(array $mapping, $message)
+    public static function getValueFromMessageRouting(array $mapping, Envelope $envelope)
     {
-        if (isset($mapping[\get_class($message)])) {
-            return $mapping[\get_class($message)];
-        }
-        if ($parentsMapping = array_intersect_key($mapping, class_parents($message))) {
-            return current($parentsMapping);
+        if (isset($mapping[$class = \get_class($envelope->getMessage())])) {
+            return $mapping[$class];
         }
-        if ($interfaceMapping = array_intersect_key($mapping, class_implements($message))) {
-            return current($interfaceMapping);
+
+        foreach (class_parents($class) as $name) {
+            if (isset($mapping[$name])) {
+                return $mapping[$name];
+            }
         }
-        if (isset($mapping['*'])) {
-            return $mapping['*'];
+
+        foreach (class_implements($class) as $name) {
+            if (isset($mapping[$name])) {
+                return $mapping[$name];
+            }
         }
 
-        return null;
+        return $mapping['*'] ?? null;
     }
 }

Asynchronous/Routing/ContainerSenderLocator.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\Messenger\Asynchronous\Routing;
 
 use Psr\Container\ContainerInterface;
+use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Transport\SenderInterface;
 
 /**
@@ -31,9 +32,9 @@ public function __construct(ContainerInterface $senderServiceLocator, array $mes
     /**
      * {@inheritdoc}
      */
-    public function getSenderForMessage($message): ?SenderInterface
+    public function getSender(Envelope $envelope): ?SenderInterface
     {
-        $senderId = self::getValueFromMessageRouting($this->messageToSenderIdMapping, $message);
+        $senderId = self::getValueFromMessageRouting($this->messageToSenderIdMapping, $envelope);
 
         return $senderId ? $this->senderServiceLocator->get($senderId) : null;
     }

Asynchronous/Routing/SenderLocator.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Messenger\Asynchronous\Routing;
 
+use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Exception\RuntimeException;
 use Symfony\Component\Messenger\Transport\SenderInterface;
 
@@ -29,15 +30,15 @@ public function __construct(array $messageToSenderMapping)
     /**
      * {@inheritdoc}
      */
-    public function getSenderForMessage($message): ?SenderInterface
+    public function getSender(Envelope $envelope): ?SenderInterface
     {
-        $sender = self::getValueFromMessageRouting($this->messageToSenderMapping, $message);
+        $sender = self::getValueFromMessageRouting($this->messageToSenderMapping, $envelope);
         if (null === $sender) {
             return null;
         }
 
         if (!$sender instanceof SenderInterface) {
-            throw new RuntimeException(sprintf('The sender instance provided for message "%s" should be of type "%s" but got "%s".', \get_class($message), SenderInterface::class, \is_object($sender) ? \get_class($sender) : \gettype($sender)));
+            throw new RuntimeException(sprintf('The sender instance provided for message "%s" should be of type "%s" but got "%s".', \get_class($envelope->getMessage()), SenderInterface::class, \is_object($sender) ? \get_class($sender) : \gettype($sender)));
         }
 
         return $sender;

Asynchronous/Routing/SenderLocatorInterface.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Messenger\Asynchronous\Routing;
 
+use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Transport\SenderInterface;
 
 /**
@@ -21,10 +22,6 @@ interface SenderLocatorInterface
 {
     /**
      * Gets the sender (if applicable) for the given message object.
-     *
-     * @param object $message
-     *
-     * @return SenderInterface|null
      */
-    public function getSenderForMessage($message): ?SenderInterface;
+    public function getSender(Envelope $envelope): ?SenderInterface;
 }

CHANGELOG.md

@@ -27,6 +27,12 @@ CHANGELOG
  * `Envelope`'s constructor and `with()` method now accept `StampInterface` objects as variadic parameters
  * Renamed and moved `ReceivedMessage`, `ValidationConfiguration` and `SerializerConfiguration` in the `Stamp` namespace
  * Removed the `WrapIntoReceivedMessage`
+ * `SenderLocatorInterface::getSenderForMessage()` has been replaced by `getSender(Envelope $envelope)`
+ * `MessengerDataCollector::getMessages()` returns an iterable, not just an array anymore
+ * `AbstractHandlerLocator` is now internal
+ * `HandlerLocatorInterface::resolve()` has been replaced by `getHandler(Envelope $envelope)`
+ * `SenderLocatorInterface::getSenderForMessage()` has been replaced by `getSender(Envelope $envelope)`
+ * `SenderInterface::send()` returns `void`
 
 4.1.0
 -----

DataCollector/MessengerDataCollector.php

@@ -17,7 +17,6 @@
 use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface;
 use Symfony\Component\Messenger\TraceableMessageBus;
 use Symfony\Component\VarDumper\Caster\ClassStub;
-use Symfony\Component\VarDumper\Cloner\Data;
 
 /**
  * @author Samuel Roze <[email protected]>
@@ -55,14 +54,10 @@ public function lateCollect()
         }
 
         // Order by call time
-        usort($messages, function (array $a, array $b): int {
-            return $a[1] > $b[1] ? 1 : -1;
-        });
+        usort($messages, function ($a, $b) { return $a[1] <=> $b[1]; });
 
         // Keep the messages clones only
-        $this->data['messages'] = array_map(function (array $item): Data {
-            return $item[0];
-        }, $messages);
+        $this->data['messages'] = array_column($messages, 0);
     }
 
     /**
@@ -112,18 +107,21 @@ private function collectMessage(string $busName, array $tracedMessage)
 
     public function getExceptionsCount(string $bus = null): int
     {
-        return array_reduce($this->getMessages($bus), function (int $carry, Data $message) {
-            return $carry += isset($message['exception']) ? 1 : 0;
-        }, 0);
+        $count = 0;
+        foreach ($this->getMessages($bus) as $message) {
+            $count += (int) isset($message['exception']);
+        }
+
+        return $count;
     }
 
-    public function getMessages(string $bus = null): array
+    public function getMessages(string $bus = null): iterable
     {
-        $messages = $this->data['messages'] ?? array();
-
-        return $bus ? array_filter($messages, function (Data $message) use ($bus): bool {
-            return $bus === $message['bus'];
-        }) : $messages;
+        foreach ($this->data['messages'] ?? array() as $message) {
+            if (null === $bus || $bus === $message['bus']) {
+                yield $message;
+            }
+        }
     }
 
     public function getBuses(): array

Envelope.php

@@ -28,6 +28,9 @@ final class Envelope
      */
     public function __construct($message, StampInterface ...$stamps)
     {
+        if (!\is_object($message)) {
+            throw new \TypeError(sprintf('Invalid argument provided to "%s()": expected object but got %s.', __METHOD__, \gettype($message)));
+        }
         $this->message = $message;
 
         foreach ($stamps as $stamp) {

Handler/ChainHandler.php

@@ -30,7 +30,7 @@ class ChainHandler
      */
     public function __construct(array $handlers)
     {
-        if (empty($handlers)) {
+        if (!$handlers) {
             throw new InvalidArgumentException('A collection of message handlers requires at least one handler.');
         }
 

Handler/Locator/AbstractHandlerLocator.php

@@ -11,36 +11,39 @@
 
 namespace Symfony\Component\Messenger\Handler\Locator;
 
+use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;
 
 /**
  * @author Miha Vrhovnik <[email protected]>
  * @author Samuel Roze <[email protected]>
+ *
+ * @internal
  */
 abstract class AbstractHandlerLocator implements HandlerLocatorInterface
 {
-    public function resolve($message): callable
+    public function getHandler(Envelope $envelope): callable
     {
-        $class = \get_class($message);
+        $class = \get_class($envelope->getMessage());
 
-        if ($handler = $this->getHandler($class)) {
+        if ($handler = $this->getHandlerByName($class)) {
             return $handler;
         }
 
-        foreach (class_implements($class, false) as $interface) {
-            if ($handler = $this->getHandler($interface)) {
+        foreach (class_parents($class) as $name) {
+            if ($handler = $this->getHandlerByName($name)) {
                 return $handler;
             }
         }
 
-        foreach (class_parents($class, false) as $parent) {
-            if ($handler = $this->getHandler($parent)) {
+        foreach (class_implements($class) as $name) {
+            if ($handler = $this->getHandlerByName($name)) {
                 return $handler;
             }
         }
 
         throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $class));
     }
 
-    abstract protected function getHandler(string $class): ?callable;
+    abstract protected function getHandlerByName(string $name): ?callable;
 }