yin icon indicating copy to clipboard operation
yin copied to clipboard

Add support for PSR-17 Http factory

Open SamMousa opened this issue 4 years ago • 2 comments

Instead of passing in a response object the JsonApi class should use a HttpFactoryInterface to generate the response if and when it is needed.

This will break backwards compatibility and will require minor code changes from library consumers.

SamMousa avatar Oct 13 '20 11:10 SamMousa

I'm not convinced this change would bring much benefit, while adding a new dependency to the project, and taking away the choice from users whether they want to use PSR-17 or not.

If I'm not mistaken, one could just use HttpFactoryInterface to instantiate the response before passing it to JsonApi. Or do you see any use-case when the above solution would work suboptimally?

kocsismate avatar Dec 06 '20 12:12 kocsismate

The downsides to this use-case are the generic downsides of passing in an object to library code and hoping to get it back. So nothing is specific to this library. This article lists some downsides for the middleware using PSR-7 Response objects: https://blog.ircmaxell.com/2016/05/all-about-middleware.html. Some of them will apply to this case as well.

For example, currently if I pass a response with some headers, what will happen to them? Will this library always remove them, always ignore them? All of that is essentially unspecified.

while adding a new dependency to the project

The dependency contains 6 interfaces, no implementation.

A trivial implementation would change the sample code to something like this:

// Before
$jsonApi = new JsonApi($request, new Response(), $exceptionFactory);

// After if you don't want to use an implementing dependency
$responseFactory = new class implements ResponseFactoryInterface {
    public function createResponse(int $code = 200, string $reasonPhrase = ''): ResponseInterface {
        return new Response();
    }
}

$jsonApi = new JsonApi($request, responseFactory, $exceptionFactory);

SamMousa avatar Dec 07 '20 09:12 SamMousa