8000 [DependencyInjection] Added a way to define the priority of service decoration by dosten · Pull Request #15416 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] Added a way to define the priority of service decoration #15416

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.

Sign up for GitHub

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
Aug 5, 2015
Merged

[DependencyInjection] Added a way to define the priority of service decoration #15416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ CHANGELOG
* deprecated the concept of scopes
* added `Definition::setShared()` and `Definition::isShared()`
* added ResettableContainerInterface to be able to reset the container to release memory on shutdown
* added a way to define the priority of service decoration

2.7.0
-----
Expand Down
12 changes: 11 additions & 1 deletion src/Symfony/Component/DependencyInjection/Compiler/DecoratorServicePass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,28 @@
*
* @author Christophe Coevoet <stof@notk.org>
* @author Fabien Potencier <fabien@symfony.com>
* @author Diego Saint Esteben <diego@saintesteben.me>
*/
class DecoratorServicePass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
$definitions = new \SplPriorityQueue();
Copy link
Member

Choose a reason for hiding this comment

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

The usage of a priority query is a BC break here. Currently, decorators with the same priority (i.e. all of them for now) are applied in the order in which they appear in $container->getDefinitions(), which is more or less the order in which services have been defined (it is a bit more complex when a service gets overridden actually). but a SplPriorityQueue does not retain the order of insertion for the same priority.

And this is inconsistent with all other places using priorities in Symfony as we preserve the insertion order when the priority is the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stof Thanks for your feedback, I missed up that part of the documentation. Using arrays and krsort is the way to go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stof Can you check again? I've implemented the same solution of ProfilerPass.

$order = PHP_INT_MAX;

foreach ($container->getDefinitions() as $id => $definition) {
if (!$decorated = $definition->getDecoratedService()) {
continue;
}
$definitions->insert(array($id, $definition), array($decorated[2], --$order));
}

