Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EPIC: Rewrite the MVC Dispatcher #2156

Open
5 tasks
albe opened this issue Oct 7, 2020 · 6 comments
Open
5 tasks

EPIC: Rewrite the MVC Dispatcher #2156

albe opened this issue Oct 7, 2020 · 6 comments

Comments

@albe
Copy link
Member

albe commented Oct 7, 2020

With the introduction of the PSR-15 middlewares (#1928 and #2019) we cover the whole HTTP dispatch process in a standard and extendable way. At the inner core of this middleware process is currently the MVC dispatcher, which is responsible to convert an Http request to an ActionRequest, resolving a Controller, dispatching the ActionRequest to that Controller and receiving or wrapping the action return value in an ActionResponse, which is then converted into an Http Response that is further piped through the PSR-15 middlewares.

The ActionRequest and ActionResponse abstractions give a nice high level abstraction of the underlying http messages, but currently it's not possible to interact with those in a cross-cutting concern other than inside the controller. Also, the MVC dispatcher currently holds a couple of responsibilities (resolving controller, looping the dispatch attempts, handling authentication errors, etc.). Therefore it might be benefitial to split the dispatcher into another middleware layer that acts upon an ActionRequest and an ActionResponse.

Related / Todos:

@albe
Copy link
Member Author

albe commented Oct 7, 2020

/cc @sorenmalling

@sorenmalling
Copy link
Contributor

The firewall is also handled inside the Dispatcher. This could be moved out as a middleware in such a iteration

@sorenmalling
Copy link
Contributor

Assigned @albe and myself to hold os accountable on the other side of the Flow 7.0 release :)

@albe
Copy link
Member Author

albe commented Nov 1, 2020

One idea: Let's move the currently deprecated Http\Headers class into the Mvc namespace and make it part of the ActionResponse API in order to specify headers to set in the response, instead of the SetHeaderComponent parameter currently.
The idea is that ActionResponse is a higher level abstraction of a response and Headers is a (slightly) higher level abstraction of basic Http headers that hides some details like the Cache-Control stuff. Also we need a replacement for the SetHeaderComponent and just moving that to a middleware that still needs to fetch the specified headers from some global context feels weird. With the Headers inside the ActionResponse, the MVC Dispatcher can translate the ActionResponse into a HTTP Response with headers, which is more straightforward.

We could even do this right now already.

@albe
Copy link
Member Author

albe commented Apr 10, 2021

Related #2219 resp. aeb132d - reverting this commit could be a basis for re-adding the Headers class into the ActionResponse

@mhsdesign
Copy link
Member

Ill move the Headers related discussion over here: #3291
Thanks for all the crumbles you left me. Today i was quite the detective :D

@mhsdesign mhsdesign changed the title Rewrite the MVC Dispatcher EPIC: Rewrite the MVC Dispatcher Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants