nillable is not true should return NON_EMPTY OR NON_NULL , Update JaxbAnnotationIntrospector.java

[henryhulian]

in wildfly10 default cannot ignore null filed when marshall object to json

[cowtowncoder]

Could you try to explain why you think this is the case? The code was defined this way for a reason, to specifically preserve null, and I was under impression this is what JAXB implementations would do for XML. This may also be something that needs to be discussed on mailing lists as it changes observed behavior.

[henryhulian]

Before i use wildfly 8.2 , when you didn't set nillable or just set nillable==false in XmlElement annotation, the resteasy's marshall process will ignore null filed , but after i switch 8.2 to 10.0 , the behavior changed , even though i set nillable==false , it still cannot ignore null filed. So i switch resteasy version to old version , it cannot work.Then i switch jackson version to old version whicth is 2.3.2 , it work. Our project use restfull service , if it cannot ignore null filed in marshall process , it will create too many unuseful string in every api call ..

[zenbones]

I'll agree with Henry. Null values should not be preserved, and in fact left out, when fields are marked as non-nillable. This is, after all, what non-nillable means. What you have done by preserving null values in all cases is actually the breaking change in behavior. We have many REST calls which expect fields to be absent when null (this is the most common case). Now these fields are all suddenly present with null values, and it breaks our code. Probably a lot of other peoples code as well.

[cowtowncoder]

@zenbones At this point this MUST be discussed on developer list. Not many developers read the issue updates. And since the change will be backwards-compatiblity breaking, I will not change that without some sort of consensus. Especially when it seems to be that definition of non-nillable is not clear: my take was it would mean that it is actually illegal NOT to have a non-null value: so if result from serialization would pass null, that would actually be a validity problem.

Or is the assumption that there are further state of "not present", above and beyond null? Such state is not presentable by Java, for example, and trying to preserve such distinction will less to loss of state.

At the end of the day, if Jackson developers feel that this change should be made, I am ok with it.
But I personally do not see this issue as solved.

[cowtowncoder]

After thinking about this for bit more, I think the best way to proceed is actually to make JAXBAnnotationIntrospector configurable, to determine behavior here. A simple flag to indicate intended behavior, defaulting to current one (to minimize immediate breakage for existing users).

My biggest concern is that JAXB annotations are used not just for XML, but also for JSON, and changes to inclusion criteria (always vs no nulls vs no empty) have been producing many issue reports due to various breakages.

I should be able to add configurability for 2.7 release, due out very soon. Being an API change, it unfortunately can not be added for earlier versions. The work-around there is to sub-class introspector, reimplement intended behavior as per this pull request.

[zenbones]

Let's be clear. The current state of the jaxb annotations, post 2.5.0, is a breaking change. I can't upgrade past 2.5.0 because it breaks my code that has worked with every prior version. Most json code does not pass null values, nor does it expect them. The definition of nillable used to align with this. And, at least for json,  is clear and needs no other configuration. If a value is non-nillable, and is null, it's left out. What else can it mean? Are you going to throw an error on a null value? Because that's the only other definition that makes any sense, and the choice doesn't do that either. Right now, nillable has no meaning. You broke it. Nillable or not, the output is the same.

The problem with the code, at least in part, is that you applied the solution for some problem with collections to values which were not collections. At least that what it looks like.

Nillable, however, is the switch. It is the configuration. It doesn't mean that you have to apply a value. It doesn't mean that you throw an exception if a value is not applied. It means that if a value is not applied then it does not appear. What definition of nillable is the current code working off of?

David

Sent from Outlook Mobile

On Thu, Dec 31, 2015 at 9:31 AM -0800, "Tatu Saloranta" notifications@github.com wrote:

After thinking about this for bit more, I think the best way to proceed is actually to make JAXBAnnotationIntrospector configurable, to determine behavior here. A simple flag to indicate intended behavior, defaulting to current one (to minimize immediate breakage for existing users).

My biggest concern is that JAXB annotations are used not just for XML, but also for JSON, and changes to inclusion criteria (always vs no nulls vs no empty) have been producing many issue reports due to various breakages.

I should be able to add configurability for 2.7 release, due out very soon. Being an API change, it unfortunately can not be added for earlier versions. The work-around there is to sub-class introspector, reimplement intended behavior as per this pull request.

—
Reply to this email directly or view it on GitHub.

[cowtowncoder]

@zenbones nillable definition comes form XML Schema, and for JAXB is defined to mean whatever JAXB specification does. Jackson is not a JAXB implementation, so while intent is to try to approximate behavior there, there is no guarantee it is identical; especially considering that mapping of XML concepts to JSON is not always straight-forward.

Looking at suggested change again, the problem with suggested revert is this: since there is no way to distinguish between:

@XmlElement()

and

@XmlElement(nillable=false)

(since nillable has false as default value)

use of @XmlElement would be an unexpected indication to force exclusion of non-empty values.
This is not the default with Jackson (although can be configured to be so for ObjectMapper), and to me seems unintuitive.
If there was a way to distinguish between explicit false, I would agree in behavior suggested.

Further problem is that since the "new" behavior has been there for version 2.5 and 2.6, AND there has not been many problem reports, I do not want to make a change that might well break handling by other existing code.

As such, I am not going to make the suggested change to baseline settings. But I will add a setting to allow doing that, but not as default.