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

Problem with Object id handling, explicit null token #1150

Closed
wants to merge 2 commits into from

Conversation

xavitorrens
Copy link

According to #742, it shouldn't throw an exception if the value of the property is null

@cowtowncoder cowtowncoder changed the title According to [databind#742], it shouldn't throw an exception if the v… According to #742, StringDeserializer shouldn't throw an exception for null Mar 4, 2016
@cowtowncoder
Copy link
Member

I'd like bit more explanation here, as well as unit test showing in what cases should this be a problem.
Deserializers are typically never called with null tokens, as it is caller's responsibility to handle that (for root value handling it's mapper/reader, for POJO/Collection/Map properties, deserializer for that structured type)

@xavitorrens
Copy link
Author

The problem is that the JSON I want to deserialize has a null id {"id":null, "text1":"value1", .. }, and if it's defined in the bean as @JsonIdentityInfo(generator = ObjectIdGenerators.PropertyGenerator.class, property = "id"), then it throws the exception I mentioned before.

I've fixed it with a Stringify parametrization, so all the null attributes of my JSON are removed before the deserialization. Anyway, I don't know if it's the best way to fix this issue.

@cowtowncoder
Copy link
Member

@xavitorrens As I said, without a reproducible test case I can not help more -- it is possible there is a flaw, but I would need help in knowing where. Standard default deserializer does not have to deal with nulls, but it sounds like this is related to Object Id handling, which does reorder things, and may be accidentally exposing nulls. It is not something you should have to handle.

@xavitorrens
Copy link
Author

I've made a test for reproduce the case described in this case. Of course the test pass with no problems in this branch because the correction has done.

@cowtowncoder
Copy link
Member

Thanks! I think I'll just cut'n paste it to 2.7 branch to see what needs to be done.

@cowtowncoder cowtowncoder changed the title According to #742, StringDeserializer shouldn't throw an exception for null Problem with Object id handling, explicit null token Mar 9, 2016
@cowtowncoder cowtowncoder added this to the 2.7.3 milestone Mar 9, 2016
@cowtowncoder
Copy link
Member

Ok I solved the problem shown: if I am right, it's simply just matter of moving check bit earlier and based on current token, instead of relying on deserializer to handle it. This is better in that deserializers are not expected to handle nulls (for simplicity, originally, but at this point it is just the documented behavior followed by all standard deserializers -- if I was to redesign API I might not to do it that way because it also prevents deserializers from being able to coerce nulls into anything else).

@xavitorrens
Copy link
Author

Thank you very much. I see your point, it seems better.

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

Successfully merging this pull request may close these issues.

3 participants