Support BinaryFileResponse and custom streaming response classes. #132
Conversation
The responses aren't streamed yet though.
Bridges/HttpKernel.php
Outdated
| @@ -290,7 +289,8 @@ protected function mapResponse(SymfonyResponse $syResponse, $stdout='') | |||
|
|
|||
| // get contents | |||
| ob_start(); | |||
| if ($syResponse instanceof SymfonyStreamedResponse) { | |||
| $content = $syResponse->getContent(); | |||
| if ($content === false) { | |||
There was a problem hiding this comment.
Can we add a test case where the kernel returns a StreamedResponse to ensure the old functionality works as expected?
There was a problem hiding this comment.
Where does the false come from? See
https://github.com/symfony/http-foundation/blob/master/StreamedResponse.php#L102
There was a problem hiding this comment.
Also how does it handle other types of responses without content? e.g. RedirectResponse? I think we definitely need test cases with some different types of responses to make sure this works as expected.
There was a problem hiding this comment.
Responses without content return null so that would be fine.
Actually, the logic for streamed responses will also work for non-streamed response, because that's what Symfony does in index.php: only send() is used, not getContents(). Maybe we could just always call sendContent() and work with the output buffers? Then php-pm doesn't have to know about specific kinds of responses.
From the default index.php:
$response = $kernel->handle($request);
$response->send();There was a problem hiding this comment.
Good idea - I like it! There is no reason the HttpKernel needs to distinguish between response types, it just needs to buffer the content. Its also a step in the right direction if we want to implement real streamed responses later I think.
|
Updated PR to handle responses (streamed or not) like Symfony does. |
|
Please check the cs findings |
Custom streaming response classes must return false on getContent(), just like StreamingResponse and BinaryFileResponse do.
The responses aren't streamed yet though.