yiisoft / data-response

https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
17 stars 12 forks source link

Double formatting of the response body #83

Open Gerych1984 opened 9 months ago

Gerych1984 commented 9 months ago

Good day. Some DataResponse methods cause the response body to be formatted. This causes a marker to be placed about it. But also some immutable methods reset this marker, which leads to repeated formatting. This can lead to both additional processing of large amounts of data or to an error in case of using a "one way" iterator. I think it is necessary to make either an additional marker for the body or not format the body before directly outputting it. Thanks in advance

Gerych1984 commented 9 months ago

Good afternoon. I wanted to write a PR for this issue, but on tests I didn't understand the logic of working with headers. Some headers are forced from the formatter, but others can be deleted/changed from DataResponse. For example:

DataResponseTest::testWithoutHeader we deleted Content-Type from formatter, but in DataResponseTest::testFormatterCouldChange*** we forced it from formatter ignoring DataResponse called methods.

Wouldn't it be better and more logical if DataResponse behaves completely like ResponseInterface? If a user calls DataResponse::withProtocolVersion('1.0') etc then he should get 1.0.

And that's how we get:

$formatter = (new CustomDataResponseFormatter())->withProtocol('2.0');
$dataResponse = $this
    ->createDataResponse(['test'])
    ->withResponseFormatter($formatter);

echo $dataResponse->getProtocolVersion(); //'2.0'

But call $dataResponse->withProtocolVersion('*') equals $formatter->withProtocol('*') in this case

$formatter = (new CustomDataResponseFormatter())->withProtocol('2.0');
$dataResponse = $this
    ->createDataResponse(['test'])
    ->withResponseFormatter($formatter)
    ->withProtocolVersion('1.0');

echo $dataResponse->getProtocolVersion(); //'1.0'

But if we set $dataResponse->withProtocolVersion('*') before formatter in this case $formatter can force own headers. Like $dataReader->withProtocolVersion($formatter->protocol)

$formatter = (new CustomDataResponseFormatter())->withProtocol('2.0');
$dataResponse = $this
    ->createDataResponse(['test'])
    ->withProtocolVersion('1.0')
    ->withResponseFormatter($formatter);

echo $dataResponse->getProtocolVersion(); //'2.0'
samdark commented 8 months ago

@devanych do you remember why it is like that?