From 787baabf054f1f9aac60fd5f13e905c9d9b632a5 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Sat, 1 Sep 2012 04:19:44 +0200 Subject: [PATCH 1/2] [Routing] allow URLs that include the default param This can be achieved by explicitly passing the parameter. Previously it just ignored the parameter that equals the default value. You can still omit optional variables by not passing it at all or passing null as param value. --- .../Component/Routing/Generator/UrlGenerator.php | 2 +- .../Routing/Tests/Generator/UrlGeneratorTest.php | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index c1c1aefe8e7c8..baee0ad37d811 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -144,7 +144,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa $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]]) { + if (!$optional || !array_key_exists($token[3], $defaults) || isset($parameters[$token[3]]) || $this->context->hasParameter($token[3])) { // check requirement if (null !== $this->strictRequirements && !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]]); diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 224744f7dbaf6..92ed9dc465d17 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,6 +277,17 @@ public function testWithAnIntegerAsADefaultValue() $this->assertEquals('/app.php/foo', $this->getGenerator($routes)->generate('test', array('default' => 'foo'))); } + public function testUrlWithDefaultIncluded() + { + $routes = $this->getRoutes('test', new Route('/index.{_format}', array('_format' => 'html'), array('_format' => '.*'))); + $generator = $this->getGenerator($routes); + + $this->assertSame('/app.php/index.html', $generator->generate('test', array('_format' => 'html')), 'parameter that equals default is included when explicitly given'); + $this->assertSame('/app.php/index', $generator->generate('test'), 'the default is not included when param not given'); + $this->assertSame('/app.php/index', $generator->generate('test', array('_format' => null)), 'null for an optional variable is ignored'); + $this->assertSame('/app.php/index.', $generator->generate('test', array('_format' => '')), 'empty string is treated as-is (the requirement allows it)'); + } + public function testQueryParamSameAsDefault() { $routes = $this->getRoutes('test', new Route('/test', array('default' => 'value'))); From 18b54c78fb5ec33e3338c32e00a47aa01920bf50 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Wed, 3 Oct 2012 23:25:05 +0200 Subject: [PATCH 2/2] added note in changelog and upgrade about an URL with default param --- UPGRADE-2.2.md | 11 +++++++++++ src/Symfony/Component/Routing/CHANGELOG.md | 6 ++++++ 2 files changed, 17 insertions(+) create mode 100644 UPGRADE-2.2.md diff --git a/UPGRADE-2.2.md b/UPGRADE-2.2.md new file mode 100644 index 0000000000000..dde82a19936ff --- /dev/null +++ b/UPGRADE-2.2.md @@ -0,0 +1,11 @@ +UPGRADE FROM 2.1 to 2.2 +======================= + +### Routing + + * Added possibility to generate URLs that include the default param, whereas before a param + that equals the default value of an optional placeholder was never part of the URL. + Example: Given the route `new Route('/index.{_format}', array('_format' => 'html'));` + Previously generating this route with params `array('_format' => 'html')` resulted in `/index` + which means one could not generate `/index.html` at all. This has been changed. You can of course + still generate just `/index` by passing array('_format' => null) or not passing the param at all. diff --git a/src/Symfony/Component/Routing/CHANGELOG.md b/src/Symfony/Component/Routing/CHANGELOG.md index d02009d84f6b7..49759b0575e0b 100644 --- a/src/Symfony/Component/Routing/CHANGELOG.md +++ b/src/Symfony/Component/Routing/CHANGELOG.md @@ -19,6 +19,12 @@ CHANGELOG check on URL generation completely by calling `setStrictRequirements(null)`. It improves performance in production environment as you should know that params always pass the requirements (otherwise it would break your link anyway). + * [BC BREAK] Added possibility to generate URLs that include the default param, whereas before + a param that equals the default value of an optional placeholder was never part of the URL. + Example: Given the route `new Route('/index.{_format}', array('_format' => 'html'));` + Previously generating this route with params `array('_format' => 'html')` resulted in `/index` + which means one could not generate `/index.html` at all. This has been changed. You can of course + still generate just `/index` by passing array('_format' => null) or not passing the param at all. 2.1.0 -----