Skip to content

[Messenger] internal cleanups #28908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the private function increases the readability, I'd keep it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honeslty, I've had a hard time understanding what this was about. The private method didn't help me. I think the comment would make a better job. I'd keep the change :)

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Messenger\Asynchronous\Routing;

use Symfony\Component\Messenger\Envelope;
use Symfony\Component\Messenger\Transport\SenderInterface;

/**
Expand All @@ -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;
}
6 changes: 6 additions & 0 deletions src/Symfony/Component/Messenger/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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]; });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌


// 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);
}

/**
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/Messenger/Envelope.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Messenger/Handler/ChainHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Messenger\Handler\Locator;

use Symfony\Component\Messenger\Envelope;
use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;

/**
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
22 changes: 8 additions & 14 deletions src/Symfony/Component/Messenger/Middleware/LoggingMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Loading