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

JsonMappingException when field is not present #8

Closed
skizzybiz opened this issue Mar 21, 2015 · 13 comments
Closed

JsonMappingException when field is not present #8

skizzybiz opened this issue Mar 21, 2015 · 13 comments

Comments

@skizzybiz
Copy link

It looks like the library isn't designed to handle null values for a protobuf-based field. See this exception:

com.fasterxml.jackson.databind.JsonMappingException: Can not deserialize instance of com.[...]$CameraPathFrame$Builder out of null token
 at [Source: {
  "type": "[...]"
}
; line: 3, column: 1]
    at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:148)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:749)
    at com.fasterxml.jackson.databind.DeserializationContext.mappingException(DeserializationContext.java:745)
    at com.hubspot.jackson.datatype.protobuf.ProtobufDeserializer.populate(ProtobufDeserializer.java:65)
    at com.hubspot.jackson.datatype.protobuf.ProtobufDeserializer.deserialize(ProtobufDeserializer.java:52)
    at com.hubspot.jackson.datatype.protobuf.ProtobufDeserializer.deserialize(ProtobufDeserializer.java:31)
...

In this example, the source JSON doesn't include the "address" field (which is based on CameraPathFrame) intentionally, as it's meant to be optional.

@skizzybiz
Copy link
Author

I'd like to contribute a fix, but I'm not sure what Basepom is -- googling for it doesn't help.

@jhaber
Copy link
Member

jhaber commented Mar 21, 2015

That's odd, this branch should handle that case. Can you provide the full JSON string being deserialized as well as the protobuf definitions? Also which version of the library are you using?

Basepom is just the parent pom that the project uses, it provides a bunch of preconfigured maven plugins to reduce boilerplate. It's still just a normal maven project that can be compiled with mvn clean package on the commandline (you may need to set the JAVA7_HOME environment variable)

@skizzybiz
Copy link
Author

Figured it out! I made a small test case: https://gist.github.com/skizzybiz/7fe2537641bb157b868a

The problem is when I have only the main constructor designated as @JsonCreator with an annotation. I'm not very familiar with Jackson, so I'm not sure whether this is correct behavior.

@jhaber
Copy link
Member

jhaber commented Mar 21, 2015

The gist references wrapper.json but I don't see that file listed

@skizzybiz
Copy link
Author

Whoops, updated. It's just an empty object.

@jhaber
Copy link
Member

jhaber commented Mar 21, 2015

Hmm, to me it looks like a bug in Jackson. An empty object like that should be represented by a START_OBJECT token followed by an END_OBJECT token, in which case the deserializer would return an empty protobuf. But the JsonParser that Jackson passes to our ProtobufDeserializer has already been advanced to the END_OBJECT token, which doesn't seem correct to me. I can try to work around the issue, but your test won't pass as written. An empty Test.Foo will be passed to the constructor (ie, equivalent to Test.Foo.newBuilder().build()), it won't pass a null value.

@jhaber
Copy link
Member

jhaber commented Mar 22, 2015

Fixed and added some tests for @JsonCreator in d74de98. I just released as version 0.7.0

@jhaber jhaber closed this as completed Mar 22, 2015
@jhaber
Copy link
Member

jhaber commented Mar 22, 2015

@cowtowncoder is the observed behavior with @JsonCreator expected? When using a single-argument @JsonCreator constructor (something like this), the JsonParser passed to the delegate deserializer has already been advanced past the START_OBJECT token.

@cowtowncoder
Copy link

@HiJon89 Yes, unfortunately it is. This is one of not-well-documented parts: an Object deserializer may get START_OBJECT, or the token immediately after that. This is why BeanDeserializer has part that checks if parser points to START_OBJECT, and if so (but only then!) advances it to next token.
It is not guaranteed to have been consumed, but may have been. Not optimal.

I forget why such handling was included in the first place; my guess is that there is a case where allowing this will remove the need for buffering for sake of only including virtual START_OBJECT.
Regardless of the original reason, this behavior has been around for long enough (since @JsonCreator, most likely) that it unfortunately has to be considered to be "just the way things area:.

@camerondavison
Copy link

Did this get fixed in jackson 2.5? I am unable to parse something with 'field:{}' in it

@jhaber
Copy link
Member

jhaber commented Jul 22, 2015

@a86c6f7964 are you referring to an issue with jackson-datatype-protobuf or just a general Jackson problem when writing your own deserializer?

@camerondavison
Copy link

When using 0.7.0 of jackson-datatype-protobuf, and jackson version 2.5 a json that looks like {field:{}} and a protobuf definition that looks something like message OO { optional string s = 1; } message O { optional OO field = 1; } will not parse the json correctly. But, when I go back to 0.6.0 it works fine.

@jhaber
Copy link
Member

jhaber commented Jul 22, 2015

Ah I see the issue, I just fixed it in bf378ab I'm releasing as 0.9.2 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

4 participants