minor #28697 [Messenger] drop "handler." prefix from ContainerHandlerLocator (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Messenger] drop "handler." prefix from ContainerHandlerLocator

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

I fail to understand what this prefix is for. Looks like boilerplate to me, let's drop it, isn't it?
An alternative to #28621

Commits
-------

6c56e82080 [Messenger] drop "handler." prefix from ContainerHandlerLocator
This commit is contained in:
Samuel ROZE 2018-10-03 09:02:23 +01:00
commit 97aab085c3
7 changed files with 46 additions and 34 deletions

View File

@ -4,7 +4,8 @@ CHANGELOG
4.2.0 4.2.0
----- -----
* [BC BREAK] The signature of Amqp* classes changed to take a `Connection` as a first argument and an optional * The component is not experimental anymore
* [BC BREAK] The signature of `Amqp*` classes changed to take a `Connection` as a first argument and an optional
`Serializer` as a second argument. `Serializer` as a second argument.
* [BC BREAK] `SenderLocator` has been renamed to `ContainerSenderLocator` * [BC BREAK] `SenderLocator` has been renamed to `ContainerSenderLocator`
Be careful as there is still a `SenderLocator` class, but it does not rely on a `ContainerInterface` to find senders. Be careful as there is still a `SenderLocator` class, but it does not rely on a `ContainerInterface` to find senders.
@ -16,3 +17,10 @@ CHANGELOG
* [BC BREAK] The `ConsumeMessagesCommand` class now takes an instance of `Psr\Container\ContainerInterface` * [BC BREAK] The `ConsumeMessagesCommand` class now takes an instance of `Psr\Container\ContainerInterface`
as first constructor argument as first constructor argument
* [BC BREAK] The `EncoderInterface` and `DecoderInterface` have been replaced by a unified `Symfony\Component\Messenger\Transport\Serialization\SerializerInterface`. * [BC BREAK] The `EncoderInterface` and `DecoderInterface` have been replaced by a unified `Symfony\Component\Messenger\Transport\Serialization\SerializerInterface`.
* [BC BREAK] The locator passed to `ContainerHandlerLocator` should not prefix its keys by "handler." anymore
* [BC BREAK] The `AbstractHandlerLocator::getHandler()` method uses `?callable` as return type
4.1.0
-----
* Introduced the component as experimental

View File

@ -167,7 +167,7 @@ class MessengerPass implements CompilerPassInterface
foreach ($handlersByBusAndMessage as $bus => $handlersByMessage) { foreach ($handlersByBusAndMessage as $bus => $handlersByMessage) {
foreach ($handlersByMessage as $message => $handlersIds) { foreach ($handlersByMessage as $message => $handlersIds) {
if (1 === \count($handlersIds)) { if (1 === \count($handlersIds)) {
$handlersLocatorMappingByBus[$bus]['handler.'.$message] = new Reference(current($handlersIds)); $handlersLocatorMappingByBus[$bus][$message] = new Reference(current($handlersIds));
} else { } else {
$chainHandler = new Definition(ChainHandler::class, array(array_map(function (string $handlerId): Reference { $chainHandler = new Definition(ChainHandler::class, array(array_map(function (string $handlerId): Reference {
return new Reference($handlerId); return new Reference($handlerId);
@ -175,7 +175,7 @@ class MessengerPass implements CompilerPassInterface
$chainHandler->setPrivate(true); $chainHandler->setPrivate(true);
$serviceId = '.messenger.chain_handler.'.ContainerBuilder::hash($bus.$message); $serviceId = '.messenger.chain_handler.'.ContainerBuilder::hash($bus.$message);
$definitions[$serviceId] = $chainHandler; $definitions[$serviceId] = $chainHandler;
$handlersLocatorMappingByBus[$bus]['handler.'.$message] = new Reference($serviceId); $handlersLocatorMappingByBus[$bus][$message] = new Reference($serviceId);
} }
} }
} }

View File

@ -42,5 +42,5 @@ abstract class AbstractHandlerLocator implements HandlerLocatorInterface
throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $class)); throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $class));
} }
abstract protected function getHandler(string $class); abstract protected function getHandler(string $class): ?callable;
} }

View File

@ -26,10 +26,11 @@ class ContainerHandlerLocator extends AbstractHandlerLocator
$this->container = $container; $this->container = $container;
} }
protected function getHandler(string $class) /**
* {@inheritdoc}
*/
protected function getHandler(string $class): ?callable
{ {
$handlerKey = 'handler.'.$class; return $this->container->has($class) ? $this->container->get($class) : null;
return $this->container->has($handlerKey) ? $this->container->get($handlerKey) : null;
} }
} }

