-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Regression fix for #4741 Include.NON_DEFAULT
setting used on POJO, empty values are not included in json if default is null
#4744
Conversation
Fix and test Delete GoodTest.java
Include.NON_DEFAULT
setting used on POJO, empty values are not included in json if default is null
valueToSuppress = BeanPropertyWriter.MARKER_FOR_EMPTY; | ||
// [databind#4471] Different behavior when Include.NON_DEFAULT | ||
// setting is used on POJO vs global setting, as per documentation. | ||
if (!_useRealPropertyDefaults) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of re-checking configuration settings, let's rely on "_useRealPropertyDefaults" which means that POJO settings are in use (not global-, per-type or property)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense 👌🏼
@JooHyukKim I figured out a simpler check which I think works. But wanted to ask WDYT before merging. All tests pass which is good. |
I like the simpler check 🙏🏼 LGTM! @cowtowncoder |
@JooHyukKim Thank you for taking a lead on this! I'll go ahead and merge soon. |
demonstrates "possible" solution that resolves #4741