jsonb-api
jsonb-api copied to clipboard
May adapter input / output be null?
Please add information to the JSON-B Spec / JavaDocs explicitly declaring how to handle null in JsonbAdapter implementations:
- MUST a
JsonbAdapterimplementation expect to receivenull? - MAY a
JsonbAdapterimplementation returnnull?
Hi @mkarg ,
My understanding is an adapter does not receive null and can return null.
I'm not sure about Yasson but Johnzon and TCK comply with that.
@m0mus maybe you can confirm about Yasson?
Romain
@rmannibucau Thanks, but in fact it would be good to clearly put that information into the JavaDocs.
My understanding is that an adapter can receive null and can return null. The spec doesn't specify it, so it's allowed.
According to the concencus between both of you, I'd like to kindly ask you to review and accept PR #194. :-)
Hmm, not sure there is a consensus if you reread ;).
On a spec level it is also not really needed because it is about setting a default in the bean directly and the javadoc speaks about object or JSON data so not null (https://javaee.github.io/javaee-spec/javadocs/javax/json/bind/adapter/JsonbAdapter.html).
Guess that if we want to enable null we must add a @AcceptNull in next version.
@rmannibucau Oops actually I completely missed the word "not" in your answer! Sorry for that!
Anyways it won't change my PR: To be one safe sid, an adapter vendor has to take care that Yasson will input null according to @m0mus. Hence, even if Johnzon does not, the check has to be there anyways.
Proposal: As there is disagreement on the input but not the output, an adapter de-facto wouldn't be unsafe if it does not check for null. Hence my PR still is correct to be on the safe side.
@mkarg the reasoning is biaised but I guess iy is the game?
Now with a spec hat this change breaks backward compatibility. If it was ambiguous we must be conservative so I fear we have to use Johnzon option even if I would probably have desired to accept null.
Note that it does not limit any use case thanks deserializers which are iso for all the null cases.
@rmannibucau It does not limit use case as long as you don't have to migrate an existing application from Johnzon to Yasson: To be on the safe side, you must change its code to either replace adapters by deserializers, or catch the null case in the adapters. As preventing code changes in applications in product migrations is one target of specifications, I do not see it as relaxed as you do. ;-)
@mkarg I'm not following you. Here is my view:
Current state: null input in adapters is rather defined as forbidden and spec examples encourage that (unintentionally I guess but this is what users will have used as a start so the code they have written in their app). So we must consider it is unsafe to pass null to an adapter in JSON-B 1.0.
1.x, x > 0: we can enable to pass null in adapter if desired but it must be explicit by the user to not break existing code. Here I agree we must explicit in the spec it is not portable to handle null in adapters in 1.0 and clarify it in 1.1 which means we either add an option to let the user notify the impl it is ok to pass null or we just say it is not a case which is that important since deserializers handle 100% of these cases as easily as adapters in practise thanks to the deserialization context.
Indeed I think we just need to clarify that null are not passed to adapters and even actually recommand the deserializer usage rather than adapter one in cases not targetting a direct mapping (adapter are only nice to map a type to string or simple types both ways, a bit like enums do, for other cases they create headaches to end users because they never know if it loops if there are conflicting adapters, if it is automatically taken into account in all mapping calls etc....).
Personally I have no bias. I'm good with either allowing or disallowing null. What I want is just unambiguous JavaDoc, Spec and Examples. If JSON-B Committers tells me their final decision, I will modify my PR accordingly. :-)