jsr339-experts@jax-rs-spec.java.net

[jsr339-experts] Re: Comments about container filters

From: Bill Burke <bburke_at_redhat.com>
Date: Tue, 11 Sep 2012 09:39:23 -0400

On 9/11/2012 7:43 AM, Sergey Beryozkin wrote:
> On 10/09/12 22:29, Sergey Beryozkin wrote:
>> Hi
>>
>> Instead of creating a new thread every time I have some question or
>> suggestion re container filters I will just keep a single thread,
>> hopefully it wil be easier to track the comments
>>
>> IMHO, here is what needs to be updated (comments from previous threads +
>> new ones) but obviously is up for the discussion
>>
>> 1. ContainerRequestContext does not have to reference Servlet container
>> in its property-related methods. Please review the threads leading to
>> the introduction of these methods - the goal was to let filters exchange
>> the properties - with the current wording they actually provide an
>> interface to HttpServletRequest property-related methods
>>
>> 2. Update ContainerRequestContext documentation to make it obvious
>> global post-match filters run for selected resource and sub-resource
>> locator methods
>>
>> 3. Single parameter ContainerRequestContext.setRequestUri: clarify that
>> it is a relative path which is reset, equivalent to the one returned
>> from UriInfo.getPath; do similar clarifications for a 2 parameter
>> overload.
>>
> Can we drop a two-parameter variant ?
>
> I think it is wrong to let users replace the base URI after the
> underlying container matched the original request URI. Replacing the
> path value using a single-variant option can be very handy but letting
> the users to hack the base URI seems too open. Note that other options
> like redirects (back to the client or locally, using
> RequestDispatcherProvider) is possible
>

I agree with Sergey.

>> 4. ContainerRequestContext should throw IllegalStateException when
>> setters are called as part of the response chain
>>
>> 5. ContainerResponseFilter: drop @PreMatching support, get the global
>> filters executed in all the cases
>>
>> 6. ContainerResponseContext.setEntity(Object, Annotation[], MediaType)
>>
>> I wonder if this method is needed, in this form - I'd really limit it to
>> ContainerRequestContext.*setEntity(Object)*: we can set MediaType on
>> response headers directly if really needed.
>>
>> Annotation[]: this is inconsistent with the fact that
>> ContainerRequestContext has getEntityClass and getEntityType methods and
>> obviously we can not determine a correct Type from Object, hence getting
>> Annotation[] passed seems unusual. The whole idea of actually making the
>> runtime to believe that the method returns a class + type different from
>> the pair which can be reflectively introspected does not seem like a
>> good idea. Replacing the actual entity seems OK/reasonable - for a
>> 'complete' replacement we can simply use a name-bound *blocking request*
>> filter
>>
>
> I'm seeing the same in ClientRequestContext.
>
> IMHO this opens too many options for users. I'm not even sure it is the
> right thing to do, let say the server runtime invokes on a resource
> method which returns Book and then let users replace it with
> List<Chapter> - I think this is wrong and is prone with problems.
>
> I'm fine with letting users customize/override the response entity,
> example, assuming Book is returned, let users update some of Book
> properties or replace it with some subclass instance and then reset the
> entity using setEntity(Object) - the entity is still 'confined' by the
> response type info provided by the resource method.
>
> However letting users completely change the expectation of the runtime
> (re the type info of a given entity) is too loose.
>

I disagree. What if you want to have a filter that automatically
wrapped the object in a different Java type so it could send the object
as multipart/signed or encrypted? i.e. context.setEntity(new
MultipartEncrypted(context.getEntity());


-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com