Skip to content

Commit

Permalink
Fix circular reference in autowired decorators
Browse files Browse the repository at this point in the history
  • Loading branch information
shyim committed Aug 31, 2021
1 parent a665946 commit 999cfcf
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 7 deletions.
6 changes: 4 additions & 2 deletions Compiler/AutowirePass.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot,
$this->decoratedClass = null;
$this->getPreviousValue = null;

if ($isRoot && ($definition = $this->container->getDefinition($this->currentId)) && null !== ($this->decoratedId = $definition->innerServiceId) && $this->container->has($this->decoratedId)) {
$this->decoratedClass = $this->container->findDefinition($this->decoratedId)->getClass();
if ($isRoot && ($definition = $this->container->getDefinition($this->currentId)) && ($decoratedDefinition = $definition->getDecoratedService()) && null !== ($innerId = $decoratedDefinition[0]) && $this->container->has($innerId)) {
// If the class references to itself and is decorated, provide the inner service id and class to not get a circular reference
$this->decoratedClass = $this->container->findDefinition($innerId)->getClass();
$this->decoratedId = $decoratedDefinition[1] ?? $this->currentId.'.inner';
}

foreach ($this->methodCalls as $i => $call) {
Expand Down
28 changes: 23 additions & 5 deletions Tests/Compiler/AutowirePassTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -985,8 +985,8 @@ public function testAutowireDecorator()
->setAutowired(true)
;

(new DecoratorServicePass())->process($container);
(new AutowirePass())->process($container);
(new DecoratorServicePass())->process($container);

$definition = $container->getDefinition(Decorator::class);
$this->assertSame(Decorator::class.'.inner', (string) $definition->getArgument(1));
Expand All @@ -1008,8 +1008,8 @@ public function testAutowireDecoratorChain()
->setAutowired(true)
;

(new DecoratorServicePass())->process($container);
(new AutowirePass())->process($container);
(new DecoratorServicePass())->process($container);

$definition = $container->getDefinition(DecoratedDecorator::class);
$this->assertSame(DecoratedDecorator::class.'.inner', (string) $definition->getArgument(0));
Expand All @@ -1026,8 +1026,8 @@ public function testAutowireDecoratorRenamedId()
->setAutowired(true)
;

(new DecoratorServicePass())->process($container);
(new AutowirePass())->process($container);
(new DecoratorServicePass())->process($container);

$definition = $container->getDefinition(Decorator::class);
$this->assertSame('renamed', (string) $definition->getArgument(1));
Expand All @@ -1044,12 +1044,11 @@ public function testDoNotAutowireDecoratorWhenSeveralArgumentOfTheType()
->setAutowired(true)
;

(new DecoratorServicePass())->process($container);
try {
(new AutowirePass())->process($container);
$this->fail('AutowirePass should have thrown an exception');
} catch (AutowiringFailedException $e) {
$this->assertSame('Cannot autowire service "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator": argument "$decorated1" of method "__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\DecoratorInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator", "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator.inner".', (string) $e->getMessage());
$this->assertSame('Cannot autowire service "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator": argument "$decorated1" of method "__construct()" references interface "Symfony\Component\DependencyInjection\Tests\Compiler\DecoratorInterface" but no such service exists. You should maybe alias this interface to one of these existing services: "Symfony\Component\DependencyInjection\Tests\Compiler\Decorated", "Symfony\Component\DependencyInjection\Tests\Compiler\NonAutowirableDecorator".', (string) $e->getMessage());
}
}

Expand Down Expand Up @@ -1106,4 +1105,23 @@ public function testArgumentWithTarget()

$this->assertSame(BarInterface::class.' $imageStorage', (string) $container->getDefinition('with_target')->getArgument(0));
}

public function testDecorationWithServiceAndAliasedInterface()
{
$container = new ContainerBuilder();

$container->register(DecoratorImpl::class, DecoratorImpl::class)
->setAutowired(true)
->setPublic(true);
$container->setAlias(DecoratorInterface::class, DecoratorImpl::class)->setPublic(true);
$container->register(DecoratedDecorator::class, DecoratedDecorator::class)
->setAutowired(true)
->setPublic(true)
->setDecoratedService(DecoratorImpl::class);

$container->compile();

static::assertInstanceOf(DecoratedDecorator::class, $container->get(DecoratorInterface::class));
static::assertInstanceOf(DecoratedDecorator::class, $container->get(DecoratorImpl::class));
}
}
4 changes: 4 additions & 0 deletions Tests/Fixtures/includes/autowiring_classes.php
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,10 @@ interface DecoratorInterface
{
}

class DecoratorImpl implements DecoratorInterface
{
}

class Decorated implements DecoratorInterface
{
public function __construct($quz = null, \NonExistent $nonExistent = null, DecoratorInterface $decorated = null, array $foo = [])
Expand Down

0 comments on commit 999cfcf

Please sign in to comment.