users@jax-rs-spec.java.net

[jax-rs-spec users] Re: [jsr339-experts] Re: Re: Re: Re: HEADS-UP, IMPORTANT: Problems with JAX-RS interceptors

From: Santiago Pericas-Geertsen <santiago.pericasgeertsen_at_oracle.com>
Date: Mon, 14 May 2012 19:05:37 -0400

On May 14, 2012, at 12:05 PM, Martin Matula wrote:

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

 I think this is generally true, but I could also come up with exceptions to these rules

>
> For that reason, I don't think readEntity() in pre-matching filter not invoking the name-bound interceptors is an issue

 This was not the issue that prompted the proposal to drop it from a pre-matching filter. The issue is (i) the overall complexity of a model in which interceptor chains are different in pre and post filters (more for the developer to understand) and (ii) the apparent mismatch of having to pass an Annotation[] in a pre-match context.

> Also, I am not sure readEntity() has to take any annotations.

 That's one of the points of the proposal.

> What about dynamically bound interceptors?

 I don't see the issue. Could you elaborate? Dynamically bound interceptors are resolved at deploy time.

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

 That would mean the corresponding MBR will always get an empty array of annotations? Aren't we restricting its functionality by doing that? Again, I think the reason why we're going in this direction is because we are allowing readEntity in the pre-match phase. IMO, interceptors (as wrappers over MBR/MBW that deal with annotations from declarations) are really post-matching thingies ...

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