From aedb281b76de8ff5a4f651dab3f188e6ca4f19ed Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 22 Oct 2018 09:10:37 +0200 Subject: [PATCH] [Messenger] remove AllowNoHandlerMiddleware in favor of a constructor argument on HandleMessageMiddleware --- .../DependencyInjection/Configuration.php | 19 ++++--- .../FrameworkExtension.php | 11 +++- .../Resources/config/messenger.xml | 1 - .../Resources/config/schema/symfony-1.0.xsd | 12 +++- .../Fixtures/php/messenger_multiple_buses.php | 2 - .../Fixtures/xml/messenger_multiple_buses.xml | 2 - .../Fixtures/yml/messenger_multiple_buses.yml | 2 - .../FrameworkExtensionTest.php | 4 +- src/Symfony/Component/Messenger/CHANGELOG.md | 3 +- .../Locator/AbstractHandlerLocator.php | 5 +- .../Locator/HandlerLocatorInterface.php | 5 +- .../Middleware/AllowNoHandlerMiddleware.php | 33 ----------- .../Middleware/HandleMessageMiddleware.php | 16 ++++-- .../DependencyInjection/MessengerPassTest.php | 6 -- .../Locator/ContainerHandlerLocatorTest.php | 8 +-- .../AllowNoHandlerMiddlewareTest.php | 56 ------------------- .../HandleMessageMiddlewareTest.php | 18 ++++++ 17 files changed, 69 insertions(+), 134 deletions(-) delete mode 100644 src/Symfony/Component/Messenger/Middleware/AllowNoHandlerMiddleware.php delete mode 100644 src/Symfony/Component/Messenger/Tests/Middleware/AllowNoHandlerMiddlewareTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index a6d99ff4b5..c594d24c64 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -1081,19 +1081,20 @@ class Configuration implements ConfigurationInterface ->arrayNode('buses') ->defaultValue(array('messenger.bus.default' => array('default_middleware' => true, 'middleware' => array()))) ->useAttributeAsKey('name') - ->prototype('array') + ->arrayPrototype() ->addDefaultsIfNotSet() ->children() - ->booleanNode('default_middleware')->defaultTrue()->end() + ->enumNode('default_middleware') + ->values(array(true, false, 'allow_no_handlers')) + ->defaultTrue() + ->end() ->arrayNode('middleware') ->beforeNormalization() - ->ifString() - ->then(function (string $middleware) { - return array($middleware); - }) + ->ifTrue(function ($v) { return \is_string($v) || !\is_int(key($v)); }) + ->then(function ($v) { return array($v); }) ->end() ->defaultValue(array()) - ->prototype('array') + ->arrayPrototype() ->beforeNormalization() ->always() ->then(function ($middleware): array { @@ -1103,8 +1104,8 @@ class Configuration implements ConfigurationInterface if (isset($middleware['id'])) { return $middleware; } - if (\count($middleware) > 1) { - throw new \InvalidArgumentException(sprintf('There is an error at path "framework.messenger" in one of the buses middleware definitions: expected a single entry for a middleware item config, with factory id as key and arguments as value. Got "%s".', json_encode($middleware))); + if (1 < \count($middleware)) { + throw new \InvalidArgumentException(sprintf('Invalid middleware at path "framework.messenger": a map with a single factory id as key and its arguments as value was expected, %s given.', json_encode($middleware))); } return array( diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index fbd0e97080..fd242bf845 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1524,7 +1524,16 @@ class FrameworkExtension extends Extension 'after' => array(array('id' => 'route_messages'), array('id' => 'call_message_handler')), ); foreach ($config['buses'] as $busId => $bus) { - $middleware = $bus['default_middleware'] ? array_merge($defaultMiddleware['before'], $bus['middleware'], $defaultMiddleware['after']) : $bus['middleware']; + $middleware = $bus['middleware']; + + if ($bus['default_middleware']) { + if ('allow_no_handlers' === $bus['default_middleware']) { + $defaultMiddleware['after'][1]['arguments'] = array(true); + } else { + unset($defaultMiddleware['after'][1]['arguments']); + } + $middleware = array_merge($defaultMiddleware['before'], $middleware, $defaultMiddleware['after']); + } foreach ($middleware as $middlewareItem) { if (!$validationConfig['enabled'] && 'messenger.middleware.validation' === $middlewareItem['id']) { diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml index 810af57761..8fc23e18c4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml @@ -25,7 +25,6 @@ - diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index 71d376abb2..c99e543fa5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -331,6 +331,16 @@ + + + + + + + + + + @@ -408,7 +418,7 @@ - + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_multiple_buses.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_multiple_buses.php index 5f3b2b2302..1e0b402760 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_multiple_buses.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_multiple_buses.php @@ -8,14 +8,12 @@ $container->loadFromExtension('framework', array( 'messenger.bus.events' => array( 'middleware' => array( array('with_factory' => array('foo', true, array('bar' => 'baz'))), - 'allow_no_handler', ), ), 'messenger.bus.queries' => array( 'default_middleware' => false, 'middleware' => array( 'route_messages', - 'allow_no_handler', 'call_message_handler', ), ), diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_multiple_buses.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_multiple_buses.xml index f63df5bbbe..e42e4eac30 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_multiple_buses.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_multiple_buses.xml @@ -16,11 +16,9 @@ baz - - diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_multiple_buses.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_multiple_buses.yml index e279ef3bba..23894ac01b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_multiple_buses.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_multiple_buses.yml @@ -6,10 +6,8 @@ framework: messenger.bus.events: middleware: - with_factory: [foo, true, { bar: baz }] - - "allow_no_handler" messenger.bus.queries: default_middleware: false middleware: - "route_messages" - - "allow_no_handler" - "call_message_handler" diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 260f381990..bbe53dbf69 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -627,7 +627,6 @@ abstract class FrameworkExtensionTest extends TestCase $this->assertEquals(array( array('id' => 'logging'), array('id' => 'with_factory', 'arguments' => array('foo', true, array('bar' => 'baz'))), - array('id' => 'allow_no_handler', 'arguments' => array()), array('id' => 'route_messages'), array('id' => 'call_message_handler'), ), $container->getParameter('messenger.bus.events.middleware')); @@ -635,7 +634,6 @@ abstract class FrameworkExtensionTest extends TestCase $this->assertSame(array(), $container->getDefinition('messenger.bus.queries')->getArgument(0)); $this->assertEquals(array( array('id' => 'route_messages', 'arguments' => array()), - array('id' => 'allow_no_handler', 'arguments' => array()), array('id' => 'call_message_handler', 'arguments' => array()), ), $container->getParameter('messenger.bus.queries.middleware')); @@ -645,7 +643,7 @@ abstract class FrameworkExtensionTest extends TestCase /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage There is an error at path "framework.messenger" in one of the buses middleware definitions: expected a single entry for a middleware item config, with factory id as key and arguments as value. Got "{"foo":["qux"],"bar":["baz"]}" + * @expectedExceptionMessage Invalid middleware at path "framework.messenger": a map with a single factory id as key and its arguments as value was expected, {"foo":["qux"],"bar":["baz"]} given. */ public function testMessengerMiddlewareFactoryErroneousFormat() { diff --git a/src/Symfony/Component/Messenger/CHANGELOG.md b/src/Symfony/Component/Messenger/CHANGELOG.md index 33171ecedf..84acedf33b 100644 --- a/src/Symfony/Component/Messenger/CHANGELOG.md +++ b/src/Symfony/Component/Messenger/CHANGELOG.md @@ -30,7 +30,7 @@ CHANGELOG * `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)` + * `HandlerLocatorInterface::resolve()` has been replaced by `getHandler(Envelope $envelope): ?callable` and shouldn't throw when no handlers are found * `SenderLocatorInterface::getSenderForMessage()` has been replaced by `getSender(Envelope $envelope)` * `SenderInterface::send()` returns `void` * Classes in the `Middleware\Enhancers` sub-namespace have been moved to the `Middleware` one @@ -39,6 +39,7 @@ CHANGELOG * `SenderInterface` and `ChainSender` classes have been moved to the `Transport\Sender` sub-namespace * `ReceiverInterface` and its implementations have been moved to the `Transport\Receiver` sub-namespace * `ActivationMiddlewareDecorator` has been renamed `ActivationMiddleware` + * `AllowNoHandlerMiddleware` has been removed in favor of a new constructor argument on `HandleMessageMiddleware` 4.1.0 ----- diff --git a/src/Symfony/Component/Messenger/Handler/Locator/AbstractHandlerLocator.php b/src/Symfony/Component/Messenger/Handler/Locator/AbstractHandlerLocator.php index 5a16f6fe6b..95fb50c716 100644 --- a/src/Symfony/Component/Messenger/Handler/Locator/AbstractHandlerLocator.php +++ b/src/Symfony/Component/Messenger/Handler/Locator/AbstractHandlerLocator.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Messenger\Handler\Locator; use Symfony\Component\Messenger\Envelope; -use Symfony\Component\Messenger\Exception\NoHandlerForMessageException; /** * @author Miha Vrhovnik @@ -22,7 +21,7 @@ use Symfony\Component\Messenger\Exception\NoHandlerForMessageException; */ abstract class AbstractHandlerLocator implements HandlerLocatorInterface { - public function getHandler(Envelope $envelope): callable + public function getHandler(Envelope $envelope): ?callable { $class = \get_class($envelope->getMessage()); @@ -42,7 +41,7 @@ abstract class AbstractHandlerLocator implements HandlerLocatorInterface } } - throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $class)); + return null; } abstract protected function getHandlerByName(string $name): ?callable; diff --git a/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocatorInterface.php b/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocatorInterface.php index 9a040b07a4..43735dc3d2 100644 --- a/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocatorInterface.php +++ b/src/Symfony/Component/Messenger/Handler/Locator/HandlerLocatorInterface.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Messenger\Handler\Locator; use Symfony\Component\Messenger\Envelope; -use Symfony\Component\Messenger\Exception\NoHandlerForMessageException; /** * @author Samuel Roze @@ -21,8 +20,6 @@ interface HandlerLocatorInterface { /** * Returns the handler for the given message. - * - * @throws NoHandlerForMessageException When no handler is found */ - public function getHandler(Envelope $envelope): callable; + public function getHandler(Envelope $envelope): ?callable; } diff --git a/src/Symfony/Component/Messenger/Middleware/AllowNoHandlerMiddleware.php b/src/Symfony/Component/Messenger/Middleware/AllowNoHandlerMiddleware.php deleted file mode 100644 index 4f46ff599d..0000000000 --- a/src/Symfony/Component/Messenger/Middleware/AllowNoHandlerMiddleware.php +++ /dev/null @@ -1,33 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Messenger\Middleware; - -use Symfony\Component\Messenger\Envelope; -use Symfony\Component\Messenger\Exception\NoHandlerForMessageException; - -/** - * @author Samuel Roze - */ -class AllowNoHandlerMiddleware implements MiddlewareInterface -{ - /** - * {@inheritdoc} - */ - public function handle(Envelope $envelope, callable $next): void - { - try { - $next($envelope); - } catch (NoHandlerForMessageException $e) { - // We allow not having a handler for this message. - } - } -} diff --git a/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php b/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php index 0c6a759b63..12b7498708 100644 --- a/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php +++ b/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Messenger\Middleware; use Symfony\Component\Messenger\Envelope; +use Symfony\Component\Messenger\Exception\NoHandlerForMessageException; use Symfony\Component\Messenger\Handler\Locator\HandlerLocatorInterface; /** @@ -20,19 +21,26 @@ use Symfony\Component\Messenger\Handler\Locator\HandlerLocatorInterface; class HandleMessageMiddleware implements MiddlewareInterface { private $messageHandlerLocator; + private $allowNoHandlers; - public function __construct(HandlerLocatorInterface $messageHandlerLocator) + public function __construct(HandlerLocatorInterface $messageHandlerLocator, bool $allowNoHandlers = false) { $this->messageHandlerLocator = $messageHandlerLocator; + $this->allowNoHandlers = $allowNoHandlers; } /** * {@inheritdoc} + * + * @throws NoHandlerForMessageException When no handler is found and $allowNoHandlers is false */ public function handle(Envelope $envelope, callable $next): void { - $handler = $this->messageHandlerLocator->getHandler($envelope); - $handler($envelope->getMessage()); - $next($envelope); + if (null !== $handler = $this->messageHandlerLocator->getHandler($envelope)) { + $handler($envelope->getMessage()); + $next($envelope); + } elseif (!$this->allowNoHandlers) { + throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', \get_class($envelope->getMessage()))); + } } } diff --git a/src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php b/src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php index 777d13ceca..1ca439cbcc 100644 --- a/src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php +++ b/src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php @@ -27,7 +27,6 @@ use Symfony\Component\Messenger\Handler\ChainHandler; use Symfony\Component\Messenger\Handler\MessageHandlerInterface; use Symfony\Component\Messenger\Handler\MessageSubscriberInterface; use Symfony\Component\Messenger\MessageBusInterface; -use Symfony\Component\Messenger\Middleware\AllowNoHandlerMiddleware; use Symfony\Component\Messenger\Middleware\HandleMessageMiddleware; use Symfony\Component\Messenger\Middleware\MiddlewareInterface; use Symfony\Component\Messenger\Tests\Fixtures\DummyCommand; @@ -493,7 +492,6 @@ class MessengerPassTest extends TestCase public function testRegistersMiddlewareFromServices() { $container = $this->getContainerBuilder($fooBusId = 'messenger.bus.foo'); - $container->register('messenger.middleware.allow_no_handler', AllowNoHandlerMiddleware::class)->setAbstract(true); $container->register('middleware_with_factory', UselessMiddleware::class)->addArgument('some_default')->setAbstract(true); $container->register('middleware_with_factory_using_default', UselessMiddleware::class)->addArgument('some_default')->setAbstract(true); $container->register(UselessMiddleware::class, UselessMiddleware::class); @@ -502,14 +500,11 @@ class MessengerPassTest extends TestCase array('id' => UselessMiddleware::class), array('id' => 'middleware_with_factory', 'arguments' => array('foo', 'bar')), array('id' => 'middleware_with_factory_using_default'), - array('id' => 'allow_no_handler'), )); (new MessengerPass())->process($container); (new ResolveChildDefinitionsPass())->process($container); - $this->assertTrue($container->hasDefinition($childMiddlewareId = $fooBusId.'.middleware.allow_no_handler')); - $this->assertTrue($container->hasDefinition($factoryChildMiddlewareId = $fooBusId.'.middleware.middleware_with_factory')); $this->assertEquals( array('foo', 'bar'), @@ -528,7 +523,6 @@ class MessengerPassTest extends TestCase new Reference(UselessMiddleware::class), new Reference($factoryChildMiddlewareId), new Reference($factoryWithDefaultChildMiddlewareId), - new Reference($childMiddlewareId), ), $container->getDefinition($fooBusId)->getArgument(0)); $this->assertFalse($container->hasParameter($middlewareParameter)); } diff --git a/src/Symfony/Component/Messenger/Tests/Handler/Locator/ContainerHandlerLocatorTest.php b/src/Symfony/Component/Messenger/Tests/Handler/Locator/ContainerHandlerLocatorTest.php index 98680c329b..e9147e50b7 100644 --- a/src/Symfony/Component/Messenger/Tests/Handler/Locator/ContainerHandlerLocatorTest.php +++ b/src/Symfony/Component/Messenger/Tests/Handler/Locator/ContainerHandlerLocatorTest.php @@ -25,14 +25,10 @@ class ContainerHandlerLocatorTest extends TestCase $this->assertSame($handler, $resolvedHandler); } - /** - * @expectedException \Symfony\Component\Messenger\Exception\NoHandlerForMessageException - * @expectedExceptionMessage No handler for message "Symfony\Component\Messenger\Tests\Fixtures\DummyMessage" - */ - public function testThrowsNoHandlerException() + public function testNoHandlersReturnsNull() { $locator = new ContainerHandlerLocator(new Container()); - $locator->getHandler(new Envelope(new DummyMessage('Hey'))); + $this->assertNull($locator->getHandler(new Envelope(new DummyMessage('Hey')))); } public function testGetHandlerViaInterface() diff --git a/src/Symfony/Component/Messenger/Tests/Middleware/AllowNoHandlerMiddlewareTest.php b/src/Symfony/Component/Messenger/Tests/Middleware/AllowNoHandlerMiddlewareTest.php deleted file mode 100644 index c298ad9adc..0000000000 --- a/src/Symfony/Component/Messenger/Tests/Middleware/AllowNoHandlerMiddlewareTest.php +++ /dev/null @@ -1,56 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Messenger\Tests\Middleware; - -use PHPUnit\Framework\TestCase; -use Symfony\Component\Messenger\Envelope; -use Symfony\Component\Messenger\Exception\NoHandlerForMessageException; -use Symfony\Component\Messenger\Middleware\AllowNoHandlerMiddleware; -use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage; - -class AllowNoHandlerMiddlewareTest extends TestCase -{ - public function testItCallsNextMiddleware() - { - $message = new DummyMessage('Hey'); - $envelope = new Envelope($message); - - $next = $this->createPartialMock(\stdClass::class, array('__invoke')); - $next->expects($this->once())->method('__invoke')->with($envelope); - - $middleware = new AllowNoHandlerMiddleware(); - $middleware->handle($envelope, $next); - } - - public function testItCatchesTheNoHandlerException() - { - $next = $this->createPartialMock(\stdClass::class, array('__invoke')); - $next->expects($this->once())->method('__invoke')->will($this->throwException(new NoHandlerForMessageException())); - - $middleware = new AllowNoHandlerMiddleware(); - - $this->assertNull($middleware->handle(new Envelope(new DummyMessage('Hey')), $next)); - } - - /** - * @expectedException \RuntimeException - * @expectedExceptionMessage Something went wrong. - */ - public function testItDoesNotCatchOtherExceptions() - { - $next = $this->createPartialMock(\stdClass::class, array('__invoke')); - $next->expects($this->once())->method('__invoke')->will($this->throwException(new \RuntimeException('Something went wrong.'))); - - $middleware = new AllowNoHandlerMiddleware(); - $middleware->handle(new Envelope(new DummyMessage('Hey')), $next); - } -} diff --git a/src/Symfony/Component/Messenger/Tests/Middleware/HandleMessageMiddlewareTest.php b/src/Symfony/Component/Messenger/Tests/Middleware/HandleMessageMiddlewareTest.php index 6791dd6477..59d827b041 100644 --- a/src/Symfony/Component/Messenger/Tests/Middleware/HandleMessageMiddlewareTest.php +++ b/src/Symfony/Component/Messenger/Tests/Middleware/HandleMessageMiddlewareTest.php @@ -37,4 +37,22 @@ class HandleMessageMiddlewareTest extends TestCase $middleware->handle($envelope, $next); } + + /** + * @expectedException \Symfony\Component\Messenger\Exception\NoHandlerForMessageException + * @expectedExceptionMessage No handler for message "Symfony\Component\Messenger\Tests\Fixtures\DummyMessage" + */ + public function testThrowsNoHandlerException() + { + $middleware = new HandleMessageMiddleware(new HandlerLocator(array())); + + $middleware->handle(new Envelope(new DummyMessage('Hey')), function () {}); + } + + public function testAllowNoHandlers() + { + $middleware = new HandleMessageMiddleware(new HandlerLocator(array()), true); + + $this->assertNull($middleware->handle(new Envelope(new DummyMessage('Hey')), function () {})); + } }