View File

@ -26,7 +26,10 @@ class HandlerLocator extends AbstractHandlerLocator
$this->messageToHandlerMapping = $messageToHandlerMapping; $this->messageToHandlerMapping = $messageToHandlerMapping;
} }
protected function getHandler(string $class) /**
* {@inheritdoc}
*/
protected function getHandler(string $class): ?callable
{ {
return $this->messageToHandlerMapping[$class] ?? null; return $this->messageToHandlerMapping[$class] ?? null;
} }

View File

@ -69,8 +69,8 @@ class MessengerPassTest extends TestCase
$this->assertSame(ServiceLocator::class, $handlerLocatorDefinition->getClass()); $this->assertSame(ServiceLocator::class, $handlerLocatorDefinition->getClass());
$this->assertEquals( $this->assertEquals(
array( array(
'handler.'.DummyMessage::class => new ServiceClosureArgument(new Reference(DummyHandler::class)), DummyMessage::class => new ServiceClosureArgument(new Reference(DummyHandler::class)),
'handler.'.SecondMessage::class => new ServiceClosureArgument(new Reference(MissingArgumentTypeHandler::class)), SecondMessage::class => new ServiceClosureArgument(new Reference(MissingArgumentTypeHandler::class)),
), ),
$handlerLocatorDefinition->getArgument(0) $handlerLocatorDefinition->getArgument(0)
); );
@ -109,8 +109,8 @@ class MessengerPassTest extends TestCase
$this->assertSame(ServiceLocator::class, $commandBusHandlerLocatorDefinition->getClass()); $this->assertSame(ServiceLocator::class, $commandBusHandlerLocatorDefinition->getClass());
$this->assertEquals( $this->assertEquals(
array( array(
'handler.'.DummyCommand::class => new ServiceClosureArgument(new Reference(DummyCommandHandler::class)), DummyCommand::class => new ServiceClosureArgument(new Reference(DummyCommandHandler::class)),
'handler.'.MultipleBusesMessage::class => new ServiceClosureArgument(new Reference(MultipleBusesMessageHandler::class)), MultipleBusesMessage::class => new ServiceClosureArgument(new Reference(MultipleBusesMessageHandler::class)),
), ),
$commandBusHandlerLocatorDefinition->getArgument(0) $commandBusHandlerLocatorDefinition->getArgument(0)
); );
@ -119,8 +119,8 @@ class MessengerPassTest extends TestCase
$this->assertSame(ServiceLocator::class, $queryBusHandlerLocatorDefinition->getClass()); $this->assertSame(ServiceLocator::class, $queryBusHandlerLocatorDefinition->getClass());
$this->assertEquals( $this->assertEquals(
array( array(
'handler.'.DummyQuery::class => new ServiceClosureArgument(new Reference(DummyQueryHandler::class)), DummyQuery::class => new ServiceClosureArgument(new Reference(DummyQueryHandler::class)),
'handler.'.MultipleBusesMessage::class => new ServiceClosureArgument(new Reference(MultipleBusesMessageHandler::class)), MultipleBusesMessage::class => new ServiceClosureArgument(new Reference(MultipleBusesMessageHandler::class)),
), ),
$queryBusHandlerLocatorDefinition->getArgument(0) $queryBusHandlerLocatorDefinition->getArgument(0)
); );
@ -157,11 +157,11 @@ class MessengerPassTest extends TestCase
$handlerLocatorDefinition = $container->getDefinition($container->getDefinition("$busId.messenger.handler_resolver")->getArgument(0)); $handlerLocatorDefinition = $container->getDefinition($container->getDefinition("$busId.messenger.handler_resolver")->getArgument(0));
$handlerMapping = $handlerLocatorDefinition->getArgument(0); $handlerMapping = $handlerLocatorDefinition->getArgument(0);
$this->assertArrayHasKey('handler.'.DummyMessage::class, $handlerMapping); $this->assertArrayHasKey(DummyMessage::class, $handlerMapping);
$this->assertEquals(new ServiceClosureArgument(new Reference(HandlerWithMultipleMessages::class)), $handlerMapping['handler.'.DummyMessage::class]); $this->assertEquals(new ServiceClosureArgument(new Reference(HandlerWithMultipleMessages::class)), $handlerMapping[DummyMessage::class]);
$this->assertArrayHasKey('handler.'.SecondMessage::class, $handlerMapping); $this->assertArrayHasKey(SecondMessage::class, $handlerMapping);
$handlerReference = (string) $handlerMapping['handler.'.SecondMessage::class]->getValues()[0]; $handlerReference = (string) $handlerMapping[SecondMessage::class]->getValues()[0];
$definition = $container->getDefinition($handlerReference); $definition = $container->getDefinition($handlerReference);
$this->assertSame(ChainHandler::class, $definition->getClass()); $this->assertSame(ChainHandler::class, $definition->getClass());
@ -185,16 +185,16 @@ class MessengerPassTest extends TestCase
$handlerLocatorDefinition = $container->getDefinition($container->getDefinition("$busId.messenger.handler_resolver")->getArgument(0)); $handlerLocatorDefinition = $container->getDefinition($container->getDefinition("$busId.messenger.handler_resolver")->getArgument(0));
$handlerMapping = $handlerLocatorDefinition->getArgument(0); $handlerMapping = $handlerLocatorDefinition->getArgument(0);
$this->assertArrayHasKey('handler.'.DummyMessage::class, $handlerMapping); $this->assertArrayHasKey(DummyMessage::class, $handlerMapping);
$this->assertArrayHasKey('handler.'.SecondMessage::class, $handlerMapping); $this->assertArrayHasKey(SecondMessage::class, $handlerMapping);
$dummyHandlerReference = (string) $handlerMapping['handler.'.DummyMessage::class]->getValues()[0]; $dummyHandlerReference = (string) $handlerMapping[DummyMessage::class]->getValues()[0];
$dummyHandlerDefinition = $container->getDefinition($dummyHandlerReference); $dummyHandlerDefinition = $container->getDefinition($dummyHandlerReference);
$this->assertSame('callable', $dummyHandlerDefinition->getClass()); $this->assertSame('callable', $dummyHandlerDefinition->getClass());
$this->assertEquals(array(new Reference(HandlerMappingMethods::class), 'dummyMethod'), $dummyHandlerDefinition->getArgument(0)); $this->assertEquals(array(new Reference(HandlerMappingMethods::class), 'dummyMethod'), $dummyHandlerDefinition->getArgument(0));
$this->assertSame(array('Closure', 'fromCallable'), $dummyHandlerDefinition->getFactory()); $this->assertSame(array('Closure', 'fromCallable'), $dummyHandlerDefinition->getFactory());
$secondHandlerReference = (string) $handlerMapping['handler.'.SecondMessage::class]->getValues()[0]; $secondHandlerReference = (string) $handlerMapping[SecondMessage::class]->getValues()[0];
$secondHandlerDefinition = $container->getDefinition($secondHandlerReference); $secondHandlerDefinition = $container->getDefinition($secondHandlerReference);
$this->assertSame(ChainHandler::class, $secondHandlerDefinition->getClass()); $this->assertSame(ChainHandler::class, $secondHandlerDefinition->getClass());
$this->assertEquals(new Reference(PrioritizedHandler::class), $secondHandlerDefinition->getArgument(0)[1]); $this->assertEquals(new Reference(PrioritizedHandler::class), $secondHandlerDefinition->getArgument(0)[1]);
@ -291,12 +291,12 @@ class MessengerPassTest extends TestCase
$handlerLocatorDefinition = $container->getDefinition($container->getDefinition("$busId.messenger.handler_resolver")->getArgument(0)); $handlerLocatorDefinition = $container->getDefinition($container->getDefinition("$busId.messenger.handler_resolver")->getArgument(0));
$handlerMapping = $handlerLocatorDefinition->getArgument(0); $handlerMapping = $handlerLocatorDefinition->getArgument(0);
$this->assertArrayHasKey('handler.'.DummyMessage::class, $handlerMapping); $this->assertArrayHasKey(DummyMessage::class, $handlerMapping);
$firstReference = $handlerMapping['handler.'.DummyMessage::class]->getValues()[0]; $firstReference = $handlerMapping[DummyMessage::class]->getValues()[0];
$this->assertEquals(array(new Reference(HandlerWithGenerators::class), 'dummyMethod'), $container->getDefinition($firstReference)->getArgument(0)); $this->assertEquals(array(new Reference(HandlerWithGenerators::class), 'dummyMethod'), $container->getDefinition($firstReference)->getArgument(0));
$this->assertArrayHasKey('handler.'.SecondMessage::class, $handlerMapping); $this->assertArrayHasKey(SecondMessage::class, $handlerMapping);
$secondReference = $handlerMapping['handler.'.SecondMessage::class]->getValues()[0]; $secondReference = $handlerMapping[SecondMessage::class]->getValues()[0];
$this->assertEquals(array(new Reference(HandlerWithGenerators::class), 'secondMessage'), $container->getDefinition($secondReference)->getArgument(0)); $this->assertEquals(array(new Reference(HandlerWithGenerators::class), 'secondMessage'), $container->getDefinition($secondReference)->getArgument(0));
} }
@ -315,15 +315,15 @@ class MessengerPassTest extends TestCase
$eventsHandlerLocatorDefinition = $container->getDefinition($container->getDefinition($eventsBusId.'.messenger.handler_resolver')->getArgument(0)); $eventsHandlerLocatorDefinition = $container->getDefinition($container->getDefinition($eventsBusId.'.messenger.handler_resolver')->getArgument(0));
$eventsHandlerMapping = $eventsHandlerLocatorDefinition->getArgument(0); $eventsHandlerMapping = $eventsHandlerLocatorDefinition->getArgument(0);
$this->assertEquals(array('handler.'.DummyMessage::class), array_keys($eventsHandlerMapping)); $this->assertEquals(array(DummyMessage::class), array_keys($eventsHandlerMapping));
$firstReference = $eventsHandlerMapping['handler.'.DummyMessage::class]->getValues()[0]; $firstReference = $eventsHandlerMapping[DummyMessage::class]->getValues()[0];
$this->assertEquals(array(new Reference(HandlerOnSpecificBuses::class), 'dummyMethodForEvents'), $container->getDefinition($firstReference)->getArgument(0)); $this->assertEquals(array(new Reference(HandlerOnSpecificBuses::class), 'dummyMethodForEvents'), $container->getDefinition($firstReference)->getArgument(0));
$commandsHandlerLocatorDefinition = $container->getDefinition($container->getDefinition($commandsBusId.'.messenger.handler_resolver')->getArgument(0)); $commandsHandlerLocatorDefinition = $container->getDefinition($container->getDefinition($commandsBusId.'.messenger.handler_resolver')->getArgument(0));
$commandsHandlerMapping = $commandsHandlerLocatorDefinition->getArgument(0); $commandsHandlerMapping = $commandsHandlerLocatorDefinition->getArgument(0);
$this->assertEquals(array('handler.'.DummyMessage::class), array_keys($commandsHandlerMapping)); $this->assertEquals(array(DummyMessage::class), array_keys($commandsHandlerMapping));
$firstReference = $commandsHandlerMapping['handler.'.DummyMessage::class]->getValues()[0]; $firstReference = $commandsHandlerMapping[DummyMessage::class]->getValues()[0];
$this->assertEquals(array(new Reference(HandlerOnSpecificBuses::class), 'dummyMethodForCommands'), $container->getDefinition($firstReference)->getArgument(0)); $this->assertEquals(array(new Reference(HandlerOnSpecificBuses::class), 'dummyMethodForCommands'), $container->getDefinition($firstReference)->getArgument(0));
} }

