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?
> 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.
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
>>>>>
>>>>
>>>
>
>