[DependencyInjection] made some improvments to the container compiler

- added generic repeated pass - better optimization of services - started adding some integration tests
symfony · avalanche123 · Jan 8, 2011 · Jan 8, 2011 · Jan 9, 2011 · Jan 9, 2011
commit e85546ef7de757e6f36f29d17665886f47d59aa2
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php
@@ -21,9 +21,15 @@
  *
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  */
-class InlineServiceDefinitionsPass implements CompilerPassInterface
+class InlineServiceDefinitionsPass implements RepeatablePassInterface
 {
     protected $aliasMap;
+    protected $repeatedPass;
+
+    public function setRepeatedPass(RepeatedPass $repeatedPass)
+    {
+        $this->repeatedPass = $repeatedPass;
+    }
 
     public function process(ContainerBuilder $container)
     {

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
@@ -39,8 +39,10 @@ public function __construct()
         $this->removingPasses = array(
             new RemovePrivateAliasesPass(),
             new ReplaceAliasByActualDefinitionPass(),
-            new InlineServiceDefinitionsPass(),
-            new RemoveUnusedDefinitionsPass(),
+            new RepeatedPass(array(
+                new InlineServiceDefinitionsPass(),
+                new RemoveUnusedDefinitionsPass(),
+            )),
         );
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php
@@ -20,8 +20,15 @@
  *
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  */
-class RemoveUnusedDefinitionsPass implements CompilerPassInterface
+class RemoveUnusedDefinitionsPass implements RepeatablePassInterface
 {
+    protected $repeatedPass;
+
+    public function setRepeatedPass(RepeatedPass $repeatedPass)
+    {
+        $this->repeatedPass = $repeatedPass;
+    }
+
     public function process(ContainerBuilder $container)
     {
         $hasChanged = false;
@@ -45,7 +52,7 @@ public function process(ContainerBuilder $container)
         }
 
         if ($hasChanged) {
-            $this->process($container);
+            $this->repeatedPass->setRepeat();
         }
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RepeatablePassInterface.php b/src/Symfony/Component/DependencyInjection/Compiler/RepeatablePassInterface.php
@@ -0,0 +1,14 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Compiler;
+
+/**
+ * Interface that must be implemented by passes that are run as part of an
+ * RepeatedPass.
+ *
+ * @author Johannes M. Schmitt <schmittjoh@gmail.com>
+ */
+interface RepeatablePassInterface extends CompilerPassInterface
+{
+    function setRepeatedPass(RepeatedPass $repeatedPass);
+}
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RepeatedPass.php b/src/Symfony/Component/DependencyInjection/Compiler/RepeatedPass.php
@@ -0,0 +1,46 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * A pass that might be run repeatedly.
+ *
+ * @author Johannes M. Schmitt <schmittjoh@gmail.com>
+ */
+class RepeatedPass implements CompilerPassInterface
+{
+    protected $repeat;
+    protected $passes;
+
+    public function __construct(array $passes)
+    {
+        foreach ($passes as $pass) {
+            if (!$pass instanceof RepeatablePassInterface) {
+                throw new \InvalidArgumentException('$passes must be an array of RepeatablePassInterface.');
+            }
+
+            $pass->setRepeatedPass($this);
+        }
+
+        $this->passes = $passes;
+    }
+
+    public function process(ContainerBuilder $container)
+    {
+        $this->repeat = false;
+        foreach ($this->passes as $pass) {
+            $pass->process($container);
+        }
+
+        if ($this->repeat) {
+            $this->process($container);
+        }
+    }
+
+    public function setRepeat()
+    {
+        $this->repeat = true;
+    }
+}
diff --git a/tests/Symfony/Tests/Component/DependencyInjection/Compiler/IntegrationTest.php b/tests/Symfony/Tests/Component/DependencyInjection/Compiler/IntegrationTest.php
@@ -0,0 +1,75 @@
+<?php
+
+namespace Symfony\Tests\Component\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Alias;
+
+use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * This class tests the integration of the different compiler passes
+ */
+class IntegrationTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * This tests that the following dependencies are correctly processed:
+     *
+     * A is public, B/C are private
+     * A -> C
+     * B -> C
+     */
+    public function testProcessRemovesAndInlinesRecursively()
+    {
+        $container = new ContainerBuilder();
+
+        $a = $container
+            ->register('a')
+            ->addArgument(new Reference('c'))
+        ;
+
+        $b = $container
+            ->register('b')
+            ->addArgument(new Reference('c'))
+            ->setPublic(false)
+        ;
+
+        $c = $container
+            ->register('c')
+            ->setPublic(false)
+        ;
+
+        $container->freeze();
+
+        $this->assertTrue($container->hasDefinition('a'));
+        $arguments = $a->getArguments();
+        $this->assertSame($c, $arguments[0]);
+        $this->assertFalse($container->hasDefinition('b'));
+        $this->assertFalse($container->hasDefinition('c'));
+    }
+
+    public function testProcessInlinesReferencesToAliases()
+    {
+        $container = new ContainerBuilder();
+
+        $a = $container
+            ->register('a')
+            ->addArgument(new Reference('b'))
+        ;
+
+        $container->setAlias('b', new Alias('c', false));
+
+        $c = $container
+            ->register('c')
+            ->setPublic(false)
+        ;
+
+        $container->freeze();
+
+        $this->assertTrue($container->hasDefinition('a'));
+        $arguments = $a->getArguments();
+        $this->assertSame($c, $arguments[0]);
+        $this->assertFalse($container->hasAlias('b'));
+        $this->assertFalse($container->hasDefinition('c'));
+    }
+}
diff --git a/.../Symfony/Tests/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPassTest.php b/.../Symfony/Tests/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPassTest.php
@@ -2,6 +2,8 @@
 
 namespace Symfony\Tests\Component\DependencyInjection\Compiler;
 
+use Symfony\Component\DependencyInjection\Compiler\RepeatedPass;
+
 use Symfony\Component\DependencyInjection\Compiler\RemoveUnusedDefinitionsPass;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
@@ -72,6 +74,7 @@ public function testProcessWorksWithInlinedDefinitions()
     protected function process(ContainerBuilder $container)
     {
         $pass = new RemoveUnusedDefinitionsPass();
+        $repeatedPass = new RepeatedPass(array($pass));
         $pass->process($container);
     }
 }