8000 finish uri template implementation · Issue #3227 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

finish uri template implementation #3227

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

Jump to bottom
Closed
lsmith77 opened this issue Jan 30, 2012 · 7 comments
Closed

finish uri template implementation #3227

lsmith77 opened this issue Jan 30, 2012 · 7 comments
Labels
Feature Routing

Comments

@lsmith77
Copy link
Contributor

@henrikbjorn reminded me this morning that we intended to revisit this topic after 2.0 stable

http://code.google.com/p/uri-templates/

the main thing to add is support for query parameters. these would however not be used for the matching process in the Routing layer. it would only be used to automatically set the given parameters as request attributes so that they can optionally be passed to actions. also this would allow setting default value for query parameters.
@lsmith77
Copy link
Contributor Author
lsmith77 commented Feb 7, 2012

Guzzle seems to have added support:
guzzle/guzzle@b4afc98

@fabpot
Copy link
Member
fabpot commented Feb 13, 2012

Which part would like to add support for (which would be relevant for the Routing component)?

@vicb
Copy link
Contributor
vicb commented Feb 13, 2012

Support for query parameters ?

@lsmith77
Copy link
Contributor Author

yeah .. afaik we also wanted to support the syntax for defining default values

@mtdowling
Copy link
Contributor

URI templates don't necessarily provide a syntax for defining default values or representing a resource location, but rather place holders and modifiers that determine how an associative array of variables are injected into a URI. I went through each example in the URI templates draft 08 and tested Guzzle against them. For a list of these use cases, see https://github.com/guzzle/guzzle/blob/master/tests/Guzzle/Tests/Http/UriTemplateTest.php#L39-L112

This excerpt somewhat touches on this subject:

Some URI Templates can be used in reverse for the purpose of variable matching: comparing the template to a fully formed URI in order to extract the variable parts from that URI and assign them to the named variables. Variable matching only works well if the template expressions are delimited by the beginning or end of the URI or by characters that cannot be part of the expansion, such as reserved characters surrounding a simple string expression. In general, regular expression languages are better suited for variable matching.

I could see how adding some of the modifier support of URI templates could be helpful, where the modifiers specify whether a variable is url-encoded (prefix with +), how an array of values are imploded (suffix with *), etc.

What are your thoughts?

@lsmith77 lsmith77 mentioned this issue Apr 4, 2012
Closed
@asm89 asm89 mentioned this issue Apr 20, 2012
Closed
@Tobion
Copy link
Contributor
Tobion commented Apr 26, 2012

I read the URI template draft and must say I see no reason why the routing component should implement this syntax and semantics.
It complicates things and will not really make generating or matching URLs more flexible. The basic idea there is that you can define a variable that gets expanded according to the operator (with different delimiters like / or & or ,). But that would mean for us we need to support variables that are arrays. Example: Given a route like /prefix{/multiple*} and the variables $variables['multiple'] = array('path1','path2') would generate /prefix/path1/path2. But I think it's not really useful as you can currently achieve the same behavior with one simple addition $variables['multiple'] = implode('/', array('path1','path2')). And making the delimiter configurable inside the routing adds no real value. Btw, if you have an undefined number of variables like ``/prefix/{path1}/{path2}/{path3}/...` they should probably be expressed as GET parameters instead of path segments. So a variable of type array is not what we need or should promote. See #3705

The URI template also includes this variable expansion for query parameters and anchors. But anchors do not really belong in the routing as they are not used for matching URLs (and should not). And generating URLs with anchors can easily be solved outside the routing. Such feature request has already been closed by Fabien: #3910

The only thing that might make sense is adding support for query parameters in the routing layer. But the benefit is not big. Setting defaults for the query params can easiliy be achieved in the controller: $request->query->get('param', 'default'). One benefit of passing matched query params to the action is that is will abstract away the source of the variable. So you could later change a variable from the query to the path without the need to modifiy the controller etc.
Btw, passing all potential query params to the action has already been closed by Fabien for security reasons.

So all in all it might make sense to add support for query params in the routing layer, but that would have nothing to do with URI template draft. URI template does not allow defining defaults anyway and its syntax with variable expansion is nothing we need or should promote. So I would close this issue and open a new feature request with only query params in routing layer.

@lsmith77
Copy link
Contributor Author

Yeah, I was mainly interested in query support. The main benefit I see from putting it in the routing is that it would give us a canonical way to specify this sort of thing to enable reusing the information in other places:

  • API doc generation
  • link tag generation
  • ..

see also FriendsOfSymfony/FOSRestBundle#185

@fabpot fabpot closed this as completed Apr 28, 2014
@ReservedDeveloper ReservedDeveloper mentioned this issue Aug 27, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Routing
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @mtdowling @vicb @lsmith77 @Tobion
0