On 11/09/12 15:49, Bill Burke wrote:
>
>
> 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.
>
Good point about the application code...
> So, maybe this should be done within a MBW interceptor instead? Is that
> what you're saying?
>
This is what I've been thinking about.
Example, our MutipartProvider supports well-known MultipartBody object,
or, in other cases, example, when seeing a Book.class of
application/xml, it wraps it into MultipartBody or delegates to the
selected provider to create individual parts. Guess it can be done at
the writer interceptor level.
Your example above works too, my concern was that it kind makes it easy
to 'loose' the type safety, not sure now if it is a real concern or not
Sergey
>