Navigation Menu

Skip to content

Commit

Permalink
[FrameworkBundle] fix routing container parameter exception message
Browse files Browse the repository at this point in the history 
also improve regex performance and fix implementation-specific written tests and typo
  • Loading branch information
@Tobion
Tobion committed Oct 31, 2013
1 parent 8513ac9 commit 719e037
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 54 deletions.
23 changes: 9 additions & 14 deletions src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
View file
Expand Up @@ -104,7 +104,7 @@ private function resolveParameters(RouteCollection $collection)
* @param mixed $value The source which might contain "%placeholders%"
*
* @return mixed The source with the placeholders replaced by the container
* parameters. Array are resolved recursively.
* parameters. Arrays are resolved recursively.
*
* @throws ParameterNotFoundException When a placeholder does not exist as a container parameter
* @throws RuntimeException When a container value is not a string or a numeric value
Expand All @@ -125,30 +125,25 @@ private function resolve($value)

$container = $this->container;

$escapedValue = preg_replace_callback('/%%|%([^%\s]+)%/', function ($match) use ($container, $value) {
$escapedValue = preg_replace_callback('/%%|%([^%\s]++)%/', function ($match) use ($container, $value) {
// skip %%
if (!isset($match[1])) {
return '%%';
}

$key = strtolower($match[1]);

if (!$container->hasParameter($key)) {
throw new ParameterNotFoundException($key);
}

$resolved = $container->getParameter($key);
$resolved = $container->getParameter($match[1]);

if (is_string($resolved) || is_numeric($resolved)) {
return (string) $resolved;
}

throw new RuntimeException(sprintf(
'A string value must be composed of strings and/or numbers,' .
'but found parameter "%s" of type %s inside string value "%s".',
$key,
gettype($resolved),
$value)
'The container parameter "%s", used in the route configuration value "%s", ' .
'must be a string or numeric, but it is of type %s.',
$match[1],
$value,
gettype($resolved)
)
);

}, $value);
Expand Down
65 changes: 25 additions & 40 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php
View file
Expand Up @@ -32,12 +32,9 @@ public function testGenerateWithServiceParam()
));

$sc = $this->getServiceContainer($routes);

$sc->expects($this->at(1))->method('hasParameter')->will($this->returnValue(true));
$sc->expects($this->at(2))->method('getParameter')->will($this->returnValue('es'));
$sc->setParameter('locale', 'es');

$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');

$this->assertSame('/en', $router->generate('foo', array('_locale' => 'en')));
$this->assertSame('/', $router->generate('foo', array('_locale' => 'es')));
Expand All @@ -52,7 +49,7 @@ public function testDefaultsPlaceholders()
array(
'foo' => 'before_%parameter.foo%',
'bar' => '%parameter.bar%_after',
'baz' => '%%unescaped%%',
'baz' => '%%escaped%%',
'boo' => array('%parameter%', '%%escaped_parameter%%', array('%bee_parameter%', 'bee')),
'bee' => array('bee', 'bee'),
),
Expand All @@ -62,16 +59,10 @@ public function testDefaultsPlaceholders()

$sc = $this->getServiceContainer($routes);

$sc->expects($this->at(1))->method('hasParameter')->will($this->returnValue(true));
$sc->expects($this->at(2))->method('getParameter')->will($this->returnValue('foo'));
$sc->expects($this->at(3))->method('hasParameter')->will($this->returnValue(true));
$sc->expects($this->at(4))->method('getParameter')->will($this->returnValue('bar'));

$sc->expects($this->at(5))->method('hasParameter')->will($this->returnValue(true));
$sc->expects($this->at(6))->method('getParameter')->will($this->returnValue('boo'));

$sc->expects($this->at(7))->method('hasParameter')->will($this->returnValue(true));
$sc->expects($this->at(8))->method('getParameter')->will($this->returnValue('foo_bee'));
$sc->setParameter('parameter.foo', 'foo');
$sc->setParameter('parameter.bar', 'bar');
$sc->setParameter('parameter', 'boo');
$sc->setParameter('bee_parameter', 'foo_bee');

$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');
Expand All @@ -80,7 +71,7 @@ public function testDefaultsPlaceholders()
array(
'foo' => 'before_foo',
'bar' => 'bar_after',
'baz' => '%unescaped%',
'baz' => '%escaped%',
'boo' => array('boo', '%escaped_parameter%', array('foo_bee', 'bee')),
'bee' => array('bee', 'bee'),
),
Expand All @@ -99,16 +90,13 @@ public function testRequirementsPlaceholders()
array(
'foo' => 'before_%parameter.foo%',
'bar' => '%parameter.bar%_after',
'baz' => '%%unescaped%%',
'baz' => '%%escaped%%',
)
));

