users@jax-rs-spec.java.net

[jax-rs-spec users] [jsr339-experts] Re: Re: Re: Re: Re: Re: Re: Matching algorithm doesn't recurse back on Locators

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Tue, 21 May 2013 17:45:06 +0100

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

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
>>>
>>
>>
>