Fix circular referene with Factory and LazyIterator

jderusse · jderusse · commit 88abc793d805 · 2020-11-04T15:21:44.000+01:00
diff --git a/ContainerBuilder.php b/ContainerBuilder.php
@@ -1220,20 +1220,20 @@ private function doResolveServices($value, array &$inlineServices = [], bool $is
                 return $this->resolveServices($reference);
             };
         } elseif ($value instanceof IteratorArgument) {
-            $value = new RewindableGenerator(function () use ($value) {
+            $value = new RewindableGenerator(function () use ($value, &$inlineServices) {
                 foreach ($value->getValues() as $k => $v) {
                     foreach (self::getServiceConditionals($v) as $s) {
                         if (!$this->has($s)) {
                             continue 2;
                         }
                     }
                     foreach (self::getInitializedConditionals($v) as $s) {
-                        if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE)) {
+                        if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE, $inlineServices)) {
                             continue 2;
                         }
                     }
 
-                    yield $k => $this->resolveServices($v);
+                    yield $k => $this->doResolveServices($v, $inlineServices);
                 }
             }, function () use ($value): int {
                 $count = 0;
diff --git a/Dumper/PhpDumper.php b/Dumper/PhpDumper.php
@@ -451,7 +451,7 @@ private function collectCircularReferences(string $sourceId, array $edges, array
         foreach ($edges as $edge) {
             $node = $edge->getDestNode();
             $id = $node->getId();
-            if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isLazy() || $edge->isWeak()) {
+            if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isWeak()) {
                 continue;
             }
 
diff --git a/Tests/ContainerBuilderTest.php b/Tests/ContainerBuilderTest.php
@@ -1372,6 +1372,10 @@ public function testUninitializedReference()
     public function testAlmostCircular($visibility)
     {
         $container = include __DIR__.'/Fixtures/containers/container_almost_circular.php';
+        $container->compile();
+
+        $logger = $container->get('monolog.logger');
+        $this->assertEquals(new \stdClass(), $logger->handler);
 
         $foo = $container->get('foo');
         $this->assertSame($foo, $foo->bar->foobar->foo);
diff --git a/Tests/Dumper/PhpDumperTest.php b/Tests/Dumper/PhpDumperTest.php
@@ -1054,6 +1054,9 @@ public function testAlmostCircular($visibility)
 
         $container = new $container();
 
+        $logger = $container->get('monolog.logger');
+        $this->assertEquals(new \stdClass(), $logger->handler);
+
         $foo = $container->get('foo');
         $this->assertSame($foo, $foo->bar->foobar->foo);
 
diff --git a/Tests/Fixtures/containers/container_almost_circular.php b/Tests/Fixtures/containers/container_almost_circular.php
@@ -1,5 +1,6 @@
 <?php
 
+use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
@@ -8,6 +9,24 @@
 $public = 'public' === $visibility;
 $container = new ContainerBuilder();
 
+// monolog-like + handler that require monolog
+
+$container->register('monolog.logger', 'stdClass')->setPublic(true)
+    ->setProperty('handler', new Reference('mailer.transport'));
+
+$container->register('mailer.transport', 'stdClass')->setPublic($public)
+    ->setFactory([new Reference('mailer.transport_factory'), 'create']);
+
+$container->register('mailer.transport_factory', FactoryCircular::class)->setPublic($public)
+    ->addArgument(new TaggedIteratorArgument('mailer.transport'));
+
+$container->register('mailer.transport_factory.amazon', 'stdClass')->setPublic($public)
+    ->addArgument(new Reference('monolog.logger_2'))
+    ->addTag('mailer.transport');
+
+$container->register('monolog.logger_2', 'stdClass')->setPublic($public)
+    ->setProperty('handler', new Reference('mailer.transport'));
+
 // same visibility for deps
 
 $container->register('foo', FooCircular::class)->setPublic(true)
diff --git a/Tests/Fixtures/includes/classes.php b/Tests/Fixtures/includes/classes.php
@@ -111,6 +111,23 @@ public function __construct($lazyValues, $lazyEmptyValues)
     }
 }
 
+class FactoryCircular
+{
+    public $services;
+
+    public function __construct($services)
+    {
+        $this->services = $services;
+    }
+
+    public function create()
+    {
+        foreach ($this->services as $service) {
+            return $service;
+        }
+    }
+}
+
 class FoobarCircular
 {
     public function __construct(FooCircular $foo)
diff --git a/Tests/Fixtures/php/services_almost_circular_private.php b/Tests/Fixtures/php/services_almost_circular_private.php
@@ -39,6 +39,7 @@ public function __construct()
             'manager' => 'getManagerService',
             'manager2' => 'getManager2Service',
             'manager3' => 'getManager3Service',
+            'monolog.logger' => 'getMonolog_LoggerService',
             'root' => 'getRootService',
             'subscriber' => 'getSubscriberService',
         ];
@@ -80,7 +81,11 @@ public function getRemovedIds(): array
             'level5' => true,
             'level6' => true,
             'logger2' => true,
+            'mailer.transport' => true,
+            'mailer.transport_factory' => true,
+            'mailer.transport_factory.amazon' => true,
             'manager4' => true,
+            'monolog.logger_2' => true,
             'multiuse1' => true,
             'subscriber2' => true,
         ];
@@ -355,6 +360,20 @@ protected function getManager3Service($lazyLoad = true)
         return $this->services['manager3'] = new \stdClass($b);
     }
 
