Added support for explicit null values during deserialization - #1307
Added support for explicit null values during deserialization#1307timocov wants to merge 2 commits into
Conversation
So deserializers can distinguish absent fields from explicitly-null ones. Previously null members were always skipped; now consumers can opt in per-member to receive them.
| * @param memberSchema Schema of the member to check. | ||
| * @return {@code true} if the member should receive null values, {@code false} to skip them. | ||
| */ | ||
| default boolean supportsNullValues(Schema memberSchema) { |
There was a problem hiding this comment.
Is this a property of generic serde, or is it a property of a protocol?
There was a problem hiding this comment.
Good question. I assume your question is more of "should a structure should tell if it supports null vslues or the protocol enables it for every field regardless"? I can see it from both sides, and tbh I'm still not sure that a protocol matters in this case - it just tells that "you should differentiate between passed nulls and not passed values", but I can see it being used in some places even without specifying the protocol explicitly if a business logic wants to see the difference.
It does feel like more of a protocol feature to "turn it on", but whether a field supprts such differentiation should lay down to the business logic (i.e. model). If we make it a protocol feature, then generated serde logic in shapes will become protocol-aware (currently it will fail if the serde level will call deserialising of a structure on with null value), which goes against the main principles of Smithy.
What behavior changes?
Adds opt-in support for provided
nullvalues during the serialization.Almost no-op if not opted-in (the only change is to call
structMemberConsumer.supportsNullValues(member)for each field to check if it was opted-in; by default returnsfalse).By default, the feature is disabled. But it can be enabled via the codegen integration - a symbol provider should add a
SymbolProperties.SUPPORTS_NULL_VALUESproperty to a member's symbol and set it totrue. This will result in generating an override implementation ofStructMemberConsumer.supportsNullValuesand returntruefor fields that support it based on the symbol properties. After that, the codegen integration can override a type/serde for such field and, for example, use something like https://docs.vavr.io/#_option instead of a raw type (i.e.Option<String>instead of justString). Then the serde override can check if current value is null, then assignOption.none().Why is this change needed?
There is a version of application/json protocol that uses "explicit null" values to identify the intent, see https://www.rfc-editor.org/info/rfc7396/.
Currently json-codec skips deserialization of null values. This prevents to detect whether a value was provided as null or wasn't provided at all.
How was this validated?
null,nullas value ->Option.none(), value provided ->Option.some(value)).What should reviewers focus on?
The logic looks simple, but things worth checking:
generateMemberNullValuesSwitchCasesshould generate cases for every member, not just ones returningtrueJsonSettingsand check it as well, but it felt like the schema/symbols have a better understand if they support null values or not. But let me know if you think that both should be added (so that it would be much cheaper check in the deser if it is not opted-in).Also, please let me know if this is something you wouldn't want to have a support for.
Additional Links
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.