8000 merged branch Tobion/relative-path (PR #3958) · symfony/symfony@3a4869d · GitHub
[go: up one dir, main page]
Skip to content

Commit 3a4869d

Browse files
fabpotfabpot
committed
merged branch Tobion/relative-path (PR #3958)
This PR was merged into the master branch. Commits ------- 6703fb5 added changelog entries 1997e2e fix phpdoc of UrlGeneratorInterface that missed some exceptions and improve language of exception message f0415ed [Routing] made reference type fully BC and improved phpdoc considerably 7db07d9 [Routing] added tests for generating relative paths and network paths 75f59eb [Routing] add support for path-relative and scheme-relative URL generation Discussion ---------- [2.2] [Routing] add support for path-relative URL generation Tests pass: yes Feature addition: yes BC break: <del>tiny (see below)</del> NO deprecations: NO At the moment the Routing component only supports absolute and domain-relative URLs, e.g. `http://example.org/user-slug/article-slug/comments` and `/user-slug/article-slug/comments`. But there are two link types missing: schema-relative URLs and path-relative URLs. schema-relative: e.g. `//example.org/user-slug/article-slug/comments` path-relative: e.g. `comments`. Both of them would now be possible with this PR. I think it closes a huge gap in the Routing component. Use cases are pretty common. Schema-relative URLs are for example used when you want to include assets (scripts, images etc) in a secured website with HTTPS. Path-relative URLs are the only option when you want to generate static files (e.g. documentation) that can be downloaded as an HTML archive. Such use-cases are currently not possible with symfony. The calculation of the relative path based on the request path and target path is hightly unit tested. So it is really equivalent. I found several implemenations on the internet but none of them worked in all cases. Mine is pretty short and works. I also added an optional parameter to the twig `path` function, so this feature can also be used in twig templates. Ref: This implements path-relative URLs as suggested in #3908. <del>[BC BREAK] The signature of UrlGeneratorInterface::generate changed to support scheme-relative and path-relative URLs. The core UrlGenerator is BC and does not break anything, but users who implemented their own UrlGenerator need to be aware of this change. See UrlGenerator::convertReferenceType.</del> --------------------------------------------------------------------------- by jalliot at 2012-04-16T09:56:56Z @Tobion For completeness, you should add the option to the `url` and `asset` twig functions/template helpers. --------------------------------------------------------------------------- by stof at 2012-04-16T10:46:06Z @jalliot adding the option to ``url`` does not make any sense. The difference between ``path`` and ``url`` is that ``path`` generates a path and ``url`` generates an absolute url (thus including the scheme and the hostname) --------------------------------------------------------------------------- by Tobion at 2012-04-16T12:27:49Z @stof I guess jalliot meant we could then generate scheme-relative URLs with `url`. Otherwise this would have no equivalent in twig. --------------------------------------------------------------------------- by jalliot at 2012-04-16T12:34:08Z @stof Yep I meant what @Tobion said :) --------------------------------------------------------------------------- by Tobion at 2012-04-18T11:57:04Z The $relative parameter I added besides the existing $absolute parameter of the `->generate` method was not clear enough. So I merged those into a different parameter `referenceType`. I adjusted all parts of symfony to use the new signature. And also made the default `UrlGenerator` implementation BC with the old style. So almost nobody will recognize a change. The only BC break would be for somebody who implemented his own `UrlGenerator` and did not call the parent default generator. Using `referenceType` instead of a simple Boolean is much more flexible. It will for example allow a custom generator to support a new reference type like http://en.wikipedia.org/wiki/CURIE --------------------------------------------------------------------------- by Tobion at 2012-04-18T13:34:58Z ping @schmittjoh considering your https://github.com/schmittjoh/JMSI18nRoutingBundle/blob/master/Router/I18nRouter.php would need a tiny change --------------------------------------------------------------------------- by schmittjoh at 2012-04-18T13:37:39Z Can you elaborate the necessary change? --------------------------------------------------------------------------- by Tobion at 2012-04-18T13:51:10Z This PR changes the signature of `generate` to be able to generate path-relative and scheme-relative URLs. So it needs to be `public function generate($name, $parameters = array(), $referenceType = self::ABSOLUTE_PATH)` and your implementation would need to change `if ($absolute && $this->hostMap) {` to `if (self::ABSOLUTE_URL === $referenceType && $this->hostMap) {` I can do a PR if this gets merged. --------------------------------------------------------------------------- by schmittjoh at 2012-04-18T13:52:14Z If I understand correctly, the old parameter still works, no? edit: Ah, ok I see what you mean now. --------------------------------------------------------------------------- by Tobion at 2012-04-18T13:56:33Z Yeah the old parameter still works but $absolute would also evaluate to true (a string) in your case for non-absolute URLs, i.e. paths. --------------------------------------------------------------------------- by Tobion at 2012-04-19T21:09:46Z ping @fabpot --------------------------------------------------------------------------- by fabpot at 2012-04-20T04:30:18Z Let's discuss that feature for 2.2. --------------------------------------------------------------------------- by Tobion at 2012-04-20T10:40:59Z What are your objections against it? It's already implemented, it works and it adds support for things that are part of a web standard. The BC break is tiny at the moment (almost nobody is affected) because the core UrlGenerator works as before. But if we waited for 2.2 it will be much harder to make the transition because 2.1 is LTS. So I think is makes sense to add it now. Furthermore it makes it much more future-proof as custom generators can more easiliy add support for other link types like CURIE. At the moment a Boolean for absolute URLs is simply too limited and also somehow inconsistent because $absolute = false stands for an absolute path. You see the awkwardness in this naming. Btw, I added a note in the changelog. And I will add documentation of this feature in symfony-docs once this is merged. --------------------------------------------------------------------------- by fabpot at 2012-04-20T12:14:32Z nobody has ever said that 2.1 would be LTS. Actually, I think we are going to wait for 2.3 for LTS. --------------------------------------------------------------------------- by Tobion at 2012-04-20T12:27:18Z Well what I meant is, the longer we wait with this, the harder to apply it. In 04ac1fd you modified `generate` signature for better extensibility that is not even made use of. I think changing `$abolute` param goes in the same direction and has direct use. I'd like to know your reason to wait for 2.2. Not enough time to review it, or afraid of breaking something, or marketing for 2.2? --------------------------------------------------------------------------- by stof at 2012-04-20T16:28:27Z @Tobion the issue is that merging new features forces to postpone the release so that it is tested by enough devs first to be sure there is no blocking bug in it. Big changes cannot be merged when we are hunting the remaining bugs to be able to release. --------------------------------------------------------------------------- by schmittjoh at 2012-04-20T16:42:11Z Considering the changes that have been made to the Form component, and are still being made, I think this is in comparison to that a fairly minor change. Maybe a clearer guideline on the release process, or the direction would help, and avoid confusion, or wrong expectations on contributors' part. --------------------------------------------------------------------------- by Tobion at 2012-10-05T13:52:11Z @fabpot this is ready. So if you agree with it, I would create a documentation PR. --------------------------------------------------------------------------- by stof at 2012-10-13T16:09:47Z @fabpot what do you think about this PR ? --------------------------------------------------------------------------- by Crell at 2012-11-01T16:05:01Z This feels like it's overloading the generate() method to do double duty: One, make a URl based on a route. Two, make a URI based on a URI snippet. Those are two separate operations. Why not just add a second method that does the second operation and avoid the conditionals? (We're likely to do that in Drupal for our own generator as well.) --------------------------------------------------------------------------- by Tobion at 2012-11-01T16:38:39Z @Crell: No, you must have misunderstood something. The generate method still only generates a URI based on a route. The returned URI reference can now also be a relative path and a network path. Thats all. --------------------------------------------------------------------------- by Tobion at 2012-12-13T18:30:28Z @fabpot this is ready. It is fully BC! I also improved phpdoc considerably. --------------------------------------------------------------------------- by Tobion at 2012-12-14T20:51:38Z @fabpot Do you want me to write documentation for it? I would also be interested to write about the new features of the routing component in general. I wanted to do that anyway and it would probably be a good fit for your "new in symfony" articles. --------------------------------------------------------------------------- by fabpot at 2012-12-14T20:58:16Z Im' going to review this PR in the next coming days. And to answer your second question, more documentation or better documentation is always a good thing, so go for it. --------------------------------------------------------------------------- by Tobion at 2013-01-02T21:50:20Z @fabpot ping. I added changelog entries.
2 parents aa8b63b + 6703fb5 commit 3a4869d

File tree

15 files changed

+389
-79
lines changed

15 files changed

+389
-79
lines changed

‎src/Symfony/Bridge/Twig/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7 6302 ,8 @@ CHANGELOG
77
* [BC BREAK] restricted the `render` tag to only accept URIs as reference (the signature changed)
88
* added a render function to render a request
99
* The `app` global variable is now injected even when using the twig service directly.
10+
* Added an optional parameter to the `path` and `url` function which allows to generate
11+
relative paths (e.g. "../parent-file") and scheme-relative URLs (e.g. "//example.com/dir/file").
1012

1113
2.1.0
1214
-----

‎src/Symfony/Bridge/Twig/Extension/RoutingExtension.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ public function getFunctions()
4040
);
4141
}
4242

