From 54af139a4ea2eb5ac0792fc0a355e5ecc9047642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Fri, 20 Nov 2020 19:00:05 +0100 Subject: [PATCH] [DependencyInjection] Fix circular in DI with lazy + byContruct loop --- .../Compiler/AnalyzeServiceReferencesPass.php | 9 +- .../DependencyInjection/Dumper/PhpDumper.php | 2 +- .../Tests/ContainerBuilderTest.php | 6 + .../Tests/Dumper/PhpDumperTest.php | 6 + .../containers/container_almost_circular.php | 36 ++++++ .../Tests/Fixtures/includes/classes.php | 12 ++ .../php/services_almost_circular_private.php | 77 ++++++++++- .../php/services_almost_circular_public.php | 120 ++++++++++++++++-- 8 files changed, 252 insertions(+), 16 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php index 1cd5ab93ee..92e4acacfb 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php @@ -12,6 +12,7 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Argument\ArgumentInterface; +use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; @@ -35,6 +36,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe private $hasProxyDumper; private $lazy; private $byConstructor; + private $byFactory; private $definitions; private $aliases; @@ -66,6 +68,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe $this->graph->clear(); $this->lazy = false; $this->byConstructor = false; + $this->byFactory = false; $this->definitions = $container->getDefinitions(); $this->aliases = $container->getAliases(); @@ -87,7 +90,7 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe $inExpression = $this->inExpression(); if ($value instanceof ArgumentInterface) { - $this->lazy = true; + $this->lazy = !$this->byFactory || !$value instanceof IteratorArgument; parent::processValue($value->getValues()); $this->lazy = $lazy; @@ -137,7 +140,11 @@ class AnalyzeServiceReferencesPass extends AbstractRecursivePass implements Repe $byConstructor = $this->byConstructor; $this->byConstructor = $isRoot || $byConstructor; + + $byFactory = $this->byFactory; + $this->byFactory = true; $this->processValue($value->getFactory()); + $this->byFactory = $byFactory; $this->processValue($value->getArguments()); $properties = $value->getProperties(); diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index a9c46edd66..a57ce41c59 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -420,7 +420,7 @@ EOF; foreach ($edges as $edge) { $node = $edge->getDestNode(); $id = $node->getId(); - if (!($definition = $node->getValue()) instanceof Definition || $sourceId === $id || ($edge->isLazy() && ($this->proxyDumper ?? $this->getProxyDumper())->isProxyCandidate($definition)) || $edge->isWeak()) { + if ($sourceId === $id || !$node->getValue() instanceof Definition || $edge->isLazy() || $edge->isWeak()) { continue; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index dc462a0ee5..b69e875a7e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -1374,12 +1374,18 @@ class ContainerBuilderTest extends TestCase $container = include __DIR__.'/Fixtures/containers/container_almost_circular.php'; $container->compile(); + $entityManager = $container->get('doctrine.entity_manager'); + $this->assertEquals(new \stdClass(), $entityManager); + $pA = $container->get('pA'); $this->assertEquals(new \stdClass(), $pA); $logger = $container->get('monolog.logger'); $this->assertEquals(new \stdClass(), $logger->handler); + $logger_inline = $container->get('monolog_inline.logger'); + $this->assertEquals(new \stdClass(), $logger_inline->handler); + $foo = $container->get('foo'); $this->assertSame($foo, $foo->bar->foobar->foo); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index 0e5e0f5a71..9f17269846 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -1054,12 +1054,18 @@ class PhpDumperTest extends TestCase $container = new $container(); + $entityManager = $container->get('doctrine.entity_manager'); + $this->assertEquals(new \stdClass(), $entityManager); + $pA = $container->get('pA'); $this->assertEquals(new \stdClass(), $pA); $logger = $container->get('monolog.logger'); $this->assertEquals(new \stdClass(), $logger->handler); + $logger_inline = $container->get('monolog_inline.logger'); + $this->assertEquals(new \stdClass(), $logger_inline->handler); + $foo = $container->get('foo'); $this->assertSame($foo, $foo->bar->foobar->foo); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php index 6a0b2da766..8dd0531696 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php @@ -1,5 +1,6 @@ register('doctrine.config', 'stdClass')->setPublic(false) + ->setProperty('resolver', new Reference('doctrine.entity_listener_resolver')) + ->setProperty('flag', 'ok'); + +$container->register('doctrine.entity_manager', 'stdClass')->setPublic(true) + ->setFactory([FactoryChecker::class, 'create']) + ->addArgument(new Reference('doctrine.config')); +$container->register('doctrine.entity_listener_resolver', 'stdClass')->setPublic($public) + ->addArgument(new IteratorArgument([new Reference('doctrine.listener')])); +$container->register('doctrine.listener', 'stdClass')->setPublic($public) + ->addArgument(new Reference('doctrine.entity_manager')); + // multiple path detection $container->register('pA', 'stdClass')->setPublic(true) @@ -42,6 +57,27 @@ $container->register('mailer.transport_factory.amazon', 'stdClass')->setPublic($ $container->register('monolog.logger_2', 'stdClass')->setPublic($public) ->setProperty('handler', new Reference('mailer.transport')); +// monolog-like + handler that require monolog with inlined factory + +$container->register('monolog_inline.logger', 'stdClass')->setPublic(true) + ->setProperty('handler', new Reference('mailer_inline.mailer')); + +$container->register('mailer_inline.mailer', 'stdClass')->setPublic(false) + ->addArgument( + (new Definition('stdClass')) + ->setFactory([new Reference('mailer_inline.transport_factory'), 'create']) + ); + +$container->register('mailer_inline.transport_factory', FactoryCircular::class)->setPublic($public) + ->addArgument(new TaggedIteratorArgument('mailer_inline.transport')); + +$container->register('mailer_inline.transport_factory.amazon', 'stdClass')->setPublic($public) + ->addArgument(new Reference('monolog_inline.logger_2')) + ->addTag('mailer.transport'); + +$container->register('monolog_inline.logger_2', 'stdClass')->setPublic($public) + ->setProperty('handler', new Reference('mailer_inline.mailer')); + // same visibility for deps $container->register('foo', FooCircular::class)->setPublic(true) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php index 46efa450ac..dcac28effc 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php @@ -128,6 +128,18 @@ class FactoryCircular } } +class FactoryChecker +{ + public static function create($config) + { + if (!isset($config->flag)) { + throw new \LogicException('The injected config must contain a "flag" property.'); + } + + return new stdClass(); + } +} + class FoobarCircular { public function __construct(FooCircular $foo) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php index 0ef3627e69..40c86fb88f 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php @@ -28,6 +28,7 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container 'baz6' => 'getBaz6Service', 'connection' => 'getConnectionService', 'connection2' => 'getConnection2Service', + 'doctrine.entity_manager' => 'getDoctrine_EntityManagerService', 'foo' => 'getFooService', 'foo2' => 'getFoo2Service', 'foo5' => 'getFoo5Service', @@ -40,6 +41,7 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container 'manager2' => 'getManager2Service', 'manager3' => 'getManager3Service', 'monolog.logger' => 'getMonolog_LoggerService', + 'monolog_inline.logger' => 'getMonologInline_LoggerService', 'pA' => 'getPAService', 'root' => 'getRootService', 'subscriber' => 'getSubscriberService', @@ -72,6 +74,9 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container 'connection4' => true, 'dispatcher' => true, 'dispatcher2' => true, + 'doctrine.config' => true, + 'doctrine.entity_listener_resolver' => true, + 'doctrine.listener' => true, 'foo4' => true, 'foobar' => true, 'foobar2' => true, @@ -85,8 +90,12 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container 'mailer.transport' => true, 'mailer.transport_factory' => true, 'mailer.transport_factory.amazon' => true, + 'mailer_inline.mailer' => true, + 'mailer_inline.transport_factory' => true, + 'mailer_inline.transport_factory.amazon' => true, 'manager4' => true, 'monolog.logger_2' => true, + 'monolog_inline.logger_2' => true, 'multiuse1' => true, 'pB' => true, 'pC' => true, @@ -185,6 +194,22 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container return $instance; } + /** + * Gets the public 'doctrine.entity_manager' shared service. + * + * @return \stdClass + */ + protected function getDoctrine_EntityManagerService() + { + $a = new \stdClass(); + $a->resolver = new \stdClass(new RewindableGenerator(function () { + yield 0 => ($this->privates['doctrine.listener'] ?? $this->getDoctrine_ListenerService()); + }, 1)); + $a->flag = 'ok'; + + return $this->services['doctrine.entity_manager'] = \FactoryChecker::create($a); + } + /** * Gets the public 'foo' shared service. * @@ -378,6 +403,20 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container return $instance; } + /** + * Gets the public 'monolog_inline.logger' shared service. + * + * @return \stdClass + */ + protected function getMonologInline_LoggerService() + { + $this->services['monolog_inline.logger'] = $instance = new \stdClass(); + + $instance->handler = ($this->privates['mailer_inline.mailer'] ?? $this->getMailerInline_MailerService()); + + return $instance; + } + /** * Gets the public 'pA' shared service. * @@ -448,6 +487,16 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container return $this->privates['bar6'] = new \stdClass($a); } + /** + * Gets the private 'doctrine.listener' shared service. + * + * @return \stdClass + */ + protected function getDoctrine_ListenerService() + { + return $this->privates['doctrine.listener'] = new \stdClass(($this->services['doctrine.entity_manager'] ?? $this->getDoctrine_EntityManagerService())); + } + /** * Gets the private 'level5' shared service. * @@ -473,7 +522,8 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container { return $this->privates['mailer.transport'] = (new \FactoryCircular(new RewindableGenerator(function () { yield 0 => ($this->privates['mailer.transport_factory.amazon'] ?? $this->getMailer_TransportFactory_AmazonService()); - }, 1)))->create(); + yield 1 => $this->getMailerInline_TransportFactory_AmazonService(); + }, 2)))->create(); } /** @@ -492,6 +542,31 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container return $instance; } + /** + * Gets the private 'mailer_inline.mailer' shared service. + * + * @return \stdClass + */ + protected function getMailerInline_MailerService() + { + return $this->privates['mailer_inline.mailer'] = new \stdClass((new \FactoryCircular(new RewindableGenerator(function () { + return new \EmptyIterator(); + }, 0)))->create()); + } + + /** + * Gets the private 'mailer_inline.transport_factory.amazon' shared service. + * + * @return \stdClass + */ + protected function getMailerInline_TransportFactory_AmazonService() + { + $a = new \stdClass(); + $a->handler = ($this->privates['mailer_inline.mailer'] ?? $this->getMailerInline_MailerService()); + + return new \stdClass($a); + } + /** * Gets the private 'manager4' shared service. * diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php index ddc1f59a26..86efcab216 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php @@ -33,6 +33,9 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container 'connection4' => 'getConnection4Service', 'dispatcher' => 'getDispatcherService', 'dispatcher2' => 'getDispatcher2Service', + 'doctrine.entity_listener_resolver' => 'getDoctrine_EntityListenerResolverService', + 'doctrine.entity_manager' => 'getDoctrine_EntityManagerService', + 'doctrine.listener' => 'getDoctrine_ListenerService', 'foo' => 'getFooService', 'foo2' => 'getFoo2Service', 'foo4' => 'getFoo4Service', @@ -48,11 +51,15 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container 'mailer.transport' => 'getMailer_TransportService', 'mailer.transport_factory' => 'getMailer_TransportFactoryService', 'mailer.transport_factory.amazon' => 'getMailer_TransportFactory_AmazonService', + 'mailer_inline.transport_factory' => 'getMailerInline_TransportFactoryService', + 'mailer_inline.transport_factory.amazon' => 'getMailerInline_TransportFactory_AmazonService', 'manager' => 'getManagerService', 'manager2' => 'getManager2Service', 'manager3' => 'getManager3Service', 'monolog.logger' => 'getMonolog_LoggerService', 'monolog.logger_2' => 'getMonolog_Logger2Service', + 'monolog_inline.logger' => 'getMonologInline_LoggerService', + 'monolog_inline.logger_2' => 'getMonologInline_Logger2Service', 'pA' => 'getPAService', 'pB' => 'getPBService', 'pC' => 'getPCService', @@ -83,12 +90,14 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container 'bar6' => true, 'config' => true, 'config2' => true, + 'doctrine.config' => true, 'level2' => true, 'level3' => true, 'level4' => true, 'level5' => true, 'level6' => true, 'logger2' => true, + 'mailer_inline.mailer' => true, 'manager4' => true, 'multiuse1' => true, 'subscriber2' => true, @@ -260,6 +269,42 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container return $instance; } + /** + * Gets the public 'doctrine.entity_listener_resolver' shared service. + * + * @return \stdClass + */ + protected function getDoctrine_EntityListenerResolverService() + { + return $this->services['doctrine.entity_listener_resolver'] = new \stdClass(new RewindableGenerator(function () { + yield 0 => ($this->services['doctrine.listener'] ?? $this->getDoctrine_ListenerService()); + }, 1)); + } + + /** + * Gets the public 'doctrine.entity_manager' shared service. + * + * @return \stdClass + */ + protected function getDoctrine_EntityManagerService() + { + $a = new \stdClass(); + $a->resolver = ($this->services['doctrine.entity_listener_resolver'] ?? $this->getDoctrine_EntityListenerResolverService()); + $a->flag = 'ok'; + + return $this->services['doctrine.entity_manager'] = \FactoryChecker::create($a); + } + + /** + * Gets the public 'doctrine.listener' shared service. + * + * @return \stdClass + */ + protected function getDoctrine_ListenerService() + { + return $this->services['doctrine.listener'] = new \stdClass(($this->services['doctrine.entity_manager'] ?? $this->getDoctrine_EntityManagerService())); + } + /** * Gets the public 'foo' shared service. * @@ -449,13 +494,7 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container */ protected function getMailer_TransportService() { - $a = ($this->services['mailer.transport_factory'] ?? $this->getMailer_TransportFactoryService()); - - if (isset($this->services['mailer.transport'])) { - return $this->services['mailer.transport']; - } - - return $this->services['mailer.transport'] = $a->create(); + return $this->services['mailer.transport'] = ($this->services['mailer.transport_factory'] ?? $this->getMailer_TransportFactoryService())->create(); } /** @@ -467,7 +506,8 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container { return $this->services['mailer.transport_factory'] = new \FactoryCircular(new RewindableGenerator(function () { yield 0 => ($this->services['mailer.transport_factory.amazon'] ?? $this->getMailer_TransportFactory_AmazonService()); - }, 1)); + yield 1 => ($this->services['mailer_inline.transport_factory.amazon'] ?? $this->getMailerInline_TransportFactory_AmazonService()); + }, 2)); } /** @@ -477,13 +517,29 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container */ protected function getMailer_TransportFactory_AmazonService() { - $a = ($this->services['monolog.logger_2'] ?? $this->getMonolog_Logger2Service()); + return $this->services['mailer.transport_factory.amazon'] = new \stdClass(($this->services['monolog.logger_2'] ?? $this->getMonolog_Logger2Service())); + } - if (isset($this->services['mailer.transport_factory.amazon'])) { - return $this->services['mailer.transport_factory.amazon']; - } + /** + * Gets the public 'mailer_inline.transport_factory' shared service. + * + * @return \FactoryCircular + */ + protected function getMailerInline_TransportFactoryService() + { + return $this->services['mailer_inline.transport_factory'] = new \FactoryCircular(new RewindableGenerator(function () { + return new \EmptyIterator(); + }, 0)); + } - return $this->services['mailer.transport_factory.amazon'] = new \stdClass($a); + /** + * Gets the public 'mailer_inline.transport_factory.amazon' shared service. + * + * @return \stdClass + */ + protected function getMailerInline_TransportFactory_AmazonService() + { + return $this->services['mailer_inline.transport_factory.amazon'] = new \stdClass(($this->services['monolog_inline.logger_2'] ?? $this->getMonologInline_Logger2Service())); } /** @@ -562,6 +618,34 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container return $instance; } + /** + * Gets the public 'monolog_inline.logger' shared service. + * + * @return \stdClass + */ + protected function getMonologInline_LoggerService() + { + $this->services['monolog_inline.logger'] = $instance = new \stdClass(); + + $instance->handler = ($this->privates['mailer_inline.mailer'] ?? $this->getMailerInline_MailerService()); + + return $instance; + } + + /** + * Gets the public 'monolog_inline.logger_2' shared service. + * + * @return \stdClass + */ + protected function getMonologInline_Logger2Service() + { + $this->services['monolog_inline.logger_2'] = $instance = new \stdClass(); + + $instance->handler = ($this->privates['mailer_inline.mailer'] ?? $this->getMailerInline_MailerService()); + + return $instance; + } + /** * Gets the public 'pA' shared service. * @@ -691,6 +775,16 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container return $instance; } + /** + * Gets the private 'mailer_inline.mailer' shared service. + * + * @return \stdClass + */ + protected function getMailerInline_MailerService() + { + return $this->privates['mailer_inline.mailer'] = new \stdClass(($this->services['mailer_inline.transport_factory'] ?? $this->getMailerInline_TransportFactoryService())->create()); + } + /** * Gets the private 'manager4' shared service. *