Skip to content

Commit 5ae0e89

Browse files
srozesroze
committed
feature #28945 [Messenger] remove AllowNoHandlerMiddleware in favor of a constructor argument on HandleMessageMiddleware (nicolas-grekas)
This PR was merged into the 4.2-dev branch. Discussion ---------- [Messenger] remove AllowNoHandlerMiddleware in favor of a constructor argument on HandleMessageMiddleware | 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 | - `AllowNoHandlerMiddleware` adds a frame in the call stack but its concern should be handled by `HandleMessageMiddleware` to me, and `HandlerLocatorInterface::getHandler()` should not care about whether it's an error or not to not have a middleware (this makes it on par with `SenderLocator` in this regard.) Commits ------- aedb281 [Messenger] remove AllowNoHandlerMiddleware in favor of a constructor argument on HandleMessageMiddleware
2 parents 2b7ac81 + aedb281 commit 5ae0e89

17 files changed

+69
-134
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

+10-9
Original file line numberDiff line numberDiff line change
@@ -1081,19 +1081,20 @@ function ($a) {
10811081
->arrayNode('buses')
10821082
->defaultValue(array('messenger.bus.default' => array('default_middleware' => true, 'middleware' => array())))
10831083
->useAttributeAsKey('name')
1084-
->prototype('array')
1084+
->arrayPrototype()
10851085
->addDefaultsIfNotSet()
10861086
->children()
1087-
->booleanNode('default_middleware')->defaultTrue()->end()
1087+
->enumNode('default_middleware')
1088+
->values(array(true, false, 'allow_no_handlers'))
1089+
->defaultTrue()
1090+
->end()
10881091
->arrayNode('middleware')
10891092
->beforeNormalization()
1090-
->ifString()
1091-
->then(function (string $middleware) {
1092-
return array($middleware);
1093-
})
1093+
->ifTrue(function ($v) { return \is_string($v) || !\is_int(key($v)); })
1094+
->then(function ($v) { return array($v); })
10941095
->end()
10951096
->defaultValue(array())
1096-
->prototype('array')
1097+
->arrayPrototype()
10971098
->beforeNormalization()
10981099
->always()
10991100
->then(function ($middleware): array {
@@ -1103,8 +1104,8 @@ function ($a) {
11031104
if (isset($middleware['id'])) {
11041105
return $middleware;
11051106
}
1106-
if (\count($middleware) > 1) {
1107-
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)));
1107+
if (1 < \count($middleware)) {
1108+
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)));
11081109
}
11091110

