8000 [DI] Service decoration: autowire the inner service by dunglas · Pull Request #25631 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DI] Service decoration: autowire the inner service #25631

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
[DI] Service decoration: autowire the inner service
  • Loading branch information
dunglas committed Mar 20, 2018
commit 8a9743d11089f04393bbffcab03f77fd52eb9d17
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\LazyProxy\ProxyHelper;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\TypedReference;

/**
* Overwrites a service but keeps the overridden one.
*
* @author Christophe Coevoet <stof@notk.org>
* @author Fabien Potencier <fabien@symfony.com>
* @author Diego Saint Esteben <diego@saintesteben.me>
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class DecoratorServicePass implements CompilerPassInterface
{
Expand Down Expand Up @@ -62,6 +67,38 @@ public function process(ContainerBuilder $container)
}

$container->setAlias($inner, $id)->setPublic($public)->setPrivate($private);
$this->autowire($container, $definition, $renamedId);
}
}

private function autowire(ContainerBuilder $container, Definition $definition, string $renamedId): void
{
if (!$definition->isAutowired() ||
null === ($innerClass = $container->findDefinition($renamedId)->getClass()) ||
!($reflectionClass = $container->getReflectionClass($definition->getClass())) ||
!$constructor = $reflectionClass->getConstructor()
) {
return;
}

$innerIndex = null;
foreach ($constructor->getParameters() as $index => $parameter) {
if (null === ($type = ProxyHelper::getTypeHint($constructor, $parameter, true)) ||
!is_a($innerClass, $type, true)
) {
continue;
}

if (null !== $innerIndex) {
// There is more than one argument of the type of the decorated class
return;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this case should result in an exception with helpful feedback for the developer. The developer obviously tried to autowire the decorator, so I think we should tell them why it fails.

Copy link
Member Author
@dunglas dunglas Dec 30, 2017

Choose a reason for hiding this comment

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

It would be a BC break: it currently tries to autowire these services "the regular way", if we throw an exception, it would not be the case anymore.

}

$innerIndex = $index;
}

if (null !== $innerIndex) {
$definition->setArgument($innerIndex, new TypedReference($renamedId, $innerClass));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\DecoratorServicePass;
use Symfony\Component\DependencyInjection\Tests\Fixtures\Bar;
use Symfony\Component\DependencyInjection\Tests\Fixtures\BarDecorator;
use Symfony\Component\DependencyInjection\Tests\Fixtures\NonAutowirableBarDecorator;

class DecoratorServicePassTest extends TestCase
{
Expand Down Expand Up @@ -144,6 +147,53 @@ public function testProcessMovesTagsFromDecoratedDefinitionToDecoratingDefinitio
$this->assertEquals(array('bar' => array('attr' => 'baz'), 'foobar' => array('attr' => 'bar')), $container->getDefinition('baz')->getTags());
}

public function testAutowire()
{
$container = new ContainerBuilder();
$container->register(Bar::class, Bar::class);
$container
->register(BarDecorator::class, BarDecorator::class)
->setDecoratedService(Bar::class)
->setAutowired(true)
;

$this->process($container);

$definition = $container->getDefinition(BarDecorator::class);
$this->assertCount(1, $definition->getArguments(), 'The "$logger" argument must not be autowired.');
$this->assertSame('Symfony\Component\DependencyInjection\Tests\Fixtures\BarDecorator.inner', (string) $definition->getArgument(1));
}

public function testDoNotAutowireWhenSeveralArgumentOfTheType()
{
$container = new ContainerBuilder();
$container->register(Bar::class, Bar::class);
$container
->register(NonAutowirableBarDecorator::class, NonAutowirableBarDecorator::class)
->setDecoratedService(Bar::class)
->setAutowired(true)
;

$this->process($container);

$this->assertEmpty($container->getDefinition(NonAutowirableBarDecorator::class)->getArguments());
}

public function testDoNotAutowireWhenNoConstructor()
{
$container = new ContainerBuilder();
$container->register(Bar::class, Bar::class);
$container
->register(NoConstructor::class, NoConstructor::class)
->setDecoratedService(Bar::class)
->setAutowired(true)
;

$this->process($container);

$this->assertEmpty($container->getDefinition(NoConstructor::class)->getArguments());
}

protected function process(ContainerBuilder $container)
{
$repeatedPass = new DecoratorServicePass();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?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\Tests\Fixtures;

use Psr\Log\LoggerInterface;

class BarDecorator implements BarInterface
{
public function __construct(LoggerInterface $logger, BarInterface $decorated)
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?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\Tests\Fixtures;

class NoConstructor
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?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\Tests\Fixtures;

use Psr\Log\LoggerInterface;

class NonAutowirableBarDecorator implements BarInterface
{
public function __construct(LoggerInterface $logger, BarInterface $decorated1, BarInterface $decorated2)
{
}
}
0