[DI] Fix circular reference when using setters

symfony · Nov 28, 2017 · ed3c919 · ed3c919
1 parent a19d1e5
commit ed3c919
Show file tree

Hide file tree

Showing 12 changed files with 236 additions and 189 deletions.
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php b/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php
@@ -159,6 +159,8 @@ public function compile(ContainerBuilder $container)
             }
 
             throw $e;
+        } finally {
+            $this->getServiceReferenceGraph()->clear();
         }
     }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraph.php b/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraph.php
@@ -75,6 +75,9 @@ public function getNodes()
      */
     public function clear()
     {
+        foreach ($this->nodes as $node) {
+            $node->clear();
+        }
         $this->nodes = array();
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraphNode.php b/src/Symfony/Component/DependencyInjection/Compiler/ServiceReferenceGraphNode.php
@@ -107,4 +107,12 @@ public function getValue()
     {
         return $this->value;
     }
+
+    /**
+     * Clears all edges.
+     */
+    public function clear()
+    {
+        $this->inEdges = $this->outEdges = array();
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@@ -1249,24 +1249,24 @@ public function testUninitializedReference()
         $this->assertEquals(array('foo1' => new \stdClass(), 'foo3' => new \stdClass()), iterator_to_array($bar->iter));
     }
 
-    public function testAlmostCircularPrivate()
+    /**
+     * @dataProvider provideAlmostCircular
+     */
+    public function testAlmostCircular($visibility)
     {
-        $public = false;
         $container = include __DIR__.'/Fixtures/containers/container_almost_circular.php';
 
         $foo = $container->get('foo');
-
         $this->assertSame($foo, $foo->bar->foobar->foo);
+
+        $foo2 = $container->get('foo2');
+        $this->assertSame($foo2, $foo2->bar->foobar->foo);
     }
 
-    public function testAlmostCircularPublic()
+    public function provideAlmostCircular()
     {
-        $public = true;
-        $container = include __DIR__.'/Fixtures/containers/container_almost_circular.php';
-
-        $foo = $container->get('foo');
-
-        $this->assertSame($foo, $foo->bar->foobar->foo);
+        yield array('public');
+        yield array('private');
     }
 
     public function testRegisterForAutoconfiguration()

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -302,21 +302,6 @@ public function testOverrideServiceWhenUsingADumpedContainer()
         $this->assertSame($decorator, $container->get('decorator_service'), '->set() overrides an already defined service');
     }
 
-    /**
-     * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
-     */
-    public function testCircularReference()
-    {
-        $container = new ContainerBuilder();
-        $container->register('foo', 'stdClass')->addArgument(new Reference('bar'))->setPublic(true);
-        $container->register('bar', 'stdClass')->setPublic(false)->addMethodCall('setA', array(new Reference('baz')));
-        $container->register('baz', 'stdClass')->addMethodCall('setA', array(new Reference('foo')))->setPublic(true);
-        $container->compile();
-
-        $dumper = new PhpDumper($container);
-        $dumper->dump();
-    }
-
     public function testDumpAutowireData()
     {
         $container = include self::$fixturesPath.'/containers/container24.php';
@@ -774,38 +759,33 @@ public function testUninitializedReference()
         $this->assertEquals(array('foo1' => new \stdClass(), 'foo3' => new \stdClass()), iterator_to_array($bar->iter));
     }
 
-    public function testAlmostCircularPrivate()
+    /**
+     * @dataProvider provideAlmostCircular
+     */
+    public function testAlmostCircular($visibility)
     {
-        $public = false;
         $container = include self::$fixturesPath.'/containers/container_almost_circular.php';
         $container->compile();
         $dumper = new PhpDumper($container);
 
-        $this->assertStringEqualsFile(self::$fixturesPath.'/php/container_almost_circular_private.php', $dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Almost_Circular_Private')));
+        $container = 'Symfony_DI_PhpDumper_Test_Almost_Circular_'.ucfirst($visibility);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_almost_circular_'.$visibility.'.php', $dumper->dump(array('class' => $container)));
 
-        require self::$fixturesPath.'/php/container_almost_circular_private.php';
+        require self::$fixturesPath.'/php/services_almost_circular_'.$visibility.'.php';
 
-        $container = new \Symfony_DI_PhpDumper_Test_Almost_Circular_Private();
-        $foo = $container->get('foo');
+        $container = new $container();
 
+        $foo = $container->get('foo');
         $this->assertSame($foo, $foo->bar->foobar->foo);
+
+        $foo2 = $container->get('foo2');
+        $this->assertSame($foo2, $foo2->bar->foobar->foo);
     }
 
-    public function testAlmostCircularPublic()
+    public function provideAlmostCircular()
     {
-        $public = true;
-        $container = include self::$fixturesPath.'/containers/container_almost_circular.php';
-        $container->compile();
-        $dumper = new PhpDumper($container);
-
-        $this->assertStringEqualsFile(self::$fixturesPath.'/php/container_almost_circular_public.php', $dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Almost_Circular_Public')));
-
-        require self::$fixturesPath.'/php/container_almost_circular_public.php';
-
-        $container = new \Symfony_DI_PhpDumper_Test_Almost_Circular_Public();
-        $foo = $container->get('foo');
-
-        $this->assertSame($foo, $foo->bar->foobar->foo);
+        yield array('public');
+        yield array('private');
     }
 
     public function testHotPathOptimizations()
@@ -815,12 +795,12 @@ public function testHotPathOptimizations()
         $container->compile();
         $dumper = new PhpDumper($container);
 
-        $dump = $dumper->dump(array('hot_path_tag' => 'container.hot_path', 'inline_class_loader_parameter' => 'inline_requires', 'file' => self::$fixturesPath.'/php/container_inline_requires.php'));
+        $dump = $dumper->dump(array('hot_path_tag' => 'container.hot_path', 'inline_class_loader_parameter' => 'inline_requires', 'file' => self::$fixturesPath.'/php/services_inline_requires.php'));
         if ('\\' === DIRECTORY_SEPARATOR) {
             $dump = str_replace("'\\\\includes\\\\HotPath\\\\", "'/includes/HotPath/", $dump);
         }
 
-        $this->assertStringEqualsFile(self::$fixturesPath.'/php/container_inline_requires.php', $dump);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_inline_requires.php', $dump);
     }
 
     public function testDumpHandlesLiteralClassWithRootNamespace()

diff --git a/...ony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php b/...ony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php
@@ -5,8 +5,11 @@
 use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
 use Symfony\Component\DependencyInjection\Reference;
 
+$public = 'public' === $visibility;
 $container = new ContainerBuilder();
 
+// same visibility for deps
+
 $container->register('foo', FooCircular::class)->setPublic(true)
    ->addArgument(new Reference('bar'));
 
@@ -16,4 +19,21 @@
 $container->register('foobar', FoobarCircular::class)->setPublic($public)
     ->addArgument(new Reference('foo'));
 
+// mixed visibility for deps
+
+$container->register('foo2', FooCircular::class)->setPublic(true)
+   ->addArgument(new Reference('bar2'));
+
+$container->register('bar2', BarCircular::class)->setPublic(!$public)
+    ->addMethodCall('addFoobar', array(new Reference('foobar2')));
+
+$container->register('foobar2', FoobarCircular::class)->setPublic($public)
+    ->addArgument(new Reference('foo2'));
+
+// simple inline setter
+
+$def = new Definition(FoobarCircular::class);
+$container->register('bar3', BarCircular::class)->setPublic(true)
+    ->addMethodCall('addFoobar', array($def, $def));
+
 return $container;
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt
@@ -33,11 +33,11 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
 // This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
 // Returns the public 'configured_service' shared service.
 
+$this->services['configured_service'] = $instance = new \stdClass();
+
 $a = new \ConfClass();
 $a->setFoo(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->load(__DIR__.'/getBazService.php')) && false ?: '_'});
 
