Skip to content

[HttpKernel] Remove bundle inheritance #24161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2017
Merged

[HttpKernel] Remove bundle inheritance #24161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 8 additions & 23 deletions src/Symfony/Bundle/FrameworkBundle/Controller/ControllerNameParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,47 +52,32 @@ public function parse($controller)
}

$originalController = $controller;
list($bundle, $controller, $action) = $parts;
list($bundleName, $controller, $action) = $parts;
$controller = str_replace('/', '\\', $controller);
$bundles = array();

try {
// this throws an exception if there is no such bundle
$allBundles = $this->kernel->getBundle($bundle, false, true);
$bundle = $this->kernel->getBundle($bundleName);
} catch (\InvalidArgumentException $e) {
$message = sprintf(
'The "%s" (from the _controller value "%s") does not exist or is not enabled in your kernel!',
$bundle,
$bundleName,
$originalController
);

if ($alternative = $this->findAlternative($bundle)) {
if ($alternative = $this->findAlternative($bundleName)) {
$message .= sprintf(' Did you mean "%s:%s:%s"?', $alternative, $controller, $action);
}

throw new \InvalidArgumentException($message, 0, $e);
}

if (!is_array($allBundles)) {
// happens when HttpKernel is version 4+
$allBundles = array($allBundles);
$try = $bundle->getNamespace().'\\Controller\\'.$controller.'Controller';
if (class_exists($try)) {
return $try.'::'.$action.'Action';
}

foreach ($allBundles as $b) {
$try = $b->getNamespace().'\\Controller\\'.$controller.'Controller';
if (class_exists($try)) {
return $try.'::'.$action.'Action';
}

$bundles[] = $b->getName();
$msg = sprintf('The _controller value "%s:%s:%s" maps to a "%s" class, but this class was not found. Create this class or check the spelling of the class and its namespace.', $bundle, $controller, $action, $try);
}

if (count($bundles) > 1) {
$msg = sprintf('Unable to find controller "%s:%s" in bundles %s.', $bundle, $controller, implode(', ', $bundles));
}

throw new \InvalidArgumentException($msg);
throw new \InvalidArgumentException(sprintf('The _controller value "%s:%s:%s" maps to a "%s" class, but this class was not found. Create this class or check the spelling of the class and its namespace.', $bundleName, $controller, $action, $try));
}

/**
Expand Down
10 changes: 2 additions & 8 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Controller/ControllerNameParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public function testParse()

$this->assertEquals('TestBundle\FooBundle\Controller\DefaultController::indexAction', $parser->parse('FooBundle:Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
$this->assertEquals('TestBundle\FooBundle\Controller\Sub\DefaultController::indexAction', $parser->parse('FooBundle:Sub\Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
$this->assertEquals('TestBundle\Fabpot\FooBundle\Controller\DefaultController::indexAction', $parser->parse('SensioFooBundle:Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
$this->assertEquals('TestBundle\Sensio\Cms\FooBundle\Controller\DefaultController::indexAction', $parser->parse('SensioCmsFooBundle:Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
$this->assertEquals('TestBundle\FooBundle\Controller\Test\DefaultController::indexAction', $parser->parse('FooBundle:Test\\Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
$this->assertEquals('TestBundle\FooBundle\Controller\Test\DefaultController::indexAction', $parser->parse('FooBundle:Test/Default:index'), '->parse() converts a short a:b:c notation string to a class::method string');
Expand Down Expand Up @@ -138,18 +137,15 @@ public function getInvalidBundleNameTests()
{
return array(
'Alternative will be found using levenshtein' => array('FoodBundle:Default:index', 'FooBundle:Default:index'),
'Alternative will be found using partial match' => array('FabpotFooBund:Default:index', 'FabpotFooBundle:Default:index'),
'Bundle does not exist at all' => array('CrazyBundle:Default:index', false),
);
}

private function createParser()
{
$bundles = array(
'SensioFooBundle' => array($this->getBundle('TestBundle\Fabpot\FooBundle', 'FabpotFooBundle'), $this->getBundle('TestBundle\Sensio\FooBundle', 'SensioFooBundle')),
'SensioCmsFooBundle' => array($this->getBundle('TestBundle\Sensio\Cms\FooBundle', 'SensioCmsFooBundle')),
'FooBundle' => array($this->getBundle('TestBundle\FooBundle', 'FooBundle')),
'FabpotFooBundle' => array($this->getBundle('TestBundle\Fabpot\FooBundle', 'FabpotFooBundle'), $this->getBundle('TestBundle\Sensio\FooBundle', 'SensioFooBundle')),
'SensioCmsFooBundle' => $this->getBundle('TestBundle\Sensio\Cms\FooBundle', 'SensioCmsFooBundle'),
'FooBundle' => $this->getBundle('TestBundle\FooBundle', 'FooBundle'),
);

$kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\KernelInterface')->getMock();
Expand All @@ -166,11 +162,9 @@ private function createParser()
;

$bundles = array(
'SensioFooBundle' => $this->getBundle('TestBundle\Fabpot\FooBundle', 'FabpotFooBundle'),
'SensioCmsFooBundle' => $this->getBundle('TestBundle\Sensio\Cms\FooBundle', 'SensioCmsFooBundle'),
'FoooooBundle' => $this->getBundle('TestBundle\FooBundle', 'FoooooBundle'),
'FooBundle' => $this->getBundle('TestBundle\FooBundle', 'FooBundle'),
'FabpotFooBundle' => $this->getBundle('TestBundle\Fabpot\FooBundle', 'FabpotFooBundle'),
);
$kernel
->expects($this->any())
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ public function testValidationPaths()

$container = $this->createContainerFromFile('validation_annotations', array(
'kernel.bundles' => array('TestBundle' => 'Symfony\\Bundle\\FrameworkBundle\\Tests\\TestBundle'),
'kernel.bundles_metadata' => array('TestBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle\\Tests', 'parent' => null, 'path' => __DIR__.'/Fixtures/TestBundle')),
'kernel.bundles_metadata' => array('TestBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle\\Tests', 'path' => __DIR__.'/Fixtures/TestBundle')),
));

$calls = $container->getDefinition('validator.builder')->getMethodCalls();
Expand Down Expand Up @@ -641,7 +641,7 @@ public function testValidationPathsUsingCustomBundlePath()

$container = $this->createContainerFromFile('validation_annotations', array(
'kernel.bundles' => array('CustomPathBundle' => 'Symfony\\Bundle\\FrameworkBundle\\Tests\\CustomPathBundle'),
'kernel.bundles_metadata' => array('TestBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle\\Tests', 'parent' => null, 'path' => __DIR__.'/Fixtures/CustomPathBundle')),
'kernel.bundles_metadata' => array('TestBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle\\Tests', 'path' => __DIR__.'/Fixtures/CustomPathBundle')),
));

$calls = $container->getDefinition('validator.builder')->getMethodCalls();
Expand Down Expand Up @@ -848,7 +848,7 @@ public function testSerializerCacheDisabled()

public function testSerializerMapping()
{
$container = $this->createContainerFromFile('serializer_mapping', array('kernel.bundles_metadata' => array('TestBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle\\Tests', 'path' => __DIR__.'/Fixtures/TestBundle', 'parent' => null))));
$container = $this->createContainerFromFile('serializer_mapping', array('kernel.bundles_metadata' => array('TestBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle\\Tests', 'path' => __DIR__.'/Fixtures/TestBundle'))));
$configDir = __DIR__.'/Fixtures/TestBundle/Resources/config';
$expectedLoaders = array(
new Definition(AnnotationLoader::class, array(new Reference('annotation_reader'))),
Expand Down Expand Up @@ -980,7 +980,7 @@ protected function createContainer(array $data = array())
{
return new ContainerBuilder(new ParameterBag(array_merge(array(
'kernel.bundles' => array('FrameworkBundle' => 'Symfony\\Bundle\\FrameworkBundle\\FrameworkBundle'),
'kernel.bundles_metadata' => array('FrameworkBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle', 'path' => __DIR__.'/../..', 'parent' => null)),
'kernel.bundles_metadata' => array('FrameworkBundle' => array('namespace' => 'Symfony\\Bundle\\FrameworkBundle', 'path' => __DIR__.'/../..')),
'kernel.cache_dir' => __DIR__,
'kernel.project_dir' => __DIR__,
'kernel.debug' => false,
Expand Down
55 changes: 6 additions & 49 deletions src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,9 @@ public function load(array $configs, ContainerBuilder $container)
$container->getDefinition('twig.cache_warmer')->replaceArgument(2, $config['paths']);
$container->getDefinition('twig.template_iterator')->replaceArgument(2, $config['paths']);

$bundleHierarchy = $this->getBundleHierarchy($container, $config);

foreach ($bundleHierarchy as $name => $bundle) {
foreach ($this->getBundleTemplatePaths($container, $config) as $name => $paths) {
$namespace = $this->normalizeBundleName($name);

foreach ($bundle['children'] as $child) {
foreach ($bundleHierarchy[$child]['paths'] as $path) {
$twigFilesystemLoaderDefinition->addMethodCall('addPath', array($path, $namespace));
}
}

foreach ($bundle['paths'] as $path) {
foreach ($paths as $path) {
$twigFilesystemLoaderDefinition->addMethodCall('addPath', array($path, $namespace));
}
}
Expand Down Expand Up @@ -166,58 +157,24 @@ public function load(array $configs, ContainerBuilder $container)
$container->registerForAutoconfiguration(RuntimeExtensionInterface::class)->addTag('twig.runtime');
}

private function getBundleHierarchy(ContainerBuilder $container, array $config)
private function getBundleTemplatePaths(ContainerBuilder $container, array $config)
{
$bundleHierarchy = array();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe variable should be renamed in conformity with method name change?


foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) {
if (!array_key_exists($name, $bundleHierarchy)) {
$bundleHierarchy[$name] = array(
'paths' => array(),
'parents' => array(),
'children' => array(),
);
}

if (file_exists($dir = $container->getParameter('kernel.root_dir').'/Resources/'.$name.'/views')) {
$bundleHierarchy[$name]['paths'][] = $dir;
$bundleHierarchy[$name][] = $dir;
}
$container->addResource(new FileExistenceResource($dir));

if (file_exists($dir = $container->getParameterBag()->resolveValue($config['default_path']).'/bundles/'.$name)) {
$bundleHierarchy[$name]['paths'][] = $dir;
$bundleHierarchy[$name][] = $dir;
}
$container->addResource(new FileExistenceResource($dir));

if (file_exists($dir = $bundle['path'].'/Resources/views')) {
$bundleHierarchy[$name]['paths'][] = $dir;
$bundleHierarchy[$name][] = $dir;
}
$container->addResource(new FileExistenceResource($dir));

if (!isset($bundle['parent']) || 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']);

foreach ($bundleHierarchy[$bundle['parent']]['parents'] as $parent) {
$bundleHierarchy[$name]['parents'][] = $parent;
$bundleHierarchy[$parent]['children'] = array_merge($bundleHierarchy[$name]['children'], array($name), $bundleHierarchy[$parent]['children']);
}

foreach ($bundleHierarchy[$name]['children'] as $child) {
$bundleHierarchy[$child]['parents'] = array_merge($bundleHierarchy[$child]['parents'], $bundleHierarchy[$name]['parents']);
}
}

return $bundleHierarchy;
Expand Down
1 change: 0 additions & 1 deletion ...Injection/Fixtures/Bundle/ChildChildChildChildTwigBundle/Resources/views/layout.html.twig
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ...dencyInjection/Fixtures/Bundle/ChildChildChildTwigBundle/Resources/views/layout.html.twig
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ...DependencyInjection/Fixtures/Bundle/ChildChildTwigBundle/Resources/views/layout.html.twig
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion ...ests/DependencyInjection/Fixtures/Bundle/ChildTwigBundle/Resources/views/layout.html.twig
View file
Open in desktop

This file was deleted.

39 changes: 0 additions & 39 deletions src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,23 +188,9 @@ 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(__DIR__.'/Fixtures/templates/bundles/TwigBundle', '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'),
array(__DIR__.'/Fixtures/templates'),
), $paths);
Expand Down Expand Up @@ -284,37 +270,12 @@ private function createContainer()
'kernel.debug' => false,
'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',
'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,
'path' => realpath(__DIR__.'/../..'),
),
'ChildTwigBundle' => array(
'namespace' => 'Symfony\\Bundle\\TwigBundle\\Tests\\DependencyInjection\\Fixtures\\Bundle\\ChildTwigBundle\\ChildTwigBundle',
'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',
'path' => __DIR__.'/Fixtures/Bundle/ChildChildTwigBundle',
),
),
)));

Expand Down
9 changes: 0 additions & 9 deletions src/Symfony/Component/HttpKernel/Bundle/Bundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,6 @@ public function getPath()
return $this->path;
}

/**
* Returns the bundle parent name.
*
* @return string|null The Bundle parent name it overrides or null if no parent
*/
public function getParent()
{
}

/**
* Returns the bundle name (the class short name).
*
Expand Down
13 changes: 0 additions & 13 deletions src/Symfony/Component/HttpKernel/Bundle/BundleInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,6 @@ public function build(ContainerBuilder $container);
*/
public function getContainerExtension();

/**
* Returns the bundle name that this bundle overrides.
*
* Despite its name, this method does not imply any parent/child relationship
* between the bundles, just a way to extend and override an existing
* bundle.
*
* @return string The Bundle name it overrides or null if no parent
*
* @deprecated This method is deprecated as of 3.4 and will be removed in 4.0.
*/
public function getParent();

/**
* Returns the bundle name (the class short name).
*
Expand Down
Loading