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

[jsr339-experts] Re: [jax-rs-spec users] Re: Re: Re: Re: Null response entity and writer providers

From: Sergey Beryozkin <sberyozkin_at_talend.com>
Date: Mon, 25 Mar 2013 22:32:05 +0300

On 25/03/13 21:53, Marek Potociar wrote:
>
> On Mar 25, 2013, at 2:07 PM, Sergey Beryozkin<sberyozkin_at_talend.com> wrote:
>
>> Hi Bill
>> On 25/03/13 16:01, Bill Burke wrote:
>>>
>>>
>>> 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.
>>>
>>
>> Sorry I may've moved the thread in some different direction, though indeed, all agree that no writers are to be invoked in the case of a null entity,
>>
>> What confuses me even more now is that you seem to mean "override", when referring to the annotations, while Marek thinks it is actually 'add'.
>>
>> We definitely need to have a common position on that :-)
>
> It MUST be add IMO. That's how it worked in JAX-RS 1.x RI.

ResponseBuilder did not have a method for adding the annotation in 1.1 ?

> Also, I think it would be wrong to ask users to specify their method level annotations in the response builder, whenever they return a Response from a resource method. Annotation instances are not particularly easy to create...
>
Definitely not easy - why they users would *have* to do though ? It is
optional, if the user does not add them then method level annotations
are used.
May be we are talking about slightly different things ?

> Filters can then choose to add or override the response annotations as necessary.
>
How will the filter response context implementation know what the user
wants ? It is either add or override.

Sergey

> Marek
>>
>> Given that other Response relevant methods do override (as your example of @Produces), it probably makes sense to have the same for annotations, 'override'. Marek - would you agree this time ?
>>
>> Thanks,
>>
>> Sergey
>