ZF-9030: When placing the Route_Module as last in a chain an assertion on seperators in match() might fail

Description

When you define a chain with the default Module route as a final route in the chain the route will fail when the url doesn't explicitly contain a controller or module.

The following example will fail:


    public function testChainingDynamicModuleDefaultModuleMatch()
    {
        $foo = new Zend_Controller_Router_Route(':dynamic');
        $dispatcher = new Zend_Controller_Router_ChainTest_Dispatcher();
        $request = new Zend_Controller_Router_ChainTest_Request('http://www.zend.com/foo');
        $bar = new Zend_Controller_Router_Route_Module(array(),$dispatcher,$request);
        $chain = $foo->chain($bar);

        $res = $chain->match($request);
        $this->assertEquals('foo', $res['dynamic']);
        $this->assertEquals('defctrl', $res['controller']);
        $this->assertEquals('defact', $res['action']);
        $this->assertEquals('default', $res['module']);
    }

It doesn't matter if the first route is a static route or (in the above case) a dynamic one.

My patch is an adjustment of the assertion that happens in Chain::match(). In my opinion it should check if the last route is or isn't of the Module type since the Module route normally contains defaults for module, controller and action.

I'll attach a patch I've made.

Comments

The patch I've included contains (besides the fix for Chain) also an modification of the unit tests.

Of course, if there is a disagreement on the unit tests, the patch for Chain invalidates right away :D

Assigning to Ben to review.

Already had a chance to look at the unit tests?

I'm running with the patched version for a while now (ok, not in production... but still) and haven't experienced any troubles. Still when thinking about the checks that are being performed (which I added myself...) I feel that it might impose unwanted overhead on the Router. For my setup it is totally unnoticeable but for others it might not be... any thoughts?

I think the issue here is basically identical, and thinking about it in that way renders my solution useless since it only deals with the module route, not any other optional routing params

I have run the unit test proposed here under the present version of trunk and it pass as expected, because the ZF-8812 was patched yesterday.

Marking as resolved, based on report by Kim.