rmannibucau 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_r328626965
##########
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:
hey @mkarg, I would like to mention these few points before moving forward,
please let me know what you think about it:
1. here the stream will rarely be markable (with cxf it will not most of the
time) so we can surely skip this case
2. we already have a pushback inputstream for the jsonparser so i would
consider adding a callback here we can register through the parser map. High
level it would be a callback propagated through
https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jsonb/JohnzonBuilder.java#L837
from Jsonb default instance to the parser and it would be called
https://github.com/apache/johnzon/blob/156c7af77322fc0d11819756fc48fb18bd617b71/johnzon-core/src/main/java/org/apache/johnzon/core/RFC4627AwareInputStreamReader.java#L51
(this stream reader is created by the parser). The callback can then throw an
exception the readFrom can catch and process in an exceptional way (see point 3)
3. trivial impl of this exceptional way can be throw new NoContentException
but it also makes sense to have a way to respect the quote from the spec you
did in the ticket and return an "empty" object IMHO, I assume it must be an
option of the provider and handled in the catch of point 2 as well
4. you forgot to commit the associated test? ;)
wdyt?
----------------------------------------------------------------
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