-$this->services['configured_service'] = $instance = new \stdClass();
-
 $a->configureStdClass($instance);
 
 return $instance;
@@ -153,10 +153,10 @@ use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
 // This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
 // Returns the public 'foo_with_inline' shared service.
 
-$a = new \Bar();
-
 $this->services['foo_with_inline'] = $instance = new \Foo();
 
+$a = new \Bar();
+
 $a->pub = 'pub';
 $a->setBaz(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->load(__DIR__.'/getBazService.php')) && false ?: '_'});
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
@@ -129,11 +129,11 @@ protected function getBazService()
      */
     protected function getConfiguredServiceService()
     {
+        $this->services['configured_service'] = $instance = new \stdClass();
+
         $a = new \ConfClass();
         $a->setFoo(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->getBazService()) && false ?: '_'});
 
-        $this->services['configured_service'] = $instance = new \stdClass();
-
         $a->configureStdClass($instance);
 
         return $instance;
@@ -259,10 +259,10 @@ protected function getFooBarService()
      */
     protected function getFooWithInlineService()
     {
-        $a = new \Bar();
-
         $this->services['foo_with_inline'] = $instance = new \Foo();
 
+        $a = new \Bar();
+
         $a->pub = 'pub';
         $a->setBaz(${($_ = isset($this->services['baz']) ? $this->services['baz'] : $this->getBazService()) && false ?: '_'});
 

diff --git a/...php/container_almost_circular_private.php → .../php/services_almost_circular_private.php b/...php/container_almost_circular_private.php → .../php/services_almost_circular_private.php
@@ -23,7 +23,10 @@ public function __construct()
     {
         $this->services = array();
         $this->methodMap = array(
+            'bar2' => 'getBar2Service',
+            'bar3' => 'getBar3Service',
             'foo' => 'getFooService',
+            'foo2' => 'getFoo2Service',
         );
 
         $this->aliases = array();
@@ -36,6 +39,7 @@ public function getRemovedIds()
             'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
             'bar' => true,
             'foobar' => true,
+            'foobar2' => true,
         );
     }
 
@@ -56,6 +60,36 @@ public function isFrozen()
         return true;
     }
 
+    /**
+     * Gets the public 'bar2' shared service.
+     *
+     * @return \BarCircular
+     */
+    protected function getBar2Service()
+    {
+        $this->services['bar2'] = $instance = new \BarCircular();
+
+        $instance->addFoobar(new \FoobarCircular(${($_ = isset($this->services['foo2']) ? $this->services['foo2'] : $this->getFoo2Service()) && false ?: '_'}));
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'bar3' shared service.
+     *
+     * @return \BarCircular
+     */
+    protected function getBar3Service()
+    {
+        $this->services['bar3'] = $instance = new \BarCircular();
+
+        $a = new \FoobarCircular();
+
+        $instance->addFoobar($a, $a);
+
+        return $instance;
+    }
+
     /**
      * Gets the public 'foo' shared service.
      *
@@ -72,4 +106,20 @@ protected function getFooService()
 
         return $instance;
     }
+
+    /**
+     * Gets the public 'foo2' shared service.
+     *
+     * @return \FooCircular
+     */
+    protected function getFoo2Service()
+    {
+        $a = ${($_ = isset($this->services['bar2']) ? $this->services['bar2'] : $this->getBar2Service()) && false ?: '_'};
+
+        if (isset($this->services['foo2'])) {
+            return $this->services['foo2'];
+        }
+
+        return $this->services['foo2'] = new \FooCircular($a);
+    }
 }