Handle wildcard "+" segments #92
Conversation
|
Hi @weierophinney, thanks for this great patch! I will fix the Appveyor setup and trigger the build again for this PR but looks good to me :) |
| $ab = $accept->getBasePart(); | ||
| $pb = $priority->getBasePart(); | ||
| $acceptBase = $accept->getBasePart(); | ||
| $priorityBase = $priority->getBasePart(); |
There was a problem hiding this comment.
I changed these variable names as I was confused by the two-character names when attempting to make my changes. I can revert these if desired, but I think they make the intent far clearer.
| if (($acceptBase === '*' || $baseEqual) | ||
| && ($acceptSub === '*' || $subEqual) | ||
| && count($intersection) === count($accept->getParameters()) | ||
| ) { |
There was a problem hiding this comment.
I split this condition across multiple lines to better understand the precedence of operations.
| $score = 100 * $baseEqual + 10 * $subEqual + count($intersection); | ||
|
|
||
| return new Match($accept->getQuality() * $priority->getQuality(), $score, $index); | ||
| } | ||
|
|
||
| if (!strstr($acceptSub, '+') || !strstr($prioritySub, '+')) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
If either the accept header or the priority do not have + segments, we can return immediately without further checks, retaining the previous workflow.
a04be91 to
b737a69
Compare
A relatively common way to allow negotation to multiple serialization
structures for a single media type is to use "+" notation within the
mediatype:
- `application/vnd.book+json`: JSON serialization of
`application/vnd.book` mediatype.
- `application/vnd.book+xml`: XML serialization of
`application/vnd.book` mediatype.
In particular, this is useful for determining what _general_
serialization is used for an incoming request in order to select an
appropriate deserializer.
With the current 2.X branch, I expected the following to work:
```php
use Negotiation\Negotiator;
$mediaType = (new Negotiator)->getBest($accept, [
'application/json',
'application/*+json',
'application/xml',
'application/*+xml',
]);
```
However, it failed on each of the cases listed above.
This patch provides additional negotiation routines within
`Negotiation\Negotiator::match` in order to allow such negotiation to
work as expected.
b737a69 to
01e760c
Compare
|
Thanks! |
|
Released 2.3.0 with this feature! |
|
Hi @willdurand, @weierophinney! This PR and the new release (2.3) breaks our I have this accept header: Then, the It looks like something is not working properly here, nop? Thanks. (Ping @dunglas) |
|
ah :( |
Fixes willdurand#93. If there are no wildcard segments in either the subtype or + segment of either the Accept header or priority, do not attempt to compare them. Adds more tests for willdurand#92 as well, using `*` subtypes in both the Accept header and priorities, to validate that different combinations work as expected.
A relatively common way to allow negotation to multiple serialization structures for a single media type is to use "+" notation within the mediatype:
application/vnd.book+json: JSON serialization ofapplication/vnd.bookmediatype.application/vnd.book+xml: XML serialization ofapplication/vnd.bookmediatype.In particular, this is useful for determining what general serialization is used for an incoming request in order to select an appropriate deserializer.
With the current 2.X branch, I expected the following to work:
However, it failed on each of the cases listed above.
This patch provides additional negotiation routines within
Negotiation\Negotiator::matchin order to allow such negotiation to work as expected.