43-
public func A8C6 tion getPath($name, $parameters = array())
43+
public function getPath($name, $parameters = array(), $relative = false)
4444
{
45-
return $this->generator->generate($name, $parameters, false);
45+
return $this->generator->generate($name, $parameters, $relative ? UrlGeneratorInterface::RELATIVE_PATH : UrlGeneratorInterface::ABSOLUTE_PATH);
4646
}
4747

48-
public function getUrl($name, $parameters = array())
48+
public function getUrl($name, $parameters = array(), $schemeRelative = false)
4949
{
50-
return $this->generator->generate($name, $parameters, true);
50+
return $this->generator->generate($name, $parameters, $schemeRelative ? UrlGeneratorInterface::NETWORK_PATH : UrlGeneratorInterface::ABSOLUTE_URL);
5151
}
5252

5353
/**

‎src/Symfony/Bundle/FrameworkBundle/Controller/Controller.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Bundle\FrameworkBundle\Controller;
1313

14+
use Symfony\Component\HttpFoundation\Request;
1415
use Symfony\Component\HttpFoundation\Response;
1516
use Symfony\Component\HttpFoundation\RedirectResponse;
1617
use Symfony\Component\HttpFoundation\StreamedResponse;
@@ -19,8 +20,8 @@
1920
use Symfony\Component\Form\FormTypeInterface;
2021
use Symfony\Component\Form\Form;
2122
use Symfony\Component\Form\FormBuilder;
23+
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
2224
use Doctrine\Bundle\DoctrineBundle\Registry;
23-
use Symfony\Component\HttpFoundation\Request;
2425

2526
/**
2627
* Controller is a simple implementation of a Controller.
@@ -34,15 +35,17 @@ class Controller extends ContainerAware
3435
/**
3536
* Generates a URL from the given parameters.
3637
*
37-
* @param string $route The name of the route
38-
* @param mixed $parameters An array of parameters
39-
* @param Boolean $absolute Whether to generate an absolute URL
38+
* @param string $route The name of the route
39+
* @param mixed $parameters An array of parameters
40+
* @param Boolean|string $referenceType The type of reference (one of the constants in UrlGeneratorInterface)
4041
*
4142
* @return string The generated URL
43+
*
44+
* @see UrlGeneratorInterface
4245
*/
43-
public function generateUrl($route, $parameters = array(), $absolute = false)
46+
public function generateUrl($route, $parameters = array(), $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH)
4447
{
45-
return $this->container->get('router')->generate($route, $parameters, $absolute);
48+
return $this->container->get('router')->generate($route, $parameters, $referenceType);
4649
}
4750

