Read getParsedBody() instead of getBody() when Request is ServerRequest

[simPod]

Follows up #598

[coveralls]

Coverage decreased (-0.01%) to 86.272% when pulling fbe7ca4 on simPod:support-ServerRequestInterface into 5042bed on webonyx:master.

[vladar]

I think we need a test for it? Then it is good to go!

[simPod]

Sure, if this is alright, I'll cover it.

[vladar]

Looks good to me. @PowerKiKi will it work for you?

[PowerKiKi]

Thank you for reaching out. I meant to come back to this earlier, but it slipped off my mind.

I'll take some time to test this over the weekend and will get back to you.

[PowerKiKi]

This work well for me. Thank you for taking the time fix this regression.

However I still think that parsePsrRequest() signature should explicitly accept a ServerRequestInterface, because it is semantically correct.

According to the spec, RequestInterface is:

Representation of an outgoing, client-side request.

Which is incorrect for our use-case. This lib does not send outgoing requests to other targets. But instead it does accept incoming requests and create a response for them. And that is exactly what ServerRequestInterface is for:

Representation of an incoming, server-side HTTP request.

Also hardcoding "polymorphism" via instanceof looks like code smell to me. Especially in this case where it was not needed before and then complexity was introduced.

This also introduce the new responsibility to this lib to be able to parse incoming request body. I would rather let that responsibility to dedicated libraries. (Yes, if this PR is accepted, it will allow to parse via another lib, but the point is to avoid setting expectation for this lib and instead be clear that parsing should be done by another lib).

Finally, I fail to see why the original issue was not solved outside of this library by something as simple as:

function processOutgoingRequest(\Psr\Http\Message\RequestInterface $outgoingRequest)
{
    $incomingRequest = new \Laminas\Diactoros\ServerRequest();
    $incomingRequest = $incomingRequest
        ->withBody($outgoingRequest->getBody());

    $server = new StandardServer([/* ... */]);
    foreach ($outgoingRequest->getHeaders() as $key => $value) {
        $incomingRequest = $incomingRequest->withHeader($key, $value);
    }

    return $server->executePsrRequest($incomingRequest);
}

tl;dr: i'm fine with merging this because it un-breaks things, but it is a sub-optimal solution

[simPod]

TBH I don't see a reason why it shouldn't be able to parse all Requests.

[vladar]

@PowerKiKi I think you are right given the description of those PSR request interfaces. But sadly simply reverting the original PR and using ServerRequestInterface will now be another breaking change. So at least for now, I am going to merge this PR and maybe revisit before the next major.

Thanks for surfacing this problem.

[simPod]

@vladar definitely there's no doubt about those descriptions but I was wondering that allowing a wider API for lib consumers might be a good thing while RequestInterface already contains everything the Server needs.

I can prepare a revert and further cleanup if desired tho.

[PowerKiKi]

RequestInterface already contains everything the Server needs.

This is where we disagree. Since this merged PR re-introduced support for ServerRequestInterface, then it means that RequestInterface does not contains everything we need.

If I had to decide, I'd revert (in the next breaking or as a "minor" breaking), but I'll let that decision up to @vladar.

Meanwhile I can update my libs to be compatible with ^14.3. Thanks !