Clarification about Response.readEntity(XXX)

[NicoNes]

According to the javadoc of:

• Response.readEntity(Class)
• Response.readEntity(javax.ws.rs.core.GenericType)
• Response.readEntity(Class, Annotation [])
• Response.readEntity(javax.ws.rs.core.GenericType, Annotation [])

1. those methods are supposed to close the original entity input stream (unless the supplied type is input stream) and then cache the result for subsequent retrievals via Response.getEntity().
   So it clearly means that those methods MUST not close() the response itself, else subsequent retrievals via Response.getEntity() will always end up with IllegalStateException being thrown.
   Instead, only the underlying input stream should be closed.

2. Subsequent call to one of those methods MUST throw an IllegalStateException if the original entity input stream has already been fully consumed without buffering the entity data prior consuming.

In other words:

Message s1 = r.readEntity(Message.class);
System.out.println(s1.toString());
System.out.println(r.getEntity());      => should not throw no exception
s1 = r.readEntity(Message.class);                     => should throw an IllegalStateException

Is my understanding right for those 2 points ? because it seems that many implementation (https://issues.jboss.org/browse/RESTEASY-2112) including the RI (Jersey) does not behave as exepected for point 1.
If my understanding is wrong what is the intented behavior of those methods ?

[chkal]

IMO your understanding is correct and both Jersey and RESTEasy doesn't behave correctly. But it would be interesting to hear other thoughts about this as well.

[spericas]

These are good point @NicoNes. We definitely need a clarification here and an understanding as to why implementations do not appear to honor the getEntity() semantics here. Any comments @jansupol?

[jansupol]

Javadoc to getEntity says:

If the entity is represented by an un-consumed input stream the method will return the input stream.

That's what Jersey does. It does not return the cached entity, as the entity is never cached.

I am concerned about automatic caching being a performance issue unless the cached entity means a simple reference to a Java entity returned by readEntity. (In that case, if the entity is mutable, readEntity().toString() might differ from getEntity().toString()). I am not sure what the use case of caching the entity would be. Should the user want to read the entity multiple times, bufferEntity() is available to be used. The user is aware of the buffering and the impact on performance and has a choice not to buffer.

The difference between caching and buffering is not clear, which is why the implementations do not honor the javadoc to the letter, IMO.

[NicoNes]

Hi @jansupol ,

Javadoc to getEntity says:

If the entity is represented by an un-consumed input stream the method will return the input stream.

That's what Jersey does. It does not return the cached entity, as the entity is never cached.

Actually javadoc for getEntity first states that:

Get the message entity Java instance. Returns null if the message does not contain an entity body.

IMO it means that if the entity has been read as a Date Java instance for example, using readEntity(Date.class) the getEntity() should return this Date entity not matter its mutability.

Then the doc says:

If the entity is represented by an un-consumed input stream the method will return the input stream.

IMO it means that if the entity has not been read at all using any of readEntity(...) methods, the raw input stream is returned since it's the Java instance representing the raw/not consumed entity.

Then the doc says:

IllegalStateException - if the entity was previously fully consumed as an input stream, or if the response has been closed.

It means for me that, if the entity has not been read as any XXX java type but only used as an input stream (using readEntity(InputStream.class) or getEntity() returning the raw input stream), and if that input stream has been fully consumed (InputStream.read()==-1) => throw IllegalStateException

So in my understanding caching simply means storing and re-using the reference to a Java entity returned by readEntity as it is said in the doc unless I'm missing something:

A message instance returned from this method will be cached for subsequent retrievals via getEntity()

The difference between caching and buffering is not clear, which is why the implementations do not honor the javadoc to the letter, IMO.

Caching is the default behavior and it means: storing the Java entity returned by readEntity. After that, no other invocation of readEntity is possible since the stream has been consumed and closed.
It's the basic case where most of the time response entity stream only has one Java type representation. So no need to read the stream multiple time. Just read it once, store the result and get it as the Java instance as often as you want.

Buffering is optional as you said and is a way to allow multiple read of the response entity stream. It can be usefull when you want to read the stream in many different Java type for example.

That's my opinion and understanding WDYT ?

[NicoNes]

Its not the point of this issue, but following is not totally right

If the entity is represented by an un-consumed input stream the method will return the input stream.

That's what Jersey does. It does not return the cached entity, as the entity is never cached.

In following case I was expecting an IllegalState to be thrown since the entity was previously fully consumed as an input stream but instead getEntity() returns the fully consumed input stream that is not really usefull given its current state:

private static void case1() throws Exception {
	Client client = ClientBuilder.newClient();
	try {
		Response response = client.target("http://localhost:8086/test")
				.request(MediaType.APPLICATION_XML_TYPE).get();
		InputStream inputStream = (InputStream) response.getEntity();
		// Fully consumed the original response stream
		while (inputStream.read() != -1) {
		}
		// Following line Must throw an IllegalStateException but it will
		// not. Instead it will return an input stream already consumed.
		InputStream inputStreamEntity = (InputStream) response.getEntity();
		if (inputStreamEntity.read() == -1) {
			System.err.println("Inputstream entity has already been consumed");
		}
	} finally {
		client.close();
	}
}

Am I wrong ? (Using jersey 2.24)

[jansupol]

@NicoNes Caching term was not clear even back in the JAX-RS 2.0 days. If caching the entity means keeping a reference to the entity, that would be easy to implement. I'd like to see some javadoc warning that if the user would retrieve the entity once, modify it, then the entity would be retrieved once again then the user should not be surprised that it's altered, though.

---

The behavior of the example is as you describe. Strictly speaking, there is no notion of consumption in the InputStream javadoc (Is it consumed when it is not read but closed? Is it consumed when it is read but not closed?). Response javadoc does not clarify that either. (I am not saying there is no bug in Jersey, I am fine with updating it as soon as it is clear how).

BTW, your analysis contradicts itself, too:

If the entity is represented by an un-consumed input stream the method...

IMO it means that if the entity has not been read at all using any of readEntity(...) methods

Here the consumed means used by readEntity

IllegalStateException - if the entity was previously fully consumed as an input stream

and if that input stream has been fully consumed (InputStream.read()==-1) => throw IllegalStateException

Here consumed means to check the InputStream can still be read. The javadoc wording is not so clear.

[NicoNes]

I agree that javadoc wording (un-consumed, fully consumed, consumed) are a bit confusing.

So let's try to make that clear to get a precise idea of what those methods are supposed to do.

getEntity():

I do not want to talk about readEntity(...) yet in details, but in what follow, I assume that readEntity(InputStream.class) returns the message InputStream instance.

I suggest to change the javadoc as follows:

Get the message entity Java instance.
Returns null if the message does not contain an entity body, else behaves as follows:

• if readEntity(...) was previously invoked then this method returns the same Java instance as last succesfull invocation of readEntity(...).
  In the situation where last invocation of readEntity(...) returned an InputStream, this method returns the same InputStream instance if not fully read (i.e. InputStream.read()!=-1) nor closed.
• else returns the message InputStream instance if not fully read (i.e. InputStream.read()!=-1) nor closed.

Returns: the message entity Java instance or null if message does not contain an entity body (i.e. when hasEntity() returns false).
Throws :IllegalStateException - if the entity is retrieved as an InputStream fully read already or closed, or if the response has been closed.

This way this method will always returns something usable/practical (an input stream fully read or closed is no longer usefull):

• In case user has already invoked readEntity(MyObject.class) then getEntity() will return the same MyObject instance.
  If the MyObject instance has been altered after last invocation of readEntity(MyObject.class) the result of getEntity() will reflect those changes since it is the same instance.
• In case user has invoked readEntity(InputStream.class) then getEntity() will return the same InputStream instance only if this object has not been fully read nor closed already. In this case an exception will be thrown.
  If the InputStream instance has been altered after last invocation of readEntity(InputStream.class) the result of getEntity() will reflect those changes since it is the same instance.
• In case user has never invoked readEntity(...) then getEntity() will return the message InputStream instance only if this object has not been fully read nor closed already. In this case an exception will be thrown.
  If the InputStream instance has been altered after last invocation of getEntity() the result of subsequent invocations of getEntity() will reflect those changes since it refers to the message InputStream instance which is unique.

Before talking about readEntity() maybe I can get your thoughts on that guys. Is this acceptable ?

[spericas]

I agree with @NicoNes interpretation of the Javadoc. I think it's important to convert this into actual use cases:

(1) getEntity() -> InputStream

(2) getEntity() -> InputStream, InputStream consumed, getEntity() -> IllegalStateException

(3) readEntity(Foo.class) -> Foo, getEntity() -> Foo

(4) readEntity(Foo.class) -> Foo, readEntity(Bar.class) -> IllegalStateException

(5) bufferEntity(), readEntity(Foo.class) -> Foo, getEntity() -> Foo, readEntity(Bar.class) -> Bar, getEntity() -> Bar

The one caveat to the discussion above (see 2) is that, when an implementation is trying to determine if an input stream is consumed, it should probably use InputStream#available to avoid advancing it.

[NicoNes]

About case 4, I think it's important to be clear that any subsequent invocation of readEntity(xxx.class) whatever the xxx type must throw an IllegalStateException:

(4) readEntity(Foo.class) -> Foo, readEntity(XXX.class) -> IllegalStateException

it should probably use InputStream#available to avoid advancing it.

Yes probably or any other internal mechanism sure 👍

[spericas]

@NicoNes Are you interested in submitting a PR that clarifies the getEntity, readEntity and buffer Entity docs and interactions? Looks like all these javadocs can use some tweaking to clarify this behavior. We need to be precise but also concise since developers often dislike reading pages of Javadocs.

[NicoNes]

@spericas Yes sure I will do it 👍

[NicoNes]

Hi Guys,

Quick update to let you know that it is in progress. We are working on it with @ronsigal who spoted the same confusion in the doc (#706).

[ronsigal]

Hey everyone,

I've started thinking about this stuff again, and I have a couple of comments.

1. I agree with @NicoNes about getEntity() succeeding after a call to readEntity(). The problem is the TCK test

  /*
   * @testName: getEntityThrowsIllegalStateExceptionWhenConsumedTest
   * 
   * @assertion_ids: JAXRS:JAVADOC:123;
   * 
   * @test_Strategy: if the entity was previously fully consumed as an
   * InputStream input stream, or if the response has been #close() closed.
   */
  public void getEntityThrowsIllegalStateExceptionWhenConsumedTest()
      throws Fault {
    Response response = invokeGet("entity");
    response.readEntity(String.class);
    try {
      Object entity = response.getEntity();
      fault("No exception has been thrown entity=", entity);
    } catch (IllegalStateException e) {
      logMsg("#getEntity throws IllegalStateException as expected", e);
    }
  }

I think that should be changed in favor of the behavior we all (I think) agree on.

2. I have a suggestion, which would entail a behavior change, so it's for a future edition. It seems to me that there are two ways for a Client to process the contents of a Response: 1) explicitly reading the contents of a backing InputStream, and 2) using readEntity() to extract a java object. Doing both on the same Response is a bad idea. Based on that observation, I propose that it would be reasonable to do away with throwing an IllegalStateException as in

IllegalStateException - if the entity was previously fully consumed as an input stream

Why? 1) If you're working explicitly with an InputStream, then you have to be looking for end of file anyway. 2) If you mix together reading the backing InputStream and calling readEntity(), then 2a) you get an Exception because the remaining bytes can't be turned into an instance of the Class you want, or 2b) you'll get a partial object like a shortened String, or 2c) you'll get an "empty" version of your desired class, if one exists, or an Exception otherwise. Currently, the semantics would require an IllegalStateException in case 2c, but I think 2c is just a particular case of 2b, which doesn't require an IllegalStateException. I think we should say that mixing the two approaches to getting the contents is a bad idea, and you better know what you're doing if you want to do it. Beyond that, I think the semantics and the code would be simpler if we don't have to throw the IllegalStateException.

