[Routing] fixed RouteCompiler for adjacent and nested placeholders

This commit is contained in:
Tobias Schultze 2012-05-08 10:14:10 +02:00
parent 885d473366
commit 005a9a3c5b
4 changed files with 104 additions and 26 deletions

View File

@ -15,6 +15,7 @@ namespace Symfony\Component\Routing;
* RouteCompiler compiles Route instances to CompiledRoute instances.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Tobias Schultze <http://tobion.de>
*/
class RouteCompiler implements RouteCompilerInterface
{
@ -30,45 +31,50 @@ class RouteCompiler implements RouteCompilerInterface
public function compile(Route $route)
{
$pattern = $route->getPattern();
$len = strlen($pattern);
$tokens = array();
$variables = array();
$matches = array();
$pos = 0;
preg_match_all('#.\{(\w+)\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
$lastSeparator = '';
// Match all variables enclosed in "{}" and iterate over them. But we only want to match the innermost variable
// in case of nested "{}", e.g. {foo{bar}}. This in ensured because \w does not match "{" or "}" itself.
preg_match_all('#\{\w+\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
foreach ($matches as $match) {
if ($text = substr($pattern, $pos, $match[0][1] - $pos)) {
$tokens[] = array('text', $text);
}
$varName = substr($match[0][0], 1, -1);
// get all static text preceding the current variable
$precedingText = substr($pattern, $pos, $match[0][1] - $pos);
$pos = $match[0][1] + strlen($match[0][0]);
$var = $match[1][0];
$precedingChar = strlen($precedingText) > 0 ? substr($precedingText, -1) : '';
if (null !== $req = $route->getRequirement($var)) {
$regexp = $req;
} else {
// Use the character preceding the variable as a separator
$separators = array($match[0][0][0]);
if ($pos !== $len) {
// Use the character following the variable as the separator when available
$separators[] = $pattern[$pos];
}
$regexp = sprintf('[^%s]+', preg_quote(implode('', array_unique($separators)), self::REGEX_DELIMITER));
if (is_numeric($varName)) {
throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $varName, $pattern));
}
if (in_array($varName, $variables)) {
throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $pattern, $varName));
}
$tokens[] = array('variable', $match[0][0][0], $regexp, $var);
if (is_numeric($var)) {
throw new \DomainException(sprintf('Variable name "%s" cannot be numeric in route pattern "%s". Please use a different name.', $var, $route->getPattern()));
if (strlen($precedingText) > 1) {
$tokens[] = array('text', substr($precedingText, 0, -1));
}
if (in_array($var, $variables)) {
throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $route->getPattern(), $var));
// use the character preceding the variable as a separator
// save it for later use as default separator for variables that follow directly without having a preceding char e.g. "/{x}{y}"
if ('' !== $precedingChar) {
$lastSeparator = $precedingChar;
}
$variables[] = $var;
$regexp = $route->getRequirement($varName);
if (null === $regexp) {
// use the character following the variable (ignoring other placeholders) as a separator when it's not the same as the preceding separator
$nextSeparator = $this->findNextSeparator(substr($pattern, $pos));
$regexp = sprintf('[^%s]+', preg_quote($lastSeparator !== $nextSeparator ? $lastSeparator.$nextSeparator : $lastSeparator, self::REGEX_DELIMITER));
}
$tokens[] = array('variable', $precedingChar, $regexp, $varName);
$variables[] = $varName;
}
if ($pos < $len) {
if ($pos < strlen($pattern)) {
$tokens[] = array('text', substr($pattern, $pos));
}
@ -97,6 +103,25 @@ class RouteCompiler implements RouteCompilerInterface
);
}
/**
* Returns the next static character in the Route pattern that will serve as a separator.
*
* @param string $pattern The route pattern
*
* @return string The next static character (or empty string when none available)
*/
private function findNextSeparator($pattern)
{
if ('' == $pattern) {
// return empty string if pattern is empty or false (false which can be returned by substr)
return '';
}
// first remove all placeholders from the pattern so we can find the next real static character
$pattern = preg_replace('#\{\w+\}#', '', $pattern);
return isset($pattern[0]) ? $pattern[0] : '';
}
/**
* Computes the regexp used to match a specific token. It can be static text or a subpattern.
*

View File

@ -255,6 +255,19 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
$this->assertSame('/app.php/a./.a/a../..a/...', $this->getGenerator($routes)->generate('test'));
}
public function testAdjacentVariables()
{
$routes = $this->getRoutes('test', new Route('/{x}{y}{z}.{_format}', array('z' => 'default-z', '_format' => 'html'), array('y' => '\d+')));
$generator = $this->getGenerator($routes);
$this->assertSame('/app.php/foo123', $generator->generate('test', array('x' => 'foo', 'y' => '123')));
$this->assertSame('/app.php/foo123bar.xml', $generator->generate('test', array('x' => 'foo', 'y' => '123', 'z' => 'bar', '_format' => 'xml')));
// The default requirement for 'x' should not allow the separator '.' in this case because it would otherwise match everything
// and following optional variables like _format could never match.
$this->setExpectedException('Symfony\Component\Routing\Exception\InvalidParameterException');
$generator->generate('test', array('x' => 'do.t', 'y' => '123', 'z' => 'bar', '_format' => 'xml'));
}
protected function getGenerator(RouteCollection $routes, array $parameters = array(), $logger = null)
{
$context = new RequestContext('/app.php');

View File

@ -232,6 +232,26 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(array('foo' => 'text1-text2-text3', 'bar' => 'text4', '_route' => 'test'), $matcher->match('/text1-text2-text3-text4-'));
}
public function testAdjacentVariables()
{
$coll = new RouteCollection();
$coll->add('test', new Route('/{w}{x}{y}{z}.{_format}', array('z' => 'default-z', '_format' => 'html'), array('y' => 'y|Y')));
$matcher = new UrlMatcher($coll, new RequestContext());
// 'w' eagerly matches as much as possible and the other variables match the remaining chars.
// This also shows that the variables w-z must all exclude the separating char (the dot '.' in this case) by default requirement.
// Otherwise they would also comsume '.xml' and _format would never match as it's an optional variable.
$this->assertEquals(array('w' => 'wwwww', 'x' => 'x', 'y' => 'Y', 'z' => 'Z','_format' => 'xml', '_route' => 'test'), $matcher->match('/wwwwwxYZ.xml'));
// As 'y' has custom requirement and can only be of value 'y|Y', it will leave 'ZZZ' to variable z.
// So with carefully chosen requirements adjacent variables, can be useful.
$this->assertEquals(array('w' => 'wwwww', 'x' => 'x', 'y' => 'y', 'z' => 'ZZZ','_format' => 'html', '_route' => 'test'), $matcher->match('/wwwwwxyZZZ'));
// z and _format are optional.
$this->assertEquals(array('w' => 'wwwww', 'x' => 'x', 'y' => 'y', 'z' => 'default-z','_format' => 'html', '_route' => 'test'), $matcher->match('/wwwwwxy'));
$this->setExpectedException('Symfony\Component\Routing\Exception\ResourceNotFoundException');
$matcher->match('/wxy.html');
}
/**
* @expectedException Symfony\Component\Routing\Exception\ResourceNotFoundException
*/

View File

@ -120,6 +120,26 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
array('text', '/foo'),
)),
array(
'Route with nested placeholders',
array('/{static{var}static}'),
'/{stati', '#^/\{static(?<var>[^cs]+)static\}$#s', array('var'), array(
array('text', 'static}'),
array('variable', 'c', '[^cs]+', 'var'),
array('text', '/{stati'),
)),
array(
'Route without separator between variables',
array('/{w}{x}{y}{z}.{_format}', array('z' => 'default-z', '_format' => 'html'), array('y' => '(y|Y)')),
'', '#^/(?<w>[^/\.]+)(?<x>[^/\.]+)(?<y>(y|Y))(?:(?<z>[^/\.]+)(?:\.(?<_format>[^\.]+))?)?$#s', array('w', 'x', 'y', 'z', '_format'), array(
array('variable', '.', '[^\.]+', '_format'),
array('variable', '', '[^/\.]+', 'z'),
array('variable', '', '(y|Y)', 'y'),
array('variable', '', '[^/\.]+', 'x'),
array('variable', '/', '[^/\.]+', 'w'),
)),
array(
'Route with a format',
array('/foo/{bar}.{_format}'),