On 9/11/2012 10:38 AM, Sergey Beryozkin wrote:
> On 11/09/12 14:39, Bill Burke wrote:
>>
>>
>> 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());
>
> That appears to be a valid case - however - is it really a
> responsibility of MessageBodyWriter to convert POJOs/entities to
> multipart/encrypted payloads as opposed of the filter ?
>
> Example, in CXF we can wrap entities into multiparts but we do not
> require a filter for that to happen, it is all managed at the MBW level
>
Let me expand on the example. Client code does:
client.target(".../foo".request().post(Entity.json(entity));
filter does:
MediaType type = ctx.getMediaType();
ctx.getHeaders().putSingle("multipart/encrypted");
ctx.setEntity(new MultipartEncrypted(ctx.getEntity(), type, key);
Then a MBW for MultipartEncrypted java type and "multipart/encrypted"
media type is invoked. So, the app code doesn't know that the server
requires all posts as multipart/encrypted.
So, maybe this should be done within a MBW interceptor instead? Is that
what you're saying?
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com