jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

Deserializing empty JsonUnwrapped objects

Open ogmios-voice opened this issue 8 years ago • 15 comments

Using Jackson 2.8.9 based on the following example, I need an option (e.g. a new attribute for JsonUnwrapped) to specify whether Jackson should instantiate upon deserialization an empty unwrapped object (now) or not:

public class JsonUnwrapTest {
    @JsonInclude(Include.NON_DEFAULT)
    static class Container {
        @JsonUnwrapped
        Unwrapped u;
    }

    @JsonInclude(Include.NON_DEFAULT)
    static class Unwrapped {
        String s;
    }

    @Test
    public void unwrap() throws IOException {
        // GIVEN
        final ObjectMapper m = new ObjectMapper();
        final Container c1 = new Container();
        c1.u = new Unwrapped();
        final Container c2 = new Container(); // c2.u == null
        final String s = m.writeValueAsString(c1);
        Assert.assertEquals(s, m.writeValueAsString(c2));
        // WHEN
        final Container res = m.readValue(s, Container.class);
        // THEN
        Assert.assertNotNull(res.u);
    }
}

Right now the Unwrapped object is created regardless whether it was present before serialization or not. But this might not be always the desired behaviour.

Since Jackson deserializes the objects from inside out (i.e. first it will set the properties of the Unwrapped object, and only then it will set the value of the Unwrapped field in the Container object), adding support in the code for an annotation/attribute to create a new unwrapped object only on demand (i.e. when it must set a non-default value for any of its fields) might as well resolve this problem. (of course this is just an observation w/o any deep insight into the source code)

Update: unfortunately it is not even possible to specify a different setter for the unwrapped field (e.g. with JsonSetter) in the container -- or at least it has no effect --, so the only workaround till the fix might be to: check for an empty Unwrapped object in the default setter (of course an excess object is still created this way), and use a separate setter anywhere else in the application.

ogmios-voice avatar Jul 22 '17 11:07 ogmios-voice

Ok; I can see how RFE for "avoid setting unwrapper object if no properties were found" makes sense, and may be possible to implement.

Part about setter is interesting -- my first instinct was to say it should just work, but then I remembered the problematic part of these being anonomymous, essentially, and there being code to try to avoid conflicts. So it may be that setter does not quite work as expected. It would make sense, I think. to have test case that shows the expected functioning usage (also note that while @JsonSetter may be used, @JsonProperty works same way when used for setter).

cowtowncoder avatar Jul 25 '17 18:07 cowtowncoder

This would be very useful for me; I am using this to parse large flat JSON objects into a DTO, and currently am not able to easily ask "are any of the fields in this @JsonUnwrapped field present?".

edobry avatar Oct 09 '19 20:10 edobry

It's 2023 and they still haven't implemented this? Slutty lol

marcusvoltolim avatar Jan 24 '23 22:01 marcusvoltolim

Yes, how dare people not implement things you want, @marcusvoltolim. What is world coming to. How dare they.

cowtowncoder avatar Jan 26 '23 01:01 cowtowncoder

Yes, how dare people not implement things you want, @marcusvoltolim. What is world coming to. How dare they.

LoL, no problem, I will fix it soon!

marcusvoltolim avatar Jan 31 '23 03:01 marcusvoltolim

@marcusvoltolim Looking forward to your solution (no snark intended)!

cowtowncoder avatar Feb 01 '23 04:02 cowtowncoder

+1 for this feature, particularly useful for CSVs where @JsonUnwrapped is commonly needed. As a workaround the nested object can have a @JsonCreator method that returns null when all fields are null – but would be much nicer to just have a flag on @JsonUnwrapped instead.

phraktle avatar Dec 11 '25 10:12 phraktle

@JacksonJang Something you might be interested in trying to implement?

cowtowncoder avatar Dec 11 '25 16:12 cowtowncoder

@cowtowncoder Thank you for mentioning me.

Edit: I will add an option for @JsonUnwrapped that allows distinguishing whether any actual values were present in the input, so that an instance is created only when needed

I will update the PR soon.

JacksonJang avatar Dec 11 '25 21:12 JacksonJang

@cowtowncoder

Would the name ACCEPT_EMPTY_UNWRAPPED_AS_NULL be appropriate for this option?

With this option enabled, my understanding is that the unwrapped object should remain null whenever no corresponding values are present in the JSON, rather than being instantiated as an empty object.

JacksonJang avatar Dec 11 '25 21:12 JacksonJang

We can start with that, and see how PR forms. Easy enough to change name if necessary.

Good thinking wrt backwards-compatibility!

cowtowncoder avatar Dec 11 '25 23:12 cowtowncoder

@cowtowncoder

I’d like to confirm my understanding: should the implementation be adjusted so that this test case passes as written?

public class UnwrappedEmptyAsNull1709Test extends DatabindTestUtil
{

    @JsonInclude(JsonInclude.Include.NON_DEFAULT)
    static class Container {
        @JsonUnwrapped
        public Unwrapped u;
    }

    @JsonInclude(JsonInclude.Include.NON_DEFAULT)
    static class Unwrapped {
        public String s;
    }

    @Test
    public void testEmptyUnwrappedCreatesInstanceByDefault() throws Exception {
        final ObjectMapper MAPPER = JsonMapper.builder()
                .enable(DeserializationFeature.ACCEPT_EMPTY_UNWRAPPED_AS_NULL)
                .build();

        Container c1 = new Container();
        c1.u = new Unwrapped();
        Container c2 = new Container();

        String json = MAPPER.writeValueAsString(c1);
        assertEquals(json, MAPPER.writeValueAsString(c2));

        Container result = MAPPER.readValue(json, Container.class);
        assertNotNull(result);
        assertNull(result.u);
    }
}

Edit: Ah, I see.. so the expected behavior is that the container itself is created, but the unwrapped property is set to null.

In that case, the assertions should be:

assertNotNull(result);
assertNull(result.u);

JacksonJang avatar Dec 12 '25 10:12 JacksonJang

I’ll open a tofix PR for this and update the code.

JacksonJang avatar Dec 12 '25 10:12 JacksonJang

@cowtowncoder Thank you for mentioning this once again :)

I’ve just pushed an update. I would appreciate it if you could take a look when you have a moment.

JacksonJang avatar Dec 12 '25 12:12 JacksonJang

UGGGH. This is my bad -- I should have read this with more thought. I misunderstood what was being asked.

Request was also for an annotation attribute, which would be more work than DeserializationFeature.

I am also unsure about the original test since its last check:

        // WHEN
        final Container res = m.readValue(s, Container.class);
        // THEN
        Assert.assertNotNull(res.u);

does not make sense to me (shouldn't it have assertNull ? ) I also assumed we already had checked in failing test for this one but we haven't. So I am not sure requirements are clear enough for implementation.

But what I think might make sense is this:

  1. If there are ANY properties with ANY values (nulls or not) for properties of Unwrapped POJOs, regular (current) handling occurs
  2. Otherwise, Unwrapped POJO is considered "empty" and configurable handling would take place (allow for null result for Unwrapped value.

One possibly sizable complication: a POJO can contain more than one property with @JsonUnwrapped (see testDoubleUnwrapping() of TestUnwrapped.java) -- but there is just one UnwrappedPropertyHandler. I am not sure how to tell apart which unwrapped property name is used for which unwrapped POJO value.

So: before considering implementation, it'd be necessary to make sure we know the ask; start with proper tests to verify implementation.

cowtowncoder avatar Dec 12 '25 18:12 cowtowncoder