8000 [Routing] allow URLs to be generated that include the default param by Tobion · Pull Request #5410 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Routing] allow URLs to be generated that include the default param #5410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[Routing] allow URLs to be generated that include the default param #5410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
787baab
[Routing] allow URLs that include the default param
Tobion Sep 1, 2012
18b54c7
added note in changelog and upgrade about an URL with default param
Tobion Oct 3, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions UPGRADE-2.2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an upgrade instruction starting with Added looks weird to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can reformulate it when fabpot changed his mind about this pr

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.
6 changes: 6 additions & 0 deletions src/Symfony/Component/Routing/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Routing/Generator/UrlGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]]);
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')));
Expand Down
0