Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit null property value in JSON response #1106

Draft
wants to merge 16 commits into
base: release-7.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// </copyright>
//---------------------------------------------------------------------

using System.Diagnostics;

#if ODATA_CLIENT
namespace Microsoft.OData.Client
#else
Expand All @@ -23,6 +25,9 @@ namespace Microsoft.OData.Evaluation
/// </summary>
internal abstract class ODataResourceMetadataBuilder
{
/// <summary>The expanded entities that have been processed.</summary>
protected readonly HashSet<string> ProcessedExpandedEntityNames = new HashSet<string>(StringComparer.Ordinal);

#if !ODATA_CLIENT
/// <summary>
/// Gets an instance of the metadata builder which never returns anything other than nulls.
Expand All @@ -36,6 +41,7 @@ internal static ODataResourceMetadataBuilder Null
}
#endif


/// <summary>
/// Gets instance of the metadata builder which belongs to the parent odata resource
/// </summary>
Expand Down Expand Up @@ -156,6 +162,27 @@ internal virtual void MarkNestedResourceInfoProcessed(string navigationPropertyN
{
}

/// <summary>
/// Return whether the given expanded entity of specified name has been processed.
/// </summary>
/// <param name="name">The name of the expanded entity.</param>
/// <returns>True if the expanded entity has been processed; False otherwise.</returns>
internal virtual bool IsExpandedEntityProcessed(string name)
{
return this.ProcessedExpandedEntityNames.Contains(name);
}

/// <summary>
/// Marks the given expanded entity as processed.
/// </summary>
/// <param name="expandedEntityName">The name of the expanded entity we've already processed.</param>
internal virtual void MarkExpandedEntityProcessed(string expandedEntityName)
{
Debug.Assert(!string.IsNullOrEmpty(expandedEntityName), "!string.IsNullOrEmpty(expandedEntityName)");
Debug.Assert(this.ProcessedExpandedEntityNames != null, "this.ProcessedExpandedEntityNames != null");
this.ProcessedExpandedEntityNames.Add(expandedEntityName);
}

/// <summary>
/// Returns the next unprocessed nested resource info or null if there's no more navigation links to process.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ internal void WriteTopLevelProperty(ODataProperty property)
null /*owningType*/,
true /* isTopLevel */,
false /* allowStreamProperty */,
false /*omitNullValues */,
this.CreateDuplicatePropertyNameChecker());
this.JsonLightValueSerializer.AssertRecursionDepthIsZero();

