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 ------- 4a3edd0 [Messenger] internal cleanups
symfony · Oct 21, 2018 · 9aaec94 · 9aaec94
2 parents 5909a75 + 4a3edd0
commit 9aaec94
Show file tree

Hide file tree

Showing 26 changed files with 131 additions and 140 deletions.
diff --git a/src/Symfony/Component/Messenger/Asynchronous/Middleware/SendMessageMiddleware.php b/src/Symfony/Component/Messenger/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);
-    }
 }
diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/AbstractSenderLocator.php b/src/Symfony/Component/Messenger/Asynchronous/Routing/AbstractSenderLocator.php
@@ -11,28 +11,33 @@
 
 namespace Symfony\Component\Messenger\Asynchronous\Routing;
 
+use Symfony\Component\Messenger\Envelope;
+
 /**
  * @author Samuel Roze <samuel.roze@gmail.com>
  *
  * @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;
     }
 }
diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/ContainerSenderLocator.php b/src/Symfony/Component/Messenger/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;
     }

diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocator.php b/src/Symfony/Component/Messenger/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;

diff --git a/src/Symfony/Component/Messenger/Asynchronous/Routing/SenderLocatorInterface.php b/src/Symfony/Component/Messenger/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;
 }
diff --git a/src/Symfony/Component/Messenger/CHANGELOG.md b/src/Symfony/Component/Messenger/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
 -----

diff --git a/src/Symfony/Component/Messenger/DataCollector/MessengerDataCollector.php b/src/Symfony/Component/Messenger/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 <samuel.roze@gmail.com>
@@ -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

diff --git a/src/Symfony/Component/Messenger/Envelope.php b/src/Symfony/Component/Messenger/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) {

diff --git a/src/Symfony/Component/Messenger/Handler/ChainHandler.php b/src/Symfony/Component/Messenger/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.');
         }
 

diff --git a/src/Symfony/Component/Messenger/Handler/Locator/AbstractHandlerLocator.php b/src/Symfony/Component/Messenger/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 <miha.vrhovnik@gmail.com>
  * @author Samuel Roze <samuel.roze@gmail.com>
+ *
+ * @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;
 }
diff --git a/src/Symfony/Component/Messenger/Handler/Locator/ContainerHandlerLocator.php b/src/Symfony/Component/Messenger/Handler/Locator/ContainerHandlerLocator.php
@@ -29,8 +29,8 @@ public function __construct(ContainerInterface $container)
     /**
      * {@inheritdoc}
      */
-    protected function getHandler(string $class): ?callable
+    protected function getHandlerByName(string $name): ?callable
     {
-        return $this->container->has($class) ? $this->container->get($class) : null;
+        return $this->container->has($name) ? $this->container->get($name) : null;
     }
 }
diff --git a/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocator.php b/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocator.php
@@ -29,8 +29,8 @@ public function __construct(array $messageToHandlerMapping = array())
     /**
      * {@inheritdoc}
      */
-    protected function getHandler(string $class): ?callable
+    protected function getHandlerByName(string $name): ?callable
     {
-        return $this->messageToHandlerMapping[$class] ?? null;
+        return $this->messageToHandlerMapping[$name] ?? null;
     }
 }
diff --git a/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocatorInterface.php b/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocatorInterface.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Messenger\Handler\Locator;
 
+use Symfony\Component\Messenger\Envelope;
 use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;
 
 /**
@@ -21,11 +22,7 @@ interface HandlerLocatorInterface
     /**
      * Returns the handler for the given message.
      *
-     * @param object $message
-     *
-     * @throws NoHandlerForMessageException
-     *
-     * @return callable
+     * @throws NoHandlerForMessageException When no handler is found
      */
-    public function resolve($message): callable;
+    public function getHandler(Envelope $envelope): callable;
 }
diff --git a/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php b/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php
@@ -19,22 +19,20 @@
  */
 class HandleMessageMiddleware implements MiddlewareInterface
 {
-    private $messageHandlerResolver;
+    private $messageHandlerLocator;
 
-    public function __construct(HandlerLocatorInterface $messageHandlerResolver)
+    public function __construct(HandlerLocatorInterface $messageHandlerLocator)
     {
-        $this->messageHandlerResolver = $messageHandlerResolver;
+        $this->messageHandlerLocator = $messageHandlerLocator;
     }
 
     /**
      * {@inheritdoc}
      */
     public function handle(Envelope $envelope, callable $next): void
     {
-        $message = $envelope->getMessage();
-        $handler = $this->messageHandlerResolver->resolve($message);
-        $handler($message);
-
+        $handler = $this->messageHandlerLocator->getHandler($envelope);
+        $handler($envelope->getMessage());
         $next($envelope);
     }
 }
diff --git a/src/Symfony/Component/Messenger/Middleware/LoggingMiddleware.php b/src/Symfony/Component/Messenger/Middleware/LoggingMiddleware.php
@@ -32,27 +32,21 @@ public function __construct(LoggerInterface $logger)
     public function handle(Envelope $envelope, callable $next): void
     {
         $message = $envelope->getMessage();
-        $this->logger->debug('Starting handling message {class}', $this->createContext($message));
+        $context = array(
+            'message' => $message,
+            'name' => \get_class($message),
+        );
+        $this->logger->debug('Starting handling message {name}', $context);
 
         try {
             $next($envelope);
         } catch (\Throwable $e) {
-            $this->logger->warning('An exception occurred while handling message {class}', array_merge(
-                $this->createContext($message),
-                array('exception' => $e)
-            ));
+            $context['exception'] = $e;
+            $this->logger->warning('An exception occurred while handling message {name}', $context);
 
             throw $e;
         }
 
-        $this->logger->debug('Finished handling message {class}', $this->createContext($message));
-    }
-
-    private function createContext($message): array
-    {
-        return array(
-            'message' => $message,
-            'class' => \get_class($message),
-        );
+        $this->logger->debug('Finished handling message {name}', $context);
     }
 }
diff --git a/src/Symfony/Component/Messenger/Tests/Asynchronous/Middleware/SendMessageMiddlewareTest.php b/src/Symfony/Component/Messenger/Tests/Asynchronous/Middleware/SendMessageMiddlewareTest.php
@@ -158,7 +158,7 @@ public function __construct(?SenderInterface $sender)
         $this->sender = $sender;
     }
 
-    public function getSenderForMessage($message): ?SenderInterface
+    public function getSender(Envelope $envelope): ?SenderInterface
     {
         return $this->sender;
     }