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

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

andymc12 commented on pull request #866:
URL: https://github.com/apache/cxf/pull/866#issuecomment-957687969


   @reta I see your point, and I agree that this change gets us closer to Jersey's behavior.  I think I don't like the overall architecture (JAX-RS spec, not CXF) as it basically says things are different on the client vs server.  For example:
   ```
   Response r1 = Response.ok("foo").build();
   assert "foo".equals(r1.readEntity(String.class));
   assert "foo".equals(r1.getEntity()); // since foo is not backed by an input stream, it must be cached
   // it's sort of like a safeguard in case you lost a reference to the entity but still have a reference to the Response
   
   // but on the client side...
   Response r2 = someTarget.request().get();
   assert "foo".equals(r2.readEntity(String.class);
   assert exceptionThrown(IllegalStateException.class, () -> r2.getEntity());
   
   // further... (unless I'm misreading the Jersey code)
   Response r3 = someTarget.request().get();
   assert r3.bufferEntity();
   assert "foo".equals(r3.readEntity(String.class);
   assert r3.getEntity() instanceof InputStream; // what?! not String??
   ```
   
   In some cases the `Response` object is a permanent holder of the entity, but in others it is not. And "permanent" might change depending on methods called on the `Response` object - other than `close`.  It might also depend on methods called on the stream... for example:
   ```
   Response r4 = someTarget.request().get();
   InputStream is = r4.readEntity(InputStream.class);
   assert r4.getEntity() != null && r4.getEntity() instanceof InputStream; // can even call it twice because the stream is not consumed
   fullyConsume(is);
   assert exceptionThrown(IllegalStateException.class, () -> r4.getEntity());
   ```
   
   Personally, I don't like this inconsistency.  I think that `Response` should either be a permanent (until closed) holder of entities or it should not be a holder at all (i.e. once `readEntity` is called, the response no longer has an entity - the caller must keep a reference to it - but the response doesn't have to be closed - callers could still check things like status code or headers).  But I think the current described behavior is confusing - for users and implementors.  
   
   I'll plan to take this up with the JAX-RS community, but for now, I think this change works.  Thanks (and sorry for the rant! :) ). 


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