From b79d097c2af2f348e94ac99ca7cef2794baf5b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haso=C5=88?= Date: Wed, 8 Aug 2018 13:02:30 +0200 Subject: [PATCH 1/2] [DI] Fix autowire inner service --- .../Compiler/AutowirePass.php | 7 +++--- .../Tests/Compiler/AutowirePassTest.php | 23 +++++++++++++++++++ .../Fixtures/includes/autowiring_classes.php | 7 ++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 2d937f2840..e858d14fa1 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -144,11 +144,12 @@ class AutowirePass extends AbstractRecursivePass */ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot): array { + $this->decoratedId = null; + $this->decoratedClass = null; + $this->getPreviousValue = null; + if ($isRoot && ($definition = $this->container->getDefinition($this->currentId)) && $this->container->has($this->decoratedId = $definition->innerServiceId)) { $this->decoratedClass = $this->container->findDefinition($this->decoratedId)->getClass(); - } else { - $this->decoratedId = null; - $this->decoratedClass = null; } foreach ($this->methodCalls as $i => $call) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index cbbd886786..366583aba1 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -834,6 +834,29 @@ class AutowirePassTest extends TestCase $this->assertSame(Decorator::class.'.inner', (string) $definition->getArgument(1)); } + public function testAutowireDecoratorChain() + { + $container = new ContainerBuilder(); + $container->register(LoggerInterface::class, NullLogger::class); + $container->register(Decorated::class, Decorated::class); + $container + ->register(Decorator::class, Decorator::class) + ->setDecoratedService(Decorated::class) + ->setAutowired(true) + ; + $container + ->register(DecoratedDecorator::class, DecoratedDecorator::class) + ->setDecoratedService(Decorated::class) + ->setAutowired(true) + ; + + (new DecoratorServicePass())->process($container); + (new AutowirePass())->process($container); + + $definition = $container->getDefinition(DecoratedDecorator::class); + $this->assertSame(DecoratedDecorator::class.'.inner', (string) $definition->getArgument(0)); + } + public function testAutowireDecoratorRenamedId() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php index d806f294dd..93eab8c50f 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php @@ -373,6 +373,13 @@ class Decorator implements DecoratorInterface } } +class DecoratedDecorator implements DecoratorInterface +{ + public function __construct(DecoratorInterface $decorator) + { + } +} + class NonAutowirableDecorator implements DecoratorInterface { public function __construct(LoggerInterface $logger, DecoratorInterface $decorated1, DecoratorInterface $decorated2) From 4e92d10b40faf71f90e1164c96d2792054fa81da Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 8 Aug 2018 13:42:34 +0200 Subject: [PATCH 2/2] [DI] fix analyzing lazy refs involved in circular loops --- .../Compiler/AnalyzeServiceReferencesPass.php | 6 ++-- .../DependencyInjection/Dumper/PhpDumper.php | 31 +++++++------------ .../Tests/Dumper/PhpDumperTest.php | 2 +- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php index 19f01b8536..aeebbf0bf1 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php @@ -34,15 +34,17 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe private $graph; private $currentDefinition; private $onlyConstructorArguments; + private $hasProxyDumper; private $lazy; private $expressionLanguage; /** * @param bool $onlyConstructorArguments Sets this Service Reference pass to ignore method calls */ - public function __construct($onlyConstructorArguments = false) + public function __construct($onlyConstructorArguments = false, $hasProxyDumper = true) { $this->onlyConstructorArguments = (bool) $onlyConstructorArguments; + $this->hasProxyDumper = (bool) $hasProxyDumper; } /** @@ -97,7 +99,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe $targetId, $targetDefinition, $value, - $this->lazy || ($targetDefinition && $targetDefinition->isLazy()), + $this->lazy || ($this->hasProxyDumper && $targetDefinition && $targetDefinition->isLazy()), ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior() ); diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index cf6495b9c6..5530912c90 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\Argument\ArgumentInterface; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass; +use Symfony\Component\DependencyInjection\Compiler\CheckCircularReferencesPass; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -139,29 +140,19 @@ class PhpDumper extends Dumper $this->initializeMethodNamesMap('Container' === $baseClass ? Container::class : $baseClass); if ($this->getProxyDumper() instanceof NullDumper) { - (new AnalyzeServiceReferencesPass(true))->process($this->container); - $this->circularReferences = array(); - $checkedNodes = array(); - foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) { - $currentPath = array($id => $id); - $this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath); - } - foreach ($this->circularReferences as $parent => $ids) { - $path = array($parent); - while (!isset($ids[$parent])) { - foreach ($ids as $id) { - $path[] = $id; - $ids = $this->circularReferences[$id]; - break; - } - } - $path[] = $parent.'". Try running "composer require symfony/proxy-manager-bridge'; + (new AnalyzeServiceReferencesPass(true, false))->process($this->container); + try { + (new CheckCircularReferencesPass())->process($this->container); + } catch (ServiceCircularReferenceException $e) { + $path = $e->getPath(); + end($path); + $path[key($path)] .= '". Try running "composer require symfony/proxy-manager-bridge'; - throw new ServiceCircularReferenceException($parent, $path); + throw new ServiceCircularReferenceException($e->getServiceId(), $path); } } - (new AnalyzeServiceReferencesPass())->process($this->container); + (new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container); $this->circularReferences = array(); $checkedNodes = array(); foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) { @@ -367,7 +358,7 @@ EOTXT; $node = $edge->getDestNode(); $id = $node->getId(); - if ($node->getValue() && (($edge->isLazy() && !$this->getProxyDumper() instanceof NullDumper) || $edge->isWeak())) { + if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) { // no-op } elseif (isset($currentPath[$id])) { foreach (array_reverse($currentPath) as $parentId) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index b876bf48da..9c1b80cded 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -538,7 +538,7 @@ class PhpDumperTest extends TestCase $dumper = new PhpDumper($container); - $message = 'Circular reference detected for service "bar", path: "bar -> foo -> bar". Try running "composer require symfony/proxy-manager-bridge".'; + $message = 'Circular reference detected for service "foo", path: "foo -> bar -> foo". Try running "composer require symfony/proxy-manager-bridge".'; if (method_exists($this, 'expectException')) { $this->expectException(ServiceCircularReferenceException::class); $this->expectExceptionMessage($message);