diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 307b5a588f..18edd160dd 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -23,7 +23,6 @@ use Symfony\Component\HttpKernel\Log\LoggerInterface; * UrlGenerator generates a URL based on a set of routes. * * @author Fabien Potencier - * @author Tobias Schultze * * @api */ @@ -133,10 +132,13 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $absolute) { $variables = array_flip($variables); - $mergedParams = array_replace($this->context->getParameters(), $defaults, $parameters); + + $originParameters = $parameters; + $parameters = array_replace($this->context->getParameters(), $parameters); + $tparams = array_replace($defaults, $parameters); // all params must be given - if ($diff = array_diff_key($variables, $mergedParams)) { + if ($diff = array_diff_key($variables, $tparams)) { throw new MissingMandatoryParametersException(sprintf('The "%s" route has some missing mandatory parameters ("%s").', $name, implode('", "', array_keys($diff)))); } @@ -144,26 +146,30 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt $optional = true; foreach ($tokens as $token) { if ('variable' === $token[0]) { - if (!$optional || !array_key_exists($token[3], $defaults) || (string) $mergedParams[$token[3]] !== (string) $defaults[$token[3]]) { - // check requirement - if (!preg_match('#^'.$token[2].'$#', $mergedParams[$token[3]])) { - $message = sprintf('Parameter "%s" for route "%s" must match "%s" ("%s" given).', $token[3], $name, $token[2], $mergedParams[$token[3]]); - if ($this->strictRequirements) { - throw new InvalidParameterException($message); - } + if (false === $optional || !array_key_exists($token[3], $defaults) || (isset($parameters[$token[3]]) && (string) $parameters[$token[3]] != (string) $defaults[$token[3]])) { + if (!$isEmpty = in_array($tparams[$token[3]], array(null, '', false), true)) { + // check requirement + if ($tparams[$token[3]] && !preg_match('#^'.$token[2].'$#', $tparams[$token[3]])) { + $message = sprintf('Parameter "%s" for route "%s" must match "%s" ("%s" given).', $token[3], $name, $token[2], $tparams[$token[3]]); + if ($this->strictRequirements) { + throw new InvalidParameterException($message); + } - if ($this->logger) { - $this->logger->err($message); - } + if ($this->logger) { + $this->logger->err($message); + } - return null; + return null; + } + } + + if (!$isEmpty || !$optional) { + $url = $token[1].$tparams[$token[3]].$url; } - $url = $token[1].$mergedParams[$token[3]].$url; $optional = false; } - } else { - // static text + } elseif ('text' === $token[0]) { $url = $token[1].$url; $optional = false; } @@ -187,7 +193,7 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt } // add a query string if needed - $extra = array_diff_key($parameters, $variables); + $extra = array_diff_key($originParameters, $variables, $defaults); if ($extra && $query = http_build_query($extra, '', '&')) { $url .= '?'.$query; } diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 236015f976..f001067a53 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -74,15 +74,12 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase $this->assertEquals('/app.php/testing', $url); } - /** - * @expectedException Symfony\Component\Routing\Exception\InvalidParameterException - */ public function testRelativeUrlWithNullParameterButNotOptional() { $routes = $this->getRoutes('test', new Route('/testing/{foo}/bar', array('foo' => null))); - // This must raise an exception because the default requirement for "foo" is "[^/]+" which is not met with these params. - // Generating path "/testing//bar" would be wrong as matching this route would fail. - $this->getGenerator($routes)->generate('test', array(), false); + $url = $this->getGenerator($routes)->generate('test', array(), false); + + $this->assertEquals('/app.php/testing//bar', $url); } public function testRelativeUrlWithOptionalZeroParameter() @@ -93,13 +90,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase $this->assertEquals('/app.php/testing/0', $url); } - public function testNotPassedOptionalParameterInBetween() - { - $routes = $this->getRoutes('test', new Route('/{slug}/{page}', array('slug' => 'index', 'page' => 0))); - $this->assertSame('/app.php/index/1', $this->getGenerator($routes)->generate('test', array('page' => 1))); - $this->assertSame('/app.php/', $this->getGenerator($routes)->generate('test')); - } - public function testRelativeUrlWithExtraParameters() { $routes = $this->getRoutes('test', new Route('/testing')); @@ -175,15 +165,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase $this->getGenerator($routes)->generate('test', array('foo' => 'bar'), true); } - /** - * @expectedException Symfony\Component\Routing\Exception\InvalidParameterException - */ - public function testGenerateForRouteWithInvalidParameter() - { - $routes = $this->getRoutes('test', new Route('/testing/{foo}', array(), array('foo' => '1|2'))); - $this->getGenerator($routes)->generate('test', array('foo' => '0'), true); - } - public function testGenerateForRouteWithInvalidOptionalParameterNonStrict() { $routes = $this->getRoutes('test', new Route('/testing/{foo}', array('foo' => '1'), array('foo' => 'd+'))); @@ -215,15 +196,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase $routes = $this->getRoutes('test', new Route('/testing/{foo}', array(), array('foo' => 'd+'))); $this->getGenerator($routes)->generate('test', array('foo' => 'bar'), true); } - - /** - * @expectedException Symfony\Component\Routing\Exception\InvalidParameterException - */ - public function testRequiredParamAndEmptyPassed() - { - $routes = $this->getRoutes('test', new Route('/{slug}', array(), array('slug' => '.+'))); - $this->getGenerator($routes)->generate('test', array('slug' => '')); - } public function testSchemeRequirementDoesNothingIfSameCurrentScheme() { @@ -257,15 +229,6 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase $this->assertEquals('/app.php/foo', $this->getGenerator($routes)->generate('test', array('default' => 'foo'))); } - public function testQueryParamSameAsDefault() - { - $routes = $this->getRoutes('test', new Route('/test', array('default' => 'value'))); - - $this->assertSame('/app.php/test?default=foo', $this->getGenerator($routes)->generate('test', array('default' => 'foo'))); - $this->assertSame('/app.php/test?default=value', $this->getGenerator($routes)->generate('test', array('default' => 'value'))); - $this->assertSame('/app.php/test', $this->getGenerator($routes)->generate('test')); - } - public function testUrlEncoding() { // This tests the encoding of reserved characters that are used for delimiting of URI components (defined in RFC 3986)