On 15/08/12 16:57, Sergey Beryozkin wrote:
> On 15/08/12 16:51, Marek Potociar wrote:
>>
>> On Aug 15, 2012, at 2:19 PM, Sergey Beryozkin<sberyozkin_at_talend.com>
>> wrote:
>>
>>> On 07/08/12 15:22, Sergey Beryozkin wrote:
>>>> Hi Santiago
>>>> On 07/08/12 17:02, Santiago Pericas-Geertsen wrote:
>>>>>
>>>>> On Aug 7, 2012, at 3:49 AM, Sergey Beryozkin wrote:
>>>>>
>>>>>> On 07/08/12 16:15, Bill Burke wrote:
>>>>>>> I don't understand what the problem is. Javadoc is just saying
>>>>>>> that if
>>>>>>> the underlying entity is an InputStream, then the Response copy
>>>>>>> has a
>>>>>>> reference to that InputStream. You probalby should buffer it because
>>>>>>> you
>>>>>>> will get an error if two different Response objects referencing the
>>>>>>> same
>>>>>>> InputStream call readEntity().
>>>>>>
>>>>>> Actually, there are few problems here.
>>>>>> 1. Response entity of the type like String or some other type
>>>>>> different from InputStream is lost during Response.fromResponse;
>>>>>
>>>>> So you think there's a problem in the code, not the Javadoc, right?
>>>>> Here are the first few lines of that method:
>>>>>
>>>>> public static ResponseBuilder fromResponse(Response response) {
>>>>> ResponseBuilder b = status(response.getStatus());
>>>>> if (response.hasEntity()) {
>>>>> b.entity(response.getEntity());
>>>>> }
>>>>> ...
>>>>>
>>>>> It seems OK to me, but there could a problem elsewhere.
>>>>>
>>>>
>>>> It was my fault, I did not do the home work and not implemented
>>>> hasEntity(). So my apologies.
>>>> That said, I'm not sure I understand the hasEntity check, why the
>>>> following does not work:
>>>>
>>>> ResponseBuilder b = status(response.getStatus());
>>>> // idempotent
>>>> b.bufferEntity();
>>>> b.entity(response.getEntity());
>>>>
>>>> ?
>>>>>> 2. The whole idea of the user having to do 'buffer' for
>>>>>> Response.fromResponse to work properly is an anti-pattern. It's not
>>>>>> the responsibility of the calling code.
>>>>>
>>>>> It think this is a consequence of the shallow copying semantics above.
>>>>> Often a trade-off between performance and usability.
>>>>>
>>>>
>>>> I still not like the idea of the user having to make sure the method
>>>> provided by the api works correctly.
>>>>
>>>>
>>>> By the way, what about throwing an exception if bufferEntityt/close is
>>>> called on the server side, or is it impossible to implement properly ?
>>>>
>>>
>>> Can we get some absolute clarity on the use of bufferEntity() and
>>> close() on the server side ? Are those two operations supposed to
>>> work on both ends ?
>>
>> So let me ask you what is exactly unclear about the javadoc of those
>> methods? Isn't the expected behavior obvious?
>>
>
> See a follow-up message
>
>>> Also, regarding Response.fromResponse(): can the requirement to
>>> explicitly call bufferEntity() for this to work reliably can be
>>> removed ?
>>
>> I'd prefer to keep doing shallow copy and let developers to decide
>> whether or not the stream is buffered. We don't want to implicitly
>> buffer potentially large streams.
>>
>
> How on earth am I supposed to know what type of entity is there,
> especially when implementing a response filter on the server end ?
>
> The idea of calling bufferEntity() on the server end does not make sense
> any way, and having to know when actually to call is does not make sense
> at all, sorry
Let me clarify with a use case. The requirement is to add a new header
to an existing Response. One option is to use Response.fromResponse();
The developer does not need to know what type of entity is there.
Now, the docs saying that bufferEntity() is recommended for this method
to work when the entity is InputStream does not harm. But I think it
just confuses things because the developer may not know what type of
entity is there.
IMHO, if we have InputStream pointing to a 100MB file, and we call
Response.fromResponse() then lets just do a shallow copy but remove any
reference to bufferEntity() from the method's Javadocs.
If the developer calls Response.fromResponse(oldResponse) and then
attempts to read the data from oldResponse and then from a new response
then so be it, it's not a JAX-RS runtime issue at all
Sergey
> Sergey
>
>> Marek
>>
>>>
>>> Thanks, Sergey
>>>
>>>
>>>> Cheers, Sergey
>>>>
>>>>> -- Santiago
>>>>>
>>>>>>> On 8/6/2012 1:10 PM, Sergey Beryozkin wrote:
>>>>>>>> In JAX-RS 1.1 I was seeing an easy to understand
>>>>>>>> Response.fromResponse
>>>>>>>> output. For example,
>>>>>>>>
>>>>>>>> Response r = Response.entity("Hello").build();
>>>>>>>> Response r2 = Response.fromResponse(r);
>>>>>>>> assertSame("Hello", r2.getEntity());
>>>>>>>>
>>>>>>>> Note that 'r' contains a String, not 'InputStream'.
>>>>>>>> 'Response.entity("Hello").build()' is executed on the server,
>>>>>>>> where as
>>>>>>>> 'Response.fromResponse(r)' is a follow up operation executed in a
>>>>>>>> response filter.
>>>>>>>>
>>>>>>>> In 2.0 I'm seeing 'Response.fromResponse(r)' losing the original
>>>>>>>> 'String', and after checking the java docs I'm seeing:
>>>>>>>>
>>>>>>>> "Note that if the entity is backed by an un-consumed input
>>>>>>>> stream, the
>>>>>>>> reference to the stream is copied. In such case make sure to buffer
>>>>>>>> the
>>>>>>>> entity stream of the original response instance before passing
>>>>>>>> it to
>>>>>>>> this method."
>>>>>>>>
>>>>>>>> I think it's an API bug. The assumption is made there that the
>>>>>>>> Response
>>>>>>>> entity is kept as InputStream - which is an implementation
>>>>>>>> detail of
>>>>>>>> Response. I've been saying that all this buffer() stuff is out of
>>>>>>>> scope
>>>>>>>> of Response but failed to get anyone convinced.
>>>>>>>>
>>>>>>>> For the very least, the javadocs quoted above should further
>>>>>>>> clarify
>>>>>>>> that it only applies to the client side where it is reasonable to
>>>>>>>> expect
>>>>>>>> Response contains InputStream originally.
>>>>>>>>
>>>>>>>> Any comments ?
>>>>>>>> Sergey
>>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>
>
>