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

Mismatch between scalar JSON type, creators, should give InputMismatchException #1404

Closed
cowtowncoder opened this issue Oct 6, 2016 · 6 comments

Comments

@cowtowncoder
Copy link
Member

(follow-up from #1356)

With current 2.9.0-SNAPSHOT, getting a JSON value like boolean, when there's no matching delegating creator, results in InvalidDefinitionException. Should be InputMismatchException.

@nickbabcock
Copy link

Slight regression:

Expecting the following test to pass

@Test
public void shouldBeInvalidDefinitionException() throws IOException {
    final ObjectReader reader = new ObjectMapper().readerFor(BrokenRepresentation.class);
    assertThatThrownBy(() -> reader.readValue("[\"hello\"]"))
        .isInstanceOf(InvalidDefinitionException.class);
}

public class BrokenRepresentation {
    private List<String> messages;

    public BrokenRepresentation(List<String> messages) {
        this.messages = messages;
    }

    @JsonProperty
    public List<String> getMessages() {
        return messages;
    }

    @JsonProperty
    public void setMessages(List<String> messages) {
        this.messages = messages;
    }
}

stacktrace

com.fasterxml.jackson.databind.exc.InputMismatchException: Can not deserialize instance of io.dropwizard.jersey.jackson.JsonProcessingExceptionMapperTest$BrokenRepresentation out of START_ARRAY token
 at [Source: ["hello"]; line: 1, column: 1]

    at com.fasterxml.jackson.databind.exc.InputMismatchException.from(InputMismatchException.java:58)
    at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1354)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1130)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1082)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromArray(BeanDeserializerBase.java:1377)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:174)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:150)
    at com.fasterxml.jackson.databind.ObjectReader._bindAndClose(ObjectReader.java:1624)
    at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1218)
    at io.dropwizard.jersey.jackson.JsonProcessingExceptionMapperTest.shouldBeInvalidDefinitionException(JsonProcessingExceptionMapperTest.java:146)

@cowtowncoder
Copy link
Member Author

Hmmh. This is bit difficult.... I guess it is definition problem, in that there is no way to really instantiate it.
But at the point problem is encountered it is just a mismatch. I can try to figure this out, but not sure how plausible it would be to handle generally.

@nickbabcock
Copy link

My thought process, at a high level, is:

  • Get all the constructors/factory methods used to deserialize an object.
  • If there turns out to be no methods, always throw InvalidDefinitionException, as there is no input that could ever deserialize
  • On the other hand, if there does exist method(s) to allow input to be deserialized, but the given input can't be forced into one of those methods, throw InputMismatchException

Of course, devil is in the details 😈

@cowtowncoder
Copy link
Member Author

@nickbabcock Agreed with semantics. One thing, however, that I have found that adds another wrinkle is that often times it is better to defer failure. In this case, for example, just because a type can not be deserialized (due to lack of creator(s)), exception probably should not be thrown until an instance would be needed -- not right away during resolution of types.

The reason I think deferring is preferred by users is based on past experiences: while I personally prefer early failure, there are often cases where theoretical problem is not practical one. This occurs -- for example -- when information is first collected, but then later on some is discarded. For example a property with a value of non-instantiatable type is @JsonIgnored (or @JsonIgnoreProperties used on referring property). If exception was thrown upon resolving type, there is no way to work around the issue; whereas deferred exception would allow annotations to be added to avoid the problem.

Anyway: that's a lengthy explanation on how early to report the problem. I agree in that problem ought to be reported appropriately when it is necessary.

@cowtowncoder
Copy link
Member Author

@nickbabcock Implemented #1414, see if this improves handling, and what issues remain.

@nickbabcock
Copy link

I can confirm that your changes work 👏

No issues as of now

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