Allow raw formatter returning complete response

[hiqsol]

This allows to use formatting library returning complete response.
E.g. woohoolabs/yin returns whole Psr7 response and it's not
reasonable to convert it to string and back to be compatible with
current Formatter interface.

I can add tests if you're ok with the idea.

[lcobucci]

@hiqsol can you elaborate on this? I don't know that lib so I'm trying to understand why would someone want to format a response which seems to be already formatted.

[hiqsol]

No, I don't want to reformat a response :)
I want to use this library to negotiate response type in uniform way.
I want to provide both json and jsonapi response formatting.
The interface for Formatter is this:

public function format($content, array $attributes = []): string;

Which is perfectly ok for JSON:

public function format($content, array $attributes = []): string
{
    return json_encode($content, $some_flags);
}

But for JsonApi I have library that converts data to a complete PSR 7 HTTP response.

So I want this library to allow raw formatters directly to PSR 7 response:

public function format(UnformattedResponse $response): ResponseInterface
{
    return $library->format($response->getUnformattedContent());
}

Also this will allow formatters to return not only response body but other attributes too: headers, status code.

[lcobucci]

Huumm I see what you mean.
That a quite reasonable and enables users to do much more.

Like you, I'm a huge fan of consistency. So, I'd rather break the BC and change the Formatter interface, releasing this as a new major...

This would allow us to simplify ContentTypeMiddleware#process():

public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
    $response = $this->negotiator->process($request, $handler);

    if (! $response instanceof UnformattedResponse) {
        return $response;
     }

    $contentType = $this->extractContentType($response->getHeaderLine('Content-Type'));
    $formatter   = $this->formatters[$contentType] ?? new NotAcceptableFormat(); // instance can be cached if needed

    return $formatter->format($response, $this->streamFactory); // passing the stream to be optionally used
}

We can also have an abstract class for the "body-only" formatters (for the ones we ship by default):

abstract class BodyOnly implements Formatter
{
    public function format(UnformattedResponse $response, StreamFactoryInterface $streamFactory): ResponseInterface
    {
        return $response->withBody($this->formatBody($response, $streamFactory);
    }

   abstract protected function formatBody(UnformattedResponse $response, StreamFactoryInterface $streamFactory): StreamInterface; // this can follow the current interface of the Formatter, but I think that we're able to do more with the StreamFactoryInterface
}

These snippets are just samples but I believe it would be a more elegant way to achieve your desired outcome.
What do you think?

[hiqsol]

Funny, I was thinking of exactly same approach you describe but proposed this RawFormatter to keep BC :)

So, it's just up to you to choose.
I just prefer it as soon as possible :)

Also it is easy to have both: add RawFormatter in this version and change Formatter interface in new major.

[lcobucci]

Even though is BC-break, its impact is rather low. I'd prefer going with a new major 👍

[hiqsol]

Will you do it yourself? Or you expect PR?

[lcobucci]

Either way works, but it'll be probably faster if you send a PR. I don't see me handling this today or tomorrow if I have to develop it.

[hiqsol]

ok, I'll do PR today