Skip to content

Change of NON_EMPTY for Scalars (Fix #849) in 2.6 is not backward compatible #875

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

Closed
kamenik opened this issue Jul 27, 2015 · 7 comments
Closed
Milestone

Comments

@kamenik
Copy link

kamenik commented Jul 27, 2015

Hi,

We have upgraded to Jackson 2.6.0 recently and it broke lots of our javascripts, they are very surprised by all zero values changed to undefined. It would be better to add another flag (NON_EMPTY_OR_ZERO) instead of changing current functionality.

@mastojun
Copy link

👍
I don't understand why 0 is considered empty. it is not empty, zero also actual number!

why don't you use NON_DEFAULT instead of NON_EMPTY?

@cowtowncoder
Copy link
Member

@mastojun Concept of "empty" is arbitrary, but it does not mean "missing".
I am aware that zero is a number; but it is the default value of int, and so it was chosen as 'empty' value for numbers.

@kamenik Change was to address behavior, since zero was supposed to have been taken as empty from beginning; similar to empty String "" or boolean false. You can think of it as "default value for primitives".
We do not plan to add new flags, due to wide range of JsonFormat.Include values; but most likely you want NON_NULL or NON_ABSENT. NON_EMPTY has meant empty containers and default values for other types, and as unfortunate as backwards incompatibility is in this case, I don't think re-introducing the earlier flaw makes sense.

I am open to other ways to correct the problem however.

@cowtowncoder
Copy link
Member

@mastojun One more thing: while NON_DEFAULT sounds like it should work, it actually has bit different semantics for POJOs: it excludes property values that have default values -- but this does not necessarily mean 0 for int (for example), as POJO may set different defaults with direct assignment or via constructor.

However: for primitives it should actually also work; so you can use settings interchangably.

I will need to at least upgrade javadocs for @JsonInclude to document expected behavior.

@Sovietaced
Copy link

I've tried using both NON_NULL and NON_ABSENT but this changes other behavior that breaks unit tests for us.

@cowtowncoder Is there a way to set SerializationInclusion on a specific type, namely numbers?

@einmalfel
Copy link

+1 to this issue: IMHO this will be counterintuitive even if you will mention this behaviour in docs
Enabling NON_EMPTY I expect it to skip empty collections, not integers with zero value

@cowtowncoder
Copy link
Member

Regardless, the original intent was to consider default values for numbers to be empty.
I wish documentation was clear on this, as this is how values were meant to work.
I can see that from purely naming perspective this is not intuitive.

But the problem is that there is legitimate need/preference to also suppress zero numeric values, usually to reduce size of JSON serialization. Use of NON_DEFAULT could perhaps work, but its semantics are currently tied to POJO values, and not for stand-alone values.

Further, given that 2.6 does have clarified behavior, I am not sure I would want to make yet another behavior change in 2.7 to roll back changes.

With all this, note that there is not much benefit to +1:ing this issue -- issue tracker is not followed up by all (or even many) developers. The better place to argue for change in behavior is Jackson developer list at https://groups.google.com/forum/#!pendingmsg/jackson-dev.

@cowtowncoder cowtowncoder changed the title Fix #849 is not backward compatible Change of NON_EMPTY for Scalars (Fix #849) in 2.6 is not backward compatible Sep 9, 2015
@cowtowncoder cowtowncoder added this to the 2.7.0-rc2 milestone Dec 19, 2015
@cowtowncoder
Copy link
Member

Fixed via #952, included in 2.7.0-rc2.

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

5 participants