Bug #43589

Route defaults can be omitted when creating URIs

Added by Bastian Waidelich over 2 years ago. Updated over 2 years ago.

Status:Resolved Start date:2012-12-04
Priority:Should have Due date:
Assigned To:Bastian Waidelich % Done:

100%

Category:MVC - Routing
Target version:TYPO3 Flow Base Distribution - 2.0 beta 1
PHP Version:5.4 Complexity:
Has patch:No Affected Flow version:(any)

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:

1-
2  uriPattern: 'foo'
3  defaults:
4    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:

 1-
 2  name: 'Articles with optional pagination'
 3  uriPattern: 'articles(/page/{--paginate.currentPage})'
 4  defaults:
 5    '@package':    'My.Package'
 6    '@controller': 'Article'
 7    '@action':     'index'
 8    '--paginate':
 9      currentPage: '0'
10      '@package': ''
11      '@subpackage': ''
12      '@controller': ''
13      '@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:

 1-
 2  name: 'Articles with pagination'
 3  uriPattern: 'articles/page/{--paginate.currentPage}'
 4  defaults:
 5    '@package':    'My.Package'
 6    '@controller': 'Article'
 7    '@action':     'index'
 8    '--paginate':
 9      '@package': ''
10      '@subpackage': ''
11      '@controller': ''
12      '@action': 'index'
13
14-
15  name: 'Articles'
16  uriPattern: 'articles'
17  defaults:
18    '@package':    'My.Package'
19    '@controller': 'Article'
20    '@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

related to TYPO3.Flow - Feature #39740: Allow setting fixed values for route parts Closed 2012-08-12
related to TYPO3.Fluid - Bug #44376: Paginate widget throws exception Resolved 2013-01-08
duplicated by TYPO3.Flow - Bug #43588: Route defaults can be omitted when creating URIs Closed 2012-12-04

Associated revisions

Revision f17a2274
Added by Bastian Waidelich over 2 years ago

[!!!][BUGFIX] Route defaults have to be specified when creating URIs

Currently when creating URIs one can omit route values that are defined
as defaults of the route.
This is not the intended behavior though, because with a lot of routes
in place this leads to the situation where it gets difficult to target
a certain route.
Thus for a route to match the given route values, one should have to
specify all the routing defaults exactly as they are specified by the
route (except for when a route part appears in the uriPattern).

This is a breaking change if you used Router or UriBuilder to create
URIs without specifying all defaults (see #43589 for details).
Besides, all Fluid ViewHelpers that use the UriBuilder now require the
action argument to be specified.

Change-Id: I7e4e0e20a397839c89a107b28fa176dd62459ec1
Fixes: #43589
Releases: 1.2

Revision 4e327231
Added by Bastian Waidelich over 2 years ago

[TASK] Adjust routing documentation to recent changes

This fixes the routing examples provided with the definitive guide.

Change-Id: Idaff4388a489a435440078828c7267797e75674e
Related: #43589
Releases: master, 2.0

Revision 59dc58ef
Added by Bastian Waidelich over 2 years ago

[TASK] Adjust routing documentation to recent changes

This fixes the routing examples provided with the definitive guide.

Change-Id: Idaff4388a489a435440078828c7267797e75674e
Related: #43589
Releases: master, 2.0

History

#1 Updated by Gerrit Code Review over 2 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

#2 Updated by Gerrit Code Review over 2 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/16960

#3 Updated by Gerrit Code Review over 2 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/16960

#4 Updated by Karsten Dambekalns over 2 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

Also available in: Atom PDF