On 3/25/2013 8:50 AM, Sergey Beryozkin wrote:
> On 25/03/13 15:16, Sergey Beryozkin wrote:
>> On 25/03/13 13:42, Marek Potociar wrote:
>>>
>>> On Mar 25, 2013, at 9:18 AM, Sergey Beryozkin<sberyozkin_at_talend.com>
>>> wrote:
>>>
>>>> Hi
>>>> On 15/03/13 15:48, Marek Potociar wrote:
>>>>>
>>>>> On Mar 15, 2013, at 1:28 PM, Sergey Beryozkin<sberyozkin_at_talend.com>
>>>>> wrote:
>>>>>
>>>>>> On 15/03/13 11:29, Marek Potociar wrote:
>>>>>>>
>>>>>>> On Mar 13, 2013, at 11:14 PM, Sergey
>>>>>>> Beryozkin<sberyozkin_at_talend.com> wrote:
>>>>>>>
>>>>>>>> On 13/03/13 14:27, Sergey Beryozkin wrote:
>>>>>>>>> On 13/03/13 14:25, Marek Potociar wrote:
>>>>>>>>>> I'd say that if there is no entity produced, no writers are
>>>>>>>>>> invoked
>>>>>>>>>> and thus no writer interceptors are invoked. That's analogous to
>>>>>>>>>> processing empty inbound messages where the user does not try to
>>>>>>>>>> inject or read the entity.
>>>>>>>>>>
>>>>>>>>> That sounds OK, agreed
>>>>>>>>>
>>>>>>>> What should ContainerResponseContext return in case of the null
>>>>>>>> response body and the resource method actually invoked, for
>>>>>>>> example,
>>>>>>>>
>>>>>>>> public Book getBook() {
>>>>>>>> throw new SomeException();
>>>>>>>> }
>>>>>>>>
>>>>>>>> Mapper: maps it to Response with a null entity.
>>>>>>>>
>>>>>>>> The filters are processing this response, should they return
>>>>>>>> 'null' for ContainerResponseContext for
>>>>>>>> getEntityClass/Type/Annotations, or the actual Book type info ?
>>>>>>>
>>>>>>> Annotations should be always set from method.
>>>>>>> Type and Class should be only set for non-null results.
>>>>>>>
>>>>>> Yes, it helps, thanks. This is actually how I implement it now,
>>>>>> though the user can reset the method-level annotations from the
>>>>>> response filter
>>>>>
>>>>> That's fine,
>>>>>
>>>>
>>>> I've spotted that ResponseBuilder has also has a way to set the
>>>> annotations, thus effectively overriding the method level annotations
>>>> too.
>>>
>>> It's not overriding, it's adding. We discussed this earlier in this
>>> forum. IIRC it was Bill who asked.
>>>
>>
>> OK. So ClientResponseFilter is 'overriding'/resetting, see you earlier
>> confirmation above, and ResponseBuilder is 'adding', is that right ?
>>
>>>>
>>>> FYI, I'd actually prefer to remove the relevant ResponseBuilder
>>>> method, ResponseBuilder#entity(Object entity, Annotation[]
>>>> annotations) for the following reasons:
>>>>
>>>> - ResponseBuilder is all about building the response representation
>>>> - ContainerResponseFilter would be the perfect place to override the
>>>> annotations whenever it is really needed
>>>> - All the data one passes to ResponseBuilder is next available at
>>>> Response level but not the annotations - this will cause the issues
>>>> in cases where Response copy is made in the presence of multiple
>>>> JAX-RS implementations in a given container - this is actually not
>>>> that rare at all....
>>>>
>>>> Do you agree it can make sense to tackle it ? I can open a JIRA for
>>>> that, IMHO this kind of change is minor enough to do even at this stage
>>>
>>> No, I disagree. E.g. request filters on client may need to build an
>>> abort response and may need this functionality.
>> The client response is routed via the response filters, so rather than
>> have this foreign method on Response, why doin't we add it to
>> ClientResponseFilter ?
>>
>
> Well, if we want to have both "reset" (in ResponseBuilder) + "override"
> (in response interceptors) then we do have to keep it.
>
> That said, the code which 'overrides' can have the original + new
> annotations added if needed...
>
IMO, if the JAX-RS method is returning a Response, then the JAX-RS
method metadata is the default and any piece of it can be overriden by
the user within the Response, including annotations. As an analogy, if
the JAX-RS method has a @Produces, and the Response is set with a
different media type, the overriden media type is honored.
Also, +1 to no writers invoked if there is no response entity.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com