TYPO3.Flow

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

This one is a bit tricky, especially if we want to resolve it in a API-backwards-compatible (aka BC) way.
I wrote several spikes and prototypes but I always either broke the API or resulted with a overcomplex architecture..
Here are my findings:

Current request handling¶

1. The RequestHandler builds the HttpRequest¹ calling Request::createFromEnvironment()
2. The HttpRequest merges $_GET, $_POST, $_FILES with the bodyArguments² and unifying them into arguments
3. The Router calls HttpRequest::createActionRequest() and passes all arguments to the created ActionRequest
4. After a route was found the Router then merges the arguments of the ActionRequest with the $matchResults³
5. The AbstractController maps ActionRequest arguments to the parameters of the action to be called (in AbstractController::mapRequestArgumentsToControllerArguments())

Problems¶

Here are a couple of "issues" we have with the current sequence:

• The HttpRequest always merges all arguments and parses the request body in its constructor even if they might not be needed (this can cost performance & memory)
• The parsing of bodyArguments is hard-coded and only supports basic XML, JSON & urlencoded strings.The HttpRequest is built very early (e.g. before DI is available), so we can't easily inject a service that could take over that job. Currently, if you use your custom mediatype (which is encouraged in REST services for example), you'll have to parse the request body yourself somehow and in the worst case the HttpRequest trips over your format when it tries to parse it in Request::decodeBodyArguments()
• Currently there is no way to separately retrieve GET/POST and bodyArguments from the HttpRequest, resulting in the very problem described in this ticket

Long term¶

Christopher came up with the idea to refactor the request handling into some kind of chain: Each component of the chain would take a Request and Response and could apply filters and such on them (visitor pattern).
This would also greatly decouple our MVC architecture from the application (you could define custom chains that end up somewhere else than in an ActionRequest, and you could replace single components – like the Router – with a simpler/better suited implementation for certain requests)

While I really like Christophers idea, I can't exactly imagine how that would work out in practice and whether it would kill the BC cow.. In any case we probably need an intermediate step really soon. So that would be my suggestion:

Short term¶

I tried to come up with some steps that would solve most of our current issues while keeping the API kind of backwards-compatible:

• In general I think the HttpRequest should be less smart and map "1:1" to what exists in the real HTTP request
• Keep Request::getArguments() (which is public API) but let it only return merged _GET and _POST arguments, not the parsed bodyArguments (I'm not sure about _FILES, they can be merged too I guess).
  • In addition the HttpRequest could store _GET and _POST separately so that the merging only needs to be done when needed (the first time) and that _GET and _POST variables can be accessed separately if needed
• HttpRequest::createActionRequest() only passes itself to the constructor of ActionRequest (this is already the case) and does not copy it's arguments to the ActionRequest
• The router passes the $matchResults to the ActionRequest (no merging, the ActionRequest only contains the routing values)
  • Maybe it would make sense to rename ActionRequest::arguments keeping the old getters/setters as deprecated for a while) to something that distinguish them from the HTTP arguments.
  • We could keep ActionRequest::getArguments() BC by returning a merged result of $this->arguments && $this->httpRequest->getArguments()
• The ActionController would do the heavy work and merge required arguments in AbstractController::mapRequestArgumentsToControllerArguments()
  • This is also the time where the bodyArguments are parsed (preferably by some extensible service)

Breaking changes¶

The above path would result in a couple of breaking changes, but only for rare cases as far as I can see:

• arguments returned from Request::getArguments() would not contain the bodyArguments (you could use the above mentioned service to parse those)
• If you override AbstractController::mapRequestArgumentsToControllerArguments() you probably have to adjust your implementation slightly

Footnotes¶

¹ The class of the HTTP request is actually called Request – I call it HttpRequest here to avoid confusion with the ActionRequest

² With bodyArguments we refer to everything that is part of the request body and has not been mapped to _POST or _FILES by PHP – e.g. the payload in a REST request.

³ matchResults are the results of a matched route which is an array of all defaults and the uriPattern values of the route

Will be resolved with https://review.typo3.org/21134/

See #45293