+    /**
+     * Gets the public 'monolog.logger' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getMonolog_LoggerService()
+    {
+        $this->services['monolog.logger'] = $instance = new \stdClass();
+
+        $instance->handler = ($this->privates['mailer.transport'] ?? $this->getMailer_TransportService());
+
+        return $instance;
+    }
+
     /**
      * Gets the public 'root' shared service.
      *
@@ -419,6 +438,34 @@ protected function getLevel5Service()
         return $instance;
     }
 
+    /**
+     * Gets the private 'mailer.transport' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getMailer_TransportService()
+    {
+        return $this->privates['mailer.transport'] = (new \FactoryCircular(new RewindableGenerator(function () {
+            yield 0 => ($this->privates['mailer.transport_factory.amazon'] ?? $this->getMailer_TransportFactory_AmazonService());
+        }, 1)))->create();
+    }
+
+    /**
+     * Gets the private 'mailer.transport_factory.amazon' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getMailer_TransportFactory_AmazonService()
+    {
+        $a = new \stdClass();
+
+        $this->privates['mailer.transport_factory.amazon'] = $instance = new \stdClass($a);
+
+        $a->handler = ($this->privates['mailer.transport'] ?? $this->getMailer_TransportService());
+
+        return $instance;
+    }
+
     /**
      * Gets the private 'manager4' shared service.
      *
diff --git a/Tests/Fixtures/php/services_almost_circular_public.php b/Tests/Fixtures/php/services_almost_circular_public.php
@@ -45,9 +45,14 @@ public function __construct()
             'listener3' => 'getListener3Service',
             'listener4' => 'getListener4Service',
             'logger' => 'getLoggerService',
+            'mailer.transport' => 'getMailer_TransportService',
+            'mailer.transport_factory' => 'getMailer_TransportFactoryService',
+            'mailer.transport_factory.amazon' => 'getMailer_TransportFactory_AmazonService',
             'manager' => 'getManagerService',
             'manager2' => 'getManager2Service',
             'manager3' => 'getManager3Service',
+            'monolog.logger' => 'getMonolog_LoggerService',
+            'monolog.logger_2' => 'getMonolog_Logger2Service',
             'root' => 'getRootService',
             'subscriber' => 'getSubscriberService',
         ];
@@ -433,6 +438,50 @@ protected function getLoggerService()
         return $instance;
     }
 
+    /**
+     * Gets the public 'mailer.transport' shared service.
+     *
+     * @return \stdClass
+     */
+    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();
+    }
+
+    /**
+     * Gets the public 'mailer.transport_factory' shared service.
+     *
+     * @return \FactoryCircular
+     */
+    protected function getMailer_TransportFactoryService()
+    {
+        return $this->services['mailer.transport_factory'] = new \FactoryCircular(new RewindableGenerator(function () {
+            yield 0 => ($this->services['mailer.transport_factory.amazon'] ?? $this->getMailer_TransportFactory_AmazonService());
+        }, 1));
+    }
+
+    /**
+     * Gets the public 'mailer.transport_factory.amazon' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getMailer_TransportFactory_AmazonService()
+    {
+        $a = ($this->services['monolog.logger_2'] ?? $this->getMonolog_Logger2Service());
+
+        if (isset($this->services['mailer.transport_factory.amazon'])) {
+            return $this->services['mailer.transport_factory.amazon'];
+        }
+
+        return $this->services['mailer.transport_factory.amazon'] = new \stdClass($a);
+    }
+
     /**
      * Gets the public 'manager' shared service.
      *
@@ -481,6 +530,34 @@ protected function getManager3Service($lazyLoad = true)
         return $this->services['manager3'] = new \stdClass($a);
     }
 
+    /**
+     * Gets the public 'monolog.logger' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getMonolog_LoggerService()
+    {
+        $this->services['monolog.logger'] = $instance = new \stdClass();
+
+        $instance->handler = ($this->services['mailer.transport'] ?? $this->getMailer_TransportService());
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'monolog.logger_2' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getMonolog_Logger2Service()
+    {
+        $this->services['monolog.logger_2'] = $instance = new \stdClass();
+
+        $instance->handler = ($this->services['mailer.transport'] ?? $this->getMailer_TransportService());
+
+        return $instance;
+    }
+
     /**
      * Gets the public 'root' shared service.
      *

Original file line number	Diff line number	Diff line change
`@@ -1220,20 +1220,20 @@ private function doResolveServices($value, array &$inlineServices = [], bool $is`
`1220`	`1220`	`return $this->resolveServices($reference);`
`1221`	`1221`	`};`
`1222`	`1222`	`} elseif ($value instanceof IteratorArgument) {`
`1223`		`- $value = new RewindableGenerator(function () use ($value) {`
	`1223`	`+ $value = new RewindableGenerator(function () use ($value, &$inlineServices) {`
`1224`	`1224`	`foreach ($value->getValues() as $k => $v) {`
`1225`	`1225`	`foreach (self::getServiceConditionals($v) as $s) {`
`1226`	`1226`	`if (!$this->has($s)) {`
`1227`	`1227`	`continue 2;`
`1228`	`1228`	`}`
`1229`	`1229`	`}`
`1230`	`1230`	`foreach (self::getInitializedConditionals($v) as $s) {`
`1231`		`- if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE)) {`
	`1231`	`+ if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE, $inlineServices)) {`
`1232`	`1232`	`continue 2;`
`1233`	`1233`	`}`
`1234`	`1234`	`}`
`1235`	`1235`
`1236`		`- yield $k => $this->resolveServices($v);`
	`1236`	`+ yield $k => $this->doResolveServices($v, $inlineServices);`
`1237`	`1237`	`}`
`1238`	`1238`	`}, function () use ($value): int {`
`1239`	`1239`	`$count = 0;`
Original file line number	Diff line number	Diff line change
`@@ -451,7 +451,7 @@ private function collectCircularReferences(string $sourceId, array $edges, array`
`451`	`451`	`foreach ($edges as $edge) {`
`452`	`452`	`$node = $edge->getDestNode();`
`453`	`453`	`$id = $node->getId();`
`454`		`- if (!$node->getValue() instanceof Definition \|\| $sourceId === $id \|\| $edge->isLazy() \|\| $edge->isWeak()) {`
	`454`	`+ if (!$node->getValue() instanceof Definition \|\| $sourceId === $id \|\| $edge->isWeak()) {`
`455`	`455`	`continue;`
`456`	`456`	`}`
`457`	`457`