Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message when @JsonValue and @JsonProperty are applied to the same class #498

Closed
cowwoc opened this issue Jul 4, 2014 · 6 comments

Comments

@cowwoc
Copy link

cowwoc commented Jul 4, 2014

Jackson 2.4.1 (data-bind 2.4.1.1)

I attempted to deserialize ["http://example.com/"] using:

public static final class GetAuthentications
{
    private final List<URI> authentications;

    /**
     * Creates a new GetAuthentications.
     * <p/>
     * @param authentications the list of authentications
     * @throws NullPointerException if authentications is null
     */
    @JsonCreator
    public GetAuthentications(@JsonProperty("authentications") List<URI> uthentications)
        throws NullPointerException
    {
        Preconditions.requireThat(authentications, "authentications").isNotNull();
        this.authentications = ImmutableList.copyOf(authentications);
    }

    /**
     * @return the list of authentication tokens
     */
    @SuppressWarnings("ReturnOfCollectionOrArrayField")
    @JsonValue
    public List<URI> getAuthentications()
    {
        return authentications;
    }
}

This resulted in Jackson throwing:

com.fasterxml.jackson.databind.JsonMappingException: Can not deserialize instance of com.vtlr.backend.entity.AuthenticationsEntities$GetAuthentications out of START_ARRAY token
 at [Source: org.glassfish.jersey.message.internal.EntityInputStream@26c613d1; line: 1, column: 1]
    at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:164)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:749)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:745)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromArray(BeanDeserializerBase.java:1210)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:151)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:126)
    at com.fasterxml.jackson.databind.ObjectReader._bind(ObjectReader.java:1232)
    at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:676)
    at com.fasterxml.jackson.jaxrs.base.ProviderBase.readFrom(ProviderBase.java:800)
    at org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$TerminalReaderInterceptor.invokeReadFrom(ReaderInterceptorExecutor.java:257)
    at org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$TerminalReaderInterceptor.aroundReadFrom(ReaderInterceptorExecutor.java:229)
    at org.glassfish.jersey.message.internal.ReaderInterceptorExecutor.proceed(ReaderInterceptorExecutor.java:149)
    at org.glassfish.jersey.message.internal.MessageBodyFactory.readFrom(MessageBodyFactory.java:1124)
    at org.glassfish.jersey.message.internal.InboundMessageContext.readEntity(InboundMessageContext.java:851)
    ... 26 more

Once I removed @JsonProperty("authentications") everything worked fine. It took me a couple of hours to figure out what was wrong because the error makes it seem as if the input stream is empty when in fact there was nothing wrong with it.

Is it possible to improve the error message so Jackson says something along the lines of: "@JsonValue and @JsonProperty annotations cannot be applied to the same class. You must remove one of these annotations."?

@cowtowncoder
Copy link
Member

The problem isn't with @JsonValue here, but rather that constructor annotation changes expected structure. @JsonValue only applies to serialization, and problem here is with deserialization.

With @JsonProperty included, assumption is that incoming JSON is an Object, with matching property values. Without that annotation, for a single-argument case, JSON will rather have to type that maps to value of that argument: in this case, a JSON Array.

What error message tries to tell, then, is that it does not know how to map a JSON Array: what is missing is perhaps indication that a JSON Object would work (although in general other types might be acceptable, depending on details).

As always, I am open to suggestions on how to improve error messaging: current message is not as informative as it could be.

@cowwoc
Copy link
Author

cowwoc commented Jul 10, 2014

How about this?

com.fasterxml.jackson.databind.JsonMappingException: Can not deserialize instance of com.vtlr.backend.entity.AuthenticationsEntities$GetAuthentications. @JsonProperty implies the incoming stream is an object, but the stream contained an array instead.
at [Source: org.glassfish.jersey.message.internal.EntityInputStream@26c613d1; line: 1, column: 1]

@cowtowncoder
Copy link
Member

Something along those lines would be an improvement, if anyone can point out where to determine this. The main challenge is that the place where error is reported may not have all pertinent information to determine more accurate cause.

@cowwoc
Copy link
Author

cowwoc commented Jul 10, 2014

Do you have enough information at the point you are currently throwing the JsonMappingException? You should have parsed all the annotations by this point.

@cowtowncoder
Copy link
Member

@cowwoc All the information has long since been processed from annotations, yes, but not all of that information is passed along the calls, so the point where low-level problem is encountered typically lacks enough context to make educated guesses.

Having said that it is possible that enough information is accessible to give more accurate suggestion for problem. Just mentioning that it's hard to know before looking at location.

@cowtowncoder
Copy link
Member

Not sure what if anything could be done here; may be reopened with suggestions (although probably better to file a new issue to keep it closer to head).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants