Skip to content

Commit

Permalink
feature #29599 [Routing] Allow force-generation of trailing parameter…
Browse files Browse the repository at this point in the history
…s using eg "/exports/news.{!_format}" (zavulon)

This PR was squashed before being merged into the 4.3-dev branch (closes #29599).

Discussion
----------

[Routing] Allow force-generation of trailing parameters using eg "/exports/news.{!_format}"

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29593
| License       | MIT

When a route is defined as path: `/exports/news.{!_format}`, we should force `_format` be defined in `defaults` and the generator should generate URLs with that default when none is provided (should work with any parameter of course).

Commits
-------

9fab3d6 [Routing] Allow force-generation of trailing parameters using eg \"/exports/news.{!_format}\"
  • Loading branch information
nicolas-grekas committed Dec 24, 2018
2 parents 5f737e8 + 9fab3d6 commit 57dad66
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 6 deletions.
15 changes: 10 additions & 5 deletions src/Symfony/Component/Routing/Generator/UrlGenerator.php
Expand Up @@ -156,21 +156,26 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
$message = 'Parameter "{parameter}" for route "{route}" must match "{expected}" ("{given}" given) to generate a corresponding URL.';
foreach ($tokens as $token) {
if ('variable' === $token[0]) {
if (!$optional || !array_key_exists($token[3], $defaults) || null !== $mergedParams[$token[3]] && (string) $mergedParams[$token[3]] !== (string) $defaults[$token[3]]) {
$varName = $token[3];
if ($important = ('!' === $varName[0])) {
$varName = substr($varName, 1);
}

if (!$optional || $important || !array_key_exists($varName, $defaults) || (null !== $mergedParams[$varName] && (string) $mergedParams[$varName] !== (string) $defaults[$varName])) {
// check requirement
if (null !== $this->strictRequirements && !preg_match('#^'.$token[2].'$#'.(empty($token[4]) ? '' : 'u'), $mergedParams[$token[3]])) {
if (null !== $this->strictRequirements && !preg_match('#^'.$token[2].'$#'.(empty($token[4]) ? '' : 'u'), $mergedParams[$varName])) {
if ($this->strictRequirements) {
throw new InvalidParameterException(strtr($message, array('{parameter}' => $token[3], '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$token[3]])));
throw new InvalidParameterException(strtr($message, array('{parameter}' => $varName, '{route}' => $name, '{expected}' => $token[2], '{given}' => $mergedParams[$varName])));
}

if ($this->logger) {
$this->logger->error($message, array('parameter' => $token[3], 'route' => $name, 'expected' => $token[2], 'given' => $mergedParams[$token[3]]));
$this->logger->error($message, array('parameter' => $varName, 'route' => $name, 'expected' => $token[2], 'given' => $mergedParams[$varName]));
}

return;
}

$url = $token[1].$mergedParams[$token[3]].$url;
$url = $token[1].$mergedParams[$varName].$url;
$optional = false;
}
} else {
Expand Down
9 changes: 8 additions & 1 deletion src/Symfony/Component/Routing/RouteCompiler.php
Expand Up @@ -111,7 +111,7 @@ private static function compilePattern(Route $route, $pattern, $isHost)

// 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);
preg_match_all('#\{!?\w+\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
foreach ($matches as $match) {
$varName = substr($match[0][0], 1, -1);
// get all static text preceding the current variable
Expand Down Expand Up @@ -184,6 +184,9 @@ private static function compilePattern(Route $route, $pattern, $isHost)
}

$tokens[] = array('variable', $isSeparator ? $precedingChar : '', $regexp, $varName);
if ('!' === $varName[0]) {
$varName = substr($varName, 1);
}
$variables[] = $varName;
}

Expand Down Expand Up @@ -283,6 +286,10 @@ private static function computeRegexp(array $tokens, int $index, int $firstOptio
// Text tokens
return preg_quote($token[1], self::REGEX_DELIMITER);
} else {
if ('variable' === $token[0] && '!' === $token[3][0]) {
$token[3] = substr($token[3], 1);
}

// Variable tokens
if (0 === $index && 0 === $firstOptional) {
// When the only token is an optional variable token, the separator is required
Expand Down
21 changes: 21 additions & 0 deletions src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
Expand Up @@ -397,6 +397,27 @@ public function testDefaultRequirementOfVariable()
$this->assertSame('/app.php/index.mobile.html', $generator->generate('test', array('page' => 'index', '_format' => 'mobile.html')));
}

public function testImportantVariable()
{
$routes = $this->getRoutes('test', (new Route('/{page}.{!_format}'))->addDefaults(array('_format' => 'mobile.html')));
$generator = $this->getGenerator($routes);

$this->assertSame('/app.php/index.xml', $generator->generate('test', array('page' => 'index', '_format' => 'xml')));
$this->assertSame('/app.php/index.mobile.html', $generator->generate('test', array('page' => 'index', '_format' => 'mobile.html')));
$this->assertSame('/app.php/index.mobile.html', $generator->generate('test', array('page' => 'index')));
}

/**
* @expectedException \Symfony\Component\Routing\Exception\MissingMandatoryParametersException
*/
public function testImportantVariableWithNoDefault()
{
$routes = $this->getRoutes('test', new Route('/{page}.{!_format}'));
$generator = $this->getGenerator($routes);

$generator->generate('test', array('page' => 'index'));
}

/**
* @expectedException \Symfony\Component\Routing\Exception\InvalidParameterException
*/
Expand Down
Expand Up @@ -177,6 +177,15 @@ public function testMatchSpecialRouteName()
$this->assertEquals(array('_route' => '$péß^a|'), $matcher->match('/bar'));
}

public function testMatchImportantVariable()
{
$collection = new RouteCollection();
$collection->add('index', new Route('/index.{!_format}', array('_format' => 'xml')));

$matcher = $this->getUrlMatcher($collection);
$this->assertEquals(array('_route' => 'index', '_format' => 'xml'), $matcher->match('/index.xml'));
}

/**
* @expectedException \Symfony\Component\Routing\Exception\ResourceNotFoundException
*/
Expand Down

0 comments on commit 57dad66

Please sign in to comment.