From 88fa5ef211a323a66930452e75fce47db94e43d9 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Sun, 13 May 2018 23:59:56 -0700 Subject: [PATCH] Addressing CR comments. 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. --- .../ODataJsonLightPropertySerializer.cs | 23 ++- .../JsonLight/ODataJsonLightReader.cs | 128 ++++++++---- .../ODataJsonLightResourceDeserializer.cs | 17 +- .../JsonLight/ODataJsonLightWriter.cs | 20 +- .../Microsoft.OData.Core.cs | 3 - .../Microsoft.OData.Core.txt | 3 - src/Microsoft.OData.Core/ODataInputContext.cs | 9 + .../ODataMessageReader.cs | 9 +- src/Microsoft.OData.Core/ODataReaderCore.cs | 10 + .../Parameterized.Microsoft.OData.Core.cs | 8 - .../SelectedPropertiesNode.cs | 26 ++- .../WriteJsonWithoutModelTests.cs | 189 ++++++++++++++++++ ...AndValueJsonLightReaderIntegrationTests.cs | 162 +++++++++++++-- 13 files changed, 509 insertions(+), 98 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index 8fed1507b8..f0b7fa6871 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -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 /// - /// Whether to omit null property values. + /// Whether to omit null property values. + /// Value should be 'false' if writing delta payload, otherwise derived from request preference header. + /// /// The DuplicatePropertyNameChecker to use. internal void WriteProperties( IEdmStructuredType owningType, @@ -168,7 +170,9 @@ private bool IsOpenProperty(ODataProperty property) /// true when writing a top-level property; false for nested properties. /// 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. - /// whether to omit null property values for writing. + /// 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. + /// /// The DuplicatePropertyNameChecker to use. [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( @@ -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. @@ -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) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index 077cff9ba3..b9c49cbb21 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -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; @@ -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; - 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 readonlyEnumerable = - resource.Properties as ReadOnlyEnumerable; - 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() { 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; + } + + /// + /// Restore the null property value in the resource. + /// + /// Name of the property to restore the null value. + /// The resource state. + 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 readonlyEnumerable = + resource.Properties as ReadOnlyEnumerable; + 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() { property }); + } + } + /// /// Get all properties defined by the EDM structural type and its base types. /// diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs index 34b8372881..49080d223a 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs @@ -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())); @@ -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; diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs index 2d76f3ac28..38f5e78528 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs @@ -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; } diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs index cfe474ec3b..ea94bc7d37 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs @@ -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"; diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt index bf4eba7e90..105740fc13 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt @@ -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}'. diff --git a/src/Microsoft.OData.Core/ODataInputContext.cs b/src/Microsoft.OData.Core/ODataInputContext.cs index e4c3abd4db..321983d81f 100644 --- a/src/Microsoft.OData.Core/ODataInputContext.cs +++ b/src/Microsoft.OData.Core/ODataInputContext.cs @@ -646,6 +646,15 @@ internal Uri ResolveUri(Uri baseUri, Uri payloadUri) return null; } + /// + /// Returns whether properties of null values should be restored when materializing the payload. + /// + /// Return true to restore null-value properties; false otherwise. + internal bool ShouldRestoreNullValues() + { + return this.ReadingResponse && this.MessageReaderSettings.NullValuesOmitted; + } + /// /// Perform the actual cleanup work. /// diff --git a/src/Microsoft.OData.Core/ODataMessageReader.cs b/src/Microsoft.OData.Core/ODataMessageReader.cs index d28afb6f32..598b1ae311 100644 --- a/src/Microsoft.OData.Core/ODataMessageReader.cs +++ b/src/Microsoft.OData.Core/ODataMessageReader.cs @@ -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 diff --git a/src/Microsoft.OData.Core/ODataReaderCore.cs b/src/Microsoft.OData.Core/ODataReaderCore.cs index 1afac8a4ac..4953894389 100644 --- a/src/Microsoft.OData.Core/ODataReaderCore.cs +++ b/src/Microsoft.OData.Core/ODataReaderCore.cs @@ -664,6 +664,16 @@ protected void DecreaseResourceDepth() this.currentResourceDepth--; } + + /// + /// Returns whether properties of null values should be restored when materializing the payload. + /// + /// Return true to restore null-value properties; false otherwise. + protected bool ShouldRestoreNullValues() + { + return this.inputContext.ShouldRestoreNullValues(); + } + /// /// Reads the next from the message payload. /// diff --git a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs index 64f6695f77..b4586e0db0 100644 --- a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs @@ -1648,14 +1648,6 @@ internal static string HttpHeaderValueLexer_EndOfFileAfterSeparator(object p0, o return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.HttpHeaderValueLexer_EndOfFileAfterSeparator, p0, p1, p2, p3); } - /// - /// A string like "The value '{0}' is not a valid value for OData token '{1}' in HTTP Prefer or Preference-Applied headers." - /// - internal static string HttpPreferenceHeader_InvalidValueForToken(object p0, object p1) - { - return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.HttpPreferenceHeader_InvalidValueForToken, p0, p1); - } - /// /// A string like "The character set '{0}' is not supported." /// diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index c606d68694..3de3a34af7 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -340,10 +340,11 @@ internal IEnumerable GetSelectedNavigationProperties(IEd } /// - /// Gets all selected properties at the current node. + /// Gets all selected structured properties at the current node. /// /// The current structured type. - /// The selected properties at this node level. + /// The selected structured properties at this node level. + /// Result does not include the dynamic properties, which can be retrieved by GetSelectedDynamicProperties internal IEnumerable GetSelectedProperties(IEdmStructuredType structuredType) { if (this.selectionType == SelectionType.Empty) @@ -368,6 +369,27 @@ internal IEnumerable GetSelectedProperties(IEdmStructuredType stru return this.selectedProperties.Select(structuredType.FindProperty).OfType(); } + /// + /// Return names of dynamic properties if current structured node is open type. + /// + /// The current structured type. + /// The names of dynamic properties, empty if the current structured type is not open type. + internal IEnumerable GetSelectedDynamicProperties(IEdmStructuredType structuredType) + { + IEnumerable dynamicProperties = Enumerable.Empty(); + + if (structuredType.IsOpen && this.selectedProperties != null && this.selectedProperties.Count > 0) + { + dynamicProperties = this.selectedProperties.Select( + prop => structuredType.FindProperty(prop) == null + ? prop + : null) + .OfType(); + } + + return dynamicProperties; + } + /// /// Gets the selected stream properties for the current node. /// diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs index edb127832d..288d38bc7f 100644 --- a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs @@ -17,6 +17,8 @@ namespace Microsoft.Test.OData.Tests.Client.WriteJsonPayloadTests using Microsoft.Test.OData.Services.TestServices; using Microsoft.Test.OData.Tests.Client.Common; using Microsoft.VisualStudio.TestTools.UnitTesting; + using Microsoft.OData.Edm; + using Microsoft.OData.UriParser; /// /// Write various payload with/without EDM model in Atom/VerboseJosn/JsonLight formats. @@ -177,6 +179,193 @@ public void ExpandedCustomerEntryOmitNullValuesTest() } } + // Test provided by Mike Pizzo as omit-values=nulls requirement test cases related to + // dynamic properties and annotation-only properties. + [TestMethod] + public void Mikep_ReadWriteNullNavigationTest() + { + // set up model + var model = new EdmModel(); + var addressType = model.AddComplexType("test", "Address"); + addressType.AddStructuralProperty("city", EdmPrimitiveTypeKind.String, true); + var employeeType = model.AddEntityType("test", "Employee", null, false, true); + var key = employeeType.AddStructuralProperty("Name", EdmPrimitiveTypeKind.String, false); + employeeType.AddKeys(key); + employeeType.AddStructuralProperty("Title", EdmPrimitiveTypeKind.String, true); + employeeType.AddStructuralProperty("Address", new EdmComplexTypeReference(addressType, true)); + + employeeType.AddUnidirectionalNavigation( + new EdmNavigationPropertyInfo + { + Name = "Manager", + TargetMultiplicity = EdmMultiplicity.ZeroOrOne, + Target = employeeType + } + ); + var container = model.AddEntityContainer("test", "service"); + var employeeSet = container.AddEntitySet("employees", employeeType); + + // set up writer and reader + Uri serviceUri = new Uri("http://test/"); + + // string expectedNulls = "{\"@odata.context\":\"" + serviceUri + "$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity\",\"Name\":\"Fred\",\"Title@test.annotation\":\"annotationValue\",\"Title\":null,\"Dynamic\":null,\"DynamicAnnotated@test.annotation\":\"annotationValue\",\"DynamicAnnotated\":null,\"Address\":null,\"Manager\":null}"; + string expectedNulls = @"{ + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"",""Title"":null, + ""Dynamic"":null, + ""DynamicAnnotated@test.annotation"":""annotationValue"",""DynamicAnnotated"":null, + ""Address"":null,""Manager"":null + }"; + expectedNulls = Regex.Replace(expectedNulls, @"\s*", string.Empty, RegexOptions.Multiline); + Assert.IsTrue(expectedNulls.Contains(serviceUri.ToString())); + + // string expectedNoNulls = "{\"@odata.context\":\"" + serviceUri + "$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity\",\"Name\":\"Fred\",\"Title@test.annotation\":\"annotationValue\",\"DynamicAnnotated@test.annotation\":\"annotationValue\"}"; + string expectedNoNulls = @"{ + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"", + ""DynamicAnnotated@test.annotation"":""annotationValue"" + }"; + expectedNoNulls = Regex.Replace(expectedNoNulls, @"\s*", string.Empty, RegexOptions.Multiline); + Assert.IsTrue(expectedNoNulls.Contains(serviceUri.ToString())); + + var writeSettings = new ODataMessageWriterSettings() { BaseUri = serviceUri }; + writeSettings.ODataUri = new ODataUriParser(model, serviceUri, new Uri("employees?$select=Name,Title,Dynamic,Address&$expand=Manager", UriKind.Relative)).ParseUri(); + + foreach (bool omitNulls in new bool[] { false, true }) + { + // validate writing a null navigation property value + var stream = new MemoryStream(); + var responseMessage = new StreamResponseMessage(stream); + if (omitNulls) + { + responseMessage.SetHeader("Preference-Applied", "omit-values=nulls,odata.include-annotations=\"*\""); + } + else + { + responseMessage.SetHeader("Preference-Applied", "odata.include-annotations=\"*\""); + } + var messageWriter = new ODataMessageWriter(responseMessage, writeSettings, model); + var writer = messageWriter.CreateODataResourceWriter(employeeSet, employeeType); + writer.WriteStart(new ODataResource + { + Properties = new ODataProperty[] + { + new ODataProperty {Name = "Name", Value = "Fred" }, + new ODataProperty {Name = "Title", Value = new ODataNullValue(), + InstanceAnnotations = new ODataInstanceAnnotation[] { + new ODataInstanceAnnotation("test.annotation", new ODataPrimitiveValue( "annotationValue" )) + } + }, + new ODataProperty {Name = "Dynamic", Value = new ODataNullValue() }, + new ODataProperty {Name = "DynamicAnnotated", Value = new ODataNullValue(), + InstanceAnnotations = new ODataInstanceAnnotation[] { + new ODataInstanceAnnotation("test.annotation", new ODataPrimitiveValue( "annotationValue" )) + } + } + } + }); + + // write address + writer.WriteStart(new ODataNestedResourceInfo + { + Name = "Address", + IsCollection = false + }); + writer.WriteStart((ODataResource)null); + writer.WriteEnd(); //address + writer.WriteEnd(); //address nestedInfo + + // write manager + writer.WriteStart(new ODataNestedResourceInfo + { + Name = "Manager", + IsCollection = false + }); + writer.WriteStart((ODataResource)null); + writer.WriteEnd(); // manager resource + writer.WriteEnd(); // manager nested info + + writer.WriteEnd(); // resource + stream.Flush(); + + // compare written stream to expected stream + stream.Seek(0, SeekOrigin.Begin); + var streamReader = new StreamReader(stream); + Assert.AreEqual(omitNulls ? expectedNoNulls : expectedNulls, streamReader.ReadToEnd(), String.Format("Did not generate expected string when {0}omitting nulls", omitNulls ? "" : "not ")); + + // validate reading back the stream + var readSettings = new ODataMessageReaderSettings() { BaseUri = serviceUri, ReadUntypedAsString = false }; + stream.Seek(0, SeekOrigin.Begin); + var messageReader = new ODataMessageReader(responseMessage, readSettings, model); + var reader = messageReader.CreateODataResourceReader(employeeSet, employeeType); + string reading = "Employee"; + bool addressReturned = false; + bool managerReturned = false; + ODataResource employeeResource = null; + ODataResource managerResource = null; + ODataResource addressResource = null; + while (reader.Read()) + { + switch (reader.State) + { + case ODataReaderState.ResourceEnd: + switch (reading) + { + case "Manager": + managerResource = reader.Item as ODataResource; + managerReturned = true; + break; + case "Address": + addressResource = reader.Item as ODataResource; + addressReturned = true; + break; + case "Employee": + employeeResource = reader.Item as ODataResource; + break; + } + break; + case ODataReaderState.NestedResourceInfoStart: + reading = ((ODataNestedResourceInfo)reader.Item).Name; + break; + case ODataReaderState.NestedResourceInfoEnd: + reading = "Employee"; + break; + } + } + + if (!omitNulls) + { + Assert.IsTrue(managerReturned, "Manager not returned when {0}omitting nulls", omitNulls ? "" : "not "); + } + Assert.IsNull(managerResource, "Manager resource not null when {0}omitting nulls", omitNulls ? "" : "not "); + + if (!omitNulls) + { + Assert.IsTrue(addressReturned, "Address not returned when {0}omitting nulls", omitNulls ? "" : "not "); + } + Assert.IsNull(addressResource, "Address resource not null when {0}omitting nulls", omitNulls ? "" : "not "); + + Assert.IsNotNull(employeeResource, "Employee should not be null"); + var titleProperty = employeeResource.Properties.FirstOrDefault(p => p.Name == "Title"); + Assert.IsNotNull(titleProperty, "Title property should not be null"); + Assert.IsNull(titleProperty.Value, "Title property value should be null"); + var titleAnnotation = titleProperty.InstanceAnnotations.FirstOrDefault(a => a.Name == "test.annotation"); + Assert.IsNotNull(titleAnnotation, "Title property missing the test.annotation"); + Assert.AreEqual(((ODataPrimitiveValue)titleAnnotation.Value).Value.ToString(), "annotationValue"); + var dynamicProperty = employeeResource.Properties.FirstOrDefault(p => p.Name == "Dynamic"); + Assert.IsNotNull(dynamicProperty, "Dynamic property should not be null"); + Assert.IsNull(dynamicProperty.Value, "Dynamic property value should be null"); + var dynamicAnnotatedProperty = employeeResource.Properties.FirstOrDefault(p => p.Name == "DynamicAnnotated"); + Assert.IsNotNull(dynamicAnnotatedProperty, "DynamicAnnotated property should not be null"); + Assert.IsNull(dynamicAnnotatedProperty.Value, "DynamicAnnotated property value should be null"); + var dynamicAnnotation = dynamicAnnotatedProperty.InstanceAnnotations.FirstOrDefault(a => a.Name == "test.annotation"); + Assert.IsNotNull(dynamicAnnotation, "DynamicAnnotated property missing the test.annotation"); + Assert.AreEqual(((ODataPrimitiveValue)dynamicAnnotation.Value).Value.ToString(), "annotationValue"); + } + } + /// /// Write an entry containing stream, named stream /// diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/IntegrationTests/Reader/JsonLight/PropertyAndValueJsonLightReaderIntegrationTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/IntegrationTests/Reader/JsonLight/PropertyAndValueJsonLightReaderIntegrationTests.cs index 1fa92ed8cc..5aec9c75fc 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/IntegrationTests/Reader/JsonLight/PropertyAndValueJsonLightReaderIntegrationTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/IntegrationTests/Reader/JsonLight/PropertyAndValueJsonLightReaderIntegrationTests.cs @@ -13,6 +13,7 @@ using FluentAssertions; using Microsoft.OData.Tests.JsonLight; using Microsoft.OData.Edm; +using Microsoft.OData.Tests.Evaluation; using Microsoft.Test.OData.DependencyInjection; using Xunit; using ODataErrorStrings = Microsoft.OData.Strings; @@ -412,16 +413,8 @@ public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues(bool bNul .Value.Should().Be(0); person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)) .Value.Should().Be(60.5); - if (bNullValuesOmitted) - { - // Omitted value should be restored to null. - person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) - .Value.Should().BeNull(); - } - else - { - person.Properties.Any(s => string.Equals(s.Name, "Education",StringComparison.Ordinal)).Should().BeFalse(); - } + + person.Properties.Any(s => string.Equals(s.Name, "Education",StringComparison.Ordinal)).Should().BeFalse(); ODataResource address = entries[1]; address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.Ordinal)) @@ -1024,16 +1017,8 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) .Value.Should().Be(0); - if (bNullValuesOmitted) - { - // selected null-able property should be restored as null. - person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Address", StringComparison.Ordinal)) - .Value.Should().BeNull(); - } - else - { - person.Properties.Any(s => string.Equals(s.Name, "Address", StringComparison.Ordinal)).Should().BeFalse(); - } + // Complex type as nested resource is not restored as null property. + person.Properties.Any(s => string.Equals(s.Name, "Address", StringComparison.Ordinal)).Should().BeFalse(); // null-able but not selected properties and not-null-able properties should not be restored. person.Properties.Any(s => string.Equals(s.Name, "Height", StringComparison.Ordinal)).Should().BeFalse(); @@ -1108,6 +1093,143 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); } + [Fact] + public void ReadingNormalDynamicCollectionPropertyInOpenStructuralTypeShouldWork() + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = GetDynamicModel(out entityType, out entitySet); + + const string payload = + "{" + + "\"@odata.context\":\"http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20MyDynamic,%20Address/MyDynamic1)/$entity\"," + + "\"@odata.id\":\"http://mytest\"," + + "\"Id\":0," + + "\"Test@odata.type\":\"#Collection(Edm.String)\"," + + "\"Test\":null," + + "\"MyDynamic@odata.type\":\"#Collection(Edm.String)\"," + + "\"MyDynamic\":[\"mystr\"]," + + "\"Address\":{\"CountryRegion\":\"China\",\"Test1@odata.type\":\"#Collection(Edm.Int32)\",\"Test1\":null, \"MyDynamic1@odata.type\":\"#Collection(Edm.Int32)\",\"MyDynamic1\":[1]}" + + "}"; + + List entries = new List(); + List nestedResourceInfos = new List(); + + this.ReadEntryPayload(model, payload, entitySet, entityType, + reader => + { + switch (reader.State) + { + case ODataReaderState.ResourceStart: + entries.Add(reader.Item as ODataResource); + break; + case ODataReaderState.NestedResourceInfoStart: + nestedResourceInfos.Add(reader.Item as ODataNestedResourceInfo); + break; + default: + break; + } + } + ); + + entries.Count.Should().Be(2); + nestedResourceInfos.Count.Should().Be(1); + + ODataResource person = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); + person.Should().NotBeNull(); + person.Properties.FirstOrDefault(p => string.Equals(p.Name, "MyDynamic", StringComparison.Ordinal)).Value.Should().NotBeNull(); + + ODataResource address = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Address", StringComparison.Ordinal)); + address.Should().NotBeNull(); + address.Properties.FirstOrDefault(p => string.Equals(p.Name, "MyDynamic1", StringComparison.Ordinal)) + .Value.Should().NotBeNull(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingNullableDynamicCollectionPropertyInOpenStructuralTypeShouldWork(bool bNullValuesOmitted) + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = GetDynamicModel(out entityType, out entitySet); + + const string payload = + "{" + + "\"@odata.context\":\"http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20MyDynamic,%20Address/MyDynamic1)/$entity\"," + + "\"@odata.id\":\"http://mytest\"," + + "\"Id\":0," + + "\"Test@odata.type\":\"#Collection(Edm.String)\"," + + "\"Test\":null," + + "\"Address\":{\"CountryRegion\":\"China\",\"Test1@odata.type\":\"#Collection(Edm.Int32)\",\"Test1\":null}" + + "}"; + + List entries = new List(); + List nestedResourceInfos = new List(); + + this.ReadEntryPayload(model, payload, entitySet, entityType, + reader => + { + switch (reader.State) + { + case ODataReaderState.ResourceStart: + entries.Add(reader.Item as ODataResource); + break; + case ODataReaderState.NestedResourceInfoStart: + nestedResourceInfos.Add(reader.Item as ODataNestedResourceInfo); + break; + default: + break; + } + }, + nullValuesOmitted: bNullValuesOmitted); + + entries.Count.Should().Be(2); + nestedResourceInfos.Count.Should().Be(1); + + ODataResource person = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); + person.Should().NotBeNull(); + if (bNullValuesOmitted) + { + person.Properties.Any(p => string.Equals(p.Name, "MyDynamic", StringComparison.Ordinal)).Should().BeTrue(); + } + else + { + person.Properties.Any(p => string.Equals(p.Name, "MyDynamic", StringComparison.Ordinal)).Should().BeFalse(); + } + + ODataResource address =entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Address", StringComparison.Ordinal)); + address.Should().NotBeNull(); + if (bNullValuesOmitted) + { + address.Properties.Any(p => string.Equals(p.Name, "MyDynamic1", StringComparison.Ordinal)).Should().BeTrue(); + } + else + { + address.Properties.Any(p => string.Equals(p.Name, "MyDynamic1", StringComparison.Ordinal)).Should().BeFalse(); + } + } + + private EdmModel GetDynamicModel(out EdmEntityType entityType, out EdmEntitySet entitySet) + { + EdmModel model = new EdmModel(); + + EdmComplexType complexType = new EdmComplexType("NS", "Address", null, false, true/*isOpen*/); + complexType.AddStructuralProperty("CountryRegion", EdmPrimitiveTypeKind.String); + model.AddElement(complexType); + + entityType = new EdmEntityType("NS", "Person", null, false, true/*isOpen*/); + entityType.AddKeys(entityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32)); + entityType.AddStructuralProperty("Address", new EdmComplexTypeReference(complexType, false)); + model.AddElement(entityType); + + EdmEntityContainer container = new EdmEntityContainer("EntityNs", "MyContainer"); + entitySet = container.AddEntitySet("People", entityType); + model.AddElement(container); + + return model; + } + [Theory] [InlineData(true)] [InlineData(false)]