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

JsonNode? missing value should be deserialized as null and not as NullNode #491

Merged
merged 2 commits into from
Sep 3, 2021

Conversation

novtor
Copy link

@novtor novtor commented Aug 26, 2021

Issue: #490

@dinomite
Copy link
Member

dinomite commented Sep 3, 2021

This looks good. Could you rebase this against the 2.13 branch? An issue like this isn't going to trigger new 2.11 releases and I don't think @cowtowncoder is planning to do any more 2.12 releases.

Also, add an entry to the Contributors: section of CREDITS-2.x and send a signed CLA to info at fasterxml dot com, then I can get this merged.

@dinomite dinomite added the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Sep 3, 2021
@dinomite dinomite merged commit 842e2ea into FasterXML:2.11 Sep 3, 2021
@dinomite
Copy link
Member

dinomite commented Sep 3, 2021

Ugh, I meant to push to your branch but accidentally pushed to the 2.11; would you re-open this PR?

Also, I have a commit that I couldn't push, if you'd apply this patch and commit it, please.

491.patch.txt

@cowtowncoder
Copy link
Member

@dinomite One very quick note: Jackson-databind 2.13.0 (post rc2) now has new method:

JsonDeserializer.getAbsentValue()

used to solve issue:

FasterXML/jackson-databind#3214

in which missing (absent) value passed via @JsonCreator annotated constructor became NullNode instead of plain null.

As such this method might make more sense to call than getNullValue() -- semantics differ at least for JsonNode.
In future we'll have to think of whether to similarly change behavior for "referential" types like Java 8 Optional.
But I think semantics here would be for "absent" value more so than "null replacement" (which mostly/only makes sense for primitive values for which null is illegal to pass due to unboxing).

@dinomite
Copy link
Member

dinomite commented Sep 6, 2021

@novtor I both accidentally merge-closed this and forgot to mention you—would you reopen this PR and add that patch?

@novtor
Copy link
Author

novtor commented Sep 8, 2021

@dinomite, here is the PR for 2.13: #500

@cowtowncoder
Copy link
Member

CLA received for @novtor.

@cowtowncoder cowtowncoder removed the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Sep 9, 2021
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.

Missing value of type JsonNode? is deserialized as NullNode instead of null
3 participants