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

When Include.NON_DEFAULT setting is used on POJO, empty values are not included in json if default is null #4741

Closed
1 task done
ragnhov opened this issue Oct 9, 2024 · 10 comments
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Milestone

Comments

@ragnhov
Copy link

ragnhov commented Oct 9, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

After upgrading to 2.18.0, empty Strings/objects are no longer included in json output if NON_DEFAULT annotation is used on the class and the default value is null. This is possibly related to fixing #4464
As far as I can understand, the documentation states that the test below should pass, as it does in 2.17.1.
image

Version Information

2.18.0

Reproduction

@Test
void testSerialization() throws JsonProcessingException {
    MyString input = new MyString();
    input.setValue("");
    ObjectMapper objectMapper = new ObjectMapper();
    String json = objectMapper.writeValueAsString(input);
    MyString output = objectMapper.readValue(json, MyString.class);
    assertEquals(input.getValue(), output.getValue());
}

@JsonInclude(JsonInclude.Include.NON_DEFAULT)
@Data
public static class MyString {
    private String value = null;
}

Expected behavior

No response

Additional context

No response

@ragnhov ragnhov added the to-evaluate Issue that has been received but not yet evaluated label Oct 9, 2024
@pjfanning
Copy link
Member

pjfanning commented Oct 9, 2024

That annotation has this in the javadoc:

When used for a POJO, definition is that only values that differ from the default values of POJO properties are included.

I don't think this annotation value is about nulls - there are other annotation values you can use. If you look at the test cases in jackson-databind, you will see that none of the tests related to Include.NON_DEFAULT are about null handling.

Have a look at https://www.javadoc.io/doc/com.fasterxml.jackson.core/jackson-annotations/latest/index.html

You could try @JsonInclude(JsonInclude.Include.ALWAYS). This seems to be closer to what you want.

You say that the behaviour of Include.NON_DEFAULT has changed in Jackson 2.18 and that needs investigation.

@pjfanning
Copy link
Member

I created #4743 which does demo your issue. I have verified that 2.18 behaves differently.

@pjfanning pjfanning added has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue and removed to-evaluate Issue that has been received but not yet evaluated labels Oct 9, 2024
@pjfanning
Copy link
Member

Reasonably good chance that this relates to #4464

@rPraml
Copy link

rPraml commented Oct 9, 2024

Hello, we have the same problem. We have lots of pojos, where the default value is null and may be set to empty string "". This makes a semantic difference. We used also @JsonInclude(Include.NON_DEFAULT) not to serialize the default values (mainly nulls or empty lists etc.)

That annotation has this in the javadoc:

When used for a POJO, definition is that only values that differ from the default values of POJO properties are included.

I don't think this annotation value is about nulls

Sorry, I do think so, as this is clearly mentioned in the javadoc

... This is done by creating an instance of POJO using zero-argument constructor, and accessing property values:
value is used as the default value by using equals() method, except for the case where property has null value in which case straight null check is used.

Roland

@JooHyukKim
Copy link
Member

Tough call... seems like everyone has their own interpretation of the JavaDoc.
@teodord Could you help us out here? This issue here is sort of contracting to what we agreed to.
Your opinion would matter, be welcome as well

@ragnhov
Copy link
Author

ragnhov commented Oct 10, 2024

The documentation states that the behavior depends on context. I believe that the context is different in this case and @teodord 's case. The POJO case should include empty strings, the global case should not.

@JooHyukKim
Copy link
Member

I believe that the context is different in this case and @teodord 's case. The POJO case should include empty strings, the global case should not.

I concur. I might as well re-read the documentation one more time and see what we can do to fix the regression later today. Thank you for explanting this @ragnhov @rPraml @pjfanning !

@JooHyukKim
Copy link
Member

JooHyukKim commented Oct 10, 2024

Okay, re-reading through the documentation I am leaning toward towards keeping both behavior, thus #4744.

I would like to ask @cowtowncoder for direction we should head, while I am going back to re-examine the documentation-behavior sync one more time.

EDIT : Yup, it seems like both this issue and #4464 are meant to be suuported. This issue about per-POJO cases and latter about global setting.

@cowtowncoder
Copy link
Member

On this particular case: in this example if POJO has null as value for String, then null AND ONLY null should be excluded, not empty String. So I think this is valid bug report indeed.

@cowtowncoder cowtowncoder changed the title When Include.NON_DEFAULT setting is used on POJO, empty values are not included in json if default is null When Include.NON_DEFAULT setting is used on POJO, empty values are not included in json if default is null Oct 16, 2024
cowtowncoder pushed a commit that referenced this issue Oct 16, 2024
…empty values are not included in json if default is null (#4744)
@cowtowncoder cowtowncoder added this to the 2.18.1 milestone Oct 16, 2024
@cowtowncoder
Copy link
Member

Fixed for 2.18.1

cowtowncoder added a commit that referenced this issue Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

5 participants