oscarotero / psr7-middlewares

[DEPRECATED] Collection of PSR-7 middlewares
MIT License
668 stars 56 forks source link

FormatNegotiator - hard coded priorities? #80

Closed mlambley closed 7 years ago

mlambley commented 7 years ago

Within FormatNegotiator, the negotiateHeader function call sends the hardcoded $headers as the $priority parameter to the Negotiator instance.

Therefore, if the Accept header is set to "*/*", the result will always be "html" regardless of what is specified as the default format.

//Code
Middleware::FormatNegotiator()
    ->defaultFormat('json')

//Request header
Accept: */*

//Result: 
FormatNegotiator::getFormat($request) === "html"

Negotiator allows you to specify a list of priorities. Could this functionality be emulated in FormatNegotiator?

For example:

//Code
Middleware::FormatNegotiator()
    ->setPriorities(['json', 'zip'])

//Request header
Accept: */*

//Result: 
FormatNegotiator::getFormat($request) === "json"
oscarotero commented 7 years ago

Ok, the */* header should return the default format. I think a better way is just adding a if in the getFromHeader function to check whether the header is missing or is * or */* and return null. Do you mind to create a pull request with this patch? Thanks!

mlambley commented 7 years ago

Hi Oscar, thank you for getting back to me.

While it is true that your suggested patch would fix the exact issue I described, I was hoping that we could go a little further, and allow for the full functionality provided by willdurand/negotiation. The way that Negotiation works is as follows:

Given a list of content types which the server supports (in priority order), 
and a list of types which the client accepts, 
determine the best type in which to serve the content.

My API supports only application/json and application/zip. It does not serve pdf, html, or anything else. I can create a Negotiator instance to reflect this:

$negotiator = new \Negotiation\Negotiator();
$acceptHeader = 'application/pdf, application/zip';
$priorities   = array('application/json', 'application/zip');
$mediaType = $negotiator->getBest($acceptHeader, $priorities);
//$mediaType->getType() === "application/zip"

Whereas I cannot tell FormatNegotiator that I do not want to serve PDF:

//Code
Middleware::FormatNegotiator()
    ->defaultFormat('json')

//Request header
Accept: application/pdf, application/zip

//Result: 
FormatNegotiator::getFormat($request) === "pdf"

I'd like to create a PR which will allow the user to filter the list of $formats to only include the ones which their server supports.

mlambley commented 7 years ago

Have a look at https://github.com/oscarotero/psr7-middlewares/pull/81 and tell me what you think. Thanks.

oscarotero commented 7 years ago

PR merged.