Bug #43589
Route defaults can be omitted when creating URIs
100%
Description
The intended behavior is following:
When generating an URI using the UriBuilder, a route only matches the specified routeValues if they are all equal to the defaults of the route. So given the following route:
- uriPattern: 'foo' defaults: bar: 'baz'
(@package, @controller etc left out intentionally to keep it simple)
following should be the case:
Router::resolve(array('bar' => 'baz')); // resolves to "foo" Router::resolve(array('bar' => 'xyz')); // does NOT resolve Router::resolve(array()); // does NOT resolve
But currently this is the behavior:
Router::resolve(array('bar' => 'baz')); // resolves to "foo" Router::resolve(array('bar' => 'xyz')); // does NOT resolve Router::resolve(array()); // resolves to "foo"
Meaning: you can omit default values (if they are not part of the uriPattern).
This works fine in most cases, but it makes it quite hard to get everything right, because too many routes match and it's hard to comprehend.
I already work on a fix, but it has two drawbacks I'd like to get your feedback for:
1. It's a breaking change and you'll have to check all your routes after applying the fix..
I did that with a quite large project and most routes still worked out of the box. Except for the pagination routes:
- name: 'Articles with optional pagination' uriPattern: 'articles(/page/{--paginate.currentPage})' defaults: '@package': 'My.Package' '@controller': 'Article' '@action': 'index' '--paginate': currentPage: '0' '@package': '' '@subpackage': '' '@controller': '' '@action': 'index'
This route wouldn't resolve if $routeValues['--paginate']['@action‘] is not set to "index" from now on, so I had to create an additional route:
- name: 'Articles with pagination' uriPattern: 'articles/page/{--paginate.currentPage}' defaults: '@package': 'My.Package' '@controller': 'Article' '@action': 'index' '--paginate': '@package': '' '@subpackage': '' '@controller': '' '@action': 'index' - name: 'Articles' uriPattern: 'articles' defaults: '@package': 'My.Package' '@controller': 'Article' '@action': 'index'
2. @action can't be omitted
The other drawback is the fallback behavior of the UriBuilder:
Currently, if you don't specify @package the package of the current request is used. The same for @subpackage and @controller (that's why <f:link.action action="foo" /> links to the fooAction of the current controller.
For @action it's different currently: If you don't specify it, the @action is not set in the $routeValues.
Because of the current behavior a route specifying the @action will still resolve. With the fix that wouldn't be the case anymore..
I see two solutions:
A) Make the actionName argument of UriBuilder::uriFor() required (as well as the action arguments of uri.action, link.action and form.
B) Fallback to "index" (or detect the first action of the controller class) I'm adding this for completeness, but this would be quite error prone IMO..
Related issues
Updated by Gerrit Code Review about 8 years ago
- Status changed from Accepted to Under Review
Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/16960
Updated by Gerrit Code Review about 8 years ago
Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/16960
Updated by Gerrit Code Review about 8 years ago
Patch set 3 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/16960
Updated by Karsten Dambekalns about 8 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100