Hi guys,
Hope everyone is fine during this world wide quarantine :).
This mail is to talk about issue
#736 I opened a while ago now.
At the time, we all agreed that following items needs to be clarified:
- the cache concept introduced by all readEntity(...) methods javadoc.
- the javadoc wording with "consumed", "unconsumed", "un-consumed", "fully consumed" to better understand methods behavior.
So I worked on it with Ron Sigal from the RESTEasy project team and we try to find the best way to clarify things. Here is our proposal:
1 - The cache concept between readEntity() and getEntity()
All readEntity(...) methods javadoc introduce a cache concept so that once read, entity is cached for subsequent retrieval via getEntity() method.
Well that's for the theory but in practice Ron spotted out that a TCK test from "com.sun.ts.tests.jaxrs.ee.rs.core.response.JAXRSClient" called "getEntityThrowsIllegalStateExceptionWhenConsumedTest" was in conflict with the javadoc on this cache concept point.
Actually according to this TCK test, if readEntity(...) was previously invoked then the next invocation of getEntity() must throw an IllegalStateException.
Then, given that for any JAX-RS implementations that passes the TCK, the cache concept described in the javadoc does not work anyway, we think that the best option is to remove this cache concept from the javadoc. This would not imply no change from JAX-RS vendors and things will be clearer.
- The getEntity() javadoc could be reviewed to look like:
Get the response entity Java instance.
Returns null if the response does not contain an entity body. Otherwise, it behaves as follow:
• If a Java object was supplied by a call to Response.accepted(), Response.ok() or
ResponseBuilder.entity(), then that object is returned.
• Else if bufferEntity() was previously invoked successfully, a copy of the reset
buffered entity input stream is returned
.
• Else if the entity is represented by the original input stream not fully consumed already
(i.e inputStream.read() != -1), then that input stream is returned.
Returns:
The response entity or null if response does not contain an entity body
(i.e. when hasEntity() returns false).
Throws:
IllegalStateException - if the entity is represented by the original
input stream fully consumed already (i.e inputStream.read() == -1), or if the response
has been closed.
-
The readEntity(...) methods javadoc could be
reviewed
to remove following line:
A message instance returned from this method will be cached for subsequent retrievals via getEntity().
2 - The javadoc wording
-
The
getEntity() javadoc could be reviewed as described above
-
Following changes could be made to the readEntity(...) methods javadoc:
- "or if the original entity input stream has already been consumed" => "or if the original entity input stream has already been fully consumed
(i.e inputStream.read() == -1)"
- "this method automatically closes the an unconsumed original response entity data stream if open." => "this method automatically fully consumes (i.e inputStream.read() == -1)
and closes the original response entity input stream if open."
- "or if the entity input stream has been fully consumed" => "or if the original entity input stream has been fully consumed (i.e inputStream.read() == -1)"
- Following changes could be made to the bufferEntity(...) method javadoc:
- "Buffer the message entity data." => "Buffer the original response entity input stream."
- "In case the message entity is backed by an unconsumed entity input stream, all the bytes of the original entity input stream are read and stored in a local buffer. The original entity input stream is consumed and automatically closed as part of the operation and the method returns
true.
" => "In case the reponse entity is backed by an input stream not consumed at all, all the bytes of that input stream are read (i.e until inputStream.read() == -1)
and stored in a local buffer.
That input stream is then automatically closed as part of the operation and the method returns true.
"
- "In case the response entity instance is not backed by an unconsumed input stream an invocation of
bufferEntity
method is ignored and the method returns false
." => "Otherwise
an invocation of bufferEntity
method is ignored and the method returns false
."
-
Following change could be made to the close(...) method javadoc:
- "The
close()
method should be invoked on all instances that contain an un-consumed entity input stream" =>
"The close()
method should be invoked on all instances that contain an original entity input stream not fully consumed (i.e until inputStream.read() != -1)
"
- Following change could be made to the ResponseBuilder.fromResponse(...) method javadoc:
- "Note that if the entity is backed by an un-consumed input stream" =>
"Note that if the entity is backed by the original input stream not consumed at all"
3 - Adding isClosed() method
Last point is about adding an
isClosed() method as suggested by Ron here
#706.
This would be an additional, rather than changed, behavior but we wonderf if it would be legal to include it in the PR ?
In order JAX-RS vendors have no extra work to do one option would be to provide a default implementation as follow:
public boolean isClosed() {
try {
hasEntity();
return false;
} catch (IllegalStateException e) {
return true;
}
}
Well, I tried to sum up the discussion we had on github with Ron and the other guys, so I hope this mail is not too long.
What do you think of the proposal ?
Thanks !