On May 21, 2013, at 6:45 PM, Sergey Beryozkin <sberyozkin_at_talend.com> wrote:
> On 21/05/13 17:39, Marek Potociar wrote:
>>
>> On May 21, 2013, at 6:22 PM, Sergey Beryozkin <sberyozkin_at_talend.com
>> <mailto:sberyozkin_at_talend.com>> wrote:
>>
>>> Hi Marek
>>>
>>> On 21/05/13 16:52, Marek Potociar wrote:
>>>>
>>>> On May 21, 2013, at 5:01 PM, Bill Burke <bburke_at_redhat.com
>>>> <mailto:bburke_at_redhat.com>
>>>> <mailto:bburke_at_redhat.com>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On 5/21/2013 10:50 AM, Marek Potociar wrote:
>>>>>>> And the TCK assumes that 2a also tries to match the HTTP method.
>>>>>>> (Santiago admitted this is an error in the spec).
>>>>>>
>>>>>> I have checked with TCK team - we don't have a clue what test are you
>>>>>> referring to. Can you please work with our TCK team offline and let
>>>>>> them know?
>>>>>>
>>>>>
>>>>> https://issues.jboss.org/browse/CTS-81
>>>>>
>>>>>>> IMO, it is not much of a stretch to do this algorithm.
>>>>>>
>>>>>> It's a stretch that would make it incompatible with JAX-RS 1.x. And
>>>>>> why? So that we fit some implementations into JAX-RS compliant bounds
>>>>>> while exposing 1.x RI users to the incompatible changes. FWIW, I'm
>>>>>> not objecting to opening a discussion about revising/simplifying the
>>>>>> algorithm in general, I am however not comfortable with making
>>>>>> serious BW incompatible decisions under such pressure at all.
>>>>>>
>>>>>
>>>>> Its not incompatible because the matching is a superset.
>>>>
>>>> The problem is that with default OPTIONS and HEAD there are use cases
>>>> that suddenly would start working differently:
>>>>
>>>> @Path("root")
>>>> class Root {
>>>> @GET
>>>> @Path("sub")
>>>> String getSub() { ... }
>>>>
>>>> @Path("sub")
>>>> Sub getLocator() { return new Sub(); }
>>>> }
>>>>
>>>> class Sub {
>>>> @OPTIONS
>>>> Stringoptions() { ... }
>>>> }
>>>>
>>>>
>>>> With JAX-RS 1.1, the above example, for an OPTIONS request to
>>>> "/root/sub" requires default OPTIONS method to be invoked. Your proposal
>>>> would change this to invocation chain Root.getLocator() ->
>>>> Sub.options(). That is my problem with your proposal.
>>>
>>> You probably refer to HEAD here.
>>
>> No, I refer to OPTIONS, it should not however matter in this case if its
>> HEAD or OPTIONS - default method provided by runtime should get in play.
>
> Agreed - unless we have a valid custom OPTIONS handler, The above case is actually spec compliant, where GET is selected. But see my notes immediately below on how you actually will get Sub OPTIONS selected
I think we need to understand this in context of step 3 of the algorithm, where the support for these additional methods is mentioned. So to support HEAD, the GET method should be in the M (list of selected resource/sub-resource methods in step 2(h)), i.e. it should be on the currently matched resource or set of resources (not necessarily just root resource(s)).
I do agree that we could make this more obvious and clear in the spec.
Marek
>
> Sergey
>
>>
>>>
>>> To be honest, defaulting of HEAD to GET is in itself not a great idea
>>> as (even the spec acknowledges) it can affect the performance as you
>>> never know what @GET will return. It is a side note though, but either
>>> way, blocking the algorithm improvements to get this very rare case
>>> supported, instead of, important point here, of getting a valid HEAD
>>> handler selected, does seem wrong to me.
>>>
>>> It does not matter that HEAD is on Sub, the client does not care. The
>>> spec says that if no HEAD is available then we have to try @GET, it
>>> does not say this @GET has to be on the root resource.
>>>
>>> So what happens in the above case if we do go to Sub and it has no HEAD ?
>>>
>>> That is the only problem which I do not know how to fix. It would work
>>> for OPTIONS (we just check all the supported methods starting from Sub
>>> up to Root), but not for HEAD.
>>>
>>> Seriously, this is the only issue with HEAD, who no one uses I think.
>>> I'd update the HEAD handling somehow, make a special case for it
>>> (example, if you have HEAD - then GET match will do). This will make
>>> you example work and let us proceed with the algo supporting more
>>> realistic examples where the subresources offer the valid handlers.
>>>
>>> And by the way. Re issue 405 which I think is valid,
>>>
>>> replace @Path("sub") with @Path("{sub}") on the first method and we
>>> have to go to Locator because that is how it worked in 1.1
>>>
>>>>
>>>>> The current 2.0 draft is *ALREADY* a superset of JAX-RS 1.1 because it
>>>>> allows multiple resource class matches. It doesn't effect
>>>>> compatibility because 1.1 applications would still run with the
>>>>> current changes in 2.0 and the additional small change I'm proposing.
>>>>
>>>> But if I'm correct, the change we made does not pose the same issues as
>>>> the change being proposed now. We're just enabling something that was
>>>> clearly forbidden earlier. I do not think that there are any JAX-RS
>>>> compliant applications out there that contain multiple root resource
>>>> classes with the same @Path values.
>>>>
>>>>>
>>>>>>> 1. Select resource and subresource methods that match the request URI
>>>>>>> 2 if there is an http method match, and produce/consume/accept
>>>>>>> match, then you're good
>>>>>>> 3. If there is no match for URI, http method, and
>>>>>>> produce/consume/accept, try the locators
>>>>>>>
>>>>>>> Markus's (and my) request to support multiple resource classes with
>>>>>>> the same matching path was not suported in 1.1. We expanded the
>>>>>>> matching capabilities in 2.0. I suggest we do the same again for
>>>>>>> media types. Since this 2.0 algorithm is already broken and must be
>>>>>>> fixed, just add a bullet point to 3(a)
>>>>>>
>>>>>> Sorry for being slow here, where exactly is the current algorithm
>>>>>> broken again?
>>>>>>
>>>>>
>>>>> 1.1 algorithm would go onto locators if no HTTP method was matched.
>>>>> See sergey's emails.
>>>>
>>>> I see now, thanks. So to be JAX-RS 1.1 compliant we need to update the
>>>> step 2.(i) to something like:
>>>>
>>>> 5.
>>>>
>>>> Let L be a sub-resource locator such that Rmatch = R(TL).
>>>> Implementations SHOULD report an error if there is more than one
>>>> sub-resource locator that satisfies this condition. *Invoke the
>>>> sub-resource locator method of O and set O **to the value returned
>>>> from that method.* Set U to be the value of the final capturing
>>>> group of R(TL) when matched against U, and set C′ to be the
>>>> singleton set containing only the class *of O*.
>>>>
>>>
>>> Not really sure, we need to apply the whole sequence to issue 405 and
>>> see if it fixes it.
>>
>> I think we also need the fix of 2(h) as suggested by Santiago earlier:
>>
>> Set M as follows,
>> M = {subresource methods of all classes in C′ *where R(T_method) =
>> R_match* (excluding sub-resource locators)}
>> and go to step 3 if M ̸= {}.
>>
>> Marek
>>
>>>> M = { subresource methods of all classes in C′ where R(T_method) =
>>>> R_match (excluding sub-resource locators) }
>>
>>>
>>> Thanks, Sergey
>>>
>>>>
>>>> Correct?
>>>>
>>>> Marek
>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Bill Burke
>>>>> JBoss, a division of Red Hat
>>>>> http://bill.burkecentral.com
>>>>
>>>
>>>
>>
>