Adjust to atk4/ui PSR-7 response

[mvorisek]

related with atk4/ui#1706

[abbadon1334]

Pull Request Draft: Stream Handling Improvement

This pull request aims to remove the problematic exit statement in a cleaner way by implementing better stream handling. The proposed changes include adding a new condition to App::terminate and modifying the Helper::output method.

Changes to App::terminate

Add the following if condition at the beginning of the method:

if ($output instanceof StreamInterface) { // if is a stream, emit directly
    $this->response = $this->response->withBody($output);
    $this->emitResponse();

} elseif ($type === 'application/json') {

Changes to Helper::output

Modify the method with the following code:

$resource = $model->flysystem->readStream($path);
$factory = new \Nyholm\Psr7\Factory\Psr17Factory();
$app->terminate($factory->createStreamFromResource($resource));

Rationale

Basing the output guessing on headers is not suitable for streams. For example, if a file with application/json content needs to be streamed, it will pass the current first condition and proceed to App::outputResponseJson rather than being treated as a stream. Furthermore, during the processing of App::terminate, the JSON will be added with a new property portals (which creates a useless property for those using ATK to create API services).

Additionally, it is incorrect to pass the stream to the App::outputResponse method, where a stream is created and must be a string.

Implementing the proposed changes will address these issues and provide a cleaner solution.

Please let me know your thoughts on this proposal, @mvorisek and @DarkSide666. so @DarkSide666 or I can complete #33 and open a new PR on Atk/Ui

[DarkSide666]

I like that.

Instead of
$stream = (new \Nyholm\Psr7\Factory\Psr17Factory())->createStreamFromResource($resource)
I was using
$stream = \Nyholm\Psr7\Stream::create($resource)
but I guess that's the same thing at the end.

[mvorisek]

Changes to App::terminate

The proposed if ($output instanceof StreamInterface) { placed as the first if looks reasonable.

✔ https://github.com/atk4/ui/blob/4e5fc69160677/src/App.php#L403 Content-Type will still be asserted to be set.

The if should look more like:

if ($output instanceof StreamInterface) {
    $this->response = $this->response->withBody($output);
    $this->outputResponse('');

} elseif ($type === 'application/json') {

to trigger other assertions like unexpected output and/or headers already set in https://github.com/atk4/ui/blob/4e5fc69160677/src/App.php#L1109-L1121.

[DarkSide666]

OK, but I'm not sure about this part https://github.com/atk4/ui/blob/4e5fc69160677/src/App.php#L1131. I think it will overwrite Stream with empty data then isn't it?
Maybe it should be like this then?

if ($data !== '') {
    $this->response->getBody()->write($data);
}

[mvorisek]

I expect the (empty string) data appended.

[DarkSide666]

Proably this PR should be closed in favor of #33

[mvorisek]

Proably this PR should be closed in favor of #33

I will finish this PR, please rebase your #33 on top of this PR.