mkarg commented on a change in pull request #52: JOHNZON-281 JAX-RS Provider
MUST throw NoContentException
URL: https://github.com/apache/johnzon/pull/52#discussion_r328639525
##########
File path:
johnzon-jsonb/src/main/java/org/apache/johnzon/jaxrs/jsonb/jaxrs/JsonbJaxrsProvider.java
##########
@@ -187,7 +189,23 @@ public long getSize(final T t, final Class<?> type, final
Type genericType, fina
@Override
public T readFrom(final Class<T> type, final Type genericType, final
Annotation[] annotations, final MediaType mediaType,
- final MultivaluedMap<String, String> httpHeaders, final
InputStream entityStream) throws IOException, WebApplicationException {
+ final MultivaluedMap<String, String> httpHeaders, InputStream
entityStream) throws IOException, WebApplicationException {
+ if (entityStream.markSupported()) {
Review comment:
@rmannibucau Thank you for the quick review. :-)
1. Actually I tried it out with the following result: Payara 4 really
provides a markable stream, pure Jersey does not (or vice versa, I haven't
actually written down), so dropping the mark case would _actually_ turn down
performance at least for *some* configurations. So I would vote for keeping
this case to provide *best possible* performance in each particular enviroment.
2. I will look into that, but quickly answered this sounds complex,
error-prone and slow (try-catch IMHO is slower than double buffering). What
would be the _actual_ benefit compared to my original proposal?
3. The intention of *this* PR is to provide JAX-RS 2.1 compliance **ASAP**.
I see your point here and would love to have a more sophisticated solution,
which e. g. checks if the requested class would allow creation of empty
instances or not. But as the currently wrong code does not have this solution,
and as it sounds non-trivial and slow, I think we must spend rather long time
to get that attempt right _and_ fast _and_ bug-free. So IMHO we should do that
in a _later_ PR _ontop_ of this one.
4. What is a test? ;-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
