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
DISCUSSION: Replace ActionResponse
with ResponseInterface
?
#3289
Comments
christian wrote
|
Yes i know. I also thought a bit about this before i wanted to experiment with this. The way i see it is that the current But even before Flow 7 and prs 7, the ActionResponse was at least partially inconsistent and the abstraction broken.
the first 5 are a well fit abstraction, but the What if i actually set cookies AND a After reading the code, i can say that the "headers" functionality is just an array carried around with no validation or transformation. So that will cause conflicts one would ran into with duplicated headers set on the final result. Additionally the mutability of the The in our case transforming the but im clearly in favour of working with the pretty immutable For the |
christian wrote
|
@bwaidelich mentioned once:
but im not sure that we can ever found a well fit response abstraction and then we will always still have the problem that a view might return a psr response which is too low level for us to handle, if were not that low level ourselves. It seems this So it seems the abstraction of the response was intact before psr 7. Now we just need to find out what to do :D Either we remove |
Given that we haven't had that many complains about it in the last years I feel "broken" over-dramatizes this 😆 it certainly has many problems but broken it is not, as it demonstratively works in many common usecases. |
Yeah the setHeader was introduced on pressure of "ease of use" from people. I liked the SetHeader thing. it was clearly marked as escape hatch |
ActionResponse
abstraction is broken since Psr7ActionResponse
abstraction is inconsistent since Psr7
But when seeing that all these steps taken to make the |
And it seems we need easy of use.
i hope that once we allow to directly return a psr response interface from the action (which doesnt work currently!) and once we use this ourselves instead of manipulating
|
What about supporting both, or even more return types? |
We need an anti corruption layer though, an action might return more, but then what about the processRequest interface method which is the actual public border for controllers? that would already be somewhat nasty to be multi return, next step would be dispatcher and at that point I would really like to know what I get from my controller. After that we are in the middlewares which definitely want PSR. |
id say inside the "historic" action controller we can allow all kinds of things, like we already do from views In an Actually returning an The only way to mutate the response currently is via The new For the public api of the controller class Dispatcher
{
public function dispatch(ActionRequest $request): ResponseInterface;
}
interface ControllerInterface
{
public function processRequest(ActionRequest $request): ResponseInterface;
}
class MyController extends ActionController
{
public function someAction()
{
return new \GuzzleHttp\Psr7\Response(body: 'hi');
}
public function someOtherAction()
{
// legacy but will still work
$this->response->setCookie(new Cookie(...));
return "my body";
}
} |
Ha it seems flow went through much more than i could have imagined. The
So @kitsunet please forgive me from calling it broken. I was absolutely in the wrong of doing so ^^. Everything was super well discussed and happened exactly as it should :D Thank you all for the effort. But back to why the
So its only natural that the response needs to now its headers. |
ActionResponse
abstraction is inconsistent since Psr7ActionResponse
with ResponseInterface
?
Funnily the idea to use Psr response is not that new ^^ In Flow 5.3 the
|
while working on #3232 and #2492
I stumbled over the fact that the whole
ActionResponse
abstraction is "broken".In #3294 i experimented with returning a psr response directly from a controller:
The text was updated successfully, but these errors were encountered: