symfony/symfony — pull request #5129 by kostiklv (2012-07-31 17:50:09+00:00)
Title: [Routing] Fixed handling of optional parameters with text suffix in the route (/foo-{bar}-{baz}.html)

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/kostiklv/symfony.png?branch=routes-w-optional-params-and-suffix)](http://travis-ci.org/kostiklv/symfony)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

The routes containing a suffix (such as fake .html extensions) following optional variables were not handled correctly. RouteCompiler was looking for the index of the first optional variable, but if the first token (starting from the end) was not a variable, but text, it was just breaking out, assuming there are no optional variables.

Fixing the detection of first optional variable lead to a problem with one of the test cases (for route '/{bar}/foo'). The RouteCompiler was creating nested optional subpatterns for optional variables, and closing them at once by calculating the difference between total number of tokens and the first optional variable's token, but it was assuming that last token (which should have closed the subpatterns) is always variable's token.

To fix that, and also a problem with correct matching of several optional variables with different requirements, I have changed it to create sequential optional subpatterns instead of nesting them. This change resulted in incorrect merging of defaults, when optional subpattern was returning an empty match (it was not possible before, because of nesting subpatterns), so I fixed mergeDefaults in UrlMatcher to skip empty matches (and effectively use defaults instead).

This fixes the following cases (which I've added to RouteCompiler's and UrlMatcher's tests):
/bar-{foo}.html (with default for foo)
/bar-{foo}-{baz}.html (with defaults for both foo and baz)

I had to adjust one of the existing RouteCompiler's tests (since the regexp has changed), but note that none of existing UrlMatcher tests were changed, meaning the fix is not breaking anything in the matching process.

I've first faced the issue while trying to create the following URL structure:
/products.html - lists first page of products, sorted by popularity
/products-by-name.html - lists first page, sorted by name
/products-2.html - the second page, by popularity
/products-by-price-2.html - the second page, by price

(more details in my [Google Groups post](https://groups.google.com/forum/?fromgroups#!topic/symfony2/5kvtqg4Xc4Y))