$sc = $this->getServiceContainer($routes);

$sc->expects($this->at(1))->method('hasParameter')->with('parameter.foo')->will($this->returnValue(true));
$sc->expects($this->at(2))->method('getParameter')->with('parameter.foo')->will($this->returnValue('foo'));
$sc->expects($this->at(3))->method('hasParameter')->with('parameter.bar')->will($this->returnValue(true));
$sc->expects($this->at(4))->method('getParameter')->with('parameter.bar')->will($this->returnValue('bar'));
$sc->setParameter('parameter.foo', 'foo');
$sc->setParameter('parameter.bar', 'bar');

$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');
Expand All @@ -117,7 +105,7 @@ public function testRequirementsPlaceholders()
array(
'foo' => 'before_foo',
'bar' => 'bar_after',
'baz' => '%unescaped%',
'baz' => '%escaped%',
),
$route->getRequirements()
);
Expand All @@ -127,18 +115,16 @@ public function testPatternPlaceholders()
{
$routes = new RouteCollection();

$routes->add('foo', new Route('/before/%parameter.foo%/after/%%unescaped%%'));
$routes->add('foo', new Route('/before/%parameter.foo%/after/%%escaped%%'));

$sc = $this->getServiceContainer($routes);

$sc->expects($this->at(1))->method('hasParameter')->with('parameter.foo')->will($this->returnValue(true));
$sc->expects($this->at(2))->method('getParameter')->with('parameter.foo')->will($this->returnValue('foo'));
$sc->setParameter('parameter.foo', 'foo');

$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');

$this->assertEquals(
'/before/foo/after/%unescaped%',
'/before/foo/after/%escaped%',
$route->getPath()
);
}
Expand All @@ -148,20 +134,18 @@ public function testHostPlaceholders()
$routes = new RouteCollection();

$route = new Route('foo');
$route->setHost('/before/%parameter.foo%/after/%%unescaped%%');
$route->setHost('/before/%parameter.foo%/after/%%escaped%%');

$routes->add('foo', $route);

$sc = $this->getServiceContainer($routes);

$sc->expects($this->at(1))->method('hasParameter')->with('parameter.foo')->will($this->returnValue(true));
$sc->expects($this->at(2))->method('getParameter')->with('parameter.foo')->will($this->returnValue('foo'));
$sc->setParameter('parameter.foo', 'foo');

$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');

$this->assertEquals(
'/before/foo/after/%unescaped%',
'/before/foo/after/%escaped%',
$route->getHost()
);
}
Expand All @@ -178,15 +162,13 @@ public function testExceptionOnNonExistentParameter()

$sc = $this->getServiceContainer($routes);

$sc->expects($this->at(1))->method('hasParameter')->with('nope')->will($this->returnValue(false));

$router = new Router($sc, 'foo');
$router->getRouteCollection()->get('foo');
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage A string value must be composed of strings and/or numbers,but found parameter "object" of type object inside string value "/%object%".
* @expectedExceptionMessage The container parameter "object", used in the route configuration value "/%object%", must be a string or numeric, but it is of type object.
*/
public function testExceptionOnNonStringParameter()
{
Expand All @@ -195,9 +177,7 @@ public function testExceptionOnNonStringParameter()
$routes->add('foo', new Route('/%object%'));

$sc = $this->getServiceContainer($routes);

$sc->expects($this->at(1))->method('hasParameter')->with('object')->will($this->returnValue(true));
$sc->expects($this->at(2))->method('getParameter')->with('object')->will($this->returnValue(new \stdClass()));
$sc->setParameter('object', new \stdClass());

$router = new Router($sc, 'foo');
$router->getRouteCollection()->get('foo');
Expand Down Expand Up @@ -225,6 +205,11 @@ public function getNonStringValues()
return array(array(null), array(false), array(true), array(new \stdClass()), array(array('foo', 'bar')), array(array(array())));
}

/**
* @param RouteCollection $routes
*
* @return \Symfony\Component\DependencyInjection\Container
*/
private function getServiceContainer(RouteCollection $routes)
{
$loader = $this->getMock('Symfony\Component\Config\Loader\LoaderInterface');
Expand All @@ -235,7 +220,7 @@ private function getServiceContainer(RouteCollection $routes)
->will($this->returnValue($routes))
;

$sc = $this->getMock('Symfony\\Component\\DependencyInjection\\ContainerInterface');
$sc = $this->getMock('Symfony\\Component\\DependencyInjection\\Container', array('get'));

$sc
->expects($this->once())
Expand Down

0 comments on commit 719e037

Please sign in to comment.