FasterXML/jackson-databind#1296 @JsonIncludeProperties

[sp4ce]

No description provided.

[sp4ce]

This PR is not for being merged as-is, it's just to get early input
see FasterXML/jackson-databind#2771 as well

[cowtowncoder]

Makes sense, maybe add suffix or prefix "[WIP]" in title?

[sp4ce]

I removed the [WIP] as the related PR is ready to be reviewed and merged (once this one is approved as well): FasterXML/jackson-databind#2771

[cowtowncoder]

One bigger question: does it make sense to assume that empty set means "include all"?
Would that be merged with ignorals?
I am thinking there may be use case where someone might want to have no properties for sort of empty, non-null marker object, and this would be impossible if { } means (add all)

Alternatively there would need to be another property for "includeAll" or something.

[sp4ce]

does it make sense to assume that empty set means "include all"?

Currently I assumed:

• null for _included means all property.
• emptySet means include nothing

I took that convention because emptySet meaning all would be too messy to handle in the code and somehow counter intuitive to me. Also if you want to merge different inclusions direction and the intersection is empty, it would means in that case include nothing, not include all.

Would that be merged with ignorals?

What do you mean ?

I am thinking there may be use case where someone might want to have no properties for sort of empty, non-null marker object, and this would be impossible if { } means (add all)

Yes, see above, but that's why {} means nothing.

Alternatively there would need to be another property for "includeAll" or something.

includeAll is the same as @JsonIncludeProperties(null) (but I am not sure java allows it, so i can add the flag to be explicit.

[sp4ce]

Related to your comment in FasterXML/jackson-databind#2771 (comment)
I added the method withOverrides to merge two JsonIncludeProperties.Value, I made the intersection of the two sets. I am however not too sure how AnnotationIntrospectorPair is used, and that might not be the right thing to do. I will dig more into some unit test to see if I can make the related behaviour more evident, but please share with me any additional context.

[cowtowncoder]

@sp4ce Ok: maybe it makes sense to discuss this in DB/2771. But just to summarize here:

1. null would mean "not defined" which in turn means roughly "just include everything (if not separately excluded)")
2. { } would mean "included nothing", explicitly.

That makes sense to me; and only leaves theoretical need to override an existing (but somehow incorrect) @JsonIncludeProperties from base class, which could be handled by adding enabled marker-property, if we wanted that. But probably not really needed yet.

Part about intersection/conjunction would probably lead to another question on inheritance (if sub-class overrides, should this extend, limit, or replace settings -- currently it can really only replace), but I'll leave that out for now.

[cowtowncoder]

Should explain logic (include if in both (intersection), include if in either (conjunction?)).

[cowtowncoder]

Ok I think I'd be ready to merge this part first, and hopefully databind soon as well.
But one last thing I'd need before that: I would need Contributor License Agreement from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

(unless I have asked for and received one already)

This is only needed once for all contributions; the usual method is to print, fill & sign, scan / photo, email to info at fasterxml dot com. Once I receive it, I'll go ahead and merge this PR, start looking into the other part.

Thank you again for contributing this; looking forward to merging!

[sp4ce]

Hello, sorry for the late reply, I sent you the contributor agreement. I have a bit lost track about what we were saying about the null value vs having a dedicated flag to indicate all fields. If you are fine merging it like this, no problem. I can help in a future PR as well.

[cowtowncoder]

Thanks; yes, I think I figured the null part out and it's not a problem. Will try to go over this again today or tomorrow. Thank you for the CLA!