OData
Omit null property value in JSON response
Issues
This pull request introduces omitNullValues controlled by omit-values=nulls Preference header per OData v4.01.
Description
- Current changes include the following areas: *
- serialize JSON object to response with omit null values option according to Preference-Applied header's parameter value.
- de-serialize JSON object from response with omitted null values restored per the option used according to the Preference-Applied header's parameter value.
- has no effects on request payload and delta payload.
Based on PR #944 from kosinsky.
Checklist (Uncheck if it is not completed)
- [x] Test cases added
- [x] Build and test with one-click build and test script passed
Additional work necessary
If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.
if (this.JsonLightOutputContext.MessageWriterSettings.Validations != ValidationKinds.None)
For performance reasons consider adding following optimization:
ODataValue value = property.ODataValue;
// If no validation is required, we don't need property serialization info and could try to write null property right away
// For very wide and sparse outputs it allows to avoid a lot of dictionary lookups
if (!this.MessageWriterSettings.ThrowIfTypeConflictsWithMetadata && omitNullValues)
{
if (value is ODataNullValue || value == null)
{
return;
}
} #Closed
Refers to: src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs:192 in df084b6. [](commit_id = df084b6eb92bcc26940b549e7863d95b36fe9ecf, deletion_comment = False)
@mikepizzo I just pushed latest commit for CR updates above. As we discussed offline, please check whether it is OK to not emitting the null for nested resource omitted on the wire; otherwise we need to make further change related to the overall json reader that could be hacky. Thanks. #Closed
@mikepizzo I have updated with the latest commit to not omit null values for expanded navigation properties. Can you please take a look? Thanks. #Closed
@mikepizzo I have just updated the PR per your comments. Can you please take a look at the pending comments? Thanks! #Closed
@Michael Pizzo Thanks for the comments. Can you check again to see whether all your comments have been addressed properly? #Closed
@mikepizzo The latest changes for expanded entity's omit-nulls are in last commit (df084b6) above. All other commits above are the original commits w/ re-base to the tip of master branch. Can you please take a look? Thanks! #Closed
What is the behavior if we have annotations for a property but no value? Do we write the annotations but no value? What happens on restore; do we restore a null in that case? What do we do without omit-nulls when we ready a property with annotations but no value? is the value of the ODataProperty null, rather than ODataNullValue?
If we have annotations for a property but no value, we write annotations but no value; and restore null on read. Without omit-nulls, read would do nothing w.r.t. null restoration (write does nothing either). The property value of null or ODataNullValue are equivalent.
In reply to: 435924987 [](ancestors = 435924987)
@mikepizzo Thanks for the latest round of review (primarily for $expand case of omit-null)! I have addressed the new comments. Please also follow up with @kosinsky about throwOnUndeclaredProperty optimization as we discussed, and I will do update as needed. #Closed
@mikepizzo I have made one more update for the optimization we discussed in office yesterday. Thanks for the suggestion. #Closed
I like the new code much better. See my comments inline:
- it looks like the old methods, that separately read declared and dynamic, are still there and should be removed?
- rather than determine the list of nullable properties each time we read a resource, can we do it once for the result, and then compare what we've read with that static list of nullable properties?
In reply to: 436729775 [](ancestors = 436729775)
As per separate thread, based on clarification from OASIS, we should not omit annotated values that are null, and should not restore missing values for annotated properties.
In reply to: 436424268 [](ancestors = 436424268,435924987)
The changes related to excluding annotated values is in a forked branch I have shared with you. Will be integrated into this PR, pending on double-checking with detailed requirements as we discussed. Edit: Actually the change related to excluding annotated values has been integrated into this PR as commit fd97a4455c. Pending on finalized requirement.
In reply to: 438406870 [](ancestors = 438406870,436424268,435924987)
@mikepizzo Please see the latest updates per your comments today. Currently there is one issue pending related to the annotated properties skipping omit-nulls, which will be merged into this PR when the detailed requirement is finalized. Thanks. #Closed
@mikepizzo I have updated the names to 'nestedResourceInfo'. They are from some original test cases that are not changed by this PR but it makes sense to update them. I have also provided my thoughts on your question related to caching null properties during restoration. Thanks. #Closed
I don't see fd97a4455c as part of this PR.
It looks like https://github.com/biaol-odata/odata.net/commit/fd97a4455c69cd357d7e0f39b347dd76baf5b62e handles not omitting writing the null value for annotated properties, but we need to validate handling of reading an annotated property that has no value.
Can ODL read/write a property with annotations and no value? Is Value=null handled the same as Value=ODataNull? Seems like they should be handled differently (the later former should write the annotations, if any, but no value). This could be a breaking change, however, if this is not how it's handled currently.
In reply to: 438516788 [](ancestors = 438516788,438406870,436424268,435924987)
@mikepizzo @biaol-odata @xuzhg we need above feature for one of above api, because many properties tend to be null there. It seems like this PR has not been touched for a while. What can be done to get this PR checked in
