feature #11210 [FrameworkBundle] Improving bad bundle exception in _controller (weaverryan)
This PR was merged into the 2.6-dev branch.
Discussion
----------
[FrameworkBundle] Improving bad bundle exception in _controller
...ntroller in a routeHi guys!
This improves the exception message when you use a bad bundle name in the `_controller` syntax in a routing file. Here is the before and after of the message with this mistake (real bundle is `KnpUniversityBundle`):
```yaml
some_route:
pattern: /
defaults: { _controller: "Knp2UniversityBundle:Course:index" }
```
![screen shot 2014-06-23 at 9 27 55 pm](https://cloud.githubusercontent.com/assets/121003/3367065/448e8298-fb54-11e3-92ea-9bf04510cb6d.png)
![screen shot 2014-06-23 at 9 48 14 pm](https://cloud.githubusercontent.com/assets/121003/3367063/3c79cf36-fb54-11e3-87c4-29428248ee47.png)
Notice the before and after behavior is the same `InvalidArgumentException` (just a different message).
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
Catching the plain `InvalidArgumentException` from `Kernel::getBundles()` seems a bit "loose". Should we consider creating a new exception (e.g. `BundleDoesNotExistException`) that extends `InvalidArgumentException` and throw it from inside `Kernel::getBundles`? This would allow us to catch more precisely, and it seems like it would be BC.
Suggestions and thoughts warmly welcome!
Thanks!
Commits
-------
f9b88c6
Improving the exception message when the bundle name is wrong for the controller in a route
This commit is contained in:
commit
229828da8a
@ -46,6 +46,7 @@ class ControllerNameParser
|
|||||||
*/
|
*/
|
||||||
public function parse($controller)
|
public function parse($controller)
|
||||||
{
|
{
|
||||||
|
$originalController = $controller;
|
||||||
if (3 != count($parts = explode(':', $controller))) {
|
if (3 != count($parts = explode(':', $controller))) {
|
||||||
throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller));
|
throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller));
|
||||||
}
|
}
|
||||||
@ -54,8 +55,24 @@ class ControllerNameParser
|
|||||||
$controller = str_replace('/', '\\', $controller);
|
$controller = str_replace('/', '\\', $controller);
|
||||||
$bundles = array();
|
$bundles = array();
|
||||||
|
|
||||||
|
try {
|
||||||
// this throws an exception if there is no such bundle
|
// this throws an exception if there is no such bundle
|
||||||
foreach ($this->kernel->getBundle($bundle, false) as $b) {
|
$allBundles = $this->kernel->getBundle($bundle, false);
|
||||||
|
} catch (\InvalidArgumentException $e) {
|
||||||
|
$message = sprintf(
|
||||||
|
'The "%s" (from the _controller value "%s") does not exist or is not enabled in your kernel!',
|
||||||
|
$bundle,
|
||||||
|
$originalController
|
||||||
|
);
|
||||||
|
|
||||||
|
if ($alternative = $this->findAlternative($bundle)) {
|
||||||
|
$message .= sprintf(' Did you mean "%s:%s:%s"?', $alternative, $controller, $action);
|
||||||
|
}
|
||||||
|
|
||||||
|
throw new \InvalidArgumentException($message, 0, $e);
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach ($allBundles as $b) {
|
||||||
$try = $b->getNamespace().'\\Controller\\'.$controller.'Controller';
|
$try = $b->getNamespace().'\\Controller\\'.$controller.'Controller';
|
||||||
if (class_exists($try)) {
|
if (class_exists($try)) {
|
||||||
return $try.'::'.$action.'Action';
|
return $try.'::'.$action.'Action';
|
||||||
@ -100,4 +117,33 @@ class ControllerNameParser
|
|||||||
|
|
||||||
throw new \InvalidArgumentException(sprintf('Unable to find a bundle that defines controller "%s".', $controller));
|
throw new \InvalidArgumentException(sprintf('Unable to find a bundle that defines controller "%s".', $controller));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Attempts to find a bundle that is *similar* to the given bundle name
|
||||||
|
*
|
||||||
|
* @param string $nonExistentBundleName
|
||||||
|
* @return string
|
||||||
|
*/
|
||||||
|
private function findAlternative($nonExistentBundleName)
|
||||||
|
{
|
||||||
|
$bundleNames = array_map(function ($b) {
|
||||||
|
return $b->getName();
|
||||||
|
}, $this->kernel->getBundles());
|
||||||
|
|
||||||
|
$alternative = null;
|
||||||
|
$shortest = null;
|
||||||
|
foreach ($bundleNames as $bundleName) {
|
||||||
|
// if there's a partial match, return it immediately
|
||||||
|
if (false !== strpos($bundleName, $nonExistentBundleName)) {
|
||||||
|
return $bundleName;
|
||||||
|
}
|
||||||
|
|
||||||
|
$lev = levenshtein($nonExistentBundleName, $bundleName);
|
||||||
|
if ($lev <= strlen($nonExistentBundleName) / 3 && ($alternative === null || $lev < $shortest)) {
|
||||||
|
$alternative = $bundleName;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $alternative;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -107,6 +107,36 @@ class ControllerNameParserTest extends TestCase
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException
|
||||||
|
* @dataProvider getInvalidBundleNameTests
|
||||||
|
*/
|
||||||
|
public function testInvalidBundleName($bundleName, $suggestedBundleName)
|
||||||
|
{
|
||||||
|
$parser = $this->createParser();
|
||||||
|
|
||||||
|
try {
|
||||||
|
$parser->parse($bundleName);
|
||||||
|
} catch (\Exception $e) {
|
||||||
|
$this->assertInstanceOf('\InvalidArgumentException', $e, '->parse() throws a \InvalidArgumentException if the bundle does not exist');
|
||||||
|
|
||||||
|
if (false === $suggestedBundleName) {
|
||||||
|
// make sure we don't have a suggestion
|
||||||
|
$this->assertNotContains('Did you mean', $e->getMessage());
|
||||||
|
} else {
|
||||||
|
$this->assertContains(sprintf('Did you mean "%s"', $suggestedBundleName), $e->getMessage());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getInvalidBundleNameTests()
|
||||||
|
{
|
||||||
|
return array(
|
||||||
|
array('FoodBundle:Default:index', 'FooBundle:Default:index'),
|
||||||
|
array('CrazyBundle:Default:index', false),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
private function createParser()
|
private function createParser()
|
||||||
{
|
{
|
||||||
$bundles = array(
|
$bundles = array(
|
||||||
@ -121,6 +151,10 @@ class ControllerNameParserTest extends TestCase
|
|||||||
->expects($this->any())
|
->expects($this->any())
|
||||||
->method('getBundle')
|
->method('getBundle')
|
||||||
->will($this->returnCallback(function ($bundle) use ($bundles) {
|
->will($this->returnCallback(function ($bundle) use ($bundles) {
|
||||||
|
if (!isset($bundles[$bundle])) {
|
||||||
|
throw new \InvalidArgumentException(sprintf('Invalid bundle name "%s"', $bundle));
|
||||||
|
}
|
||||||
|
|
||||||
return $bundles[$bundle];
|
return $bundles[$bundle];
|
||||||
}))
|
}))
|
||||||
;
|
;
|
||||||
|
Reference in New Issue
Block a user