Expand All @@ -100,11 +101,15 @@ internal void WriteTopLevelProperty(ODataProperty property)
/// Whether the properties are being written for complex value. Also used for detecting whether stream properties
/// are allowed as named stream properties should only be defined on ODataResource instances
/// </param>
/// <param name="omitNullValues">Whether to omit null property values.
/// Value should be 'false' if writing delta payload, otherwise derived from request preference header.
/// </param>
/// <param name="duplicatePropertyNameChecker">The DuplicatePropertyNameChecker to use.</param>
internal void WriteProperties(
IEdmStructuredType owningType,
IEnumerable<ODataProperty> properties,
bool isComplexValue,
bool omitNullValues,
IDuplicatePropertyNameChecker duplicatePropertyNameChecker)
{
if (properties == null)
Expand All @@ -119,6 +124,8 @@ internal void WriteProperties(
owningType,
false /* isTopLevel */,
!isComplexValue,
// Annotated properties won't be omitted even if it is null.
omitNullValues && property.InstanceAnnotations.Count == 0,
duplicatePropertyNameChecker);
}
}
Expand All @@ -141,7 +148,7 @@ private bool IsOpenProperty(ODataProperty property)
else
{
// TODO: (issue #888) this logic results in type annotations not being written for dynamic properties on types that are not
// marked as open. Type annotations should always be written for dynamic properties whose type cannot be hueristically
// marked as open. Type annotations should always be written for dynamic properties whose type cannot be heuristically
// determined. Need to change this.currentPropertyInfo.MetadataType.IsOpenProperty to this.currentPropertyInfo.MetadataType.IsDynamic,
// and fix related tests and other logic (this change alone results in writing type even if it's already implied by context).
isOpenProperty = (!this.WritingResponse && this.currentPropertyInfo.MetadataType.OwningType == null) // Treat property as dynamic property when writing request and owning type is null
Expand All @@ -164,13 +171,17 @@ private bool IsOpenProperty(ODataProperty property)
/// <param name="isTopLevel">true when writing a top-level property; false for nested properties.</param>
/// <param name="allowStreamProperty">Should pass in true if we are writing a property of an ODataResource instance, false otherwise.
/// Named stream properties should only be defined on ODataResource instances.</param>
/// <param name="omitNullValues"> Whether to omit null property values for writing.
/// Value should be 'false' if writing top level properties or delta payload, otherwise derived from request preference header.
/// </param>
/// <param name="duplicatePropertyNameChecker">The DuplicatePropertyNameChecker to use.</param>
[SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Splitting the code would make the logic harder to understand; class coupling is only slightly above threshold.")]
private void WriteProperty(
ODataProperty property,
IEdmStructuredType owningType,
bool isTopLevel,
bool allowStreamProperty,
bool omitNullValues,
IDuplicatePropertyNameChecker duplicatePropertyNameChecker)
{
WriterValidationUtils.ValidatePropertyNotNull(property);
Expand Down Expand Up @@ -204,6 +215,15 @@ private void WriteProperty(

ODataValue value = property.ODataValue;

Copy link
Member

@mikepizzo mikepizzo Nov 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why these were moved? Can we put the two checks back before line 206 (before we write the type annotation for undeclared properties)? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved back to before L206. Thanks.


In reply to: 233183156 [](ancestors = 233183156)

bool isNullValue = (value == null || value is ODataNullValue);
if (isNullValue && omitNullValues)
{
if (!this.currentPropertyInfo.IsTopLevel)
{
return;
}
}

// handle ODataUntypedValue
ODataUntypedValue untypedValue = value as ODataUntypedValue;
if (untypedValue != null)
Expand All @@ -227,7 +247,7 @@ private void WriteProperty(
return;
}

if (value is ODataNullValue || value == null)
if (isNullValue)
{
this.WriteNullProperty(property);
return;
Expand Down
85 changes: 85 additions & 0 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1658,6 +1658,12 @@ private void ReadExpandedNestedResourceInfoEnd(bool isCollection)
IODataJsonLightReaderResourceState parentResourceState = (IODataJsonLightReaderResourceState)this.ParentScope;
parentResourceState.NavigationPropertiesRead.Add(this.CurrentNestedResourceInfo.Name);

if (parentResourceState.SelectedProperties.SelectedExpandedEntities.Contains(this.CurrentNestedResourceInfo.Name))
{
// Record that we read the expanded entity
parentResourceState.MetadataBuilder.MarkExpandedEntityProcessed(this.CurrentNestedResourceInfo.Name);
}

// replace the 'NestedResourceInfoStart' scope with the 'NestedResourceInfoEnd' scope
this.ReplaceScope(ODataReaderState.NestedResourceInfoEnd);
}
Expand Down Expand Up @@ -2232,6 +2238,9 @@ private void EndEntry()

if (this.State == ODataReaderState.ResourceStart)
{
// For non-delta response, need to restore omitted null values as required.
RestoreOmittedNullValues();

this.EndEntry(
new JsonLightResourceScope(
ODataReaderState.ResourceEnd,
Expand All @@ -2256,6 +2265,82 @@ private void EndEntry()
}
}

/// <summary>
/// For response de-serialization, restore the null values for properties that are omitted
/// by the omit-values=nulls preference header.
/// </summary>
private void RestoreOmittedNullValues()
{
// Restore omitted null value only when processing response and when
// Preference header omit-values=nulls is specified.
if (ShouldRestoreNullValues())
{
IODataJsonLightReaderResourceState resourceState = this.CurrentResourceState;
IEdmStructuredType edmStructuredType = resourceState.ResourceType;
SelectedPropertiesNode selectedProperties = resourceState.SelectedProperties;

if (selectedProperties == SelectedPropertiesNode.Empty)
{
return;
}
else if (resourceState.Resource != null)
{
Debug.Assert(edmStructuredType != null, "edmStructuredType != null");
ODataResourceBase resource = resourceState.Resource;


IList<string> nullPropertyNames =
selectedProperties.GetSelectedNullValueProperties(edmStructuredType, resourceState);

// Restore null Expanded entities, which are specified as "isExpandedNavigationProperty" the child nodes.
foreach (string name in selectedProperties.SelectedExpandedEntities)
{
if (!resource.MetadataBuilder.IsExpandedEntityProcessed(name))
{
nullPropertyNames.Add(name);
}
}

// Mark as processed, will throw if duplicate is detected.
foreach (string name in nullPropertyNames)
{
this.CurrentResourceState.PropertyAndAnnotationCollector.MarkPropertyAsProcessed(name);
}

RestoreNullODataProperties(nullPropertyNames);
}
}
}

/// <summary>
/// Restore the null property values in the resource.
/// </summary>
/// <param name="nullPropertyNames">The list of names of the properties to be restored with null values.</param>
private void RestoreNullODataProperties(IList<string> nullPropertyNames)
{
IList<ODataProperty> properties = nullPropertyNames.Select(name => new ODataProperty()
{
Name = name,
Value = new ODataNullValue()
}).ToList();

ODataResourceBase resource = this.CurrentResourceState.Resource;

ReadOnlyEnumerable<ODataProperty> readonlyEnumerable =
resource.Properties as ReadOnlyEnumerable<ODataProperty>;
if (readonlyEnumerable != null)
{
// For non-entity resource, add the property underneath the read-only enumerable.
resource.Properties.ConcatToReadOnlyEnumerable("Properties", properties);
}
else
{
// For entity resource, concatenate the property to original enumerable.
resource.Properties =
resource.Properties.Concat(properties);
}
}

/// <summary>
/// Add info resolved from context url to current scope.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,10 @@ internal ODataJsonLightReaderNestedResourceInfo ReadPropertyWithoutValue(IODataJ
ODataStreamReferenceValue streamPropertyValue = this.ReadStreamPropertyValue(resourceState, propertyName);
AddResourceProperty(resourceState, edmProperty.Name, streamPropertyValue);
}
else if (this.JsonLightInputContext.ShouldRestoreNullValues())
{
AddResourceProperty(resourceState, propertyName, new ODataNullValue());
}
else
{
throw new ODataException(ODataErrorStrings.ODataJsonLightResourceDeserializer_PropertyWithoutValueWithWrongType(propertyName, propertyTypeReference.FullName()));
Expand Down Expand Up @@ -1320,7 +1324,18 @@ private ODataJsonLightReaderNestedResourceInfo InnerReadUndeclaredProperty(IODat
// Property without a value can't be ignored if we don't know what it is.
if (!propertyWithValue)
{
throw new ODataException(ODataErrorStrings.ODataJsonLightResourceDeserializer_OpenPropertyWithoutValue(propertyName));
if (this.JsonLightInputContext.ShouldRestoreNullValues())
{
// For case of property without values that are possibly omitted, restore omitted null value
// and return nested resource as null.
AddResourceProperty(resourceState, propertyName, new ODataNullValue());
return null;
}
else
{
throw new ODataException(
ODataErrorStrings.ODataJsonLightResourceDeserializer_OpenPropertyWithoutValue(propertyName));
}
}

object propertyValue = null;
Expand Down
27 changes: 23 additions & 4 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,32 @@ protected override void StartResource(ODataResource resource)
}

this.jsonLightResourceSerializer.InstanceAnnotationWriter.WriteInstanceAnnotations(parentNavLink.GetInstanceAnnotations(), parentNavLink.Name);
}

// Write the property name of an expanded navigation property to start the value.
this.jsonWriter.WriteName(parentNavLink.Name);
// Annotated resource won't be omitted even if it is null.
if (!this.ShouldOmitNullValues() || parentNavLink.GetInstanceAnnotations().Count > 0)
{
// Write the property name of an expanded navigation property to start the value of null per preference setting.
this.jsonWriter.WriteName(parentNavLink.Name);

// Optimization: write null and return directly.
this.jsonWriter.WriteValue((string)null);
return;
}
}
else
{
// Write the property name of an expanded navigation property to start the value.
this.jsonWriter.WriteName(parentNavLink.Name);
}
}

if (resource == null)
{
this.jsonWriter.WriteValue((string)null);
if (!this.ShouldOmitNullValues())
{
this.jsonWriter.WriteValue((string)null);
}

return;
}

Expand Down Expand Up @@ -331,6 +348,7 @@ protected override void StartResource(ODataResource resource)
this.ResourceType,
resource.Properties,
false /* isComplexValue */,
this.ShouldOmitNullValues(),
this.DuplicatePropertyNameChecker);
this.jsonLightResourceSerializer.JsonLightValueSerializer.AssertRecursionDepthIsZero();

Expand Down Expand Up @@ -1228,6 +1246,7 @@ private void WriteDeltaResourceProperties(IEnumerable<ODataProperty> properties)
this.ResourceType,
properties,
false /* isComplexValue */,
false /* omitNullValues */,
this.DuplicatePropertyNameChecker);
this.jsonLightResourceSerializer.JsonLightValueSerializer.AssertRecursionDepthIsZero();
}
Expand Down
1 change: 0 additions & 1 deletion src/Microsoft.OData.Core/Microsoft.OData.Core.txt
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ HttpHeaderValueLexer_TokenExpectedButFoundQuotedString=An error occurred when pa
HttpHeaderValueLexer_FailedToReadTokenOrQuotedString=An error occurred when parsing the HTTP header '{0}'. The header value '{1}' is incorrect at position '{2}' because a token or a quoted-string is expected at this position but were not found.
HttpHeaderValueLexer_InvalidSeparatorAfterQuotedString=An error occurred when parsing the HTTP header '{0}'. The header value '{1}' is incorrect at position '{2}' because '{3}' is not a valid separator after a quoted-string.
HttpHeaderValueLexer_EndOfFileAfterSeparator=An error occurred when parsing the HTTP header '{0}'. The header value '{1}' is incorrect at position '{2}' because the header value should not end with the separator '{3}'.