WDYT?

[NicoNes]

Hey @ronsigal,

I agree with you on point 1.

About point 2, I agree that things would be clearer without this specific IllegalStateException and it would be great for a future edition.

But before talking about it in details maybe we should clarify the current expected behaviour for this current edition. It will help us later to remove the IllegalStateException you talked about.

Based on what we started a while ago, I've just finish writting a version of what the current doc could be to better clarify getEntity, bufferEntity, readEntity and all methods dealing with entity in the Response class.

I sent it to you first for review.

Thanks

[NicoNes]

Hey guys,

Forget about my last comment. I fully agree with @ronsigal.

I was reading again the version I sent to him and I figured out that few cases, the ones dealing with getEntity() and readEntity() returning InputStream, was not well handled/explained.

I think that Ron is right. The reason why it is so difficult for us to explain things clearly is because in these cases we try to handle InputStream fully consumed (i.e inputStream.read() ==-1) instead of just returning it and let the user handle it.
As Ron said:

If you're working explicitly with an InputStream, then you have to be looking for end of file anyway

So based on Ron analisys, I thought about a new version that behave like that:

Below resetable means that user can invoke reset() on the InputStream. When he reaches end of stream for example (i.e inputStream.read() ==-1).

(1) getEntity() -> InputStream (not resetable)

