merged branch vicb/router/kp (PR #4745)

Commits
-------

0555913 [FrameworkBundle] Allow using kernel parameters in routes

Discussion
----------

[FrameworkBundle] Allow using kernel parameters in routes

Kernel parameters can now be used at any position in patterns, defaults and requirements.

Relates to: #3316, #3276

**Differences from 3316:**

- The substitution is now done in the `Router`,
- 3316 uses `$container->getParameterBag()` which is not part of the `ContainerInterface`. The way it been solved in this PR is that some code have been duplicated inside the `Router`, see `resolveString()`.

**BC break:**

Before this PR, nonexistent parameters would have be silently ignored (ie `%idontexist%` would not have been replaced). After this PR, they will throw an exception. However you can escape the value (ie `%%idontexist%%` will be accepted and replaced by `%idontexist%`).

_This behavior is not mandatory and can be reverted if needed. However this keeps the router more consistent with the DI_.

Any feedback ? @helmer @Koc

---------------------------------------------------------------------------

by Seldaek at 2012-07-04T12:40:08Z

👍 for consistency.

---------------------------------------------------------------------------

by helmer at 2012-07-04T13:07:11Z

+1 a much better solution to the problem than mine, closing #3316

---------------------------------------------------------------------------

by Tobion at 2012-07-04T13:21:59Z

How about escaping kernel params with `\%idontexist%` as I suggested it for route params in 4563?

---------------------------------------------------------------------------

by stof at 2012-07-04T13:28:55Z

@Tobion this would not be consistent with the way DI parameters are escaped elsewhere

---------------------------------------------------------------------------

by Koc at 2012-07-04T14:24:25Z

Looks good for me, thanx.
This commit is contained in:
Fabien Potencier 2012-07-04 18:48:59 +02:00
commit d4bd50a340
3 changed files with 184 additions and 65 deletions

View File

@ -38,3 +38,5 @@ CHANGELOG
create the class cache.
* [BC BREAK] TemplateNameParser::parseFromFilename() has been moved to a dedicated
parser: TemplateFilenameParser::parse().
* [BC BREAK] Kernel parameters are replaced by their value whereever they appear
in Route patterns, requirements and defaults. Use '%%' as the escaped value for '%'.

View File

@ -16,6 +16,8 @@ use Symfony\Component\Routing\RequestContext;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface;
use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
/**
* This Router only creates the Loader only when the cache is empty.
@ -72,9 +74,12 @@ class Router extends BaseRouter implements WarmableInterface
}
/**
* Replaces placeholders with service container parameter values in route defaults and requirements.
* Replaces placeholders with service container parameter values in:
* - the route defaults,
* - the route requirements,
* - the route pattern.
*
* @param $collection
* @param RouteCollection $collection
*/
private function resolveParameters(RouteCollection $collection)
{
@ -83,27 +88,60 @@ class Router extends BaseRouter implements WarmableInterface
$this->resolveParameters($route);
} else {
foreach ($route->getDefaults() as $name => $value) {
if (!$value || '%' !== $value[0] || '%' !== substr($value, -1)) {
continue;
}
$key = substr($value, 1, -1);
if ($this->container->hasParameter($key)) {
$route->setDefault($name, $this->container->getParameter($key));
}
$route->setDefault($name, $this->resolveString($value));
}
foreach ($route->getRequirements() as $name => $value) {
if (!$value || '%' !== $value[0] || '%' !== substr($value, -1)) {
continue;
}
$key = substr($value, 1, -1);
if ($this->container->hasParameter($key)) {
$route->setRequirement($name, $this->container->getParameter($key));
}
$route->setRequirement($name, $this->resolveString($value));
}
$route->setPattern($this->resolveString($route->getPattern()));
}
}
}
/**
* Replaces placeholders with the service container parameters in the given string.
*
* @param string $value The source string which might contain %placeholders%
*
* @return string A string where the placeholders have been replaced.
*
* @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
*/
private function resolveString($value)
{
$container = $this->container;
$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);
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)
);
}, $value);
return str_replace('%%', '%', $escapedValue);
}
}

View File