11101111
return array(

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -1524,7 +1524,16 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
15241524
'after' => array(array('id' => 'route_messages'), array('id' => 'call_message_handler')),
15251525
);
15261526
foreach ($config['buses'] as $busId => $bus) {
1527-
$middleware = $bus['default_middleware'] ? array_merge($defaultMiddleware['before'], $bus['middleware'], $defaultMiddleware['after']) : $bus['middleware'];
1527+
$middleware = $bus['middleware'];
1528+
1529+
if ($bus['default_middleware']) {
1530+
if ('allow_no_handlers' === $bus['default_middleware']) {
1531+
$defaultMiddleware['after'][1]['arguments'] = array(true);
1532+
} else {
1533+
unset($defaultMiddleware['after'][1]['arguments']);
1534+
}
1535+
$middleware = array_merge($defaultMiddleware['before'], $middleware, $defaultMiddleware['after']);
1536+
}
15281537

15291538
foreach ($middleware as $middlewareItem) {
15301539
if (!$validationConfig['enabled'] && 'messenger.middleware.validation' === $middlewareItem['id']) {

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml

-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
</service>
2626

2727
<!-- Middleware -->
28-
<service id="messenger.middleware.allow_no_handler" class="Symfony\Component\Messenger\Middleware\AllowNoHandlerMiddleware" abstract="true" />
2928
<service id="messenger.middleware.call_message_handler" class="Symfony\Component\Messenger\Middleware\HandleMessageMiddleware" abstract="true">
3029
<argument /> <!-- Bus handler resolver -->
3130
</service>

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

+11-1
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,16 @@
331331
</xsd:sequence>
332332
</xsd:complexType>
333333

334+
<xsd:simpleType name="default_middleware">
335+
<xsd:restriction base="xsd:string">
336+
<xsd:enumeration value="true" />
337+
<xsd:enumeration value="false" />
338+
<xsd:enumeration value="1" />
339+
<xsd:enumeration value="0" />
340+
<xsd:enumeration value="allow_no_handlers" />
341+
</xsd:restriction>
342+
</xsd:simpleType>
343+
334344
<xsd:simpleType name="cookie_secure">
335345
<xsd:restriction base="xsd:string">
336346
<xsd:enumeration value="true" />
@@ -408,7 +418,7 @@
408418
<xsd:element name="middleware" type="messenger_middleware" minOccurs="0" maxOccurs="unbounded" />
409419
</xsd:sequence>
410420
<xsd:attribute name="name" type="xsd:string" use="required"/>
411-
<xsd:attribute name="default-middleware" type="xsd:boolean"/>
421+
<xsd:attribute name="default-middleware" type="default_middleware"/>
412422
</xsd:complexType>
413423

414424
<xsd:complexType name="messenger_middleware">

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_multiple_buses.php

-2
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@
88
'messenger.bus.events' => array(
99
'middleware' => array(
1010
array('with_factory' => array('foo', true, array('bar' => 'baz'))),
11-
'allow_no_handler',
1211
),
1312
),
1413
'messenger.bus.queries' => array(
1514
'default_middleware' => false,
1615
'middleware' => array(
1716
'route_messages',
18-
'allow_no_handler',
1917
'call_message_handler',
2018
),
2119
),

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_multiple_buses.xml

-2
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,9 @@
1616
<framework:bar>baz</framework:bar>
1717
</framework:argument>
1818
</framework:middleware>
19-
<framework:middleware id="allow_no_handler" />
2019
</framework:bus>
2120
<framework:bus name="messenger.bus.queries" default-middleware="false">
2221
<framework:middleware id="route_messages" />
23-
<framework:middleware id="allow_no_handler" />
2422
<framework:middleware id="call_message_handler" />
2523
</framework:bus>
2624
</framework:messenger>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_multiple_buses.yml

-2
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@ framework:
66
messenger.bus.events:
77
middleware:
88
- with_factory: [foo, true, { bar: baz }]
9-
- "allow_no_handler"
109
messenger.bus.queries:
1110
default_middleware: false
1211
middleware:
1312
- "route_messages"
14-
- "allow_no_handler"
1513
- "call_message_handler"

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

+1-3
Original file line numberDiff line numberDiff line change
@@ -627,15 +627,13 @@ public function testMessengerWithMultipleBuses()
627627
$this->assertEquals(array(
628628
array('id' => 'logging'),
629629
array('id' => 'with_factory', 'arguments' => array('foo', true, array('bar' => 'baz'))),
630-
array('id' => 'allow_no_handler', 'arguments' => array()),
631630
array('id' => 'route_messages'),
632631
array('id' => 'call_message_handler'),
633632
), $container->getParameter('messenger.bus.events.middleware'));
634633
$this->assertTrue($container->has('messenger.bus.queries'));
635634
$this->assertSame(array(), $container->getDefinition('messenger.bus.queries')->getArgument(0));
636635
$this->assertEquals(array(
637636
array('id' => 'route_messages', 'arguments' => array()),
638-
array('id' => 'allow_no_handler', 'arguments' => array()),
639637
array('id' => 'call_message_handler', 'arguments' => array()),
640638
), $container->getParameter('messenger.bus.queries.middleware'));
641639

@@ -645,7 +643,7 @@ public function testMessengerWithMultipleBuses()
645643

646644
/**
647645
* @expectedException \InvalidArgumentException
648-
* @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"]}"
646+
* @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.
649647
*/
650648
public function testMessengerMiddlewareFactoryErroneousFormat()
651649
{

‎src/Symfony/Component/Messenger/CHANGELOG.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ CHANGELOG
3030
* `SenderLocatorInterface::getSenderForMessage()` has been replaced by `getSender(Envelope $envelope)`
3131
* `MessengerDataCollector::getMessages()` returns an iterable, not just an array anymore
3232
* `AbstractHandlerLocator` is now internal
33-
* `HandlerLocatorInterface::resolve()` has been replaced by `getHandler(Envelope $envelope)`
33+
* `HandlerLocatorInterface::resolve()` has been replaced by `getHandler(Envelope $envelope): ?callable` and shouldn't throw when no handlers are found
3434
* `SenderLocatorInterface::getSenderForMessage()` has been replaced by `getSender(Envelope $envelope)`
3535
* `SenderInterface::send()` returns `void`
3636
* Classes in the `Middleware\Enhancers` sub-namespace have been moved to the `Middleware` one
@@ -39,6 +39,7 @@ CHANGELOG
3939
* `SenderInterface` and `ChainSender` classes have been moved to the `Transport\Sender` sub-namespace
4040
* `ReceiverInterface` and its implementations have been moved to the `Transport\Receiver` sub-namespace
4141
* `ActivationMiddlewareDecorator` has been renamed `ActivationMiddleware`
42+
* `AllowNoHandlerMiddleware` has been removed in favor of a new constructor argument on `HandleMessageMiddleware`
4243

4344
4.1.0
4445
-----

‎src/Symfony/Component/Messenger/Handler/Locator/AbstractHandlerLocator.php

+2-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
namespace Symfony\Component\Messenger\Handler\Locator;
1313

1414
use Symfony\Component\Messenger\Envelope;
15-
use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;
1615

1716
/**
1817
* @author Miha Vrhovnik <miha.vrhovnik@gmail.com>
@@ -22,7 +21,7 @@
2221
*/
2322
abstract class AbstractHandlerLocator implements HandlerLocatorInterface
2423
{
25-
public function getHandler(Envelope $envelope): callable
24+
public function getHandler(Envelope $envelope): ?callable
2625
{
2726
$class = \get_class($envelope->getMessage());
2827

@@ -42,7 +41,7 @@ public function getHandler(Envelope $envelope): callable
4241
}
4342
}
4443

45-
throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', $class));
44+
return null;
4645
}
4746

4847
abstract protected function getHandlerByName(string $name): ?callable;

‎src/Symfony/Component/Messenger/Handler/Locator/HandlerLocatorInterface.php

+1-4
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
namespace Symfony\Component\Messenger\Handler\Locator;
1313

1414
use Symfony\Component\Messenger\Envelope;
15-
use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;
1615

1716
/**
1817
* @author Samuel Roze <samuel.roze@gmail.com>
@@ -21,8 +20,6 @@ interface HandlerLocatorInterface
2120
{
2221
/**
2322
* Returns the handler for the given message.
24-
*
25-
* @throws NoHandlerForMessageException When no handler is found
2623
*/
27-
public function getHandler(Envelope $envelope): callable;
24+
public function getHandler(Envelope $envelope): ?callable;
2825
}

‎src/Symfony/Component/Messenger/Middleware/AllowNoHandlerMiddleware.php

-33
This file was deleted.

‎src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php

+12-4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Messenger\Middleware;
1313

1414
use Symfony\Component\Messenger\Envelope;
15+
use Symfony\Component\Messenger\Exception\NoHandlerForMessageException;
1516
use Symfony\Component\Messenger\Handler\Locator\HandlerLocatorInterface;
1617

1718
/**
@@ -20,19 +21,26 @@
2021
class HandleMessageMiddleware implements MiddlewareInterface
2122
{
2223
private $messageHandlerLocator;
24+
private $allowNoHandlers;
2325

24-
public function __construct(HandlerLocatorInterface $messageHandlerLocator)
26+
public function __construct(HandlerLocatorInterface $messageHandlerLocator, bool $allowNoHandlers = false)
2527
{
2628
$this->messageHandlerLocator = $messageHandlerLocator;
29+
$this->allowNoHandlers = $allowNoHandlers;
2730
}
2831

2932
/**
3033
* {@inheritdoc}
34+
*
35+
* @throws NoHandlerForMessageException When no handler is found and $allowNoHandlers is false
3136
*/
3237
public function handle(Envelope $envelope, callable $next): void
3338
{
34-
$handler = $this->messageHandlerLocator->getHandler($envelope);
35-
$handler($envelope->getMessage());
36-
$next($envelope);
39+
if (null !== $handler = $this->messageHandlerLocator->getHandler($envelope)) {
40+
$handler($envelope->getMessage());
41+
$next($envelope);
42+
} elseif (!$this->allowNoHandlers) {
43+
throw new NoHandlerForMessageException(sprintf('No handler for message "%s".', \get_class($envelope->getMessage())));
44+
}
3745
}
3846
}

‎src/Symfony/Component/Messenger/Tests/DependencyInjection/MessengerPassTest.php

-6
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
use Symfony\Component\Messenger\Handler\MessageHandlerInterface;
2828
use Symfony\Component\Messenger\Handler\MessageSubscriberInterface;
2929
use Symfony\Component\Messenger\MessageBusInterface;
30-
use Symfony\Component\Messenger\Middleware\AllowNoHandlerMiddleware;
3130
use Symfony\Component\Messenger\Middleware\HandleMessageMiddleware;
3231
use Symfony\Component\Messenger\Middleware\MiddlewareInterface;
3332
use Symfony\Component\Messenger\Tests\Fixtures\DummyCommand;
@@ -493,7 +492,6 @@ public function testRegistersTraceableBusesToCollector()
493492
public function testRegistersMiddlewareFromServices()
494493
{
495494
$container = $this->getContainerBuilder($fooBusId = 'messenger.bus.foo');
496-
$container->register('messenger.middleware.allow_no_handler', AllowNoHandlerMiddleware::class)->setAbstract(true);
497495
$container->register('middleware_with_factory', UselessMiddleware::class)->addArgument('some_default')->setAbstract(true);
498496
$container->register('middleware_with_factory_using_default', UselessMiddleware::class)->addArgument('some_default')->setAbstract(true);
499497
$container->register(UselessMiddleware::class, UselessMiddleware::class);
@@ -502,14 +500,11 @@ public function testRegistersMiddlewareFromServices()
502500
array('id' => UselessMiddleware::class),
503501
array('id' => 'middleware_with_factory', 'arguments' => array('foo', 'bar')),
504502
array('id' => 'middleware_with_factory_using_default'),
505-
array('id' => 'allow_no_handler'),
506503
));
507504