foreach ($definitions as $arr) {
list($id, $definition) = $arr;
list($inner, $renamedId) = $definition->getDecoratedService();

$definition->setDecoratedService(null);

list($inner, $renamedId) = $decorated;
if (!$renamedId) {
$renamedId = $id.'.inner';
}
Expand Down
7 changes: 4 additions & 3 deletions src/Symfony/Component/DependencyInjection/Definition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,13 @@ public function setFactoryMethod($factoryMethod)
*
* @param null|string $id The decorated service id, use null to remove decoration
* @param null|string $renamedId The new decorated service id
* @param int $priority The priority of decoration
*
* @return Definition The current instance
*
* @throws InvalidArgumentException In case the decorated service id and the new decorated service id are equals.
*/
public function setDecoratedService($id, $renamedId = null)
public function setDecoratedService($id, $renamedId = null, $priority = 0)
{
if ($renamedId && $id == $renamedId) {
throw new \InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id));
Expand All @@ -164,7 +165,7 @@ public function setDecoratedService($id, $renamedId = null)
if (null === $id) {
$this->decoratedService = null;
} else {
$this->decoratedService = array($id, $renamedId);
$this->decoratedService = array($id, $renamedId, (int) $priority);
}

return $this;
Expand All @@ -173,7 +174,7 @@ public function setDecoratedService($id, $renamedId = null)
/**
* Gets the service that decorates this service.
*
* @return null|array An array composed of the decorated service id and the new id for it, null if no service is decorated
* @return null|array An array composed of the decorated service id, the new id for it and the priority of decoration, null if no service is decorated
*/
public function getDecoratedService()
{
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/DependencyInjection/DefinitionDecorator.php
View file 67E6
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,11 @@ public function setLazy($boolean)
/**
* {@inheritdoc}
*/
public function setDecoratedService($id, $renamedId = null)
public function setDecoratedService($id, $renamedId = null, $priority = 0)
{
$this->changes['decorated_service'] = true;

return parent::setDecoratedService($id, $renamedId);
return parent::setDecoratedService($id, $renamedId, $priority);
}

/**
Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,14 @@ private function addService($definition, $id, \DOMElement $parent)
$service->setAttribute('lazy', 'true');
}
if (null !== $decorated = $definition->getDecoratedService()) {
list($decorated, $renamedId) = $decorated;
list($decorated, $renamedId, $priority) = $decorated;
$service->setAttribute('decorates', $decorated);
if (null !== $renamedId) {
$service->setAttribute('decoration-inner-name', $renamedId);
}
if (0 !== $priority) {
$service->setAttribute('decoration-priority', $priority);
}
}

foreach ($definition->getTags() as $name => $tags) {
Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,14 @@ private function addService($id, $definition)
}

if (null !== $decorated = $definition->getDecoratedService()) {
list($decorated, $renamedId) = $decorated;
list($decorated, $renamedId, $priority) = $decorated;
$code .= sprintf(" decorates: %s\n", $decorated);
if (null !== $renamedId) {
$code .= sprintf(" decoration_inner_name: %s\n", $renamedId);
}
if (0 !== $priority) {
$code .= sprintf(" decoration_priority: %s\n", $priority);
}
}

if ($callable = $definition->getFactory()) {
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ private function parseDefinition(\DOMElement $service, $file)

if ($value = $service->getAttribute('decorates')) {
$renameId = $service->hasAttribute('decoration-inner-name') ? $service->getAttribute('decoration-inner-name') : null;
$definition->setDecoratedService($value, $renameId);
$priority = $service->hasAttribute('decoration-priority') ? $service->getAttribute('decoration-priority') : 0;
$definition->setDecoratedService($value, $renameId, $priority);
}

return $definition;
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ private function parseDefinition($id, $service, $file)

if (isset($service['decorates'])) {
$renameId = isset($service['decoration_inner_name']) ? $service['decoration_inner_name'] : null;
$definition->setDecoratedService($service['decorates'], $renameId);
$priority = isset($service['decoration_priority']) ? $service['decoration_priority'] : 0;
$definition->setDecoratedService($service['decorates'], $renameId, $priority);
}

$this->container->setDefinition($id, $definition);
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
<xsd:attribute name="parent" type="xsd:string" />
<xsd:attribute name="decorates" type="xsd:string" />
<xsd:attribute name="decoration-inner-name" type="xsd:string" />
<xsd:attribute name="decoration-priority" type="xsd:integer" />
</xsd:complexType>

<xsd:complexType name="tag">
Expand Down
42 changes: 42 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Compiler/DecoratorServicePassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,48 @@ public function testProcessWithAlias()
$this->assertNull($fooExtendedDefinition->getDecoratedService());
}

public function testProcessWithPriority()
{
$container = new ContainerBuilder();
$fooDefinition = $container
->register('foo')
->setPublic(false)
;
$barDefinition = $container
->register('bar')
->setPublic(true)
->setDecoratedService('foo')
;
$bazDefinition = $container
->register('baz')
->setPublic(true)
->setDecoratedService('foo', null, 5)
;
$quxDefinition = $container
->register('qux')
->setPublic(true)
->setDecoratedService('foo', null, 3)
;

$this->process($container);

$this->assertEquals('bar', $container->getAlias('foo'));
$this->assertFalse($container->getAlias('foo')->isPublic());

$this->assertSame($fooDefinition, $container->getDefinition('baz.inner'));
$this->assertFalse($container->getDefinition('baz.inner')->isPublic());

$this->assertEquals('qux', $container->getAlias('bar.inner'));
$this->assertFalse($container->getAlias('bar.inner')->isPublic());

$this->assertEquals('baz', $container->getAlias('qux.inner'));
$this->assertFalse($container->getAlias('qux.inner')->isPublic());

$this->assertNull($barDefinition->getDecoratedService());
$this->assertNull($bazDefinition->getDecoratedService());
$this->assertNull($quxDefinition->getDecoratedService());
}

protected function process(ContainerBuilder $container)
{
$repeatedPass = new DecoratorServicePass();
Expand Down
2 changes: 1 addition & 1 deletion ...mfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public function testSetDecoratedServiceOnServiceHasParent()
->setDecoratedService('foo', 'foo_inner')
;

$this->assertEquals(array('foo', 'foo_inner'), $container->getDefinition('child1')->getDecoratedService());
$this->assertEquals(array('foo', 'foo_inner', 0), $container->getDefinition('child1')->getDecoratedService());
}

protected function process(ContainerBuilder $container)
Expand Down
11 changes: 9 additions & 2 deletions src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,23 @@ public function testSetGetClass()

public function testSetGetDecoratedService()
{
$def = new Definition('stdClass');
$this->assertNull($def->getDecoratedService());
$def->setDecoratedService('foo', 'foo.renamed', 5);
$this->assertEquals(array('foo', 'foo.renamed', 5), $def->getDecoratedService());
$def->setDecoratedService(null);
$this->assertNull($def->getDecoratedService());

$def = new Definition('stdClass');
$this->assertNull($def->getDecoratedService());
$def->setDecoratedService('foo', 'foo.renamed');
$this->assertEquals(array('foo', 'foo.renamed'), $def->getDecoratedService());
$this->assertEquals(array('foo', 'foo.renamed', 0), $def->getDecoratedService());
$def->setDecoratedService(null);
$this->assertNull($def->getDecoratedService());

$def = new Definition('stdClass');
$def->setDecoratedService('foo');
$this->assertEquals(array('foo', null), $def->getDecoratedService());
$this->assertEquals(array('foo', null, 0), $def->getDecoratedService());
$def->setDecoratedService(null);
$this->assertNull($def->getDecoratedService());

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
<service id="another_alias_for_foo" alias="foo" public="false" />
<service id="decorator_service" decorates="decorated" />
<service id="decorator_service_with_name" decorates="decorated" decoration-inner-name="decorated.pif-pouf"/>
<service id="decorator_service_with_name_and_priority" decorates="decorated" decoration-inner-name="decorated.pif-pouf" decoration-priority="5"/>
<service id="new_factory1" class="FooBarClass">
<factory function="factory" />
</service>
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ services:
decorator_service_with_name:
decorates: decorated
decoration_inner_name: decorated.pif-pouf
decorator_service_with_name_and_priority:
decorates: decorated
decoration_inner_name: decorated.pif-pouf
decoration_priority: 5
new_factory1: { class: FooBarClass, factory: factory}
new_factory2: { class: FooBarClass, factory: [@baz, getClass]}
new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]}
5 changes: 3 additions & 2 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,9 @@ public function testLoadServices()
$this->assertEquals('foo', (string) $aliases['another_alias_for_foo']);
$this->assertFalse($aliases['another_alias_for_foo']->isPublic());

$this->assertEquals(array('decorated', null), $services['decorator_service']->getDecoratedService());
$this->assertEquals(array('decorated', 'decorated.pif-pouf'), $services['decorator_service_with_name']->getDecoratedService());
$this->assertEquals(array('decorated', null, 0), $services['decorator_service']->getDecoratedService());
$this->assertEquals(array('decorated', 'decorated.pif-pouf', 0), $services['decorator_service_with_name']->getDecoratedService());
$this->assertEquals(array('decorated', 'decorated.pif-pouf', 5), $services['decorator_service_with_name_and_priority']->getDecoratedService());
}

public function testParsesTags()
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,9 @@ public function testLoadServices()
$this->assertEquals('foo', (string) $aliases['another_alias_for_foo']);
$this->assertFalse($aliases['another_alias_for_foo']->isPublic());

$this->assertEquals(array('decorated', null), $services['decorator_service']->getDecoratedService());
$this->assertEquals(array('decorated', 'decorated.pif-pouf'), $services['decorator_service_with_name']->getDecoratedService());
$this->assertEquals(array('decorated', null, 0), $services['decorator_service']->getDecoratedService());
$this->assertEquals(array('decorated', 'decorated.pif-pouf', 0), $services['decorator_service_with_name']->getDecoratedService());
$this->assertEquals(array('decorated', 'decorated.pif-pouf', 5), $services['decorator_service_with_name_and_priority']->getDecoratedService());
}

public function testLoadFactoryShortSyntax()
Expand Down
0