Skip to content

Commit

Permalink
Addressing CR comments.
Browse files Browse the repository at this point in the history
1. Writing: Omit null value for nested resource.
2. Reading:
    a). Restore annotated property whose value is omitted on the wire (result is null property value with annotation).
    b). Restore dynamic property.
3. Misc updates.
  • Loading branch information
biaol-odata committed May 14, 2018
1 parent bc4a274 commit 954c798
Show file tree
Hide file tree
Showing 13 changed files with 509 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ 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.</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,
Expand Down Expand Up @@ -168,7 +170,9 @@ 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.</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(
Expand Down Expand Up @@ -199,6 +203,14 @@ private void WriteProperty(
this.currentPropertyInfo = this.JsonLightOutputContext.PropertyCacheHandler.GetProperty(propertyName, owningType);
}


if (currentPropertyInfo.MetadataType.IsUndeclaredProperty)
{
WriteODataTypeAnnotation(property, isTopLevel);
}

WriteInstanceAnnotation(property, isTopLevel, currentPropertyInfo.MetadataType.IsUndeclaredProperty);

// Optimization for null values:
// If no validation is required, we don't need property serialization info and could try to skip writing null property right away
// If this property is top-level, we cannot optimize here due to backward-compatibility requirement for OData-6.x.
Expand All @@ -216,13 +228,6 @@ private void WriteProperty(

duplicatePropertyNameChecker.ValidatePropertyUniqueness(property);

if (currentPropertyInfo.MetadataType.IsUndeclaredProperty)
{
WriteODataTypeAnnotation(property, isTopLevel);
}

WriteInstanceAnnotation(property, isTopLevel, currentPropertyInfo.MetadataType.IsUndeclaredProperty);

// handle ODataUntypedValue
ODataUntypedValue untypedValue = value as ODataUntypedValue;
if (untypedValue != null)
Expand Down
128 changes: 85 additions & 43 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2267,8 +2267,7 @@ private void RestoreOmittedNullValues()
{
// Restore omitted null value only when processing response and when
// Preference header omit-values=nulls is specified.
if (this.jsonLightInputContext.ReadingResponse
&& this.jsonLightInputContext.MessageReaderSettings.NullValuesOmitted)
if (ShouldRestoreNullValues())
{
IODataJsonLightReaderResourceState resourceState = this.CurrentResourceState;
IEdmStructuredType edmStructuredType = resourceState.ResourceType;
Expand All @@ -2291,62 +2290,105 @@ private void RestoreOmittedNullValues()
}
else
{
// Partial subtree. Combine navigation properties and selected properties at the node with distinct.
selectedProperties =
resourceState.SelectedProperties.GetSelectedNavigationProperties(edmStructuredType)
as IEnumerable<IEdmProperty>;
selectedProperties = selectedProperties.Concat(
resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType))
.Distinct();
//Partial subtree.
selectedProperties = resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType);
}

// All dynamic properties: null value restoration.
foreach (string name in resourceState.SelectedProperties.GetSelectedDynamicProperties(edmStructuredType))
{
RestoreNullODataProperty(name, resourceState);
}

foreach (IEdmProperty currentProperty in selectedProperties)
{
Debug.Assert(currentProperty.Type != null, "currentProperty.Type != null");
if (!currentProperty.Type.IsNullable || currentProperty.PropertyKind == EdmPropertyKind.Navigation)

// 1. Skip declared properties that are not null-able types.
// 2. Don't need to restored null value for resources or nested resources omitted on the wire, since
// reader just don't report them anyway.
// 3. Skip properties (including navigation properties) that have been read.
if (!IsNullableProperty(currentProperty)
|| resource.Properties.Any(p => p.Name.Equals(currentProperty.Name, StringComparison.Ordinal))
|| resourceState.NavigationPropertiesRead.Contains(currentProperty.Name))
{
// Skip declared properties that are not null-able types, and declared navigation properties
// which are specified using odata.navigationlink annotations whose absence stand for null values.
continue;
}

// Response should be generated using case sensitive, matching EDM defined by metadata.
// In order to find potential omitted properties, need to check
// resource properties read and navigation properties read.
if (!resource.Properties.Any(
p => p.Name.Equals(currentProperty.Name, StringComparison.Ordinal))
&& !resourceState.NavigationPropertiesRead.Contains(currentProperty.Name))
{
// Add null value to omitted property declared in the type
ODataProperty property = new ODataProperty
{
Name = currentProperty.Name,
Value = new ODataNullValue()
};

// Mark as processed, will throw if duplicate is detected.
resourceState.PropertyAndAnnotationCollector.MarkPropertyAsProcessed(property.Name);

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", property);
}
else
{
// For entity resource, concatenate the property to original enumerable.
resource.Properties =
resource.Properties.Concat(new List<ODataProperty>() { property });
}
}
RestoreNullODataProperty(currentProperty.Name, resourceState);
}
}
}
}
}

private static bool IsNullableProperty(IEdmProperty property)
{
bool isNullableProperty = true;

if (!property.Type.IsNullable)
{
// From customized model whether it is null-able
isNullableProperty = false;
}
else
{
// From definition type kind.
switch (property.Type.Definition.TypeKind)
{
case EdmTypeKind.Complex: // complex types
case EdmTypeKind.Entity:
case EdmTypeKind.EntityReference:
case EdmTypeKind.Path:
isNullableProperty = false;
break;

case EdmTypeKind.Collection:
IEdmCollectionType collectionType = property.Type.Definition as EdmCollectionType;
isNullableProperty = collectionType.ElementType.IsNullable;
break;

default:
break;
}
}

return isNullableProperty;
}

