[GitHub] [cxf] andymc12 commented on a change in pull request #866: CXF-8601: [Regression] jaxrs.ee.rs.core.response tests

[GitBox <gi...@apache.org>]

andymc12 commented on a change in pull request #866:
URL: https://github.com/apache/cxf/pull/866#discussion_r740669134



##########
File path: rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/ResponseImplTest.java
##########
@@ -598,6 +601,42 @@ public void testGetLinksMultipleMultiline() {
         }
     }
 
+    @Test
+    public void testReadEntityAndGetEntityAfter() {
+        final String str = "ouch";
+
+        final ResponseImpl response = new ResponseImpl(500, str);
+        final Message outMessage = createMessage();
+        outMessage.put(Message.REQUEST_URI, "http://localhost");
+        response.setOutMessage(outMessage);
+
+        final MultivaluedMap<String, Object> headers = new MetadataMap<>();
+        headers.putSingle("Content-Type", "text/xml");
+        response.addMetadata(headers);
+
+        assertEquals(str, response.readEntity(String.class));
+        assertThrows(IllegalStateException.class, () -> response.getEntity());

Review comment:
       I think that this is an incorrect behavior.  Per the javadocs at https://javadoc.io/static/javax.ws.rs/javax.ws.rs-api/2.1.1/javax/ws/rs/core/Response.html#getEntity-- , the `response.getEntity()` method should not throw an `IllegalStateException` unless the response has been closed or if the entity type is a stream that has been fully consumed.
   > Throws:
   IllegalStateException - if the entity was previously fully consumed as an input stream, or if the response has been closed.
   
   And I believe that it is expected that the `response.getEntity()` _should_ return the cached entity after calling `readEntity` - see the javadoc from `readEntity(Class)` ( https://javadoc.io/static/javax.ws.rs/javax.ws.rs-api/2.1.1/javax/ws/rs/core/Response.html#readEntity-java.lang.Class- ) :
   > A message instance returned from this method will be cached for subsequent retrievals via getEntity(). Unless the supplied entity type is an input stream, this method automatically closes the an unconsumed original response entity data stream if open. In case the entity data has been buffered, the buffer will be reset prior consuming the buffered data to enable subsequent invocations of readEntity(...) methods on this response.
   
   So my take is that the these lines should be:
    ```suggestion
           assertEquals(str, response.readEntity(String.class));
           assertEquals(str, response.getEntity());
           // ideally we would verify that the input stream that backs the entity was closed, but that the Response was not - however in this case the stream is temporary (created via `convertEntityToStreamIfPossible()`)
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org