Skip to content

[DependencyInjection] added a simple way to replace a service by keeping a reference to the old one #10600

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 2 commits into from
Apr 1, 2014
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
5 changes: 5 additions & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

2.5.0
-----

* added DecoratorServicePass and a way to override a service definition (Definition::setDecoratedService())

2.4.0
-----

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?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\Component\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Alias;

/**
* Overwrites a service but keeps the overridden one.
*
* @author Christophe Coevoet <stof@notk.org>
* @author Fabien Potencier <fabien@symfony.com>
*/
class DecoratorServicePass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
foreach ($container->getDefinitions() as $id => $definition) {
if (!$decorated = $definition->getDecoratedService()) {
continue;
}
$definition->setDecoratedService(null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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


list ($inner, $renamedId) = $decorated;
if (!$renamedId) {
$renamedId = $id.'.inner';
}

// we create a new alias/service for the service we are replacing
// to be able to reference it in the new one
if ($container->hasAlias($inner)) {
$alias = $container->getAlias($inner);
$public = $alias->isPublic();
$container->setAlias($renamedId, new Alias((string) $alias, false));
} else {
$definition = $container->getDefinition($inner);
$public = $definition->isPublic();
$definition->setPublic(false);
$container->setDefinition($renamedId, $definition);
}

$container->setAlias($inner, new Alias($id, $public));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function __construct()

$this->optimizationPasses = array(
new ResolveDefinitionTemplatesPass(),
new DecoratorServicePass(),
new ResolveParameterPlaceHoldersPass(),
new CheckDefinitionValidityPass(),
new ResolveReferencesToAliasesPass(),
Expand Down
36 changes: 36 additions & 0 deletions src/Symfony/Component/DependencyInjection/Definition.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class Definition
private $abstract = false;
private $synchronized = false;
private $lazy = false;
private $decoratedService;

protected $arguments;

Expand Down Expand Up @@ -100,6 +101,41 @@ public function setFactoryMethod($factoryMethod)
return $this;
}

/**
* Sets the service that this service is decorating.
*
* @param null|string $id The decorated service id, use null to remove decoration
* @param null|string $renamedId The new decorated service id
*
* @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)
Copy link
Member

Choose a reason for hiding this comment

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

this method currently does not allow to reset it. We should find a way to do it (maybe passing null as id) as we will need to do it

{
if ($renamedId && $id == $renamedId) {
throw new \InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id));
}

if (null === $id) {
$this->decoratedService = null;
} else {
$this->decoratedService = array($id, $renamedId);
}

return $this;
}

/**
* 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
*/
public function getDecoratedService()
{
return $this->decoratedService;
}

/**
* Gets the factory method.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ private function addService($definition, $id, \DOMElement $parent)
if ($definition->isLazy()) {
$service->setAttribute('lazy', 'true');
}
if (null !== $decorated = $definition->getDecoratedService()) {
list ($decorated, $renamedId) = $decorated;
$service->setAttribute('decorates', $decorated);
if (null !== $renamedId) {
$service->setAttribute('decoration-inner-name', $renamedId);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I see an issue here: given that the compiler pass does not remove the decoration settings after resolving the decorators, dumping and reloading will break everything (circular reference as the service will decorate itself).

I think the only way to make it work is to alter the definition to remove the decoration setting once it has been processed, to allow dumping a compiled container without issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand correctly, this should be done in DecoratorServicePass, right ?

Copy link
Member

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

humm, actually I don't get the issue: I'm trying this:

require __DIR__ . '/vendor/autoload.php';

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\Config\Loader\Loader;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\DependencyInjection\Dumper\XmlDumper;
use Symfony\Component\Config\Loader\LoaderResolver;
use Symfony\Component\Config\FileLocator;

$file = __DIR__ . '/generated.xml';

$container = new ContainerBuilder();
$loader = new XmlFileLoader($container, new FileLocator(__DIR__));
$loader->load('services.xml');
$container->compile();

$dumper = new XmlDumper($container);
file_put_contents($file, $dumper->dump());

$container2 = new ContainerBuilder();
$loader = new XmlFileLoader($container2, new FileLocator(__DIR__));
$loader->load($file);
$container2->compile();

with sevices.xml:

<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
  <services>
    <service id="decorated" class="stdClass" />
    <service id="decorator_service" class="stdClass" decorates="decorated" />
    <service id="decorator_service_with_name" class="stdClass" decorates="decorated" decoration-inner-name="decorated.pif-pouf"/>
  </services>
</container>

that generates:

<?xml version="1.0" encoding="utf-8"?>
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
  <services>
    <service id="decorator_service" class="stdClass" decorates="decorated"/>
    <service id="decorator_service_with_name" class="stdClass" decorates="decorated" decoration-inner-name="decorated.pif-pouf"/>
    <service id="decorated" alias="decorator_service_with_name"/>
  </services>
</container>

Copy link
Member

Choose a reason for hiding this comment

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

Use services which injects the inner service as argument (otherwise, using decorates is quite useless compared to just overwriting the service. The goal of decorates is to apply a decorator)

Copy link
Member

Choose a reason for hiding this comment

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

<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
  <services>
    <service id="decorated" class="stdClass" />
    <service id="decorator_service" class="stdClass" decorates="decorated">
        <argument type="service" id="decorator_service.inner" />
    </service>
    <service id="decorator_service_with_name" class="stdClass" decorates="decorated" decoration-inner-name="decorated.pif-pouf">
        <argument type="service" id="decorated.pif-pouf" />
    </service>
  </services>
</container>


foreach ($definition->getTags() as $name => $tags) {
foreach ($tags as $attributes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ private function addService($id, $definition)
$code .= sprintf(" scope: %s\n", $scope);
}

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

if ($callable = $definition->getConfigurator()) {
if (is_array($callable)) {
if ($callable[0] instanceof Reference) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ private function parseDefinition($id, $service, $file)
$definition->addTag((string) $tag['name'], $parameters);
}

if (isset($service['decorates'])) {
$renameId = isset($service['decoration-inner-name']) ? (string) $service['decoration-inner-name'] : null;
$definition->setDecoratedService((string) $service['decorates'], $renameId);
}

$this->container->setDefinition($id, $definition);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ 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);
}

$this->container->setDefinition($id, $definition);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@
<xsd:attribute name="factory-service" type="xsd:string" />
<xsd:attribute name="alias" type="xsd:string" />
<xsd:attribute name="parent" type="xsd:string" />
<xsd:attribute name="decorates" type="xsd:string" />
<xsd:attribute name="decoration-inner-name" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="tag">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

namespace Symfony\Component\DependencyInjection\Tests\Compiler;

use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\DecoratorServicePass;

class DecoratorServicePassTest extends \PHPUnit_Framework_TestCase
{
public function testProcessWithoutAlias()
{
$container = new ContainerBuilder();
$fooDefinition = $container
->register('foo')
->setPublic(false)
;
$fooExtendedDefinition = $container
->register('foo.extended')
->setPublic(true)
->setDecoratedService('foo')
;
$barDefinition = $container
->register('bar')
->setPublic(true)
;
$barExtendedDefinition = $container
->register('bar.extended')
->setPublic(true)
->setDecoratedService('bar', 'bar.yoo')
;

$this->process($container);

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

$this->assertEquals('bar.extended', $container->getAlias('bar'));
$this->assertTrue($container->getAlias('bar')->isPublic());

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

$this->assertSame($barDefinition, $container->getDefinition('bar.yoo'));
$this->assertFalse($container->getDefinition('bar.yoo')->isPublic());

$this->assertNull($fooExtendedDefinition->getDecoratedService());
$this->assertNull($barExtendedDefinition->getDecoratedService());
}

public function testProcessWithAlias()
{
$container = new ContainerBuilder();
$container
->register('foo')
->setPublic(true)
;
$container->setAlias('foo.alias', new Alias('foo', false));
$fooExtendedDefinition = $container
->register('foo.extended')
->setPublic(true)
->setDecoratedService('foo.alias')
;

$this->process($container);

$this->assertEquals('foo.extended', $container->getAlias('foo.alias'));
$this->assertFalse($container->getAlias('foo.alias')->isPublic());

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

$this->assertNull($fooExtendedDefinition->getDecoratedService());
}

protected function process(ContainerBuilder $container)
{
$repeatedPass = new DecoratorServicePass();
$repeatedPass->process($container);
}
}
20 changes: 20 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,26 @@ public function testSetGetClass()
$this->assertEquals('foo', $def->getClass(), '->getClass() returns the class name');
}

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

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

$def = new Definition('stdClass');
$this->setExpectedException('InvalidArgumentException', 'The decorated service inner name for "foo" must be different than the service name itself.');
$def->setDecoratedService('foo', 'foo');
}

/**
* @covers Symfony\Component\DependencyInjection\Definition::setArguments
* @covers Symfony\Component\DependencyInjection\Definition::getArguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function testAddService()

public function testDumpAnonymousServices()
{
include self::$fixturesPath.'/containers/container11.php';
$container = include self::$fixturesPath.'/containers/container11.php';
$dumper = new XmlDumper($container);
$this->assertEquals("<?xml version=\"1.0\" encoding=\"utf-8\"?>
<container xmlns=\"http://symfony.com/schema/dic/services\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:schemaLocation=\"http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd\">
Expand All @@ -87,7 +87,7 @@ public function testDumpAnonymousServices()

public function testDumpEntities()
{
include self::$fixturesPath.'/containers/container12.php';
$container = include self::$fixturesPath.'/containers/container12.php';
$dumper = new XmlDumper($container);
$this->assertEquals("<?xml version=\"1.0\" encoding=\"utf-8\"?>
<container xmlns=\"http://symfony.com/schema/dic/services\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:schemaLocation=\"http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd\">
Expand All @@ -100,4 +100,35 @@ public function testDumpEntities()
</container>
", $dumper->dump());
}

/**
* @dataProvider provideDecoratedServicesData
*/
public function testDumpDecoratedServices($expectedXmlDump, $container)
{
$dumper = new XmlDumper($container);
$this->assertEquals($expectedXmlDump, $dumper->dump());
}

public function provideDecoratedServicesData()
{
$fixturesPath = realpath(__DIR__.'/../Fixtures/');

return array(
array("<?xml version=\"1.0\" encoding=\"utf-8\"?>
<container xmlns=\"http://symfony.com/schema/dic/services\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:schemaLocation=\"http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd\">
<services>
<service id=\"foo\" class=\"FooClass\Foo\" decorates=\"bar\" decoration-inner-name=\"bar.woozy\"/>
</services>
</container>
", include $fixturesPath.'/containers/container15.php'),
array("<?xml version=\"1.0\" encoding=\"utf-8\"?>
<container xmlns=\"http://symfony.com/schema/dic/services\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:schemaLocation=\"http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd\">
<services>
<service id=\"foo\" class=\"FooClass\Foo\" decorates=\"bar\"/>
</services>
</container>
", include $fixturesPath.'/containers/container16.php'),
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

use Symfony\Component\DependencyInjection\ContainerBuilder;

$container = new ContainerBuilder();
$container
->register('foo', 'FooClass\\Foo')
->setDecoratedService('bar', 'bar.woozy')
;

return $container;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

use Symfony\Component\DependencyInjection\ContainerBuilder;

$container = new ContainerBuilder();
$container
->register('foo', 'FooClass\\Foo')
->setDecoratedService('bar')
;

return $container;
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,16 @@
->register('configured_service', 'stdClass')
->setConfigurator(array(new Reference('configurator_service'), 'configureStdClass'))
;
$container
->register('decorated', 'stdClass')
;
$container
->register('decorator_service', 'stdClass')
->setDecoratedService('decorated')
;
$container
->register('decorator_service_with_name', 'stdClass')
->setDecoratedService('decorated', 'decorated.pif-pouf')
;

return $container;
Loading