users@jax-rs-spec.java.net

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

From: Martin Matula <martin.matula_at_oracle.com>
Date: Wed, 09 May 2012 17:28:48 +0200

Perhaps the writeEntity() method should be removed from the request
context - it adds complexity and I haven't found a use-case for it.
Martin

On 5/9/12 4:56 PM, Bill Burke wrote:
>
>
> On 5/9/12 9:07 AM, Marek Potociar wrote:
>> Resending in case you missed this.
>>
>
> Ah I did miss, thanks. Response inline
>
>
>> Marek
>> On May 3, 2012, at 1:56 PM, Marek Potociar wrote:
>>
>>>
>>> On Apr 27, 2012, at 9:52 PM, Bill Burke wrote:
>>>
>>>> Resteasy does not have a PreProcessor interceptor that reads the
>>>> entity. You guys have a valid strong use case that does. It is a
>>>> valid issue that we should talk about and work through to see what
>>>> constraints we should add to the API or to see if we can get an
>>>> improved API.
>>>>
>>>> I freaked out because I thought you were really serious about
>>>> removing interceptors. Maybe start over and discuss again?
>>>
>>> Yes, let's do it.
>>>
>>>>
>>>> I think:
>>>>
>>>> a) Removing generics might be a good idea. I've never used them. Have
>>>> you guys? Then again, what if it could be used as a deployment check?
>>>> If an typed interceptor is assigned to a resource method with
>>>> non-matching types, it could be flagged at deployment time
>>>
>>> As Santiago mentioned, the generic type on interceptors was removed
>>> from the API.
>>>
>>>> b) We should investigate the behavior of calling write/readEntity
>>>> within a filter.
>>>
>>> Perhaps we need to take a step back and investigate the role the
>>> interceptors play in consuming/producing messages. From the end user
>>> point of view, I would specify reader interceptors with regard to the
>>> inbound message (i.e request on the server and response on the client)
>>> and writer interceptors with regard to the outbound message (i.e.
>>> response on the server, request on the client). As a user or a filter
>>> writer I would thus find it *very confusing* if replacing an entity in
>>> an inbound filter via writeEntity invoked writer interceptors that
>>> were set up for processing outbound messages.
>>>
>
> IMO, both outbound and writeEntity() are the same. They are
> marshalling a Java object to a buffer. I would find it confusing if
> interceptors weren't invoked. But I don't care that much. I was just
> trying to offer some solution to Martin's complaint that
> read/writeEntity() could screw up filters/interceptors down the chain
> that were expecting the inbound buffer to be a specific format.
>
>
>>> So if we want to keep the ability to replace entity in the inbound
>>> filter, we must make sure we are not invoking outbound interceptors as
>>> part of this operation. Alternatively, we can consider removing the
>>> writeEntity support in inbound filters with the risk of missing some
>>> use case that would be awkward to implement.
>>>
>
> Again, IMO, global and any interceptor triggered by the annotation
> array passed into writeEntity() should be applied.
>
>
>
>>> A short side note: this whole discussion also brings a question
>>> whether for an end user configuring filters or interceptors directly
>>> is the right granularity. Perhaps we should consider bringing the
>>> client-side Feature concept to the server-side too? The feature would
>>> provide a better granularity and would encapsulate configuration and
>>> setup of all the providers needed for implementing a feature.
>
> I like the idea of a separate ClientFeature and a ServerFeature (or an
> annotation to designate client or server). BTW, the only problem I
> see with Feature (both on client and server) is scanning. I assume it
> will be allowed for filters/interceptors/mbr/mbw that are managed by a
> Feature to not have to be annotated with @Provider? Otherwise they
> could be doubly bound/instantiated/configured. Once from the Feature,
> once from the scanning.
>
>
>>>
>>>>
>>>> c) Maybe writeEntity within PreProcessing should create its own
>>>> interceptor chain based on the type and annotation[] array passed
>>>> into it?
>>>
>>> Just to clarify:
>>> Are you talking about the named binding annotations here?
>>> How would you instantiate those custom annotations?
>>> Would this concept be used in the post-matching inbound filters too?
>>>
>
> I'm saying that anytime anybody calls writeEntity() global
> interceptors and anything triggered by the annotations you pass in
> gets invoked. There are ways to instantiate annotations.
>
>
>>> I'm also afraid that this extra dynamism would have a negative
>>> performance impact. IOW, you cannot calculate everything at
>>> deploy-time but you need to re-construct the interceptor chains at
>>> run-time again and again. Any implementation of caching the
>>> pre-created chains based on values of an array would be rather time
>>> consuming too.
>>>
>
> Calling writeEntity() is already a performance hit as it is
> marshalling the Java object to a buffer which will probably be much
> much higher penalty than any interceptor you invoke.
>
>
>>>> d) Maybe readEntity() should automatically buffer the request?
>>>
>>> What about readEntity(InputStream.class)? I thought you are against
>>> tiny exceptions in the javadoc...
>>>
>
> True. So automatic buffering is probably not a good idea.
>
>
>>>> e) Maybe methods on ContainerRequestContext should be added like
>>>> wasBuffered(), wasRead() etc., then other interceptors will know that
>>>> a read/write/buffering happened?
>>>
>>> Let's not focus on container and let's talk about inbound filter
>>> contexts in general instead.
>>>
>>> Currently bufferEntity() is documented to be idempotent. So calling it
>>> in every filter to ensure the entity is buffered does the trick
>>> without any negative performance impact. But I don't have a strong
>>> opinion here - if you think that having isEntityBuffered() would make
>>> the API more understandable by end users, we can add it.
>>>
>
> Its not an understandability issue. Martin is worried about input
> being emptied or modified by one filter that screws up another filter
> later on in the chain. I'm wondering if methods like wasRead(), etc.
> would help the filter developer identify these problem areas. Or
> maybe its just overkill?
>
>>> I don't see a use case for adding isEntityRead().
>>>
>>> Also let me go back in the emails history a bit and ask a question on
>>> your verification example:
>>> Provided the fact that I have a resource method that requires the
>>> inbound message (with a potential entity) to be signed, but does not
>>> consume the entity, how is your signature verification implementation
>>> based on reader interceptors going to kick in?
>>>
>
> Good point. Verification might be a good usecase for why you'd want
> to bufferEntity() and readEntity() within a Filter.
>
>