View File

@ -16,7 +16,7 @@ class ContainerHandlerLocatorTest extends TestCase
$handler = function () {}; $handler = function () {};
$container = new Container(); $container = new Container();
$container->set('handler.'.DummyMessage::class, $handler); $container->set(DummyMessage::class, $handler);
$locator = new ContainerHandlerLocator($container); $locator = new ContainerHandlerLocator($container);
$resolvedHandler = $locator->resolve(new DummyMessage('Hey')); $resolvedHandler = $locator->resolve(new DummyMessage('Hey'));
@ -39,7 +39,7 @@ class ContainerHandlerLocatorTest extends TestCase
$handler = function () {}; $handler = function () {};
$container = new Container(); $container = new Container();
$container->set('handler.'.DummyMessageInterface::class, $handler); $container->set(DummyMessageInterface::class, $handler);
$locator = new ContainerHandlerLocator($container); $locator = new ContainerHandlerLocator($container);
$resolvedHandler = $locator->resolve(new DummyMessage('Hey')); $resolvedHandler = $locator->resolve(new DummyMessage('Hey'));
@ -52,7 +52,7 @@ class ContainerHandlerLocatorTest extends TestCase
$handler = function () {}; $handler = function () {};
$container = new Container(); $container = new Container();
$container->set('handler.'.DummyMessage::class, $handler); $container->set(DummyMessage::class, $handler);
$locator = new ContainerHandlerLocator($container); $locator = new ContainerHandlerLocator($container);
$resolvedHandler = $locator->resolve(new ChildDummyMessage('Hey')); $resolvedHandler = $locator->resolve(new ChildDummyMessage('Hey'));