Issues

ZF-777: Pass entire Request object to Route instances

Description

Currently, instances of the Zend_Controller_Router_Route_Interface receive only the path of the request URI as a string argument to match(). If this method were passed the Zend_Controller_Request_Http object directly, it would have considerable more freedom about the criteria with which to make route matches. As an example, this change would make it easy to implement a Route class that matches routes based on both the request URI and the request method without having to access $_SERVER directly.

Comments

These patches implement the suggested improvement in the various Route classes.

This patch implements the suggested improvement in the RewriteRouter.

{quote}Currently, instances of the Zend_Controller_Router_Route_Interface receive only the path of the request URI as a string argument to match(){quote}

They recieve path only because they were designed to operate on the path element (Dispatcher and Router are the objects which operate on the request data). The only element missing is the host (and maybe port) element. Whole Request object is far too much (POST, COOKIE, FILE, etc) for the route classes.

Moreover, it introduces a tight coupling between additional framework objects, so using the Router alone (outside the ZF MVC) will be much harder this way. You will have to subclass every single route class. And now it's only a matter of subclassing the Router.

With this change the fine line between Dispatcher and Router will be blurred too much in my opinion.

I agree we have to come up with something that allows to match domains (just a random thought: maybe different set of routes for different domains?), but this patch seems as a change in a wrong direction to me. But the last word is of course Matthew's.

That's fine; I obviously wasn't privy to the complete background design philosphy. If you could at least allow the Router to get both the Request Method, as well as the Request URI, that would completely meet my use case. Passing the domain wouldn't hurt, but it doesn't really help me in particular.

I want to be able to take a more RESTful approach by being able to process a GET request differently from a POST request (for example) to the same URI. Of course, this leaves the door open to handle PUT's, DELETE's, etc. also.

To tell you the truth I haven't thought about the REST up until now. And I'm not really sure where it belongs. Is it in the Router, Dispatcher, maybe Controllers itself?

The REST aware route is one way to do it, and Java servlet-like controller 'do' methods are another (ie. doGet, doPost, doHead, doPut, doDelete). Your solution seems easier and less intrusive at a first glance. I guess I have to think it through.

For what it's worth, the Route seems to me like the appropriate place, since it's the piece that actually makes the decision about what requested resource matches what Controller and Action method. For my case, the rest of the system can operate more or less as-is, as long as the Route can say "This combination of URI and Method goes to this Controller and Action method".

Also, for whatever it might be worth, in my application I actually subclassed the Dispatcher to prevent it from "mangling" the Controller and Action names. This way, when I instatiate my Route objects, I can directly map the Request URI/Method to a real Controller classname and Action method name, without muddying the waters. This is only useful to me because I actually hand-crafted the entire URI interface for my application, which everyone else probably isn't going to do, but it seems like the RESTful way to me. :-)

You can grab the request using Zend_Controller_Front::getInstance()->getRequest() at any time.

As the RewriteRouter routes work on paths, I'm not sure it makes sense to push the full request into these classes.

That's a good point, I didn't think about using Zend_Controller_Front::getInstance()->getRequest(). I guess passing the entire request object would be a little excessive. However, it might be a useful enhancement to just pass the request method in addition to the request URI.

If you're creating your own Route implementation, then you can use __construct to pass practically anything you want. Take a look at current Zend_Controller_Router_Route_Module and how Rewrite Router attaches it to itself in addDefaultRoutes() method.

Reopenning for future implementation.

Marking fix version as unknown. Please set fix version when this issue is resolved.

I spent a cross-country flight implementing almost the same thing in subclasses, as full REST support is terribly important to me (with hacks for non-compliant thin clients).

If passing a pointer to the *Request is unacceptable due to coupling concerns, could we modify match(String $path) to something like match(Zend_Controller_Router_Routable_Interface $routable).

This might give the flexibility to create a new *Routable that stores a *Request, adding cohesion while removing coupling.

If this is an interesting approach to you all, I can extract the code and tests from my layer (sitting on top of ZFW) and submit patches. I welcome the assignment.

Your patch is most welcome, Toby. Go ahead and submit.

Scheduling for next minor release; Martel appears to have made headway in a BC way. Martel, please let me know if this is not feasible for 1.6.0.

Changes made to the trunk - please give it a try. Take a look at the last test in RewriteRouter Test Case to get the idea how it should work.

Basically you have to provide a setRequest() method on the route which will be used to inject a request object on addRoute(). It should be interface based but was designed this way just to be consistent with what Zend_View does with it's setView method. For now.

Leaving open until merged to release branch.

Next change implements even better way to get request object to the route. Just extend the abstract route (or implement getVersion method returning int 2 on the route) to make it receive request object as a first parameter to match method.

Leaving open until merged to release.

Released with 16.0

Changing issues in preparation for the 1.7.0 release.