Skip to content

[Messenger] Middleware factories support in config #27128

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
May 14, 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
@@ -0,0 +1,37 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\Messenger;

use Symfony\Bridge\Doctrine\ManagerRegistry;

/**
* Create a Doctrine ORM transaction middleware to be used in a message bus from an entity manager name.
*
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
*
* @experimental in 4.1
* @final
*/
class DoctrineTransactionMiddlewareFactory
{
private $managerRegistry;

public function __construct(ManagerRegistry $managerRegistry)
{
$this->managerRegistry = $managerRegistry;
}

public function createMiddleware(string $managerName): DoctrineTransactionMiddleware
{
return new DoctrineTransactionMiddleware($this->managerRegistry, $managerName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,36 @@ function ($a) {
})
->end()
->defaultValue(array())
->prototype('scalar')->end()
->prototype('array')
->beforeNormalization()
->always()
->then(function ($middleware): array {
if (!\is_array($middleware)) {
return array('id' => $middleware);
}
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)));
}

return array(
'id' => key($middleware),
'arguments' => current($middleware),
);
})
->end()
->fixXmlConfig('argument')
->children()
->scalarNode('id')->isRequired()->cannotBeEmpty()->end()
->arrayNode('arguments')
->normalizeKeys(false)
->defaultValue(array())
->prototype('variable')
->end()
->end()
->end()
->end()
->end()
->end()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1471,12 +1471,17 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
$config['default_bus'] = key($config['buses']);
}

$defaultMiddleware = array('before' => array('logging'), 'after' => array('route_messages', 'call_message_handler'));
$defaultMiddleware = array(
'before' => array(array('id' => 'logging')),
'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'];