@ -17,72 +17,151 @@ use Symfony\Component\Routing\RouteCollection;
class RoutingTest extends \PHPUnit_Framework_TestCase
{
public function testPlaceholders()
public function testDefaultsPlaceholders()
{
$routes = new RouteCollection();
$routes->add('foo', new Route('/foo', array(
'foo' => '%foo%',
'bar' => '%bar%',
'foobar' => 'foobar',
'foo1' => '%foo',
'foo2' => 'foo%',
'foo3' => 'f%o%o',
), array(
'foo' => '%foo%',
'bar' => '%bar%',
'foobar' => 'foobar',
'foo1' => '%foo',
'foo2' => 'foo%',
'foo3' => 'f%o%o',
)));
$routes->add('foo', new Route(
'/foo',
array(
'foo' => 'before_%parameter.foo%',
'bar' => '%parameter.bar%_after',
'baz' => '%%unescaped%%',
),
array(
)
));
$sc = $this->getServiceContainer($routes);
$sc->expects($this->at(1))->method('hasParameter')->will($this->returnValue(false));
$sc->expects($this->at(2))->method('hasParameter')->will($this->returnValue(true));
$sc->expects($this->at(3))->method('getParameter')->will($this->returnValue('bar'));
$sc->expects($this->at(4))->method('hasParameter')->will($this->returnValue(false));
$sc->expects($this->at(5))->method('hasParameter')->will($this->returnValue(true));
$sc->expects($this->at(6))->method('getParameter')->will($this->returnValue('bar'));
$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'));
$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');
$this->assertEquals('%foo%', $route->getDefault('foo'));
$this->assertEquals('bar', $route->getDefault('bar'));
$this->assertEquals('foobar', $route->getDefault('foobar'));
$this->assertEquals('%foo', $route->getDefault('foo1'));
$this->assertEquals('foo%', $route->getDefault('foo2'));
$this->assertEquals('f%o%o', $route->getDefault('foo3'));
$this->assertEquals(
array(
'foo' => 'before_foo',
'bar' => 'bar_after',
'baz' => '%unescaped%',
),
$route->getDefaults()
);
}
$this->assertEquals('%foo%', $route->getRequirement('foo'));
$this->assertEquals('bar', $route->getRequirement('bar'));
$this->assertEquals('foobar', $route->getRequirement('foobar'));
$this->assertEquals('%foo', $route->getRequirement('foo1'));
$this->assertEquals('foo%', $route->getRequirement('foo2'));
$this->assertEquals('f%o%o', $route->getRequirement('foo3'));
public function testRequirementsPlaceholders()
{
$routes = new RouteCollection();
$routes->add('foo', new Route(
'/foo',
array(
),
array(
'foo' => 'before_%parameter.foo%',
'bar' => '%parameter.bar%_after',
'baz' => '%%unescaped%%',
)
));
$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'));
$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');
$this->assertEquals(
array(
'foo' => 'before_foo',
'bar' => 'bar_after',
'baz' => '%unescaped%',
),
$route->getRequirements()
);
}
public function testPatternPlaceholders()
{
$routes = new RouteCollection();
$routes->add('foo', new Route('/before/%parameter.foo%/after/%%unescaped%%'));
$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'));
$router = new Router($sc, 'foo');
$route = $router->getRouteCollection()->get('foo');
$this->assertEquals(
'/before/foo/after/%unescaped%',
$route->getPattern()
);
}
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException
* @expectedExceptionMessage You have requested a non-existent parameter "nope".
*/
public function testExceptionOnNonExistentParameter()
{
$routes = new RouteCollection();
$routes->add('foo', new Route('/%nope%'));
$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%".
*/
public function testExceptionOnNonStringParameter()
{
$routes = new RouteCollection();
$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()));
$router = new Router($sc, 'foo');
$router->getRouteCollection()->get('foo');
}
private function getServiceContainer(RouteCollection $routes)
{
$sc = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
$sc
->expects($this->once())
->method('get')
->will($this->returnValue($this->getLoader($routes)))
;
return $sc;
}
private function getLoader(RouteCollection $routes)
{
$loader = $this->getMock('Symfony\Component\Config\Loader\LoaderInterface');
$loader
->expects($this->any())
->method('load')
->will($this->returnValue($routes))
;
return $loader;
$sc = $this->getMock('Symfony\\Component\\DependencyInjection\\ContainerInterface');
$sc
->expects($this->once())
->method('get')
->will($this->returnValue($loader))
;
return $sc;
}
}