508505
(new MessengerPass())->process($container);
509506
(new ResolveChildDefinitionsPass())->process($container);
510507

511-
$this->assertTrue($container->hasDefinition($childMiddlewareId = $fooBusId.'.middleware.allow_no_handler'));
512-
513508
$this->assertTrue($container->hasDefinition($factoryChildMiddlewareId = $fooBusId.'.middleware.middleware_with_factory'));
514509
$this->assertEquals(
515510
array('foo', 'bar'),
@@ -528,7 +523,6 @@ public function testRegistersMiddlewareFromServices()
528523
new Reference(UselessMiddleware::class),
529524
new Reference($factoryChildMiddlewareId),
530525
new Reference($factoryWithDefaultChildMiddlewareId),
531-
new Reference($childMiddlewareId),
532526
), $container->getDefinition($fooBusId)->getArgument(0));
533527
$this->assertFalse($container->hasParameter($middlewareParameter));
534528
}

‎src/Symfony/Component/Messenger/Tests/Handler/Locator/ContainerHandlerLocatorTest.php

+2-6
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,10 @@ public function testItLocatesHandlerUsingTheMessageClass()
2525
$this->assertSame($handler, $resolvedHandler);
2626
}
2727

28-
/**
29-
* @expectedException \Symfony\Component\Messenger\Exception\NoHandlerForMessageException
30-
* @expectedExceptionMessage No handler for message "Symfony\Component\Messenger\Tests\Fixtures\DummyMessage"
31-
*/
32-
public function testThrowsNoHandlerException()
28+
public function testNoHandlersReturnsNull()
3329
{
3430
$locator = new ContainerHandlerLocator(new Container());
35-
$locator->getHandler(new Envelope(new DummyMessage('Hey')));
31+
$this->assertNull($locator->getHandler(new Envelope(new DummyMessage('Hey'))));
3632
}
3733

3834
public function testGetHandlerViaInterface()

0 commit comments

Comments
 (0)