When Include.NON_DEFAULT setting is used, isEmpty() method is not called on the serializer

[teodord]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

Documentation says that when Include.NON_DEFAULT is used, null, empty and default values of a property are skipped on serialization. It is not clear how the empty check is done because the isEmpty method of the serializer is called only when Include.NON_EMPTY is used explicitly.

I have a case where I set Include.NON_DEFAULT globally on the ObjectMapper, and I have implemented a custom serializer to override the isEmpty method and thus let Jackson know when my object is empty. But my custom implementation of the isEmpty method is not called.
The only way to have it called by Jackson is to explicitly override the property inclusion configuration and use NON_EMPTY instead of the global NON_DEFAULT.

Version Information

2.15.3

Reproduction

<-- Any of the following

1. Brief code sample/snippet: include here in preformatted/code section
2. Longer example stored somewhere else (diff repo, snippet), add a link
3. Textual explanation: include here
   -->

// Your code here

Expected behavior

No response

Additional context

No response

[JooHyukKim]

Could you provide simple reproduction, with Mapper configuration, class declarations and such?

I tried writing a reproduction as below, but it seems to behave differently than the decription above.

    public static class Foo {
        public String getFoo() {
            return "";
        }
    }

    @Test
    public void test86() throws IOException {
        ObjectMapper mapper = JsonMapper.builder()
                .serializationInclusion(JsonInclude.Include.NON_DEFAULT)
                .build();
        String json = mapper.writeValueAsString(new Foo());
        assertEquals("{}", json);
    }
    ```

[teodord]

I probably did not make myself clear, but my case is about a custom serializer overriding the isEmpty method, which does not get called when NON_DEFAULT is used. It gets called only when NON_EMPTY is used, although documentation says NON_DEFAULT also excludes empty values.

I attached a small Maven project demonstrating the issue. It works if we uncomment the NON_EMPTY annotation in Foo.java at line 9.

jackson-test.zip

[JooHyukKim]

Documentation says that when Include.NON_DEFAULT is used, null, empty and default values of a property are skipped on serialization.

Hmmm, the documentation is somewhat context-driven, meaning the behavior varies depending on context, POJO vs non-POJO, etc....

Could you check again, if it really matches your case? I attached the documenation.

[JooHyukKim]

From what I read the documenation, one thing for sure is that NON_DEFAULT is not a superset of NON_EMPTY or vice versa.~~ ~~How about using NON_EMPTY` if possible? It seems more straight forward 🤔?

[teodord]

I am not sure why you say NON_DEFAULT is not a superset of NON_EMPTY. In the documentation snippet you posted above, I am clearly in this case:

I have set the NON_DEFAULT globally, on the ObjectMapper and so I qualify for this case.
First bullet says: "All Values considerer empty as per NON_EMPTY are excluded". So it thus confirms NON_DEFAULT is a superset of NON_EMPTY.
What do I get wrong here?
Furthermore, your own test showed that when NON_DEFAULT is used globally, empty strings are excluded.
What does not work is when custom serializer is used.

[JooHyukKim]

Furthermore, your own test showed that when NON_DEFAULT is used globally, empty strings are excluded.
What does not work is when custom serializer is used.

Thank you for pointing out. I sort of drifted to a wrong point.

You are right that isEmpty() is not called with custom serializer.

[teodord]

Thank you for confirming, but is this a bug or it was meant to work this way and documentation is faulty or incomplete?

[JooHyukKim]

Bug most likely.
But not sure when the fix will be, or which version.

[JooHyukKim]

Here is the missing reproduction

public class Test4464Test {
    public static class BarSerializer extends StdScalarSerializer<Bar> {
        
        public BarSerializer() {
            super((Class<Bar>) null);
        }

        @Override
        public void serialize(Bar value, JsonGenerator gen, SerializerProvider provider) throws IOException {
            gen.writeObject(value);
        }

        @Override
        public boolean isEmpty(SerializerProvider provider, Bar value) {
            return Bar.I_AM_EMPTY.equals(value.getName());
        }
    }

    public static class Bar {
        public static final String I_AM_EMPTY = "I_AM_EMPTY";

        public String getName() {
            return I_AM_EMPTY;
        }
    }

    public static class Foo {
        //@JsonInclude(Include.NON_EMPTY)
        @JsonSerialize(using = BarSerializer.class)
        public Bar getBar() {
            return new Bar();
        }
    }

    @Test
    public void test86() throws IOException {
        ObjectMapper mapper = JsonMapper.builder().serializationInclusion(JsonInclude.Include.NON_DEFAULT).build();
        String json = mapper.writeValueAsString(new Foo());
        assertEquals("{}", json);
    }
}

[cowtowncoder]

Thank you @JooHyukKim -- merged for inclusion in 2.18.0.

[pjfanning]

@JooHyukKim @cowtowncoder this seems likely to have led to #4741 - is there any chance that this change could be made configurable so that users can get the old behaviour if they prefer it?

[JooHyukKim]

Thank you for investigating, @pjfanning!

Awkward situation here, the intention was to get behavior sync with what the documentation says it would.

Considering it being a regression? Yes, I think we could offer the old behavior configurable (SemVer..)

A bit worried about maintainability tho, I hope who issued #4741 could take your suggestion of using Always.

Considering the change was to actually get the right behavior --again as per documentation -- we might need some thinking on Jackson 3 tho.

[cowtowncoder]

I am bit skeptical of technical feasibility of configurability option. At the same, regression is unfortunate.

As to semantics, I am not sure why the claim wrt "isEmpty()" was made in documents: to me, DEFAULT really means either:

1. Default value assigned to property of a class (when used for class) -- and this matches empty case if (and only if) default value is considered empty value -- but the check would be done with equality and NOT with isEmpty().
2. In case of property it should be wrt equality of default instance (one constructed with no-args constructor, or, for primitive types well-known default value). Similarly, no isEmpty() calls

So no, NON_DEFAULT should NOT be considered a super-set of NON_EMPTY at all. It is more dis-joint.

Given all of these, I wonder if we should actually revert change wrt this issue.

EDIT: Actually re-reading Annotation Javadocs for @JsonInclude.Include reminded me of the logic. And while it could be argued behavior is bit weird, I think it is what we have.
And I think we can fix implementation to behave as documented via #4744 (fixing #4741).

[JooHyukKim]

Given all of these, I wonder if we should actually revert change wrt this issue.

If we get intended and right behavior back on production? Yes.

I'd like to suggest we get the correct behavior working, then provide workaround for use cases expecting different(unintended) behavior. Because it seems like the documentation is being interpreted in different perspective.

[cowtowncoder]

@JooHyukKim I was bit too hasty there; I don't think we need to revert behavior, fortunately. Thank you and @pjfanning for help here.