[jsr339-experts] Fwd: [jax-rs-spec users] Re: Re: Re: Re: HEADS-UP, IMPORTANT: Problems with JAX-RS interceptors
FYI
Begin forwarded message:
> From: Martin Matula <martin.matula_at_oracle.com>
> Subject: [jax-rs-spec users] Re: [jsr339-experts] Re: Re: Re: Re: HEADS-UP, IMPORTANT: Problems with JAX-RS interceptors
> Date: May 14, 2012 6:05:20 PM GMT+02:00
> To: users_at_jax-rs-spec.java.net, "PERICASGEERTSEN,SANTIAGO" <santiago.pericasgeertsen_at_oracle.com>, "POTOCIAR,MAREK" <marek.potociar_at_oracle.com>, sberyozkin_at_talend.com
> Reply-To: users_at_jax-rs-spec.java.net
>
> My thoughts:
> - for things that make the stream readable by others (i.e. deal with encoding), interceptors will always be global (because it is the sender who decides how the stream is encoded and indicates so in the header - for that reason the decoding interceptors will typically be global and depending on the header info - it does not make any sense to make these name-bound).
> - interceptors that intercept incoming messages which are name bound, are typically not essential for reading the stream - they are specific to how the method annotated by the given annotation wants to pre-process the stream (i.e. they have not much to do with how the sender encoded the message, but more with how the receiving method wants to read it)
>
> For that reason, I don't think readEntity() in pre-matching filter not invoking the name-bound interceptors is an issue - it does the same as any other resource method not annotated by a given annotation if it receives the message. Has anyone came up with an example that breaks it?
>
> Also, I am not sure readEntity() has to take any annotations. What about dynamically bound interceptors?
> IMO annotations parameter should be removed from readEntity() - interceptors chain should be built by the JAX-RS runtime based on the current context - if it is in the pre-matching phase, no name-bound or dynamically bound interceptors are relevant, so only global ones would be used. If the context, already has info about the matched method, the right dynamically bound as well as name-bound interceptors should be in the chain.
>
> And, if we agree with the second bullet at the beginning of my e-mail, a boolean parameter like invokeDynamicAndNameBoundInterceptors in readEntity() could be useful, as I am thinking the filter may (more often than not) want to skip the name bound/dynamic interceptors even in the post-matching phase. Consider this:
> OAuth filter needs to be able to read oauth params from form params (oauth spec allows sending oauth params in forms). But what if an app adds an interceptor (for convenience of the methods annotated with some annotation) that pre-proceses the form somehow (for the purpose of more convenient reading by the annotated set of resource methods). The filter does not want to work with the pre-processed form - it wants to work with the original form.
>
> Martin
>
>
>
> On 5/11/12 6:09 PM, Bill Burke wrote:
>>
>>
>> On 5/11/12 11:58 AM, Santiago Pericas-Geertsen wrote:
>>>
>>> On May 11, 2012, at 10:15 AM, Bill Burke wrote:
>>>
>>>>> Or do you mean that in pre-matching phase I would take all the global
>>>>> interceptors, add them into chain and let them decide based on the
>>>>> annotations and other data exposed via context what to do
>>>>
>>>> yes
>>>>
>>>>
>>>>> and then in post-matching phase I'd add the bound interceptors to the
>>>>> chain and again let the interceptors decide what to do based on the
>>>>> annotations and other data exposed via context?
>>>>
>>>>
>>>> I don't know. Maybe for all filters readEntity() applies global
>>>> interceptors only and "decides based on the annotations and other data
>>>> exposed via context what to do?"
>>>>
>>>> No bound interceptors for readEntity() in filters? Does that make it
>>>> simpler? Filters should call bufferEntity() to be safe. Actually, this
>>>> should be reflected in the Javadoc of readEntity(), IMO. That
>>>> readEntity() consumes the input stream and that to be safe, filter
>>>> implementations should call buffer entiy.
>>>
>>> The fact that we need to restrict the interceptor chain to only run the
>>> global interceptors in a pre-match filter makes me think that there's
>>> still something that is not quite right with the design.
>>>
>>> Personally, I think of pre-match filters as the exception (as a way to
>>> impact the matching algorithm itself) and post-match filters as the
>>> rule. What if readEntity() is only allowed in post-match filters and
>>> throws an illegal state exception in a pre-match filter? In a post-match
>>> filter we have annotations and we can run all the interceptors without
>>> any special rules. It seems to make more sense in that case.
>>>
>>
>> Well, again you have Martin's case of a form-parameter specifying the HTTP method to invoke. Since the HTTP method is part of matching, I don't see how you can get around this.
>>
>> BTW, this use case is very weird anyways. I know you can't use a header to override the HTTP method for browser-based applications, but wouldn't it be better to use a query param? That way you have one simple way to override the HTTP method that works with all media types instead of just working only with forms.
>>
>> So, if Martin can agree that this is a bad way of doing HTTP method overrides, then lets remove readEntity() from PreMatch.
>>
>> Bill
>>
>