(2) bufferEntity(), getEntity() -> InputStream (resetable)

(3) readEntity(Foo.class) -> Foo, getEntity() -> Foo

(4) readEntity(Foo.class) -> Foo, readEntity(Bar.class) -> IllegalStateException

(5) readEntity(Foo.class) -> Foo, readEntity(InputStream.class) -> InputStream (not resetable)

(6) bufferEntity(), readEntity(Foo.class) -> Foo, getEntity() -> Foo, readEntity(Bar.class) -> Bar, getEntity() -> Bar, readEntity(InputStream.class) -> InputStream (resetable), getEntity() -> InputStream (resetable)

WDYT ?

[spericas]

@NicoNes Sorry, it's been a while. Could you clarify in (5) the state of InputStream returned by readEntity(InputStream.class)? Is it of any use to support that?

[NicoNes]

Hey @spericas ,

Good catch.

In (5), the InputStream is fully consumed (i.e inputStream.read()==-1) and also closed. So:

(5) readEntity(Foo.class) -> Foo, readEntity(InputStream.class) -> IllegalStateException

The general rule must be that unless bufferEntity() was previously invoked readEntity(...) must not be invoked mulitple times whatever the supplied type. Such cases should throw an IllegalStateException, especially following one:

(7) readEntity(InputStream.class) -> InputStream, readEntity(InputStream.class) -> IllegalStateException

