CXF-8518: Fixing jaxrs.spec.provider.standardnotnull clientJaxbProviderTest

[reta]

Per specification:

for a zero-length response entities returns a corresponding
Java object that represents zero-length data. In case no zero-length representation
is defined for the Java type, a {@link ProcessingException} wrapping the
underlying {@link NoContentException} is thrown.

For JAXB serialization / desearialization, the empty input stream (empty response) leads to javax.xml.bind.UnmarshalException, for example:

[com.ctc.wstx.exc.WstxEOFException: Unexpected EOF in prolog
 at [row,col {unknown-source}]: [1,0]]
	at com.sun.xml.internal.bind.v2.runtime.unmarshaller.UnmarshallerImpl.handleStreamException(UnmarshallerImpl.java:470)
	at com.sun.xml.internal.bind.v2.runtime.unmarshaller.UnmarshallerImpl.unmarshal0(UnmarshallerImpl.java:402)
	at com.sun.xml.internal.bind.v2.runtime.unmarshaller.UnmarshallerImpl.unmarshal(UnmarshallerImpl.java:379)
	at org.apache.cxf.jaxrs.provider.JAXBElementProvider.readFrom(JAXBElementProvider.java:177)
	at org.apache.cxf.jaxrs.provider.JAXBElementTypedProvider.readFrom(JAXBElementTypedProvider.java:41)
	at org.apache.cxf.jaxrs.provider.JAXBElementTypedProvider.readFrom(JAXBElementTypedProvider.java:34)
	at org.apache.cxf.jaxrs.utils.JAXRSUtils.readFromMessageBodyReader(JAXRSUtils.java:1444)
	at org.apache.cxf.jaxrs.impl.ResponseImpl.doReadEntity(ResponseImpl.java:472)
	at org.apache.cxf.jaxrs.impl.ResponseImpl.readEntity(ResponseImpl.java:415)
	at org.apache.cxf.jaxrs.impl.ResponseImpl.readEntity(ResponseImpl.java:403)

[reta]

Build is green: https://ci-builds.apache.org/job/CXF/job/CXF-JDK8-PR-Manual/14/

[reta]

@andymc12 as always, need your expertise, mind please taking a look? thanks a lot!

[andymc12]

@reta - we didn't need to make any changes to JAXBElementProvider, so we must've fixed this test case some other way. We did add a doPriv in the unmarshallFromInputStream method - I'll plan to contribute that back shortly. In any case, I think this change looks good. Thanks!

[dblevins]

I like this better than my too-brittle hack :)

• dblevins@3210390

The one concern I'd have is that inputStream.available() is a guess and could potentially return zero when there is a valid payload coming. That might happen in scenarios where the server-side could just be taking a while streaming back the payload.

I wonder if we wrapped the input stream instead of returning null and had the wrapper count the bytes read. We could check the count on an exception and use that to determine our blocking read returned no payload.

It'd be sort of a hybrid of the two approaches. Thoughts?

[reta]

@dblevins thanks a lot for looking, that is correct, we do trust inputStream.available() only if it returns non-zero value

            // if available is 0 it does not mean it is empty
            if (is.available() > 0) {
                return false;
            }

Otherwise the assumption is that stream is not empty (and we proceed further with mark / reset of pushback stream). What do you think?

[reta]

I wonder if we wrapped the input stream instead of returning null and had the wrapper count the bytes read. We could check the count on an exception and use that to determine our blocking read returned no payload.

Thought about that as well but dealing with exceptions and counting the bytes would be more complicated comparing to the simple check that stream is empty.

[dblevins]

@reta You're totally right. I read too quickly and missed that available() is only getting acted upon if the response is a definitive "yes" and there's pretty comprehensive fallback logic in the isEmpty method. I think you have it covered pretty well.

My +1