Skip to content

Commit

Permalink
feature #24161 [HttpKernel] Remove bundle inheritance (fabpot)
Browse files Browse the repository at this point in the history
This PR was merged into the 4.0-dev branch.

Discussion
----------

[HttpKernel] Remove bundle inheritance

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Blocked by #24160

Commits
-------

a8a0a21 [HttpKernel] removed bundle inheritance
  • Loading branch information
fabpot committed Sep 26, 2017
2 parents d47b833 + a8a0a21 commit 92d5134
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 481 deletions.
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
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
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
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();

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

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

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
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
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

0 comments on commit 92d5134

Please sign in to comment.