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

@JsonInclude(NON_DEFAULT) doesn't omit null fields #1351

Closed
cowwoc opened this issue Aug 30, 2016 · 8 comments
Closed

@JsonInclude(NON_DEFAULT) doesn't omit null fields #1351

cowwoc opened this issue Aug 30, 2016 · 8 comments
Milestone

Comments

@cowwoc
Copy link

cowwoc commented Aug 30, 2016

Jackson 2.8.0

  1. I set ObjectMapper.setSerializationInclusion(NON_DEFAULT).
  2. I have a POJO without a default constructor containing a double field and a LocalDate field. The fields are set to 0 and null respectively.
  3. When I serialize the POJO, the null date gets serialized but the double gets omitted.

I am expecting both fields to get omitted because both contain the type's default value.

@cowwoc
Copy link
Author

cowwoc commented Aug 30, 2016

I can reproduce the same problem if I replace the date with a String and even if I set @JsonInclude(NON_DEFAULT) on the null field (it still gets serialized).

@cowtowncoder
Copy link
Member

I'd need more complete explanation to make sure I understand. But as to NON_DEFAULT, it has two distinct modes:

  1. When used for specific class, it will try to use default values for properties as assigned by the default no-arguments constructor: one must exist.
  2. If used on property or as global default inclusion, value to check are default values for the type (in case of scalars, value that a class member would be initialized to; for other types, same as "empty" value)

So with global defaults, excluded value for double should be just 0.0. For LocalDate I would except null or one with timestamp of 0; although that depends on LocalDate serializer defining that rule (via isEmpty()).

Put another way, rules for LocalDate and String should be identical to NON_EMPTY unless annotation was used on POJO class that defines properties.

@cowwoc
Copy link
Author

cowwoc commented Aug 30, 2016

Here is a minimal testcase for your review:

import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectWriter;

public class Main {

    public final String first;
    public final double second;

    public Main(String first, double second) {
        this.first = first;
        this.second = second;
    }

    public static void main(String[] args) throws JsonProcessingException {
        ObjectMapper om = new ObjectMapper().setSerializationInclusion(Include.NON_DEFAULT);
        ObjectWriter writer = om.writerWithDefaultPrettyPrinter();
        String result = writer.writeValueAsString(new Main(null, 0));
        System.out.println("result: " + result);
    }
}

This outputs:

result: {
  "first" : null
}

I am expecting it to output:

result: {}

Does this help?

@cowtowncoder
Copy link
Member

@cowwoc Thanks, it does: result seems wrong to me indeed.

For sake of completeness, I just released 2.8.2 yesterday, so maybe worth checking out. Otherwise I'll look into this when I get a chance. Thank you for reporting the issue.

@cowwoc
Copy link
Author

cowwoc commented Aug 30, 2016

Just checked, 2.8.2 has the same behavior. Thanks for confirming this is a bug.

@cowtowncoder
Copy link
Member

Hmmh. I can see where this occurs but looks like solution may get trickier than I thought.
There is the case of using NON_DEFAULT on a class, and in that case null or empty String are only to be suppressed if that's the default value for property. This should not apply here, since setting is the global default, but somehow does. Trying to figure out what gives.

@cowtowncoder cowtowncoder removed the 2.8 label Sep 9, 2016
@cowtowncoder cowtowncoder added this to the 2.8.3 milestone Sep 9, 2016
@cowtowncoder
Copy link
Member

Needed bit bigger changes than I hoped for, but this is now fixed for 2.8.3 and should reliably detect cases where null should be suppressed.

@arifogel
Copy link
Contributor

This is not fixed for me. I have created a pull request #1416 with a modified version of the test @cowtowncoder added that still fails.

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

3 participants