4851
/**

‎src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
namespace Symfony\Bundle\FrameworkBundle\Controller;
1313

1414
use Symfony\Component\DependencyInjection\ContainerAware;
15-
use Symfony\Component\HttpFoundation\Response;
1615
use Symfony\Component\HttpFoundation\RedirectResponse;
16+
use Symfony\Component\HttpFoundation\Response;
17+
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
1718

1819
/**
1920
* Redirects a request to another URL.
@@ -45,7 +46,7 @@ public function redirectAction($route, $permanent = false)
4546
$attributes = $this->container->get('request')->attributes->get('_route_params');
4647
unset($attributes['route'], $attributes['permanent']);
4748

48-
return new RedirectResponse($this->container->get('router')->generate($route, $attributes, true), $permanent ? 301 : 302);
49+
return new RedirectResponse($this->container->get('router')->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $permanent ? 301 : 302);
4950
}
5051

5152
/**

‎src/Symfony/Bundle/FrameworkBundle/Templating/Helper/RouterHelper.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,17 @@ public function __construct(UrlGeneratorInterface $router)
3636
/**
3737
* Generates a URL from the given parameters.
3838
*
39-
* @param string $name The name of the route
40-
* @param mixed $parameters An array of parameters
41-
* @param Boolean $absolute Whether to generate an absolute URL
39+
* @param string $name The name of the route
40+
* @param mixed $parameters An array of parameters
41+
* @param Boolean|string $referenceType The type of reference (one of the constants in UrlGeneratorInterface)
4242
*
4343
* @return string The generated URL
44+
*
45+
* @see UrlGeneratorInterface
4446
*/
45-
public function generate($name, $parameters = array(), $absolute = false)
47+
public function generate($name, $parameters = array(), $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH)
4648
{
47-
return $this->generator->generate($name, $parameters, $absolute);
49+
return $this->generator->generate($name, $parameters, $referenceType);
4850
}
4951