To hilight what Ron was saying about preventing user from using both response entity InputStream and readEntity(...) to process request content, we could state that readEntity(..) always throws an IllegalStateException if response entity InputStream has already been fully or partially consumed:

(8) getEntity() -> InputStream, inputstream.read(), readEntity(...) -> IllegalStateException

WDYT ?

[spericas]

Hey @spericas ,

Good catch.

In (5), the InputStream is fully consumed (i.e inputStream.read()==-1) and also closed. So:

(5) readEntity(Foo.class) -> Foo, readEntity(InputStream.class) -> IllegalStateException

Right.

The general rule must be that unless bufferEntity() was previously invoked readEntity(...) must not be invoked mulitple times whatever the supplied type. Such cases should throw an IllegalStateException, especially following one:

(7) readEntity(InputStream.class) -> InputStream, readEntity(InputStream.class) -> IllegalStateException

Makes sense.

To hilight what Ron was saying about preventing user from using both response entity InputStream and readEntity(...) to process request content, we could state that readEntity(..) always throws an IllegalStateException if response entity InputStream has already been fully or partially consumed:

(8) getEntity() -> InputStream, inputstream.read(), readEntity(...) -> IllegalStateException

WDYT ?

I am not sure we can always detect this condition. It's sometimes difficult to prevent users from shooting themselves on the foot.

