[HttpKernel] made sure that parent bundles are registered before thei…

…r descendants
symfony · Jan 29, 2011 · 5e5b6f0 · 5e5b6f0
1 parent 996c2b0
commit 5e5b6f0
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 25 deletions.
diff --git a/src/Symfony/Component/HttpKernel/Kernel.php b/src/Symfony/Component/HttpKernel/Kernel.php
@@ -329,48 +329,61 @@ public function getLogDir()
 
     /**
      * Initialize the data structures related to the bundle management:
-     *  - the bundle property maps a bundle name to a bundle instance,
-     *  - the bundleMap property maps a bundle name to the bundle inheritance hierarchy.
+     *  - the bundle property maps a bundle name to the bundle instance,
+     *  - the bundleMap property maps a bundle name to the bundle inheritance hierarchy (most derived bundle first).
      *
      * @throws \LogicException if two bundles share a common name
-     * @throws \LogicException if a bundle tries to extend a non-existing bundle
+     * @throws \LogicException if a bundle tries to extend a non-registered bundle
      * @throws \LogicException if two bundles extend the same ancestor
      *
      */
     protected function initializeBundles()
     {
         // init bundles
         $this->bundles = array();
-        $this->bundleMap = array();
+        $topMostBundles = array();
+        $directChildren = array();
+
         foreach ($this->registerBundles() as $bundle) {
             $name = $bundle->getName();
             if (isset($this->bundles[$name])) {
                 throw new \LogicException(sprintf('Trying to register two bundles with the same name "%s"', $name));
-            } 
+            }
             $this->bundles[$name] = $bundle;
-            $this->bundleMap[$name] = array($bundle);
-        }
 
-        // inheritance
-        $extended = array();
-        foreach ($this->bundles as $name => $bundle) {
-            $parent = $bundle;
-            $first = true;
-            while ($parentName = $parent->getParent()) {
-                if (!isset($this->bundles[$parentName])) {
-                    throw new \LogicException(sprintf('Bundle "%s" extends bundle "%s", which is not registered.', $name, $parentName));
+            if ($parentName = $bundle->getParent()) {
+                if (isset($directChildren[$parentName])) {
+                    throw new \LogicException(sprintf('Bundle "%s" is directly extended by two bundles "%s" and "%s".', $parentName, $name, $directChildren[$parentName]));
                 }
+                $directChildren[$parentName] = $name;
+            } else {
+                $topMostBundles[$name] = $bundle;
+            }            
+        }
 
-                if ($first && isset($extended[$parentName])) {
-                    throw new \LogicException(sprintf('Bundle "%s" is directly extended by two bundles "%s" and "%s".', $parentName, $name, $extended[$parentName]));
-                }
+        // look for orphans
+        if (count($diff = array_diff(array_keys($directChildren), array_keys($this->bundles)))) {
+            throw new \LogicException(sprintf('Bundle "%s" extends bundle "%s", which is not registered.', $directChildren[$diff[0]], $diff[0]));
+        }
 
-                $first = false;
-                $parent = $this->bundles[$parentName];
-                $extended[$parentName] = $name;
-                array_unshift($this->bundleMap[$parentName], $bundle);
+        // inheritance
+        $this->bundleMap = array();
+        foreach ($topMostBundles as $name => $bundle) {
+            $bundleMap = array($bundle);
+            $hierarchy = array($name);
+
+            while (isset($directChildren[$name])) {
+                $name = $directChildren[$name];
+                array_unshift($bundleMap, $this->bundles[$name]);
+                $hierarchy[] = $name;
+            }
+
+            foreach ($hierarchy as $bundle) {
+                $this->bundleMap[$bundle] = $bundleMap;
+                array_pop($bundleMap);
             }
         }
+
     }
 
     protected function initializeContainer()

diff --git a/tests/Symfony/Tests/Component/HttpKernel/KernelTest.php b/tests/Symfony/Tests/Component/HttpKernel/KernelTest.php
@@ -12,6 +12,7 @@
 namespace Symfony\Tests\Component\HttpKernel;
 
 use Symfony\Component\HttpKernel\Kernel;
+use Symfony\Component\HttpKernel\Bundle\BundleInterface;
 use Symfony\Component\DependencyInjection\Loader\LoaderInterface;
 
 class KernelTest extends \PHPUnit_Framework_TestCase
@@ -167,7 +168,7 @@ public function testInitializeBundlesSupportInheritanceCascade()
         $kernel
             ->expects($this->once())
             ->method('registerBundles')
-            ->will($this->returnValue(array($parent, $grandparent, $child)))
+            ->will($this->returnValue(array($grandparent, $parent, $child)))
         ;
 
         $kernel->initializeBundles();
@@ -194,6 +195,27 @@ public function testInitializeBundlesThrowsExceptionWhenAParentDoesNotExists()
         $kernel->initializeBundles();
     }
 
+    public function testInitializeBundlesSupportsArbitaryBundleRegistrationOrder()
+    {
+        $grandparent = $this->getBundle(null, null, 'GrandParentCCundle');
+        $parent = $this->getBundle(null, 'GrandParentCCundle', 'ParentCCundle');
+        $child = $this->getBundle(null, 'ParentCCundle', 'ChildCCundle');
+
+        $kernel = $this->getKernel();
+        $kernel
+            ->expects($this->once())
+            ->method('registerBundles')
+            ->will($this->returnValue(array($parent, $grandparent, $child)))
+        ;
+
+        $kernel->initializeBundles();
+
+        $map = $kernel->getBundleMap();
+        $this->assertEquals(array($child, $parent, $grandparent), $map['GrandParentCCundle']);
+        $this->assertEquals(array($child, $parent), $map['ParentCCundle']);
+        $this->assertEquals(array($child), $map['ChildCCundle']);
+    }
+
     /**
      * @expectedException \LogicException
      */
@@ -232,7 +254,7 @@ public function testInitializeBundleThrowsExceptionWhenRegisteringTwoBundlesWith
     protected function getBundle($dir = null, $parent = null, $className = null, $bundleName = null)
     {
         $bundle = $this
-            ->getMockBuilder('Symfony\Tests\Component\HttpKernel\KernelForTest')
+            ->getMockBuilder('Symfony\Tests\Component\HttpKernel\BundleForTest')
             ->setMethods(array('getPath', 'getParent', 'getName'))
             ->disableOriginalConstructor()
         ;
@@ -241,7 +263,7 @@ protected function getBundle($dir = null, $parent = null, $className = null, $bu
             $bundle->setMockClassName($className);
         }
 
-        $bundle = $bundle->getMock();
+        $bundle = $bundle->getMockForAbstractClass();
 
         $bundle
             ->expects($this->any())
@@ -312,3 +334,8 @@ public function initializeBundles()
         parent::initializeBundles();
     }
 }
+
+abstract class BundleForTest implements BundleInterface
+{
+    // We can not extend Symfony\Component\HttpKernel\Bundle\Bundle as we want to mock getName() which is final
+}