5052
/**

‎src/Symfony/Bundle/SecurityBundle/Templating/Helper/LogoutUrlHelper.php

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,36 +55,40 @@ public function registerListener($key, $logoutPath, $intention, $csrfParameter,
5555
}
5656

5757
/**
58-
* Generate the relative logout URL for the firewall.
58+
* Generates the absolute logout path for the firewall.
5959
*
6060
* @param string $key The firewall key
61-
* @return string The relative logout URL
61+
*
62+
* @return string The logout path
6263
*/
6364
public function getLogoutPath($key)
6465
{
65-
return $this->generateLogoutUrl($key, false);
66+
return $this->generateLogoutUrl($key, UrlGeneratorInterface::ABSOLUTE_PATH);
6667
}
6768

6869
/**
69-
* Generate the absolute logout URL for the firewall.
70+
* Generates the absolute logout URL for the firewall.
7071
*
7172
* @param string $key The firewall key
72-
* @return string The absolute logout URL
73+
*
74+
* @return string The logout URL
7375
*/
7476
public function getLogoutUrl($key)
7577
{
76-
return $this->generateLogoutUrl($key, true);
78+
return $this->generateLogoutUrl($key, UrlGeneratorInterface::ABSOLUTE_URL);
7779
}
7880

7981
/**
80-
* Generate the logout URL for the firewall.
82+
* Generates the logout URL for the firewall.
83+
*
84+
* @param string $key The firewall key
85+
* @param Boolean|string $referenceType The type of reference (one of the constants in UrlGeneratorInterface)
8186
*
82-
* @param string $key The firewall key
83-
* @param Boolean $absolute Whether to generate an absolute URL
8487
* @return string The logout URL
88+
*
8589
* @throws \InvalidArgumentException if no LogoutListener is registered for the key
8690
*/
87-
private function generateLogoutUrl($key, $absolute)
91+
private function generateLogoutUrl($key, $referenceType)
8892
{
8993
if (!array_key_exists($key, $this->listeners)) {
9094
throw new \InvalidArgumentException(sprintf('No LogoutListener found for firewall key "%s".', $key));
@@ -97,13 +101,13 @@ private function generateLogoutUrl($key, $absolute)
97101
if ('/' === $logoutPath[0]) {
98102
$request = $this->container->get('request');
99103

100-
$url = ($absolute ? $request->getUriForPath($logoutPath) : $request->getBasePath() . $logoutPath);
104+
$url = UrlGeneratorInterface::ABSOLUTE_URL === $referenceType ? $request->getUriForPath($logoutPath) : $request->getBasePath() . $logoutPath;
101105

102106
if (!empty($parameters)) {
103107
$url .= '?' . http_build_query($parameters);
104108
}
105109
} else {
106-
$url = $this->router->generate($logoutPath, $parameters, $absolute);
110+
$url = $this->router->generate($logoutPath, $parameters, $referenceType);
107111
}
108112

109113
return $url;

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Security/LocalizedFormFailureHandler.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@
1212
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Security;
1313

1414
use Symfony\Component\HttpFoundation\RedirectResponse;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
1517
use Symfony\Component\Routing\RouterInterface;
1618
use Symfony\Component\Security\Core\Exception\AuthenticationException;
17-
use Symfony\Component\HttpFoundation\Request;
1819
use Symfony\Component\Security\Http\Authentication\AuthenticationFailureHandlerInterface;
1920

2021
class LocalizedFormFailureHandler implements AuthenticationFailureHandlerInterface
@@ -28,6 +29,6 @@ public function __construct(RouterInterface $router)
2829

2930
public function onAuthenticationFailure(Request $request, AuthenticationException $exception)
3031
{
31-
return new RedirectResponse($this->router->generate('localized_login_path', array(), true));
32+
return new RedirectResponse($this->router->generate('localized_login_path', array(), UrlGeneratorInterface::ABSOLUTE_URL));
3233
}
3334
}

‎src/Symfony/Component/Routing/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ CHANGELOG
8181
are now also allowed.
8282
* [BC BREAK] `RouteCompilerInterface::compile(Route $route)` was made static
8383
(only relevant if you implemented your own RouteCompiler).
84+
* Added possibility to generate relative paths and network paths in the UrlGenerator, e.g.
85+
"../parent-file" and "//example.com/dir/file". The third parameter in
86+
`UrlGeneratorInterface::generate($name, $parameters = array(), $referenceType = self::ABSOLUTE_PATH)`
87+
now accepts more values and you should use the constants defined in `UrlGeneratorInterface` for
88+
claritiy. The old method calls with a Boolean parameter will continue to work because they
89+
equal the signature using the constants.
8490

8591
2.1.0
8692
-----

‎src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,15 @@ private function generateDeclaredRoutes()
108108
private function generateGenerateMethod()
109109
{
110110
return <<<EOF
111-
public function generate(\$name, \$parameters = array(), \$absolute = false)
111+
public function generate(\$name, \$parameters = array(), \$referenceType = self::ABSOLUTE_PATH)
112112
{
113113
if (!isset(self::\$declaredRoutes[\$name])) {
114114
throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', \$name));
115115
}
116116
117117
list(\$variables, \$defaults, \$requirements, \$tokens, \$hostnameTokens) = self::\$declaredRoutes[\$name];
118118
119-
return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$absolute, \$hostnameTokens);
119+
return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostnameTokens);
120120
}
121121
EOF;
122122
}

0 commit comments

Comments
 (0)
0