[NicoNes]

I am not sure we can always detect this condition. It's sometimes difficult to prevent users from shooting themselves on the foot.

Sure.

I think we agree that response entity InputStream fully consumed or closed is easy to detect with inputStream.read()==-1.

Now if partially consumed it is less obvious. So to keep it simple maybee we should just keep what is already stated by the doc about throwing a ProcessingException if the response entity stream cannot be read as the supplied java type.

Does it sound acceptable to you ?

[ronsigal]

Hi @NicoNes,

On Sunday I saw your comment about working on clarifying things first, and then changing the behavior in a later edition, but I wasn't in working mode, so I didn't respond at the time. Now, I've just come by to say that I agree with YOU. Lolll.

1. For stage 1, maybe the best we can do is clarify the "difference between caching and buffering", which you've done in items 1-7 (with modified 5). But now I'm concerned that the faulty TCK test gets in the way. Changing the test to conform to the javadoc would entail a behavior change. Actually, it would be safer to change the javadoc to match the TCK, i.e., eliminating the caching concept. At first, I thought that sounded terrible, but ... I don't know, maybe not. Is getEntity() following readEntity() even useful? The fact is that, for any implementation that passes the TCK, caching doesn't work anyway. So one possibility is to eliminate the concept of caching and clarify that getEntity() works only when an entity is set explicitly by ok(). I think that that change can be made safely without changing any behavior. If we did that, then I think we should be committed to it. I.e., we probably don't want to change it now and then change it back in a future behavior changing edition. In fact, I think I would go a step further and move bufferEntity() and readEntity() into a separate "client side" subclass of Response, which would keep getEntity() and readEntity() apart.

2. For the future, I would suggest eliminating the need to check the state of a backing InputStream, and I thought you (@NicoNes) agreed with me when you said, "it is so difficult for us to explain things clearly is because in these cases we try to handle InputStream fully consumed (i.e inputStream.read() ==-1) instead of just returning it and let the user handle it", but then you mentioned, "what Ron was saying about preventing user from using both response entity InputStream and readEntity(...) to process request content". Actually, I agree with @spericas when he says, "It's sometimes difficult to prevent users from shooting themselves on the foot." I think we should just rely on the user to either use InputStream or use readEntity(), and we could point out that it's not a good idea to mix them.

3. I see how buffering supports resetting the InputStream, so that's kind of nice. Is it something that would be useful? Just asking.

-Ron

[NicoNes]

Hey @ronsigal ,

I share your analysis and it makes me wonder too if caching support is useful.

• Stage 1:

As you said, since all implementations that passes TCK does not support caching, I think that eliminate this concept is the best things to do.

Is getEntity() following readEntity() even useful

Well, I'm not sure. I personnally use one or the other but not both (it does not work anyway).

So based on what you said, for stage 1 getEntity() javadoc could be:

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 the entity is represented by an input stream not fully consumed already (i.e inputStream.read() != -1), then that input stream is returned.

Returns:
the response entity or null if message does not contain an entity body (i.e. when hasEntity() returns false).

Throws:
IllegalStateException - if the entity is represented by a fully consumed input stream (i.e inputStream.read() == -1), or if the response has been closed.

readEntity() javadoc could remain the same without:

A message instance returned from this method will be cached for subsequent retrievals via getEntity().

• Stage 2:

I don't think that adding the cache concept back will be a good thing.

So getEntity() javadoc could remain the same as in stage 1 without the IllegalStateException and the check on the backing input stream state.

readEntity() javadoc could remain the same as in stage 1 changing the parts about the IllegalStateException with something like:

IllegalStateException - if the entity is not backed by an input stream, the response has been closed already, or if the method has already been invoked without buffering the entity data prior consuming.

This way if user uses both InputStream and readEntity(), readEntity() will either return a partial object or throw a ProcessingException.

• Point 3

I see how buffering supports resetting the InputStream, so that's kind of nice. Is it something that would be useful? Just asking.

If we all agree to remove caching concept and about the behaviour of getEntity() method, I think that instead of returning the resetable InputStream as I said, we could return the reset InputStream.
This way when bufferEntity() has been invoked, both readEntity() and getEntity() behave the same by reseting the entity input stream buffer first.

Does it sound good to you ?

[ronsigal]

@NicoNes, yes, I mostly agree.

Stage 1.

+1. I like that you added "(i.e inputStream.read() != -1)". I was always confused by "consumed" and "fully consumed" here and there. I think it would be reasonable to add that explanation every place "consumed" appears, at least for Stage 1. In Stage 2, the concept of "consumed" goes away, I think.

This is a minor point, but I read

Get the response entity Java instance. Returns null if the response does not contain an entity body.

Otherwise, it behaves as follow:

as "Get the response entity Java instance. ... Otherwise, it behaves as follows." I would do something like

Get the response entity Java instance.

Returns null if the response does not contain an entity body. Otherwise, it behaves as follow:

Stage 2.

+1

Point 3.

I don't understand this part. My feeling is everything would be much clearer if getEntity() is considered to be the getter that matches ok(), etc., and that it should live in a different world than a received entity stream. This would be a change in behavior, since getEntity() can currently return a received entity stream. So, I would have, for example, ok() in Response and bufferEntity() and readEntity() in a subclass.

One concern.

Removing the bit about caching for getEntity() makes the updated version look like a behavior change. Well, technically, I guess it is a behavior change if the semantics is defined by the javadoc and not by the TCK. But, in reality, it shouldn't be a behavior change for any JAX-RS implementation. I think there should be someplace where that is made clear.

[NicoNes]

@ronsigal,

About point 3 I think that I get you mean. Tell me if I'm wrong.

It would be like what is already done with Cookie (representing request cookie) and NewCookie (representing response cookie).
In what you described it will be something like:

• Response that represents response sent (with ok(), getEntity() etc).
  It will be the response type returned by server side methods and used in both client and server filter (abortWith(Response)).
• SubResponseClass that represents response received (with readEntity(), bufferEntity()).
  It will be the response type returned by client side methods

Am I right?

My feeling is that I like the current the semantic (without the cache concept) of getEntity() and readEntity() :

• getEntity() has a get semantic and allows to get the raw entity (either the object set with ok() etc or the backing InputStream) without extra treament. If an entity exists this get method will return it. It works well with hasEntity().
• readEntity() has a read semantic and allows to transform the backing InputStream into a Java object. If there is nothing to transform, no backing InputStream, it will return nothing.

What I meant in point 3 about how bufferEntity() and getEntity() should work together in my previous comment is:

If bufferEntity(), buffers all bytes of the backing InputStream and close it.
And then readEntity(InputStream.class) return a copy of the buffer.
Then getEntity() should also retrun a copy of the buffer.

[ronsigal]

Hey @NicoNes,

re; "Am I right"

Yes

re: "the current semantic (without the cache concept) of getEntity() and readEntity()"

I guess I don't have strong feelings about the issue. I would say

In favor of my version of the semantics:

1. It's simpler, and
2. The ability of getEntity() to return the backing InputStream is redundant since readEntity() can do the same thing.

In favor of your version:

1. It preserves part of the existing semantics, so there's less of a disruption.
2. ??

I think we need more input. Maybe a note on the eclipse-ee4j/jaxrs-api mailing list?

Btw, I'm glad you mentioned hasEntity(). That javadoc should probably be clarified also.

[ronsigal]

