From bab16f6b1bfdf8baa11987aab4b59eaf01d97cb1 Mon Sep 17 00:00:00 2001 From: Wesley Lancel Date: Wed, 4 Jan 2017 21:54:50 +0100 Subject: [PATCH 1/2] [TwigBundle] Fix bug where namespaced paths don't take parent bundles in account --- .../DependencyInjection/TwigExtension.php | 40 ++++++++++++++++++- .../Resources/views/layout.html.twig | 1 + .../Resources/views/layout.html.twig | 1 + .../DependencyInjection/TwigExtensionTest.php | 35 +++++++++++++++- 4 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildTwigBundle/Resources/views/layout.html.twig create mode 100644 src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildTwigBundle/Resources/views/layout.html.twig diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php index e9f55f758da21..f9dd42fff214c 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php @@ -97,6 +97,10 @@ public function load(array $configs, ContainerBuilder $container) if (is_dir($dir = $bundle['path'].'/Resources/views')) { $this->addTwigPath($twigFilesystemLoaderDefinition, $dir, $name); } + + if (null !== $bundle['parent']) { + $this->prependParentPaths($twigFilesystemLoaderDefinition, $container, $name, $bundle['parent']); + } } if (is_dir($dir = $container->getParameter('kernel.root_dir').'/Resources/views')) { @@ -141,11 +145,43 @@ public function load(array $configs, ContainerBuilder $container) private function addTwigPath($twigFilesystemLoaderDefinition, $dir, $bundle) { - $name = $bundle; + $name = $this->normalizeBundleName($bundle); + $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($dir, $name)); + } + + private function prependTwigPath($twigFilesystemLoaderDefinition, $dir, $bundle) + { + $name = $this->normalizeBundleName($bundle); + $twigFilesystemLoaderDefinition->addMethodCall('prependPath', array($dir, $name)); + } + + private function prependParentPaths($twigFilesystemLoaderDefinition, ContainerBuilder $container, $name, $parentName) + { + $bundleMetadata = $container->getParameter('kernel.bundles_metadata'); + $bundle = $bundleMetadata[$name]; + + if (is_dir($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views')) { + $this->prependTwigPath($twigFilesystemLoaderDefinition, $dir, $parentName); + } + + if (is_dir($dir = $bundle['path'].'/Resources/views')) { + $this->prependTwigPath($twigFilesystemLoaderDefinition, $dir, $parentName); + } + + $parentBundle = $bundleMetadata[$parentName]; + + if (null !== $parentBundle['parent']) { + $this->prependParentPaths($twigFilesystemLoaderDefinition, $container, $name, $parentBundle['parent']); + } + } + + private function normalizeBundleName($name) + { if ('Bundle' === substr($name, -6)) { $name = substr($name, 0, -6); } - $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($dir, $name)); + + return $name; } /** diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildTwigBundle/Resources/views/layout.html.twig b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildTwigBundle/Resources/views/layout.html.twig new file mode 100644 index 0000000000000..bb07ecfe55a36 --- /dev/null +++ b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildTwigBundle/Resources/views/layout.html.twig @@ -0,0 +1 @@ +This is a layout diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildTwigBundle/Resources/views/layout.html.twig b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildTwigBundle/Resources/views/layout.html.twig new file mode 100644 index 0000000000000..bb07ecfe55a36 --- /dev/null +++ b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildTwigBundle/Resources/views/layout.html.twig @@ -0,0 +1 @@ +This is a layout diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php index ab4d649428a58..730870658c299 100644 --- a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php +++ b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php @@ -189,9 +189,12 @@ public function testTwigLoaderPaths($format) $def = $container->getDefinition('twig.loader.filesystem'); $paths = array(); + $prependPaths = array(); foreach ($def->getMethodCalls() as $call) { if ('addPath' === $call[0] && false === strpos($call[1][0], 'Form')) { $paths[] = $call[1]; + } elseif ('prependPath' === $call[0]) { + $prependPaths[] = $call[1]; } } @@ -203,8 +206,16 @@ public function testTwigLoaderPaths($format) array('namespaced_path3', 'namespace3'), array(__DIR__.'/Fixtures/Resources/TwigBundle/views', 'Twig'), array(realpath(__DIR__.'/../..').'/Resources/views', 'Twig'), + array(__DIR__.'/Fixtures/Bundle/ChildTwigBundle/Resources/views', 'ChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'ChildChildTwig'), array(__DIR__.'/Fixtures/Resources/views'), ), $paths); + + $this->assertEquals(array( + array(__DIR__.'/Fixtures/Bundle/ChildTwigBundle/Resources/views', 'Twig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'ChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'Twig'), + ), $prependPaths); } public function getFormats() @@ -254,8 +265,28 @@ private function createContainer() 'kernel.root_dir' => __DIR__.'/Fixtures', 'kernel.charset' => 'UTF-8', 'kernel.debug' => false, - 'kernel.bundles' => array('TwigBundle' => 'Symfony\\Bundle\\TwigBundle\\TwigBundle'), - 'kernel.bundles_metadata' => array('TwigBundle' => array('namespace' => 'Symfony\\Bundle\\TwigBundle', 'parent' => null, 'path' => realpath(__DIR__.'/../..'))), + 'kernel.bundles' => array( + 'TwigBundle' => 'Symfony\\Bundle\\TwigBundle\\TwigBundle', + 'ChildTwigBundle' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildTwigBundle\\ChildTwigBundle', + 'ChildChildTwigBundle' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildTwigBundle\\ChildChildTwigBundle', + ), + 'kernel.bundles_metadata' => array( + 'TwigBundle' => array( + 'namespace' => 'Symfony\\Bundle\\TwigBundle', + 'parent' => null, + 'path' => realpath(__DIR__.'/../..'), + ), + 'ChildTwigBundle' => array( + 'namespace' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildTwigBundle\\ChildTwigBundle', + 'parent' => 'TwigBundle', + 'path' => __DIR__.'/Fixtures/Bundle/ChildTwigBundle', + ), + 'ChildChildTwigBundle' => array( + 'namespace' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildTwigBundle\\ChildChildTwigBundle', + 'parent' => 'ChildTwigBundle', + 'path' => __DIR__.'/Fixtures/Bundle/ChildChildTwigBundle', + ), + ), ))); return $container; From 4e4702cb83fb2d22bdbd96565d658790bde28ef0 Mon Sep 17 00:00:00 2001 From: Wesley Lancel Date: Fri, 6 Jan 2017 22:59:53 +0100 Subject: [PATCH 2/2] Build all paths first, then add to loader --- .../DependencyInjection/TwigExtension.php | 87 ++++++++++++------- .../Resources/views/layout.html.twig | 1 + .../Resources/views/layout.html.twig | 1 + .../DependencyInjection/TwigExtensionTest.php | 33 +++++-- 4 files changed, 82 insertions(+), 40 deletions(-) create mode 100644 src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views/layout.html.twig create mode 100644 src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views/layout.html.twig diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php index f9dd42fff214c..c4637a45dba7d 100644 --- a/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php +++ b/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php @@ -88,18 +88,19 @@ public function load(array $configs, ContainerBuilder $container) } } - // register bundles as Twig namespaces - foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) { - if (is_dir($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views')) { - $this->addTwigPath($twigFilesystemLoaderDefinition, $dir, $name); - } + $bundleHierarchy = $this->getBundleHierarchy($container); - if (is_dir($dir = $bundle['path'].'/Resources/views')) { - $this->addTwigPath($twigFilesystemLoaderDefinition, $dir, $name); + foreach ($bundleHierarchy as $name => $bundle) { + $namespace = $this->normalizeBundleName($name); + + foreach ($bundle['children'] as $child) { + foreach ($bundleHierarchy[$child]['paths'] as $path) { + $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($path, $namespace)); + } } - if (null !== $bundle['parent']) { - $this->prependParentPaths($twigFilesystemLoaderDefinition, $container, $name, $bundle['parent']); + foreach ($bundle['paths'] as $path) { + $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($path, $namespace)); } } @@ -143,36 +144,60 @@ public function load(array $configs, ContainerBuilder $container) )); } - private function addTwigPath($twigFilesystemLoaderDefinition, $dir, $bundle) + private function getBundleHierarchy(ContainerBuilder $container) { - $name = $this->normalizeBundleName($bundle); - $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($dir, $name)); - } + $bundleHierarchy = array(); - private function prependTwigPath($twigFilesystemLoaderDefinition, $dir, $bundle) - { - $name = $this->normalizeBundleName($bundle); - $twigFilesystemLoaderDefinition->addMethodCall('prependPath', array($dir, $name)); - } + foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) { + if (!array_key_exists($name, $bundleHierarchy)) { + $bundleHierarchy[$name] = array( + 'paths' => array(), + 'parents' => array(), + 'children' => array(), + ); + } - private function prependParentPaths($twigFilesystemLoaderDefinition, ContainerBuilder $container, $name, $parentName) - { - $bundleMetadata = $container->getParameter('kernel.bundles_metadata'); - $bundle = $bundleMetadata[$name]; + if (is_dir($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views')) { + $bundleHierarchy[$name]['paths'][] = $dir; + } - if (is_dir($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views')) { - $this->prependTwigPath($twigFilesystemLoaderDefinition, $dir, $parentName); - } + if (is_dir($dir = $bundle['path'].'/Resources/views')) { + $bundleHierarchy[$name]['paths'][] = $dir; + } - if (is_dir($dir = $bundle['path'].'/Resources/views')) { - $this->prependTwigPath($twigFilesystemLoaderDefinition, $dir, $parentName); - } + if (null === $bundle['parent']) { + continue; + } + + $bundleHierarchy[$name]['parents'][] = $bundle['parent']; + + if (!array_key_exists($bundle['parent'], $bundleHierarchy)) { + $bundleHierarchy[$bundle['parent']] = array( + 'paths' => array(), + 'parents' => array(), + 'children' => array(), + ); + } + + $bundleHierarchy[$bundle['parent']]['children'] = array_merge($bundleHierarchy[$name]['children'], array($name), $bundleHierarchy[$bundle['parent']]['children']); - $parentBundle = $bundleMetadata[$parentName]; + foreach ($bundleHierarchy[$bundle['parent']]['parents'] as $parent) { + $bundleHierarchy[$name]['parents'][] = $parent; + $bundleHierarchy[$parent]['children'] = array_merge($bundleHierarchy[$name]['children'], array($name), $bundleHierarchy[$parent]['children']); + } - if (null !== $parentBundle['parent']) { - $this->prependParentPaths($twigFilesystemLoaderDefinition, $container, $name, $parentBundle['parent']); + foreach ($bundleHierarchy[$name]['children'] as $child) { + $bundleHierarchy[$child]['parents'] = array_merge($bundleHierarchy[$child]['parents'], $bundleHierarchy[$name]['parents']); + } } + + return $bundleHierarchy; + } + + private function addTwigPath($twigFilesystemLoaderDefinition, $dir, $bundle) + { + $name = $this->normalizeBundleName($bundle); + $twigFilesystemLoaderDefinition->addMethodCall('addPath', array($dir, $name)); } private function normalizeBundleName($name) diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views/layout.html.twig b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views/layout.html.twig new file mode 100644 index 0000000000000..bb07ecfe55a36 --- /dev/null +++ b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views/layout.html.twig @@ -0,0 +1 @@ +This is a layout diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views/layout.html.twig b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views/layout.html.twig new file mode 100644 index 0000000000000..bb07ecfe55a36 --- /dev/null +++ b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views/layout.html.twig @@ -0,0 +1 @@ +This is a layout diff --git a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php index 730870658c299..f0f8ad048466a 100644 --- a/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php +++ b/src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php @@ -189,12 +189,9 @@ public function testTwigLoaderPaths($format) $def = $container->getDefinition('twig.loader.filesystem'); $paths = array(); - $prependPaths = array(); foreach ($def->getMethodCalls() as $call) { if ('addPath' === $call[0] && false === strpos($call[1][0], 'Form')) { $paths[] = $call[1]; - } elseif ('prependPath' === $call[0]) { - $prependPaths[] = $call[1]; } } @@ -204,18 +201,24 @@ public function testTwigLoaderPaths($format) array('namespaced_path1', 'namespace1'), array('namespaced_path2', 'namespace2'), array('namespaced_path3', 'namespace3'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views', 'ChildChildChildChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views', 'ChildChildChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views', 'ChildChildChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views', 'Twig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views', 'Twig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'Twig'), + array(__DIR__.'/Fixtures/Bundle/ChildTwigBundle/Resources/views', 'Twig'), array(__DIR__.'/Fixtures/Resources/TwigBundle/views', 'Twig'), array(realpath(__DIR__.'/../..').'/Resources/views', 'Twig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views', 'ChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views', 'ChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'ChildTwig'), array(__DIR__.'/Fixtures/Bundle/ChildTwigBundle/Resources/views', 'ChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views', 'ChildChildTwig'), + array(__DIR__.'/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views', 'ChildChildTwig'), array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'ChildChildTwig'), array(__DIR__.'/Fixtures/Resources/views'), ), $paths); - - $this->assertEquals(array( - array(__DIR__.'/Fixtures/Bundle/ChildTwigBundle/Resources/views', 'Twig'), - array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'ChildTwig'), - array(__DIR__.'/Fixtures/Bundle/ChildChildTwigBundle/Resources/views', 'Twig'), - ), $prependPaths); } public function getFormats() @@ -269,8 +272,15 @@ private function createContainer() 'TwigBundle' => 'Symfony\\Bundle\\TwigBundle\\TwigBundle', 'ChildTwigBundle' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildTwigBundle\\ChildTwigBundle', 'ChildChildTwigBundle' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildTwigBundle\\ChildChildTwigBundle', + 'ChildChildChildTwigBundle' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildChildTwigBundle\\ChildChildChildTwigBundle', + 'ChildChildChildChildTwigBundle' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildChildChildTwigBundle\\ChildChildChildChildTwigBundle', ), 'kernel.bundles_metadata' => array( + 'ChildChildChildChildTwigBundle' => array( + 'namespace' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildChildChildTwigBundle\\ChildChildChildChildTwigBundle', + 'parent' => 'ChildChildChildTwigBundle', + 'path' => __DIR__.'/Fixtures/Bundle/ChildChildChildChildTwigBundle', + ), 'TwigBundle' => array( 'namespace' => 'Symfony\\Bundle\\TwigBundle', 'parent' => null, @@ -281,6 +291,11 @@ private function createContainer() 'parent' => 'TwigBundle', 'path' => __DIR__.'/Fixtures/Bundle/ChildTwigBundle', ), + 'ChildChildChildTwigBundle' => array( + 'namespace' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildChildTwigBundle\\ChildChildChildTwigBundle', + 'parent' => 'ChildChildTwigBundle', + 'path' => __DIR__.'/Fixtures/Bundle/ChildChildChildTwigBundle', + ), 'ChildChildTwigBundle' => array( 'namespace' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildChildTwigBundle\\ChildChildTwigBundle', 'parent' => 'ChildTwigBundle',