MediaType_EncodingNotSupported=The character set '{0}' is not supported.

MediaTypeUtils_DidNotFindMatchingMediaType=A supported MIME type could not be found that matches the acceptable MIME types for the request. The supported type(s) '{0}' do not match any of the acceptable MIME types '{1}'.
Expand Down
14 changes: 14 additions & 0 deletions src/Microsoft.OData.Core/ODataConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,19 @@ internal static class ODataInternalConstants
/// </summary>
public const string ContentIdHeader = "Content-ID";

#region Preference Headers
/// <summary>
/// Valid value for <code>OmitValuesPreferenceToken</code> indicating nulls are omitted.
/// </summary>
public const string OmitValuesNulls = "nulls";

/// <summary>
/// Valid value for <code>OmitValuesPreferenceToken</code> indicating defaults are omitted.
/// Internal access only since "defaults" is not supported yet.
/// </summary>
internal const string OmitValuesDefaults = "defaults";
#endregion Preference Headers

/// <summary>
/// Name of the Content-Length HTTP header.
/// </summary>
Expand Down Expand Up @@ -205,6 +218,7 @@ internal static class ODataInternalConstants

/// <summary>The $deletedLink token indicates delta deleted link.</summary>
internal const string ContextUriDeletedLink = UriSegmentSeparator + DeletedLink;

#endregion Context URL
}
}
2 changes: 0 additions & 2 deletions src/Microsoft.OData.Core/ODataContextUrlInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,6 @@ private static string CreateSelectExpandContextUriSegment(SelectExpandClause sel
/// <returns>The generated expand string.</returns>
private static string ProcessSubExpand(string expandNode, string subExpand)
{

return expandNode + ODataConstants.ContextUriProjectionStart + subExpand + ODataConstants.ContextUriProjectionEnd;
}

Expand All @@ -415,7 +414,6 @@ private static string CombineSelectAndExpandResult(IList<string> selectList, ILi

if (selectList.Any())
{

currentExpandClause += String.Join(ODataConstants.ContextUriProjectionPropertySeparator, selectList.ToArray());
}

Expand Down
Loading