if (!$validationConfig['enabled'] && \in_array('messenger.middleware.validation', $middleware, true)) {
throw new LogicException('The Validation middleware is only available when the Validator component is installed and enabled. Try running "composer require symfony/validator".');
foreach ($middleware as $middlewareItem) {
if (!$validationConfig['enabled'] && 'messenger.middleware.validation' === $middlewareItem['id']) {
throw new LogicException('The Validation middleware is only available when the Validator component is installed and enabled. Try running "composer require symfony/validator".');
}
}

$container->setParameter($busId.'.middleware', $middleware);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,16 @@

<xsd:complexType name="messenger_bus">
<xsd:sequence>
<xsd:element name="middleware" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
<xsd:element name="middleware" type="messenger_middleware" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
<xsd:attribute name="name" type="xsd:string" use="required"/>
<xsd:attribute name="default-middleware" type="xsd:boolean"/>
</xsd:complexType>

<xsd:complexType name="messenger_middleware">
<xsd:sequence>
<xsd:element name="argument" type="xsd:anyType" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
<xsd:attribute name="id" type="xsd:string" use="required"/>
</xsd:complexType>
</xsd:schema>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

$container->loadFromExtension('framework', array(
'messenger' => array(
'buses' => array(
'command_bus' => array(
'middleware' => array(
array(
'foo' => array('qux'),
'bar' => array('baz'),
),
),
),
),
),
));
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
'messenger.bus.commands' => null,
'messenger.bus.events' => array(
'middleware' => array(
array('with_factory' => array('foo', true, array('bar' => 'baz'))),
'allow_no_handler',
),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,19 @@
<framework:messenger default-bus="messenger.bus.commands">
<framework:bus name="messenger.bus.commands" />
<framework:bus name="messenger.bus.events">
<framework:middleware>allow_no_handler</framework:middleware>
<framework:middleware id="with_factory">
<framework:argument>foo</framework:argument>
<framework:argument>true</framework:argument>
<framework:argument>
<framework:bar>baz</framework:bar>
</framework:argument>
</framework:middleware>
<framework:middleware id="allow_no_handler" />
</framework:bus>
<framework:bus name="messenger.bus.queries" default-middleware="false">
<framework:middleware>route_messages</framework:middleware>
<framework:middleware>allow_no_handler</framework:middleware>
<framework:middleware>call_message_handler</framework:middleware>
<framework:middleware id="route_messages" />
<framework:middleware id="allow_no_handler" />
<framework:middleware id="call_message_handler" />
</framework:bus>
</framework:messenger>
</framework:config>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
framework:
messenger:
buses:
command_bus:
middleware:
- foo: ['qux']
bar: ['baz']
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ framework:
messenger.bus.commands: ~
messenger.bus.events:
middleware:
- with_factory: [foo, true, { bar: baz }]
- "allow_no_handler"
messenger.bus.queries:
default_middleware: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,18 +604,41 @@ public function testMessengerWithMultipleBuses()

$this->assertTrue($container->has('messenger.bus.commands'));
$this->assertSame(array(), $container->getDefinition('messenger.bus.commands')->getArgument(0));
$this->assertEquals(array('logging', 'route_messages', 'call_message_handler'), $container->getParameter('messenger.bus.commands.middleware'));
$this->assertEquals(array(
array('id' => 'logging'),
array('id' => 'route_messages'),
array('id' => 'call_message_handler'),
), $container->getParameter('messenger.bus.commands.middleware'));
$this->assertTrue($container->has('messenger.bus.events'));
$this->assertSame(array(), $container->getDefinition('messenger.bus.events')->getArgument(0));
$this->assertEquals(array('logging', 'allow_no_handler', 'route_messages', 'call_message_handler'), $container->getParameter('messenger.bus.events.middleware'));
$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'));
$this->assertTrue($container->has('messenger.bus.queries'));
$this->assertSame(array(), $container->getDefinition('messenger.bus.queries')->getArgument(0));
$this->assertEquals(array('route_messages', 'allow_no_handler', 'call_message_handler'), $container->getParameter('messenger.bus.queries.middleware'));
$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'));

$this->assertTrue($container->hasAlias('message_bus'));
$this->assertSame('messenger.bus.commands', (string) $container->getAlias('message_bus'));
}

/**
* @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"]}"
*/
public function testMessengerMiddlewareFactoryErroneousFormat()
{
$this->createContainerFromFile('messenger_middleware_factory_erroneous_format');
}

public function testTranslator()
{
$container = $this->createContainerFromFile('full');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@ public function testAssetsHelperIsRemovedWhenPhpTemplatingEngineIsEnabledAndAsse
{
$this->markTestSkipped('The assets key cannot be set to false using the XML configuration format.');
}

public function testMessengerMiddlewareFactoryErroneousFormat()
{
$this->markTestSkipped('XML configuration will not allow eeroneous format.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -221,24 +221,37 @@ private function registerBusToCollector(ContainerBuilder $container, string $bus
$container->getDefinition('messenger.data_collector')->addMethodCall('registerBus', array($busId, new Reference($tracedBusId)));
}

private function registerBusMiddleware(ContainerBuilder $container, string $busId, array $middleware)
private function registerBusMiddleware(ContainerBuilder $container, string $busId, array $middlewareCollection)
{
$container->getDefinition($busId)->replaceArgument(0, array_map(function (string $name) use ($container, $busId) {
if (!$container->has($messengerMiddlewareId = 'messenger.middleware.'.$name)) {
$messengerMiddlewareId = $name;
$middlewareReferences = array();
foreach ($middlewareCollection as $middlewareItem) {
$id = $middlewareItem['id'];
$arguments = $middlewareItem['arguments'] ?? array();
if (!$container->has($messengerMiddlewareId = 'messenger.middleware.'.$id)) {
$messengerMiddlewareId = $id;
}

if (!$container->has($messengerMiddlewareId)) {
throw new RuntimeException(sprintf('Invalid middleware "%s": define such service to be able to use it.', $name));
throw new RuntimeException(sprintf('Invalid middleware "%s": define such service to be able to use it.', $id));
}

if ($container->getDefinition($messengerMiddlewareId)->isAbstract()) {
if (($definition = $container->findDefinition($messengerMiddlewareId))->isAbstract()) {
$childDefinition = new ChildDefinition($messengerMiddlewareId);
$count = \count($definition->getArguments());
foreach (array_values($arguments ?? array()) as $key => $argument) {
// Parent definition can provide default arguments.
// Replace each explicitly or add if not set:
$key < $count ? $childDefinition->replaceArgument($key, $argument) : $childDefinition->addArgument($argument);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just using setArgument here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to the way child definitions arguments work, using replaceArgument saves an extra hard coded index_ prefix for the key here.

if (is_int($index)) {
$this->arguments['index_'.$index] = $value;

}

$container->setDefinition($messengerMiddlewareId = $busId.'.middleware.'.$name, $childDefinition);
$container->setDefinition($messengerMiddlewareId = $busId.'.middleware.'.$id, $childDefinition);
} elseif ($arguments) {
throw new RuntimeException(sprintf('Invalid middleware factory "%s": a middleware factory must be an abstract definition.', $id));
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we put that first as a simple if so we don't have this extra indentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, it'll still need the abstract check and won't save any indent, right?

}

return new Reference($messengerMiddlewareId);
}, $middleware));
$middlewareReferences[] = new Reference($messengerMiddlewareId);
}

$container->getDefinition($busId)->replaceArgument(0, $middlewareReferences);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ServiceLocator;
Expand Down Expand Up @@ -312,14 +313,42 @@ public function testRegistersMiddlewareFromServices()
$container = $this->getContainerBuilder();
$container->register($fooBusId = 'messenger.bus.foo', MessageBusInterface::class)->setArgument(0, array())->addTag('messenger.bus');
$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);

$container->setParameter($middlewareParameter = $fooBusId.'.middleware', array(UselessMiddleware::class, 'allow_no_handler'));
$container->setParameter($middlewareParameter = $fooBusId.'.middleware', array(
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->assertEquals(array(new Reference(UselessMiddleware::class), new Reference($childMiddlewareId)), $container->getDefinition($fooBusId)->getArgument(0));

$this->assertTrue($container->hasDefinition($factoryChildMiddlewareId = $fooBusId.'.middleware.middleware_with_factory'));
$this->assertEquals(
array('foo', 'bar'),
$container->getDefinition($factoryChildMiddlewareId)->getArguments(),
'parent default argument is overridden, and next ones appended'
);

$this->assertTrue($container->hasDefinition($factoryWithDefaultChildMiddlewareId = $fooBusId.'.middleware.middleware_with_factory_using_default'));
$this->assertEquals(
array('some_default'),
$container->getDefinition($factoryWithDefaultChildMiddlewareId)->getArguments(),
'parent default argument is used'
);

$this->assertEquals(array(
new Reference(UselessMiddleware::class),
new Reference($factoryChildMiddlewareId),
new Reference($factoryWithDefaultChildMiddlewareId),
new Reference($childMiddlewareId),
), $container->getDefinition($fooBusId)->getArgument(0));
$this->assertFalse($container->hasParameter($middlewareParameter));
}

Expand All @@ -331,7 +360,25 @@ public function testCannotRegistersAnUndefinedMiddleware()
{
$container = $this->getContainerBuilder();
$container->register($fooBusId = 'messenger.bus.foo', MessageBusInterface::class)->setArgument(0, array())->addTag('messenger.bus');
$container->setParameter($middlewareParameter = $fooBusId.'.middleware', array('not_defined_middleware'));
$container->setParameter($middlewareParameter = $fooBusId.'.middleware', array(
array('id' => 'not_defined_middleware', 'arguments' => array()),
));

(new MessengerPass())->process($container);
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Invalid middleware factory "not_an_abstract_definition": a middleware factory must be an abstract definition.
*/
public function testMiddlewareFactoryDefinitionMustBeAbstract()
{
$container = $this->getContainerBuilder();
$container->register('not_an_abstract_definition', UselessMiddleware::class);
$container->register($fooBusId = 'messenger.bus.foo', MessageBusInterface::class)->setArgument(0, array())->addTag('messenger.bus', array('name' => 'foo'));
$container->setParameter($middlewareParameter = $fooBusId.'.middleware', array(
array('id' => 'not_an_abstract_definition', 'arguments' => array('foo')),
));

(new MessengerPass())->process($container);
}
Expand Down