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

Revert non-empty handling of primitive numbers wrt NON_EMPTY; make NON_DEFAULT use extended criteria #952

Closed
cowtowncoder opened this issue Sep 29, 2015 · 6 comments
Milestone

Comments

@cowtowncoder
Copy link
Member

(note: follow up to #849)

Although the original intent of NON_EMPTY was to include a wider set of criteria, such as number values (both primitives and matching wrappers) and boolean/Boolean, implementation was incomplete. As a result users were under impression that NON_EMPTY only applies to following things:

  1. null values of all types
  2. Collections and Maps with size 0
  3. Arrays with length 0
  4. Empty Strings (length 0)

Since the fix #849 changed behavior, and since this change was not well-received, it seems best to revert the change with respect to extended types, and make int value of 0, for example, NOT to be excluded with criteria of NON_EMPTY.

But the original use case that I had in mind was to minimize number of properties written, including default values. To support this option we should be able to refine meaning of existing choice NON_DEFAULT. In addition to its meaning when used for POJOs (to mean that properties with original values when POJO is created with a default constructor), it should be possible to also allow its use for properties, and even as default exclusion settings. In those cases it should simply mean exclusion of all values covered by NON_EMPTY and in addition exclude:

  1. Default numeric values for int, long, short,byte,float,doubleandchar`, as well as matching Wrapper values
  2. Default value of false for boolean and Boolean
@talawahtech
Copy link

Thanks for your prompt response on this. I will try reverting to 2.5 and then wait till 2.7 to upgrade.

@JoergM
Copy link

JoergM commented Sep 29, 2015

Great! Thanks for the quick response.

@Sovietaced
Copy link

Thank you, I will also await 2.7!

@beanlover
Copy link

"2. Default value of false for boolean and Boolean"

I understand that the Java primitive boolean defaults to false, but the java.lang.Boolean object defaults to "null" when declared and not-initialized.

This also causes problems when using the "NOT_NULL" annotation because Jackson (2.6.3) attempts to deserialize uninitialized fields of type Boolean and throws a NPE.

Edit: strikethrough because that part was a bug on our side

@cowtowncoder
Copy link
Member Author

@beanlover The question on whether Integer.valueOf(0) should be considered NON_DEFAULT or not is bit tricky. But the reason I do think it should behave this way is because:

  1. If filtering of just nulls is needed, NON_NULL may be used
  2. Use of NON_DEFAULT is expected to be used for reducing size of output by removing redundant information.
  3. Unless NON_DEFAULT filters out same values for wrappers as primitives, users may find this confusing
  4. Since NON_EMPTY will NOT consider Integer.valueOf(0) to be empty, there would be no way to suppress such values, if only nulls were excluded with NON_DEFAULT

@cowtowncoder cowtowncoder added this to the 2.7.0 milestone Nov 12, 2015
@cowtowncoder cowtowncoder changed the title (2.7) Revert non-empty handling of primitive numbers wrt NON_EMPTY; make NON_DEFAULT use that criteria Revert non-empty handling of primitive numbers wrt NON_EMPTY; make NON_DEFAULT use that criteria Nov 12, 2015
@cowtowncoder cowtowncoder changed the title Revert non-empty handling of primitive numbers wrt NON_EMPTY; make NON_DEFAULT use that criteria Revert non-empty handling of primitive numbers wrt NON_EMPTY; make NON_DEFAULT use extended criteria Nov 12, 2015
@cowtowncoder
Copy link
Member Author

Looking at code for date/time serializers, I realized that there is one more group of "empty" values:

  • JDK java.util.Date, java.util.Calendar where long value of 0L is used (January 1, 1970, UTC)
  • Joda date/time types with similar epoch-based value

which is the behavior that 2.7 has. Whether this is as it should is up to debate: Date/time is not container type or sequence, so it would seem they should not. But on the other hand, there is no alternate mechanism to use because NON_DEFAULT has basically hard-coded selection criteria at this point, and can not support other types.

I think behavior can not change for 2.7 (we are at 2.7.5 already), but 2.8.0 could still be changed.
On the other hand, hope is for 2.9 to support custom per-property exclusion criteria, use of which could finally support fully custom behavior, so that could be the best point to change behavior.

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