finish uri template implementation #3227

Closed
lsmith77 opened this Issue Jan 30, 2012 · 7 comments

Comments

Projects
None yet
5 participants
@lsmith77
Contributor

lsmith77 commented Jan 30, 2012

@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

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Feb 7, 2012

Contributor

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

Contributor

lsmith77 commented Feb 7, 2012

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

@fabpot

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Feb 13, 2012

Member

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

Member

fabpot commented Feb 13, 2012

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

@vicb

This comment has been minimized.

Show comment Hide comment
@vicb

vicb Feb 13, 2012

Contributor

Support for query parameters ?

Contributor

vicb commented Feb 13, 2012

Support for query parameters ?

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Feb 13, 2012

Contributor

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

Contributor

lsmith77 commented Feb 13, 2012

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

@mtdowling

This comment has been minimized.

Show comment Hide comment
@mtdowling

mtdowling Mar 27, 2012

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?

Contributor

mtdowling commented Mar 27, 2012

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?

@Tobion

This comment has been minimized.

Show comment Hide comment
@Tobion

Tobion Apr 26, 2012

Member

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.

Member

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

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Apr 26, 2012

Contributor

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

Contributor

lsmith77 commented Apr 26, 2012

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment