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

Json Merge set null (remove) value for merge pathes (following #1399 and #1437) #1598

Closed
Tetradeus opened this issue Apr 9, 2017 · 3 comments
Milestone

Comments

@Tetradeus
Copy link

Tetradeus commented Apr 9, 2017

Version : 2.9 r2

Related to #1399 and #1437

Wouldn't it be nice to have an option to enable setting null values at merge pathes ? Ref #1437

Presently in tests about Json merge :

    // Also: nulls better not override
    public void testBeanMergingWithNull() throws Exception
    {
        Config config = MAPPER.readerForUpdating(new Config(5, 7))
                .readValue(aposToQuotes("{'loc':null}"));
        assertNotNull(config.loc);
        assertEquals(5, config.loc.a);
        assertEquals(7, config.loc.b);
    }

What if I really want to be able to set null for loc ? assertNull(config.loc);
Not totally sure, but this would make it Json Merge Patch compatible.

@cowtowncoder
Copy link
Member

Sounds reasonable. May need to re-wire handling of @JsonMerge quite significantly but other than that.

@cowtowncoder
Copy link
Member

cowtowncoder commented Apr 18, 2017

Toying with an alternate idea of combining this with @JsonSetter(nulls=Nulls.SET) (vs Nulls.SKIP).
On one hand, that setting already exists, and would work well for per-property annotations.
But on the other hand might be bit messy for defaulting (per-type, globals); and needs of merging may be different from use for straight "regular" deserialization.

@cowtowncoder cowtowncoder added this to the 2.9.0.pr3 milestone Apr 18, 2017
@cowtowncoder
Copy link
Member

Actually going with @JsonSetter route makes tons of sense because that is fully wired already, and conversely trying to use different settings for merge/non-merge case would be a major PITA.

So: changes I will make are:

  1. Default for merge for nulls is same as regular deserialization setting: that is, just set it as null (or replacement null value as case may be)
  2. To skip nulls during merging, JsonSetter.Value must be used at one of 3 levels (property annotation, per value-type annotation, global defaults) -- Nulls.SKIP will achieve this.

And with that it will be possible to make nulls overwrite non-null values.

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