As for bufferEntity() and getEntity(), if we keep the ability of getEntity() to retrieve a backing InputStream, then I think your suggestion makes sense. Otherwise, it's moot.

[NicoNes]

Hey @ronsigal

As for bufferEntity() and getEntity(), if we keep the ability of getEntity() to retrieve a backing InputStream, then I think your suggestion makes sense. Otherwise, it's moot.

Yes that's it.

At least for stage 1, since we think it's better to not introduce change behavior, my suggestion is just to clarify relationship between bufferEntity() and getEntity() to be consistent with readEntity() behavior.

So let me try to sum up everything for every one on how we would like to fix the initial issue without introduce no behavior change for this version.

The goal is to remove the cache concept from both getEntity() and readEntity() to macth the existing TCK.

• getEntity() javadoc could 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 an input stream not fully consumed already 
(i.e inputStream.read() != -1), then that input stream is returned.

Returns:
    The response entity or null if message does not contain an entity body 
    (i.e. when hasEntity() returns false).
Throws:
    IllegalStateException - if the entity is represented by a fully consumed 
    input stream (i.e inputStream.read() == -1), or if the response 
    has been closed.

• readEntity() javadoc could remain the same without:

A message instance returned from this method will be cached for 
subsequent retrievals via getEntity().

• Remove consumed, un-consumed, unconsumed everywhere in the javadoc to replace it by fully consumed (i.e inputStream.read() == -1) , not fully consumed (i.e inputStream.read() != -1) or not consumed at all

@ronsigal Once this issue will be closed, I suggest we open and another issue to propose the stage 2 so that we treat them separately. WDYT ?

If it's Ok for you, yes we could submit it to the mailing list (or mention them here) to get more input about it.

Thanks !!

[ronsigal]

Hi @NicoNes,

1. I think your updated javadoc is good. I think there should also be an additional sentence or so to clarify that hasEntity() applies to either an entity object or a backing InputStream.

2. re: "another issue to propose the stage 2"

+1

3. @NicoNes, would you be interested in sending a summary to the mailing list?

[NicoNes]

Hey @ronsigal ,

I think your updated javadoc is good. I think there should also be an additional sentence or so to clarify that hasEntity() applies to either an entity object or a backing InputStream.

Ok, I will add a line about it 👍

@NicoNes, would you be interested in sending a summary to the mailing list?

Sure. What is the email address ?

[ronsigal]

@NicoNes, [email protected]. See https://accounts.eclipse.org/mailing-list/jaxrs-dev.

[ronsigal]

I was just reminded that my original motivation for raising #706 "Clarify javax.ws.rs.core.Response javadoc wrt extracting entities." was to ask for a Response.isClosed() method to make it possible to avoid getting an exception when calling, for example, hasEntity(). This would be an additional, rather than changed, behavior. I wonder if it would be legal to include it in Stage 1.

[NicoNes]

Hey @ronsigal ,

Thanks for the address. I'll suscribe to the mailing list and send the mail to the group.

I wonder if it would be legal to include it in Stage 1.

You're right we almost forget your initial need.
I don't know neither if it would be legal to add it in stage 1 but I'll talk about it in the mail too.

Maybe for stage 1 the quick workaround to add this isClosed() method in a way that JAX-RS vendors do not have nothing to do, could be adding a default implementation in the Response class like following:

	public boolean isClosed() {
		try {
			hasEntity();
			return false;
		} catch (IllegalStateException e) {
			return true;
		}
	}

It's a bit weird I agree, but it works..anyway it's just a proposal.

[ronsigal]

Hey @NicoNes, your isClosed() makes sense, and it makes life a little easier for using all of those methods that throw an exception when the response is closed. +1

[mkarg]

@NicoNes This is a kind reminder that this issue is assigned to you and is planned for the next release of JAX-RS. :-)

[spericas]

Are we still targeting this clarification for 3.1?

[NicoNes]

@spericas I finally take time to do the PR. Hope it is not too late.

Thanks