/// <summary>
/// Restore the null property value in the resource.
/// </summary>
/// <param name="nullPropertyName">Name of the property to restore the null value.</param>
/// <param name="resourceState">The resource state.</param>
private static void RestoreNullODataProperty(string nullPropertyName, IODataJsonLightReaderResourceState resourceState)
{
// Add null value to omitted property declared in the type
ODataProperty property = new ODataProperty
{
Name = nullPropertyName,
Value = new ODataNullValue()
};

// Mark as processed, will throw if duplicate is detected.
resourceState.PropertyAndAnnotationCollector.MarkPropertyAsProcessed(property.Name);
ODataResourceBase resource = resourceState.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", property);
}
else
{
// For entity resource, concatenate the property to original enumerable.
resource.Properties =
resource.Properties.Concat(new List<ODataProperty>() { property });
}
}

/// <summary>
/// Get all properties defined by the EDM structural type and its base types.
/// </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
20 changes: 16 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,27 @@ 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);
if (!this.ShouldOmitNullValues())
{
// Write the property name of an expanded navigation property to start the value per preference setting.
this.jsonWriter.WriteName(parentNavLink.Name);
}
}
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
3 changes: 0 additions & 3 deletions src/Microsoft.OData.Core/Microsoft.OData.Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,6 @@ internal sealed class TextRes {
internal const string HttpHeaderValueLexer_FailedToReadTokenOrQuotedString = "HttpHeaderValueLexer_FailedToReadTokenOrQuotedString";
internal const string HttpHeaderValueLexer_InvalidSeparatorAfterQuotedString = "HttpHeaderValueLexer_InvalidSeparatorAfterQuotedString";
internal const string HttpHeaderValueLexer_EndOfFileAfterSeparator = "HttpHeaderValueLexer_EndOfFileAfterSeparator";

internal const string HttpPreferenceHeader_InvalidValueForToken = "HttpPreferenceHeader_InvalidValueForToken";

internal const string MediaType_EncodingNotSupported = "MediaType_EncodingNotSupported";
internal const string MediaTypeUtils_DidNotFindMatchingMediaType = "MediaTypeUtils_DidNotFindMatchingMediaType";
internal const string MediaTypeUtils_CannotDetermineFormatFromContentType = "MediaTypeUtils_CannotDetermineFormatFromContentType";
Expand Down
3 changes: 0 additions & 3 deletions src/Microsoft.OData.Core/Microsoft.OData.Core.txt
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +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}'.

HttpPreferenceHeader_InvalidValueForToken=The value '{0}' is not a valid value for OData token '{1}' in HTTP Prefer or Preference-Applied headers.

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
9 changes: 9 additions & 0 deletions src/Microsoft.OData.Core/ODataInputContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,15 @@ internal Uri ResolveUri(Uri baseUri, Uri payloadUri)
return null;
}

/// <summary>
/// Returns whether properties of null values should be restored when materializing the payload.
/// </summary>
/// <returns>Return true to restore null-value properties; false otherwise.</returns>
internal bool ShouldRestoreNullValues()
{
return this.ReadingResponse && this.MessageReaderSettings.NullValuesOmitted;
}

/// <summary>
/// Perform the actual cleanup work.
/// </summary>
Expand Down
9 changes: 4 additions & 5 deletions src/Microsoft.OData.Core/ODataMessageReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,14 @@ public ODataMessageReader(IODataResponseMessage responseMessage, ODataMessageRea
this.model = model ?? GetModel(this.container);
this.edmTypeResolver = new EdmTypeReaderResolver(this.model, this.settings.ClientCustomTypeResolver);

// Whether null values were omitted in the response.
// Whether null values were omitted in the response, false by default.
this.settings.NullValuesOmitted = false;
string omitValuePreferenceApplied = responseMessage.PreferenceAppliedHeader().OmitValues;
if (omitValuePreferenceApplied != null)
if (string.Equals(responseMessage.PreferenceAppliedHeader().OmitValues, ODataConstants.OmitValuesNulls,
StringComparison.OrdinalIgnoreCase))
{
// If the Preference-Applied header's omit-values parameter is present in the response, its value should
// be applied to the reader's setting.
this.settings.NullValuesOmitted =
omitValuePreferenceApplied.Equals(ODataConstants.OmitValuesNulls, StringComparison.OrdinalIgnoreCase);
this.settings.NullValuesOmitted = true;
}

// If the Preference-Applied header on the response message contains an annotation filter, we set the filter
Expand Down
10 changes: 10 additions & 0 deletions src/Microsoft.OData.Core/ODataReaderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,16 @@ protected void DecreaseResourceDepth()
this.currentResourceDepth--;
}


/// <summary>
/// Returns whether properties of null values should be restored when materializing the payload.
/// </summary>
/// <returns>Return true to restore null-value properties; false otherwise.</returns>
protected bool ShouldRestoreNullValues()
{
return this.inputContext.ShouldRestoreNullValues();
}

/// <summary>
/// Reads the next <see cref="ODataItem"/> from the message payload.
/// </summary>
Expand Down
Loading

0 comments on commit 954c798

Please sign in to comment.