From 6f45c7c07541d9f8b9ba134a7109b18969daac0c Mon Sep 17 00:00:00 2001 From: Biao Li Date: Tue, 6 Feb 2018 17:09:56 -0800 Subject: [PATCH 01/16] port from pr944 kosinsky:users/kokosins/IgnoreNullValuesV7, commit b27eb32 --- .../ODataJsonLightPropertySerializer.cs | 2 +- .../ODataMessageWriterSettings.cs | 11 +- .../PublicApi/PublicApi.bsl | 1 + ...noreNullPropertiesInEntryTest.approved.txt | 32 +++++ .../JsonLight/JsonLightEntryWriterTests.cs | 120 +++++++++++++++--- ...osoft.Test.Taupo.OData.Writer.Tests.csproj | 3 + 6 files changed, 152 insertions(+), 17 deletions(-) create mode 100644 test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index bf15f45380..8afccfdd7d 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -382,7 +382,7 @@ private void WriteNullProperty( throw new ODataException(Strings.ODataMessageWriter_CannotWriteTopLevelNull); } } - else + else if (!this.MessageWriterSettings.IgnoreNullValues) { this.JsonWriter.WriteName(property.Name); this.JsonLightValueSerializer.WriteNullValue(); diff --git a/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs index d287c43e22..47d54965ce 100644 --- a/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs +++ b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs @@ -183,6 +183,14 @@ public ODataUri ODataUri /// internal bool ThrowOnDuplicatePropertyNames { get; private set; } + /// + /// Don't serialize null values + /// + /// + /// Default valus is false, that means serialize null values. + /// + public bool IgnoreNullValues { get; set; } + /// /// Returns whether ThrowOnUndeclaredPropertyForNonOpenType validation setting is enabled. /// @@ -404,8 +412,9 @@ private void CopyFrom(ODataMessageWriterSettings other) this.shouldIncludeAnnotation = other.shouldIncludeAnnotation; this.useFormat = other.useFormat; this.Version = other.Version; + this.IgnoreNullValues = other.IgnoreNullValues; this.LibraryCompatibility = other.LibraryCompatibility; - + this.validations = other.validations; this.ThrowIfTypeConflictsWithMetadata = other.ThrowIfTypeConflictsWithMetadata; this.ThrowOnDuplicatePropertyNames = other.ThrowOnDuplicatePropertyNames; diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl b/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl index 3d56b2704f..4c2c894a0e 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl @@ -5189,6 +5189,7 @@ public sealed class Microsoft.OData.ODataMessageWriterSettings { System.Uri BaseUri { public get; public set; } bool EnableCharactersCheck { public get; public set; } bool EnableMessageStreamDisposal { public get; public set; } + bool IgnoreNullValues { public get; public set; } string JsonPCallback { public get; public set; } Microsoft.OData.ODataLibraryCompatibility LibraryCompatibility { public get; public set; } Microsoft.OData.ODataMessageQuotas MessageQuotas { public get; public set; } diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt new file mode 100644 index 0000000000..3578c7bcee --- /dev/null +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt @@ -0,0 +1,32 @@ +Combination: 1; TestConfiguration = Format: JsonLight, Request: True, Synchronous: True +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":42,"Hobby":"Hiking"} + +Combination: 2; TestConfiguration = Format: JsonLight, Request: False, Synchronous: True +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":42,"Hobby":"Hiking"} + +Combination: 3; TestConfiguration = Format: JsonLight, Request: True, Synchronous: False +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":42,"Hobby":"Hiking"} + +Combination: 4; TestConfiguration = Format: JsonLight, Request: False, Synchronous: False +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":42,"Hobby":"Hiking"} + +Combination: 5; TestConfiguration = Format: JsonLight, Request: True, Synchronous: True +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44} + +Combination: 6; TestConfiguration = Format: JsonLight, Request: False, Synchronous: True +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44} + +Combination: 7; TestConfiguration = Format: JsonLight, Request: True, Synchronous: False +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44} + +Combination: 8; TestConfiguration = Format: JsonLight, Request: False, Synchronous: False +Model Present: true +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44} + diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs index f7491a77e3..3056e96869 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs @@ -48,7 +48,7 @@ public void PayloadOrderTest() var nonMLEBaseType = new EdmEntityType("TestModel", "NonMLEBaseType"); nonMLEBaseType.AddKeys(nonMLEBaseType.AddStructuralProperty("ID", EdmCoreModel.Instance.GetInt32(false))); model.AddElement(nonMLEBaseType); - var nonMLESet = container.AddEntitySet("NonMLESet", nonMLEBaseType); + var nonMLESet = container.AddEntitySet("NonMLESet", nonMLEBaseType); var nonMLEType = new EdmEntityType("TestModel", "NonMLEType", nonMLEBaseType); nonMLEType.AddStructuralProperty("Name", EdmCoreModel.Instance.GetString(true)); @@ -66,13 +66,13 @@ public void PayloadOrderTest() var mleType = new EdmEntityType("TestModel", "MLEType", mleBaseType, false, false, true); mleType.AddStructuralProperty("Name", EdmCoreModel.Instance.GetString(true)); mleType.AddStructuralProperty("Description", EdmCoreModel.Instance.GetString(true)); - mleType.AddStructuralProperty("StreamProperty", EdmPrimitiveTypeKind.Stream, isNullable:false); + mleType.AddStructuralProperty("StreamProperty", EdmPrimitiveTypeKind.Stream, isNullable: false); var mleNav = mleType.AddUnidirectionalNavigation(new EdmNavigationPropertyInfo { Name = "NavProp", Target = otherType, TargetMultiplicity = EdmMultiplicity.Many }); mleSet.AddNavigationTarget(mleNav, otherset); model.AddElement(mleType); IEnumerable testCases = new[] - { + { new EntryPayloadTestCase { DebugDescription = "TypeName at the beginning, nothing else", @@ -96,7 +96,7 @@ public void PayloadOrderTest() new ODataProperty { Name = "Name", Value = "test" }, } } - .WithAnnotation(new WriteEntryCallbacksAnnotation + .WithAnnotation(new WriteEntryCallbacksAnnotation { BeforeWriteStartCallback = (entry) => { entry.TypeName = "TestModel.MLEType"; }, BeforeWriteEndCallback = (entry) => { entry.TypeName = "NonExistingType"; } @@ -197,16 +197,16 @@ public void PayloadOrderTest() new ODataProperty { Name = "ID", Value = (int)42 }, new ODataProperty { Name = "Name", Value = "test" }, } - }.WithAnnotation(new WriteEntryCallbacksAnnotation + }.WithAnnotation(new WriteEntryCallbacksAnnotation { BeforeWriteStartCallback = (entry) => - { + { entry.EditLink = null; entry.MediaResource.EditLink = null; entry.MediaResource.ETag = null; entry.MediaResource.ContentType = null; }, - BeforeWriteEndCallback = (entry) => + BeforeWriteEndCallback = (entry) => { entry.EditLink = new Uri("http://odata.org/editlink"); entry.MediaResource.EditLink = new Uri("http://odata.org/mediaeditlink"); @@ -240,7 +240,7 @@ public void PayloadOrderTest() testCase.Items, tc => new JsonWriterTestExpectedResults(this.Settings.ExpectedResultSettings) { - Json = string.Format(CultureInfo.InvariantCulture, testCase.Json, + Json = string.Format(CultureInfo.InvariantCulture, testCase.Json, string.Empty, JsonLightWriterUtils.GetMetadataUrlPropertyForEntry(testCase.EntitySet.Name) + ","), FragmentExtractor = (result) => result.RemoveAllAnnotations(true) @@ -338,7 +338,7 @@ public void ActionAndFunctionPayloadOrderTest() model.AddElement(nonMLEType); container.AddEntitySet("NonMLEType", nonMLEType); - ODataAction action = new ODataAction + ODataAction action = new ODataAction { Metadata = new Uri("http://odata.org/test/$metadata#defaultAction"), Title = "Default Action", @@ -432,6 +432,96 @@ public void ActionAndFunctionPayloadOrderTest() }); } + [TestMethod, Variation(Description = "Test skipping null values when writing JSON Lite entries with IgnoreNullValues = true.")] + public void IgnoreNullPropertiesInEntryTest() + { + EdmModel model = new EdmModel(); + var container = new EdmEntityContainer("TestModel", "TestContainer"); + model.AddElement(container); + + var customerType = new EdmEntityType("TestModel", "CustomerType", null, false, isOpen: false); + customerType.AddKeys(customerType.AddStructuralProperty("ID", EdmCoreModel.Instance.GetInt32(false))); + customerType.AddKeys(customerType.AddStructuralProperty("Hobby", EdmCoreModel.Instance.GetString(true))); + model.AddElement(customerType); + var customerSet = container.AddEntitySet("CustomerSet", customerType); + + var addressType = new EdmComplexType("TestModel", "AddressType"); + addressType.AddStructuralProperty("Street", EdmCoreModel.Instance.GetStream(true)); + model.AddElement(addressType); + + IEnumerable testCases = new[] + { + new EntryPayloadTestCase + { + DebugDescription = "Customer instance with all properties set.", + Items = new[] { new ODataResource() + { + TypeName = "TestModel.CustomerType", + Properties = new ODataProperty[] + { + new ODataProperty { Name = "ID", Value = (int)42 }, + new ODataProperty { Name = "Hobby", Value = "Hiking" }, + } + } }, + Model = model, + EntitySet = customerSet, + Json = string.Join("$(NL)", + "{{", + "{0}\"ID\":\"42\", \"Hobby\":\"Hiking\"", + "}}") + }, + new EntryPayloadTestCase + { + DebugDescription = "Customer instance without Hobby.", + Items = new[] {new ODataResource() + { + TypeName = "TestModel.CustomerType", + Properties = new ODataProperty[] + { + new ODataProperty { Name = "ID", Value = (int)44 }, + new ODataProperty { Name = "Hobby", Value = null }, + } + } }, + Model = model, + EntitySet = customerSet, + Json = string.Join("$(NL)", + "{{", + "{0}\"ID\":\"44\"", + "}}") + }, + + }; + + IEnumerable> testDescriptors = testCases.Select(testCase => + new PayloadWriterTestDescriptor( + this.Settings, + testCase.Items, + tc => new JsonWriterTestExpectedResults(this.Settings.ExpectedResultSettings) + { + Json = string.Format( + CultureInfo.InvariantCulture, + testCase.Json, + JsonLightWriterUtils.GetMetadataUrlPropertyForEntry(testCase.EntitySet.Name) + ","), + FragmentExtractor = (result) => result.RemoveAllAnnotations(true) + }) + { + DebugDescription = testCase.DebugDescription, + Model = testCase.Model, + PayloadEdmElementContainer = testCase.EntitySet, + PayloadEdmElementType = testCase.EntityType, + SkipTestConfiguration = testCase.SkipTestConfiguration + }); + + this.CombinatorialEngineProvider.RunCombinations( + testDescriptors, + this.WriterTestConfigurationProvider.JsonLightFormatConfigurationsWithIndent, + (testDescriptor, testConfiguration) => + { + testConfiguration.MessageWriterSettings.IgnoreNullValues = true; + TestWriterUtils.WriteAndVerifyODataEdmPayload(testDescriptor, testConfiguration, this.Assert, this.Logger); + }); + } + [TestMethod, Variation(Description = "Test correct serialization format when writing JSON Lite entries with open properties.")] public void OpenPropertiesInEntryTest() { @@ -439,7 +529,7 @@ public void OpenPropertiesInEntryTest() var container = new EdmEntityContainer("TestModel", "TestContainer"); model.AddElement(container); - var openCustomerType = new EdmEntityType("TestModel", "OpenCustomerType", null, false, isOpen:true); + var openCustomerType = new EdmEntityType("TestModel", "OpenCustomerType", null, false, isOpen: true); openCustomerType.AddKeys(openCustomerType.AddStructuralProperty("ID", EdmCoreModel.Instance.GetInt32(false))); model.AddElement(openCustomerType); var customerSet = container.AddEntitySet("CustomerSet", openCustomerType); @@ -494,7 +584,7 @@ public void OpenPropertiesInEntryTest() "\"type\":\"name\",\"properties\":{{", "\"name\":\"EPSG:4326\"", "}}", - "}}", + "}}", "}}", "}}") }, @@ -526,7 +616,7 @@ public void OpenPropertiesInEntryTest() tc => new JsonWriterTestExpectedResults(this.Settings.ExpectedResultSettings) { Json = string.Format( - CultureInfo.InvariantCulture, + CultureInfo.InvariantCulture, testCase.Json, JsonLightWriterUtils.GetMetadataUrlPropertyForEntry(testCase.EntitySet.Name) + ","), FragmentExtractor = (result) => result.RemoveAllAnnotations(true) @@ -561,7 +651,7 @@ public void SpatialPropertiesInEntryTest() customerType.AddStructuralProperty("Location2", EdmCoreModel.Instance.GetSpatial(EdmPrimitiveTypeKind.GeographyPoint, false)); model.AddElement(customerType); var customerSet = container.AddEntitySet("CustomerSet", customerType); - + ISpatial pointValue = GeographyFactory.Point(32.0, -100.0).Build(); IEnumerable testCases = new[] @@ -590,7 +680,7 @@ public void SpatialPropertiesInEntryTest() "\"type\":\"name\",\"properties\":{{", "\"name\":\"EPSG:4326\"", "}}", - "}}", + "}}", "}}", "}}") }, @@ -617,7 +707,7 @@ public void SpatialPropertiesInEntryTest() "\"type\":\"name\",\"properties\":{{", "\"name\":\"EPSG:4326\"", "}}", - "}}", + "}}", "}}", "}}") }, diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj index d9feb236ef..d1d7ea011e 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj @@ -182,6 +182,9 @@ PreserveNewest + + PreserveNewest + PreserveNewest From 2190c88b2fff4d44b56edf8c9e97b31f88c3320e Mon Sep 17 00:00:00 2001 From: Biao Li Date: Sun, 4 Mar 2018 16:17:29 -0800 Subject: [PATCH 02/16] omitNullValues: response serialization/deserialization, Preference headers handling. --- .../ODataJsonLightPropertySerializer.cs | 32 +- .../JsonLight/ODataJsonLightReader.cs | 88 +++ .../JsonLight/ODataJsonLightWriter.cs | 2 + .../Microsoft.OData.Core.cs | 3 + .../Microsoft.OData.Core.txt | 2 + src/Microsoft.OData.Core/ODataConstants.cs | 13 + .../ODataMessageReader.cs | 11 + .../ODataMessageReaderSettings.cs | 6 + .../ODataMessageWriter.cs | 8 + .../ODataMessageWriterSettings.cs | 9 +- .../ODataPreferenceHeader.cs | 41 ++ src/Microsoft.OData.Core/ODataWriterCore.cs | 13 + .../Parameterized.Microsoft.OData.Core.cs | 8 + .../SelectedPropertiesNode.cs | 37 +- ...AndValueJsonLightReaderIntegrationTests.cs | 503 +++++++++++++++--- .../ODataJsonLightDeltaWriterTests.cs | 184 ++++++- .../SelectedPropertiesNodeTests.cs | 31 +- .../PublicApi/PublicApi.bsl | 3 +- ...noreNullPropertiesInEntryTest.approved.txt | 4 +- .../JsonLight/JsonLightEntryWriterTests.cs | 40 +- .../OData.Writer.Tests/TestWriterUtils.cs | 29 +- 21 files changed, 967 insertions(+), 100 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index 8afccfdd7d..2bce810611 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -84,6 +84,7 @@ internal void WriteTopLevelProperty(ODataProperty property) null /*owningType*/, true /* isTopLevel */, false /* allowStreamProperty */, + false /*omitNullValues */, this.CreateDuplicatePropertyNameChecker()); this.JsonLightValueSerializer.AssertRecursionDepthIsZero(); @@ -91,6 +92,26 @@ internal void WriteTopLevelProperty(ODataProperty property) }); } + /// + /// This is the backward-compatible method to write property names and value pairs + /// without omitting null property values. + /// + /// The of the resource (or null if not metadata is available). + /// The enumeration of properties to write out. + /// + /// 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 + /// + /// The DuplicatePropertyNameChecker to use. + internal void WriteProperties( + IEdmStructuredType owningType, + IEnumerable properties, + bool isComplexValue, + IDuplicatePropertyNameChecker duplicatePropertyNameChecker) + { + this.WriteProperties(owningType, properties, isComplexValue, /*omitNullValues*/ false, duplicatePropertyNameChecker); + } + /// /// Writes property names and value pairs. /// @@ -100,11 +121,13 @@ 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. /// The DuplicatePropertyNameChecker to use. internal void WriteProperties( IEdmStructuredType owningType, IEnumerable properties, bool isComplexValue, + bool omitNullValues, IDuplicatePropertyNameChecker duplicatePropertyNameChecker) { if (properties == null) @@ -119,6 +142,7 @@ internal void WriteProperties( owningType, false /* isTopLevel */, !isComplexValue, + omitNullValues, duplicatePropertyNameChecker); } } @@ -171,6 +195,7 @@ private void WriteProperty( IEdmStructuredType owningType, bool isTopLevel, bool allowStreamProperty, + bool omitNullValues, IDuplicatePropertyNameChecker duplicatePropertyNameChecker) { WriterValidationUtils.ValidatePropertyNotNull(property); @@ -229,7 +254,7 @@ private void WriteProperty( if (value is ODataNullValue || value == null) { - this.WriteNullProperty(property); + this.WriteNullProperty(property, omitNullValues); return; } @@ -357,7 +382,8 @@ private void WriteStreamReferenceProperty(string propertyName, ODataStreamRefere /// /// The property to write out. private void WriteNullProperty( - ODataProperty property) + ODataProperty property, + bool omitNullValues) { this.WriterValidator.ValidateNullPropertyValue( this.currentPropertyInfo.MetadataType.TypeReference, property.Name, @@ -382,7 +408,7 @@ private void WriteNullProperty( throw new ODataException(Strings.ODataMessageWriter_CannotWriteTopLevelNull); } } - else if (!this.MessageWriterSettings.IgnoreNullValues) + else if (!omitNullValues) { this.JsonWriter.WriteName(property.Name); this.JsonLightValueSerializer.WriteNullValue(); diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index db2a72ba80..94dfe4efaf 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -2232,6 +2232,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, @@ -2256,6 +2259,91 @@ private void EndEntry() } } + /// + /// For response de-serialization, restore the null values for properties that are omitted + /// by the omit-values=nulls preference header. + /// + 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) + { + IODataJsonLightReaderResourceState resourceState = this.CurrentResourceState; + IEdmStructuredType edmStructuredType = resourceState.ResourceType; + + if (resourceState.SelectedProperties == SelectedPropertiesNode.Empty) + { + return; + } + else + { + if (resourceState.Resource != null && edmStructuredType != null) + { + ODataResourceBase resource = resourceState.Resource; + + IEnumerable selectedProperties; + if (resourceState.SelectedProperties == SelectedPropertiesNode.EntireSubtree) + { + selectedProperties = edmStructuredType.DeclaredProperties; + } + 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).Values) + .Distinct(); + } + + foreach (IEdmProperty currentProperty in selectedProperties) + { + Debug.Assert(currentProperty.Type != null, "currentProperty.Type != null"); + if (!currentProperty.Type.IsNullable) + { + // Skip declared properties that are not null-able types. + 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}); + } + } + } + } + } + } + } + /// /// Add info resolved from context url to current scope. /// diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs index d05623db7f..2d76f3ac28 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs @@ -331,6 +331,7 @@ protected override void StartResource(ODataResource resource) this.ResourceType, resource.Properties, false /* isComplexValue */, + this.ShouldOmitNullValues(), this.DuplicatePropertyNameChecker); this.jsonLightResourceSerializer.JsonLightValueSerializer.AssertRecursionDepthIsZero(); @@ -1228,6 +1229,7 @@ private void WriteDeltaResourceProperties(IEnumerable properties) this.ResourceType, properties, false /* isComplexValue */, + false /* omitNullValues */, this.DuplicatePropertyNameChecker); this.jsonLightResourceSerializer.JsonLightValueSerializer.AssertRecursionDepthIsZero(); } diff --git a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs index 452a705ec4..7c0ca67898 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.cs @@ -272,6 +272,9 @@ 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 b4b10e8f0e..6fe1609fad 100644 --- a/src/Microsoft.OData.Core/Microsoft.OData.Core.txt +++ b/src/Microsoft.OData.Core/Microsoft.OData.Core.txt @@ -242,6 +242,8 @@ HttpHeaderValueLexer_FailedToReadTokenOrQuotedString=An error occurred when pars 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/ODataConstants.cs b/src/Microsoft.OData.Core/ODataConstants.cs index e0d8069b5d..661afa5135 100644 --- a/src/Microsoft.OData.Core/ODataConstants.cs +++ b/src/Microsoft.OData.Core/ODataConstants.cs @@ -205,6 +205,19 @@ internal static class ODataInternalConstants /// The $deletedLink token indicates delta deleted link. internal const string ContextUriDeletedLink = UriSegmentSeparator + DeletedLink; + #endregion Context URL + + #region Preference Headers + /// + /// Valid value for OmitValuesPreferenceToken indicating nulls are omitted. + /// + internal const string OmitValuesNulls = "nulls"; + + /// + /// Valid value for OmitValuesPreferenceToken indicating defaults are omitted. + /// + internal const string OmitValuesDefaults = "defaults"; + #endregion Preference Headers } } diff --git a/src/Microsoft.OData.Core/ODataMessageReader.cs b/src/Microsoft.OData.Core/ODataMessageReader.cs index ede858649b..d28afb6f32 100644 --- a/src/Microsoft.OData.Core/ODataMessageReader.cs +++ b/src/Microsoft.OData.Core/ODataMessageReader.cs @@ -169,6 +169,17 @@ 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. + this.settings.NullValuesOmitted = false; + string omitValuePreferenceApplied = responseMessage.PreferenceAppliedHeader().OmitValues; + if (omitValuePreferenceApplied != null) + { + // 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); + } + // If the Preference-Applied header on the response message contains an annotation filter, we set the filter // to the reader settings if it's not already set, so that we would only read annotations that satisfy the filter. string annotationFilter = responseMessage.PreferenceAppliedHeader().AnnotationFilter; diff --git a/src/Microsoft.OData.Core/ODataMessageReaderSettings.cs b/src/Microsoft.OData.Core/ODataMessageReaderSettings.cs index b168c674ad..b2398e3794 100644 --- a/src/Microsoft.OData.Core/ODataMessageReaderSettings.cs +++ b/src/Microsoft.OData.Core/ODataMessageReaderSettings.cs @@ -202,6 +202,11 @@ public ODataMessageQuotas MessageQuotas /// internal bool ThrowOnUndeclaredPropertyForNonOpenType { get; private set; } + /// + /// True if null values are omitted, per the omit-values parameter in Preference header of the message. + /// + internal bool NullValuesOmitted { get; set; } + /// /// Creates a shallow copy of this . /// @@ -259,6 +264,7 @@ private void CopyFrom(ODataMessageReaderSettings other) this.MaxProtocolVersion = other.MaxProtocolVersion; this.ReadUntypedAsString = other.ReadUntypedAsString; this.ShouldIncludeAnnotation = other.ShouldIncludeAnnotation; + this.NullValuesOmitted = other.NullValuesOmitted; this.validations = other.validations; this.ThrowOnDuplicatePropertyNames = other.ThrowOnDuplicatePropertyNames; this.ThrowIfTypeConflictsWithMetadata = other.ThrowIfTypeConflictsWithMetadata; diff --git a/src/Microsoft.OData.Core/ODataMessageWriter.cs b/src/Microsoft.OData.Core/ODataMessageWriter.cs index d0a5788699..b13010ba53 100644 --- a/src/Microsoft.OData.Core/ODataMessageWriter.cs +++ b/src/Microsoft.OData.Core/ODataMessageWriter.cs @@ -151,6 +151,14 @@ public ODataMessageWriter(IODataResponseMessage responseMessage, ODataMessageWri WriterValidationUtils.ValidateMessageWriterSettings(this.settings, this.writingResponse); this.message = new ODataResponseMessage(responseMessage, /*writing*/ true, this.settings.EnableMessageStreamDisposal, /*maxMessageSize*/ -1); + // Set the Preference-Applied header's parameter omit-values=nulls per writer settings. + // For response writing, source of specifying omit-null-values preference is the settings object + // from caller(the OData service). + if (this.settings.OmitNullValues) + { + responseMessage.PreferenceAppliedHeader().OmitValues = ODataConstants.OmitValuesNulls; + } + // If the Preference-Applied header on the response message contains an annotation filter, we set the filter // to the writer settings so that we would only write annotations that satisfy the filter. string annotationFilter = responseMessage.PreferenceAppliedHeader().AnnotationFilter; diff --git a/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs index 47d54965ce..db20f4f8a9 100644 --- a/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs +++ b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs @@ -184,12 +184,12 @@ public ODataUri ODataUri internal bool ThrowOnDuplicatePropertyNames { get; private set; } /// - /// Don't serialize null values + /// Whether to omit null values when writing response. /// /// - /// Default valus is false, that means serialize null values. + /// Default value is false, that means serialize null values. /// - public bool IgnoreNullValues { get; set; } + public bool OmitNullValues { get; set; } /// /// Returns whether ThrowOnUndeclaredPropertyForNonOpenType validation setting is enabled. @@ -412,9 +412,8 @@ private void CopyFrom(ODataMessageWriterSettings other) this.shouldIncludeAnnotation = other.shouldIncludeAnnotation; this.useFormat = other.useFormat; this.Version = other.Version; - this.IgnoreNullValues = other.IgnoreNullValues; + this.OmitNullValues = other.OmitNullValues; this.LibraryCompatibility = other.LibraryCompatibility; - this.validations = other.validations; this.ThrowIfTypeConflictsWithMetadata = other.ThrowIfTypeConflictsWithMetadata; this.ThrowOnDuplicatePropertyNames = other.ThrowOnDuplicatePropertyNames; diff --git a/src/Microsoft.OData.Core/ODataPreferenceHeader.cs b/src/Microsoft.OData.Core/ODataPreferenceHeader.cs index 82a905504b..4c2aa87948 100644 --- a/src/Microsoft.OData.Core/ODataPreferenceHeader.cs +++ b/src/Microsoft.OData.Core/ODataPreferenceHeader.cs @@ -6,8 +6,10 @@ namespace Microsoft.OData { + using System; using System.Collections.Generic; using System.Diagnostics; + using System.Diagnostics.CodeAnalysis; using System.Globalization; /// @@ -61,6 +63,11 @@ public class ODataPreferenceHeader /// private const string ODataTrackChangesPreferenceToken = "odata.track-changes"; + /// + /// The omit-values preference token. + /// + private const string OmitValuesPreferenceToken = "omit-values"; + /// /// The Prefer header name. /// @@ -243,6 +250,40 @@ public string AnnotationFilter } } + /// + /// Property to get and set the "omit-values" preference to the "Prefer" header on the underlying IODataRequestMessage or + /// the "Preference-Applied" header on the underlying IODataResponseMessage. + /// For the preference or applied preference of omitting null values, use string "nulls". + /// + public string OmitValues + { + get + { + HttpHeaderValueElement omitValues = this.Get(OmitValuesPreferenceToken); + return omitValues?.Value; + } + + [SuppressMessage("Microsoft.Globalization", "CA1308:NormalizeStringsToUppercase", Justification = "Need lower case string here.")] + set + { + ExceptionUtils.CheckArgumentStringNotEmpty(value, "OmitValues"); + + string normalizedValue = value.Trim().ToLowerInvariant(); + if (string.Equals(normalizedValue, ODataConstants.OmitValuesNulls, StringComparison.Ordinal)) + { + this.Set(new HttpHeaderValueElement(OmitValuesPreferenceToken, ODataConstants.OmitValuesNulls, EmptyParameters)); + } + else if (string.Equals(normalizedValue, ODataConstants.OmitValuesDefaults, StringComparison.Ordinal)) + { + throw new NotSupportedException("omit-values=defaults is not supported yet."); + } + else + { + throw new ODataException(Strings.HttpPreferenceHeader_InvalidValueForToken(value, OmitValuesPreferenceToken)); + } + } + } + /// /// Property to get and set the "respond-async" preference to the "Prefer" header on the underlying IODataRequestMessage or /// the "Preference-Applied" header on the underlying IODataResponseMessage. diff --git a/src/Microsoft.OData.Core/ODataWriterCore.cs b/src/Microsoft.OData.Core/ODataWriterCore.cs index a5523c3df7..59fa9d85ab 100644 --- a/src/Microsoft.OData.Core/ODataWriterCore.cs +++ b/src/Microsoft.OData.Core/ODataWriterCore.cs @@ -1144,6 +1144,19 @@ protected void ValidateNoDeltaLinkForExpandedResourceSet(ODataResourceSet resour } } + /// + /// Returns whether properties of null values should be omitted when serializing the payload. + /// + /// Return true to omit null-value properties; false otherwise. + protected bool ShouldOmitNullValues() + { + // Omit null values preference should only affect response payload, and should not + // be applied to writing delta payload. + return this.outputContext.WritingResponse + && !this.writingDelta + && this.outputContext.MessageWriterSettings.OmitNullValues; + } + /// /// Verifies that calling WriteStart resourceSet is valid. /// diff --git a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs index b6c99f1408..5f455f8809 100644 --- a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs @@ -1628,6 +1628,14 @@ 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 7411aa0a70..ab7187280a 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -38,6 +38,9 @@ internal sealed class SelectedPropertiesNode /// An empty set of navigation properties to return when nothing is selected. private static readonly IEnumerable EmptyNavigationProperties = Enumerable.Empty(); + /// An empty dictionary of EDM properties to return when nothing is selected. + private static readonly Dictionary EmptyEdmProperties = new Dictionary(StringComparer.Ordinal); + /// The type of the current node. private SelectionType selectionType = SelectionType.PartialSubtree; @@ -441,11 +444,43 @@ internal IEnumerable GetSelectedNavigationProperties(IEd return selectedNavigationProperties.Distinct(); } + /// + /// Gets all selected properties at the current node. + /// + /// The current structured type. + /// The selected properties at this node level. + internal IDictionary GetSelectedProperties(IEdmStructuredType structuredType) + { + if (this.selectionType == SelectionType.Empty) + { + return EmptyEdmProperties; + } + + // We cannot determine the selected properties without the user model. This means we won't be computing the missing properties. + if (structuredType == null) + { + return EmptyEdmProperties; + } + + if (this.selectionType == SelectionType.EntireSubtree || this.hasWildcard) + { + return structuredType.DeclaredProperties.ToDictionary(sp => sp.Name, StringComparer.Ordinal); + } + + Debug.Assert(this.selectedProperties != null, "selectedProperties != null"); + + IDictionary selectedEdmProperties = this.selectedProperties + .Select(structuredType.FindProperty) + .ToDictionary(p => p.Name, StringComparer.Ordinal); + + return selectedEdmProperties; + } + /// /// Gets the selected stream properties for the current node. /// /// The current entity type. - /// The selected stream properties. + /// The selected stream properties at this node level. internal IDictionary GetSelectedStreamProperties(IEdmEntityType entityType) { if (this.selectionType == SelectionType.Empty) 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 3a8714590e..d6f934ff14 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 @@ -20,6 +20,8 @@ namespace Microsoft.OData.Tests.IntegrationTests.Reader.JsonLight { public class PropertyAndValueJsonLightReaderIntegrationTests { + private static string omitValuesNulls = "omit-values=" + ODataConstants.OmitValuesNulls; + [Fact] public void ReadingPayloadInt64SingleDoubleDecimalWithoutSuffix() { @@ -106,7 +108,7 @@ public void ReadingPayloadInt64SingleDoubleDecimalWithoutSuffix() } [Fact] - public void ReadNullableCollectionValue() + public void ReadNullableCollectionValueExpanded() { EdmModel model = new EdmModel(); EdmEntityType entityType = new EdmEntityType("NS", "MyTestEntity"); @@ -128,25 +130,30 @@ public void ReadNullableCollectionValue() "}"; IEdmModel mainModel = TestUtils.WrapReferencedModelsToMainModel("EntityNs", "MyContainer", model); - ODataResource entry = null; - this.ReadEntryPayload(mainModel, payload, entitySet, entityType, reader => { entry = entry ?? reader.Item as ODataResource; }); - Assert.NotNull(entry); - var intCollection = entry.Properties.FirstOrDefault( - s => string.Equals( - s.Name, - "NullableIntNumbers", - StringComparison.OrdinalIgnoreCase)).Value.As(); - var list = new List(); - foreach (var val in intCollection.Items) + foreach (bool isNullValuesOmitted in new bool[] {false, true}) { - list.Add(val as int?); - } + ODataResource entry = null; + this.ReadEntryPayload(mainModel, payload, entitySet, entityType, + reader => { entry = entry ?? reader.Item as ODataResource; }, isNullValuesOmitted); + Assert.NotNull(entry); + + var intCollection = entry.Properties.FirstOrDefault( + s => string.Equals( + s.Name, + "NullableIntNumbers", + StringComparison.OrdinalIgnoreCase)).Value.As(); + var list = new List(); + foreach (var val in intCollection.Items) + { + list.Add(val as int?); + } - Assert.Equal(0, list[0]); - Assert.Null(list[1]); - Assert.Equal(1, (int)list[2]); - Assert.Equal(2, (int)list[3]); + Assert.Equal(0, list[0]); + Assert.Null(list[1]); + Assert.Equal(1, (int) list[2]); + Assert.Equal(2, (int) list[3]); + } } [Fact] @@ -176,53 +183,60 @@ public void ReadOpenCollectionPropertyValue() "}"; IEdmModel mainModel = TestUtils.WrapReferencedModelsToMainModel("EntityNs", "MyContainer", model); - ODataResource entry = null; - List complexCollection = new List(); - ODataNestedResourceInfo nestedOpenCollection = null; - bool startComplexCollection = false; - this.ReadEntryPayload(mainModel, payload, entitySet, entityType, - reader => - { - switch (reader.State) - { - case ODataReaderState.NestedResourceInfoStart: - startComplexCollection = true; - break; - case ODataReaderState.ResourceEnd: - if(startComplexCollection) - { - complexCollection.Add(reader.Item as ODataResource); - } - entry = reader.Item as ODataResource; - break; - case ODataReaderState.NestedResourceInfoEnd: - startComplexCollection = false; - nestedOpenCollection = reader.Item as ODataNestedResourceInfo; - break; - } - }); - Assert.NotNull(entry); - var intCollection = entry.Properties.FirstOrDefault( - s => string.Equals( - s.Name, - "OpenPrimitiveCollection", - StringComparison.OrdinalIgnoreCase)).Value.As(); - var list = new List(); - foreach (var val in intCollection.Items) + foreach (bool isNullValuesOmitted in new bool[] {false, true}) { - list.Add(val as int?); - } + ODataResource entry = null; + List complexCollection = new List(); + ODataNestedResourceInfo nestedOpenCollection = null; + bool startComplexCollection = false; + this.ReadEntryPayload(mainModel, payload, entitySet, entityType, + reader => + { + switch (reader.State) + { + case ODataReaderState.NestedResourceInfoStart: + startComplexCollection = true; + break; + case ODataReaderState.ResourceEnd: + if (startComplexCollection) + { + complexCollection.Add(reader.Item as ODataResource); + } + entry = reader.Item as ODataResource; + break; + case ODataReaderState.NestedResourceInfoEnd: + startComplexCollection = false; + nestedOpenCollection = reader.Item as ODataNestedResourceInfo; + break; + } + }, + nullValuesOmitted: isNullValuesOmitted); + Assert.NotNull(entry); + + var intCollection = entry.Properties.FirstOrDefault( + s => string.Equals( + s.Name, + "OpenPrimitiveCollection", + StringComparison.OrdinalIgnoreCase)).Value.As(); + var list = new List(); + foreach (var val in intCollection.Items) + { + list.Add(val as int?); + } - Assert.Equal(0, list[0]); - Assert.Equal(1, (int)list[1]); - Assert.Equal(2, (int)list[2]); + Assert.Equal(0, list[0]); + Assert.Equal(1, (int) list[1]); + Assert.Equal(2, (int) list[2]); - Assert.Equal("OpenComplexTypeCollection", nestedOpenCollection.Name); + Assert.Equal("OpenComplexTypeCollection", nestedOpenCollection.Name); - foreach (var val in complexCollection) - { - val.Properties.FirstOrDefault(s => string.Equals(s.Name, "CLongId", StringComparison.OrdinalIgnoreCase)).Value.ShouldBeEquivalentTo(1L, "value should be in correct type."); + foreach (var val in complexCollection) + { + val.Properties.FirstOrDefault( + s => string.Equals(s.Name, "CLongId", StringComparison.OrdinalIgnoreCase)) + .Value.ShouldBeEquivalentTo(1L, "value should be in correct type."); + } } } @@ -353,6 +367,64 @@ public void ReadingTypeDefinitionPayloadJsonLight() address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.OrdinalIgnoreCase)).Value.Should().Be("China"); } + [Fact] + public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues() + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + const string payload = @" +{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Weight"":60.5, + ""Address"":{""CountryRegion"":""US"", ""City"":""Redmond""} +}"; + + List entries = new List(); + ODataNestedResourceInfo navigationLink = null; + this.ReadEntryPayload(model, payload, entitySet, entityType, + reader => + { + switch (reader.State) + { + case ODataReaderState.ResourceStart: + entries.Add(reader.Item as ODataResource); + break; + case ODataReaderState.NestedResourceInfoStart: + navigationLink = (ODataNestedResourceInfo)reader.Item; + break; + default: + break; + } + }, + nullValuesOmitted: true); + + Assert.Equal(2, entries.Count); + + ODataResource person = entries[0]; + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(0); + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)) + .Value.Should().Be(60.5); + // omitted value should be restored to null. + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) + .Value.Should().BeNull(); + + ODataResource address = entries[1]; + address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.Ordinal)) + .Value.Should().Be("US"); + address.Properties.FirstOrDefault(s => string.Equals(s.Name, "City", StringComparison.Ordinal)) + .Value.Should().Be("Redmond"); + // Omitted value should be restored to null. + address.Properties.FirstOrDefault(s => string.Equals(s.Name, "ZipCode", StringComparison.Ordinal)) + .Value.Should().BeNull(); + + navigationLink.Name.Should().Be("Address"); + } + [Fact] public void ReadingTypeDefinitionPayloadWithTypeAnnotationJsonLight() { @@ -773,6 +845,246 @@ public void ReadingNullValueForDeclaredCollectionPropertyInComplexTypeShouldFail read.ShouldThrow().WithMessage("A null value was found for the property named 'CountriesOrRegions', which has the expected type 'Collection(Edm.String)[Nullable=False]'. The expected type 'Collection(Edm.String)[Nullable=False]' does not allow null values."); } + [Fact] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValues() + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + const string payload =@"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Education"":{""Id"":1} + }"; + + List entries = new List(); + this.ReadEntryPayload(model, payload, entitySet, entityType, + reader => + { + switch (reader.State) + { + case ODataReaderState.ResourceStart: + entries.Add(reader.Item as ODataResource); + break; + default: + break; + } + }, + nullValuesOmitted: true); + + Assert.Equal(2, entries.Count); + + ODataResource edu = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)).Value.Should().Be(1); + // null-able collection should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Value.Should().BeNull(); + // null-able property value should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Value.Should().BeNull(); + + ODataResource person = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); + person.Should().NotBeNull(); + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)).Value.Should().Be(0); + // omitted null-able property should be restored as null. + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Height", StringComparison.Ordinal)).Value.Should().BeNull(); + // missing non-null-able property should not be restored. + person.Properties.Any(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)).Should().BeFalse(); + } + + [Fact] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesEntireSubTree() + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + // null-able property Height is not selected, thus should not be restored. + // null-able property Address is selected, thus should be restored. + // Property Education is null-able. + const string payloadWithQueryOption = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,Education,Address)/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Education"":{""Id"":1} + }"; + const string payloadWithWildcardInQueryOption = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,Education/*,Address)/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Education"":{""Id"":1} + }"; + + foreach (string payload in new string[] {payloadWithQueryOption, payloadWithWildcardInQueryOption}) + { + List entries = new List(); + List navigationLinks = 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: + navigationLinks.Add(reader.Item as ODataNestedResourceInfo); + break; + default: + break; + } + }, + nullValuesOmitted: true); + + entries.Count.Should().Be(2); + navigationLinks.Count.Should().Be(1); + navigationLinks.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) + .Should().NotBeNull(); + + // Education + ODataResource edu = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(1); + // null-able collection should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)) + .Value.Should().BeNull(); + // null-able property value should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)) + .Value.Should().BeNull(); + + // Person + ODataResource person = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); + person.Should().NotBeNull(); + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(0); + // selected null-able property/navigationLink should be restored as null. + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Address", StringComparison.Ordinal)) + .Value.Should().BeNull(); + // 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(); + person.Properties.Any(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)).Should().BeFalse(); + } + } + + [Fact] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesPartialSubTree() + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + // null-able property Height is not selected, thus should not be restored. + // null-able property Address is selected, thus should be restored. + // Property Education is null-able. + const string payloadWithWildcardInQueryOption = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20Education/Id,%20Education/SchoolName,%20Address)/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Education"":{""Id"":1} + }"; + + foreach (string payload in new string[] { payloadWithWildcardInQueryOption }) + { + List entries = new List(); + List navigationLinks = 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: + navigationLinks.Add(reader.Item as ODataNestedResourceInfo); + break; + default: + break; + } + }, + nullValuesOmitted: true); + + entries.Count.Should().Be(2); + navigationLinks.Count.Should().Be(1); + navigationLinks.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) + .Should().NotBeNull(); + + // Education + ODataResource edu = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(1); + // null-able property value should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)) + .Value.Should().BeNull(); + // not selected property should not be restored. + edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); + } + } + + [Fact] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesPartialSubTreeAndBaseTypeProperty() + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + // null-able property Height is not selected, thus should not be restored. + // null-able property Address is selected, thus should be restored. + // Property Education is null-able. + const string payloadWithWildcardInQueryOption = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20Education/Id,%20Education/OrgId,%20Address)/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Education"":{""Id"":1} + }"; + + foreach (string payload in new string[] { payloadWithWildcardInQueryOption }) + { + List entries = new List(); + List navigationLinks = 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: + navigationLinks.Add(reader.Item as ODataNestedResourceInfo); + break; + default: + break; + } + }, + nullValuesOmitted: true); + + entries.Count.Should().Be(2); + navigationLinks.Count.Should().Be(1); + navigationLinks.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) + .Should().NotBeNull(); + + // Education + ODataResource edu = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(1); + // selected null-able property from base type should be restored to null if omitted. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "OrgId", StringComparison.Ordinal)) + .Value.Should().BeNull(); + // not selected property should not be restored. + edu.Properties.Any(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Should().BeFalse(); + edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); + } + } + [Fact] public void ReadingNullValueForDynamicCollectionPropertyInOpenStructuralTypeShouldWork() { @@ -840,13 +1152,23 @@ public void ReadDateTimeOffsetWithCustomFormat() birthday.Value.Should().Be(new DateTimeOffset(2012, 4, 12, 18, 43, 10, TimeSpan.Zero)); } - private void ReadEntryPayload(IEdmModel userModel, string payload, EdmEntitySet entitySet, IEdmEntityType entityType, Action action, bool isIeee754Compatible = true, IServiceProvider container = null) + private void ReadEntryPayload(IEdmModel userModel, string payload, EdmEntitySet entitySet, IEdmEntityType entityType, + Action action, + bool isIeee754Compatible = true, + IServiceProvider container = null, + bool nullValuesOmitted = false) { var message = new InMemoryMessage() { Stream = new MemoryStream(Encoding.UTF8.GetBytes(payload)), Container = container}; string contentType = isIeee754Compatible ? "application/json;odata.metadata=minimal;IEEE754Compatible=true" : "application/json;odata.metadata=minimal;IEEE754Compatible=false"; message.SetHeader("Content-Type", contentType); + + if (nullValuesOmitted) + { + message.SetHeader("Preference-Applied", omitValuesNulls); + } + var readerSettings = new ODataMessageReaderSettings { EnableMessageStreamDisposal = true }; readerSettings.Validations &= ~ValidationKinds.ThrowOnUndeclaredPropertyForNonOpenType; using (var msgReader = new ODataMessageReader((IODataResponseMessage)message, readerSettings, userModel)) @@ -858,5 +1180,66 @@ private void ReadEntryPayload(IEdmModel userModel, string payload, EdmEntitySet } } } + + private EdmModel BuildEdmModelForOmittedNullValuesTestCases(out EdmEntityType entityType, out EdmEntitySet entitySet) + { + EdmModel model = new EdmModel(); + + entityType = new EdmEntityType("NS", "Person"); + entityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32); + + EdmTypeDefinition weightType = new EdmTypeDefinition("NS", "Wgt", EdmPrimitiveTypeKind.Double); + EdmTypeDefinitionReference weightTypeRef = new EdmTypeDefinitionReference(weightType, false); + entityType.AddStructuralProperty("Weight", weightTypeRef); + + EdmTypeDefinition heightType = new EdmTypeDefinition("NS", "Ht", EdmPrimitiveTypeKind.Double); + EdmTypeDefinitionReference heightTypeRef = new EdmTypeDefinitionReference(heightType, true); + entityType.AddStructuralProperty("Height", heightTypeRef); + + EdmComplexType addressType = new EdmComplexType("NS", "OpnAddr"); + EdmComplexTypeReference addressTypeRef = new EdmComplexTypeReference(addressType, true); + + EdmTypeDefinition countryRegionType = new EdmTypeDefinition("NS", "CR", EdmPrimitiveTypeKind.String); + EdmTypeDefinitionReference countryRegionTypeRef = new EdmTypeDefinitionReference(countryRegionType, false); + addressType.AddStructuralProperty("CountryRegion", countryRegionTypeRef); + + // Address/City + EdmTypeDefinition cityType = new EdmTypeDefinition("NS", "Cty", EdmPrimitiveTypeKind.String); + EdmTypeDefinitionReference cityTypeRef = new EdmTypeDefinitionReference(cityType, false); + addressType.AddStructuralProperty("City", cityTypeRef); + + EdmTypeDefinition zipCodeType = new EdmTypeDefinition("NS", "ZC", EdmPrimitiveTypeKind.String); + EdmTypeDefinitionReference zipCodeTypeRef = new EdmTypeDefinitionReference(zipCodeType, true); + addressType.AddStructuralProperty("ZipCode", zipCodeTypeRef); + + entityType.AddStructuralProperty("Address", addressTypeRef); + + // Education + EdmComplexType organizationType = new EdmComplexType("NS", "Org"); + organizationType.AddStructuralProperty("OrgId", EdmPrimitiveTypeKind.Int32); + organizationType.AddStructuralProperty("OrgCategory", EdmPrimitiveTypeKind.String); + + EdmComplexType educationType = new EdmComplexType("NS", "Edu", organizationType); + EdmComplexTypeReference educationTypeRef = new EdmComplexTypeReference(educationType, true); + + // top level null-able complex type properties + educationType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32); // same property name but inside different type. + educationType.AddStructuralProperty("Campuses", + new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetString(true)))); + educationType.AddStructuralProperty("SchoolName", EdmPrimitiveTypeKind.String); + + entityType.AddStructuralProperty("Education", educationTypeRef); + + model.AddElement(weightType); + model.AddElement(heightType); + model.AddElement(addressType); + model.AddElement(entityType); + + EdmEntityContainer container = new EdmEntityContainer("EntityNs", "MyContainer"); + entitySet = container.AddEntitySet("People", entityType); + model.AddElement(container); + + return model; + } } } diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeltaWriterTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeltaWriterTests.cs index 89839a45db..3235db68d7 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeltaWriterTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeltaWriterTests.cs @@ -60,6 +60,22 @@ public class ODataJsonLightDeltaWriterTests } }; + private readonly ODataResource customerUpdatedWithNullValues = new ODataResource + { + Id = new Uri("Customers('BOTTM')", UriKind.Relative), + Properties = new List + { + new ODataProperty { Name = "ContactName", Value = null }, + }, + TypeName = "MyNS.Customer", + SerializationInfo = new ODataResourceSerializationInfo + { + NavigationSourceEntityTypeName = "MyNS.Customer", + NavigationSourceKind = EdmNavigationSourceKind.EntitySet, + NavigationSourceName = "Customers" + } + }; + private readonly ODataDeltaDeletedLink linkToOrder10643 = new ODataDeltaDeletedLink(new Uri("Customers('ALFKI')", UriKind.Relative), new Uri("Orders('10643')", UriKind.Relative), "Orders"); private readonly ODataDeltaLink linkToOrder10645 = new ODataDeltaLink(new Uri("Customers('BOTTM')", UriKind.Relative), new Uri("Orders('10645')", UriKind.Relative), "Orders"); @@ -1356,7 +1372,7 @@ public void WriteNestedDeletedEntryInDeletedEntry() new ODataDeletedResource() { Reason = DeltaDeletedEntryReason.Deleted, - Properties = new ODataProperty[] + Properties = new ODataProperty[] { new ODataProperty() { Name = "Name", Value = "Scissors" }, new ODataProperty() { Name = "Id", Value = 1 } @@ -2031,7 +2047,7 @@ public void WriteTopLevelDeletedEntityFromDifferentSet() this.TestPayload().Should().Be("{\"@context\":\"http://host/service/$metadata#Customers/$delta\",\"@count\":5,\"@deltaLink\":\"Customers?$expand=Orders&$deltatoken=8015\",\"value\":[{\"@id\":\"Customers('BOTTM')\",\"ContactName\":\"Susan Halvenstern\"},{\"@context\":\"http://host/service/$metadata#Orders/$deletedEntity\",\"@removed\":{\"reason\":\"deleted\"},\"@id\":\"Orders(10643)\"}]}"); } - + [Fact] public void WriteTopLevelDeletedEntityFromDifferentSetWithoutInfo() { @@ -2049,6 +2065,155 @@ public void WriteTopLevelDeletedEntityFromDifferentSetWithoutInfo() this.TestPayload().Should().Be("{\"@context\":\"http://host/service/$metadata#Customers/$delta\",\"@count\":5,\"@deltaLink\":\"Customers?$expand=Orders&$deltatoken=8015\",\"value\":[{\"@id\":\"Customers('BOTTM')\",\"ContactName\":\"Susan Halvenstern\"},{\"@context\":\"http://host/service/$metadata#Orders/$deletedEntity\",\"@removed\":{\"reason\":\"deleted\"},\"@id\":\"Orders(10643)\"}]}"); } + /// + /// When using regular OData writer with required writingDelta = true to write + /// containing , property value of null + /// should always be written in response (regardless of omitNullValues preference setting). + /// + [Fact] + public void WriteTopLevelEntityWithNullPropertyValues() + { + this.TestInit(this.GetModel()); + + + foreach (bool omitNullValues in new bool[] {true, false}) + { + using (this.stream = new MemoryStream()) + { + // Create resource set writer with specified omitNullValues setting + ODataJsonLightWriter writer = new ODataJsonLightWriter( + CreateJsonLightOutputContext(this.stream, this.GetModel(), omitNullValues, false, null, ODataVersion.V401), + this.GetCustomers(), + this.GetCustomerType(), + true, + false, + /*writingDelta*/ true); + writer.WriteStart(feedWithoutInfo); + writer.WriteStart(customerUpdatedWithNullValues); + writer.WriteEnd(); // customer + writer.WriteEnd(); // delta resource set + writer.Flush(); + + this.TestPayload().Should().Be( + "{\"@context\":\"http://host/service/$metadata#Customers/$delta\",\"@count\":5,\"@deltaLink\":\"Customers?$expand=Orders&$deltatoken=8015\",\"value\":[{\"@id\":\"Customers('BOTTM')\",\"ContactName\":null}]}"); + } + } + } + + /// + /// When using with required writingDelta = true to write + /// containing , property value of null + /// should always be written in response (regardless of omitNullValues preference setting). + /// + [Fact] + public void WriteEntityWithNullPropertyValueInDeltaFeed() + { + this.TestInit(this.GetModel()); + + ODataDeltaResourceSet feed = new ODataDeltaResourceSet(); + + ODataResource orderEntry = new ODataResource() + { + SerializationInfo = new ODataResourceSerializationInfo + { + NavigationSourceEntityTypeName = "MyNS.Order", + NavigationSourceKind = EdmNavigationSourceKind.EntitySet, + NavigationSourceName = "Orders" + }, + Properties = new ODataProperty[] + { + new ODataProperty() { Name = "Id", Value = 1 } + }, + }; + + ODataNestedResourceInfo shippingAddressInfo = new ODataNestedResourceInfo + { + Name = "ShippingAddress", + IsCollection = false + }; + + ODataResource shippingAddress = new ODataResource + { + Properties = new List + { + new ODataProperty { Name = "City", Value = "Shanghai" }, + new ODataProperty { Name = "PostalCode", Value = null } + } + }; + + var result = new ODataQueryOptionParser(this.GetModel(), this.GetCustomerType(), this.GetCustomers(), new Dictionary { { "$expand", "Orders($select=ShippingAddress)" }}).ParseSelectAndExpand(); + + ODataUri odataUri = new ODataUri() + { + ServiceRoot = new Uri("http://host/service"), + SelectAndExpand = result + }; + + foreach (bool omitNullValues in new bool[] {true, false}) + { + using (this.stream = new MemoryStream()) + { + var outputContext = CreateJsonLightOutputContext(this.stream, this.GetModel(), omitNullValues, + false, odataUri, ODataVersion.V401); + ODataJsonLightDeltaWriter writer = new ODataJsonLightDeltaWriter(outputContext, this.GetProducts(), + this.GetProductType()); + writer.WriteStart(feed); + writer.WriteStart(orderEntry); + writer.WriteStart(shippingAddressInfo); + writer.WriteStart(shippingAddress); + writer.WriteEnd(); // shippingAddress + writer.WriteEnd(); // shippingAddressInfo + writer.WriteEnd(); // Order + writer.WriteEnd(); // Feed + writer.Flush(); + + this.TestPayload().Should().Be( + "{\"@context\":\"http://host/service/$metadata#Products(Orders(ShippingAddress))/$delta\",\"value\":[{\"@context\":\"http://host/service/$metadata#Orders/$entity\",\"Id\":1,\"ShippingAddress\":{\"City\":\"Shanghai\",\"PostalCode\":null}}]}"); + } + } + } + + /// + /// When using regular OData writer with required writingDelta = true to write , + /// property value of null should always be written in response (regardless of omitNullValues preference setting). + /// + [Fact] + public void WriteDeletedResourceWithPropertyValueNull() + { + this.TestInit(this.GetModel()); + ODataDeletedResource deletedEntity = new ODataDeletedResource() + { + Reason = DeltaDeletedEntryReason.Changed, + TypeName = "MyNS.PhysicalProduct", + Properties = new[] + { + new ODataProperty {Name = "Id", Value = new ODataPrimitiveValue(1)}, + new ODataProperty {Name = "Name", Value = new ODataPrimitiveValue("car")}, + new ODataProperty {Name = "Material", Value = null} + } + }; + + foreach (bool omitNullValues in new bool[] {true, false}) + { + using (this.stream = new MemoryStream()) + { + ODataDeltaResourceSet feed = new ODataDeltaResourceSet(); + ODataJsonLightWriter writer = new ODataJsonLightWriter( + CreateJsonLightOutputContext(this.stream, this.GetModel(), omitNullValues, false, null, + ODataVersion.V401), + this.GetProducts(), this.GetProductType(), true, false, /*writingDelta*/true); + writer.WriteStart(feed); + writer.WriteStart(deletedEntity); + writer.WriteEnd(); + writer.WriteEnd(); + writer.Flush(); + + this.TestPayload().Should().Be( + "{\"@context\":\"http://host/service/$metadata#Products/$delta\",\"value\":[{\"@removed\":{\"reason\":\"changed\"},\"@type\":\"#MyNS.PhysicalProduct\",\"Id\":1,\"Name\":\"car\",\"Material\":null}]}"); + } + } + } + [Fact] public void WriteEntityFromDifferentSetToEntitySetShouldFail() { @@ -2423,9 +2588,20 @@ private string TestPayload() return (new StreamReader(stream)).ReadToEnd(); } - private static ODataJsonLightOutputContext CreateJsonLightOutputContext(MemoryStream stream, IEdmModel userModel, bool fullMetadata = false, ODataUri uri = null, ODataVersion version = ODataVersion.V4) + private static ODataJsonLightOutputContext CreateJsonLightOutputContext(MemoryStream stream, IEdmModel userModel, + bool fullMetadata = false, ODataUri uri = null, ODataVersion version = ODataVersion.V4) { - var settings = new ODataMessageWriterSettings { Version = version, ShouldIncludeAnnotation = ODataUtils.CreateAnnotationFilter("*") }; + return CreateJsonLightOutputContext(stream, userModel, /*omitNullValues*/ false, fullMetadata, uri, version); + } + + private static ODataJsonLightOutputContext CreateJsonLightOutputContext(MemoryStream stream, IEdmModel userModel, bool omitNullValues, bool fullMetadata = false, ODataUri uri = null, ODataVersion version = ODataVersion.V4) + { + var settings = new ODataMessageWriterSettings + { + Version = version, + ShouldIncludeAnnotation = ODataUtils.CreateAnnotationFilter("*"), + OmitNullValues = omitNullValues + }; settings.SetServiceDocumentUri(new Uri("http://host/service")); if (uri != null) { diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs index f8e89cba4b..a03a0671c3 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs @@ -113,6 +113,15 @@ public void WildcardShouldSelectAllPropertiesParsingTest() SelectedPropertiesNode.Create("*").Should().HaveStreams(this.cityType, "Photo").And.HaveNavigations(this.cityType, "Districts").And.HaveChild(this.cityType, "Districts", c => c.Should().BeSameAsEmpty()); } + [Fact] + public void WildcardShouldSelectAllProperties() + { + SelectedPropertiesNode.Create("*").Should() + .HaveProperties(this.cityType, "Photo", "Districts") + .And.HaveNavigations(this.cityType, "Districts") + .And.HaveChild(this.cityType, "Districts", c => c.Should().BeSameAsEmpty()); + } + [Fact] public void SingleStreamPropertyWithNormalProperty() { @@ -141,6 +150,12 @@ public void SpecifyingTheSameNavigationTwiceShouldNotCauseDuplicates() public void SpecifyingAWildCardShouldNotCauseDuplicates() { SelectedPropertiesNode.Create("Districts,*,Photo").Should().HaveStreams(this.cityType, "Photo").And.HaveNavigations(this.cityType, "Districts").And.HaveChild(this.cityType, "Districts", c => c.Should().HaveEntireSubtree()); + + SelectedPropertiesNode.Create("Districts,*,Photo").Should() + .HaveStreams(this.cityType, "Photo") + .And.HaveProperties(this.cityType, "Photo", "Districts") + .And.HaveNavigations(this.cityType, "Districts") + .And.HaveChild(this.cityType, "Districts", c => c.Should().HaveEntireSubtree()); } [Fact] @@ -166,6 +181,7 @@ public void DeepAndWideSelection1() // 2) 'Districts/*' should not override 'Districts' SelectedPropertiesNode.Create("*,Districts,Districts/*").Should() .HaveStreams(this.cityType, "Photo") + .And.HaveProperties(this.cityType, "Photo", "Districts") .And.HaveNavigations(this.cityType, "Districts") .And.HaveChild(this.cityType, "Districts", c => c.Should().HaveEntireSubtree()); } @@ -180,6 +196,7 @@ public void DeepAndWideSelection2() "Districts", c => c.Should() .HaveStreams(this.districtType, "Thumbnail") + .And.HaveProperties(this.districtType, "Id", "Zip", "Thumbnail", "City") .And.HaveChild(this.districtType, "City", c2 => c2.Should().HaveEntireSubtree())); } @@ -193,6 +210,7 @@ public void WildcardAfterNavigationShouldNotSelectTheEntireTree() "Districts", c => c.Should() .HaveStreams(this.districtType, "Thumbnail") + .And.HaveProperties(this.districtType, "Id", "Zip", "Thumbnail", "City") .And.HaveNavigations(this.districtType, "City")); } @@ -332,7 +350,10 @@ public void CombiningPropertyWithWildcardShouldBeWildcard() { SelectedPropertiesNode left = SelectedPropertiesNode.Create("*"); SelectedPropertiesNode right = SelectedPropertiesNode.Create("Fake"); - this.VerifyCombination(left, right, n => n.Should().HaveStreams(this.cityType, "Photo").And.HaveNavigations(this.cityType, "Districts")); + this.VerifyCombination(left, right, + n => n.Should().HaveStreams(this.cityType, "Photo") + .And.HaveProperties(this.cityType, "Districts", "Photo") + .And.HaveNavigations(this.cityType, "Districts")); } [Fact] @@ -343,6 +364,7 @@ public void CombiningPartialNodesShouldCombineProperties() Action verify = n => n.Should() .HaveStreams(this.cityType, "Photo") + .And.HaveProperties(this.cityType, "Photo") .And.HaveNavigations(this.cityType, "Districts") .And.HaveChild(this.cityType, "Districts", c => c.Should().HaveOnlyStreams(this.districtType, "Thumbnail")); @@ -362,6 +384,7 @@ public void CombiningDeepPartialNodesShouldCombineRecursively() "Districts", c => c.Should() .HaveStreams(this.districtType, "Thumbnail") + .And.HaveProperties(this.districtType, "Thumbnail") .And.HaveNavigations(this.districtType, "City") .And.HaveChild(this.districtType, "City", c2 => c2.Should().HaveOnlyNavigations(this.cityType, "Districts"))); @@ -567,6 +590,12 @@ internal SelectedPropertiesNodeAssertions(SelectedPropertiesNode node) : base(no { } + internal AndConstraint HaveProperties(IEdmEntityType entityType, params string[] propertyNames) + { + this.Subject.As().GetSelectedProperties(entityType).Keys.Should().BeEquivalentTo(propertyNames); + return new AndConstraint(this); + } + internal AndConstraint HaveStreams(IEdmEntityType entityType, params string[] streamPropertyNames) { this.Subject.As().GetSelectedStreamProperties(entityType).Keys.Should().BeEquivalentTo(streamPropertyNames); diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl b/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl index 4c2c894a0e..84c56b0efc 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl @@ -4719,6 +4719,7 @@ public class Microsoft.OData.ODataPreferenceHeader { string AnnotationFilter { public get; public set; } bool ContinueOnError { public get; public set; } System.Nullable`1[[System.Int32]] MaxPageSize { public get; public set; } + string OmitValues { public get; public set; } bool RespondAsync { public get; public set; } System.Nullable`1[[System.Boolean]] ReturnContent { public get; public set; } bool TrackChanges { public get; public set; } @@ -5189,11 +5190,11 @@ public sealed class Microsoft.OData.ODataMessageWriterSettings { System.Uri BaseUri { public get; public set; } bool EnableCharactersCheck { public get; public set; } bool EnableMessageStreamDisposal { public get; public set; } - bool IgnoreNullValues { public get; public set; } string JsonPCallback { public get; public set; } Microsoft.OData.ODataLibraryCompatibility LibraryCompatibility { public get; public set; } Microsoft.OData.ODataMessageQuotas MessageQuotas { public get; public set; } Microsoft.OData.ODataUri ODataUri { public get; public set; } + bool OmitNullValues { public get; public set; } Microsoft.OData.ValidationKinds Validations { public get; public set; } System.Nullable`1[[Microsoft.OData.ODataVersion]] Version { public get; public set; } diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt index 3578c7bcee..a1f0970148 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt @@ -16,7 +16,7 @@ Model Present: true Combination: 5; TestConfiguration = Format: JsonLight, Request: True, Synchronous: True Model Present: true -{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44} +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44,"Hobby":null} Combination: 6; TestConfiguration = Format: JsonLight, Request: False, Synchronous: True Model Present: true @@ -24,7 +24,7 @@ Model Present: true Combination: 7; TestConfiguration = Format: JsonLight, Request: True, Synchronous: False Model Present: true -{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44} +{"@odata.context":"http://odata.org/test/$metadata#CustomerSet/$entity","ID":44,"Hobby":null} Combination: 8; TestConfiguration = Format: JsonLight, Request: False, Synchronous: False Model Present: true diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs index 3056e96869..11aae8bd24 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.cs @@ -127,7 +127,7 @@ public void PayloadOrderTest() { DebugDescription = "TypeName at the beginning, ID and ETag at the end, ID and ETag are not written and are ignored at the end", Items = new[] { new ODataResource() { TypeName = "TestModel.NonMLEType" } - .WithAnnotation(new WriteEntryCallbacksAnnotation + .WithAnnotation(new WriteEntryCallbacksAnnotation { BeforeWriteStartCallback = (entry) => { entry.Id = null; entry.ETag = null; }, BeforeWriteEndCallback = (entry) => { entry.Id = new Uri("urn:id"); entry.ETag = "etag"; } @@ -143,7 +143,7 @@ public void PayloadOrderTest() new EntryPayloadTestCase { DebugDescription = "Everything at the beginning", - Items = new[] { new ODataResource() { + Items = new[] { new ODataResource() { TypeName = "TestModel.MLEType", Id = new Uri("urn:id"), ETag = "etag", @@ -183,7 +183,7 @@ public void PayloadOrderTest() new EntryPayloadTestCase { DebugDescription = "TypeName, Id, ETag and ReadLinks at the beginning, the rest at the end", - Items = new[] { new ODataResource() { + Items = new[] { new ODataResource() { TypeName = "TestModel.MLEType", Id = new Uri("urn:id"), ETag = "etag", @@ -432,8 +432,8 @@ public void ActionAndFunctionPayloadOrderTest() }); } - [TestMethod, Variation(Description = "Test skipping null values when writing JSON Lite entries with IgnoreNullValues = true.")] - public void IgnoreNullPropertiesInEntryTest() + [TestMethod, Variation(Description = "Test skipping null values when writing JSON Lite entries with OmitNullValues = true.")] + public void OmitNullPropertiesInEntryTest() { EdmModel model = new EdmModel(); var container = new EdmEntityContainer("TestModel", "TestContainer"); @@ -517,7 +517,7 @@ public void IgnoreNullPropertiesInEntryTest() this.WriterTestConfigurationProvider.JsonLightFormatConfigurationsWithIndent, (testDescriptor, testConfiguration) => { - testConfiguration.MessageWriterSettings.IgnoreNullValues = true; + testConfiguration.MessageWriterSettings.OmitNullValues = true; TestWriterUtils.WriteAndVerifyODataEdmPayload(testDescriptor, testConfiguration, this.Assert, this.Logger); }); } @@ -545,9 +545,9 @@ public void OpenPropertiesInEntryTest() new EntryPayloadTestCase { DebugDescription = "Customer instance with open primitive property.", - Items = new[] { new ODataResource() - { - TypeName = "TestModel.OpenCustomerType", + Items = new[] { new ODataResource() + { + TypeName = "TestModel.OpenCustomerType", Properties = new ODataProperty[] { new ODataProperty { Name = "Age", Value = (long)42 } @@ -563,9 +563,9 @@ public void OpenPropertiesInEntryTest() new EntryPayloadTestCase { DebugDescription = "Customer instance with open spatial property.", - Items = new[] { new ODataResource() - { - TypeName = "TestModel.OpenCustomerType", + Items = new[] { new ODataResource() + { + TypeName = "TestModel.OpenCustomerType", Properties = new ODataProperty[] { new ODataProperty { Name = "Location", Value = pointValue } @@ -659,9 +659,9 @@ public void SpatialPropertiesInEntryTest() new EntryPayloadTestCase { DebugDescription = "Customer instance with spatial property (expected and payload type don't match).", - Items = new[] { new ODataResource() - { - TypeName = "TestModel.CustomerType", + Items = new[] { new ODataResource() + { + TypeName = "TestModel.CustomerType", Properties = new ODataProperty[] { new ODataProperty { Name = "Location1", Value = pointValue } @@ -687,9 +687,9 @@ public void SpatialPropertiesInEntryTest() new EntryPayloadTestCase { DebugDescription = "Customer instance with spatial property (expected and payload type match).", - Items = new[] { new ODataResource() - { - TypeName = "TestModel.CustomerType", + Items = new[] { new ODataResource() + { + TypeName = "TestModel.CustomerType", Properties = new ODataProperty[] { new ODataProperty { Name = "Location2", Value = pointValue } @@ -776,7 +776,7 @@ public void TopLevelOpenComplexProperties() tc => new JsonWriterTestExpectedResults(this.Settings.ExpectedResultSettings) { Json = string.Format( - CultureInfo.InvariantCulture, + CultureInfo.InvariantCulture, testCase.Json, JsonLightWriterUtils.GetMetadataUrlPropertyForProperty(testCase.EntityType.FullTypeName()) + ","), FragmentExtractor = (result) => result.RemoveAllAnnotations(true) @@ -796,7 +796,7 @@ public void TopLevelOpenComplexProperties() { TestWriterUtils.WriteAndVerifyODataEdmPayload(testDescriptor, testConfiguration, this.Assert, this.Logger); }); - + } private sealed class EntryPayloadTestCase { diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs index 4c1a4f7a4d..4bbe0a1ff0 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs @@ -166,6 +166,29 @@ internal static void WriteActionAndVerifyODataPayload(Action( this PayloadWriterTestDescriptor testDescriptor, WriterTestConfiguration testConfiguration, bool alwaysSpecifyOwningContainer = false, - BaselineLogger baselineLogger = null, + BaselineLogger baselineLogger = null, Action writeAction = null) { T property = testDescriptor.PayloadItems.Single(); @@ -679,9 +702,9 @@ private static void WritePayload(ODataMessageWriterTestWrapper messageWriter, OD /// A boolean flag indicating whether to flush the writer after writing. /// The payload items to write. /// - /// The list of is interpreted in the following way: + /// The list of is interpreted in the following way: /// for every non-null item we call WriteStart; for every null item we call WriteEnd. - /// null items can be omitted at the end of the list, e.g., a list of a feed and an entry + /// null items can be omitted at the end of the list, e.g., a list of a feed and an entry /// item would be 'auto-completed' with two null entries. /// internal static void WritePayload(ODataMessageWriterTestWrapper messageWriter, ODataWriter writer, bool flush, IList items, int throwUserExceptionAt = -1) From fdcbb0a24ef703e6f80a05263ed7bc62ac98fb8e Mon Sep 17 00:00:00 2001 From: Biao Li Date: Mon, 5 Mar 2018 02:22:44 -0800 Subject: [PATCH 03/16] Code updates for walk-thru comments. --- .../ODataJsonLightPropertySerializer.cs | 24 +++--------------- .../JsonLight/ODataJsonLightReader.cs | 2 +- src/Microsoft.OData.Core/ODataConstants.cs | 25 ++++++++++--------- .../ODataMessageWriter.cs | 10 ++++---- .../ODataMessageWriterSettings.cs | 2 +- .../ODataOutputContext.cs | 9 +++++++ .../ODataPreferenceHeader.cs | 12 +++------ src/Microsoft.OData.Core/ODataWriterCore.cs | 7 ++---- .../ODataJsonLightPropertySerializerTests.cs | 1 + ...ataJsonLightPropertyTypeSerializerTests.cs | 1 + .../PublicApi/PublicApi.bsl | 2 +- ...mitNullPropertiesInEntryTest.approved.txt} | 0 ...osoft.Test.Taupo.OData.Writer.Tests.csproj | 2 +- .../OData.Writer.Tests/TestWriterUtils.cs | 6 +++++ 14 files changed, 47 insertions(+), 56 deletions(-) rename test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/{JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt => JsonLightEntryWriterTests.OmitNullPropertiesInEntryTest.approved.txt} (100%) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index 2bce810611..1a2797d9e3 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -92,26 +92,6 @@ internal void WriteTopLevelProperty(ODataProperty property) }); } - /// - /// This is the backward-compatible method to write property names and value pairs - /// without omitting null property values. - /// - /// The of the resource (or null if not metadata is available). - /// The enumeration of properties to write out. - /// - /// 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 - /// - /// The DuplicatePropertyNameChecker to use. - internal void WriteProperties( - IEdmStructuredType owningType, - IEnumerable properties, - bool isComplexValue, - IDuplicatePropertyNameChecker duplicatePropertyNameChecker) - { - this.WriteProperties(owningType, properties, isComplexValue, /*omitNullValues*/ false, duplicatePropertyNameChecker); - } - /// /// Writes property names and value pairs. /// @@ -165,7 +145,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 @@ -188,6 +168,7 @@ 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. /// 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( @@ -381,6 +362,7 @@ private void WriteStreamReferenceProperty(string propertyName, ODataStreamRefere /// Writes a Null property. /// /// The property to write out. + /// Whether to omit the writing of this null property value. private void WriteNullProperty( ODataProperty property, bool omitNullValues) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index 94dfe4efaf..05808e5837 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -2335,7 +2335,7 @@ private void RestoreOmittedNullValues() { // For entity resource, concatenate the property to original enumerable. resource.Properties = - resource.Properties.Concat(new List() {property}); + resource.Properties.Concat(new List() { property }); } } } diff --git a/src/Microsoft.OData.Core/ODataConstants.cs b/src/Microsoft.OData.Core/ODataConstants.cs index 661afa5135..b29469d53b 100644 --- a/src/Microsoft.OData.Core/ODataConstants.cs +++ b/src/Microsoft.OData.Core/ODataConstants.cs @@ -59,6 +59,19 @@ internal static class ODataInternalConstants /// public const string ContentIdHeader = "Content-ID"; + #region Preference Headers + /// + /// Valid value for OmitValuesPreferenceToken indicating nulls are omitted. + /// + public const string OmitValuesNulls = "nulls"; + + /// + /// Valid value for OmitValuesPreferenceToken indicating defaults are omitted. + /// Internal access only since "defaults" is not supported yet. + /// + internal const string OmitValuesDefaults = "defaults"; + #endregion Preference Headers + /// /// Name of the Content-Length HTTP header. /// @@ -207,17 +220,5 @@ internal static class ODataInternalConstants internal const string ContextUriDeletedLink = UriSegmentSeparator + DeletedLink; #endregion Context URL - - #region Preference Headers - /// - /// Valid value for OmitValuesPreferenceToken indicating nulls are omitted. - /// - internal const string OmitValuesNulls = "nulls"; - - /// - /// Valid value for OmitValuesPreferenceToken indicating defaults are omitted. - /// - internal const string OmitValuesDefaults = "defaults"; - #endregion Preference Headers } } diff --git a/src/Microsoft.OData.Core/ODataMessageWriter.cs b/src/Microsoft.OData.Core/ODataMessageWriter.cs index b13010ba53..be02bc0ccb 100644 --- a/src/Microsoft.OData.Core/ODataMessageWriter.cs +++ b/src/Microsoft.OData.Core/ODataMessageWriter.cs @@ -151,12 +151,12 @@ public ODataMessageWriter(IODataResponseMessage responseMessage, ODataMessageWri WriterValidationUtils.ValidateMessageWriterSettings(this.settings, this.writingResponse); this.message = new ODataResponseMessage(responseMessage, /*writing*/ true, this.settings.EnableMessageStreamDisposal, /*maxMessageSize*/ -1); - // Set the Preference-Applied header's parameter omit-values=nulls per writer settings. - // For response writing, source of specifying omit-null-values preference is the settings object - // from caller(the OData service). - if (this.settings.OmitNullValues) + // Set the writer settings per Preference-Applied header's parameter omit-values. + // For response writing, source of specifying omit-null-values preference is the Preference-Applied header. + if (string.Equals(responseMessage.PreferenceAppliedHeader().OmitValues, ODataConstants.OmitValuesNulls, + StringComparison.OrdinalIgnoreCase)) { - responseMessage.PreferenceAppliedHeader().OmitValues = ODataConstants.OmitValuesNulls; + this.settings.OmitNullValues = 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/ODataMessageWriterSettings.cs b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs index db20f4f8a9..fe80ffba81 100644 --- a/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs +++ b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs @@ -189,7 +189,7 @@ public ODataUri ODataUri /// /// Default value is false, that means serialize null values. /// - public bool OmitNullValues { get; set; } + internal bool OmitNullValues { get; set; } /// /// Returns whether ThrowOnUndeclaredPropertyForNonOpenType validation setting is enabled. diff --git a/src/Microsoft.OData.Core/ODataOutputContext.cs b/src/Microsoft.OData.Core/ODataOutputContext.cs index 801972a4f2..e8e7af9587 100644 --- a/src/Microsoft.OData.Core/ODataOutputContext.cs +++ b/src/Microsoft.OData.Core/ODataOutputContext.cs @@ -437,6 +437,15 @@ public virtual Task WriteErrorAsync(ODataError odataError, bool includeDebugInfo } #endif + /// + /// Returns whether properties of null values should be omitted when serializing the payload. + /// + /// Return true to omit null-value properties; false otherwise. + internal bool ShouldOmitNullValues() + { + return this.WritingResponse && this.MessageWriterSettings.OmitNullValues; + } + /// /// Writes an into the message payload. /// diff --git a/src/Microsoft.OData.Core/ODataPreferenceHeader.cs b/src/Microsoft.OData.Core/ODataPreferenceHeader.cs index 4c2aa87948..7642edb408 100644 --- a/src/Microsoft.OData.Core/ODataPreferenceHeader.cs +++ b/src/Microsoft.OData.Core/ODataPreferenceHeader.cs @@ -260,7 +260,7 @@ public string OmitValues get { HttpHeaderValueElement omitValues = this.Get(OmitValuesPreferenceToken); - return omitValues?.Value; + return omitValues != null ? omitValues.Value : null; } [SuppressMessage("Microsoft.Globalization", "CA1308:NormalizeStringsToUppercase", Justification = "Need lower case string here.")] @@ -273,14 +273,8 @@ public string OmitValues { this.Set(new HttpHeaderValueElement(OmitValuesPreferenceToken, ODataConstants.OmitValuesNulls, EmptyParameters)); } - else if (string.Equals(normalizedValue, ODataConstants.OmitValuesDefaults, StringComparison.Ordinal)) - { - throw new NotSupportedException("omit-values=defaults is not supported yet."); - } - else - { - throw new ODataException(Strings.HttpPreferenceHeader_InvalidValueForToken(value, OmitValuesPreferenceToken)); - } + + // No-ops for other preference values, including "defaults" which is in the OData protocol but not implemented yet. } } diff --git a/src/Microsoft.OData.Core/ODataWriterCore.cs b/src/Microsoft.OData.Core/ODataWriterCore.cs index 59fa9d85ab..8ed12769ae 100644 --- a/src/Microsoft.OData.Core/ODataWriterCore.cs +++ b/src/Microsoft.OData.Core/ODataWriterCore.cs @@ -1150,11 +1150,8 @@ protected void ValidateNoDeltaLinkForExpandedResourceSet(ODataResourceSet resour /// Return true to omit null-value properties; false otherwise. protected bool ShouldOmitNullValues() { - // Omit null values preference should only affect response payload, and should not - // be applied to writing delta payload. - return this.outputContext.WritingResponse - && !this.writingDelta - && this.outputContext.MessageWriterSettings.OmitNullValues; + // Per protocol, omitNullValues doesn't affect delta payload. + return !this.writingDelta && this.outputContext.ShouldOmitNullValues(); } /// diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightPropertySerializerTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightPropertySerializerTests.cs index a073b587ea..3f04d502d3 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightPropertySerializerTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightPropertySerializerTests.cs @@ -313,6 +313,7 @@ private string SerializeProperty(IEdmStructuredType owningType, ODataProperty od owningType, new[] { odataProperty }, /*isComplexValue*/ false, + /*omitNullValues*/ false, new NullDuplicatePropertyNameChecker()); jsonLightOutputContext.JsonWriter.EndObjectScope(); diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/JsonLight/ODataJsonLightPropertyTypeSerializerTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/JsonLight/ODataJsonLightPropertyTypeSerializerTests.cs index cd66510db4..d840304e18 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/JsonLight/ODataJsonLightPropertyTypeSerializerTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/Writer/JsonLight/ODataJsonLightPropertyTypeSerializerTests.cs @@ -445,6 +445,7 @@ private string SerializeProperty(ODataProperty odataProperty, ODataVersion versi this.entityType, new[] { odataProperty }, /*isComplexValue*/ false, + /*omitNullValues*/ false, new NullDuplicatePropertyNameChecker()); jsonLightOutputContext.JsonWriter.EndObjectScope(); diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl b/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl index 84c56b0efc..e653f25dfd 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl @@ -4552,6 +4552,7 @@ public sealed class Microsoft.OData.ODataConstants { public static string MethodPost = "POST" public static string MethodPut = "PUT" public static string ODataVersionHeader = "OData-Version" + public static string OmitValuesNulls = "nulls" } [ @@ -5194,7 +5195,6 @@ public sealed class Microsoft.OData.ODataMessageWriterSettings { Microsoft.OData.ODataLibraryCompatibility LibraryCompatibility { public get; public set; } Microsoft.OData.ODataMessageQuotas MessageQuotas { public get; public set; } Microsoft.OData.ODataUri ODataUri { public get; public set; } - bool OmitNullValues { public get; public set; } Microsoft.OData.ValidationKinds Validations { public get; public set; } System.Nullable`1[[Microsoft.OData.ODataVersion]] Version { public get; public set; } diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.OmitNullPropertiesInEntryTest.approved.txt similarity index 100% rename from test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt rename to test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/JsonLight/JsonLightEntryWriterTests.OmitNullPropertiesInEntryTest.approved.txt diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj index d1d7ea011e..5f09f08824 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj @@ -182,7 +182,7 @@ PreserveNewest - + PreserveNewest diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs index 4bbe0a1ff0..e96750b44b 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs @@ -511,6 +511,12 @@ public static TestMessage CreateOutputMessageFromStream( } responseMessage.StatusCode = 200; + + if (testConfiguration.MessageWriterSettings.OmitNullValues) + { + responseMessage.PreferenceAppliedHeader().OmitValues = Microsoft.OData.ODataConstants.OmitValuesNulls; + } + message = responseMessage; } From defcef96553074e456cbfeead37be376a4c9b743 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Thu, 8 Mar 2018 13:17:47 -0800 Subject: [PATCH 04/16] Various updates including: - Rebase to tip of ODL master branch. - Add e2e test ExpandedCustomerEntryOmitNullValuesTest with various cases exercising null values. - Updates restore null value processing related to base-type properties navigation properties by odata.navigationlink annotations. - Added output for comparison of omittedNullValues of true & false in same test cases. - Added test cases for unknonwn(dynamic) properties to verify that no content are restored. - Added test cases for expanded navigation link to verify that omitted properties are restored for non-null entity, and that no content are restored for null entity. --- .../ODataJsonLightPropertySerializer.cs | 19 +- .../JsonLight/ODataJsonLightReader.cs | 28 +- .../ODataMessageWriterSettings.cs | 1 + .../SelectedPropertiesNode.cs | 15 +- .../WriteJsonWithoutModelTests.cs | 139 ++++- .../WritePayloadHelper.cs | 31 +- ...AndValueJsonLightReaderIntegrationTests.cs | 482 ++++++++++++++---- .../ODataJsonLightDeserializerTests.cs | 55 +- .../SelectedPropertiesNodeTests.cs | 2 +- .../OData.Writer.Tests/TestWriterUtils.cs | 6 +- 10 files changed, 621 insertions(+), 157 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index 1a2797d9e3..8fed1507b8 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -181,6 +181,8 @@ private void WriteProperty( { WriterValidationUtils.ValidatePropertyNotNull(property); + ODataValue value = property.ODataValue; + string propertyName = property.Name; if (this.JsonLightOutputContext.MessageWriterSettings.Validations != ValidationKinds.None) @@ -197,6 +199,19 @@ private void WriteProperty( this.currentPropertyInfo = this.JsonLightOutputContext.PropertyCacheHandler.GetProperty(propertyName, owningType); } + // 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. + // For very wide and sparse outputs it allows to avoid a lot of dictionary lookups + bool isNullValue = (value == null || value is ODataNullValue); + if (isNullValue && omitNullValues) + { + if (!this.currentPropertyInfo.IsTopLevel && !this.MessageWriterSettings.ThrowIfTypeConflictsWithMetadata) + { + return; + } + } + WriterValidationUtils.ValidatePropertyDefined(this.currentPropertyInfo, this.MessageWriterSettings.ThrowOnUndeclaredPropertyForNonOpenType); duplicatePropertyNameChecker.ValidatePropertyUniqueness(property); @@ -208,8 +223,6 @@ private void WriteProperty( WriteInstanceAnnotation(property, isTopLevel, currentPropertyInfo.MetadataType.IsUndeclaredProperty); - ODataValue value = property.ODataValue; - // handle ODataUntypedValue ODataUntypedValue untypedValue = value as ODataUntypedValue; if (untypedValue != null) @@ -233,7 +246,7 @@ private void WriteProperty( return; } - if (value is ODataNullValue || value == null) + if (isNullValue) { this.WriteNullProperty(property, omitNullValues); return; diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index 05808e5837..c860b285e2 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -2284,26 +2284,29 @@ private void RestoreOmittedNullValues() ODataResourceBase resource = resourceState.Resource; IEnumerable selectedProperties; + if (resourceState.SelectedProperties == SelectedPropertiesNode.EntireSubtree) { - selectedProperties = edmStructuredType.DeclaredProperties; + selectedProperties = GetSelfAndBaseTypeProperties(edmStructuredType); } else - { // Partial subtree. Combine navigation properties and selected properties at the node with distinct. + { + // 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).Values) + resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType)) .Distinct(); } foreach (IEdmProperty currentProperty in selectedProperties) { Debug.Assert(currentProperty.Type != null, "currentProperty.Type != null"); - if (!currentProperty.Type.IsNullable) + if (!currentProperty.Type.IsNullable || currentProperty.PropertyKind == EdmPropertyKind.Navigation) { - // Skip declared properties that are not null-able types. + // 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; } @@ -2344,6 +2347,21 @@ private void RestoreOmittedNullValues() } } + /// + /// Get all properties defined by the EDM structural type and its base types. + /// + /// The EDM structural type. + /// All the properties of this type. + private static IEnumerable GetSelfAndBaseTypeProperties(IEdmStructuredType edmStructuredType) + { + if (edmStructuredType == null) + { + return Enumerable.Empty(); + } + + return edmStructuredType.DeclaredProperties.Concat(GetSelfAndBaseTypeProperties(edmStructuredType.BaseType)); + } + /// /// Add info resolved from context url to current scope. /// diff --git a/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs index fe80ffba81..a7ffb5724c 100644 --- a/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs +++ b/src/Microsoft.OData.Core/ODataMessageWriterSettings.cs @@ -414,6 +414,7 @@ private void CopyFrom(ODataMessageWriterSettings other) this.Version = other.Version; this.OmitNullValues = other.OmitNullValues; this.LibraryCompatibility = other.LibraryCompatibility; + this.validations = other.validations; this.ThrowIfTypeConflictsWithMetadata = other.ThrowIfTypeConflictsWithMetadata; this.ThrowOnDuplicatePropertyNames = other.ThrowOnDuplicatePropertyNames; diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index ab7187280a..6eb5d6c209 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -38,8 +38,8 @@ internal sealed class SelectedPropertiesNode /// An empty set of navigation properties to return when nothing is selected. private static readonly IEnumerable EmptyNavigationProperties = Enumerable.Empty(); - /// An empty dictionary of EDM properties to return when nothing is selected. - private static readonly Dictionary EmptyEdmProperties = new Dictionary(StringComparer.Ordinal); + /// An empty enumeration of EDM properties to return when nothing is selected. + private static readonly IEnumerable EmptyEdmProperties = Enumerable.Empty(); /// The type of the current node. private SelectionType selectionType = SelectionType.PartialSubtree; @@ -449,7 +449,7 @@ internal IEnumerable GetSelectedNavigationProperties(IEd /// /// The current structured type. /// The selected properties at this node level. - internal IDictionary GetSelectedProperties(IEdmStructuredType structuredType) + internal IEnumerable GetSelectedProperties(IEdmStructuredType structuredType) { if (this.selectionType == SelectionType.Empty) { @@ -464,16 +464,13 @@ internal IDictionary GetSelectedProperties(IEdmStructuredT if (this.selectionType == SelectionType.EntireSubtree || this.hasWildcard) { - return structuredType.DeclaredProperties.ToDictionary(sp => sp.Name, StringComparer.Ordinal); + return structuredType.DeclaredProperties; } Debug.Assert(this.selectedProperties != null, "selectedProperties != null"); - IDictionary selectedEdmProperties = this.selectedProperties - .Select(structuredType.FindProperty) - .ToDictionary(p => p.Name, StringComparer.Ordinal); - - return selectedEdmProperties; + // Get declared properties selected, and filter out unrecognized properties. + return this.selectedProperties.Select(structuredType.FindProperty).OfType(); } /// diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs index 72cc1e0fbc..edb127832d 100644 --- a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs @@ -4,6 +4,8 @@ // //--------------------------------------------------------------------- +using System.Runtime.CompilerServices; + namespace Microsoft.Test.OData.Tests.Client.WriteJsonPayloadTests { using System; @@ -145,6 +147,36 @@ public void ExpandedCustomerEntryTest() } } + /// + /// Write and read an expanded customer entry containing primitive, complex, collection of primitive/complex properties with null values + /// using omit-value=nulls Preference header. + /// + [TestMethod] + public void ExpandedCustomerEntryOmitNullValuesTest() + { + // Repeat with full and minimal metadata. + foreach (string mimeType in this.mimeTypes.GetRange(0, 2)) + { + var settings = new ODataMessageWriterSettings(); + settings.ODataUri = new ODataUri() {ServiceRoot = this.ServiceUri}; + string rspRecvd = null; + + var responseMessageWithModel = new StreamResponseMessage(new MemoryStream()); + responseMessageWithModel.SetHeader("Content-Type", mimeType); + responseMessageWithModel.PreferenceAppliedHeader().OmitValues = ODataConstants.OmitValuesNulls; + + using (var messageWriter = new ODataMessageWriter(responseMessageWithModel, settings, + WritePayloadHelper.Model)) + { + var odataWriter = messageWriter.CreateODataResourceWriter(WritePayloadHelper.CustomerSet, + WritePayloadHelper.CustomerType); + rspRecvd = this.WriteAndVerifyExpandedCustomerEntryWithSomeNullValues( + responseMessageWithModel, odataWriter, /* hasModel */false); + Assert.IsNotNull(rspRecvd); + } + } + } + /// /// Write an entry containing stream, named stream /// @@ -519,10 +551,9 @@ private string WriteAndVerifyOrderFeed(StreamResponseMessage responseMessage, OD return WritePayloadHelper.ReadStreamContent(stream); } - private string WriteAndVerifyExpandedCustomerEntry(StreamResponseMessage responseMessage, - ODataWriter odataWriter, bool hasModel, string mimeType) + private void WriteCustomerEntry( ODataWriter odataWriter, bool hasModel, bool withSomeNullValues) { - ODataResourceWrapper customerEntry = WritePayloadHelper.CreateCustomerEntry(hasModel); + ODataResourceWrapper customerEntry = WritePayloadHelper.CreateCustomerEntry(hasModel, withSomeNullValues); var loginFeed = new ODataResourceSet() { Id = new Uri(this.ServiceUri + "Customer(-9)/Logins") }; if (!hasModel) @@ -533,30 +564,41 @@ private string WriteAndVerifyExpandedCustomerEntry(StreamResponseMessage respons var loginEntry = WritePayloadHelper.CreateLoginEntry(hasModel); - customerEntry.NestedResourceInfoWrappers = customerEntry.NestedResourceInfoWrappers.Concat(WritePayloadHelper.CreateCustomerNavigationLinks()); - customerEntry.NestedResourceInfoWrappers = customerEntry.NestedResourceInfoWrappers.Concat(new[]{ new ODataNestedResourceInfoWrapper() - { - NestedResourceInfo = new ODataNestedResourceInfo() - { - Name = "Logins", - IsCollection = true, - Url = new Uri(this.ServiceUri + "Customer(-9)/Logins") - }, - NestedResourceOrResourceSet = new ODataResourceSetWrapper() - { - ResourceSet = loginFeed, - Resources = new List() - { - new ODataResourceWrapper() + // Setting null values for some navigation links, those null links won't be serialized + // as odata.navigationlink annotations. + customerEntry.NestedResourceInfoWrappers = customerEntry.NestedResourceInfoWrappers.Concat( + WritePayloadHelper.CreateCustomerNavigationLinks(withSomeNullValues)); + customerEntry.NestedResourceInfoWrappers = customerEntry.NestedResourceInfoWrappers.Concat( + new[]{ new ODataNestedResourceInfoWrapper() { - Resource = loginEntry, - NestedResourceInfoWrappers = WritePayloadHelper.CreateLoginNavigationLinksWrapper().ToList() + NestedResourceInfo = new ODataNestedResourceInfo() + { + Name = "Logins", + IsCollection = true, + Url = new Uri(this.ServiceUri + "Customer(-9)/Logins") + }, + NestedResourceOrResourceSet = new ODataResourceSetWrapper() + { + ResourceSet = loginFeed, + Resources = new List() + { + new ODataResourceWrapper() + { + Resource = loginEntry, + NestedResourceInfoWrappers = WritePayloadHelper.CreateLoginNavigationLinksWrapper(withSomeNullValues).ToList() + } + } + } } - } - } - }}); + }); ODataWriterHelper.WriteResource(odataWriter, customerEntry); + } + + private string WriteAndVerifyExpandedCustomerEntry(StreamResponseMessage responseMessage, + ODataWriter odataWriter, bool hasModel, string mimeType) + { + WriteCustomerEntry(odataWriter, hasModel, false); // Some very basic verification for the payload. bool verifyFeedCalled = false; @@ -601,6 +643,57 @@ private string WriteAndVerifyExpandedCustomerEntry(StreamResponseMessage respons return WritePayloadHelper.ReadStreamContent(stream); } + private string WriteAndVerifyExpandedCustomerEntryWithSomeNullValues(StreamResponseMessage responseMessage, + ODataWriter odataWriter, bool hasModel) + { + WriteCustomerEntry(odataWriter, hasModel, /* withSomeNullValues */true); + + // Some very basic verification for the payload. + bool verifyFeedCalled = false; + int verifyEntryCalled = 0; + int verifyNullValuesCount = 0; + bool verifyNavigationCalled = false; + Action verifyFeed = (feed) => + { + verifyFeedCalled = true; + }; + + Action verifyEntry = (entry) => + { + if (entry.TypeName.Contains("Customer")) + { + Assert.AreEqual(4, entry.Properties.Count()); + verifyEntryCalled++; + } + + if (entry.TypeName.Contains("Login")) + { + Assert.AreEqual(2, entry.Properties.Count()); + verifyEntryCalled++; + } + + // Counting restored null property value during response de-serialization. + verifyNullValuesCount += entry.Properties.Count(p => p.Value == null); + }; + + Action verifyNavigation = (navigation) => + { + Assert.IsNotNull(navigation.Name); + verifyNavigationCalled = true; + }; + + Stream stream = responseMessage.GetStream(); + stream.Seek(0, SeekOrigin.Begin); + WritePayloadHelper.ReadAndVerifyFeedEntryMessage(false, responseMessage, WritePayloadHelper.CustomerSet, WritePayloadHelper.CustomerType, + verifyFeed, verifyEntry, verifyNavigation); + Assert.IsTrue(verifyFeedCalled); + Assert.IsTrue(verifyEntryCalled == 2); + Assert.IsTrue(verifyNullValuesCount == 4); + Assert.IsTrue(verifyNavigationCalled); + + return WritePayloadHelper.ReadStreamContent(stream); + } + private string WriteAndVerifyCarEntry(StreamResponseMessage responseMessage, ODataWriter odataWriter, bool hasModel, string mimeType) { diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WritePayloadHelper.cs b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WritePayloadHelper.cs index b6aeb4bce0..e832e50405 100644 --- a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WritePayloadHelper.cs +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WritePayloadHelper.cs @@ -4,6 +4,8 @@ // //--------------------------------------------------------------------- +using System.Diagnostics; + namespace Microsoft.Test.OData.Tests.Client.WriteJsonPayloadTests { using System; @@ -594,9 +596,9 @@ public static ODataResourceWrapper CreateOrderEntry2(bool hasModel) #region ExpandedCustomerEntryTestHelper - public static ODataResourceWrapper CreateCustomerEntry(bool hasModel) + public static ODataResourceWrapper CreateCustomerEntry(bool hasModel, bool withSomeNullValues = false) { - var customerEntryWrapper = CreateCustomerResourceWrapperNoMetadata(hasModel); + var customerEntryWrapper = CreateCustomerResourceWrapperNoMetadata(hasModel, withSomeNullValues); var customerEntry = customerEntryWrapper.Resource; customerEntry.Id = new Uri(ServiceUri + "Customer(-9)"); @@ -1079,7 +1081,7 @@ public static ODataResourceWrapper CreateAuditInforWrapper() }; } - public static IEnumerable CreateCustomerNavigationLinks() + public static IEnumerable CreateCustomerNavigationLinks(bool withSomeNullValues) { return new List() { @@ -1098,7 +1100,9 @@ public static IEnumerable CreateCustomerNavigati { Name = "Husband", IsCollection = false, - Url = new Uri(ServiceUri + "Customer(-9)/Husband") + Url = withSomeNullValues + ? null + : new Uri(ServiceUri + "Customer(-9)/Husband") } }, new ODataNestedResourceInfoWrapper() @@ -1107,7 +1111,9 @@ public static IEnumerable CreateCustomerNavigati { Name = "Wife", IsCollection = false, - Url = new Uri(ServiceUri + "Customer(-9)/Wife") + Url = withSomeNullValues + ? null + : new Uri(ServiceUri + "Customer(-9)/Wife") } }, new ODataNestedResourceInfoWrapper() @@ -1131,7 +1137,7 @@ public static ODataResource CreateLoginEntry(bool hasModel) return loginEntry; } - public static IEnumerable CreateLoginNavigationLinksWrapper() + public static IEnumerable CreateLoginNavigationLinksWrapper(bool withSomeNullValues) { return new List() { @@ -1150,7 +1156,9 @@ public static IEnumerable CreateLoginNavigationL { Name = "LastLogin", IsCollection = false, - Url = new Uri(ServiceUri + "Login('2')/LastLogin") + Url = withSomeNullValues + ? null + : new Uri(ServiceUri + "Login('2')/LastLogin") } }, new ODataNestedResourceInfoWrapper() @@ -1159,7 +1167,9 @@ public static IEnumerable CreateLoginNavigationL { Name = "SentMessages", IsCollection = true, - Url = new Uri(ServiceUri + "Login('2')/SentMessages") + Url = withSomeNullValues + ? null + : new Uri(ServiceUri + "Login('2')/SentMessages") } }, new ODataNestedResourceInfoWrapper() @@ -1386,7 +1396,7 @@ public static ODataNestedResourceInfo AddOrderEntryCustomNavigation(ODataResourc return orderEntry2Navigation; } - public static ODataResourceWrapper CreateCustomerResourceWrapperNoMetadata(bool hasModel) + public static ODataResourceWrapper CreateCustomerResourceWrapperNoMetadata(bool hasModel, bool withSomeNullValues = false) { var customerEntry = new ODataResource() { @@ -1394,7 +1404,8 @@ public static ODataResourceWrapper CreateCustomerResourceWrapperNoMetadata(bool }; var customerEntryP1 = new ODataProperty { Name = "CustomerId", Value = -9 }; - var customerEntryP2 = new ODataProperty { Name = "Name", Value = "CustomerName" }; + string aCustomerName = withSomeNullValues ? null : "CustomerName"; + var customerEntryP2 = new ODataProperty {Name = "Name", Value = aCustomerName }; var primaryContactInfo_nestedInfoWrapper = new ODataNestedResourceInfoWrapper() { 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 d6f934ff14..1fa92ed8cc 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 @@ -8,6 +8,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using System.Text; using FluentAssertions; using Microsoft.OData.Tests.JsonLight; @@ -367,24 +368,26 @@ public void ReadingTypeDefinitionPayloadJsonLight() address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.OrdinalIgnoreCase)).Value.Should().Be("China"); } - [Fact] - public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues(bool bNullValuesOmitted) { EdmEntityType entityType; EdmEntitySet entitySet; EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); const string payload = @" -{ + { ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People/$entity"", ""@odata.id"":""http://mytest"", ""Id"":0, ""Weight"":60.5, ""Address"":{""CountryRegion"":""US"", ""City"":""Redmond""} -}"; + }"; List entries = new List(); - ODataNestedResourceInfo navigationLink = null; + List nestedResourceInfos = new List(); this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { @@ -394,13 +397,13 @@ public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues() entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLink = (ODataNestedResourceInfo)reader.Item; + nestedResourceInfos.Add((ODataNestedResourceInfo)reader.Item); break; default: break; } }, - nullValuesOmitted: true); + nullValuesOmitted: bNullValuesOmitted); Assert.Equal(2, entries.Count); @@ -409,20 +412,38 @@ public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues() .Value.Should().Be(0); person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)) .Value.Should().Be(60.5); - // omitted value should be restored to null. - person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) - .Value.Should().BeNull(); + 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(); + } ODataResource address = entries[1]; address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.Ordinal)) .Value.Should().Be("US"); address.Properties.FirstOrDefault(s => string.Equals(s.Name, "City", StringComparison.Ordinal)) .Value.Should().Be("Redmond"); - // Omitted value should be restored to null. - address.Properties.FirstOrDefault(s => string.Equals(s.Name, "ZipCode", StringComparison.Ordinal)) - .Value.Should().BeNull(); - navigationLink.Name.Should().Be("Address"); + if (bNullValuesOmitted) + { + // Omitted value should be restored to null. + address.Properties.FirstOrDefault(s => string.Equals(s.Name, "ZipCode", StringComparison.Ordinal)) + .Value.Should().BeNull(); + } + else + { + address.Properties.Any(s => string.Equals(s.Name, "ZipCode", StringComparison.Ordinal)).Should().BeFalse(); + } + + nestedResourceInfos.Count().Should().Be(2); + nestedResourceInfos[0].Name.Should().Be("Address"); + // Navigation link missing from payload. + nestedResourceInfos[1].Name.Should().Be("Company"); } [Fact] @@ -845,8 +866,11 @@ public void ReadingNullValueForDeclaredCollectionPropertyInComplexTypeShouldFail read.ShouldThrow().WithMessage("A null value was found for the property named 'CountriesOrRegions', which has the expected type 'Collection(Edm.String)[Nullable=False]'. The expected type 'Collection(Edm.String)[Nullable=False]' does not allow null values."); } - [Fact] - public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValues() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValues( + bool bNullValuesOmitted) { EdmEntityType entityType; EdmEntitySet entitySet; @@ -872,29 +896,55 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu break; } }, - nullValuesOmitted: true); + nullValuesOmitted: bNullValuesOmitted); Assert.Equal(2, entries.Count); ODataResource edu = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); edu.Should().NotBeNull(); edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)).Value.Should().Be(1); - // null-able collection should be restored. - edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Value.Should().BeNull(); - // null-able property value should be restored. - edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Value.Should().BeNull(); + + if (bNullValuesOmitted) + { + // null-able collection should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Value.Should().BeNull(); + // null-able property value should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Value.Should().BeNull(); + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "OrgId", StringComparison.Ordinal)).Value.Should().BeNull(); + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "OrgCategory", StringComparison.Ordinal)).Value.Should().BeNull(); + + } + else + { + edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); + edu.Properties.Any(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Should().BeFalse(); + edu.Properties.Any(s => string.Equals(s.Name, "OrgId", StringComparison.Ordinal)).Should().BeFalse(); + edu.Properties.Any(s => string.Equals(s.Name, "OrgCategory", StringComparison.Ordinal)).Should().BeFalse(); + } ODataResource person = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); person.Should().NotBeNull(); person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)).Value.Should().Be(0); - // omitted null-able property should be restored as null. - person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Height", StringComparison.Ordinal)).Value.Should().BeNull(); + + if (bNullValuesOmitted) + { + // omitted null-able property should be restored as null. + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Height", StringComparison.Ordinal)).Value.Should().BeNull(); + } + else + { + person.Properties.Any(s => string.Equals(s.Name, "Height", StringComparison.Ordinal)).Should().BeFalse(); + } + // missing non-null-able property should not be restored. person.Properties.Any(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)).Should().BeFalse(); } - [Fact] - public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesEntireSubTree() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesEntireSubTree( + bool bNullValuesOmitted) { EdmEntityType entityType; EdmEntitySet entitySet; @@ -904,13 +954,13 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu // null-able property Address is selected, thus should be restored. // Property Education is null-able. const string payloadWithQueryOption = @"{ - ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,Education,Address)/$entity"", + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,UnknownPropX,Education,Address)/$entity"", ""@odata.id"":""http://mytest"", ""Id"":0, ""Education"":{""Id"":1} }"; const string payloadWithWildcardInQueryOption = @"{ - ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,Education/*,Address)/$entity"", + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,UnknownPropX,Education/*,Address)/$entity"", ""@odata.id"":""http://mytest"", ""Id"":0, ""Education"":{""Id"":1} @@ -919,7 +969,7 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu foreach (string payload in new string[] {payloadWithQueryOption, payloadWithWildcardInQueryOption}) { List entries = new List(); - List navigationLinks = new List(); + List nestedResourceInfos = new List(); this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { @@ -929,17 +979,17 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLinks.Add(reader.Item as ODataNestedResourceInfo); + nestedResourceInfos.Add(reader.Item as ODataNestedResourceInfo); break; default: break; } }, - nullValuesOmitted: true); + nullValuesOmitted: bNullValuesOmitted); entries.Count.Should().Be(2); - navigationLinks.Count.Should().Be(1); - navigationLinks.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) + nestedResourceInfos.Count.Should().Be(1); + nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) .Should().NotBeNull(); // Education @@ -948,30 +998,54 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu edu.Should().NotBeNull(); edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) .Value.Should().Be(1); - // null-able collection should be restored. - edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)) - .Value.Should().BeNull(); - // null-able property value should be restored. - edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)) - .Value.Should().BeNull(); + if (bNullValuesOmitted) + { + // null-able collection should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)) + .Value.Should().BeNull(); + // null-able property value should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)) + .Value.Should().BeNull(); + } + else + { + edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); + edu.Properties.Any(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Should().BeFalse(); + } // Person ODataResource person = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); person.Should().NotBeNull(); + + // Verify that unknown property doesn't cause anomaly and is not restored. + person.Properties.Any(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Should().BeFalse(); + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) .Value.Should().Be(0); - // selected null-able property/navigationLink should be restored as null. - person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Address", StringComparison.Ordinal)) - .Value.Should().BeNull(); + + 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(); + } + // 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(); person.Properties.Any(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)).Should().BeFalse(); } } - [Fact] - public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesPartialSubTree() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesPartialSubTree( + bool bNullValuesOmitted) { EdmEntityType entityType; EdmEntitySet entitySet; @@ -980,74 +1054,260 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu // null-able property Height is not selected, thus should not be restored. // null-able property Address is selected, thus should be restored. // Property Education is null-able. - const string payloadWithWildcardInQueryOption = @"{ - ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20Education/Id,%20Education/SchoolName,%20Address)/$entity"", + const string payloadWithSelectedPropertiesPartialSubTreeInQueryOption = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20Education/Id,%20Education/SchoolName,%20Education/UnknownPropX,%20Address)/$entity"", ""@odata.id"":""http://mytest"", ""Id"":0, ""Education"":{""Id"":1} }"; - foreach (string payload in new string[] { payloadWithWildcardInQueryOption }) - { - List entries = new List(); - List navigationLinks = new List(); - this.ReadEntryPayload(model, payload, entitySet, entityType, - reader => + List entries = new List(); + List nestedResourceInfos = new List(); + this.ReadEntryPayload(model, payloadWithSelectedPropertiesPartialSubTreeInQueryOption, entitySet, entityType, + reader => + { + switch (reader.State) { - switch (reader.State) - { - case ODataReaderState.ResourceStart: - entries.Add(reader.Item as ODataResource); - break; - case ODataReaderState.NestedResourceInfoStart: - navigationLinks.Add(reader.Item as ODataNestedResourceInfo); - break; - default: - break; - } - }, - nullValuesOmitted: true); + 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); - navigationLinks.Count.Should().Be(1); - navigationLinks.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) - .Should().NotBeNull(); + entries.Count.Should().Be(2); + nestedResourceInfos.Count.Should().Be(1); + nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) + .Should().NotBeNull(); - // Education - ODataResource edu = - entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); - edu.Should().NotBeNull(); - edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) - .Value.Should().Be(1); + // Education + ODataResource edu = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + edu.Properties.Any(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Should().BeFalse(); + + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(1); + if (bNullValuesOmitted) + { // null-able property value should be restored. edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)) .Value.Should().BeNull(); - // not selected property should not be restored. - edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); } + else + { + edu.Properties.Any(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Should().BeFalse(); + } + + // not selected property should not be restored. + edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); } - [Fact] - public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesPartialSubTreeAndBaseTypeProperty() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValuesWithSelectedPropertiesPartialSubTreeAndBaseTypeProperty( + bool bNullValuesOmitted) { EdmEntityType entityType; EdmEntitySet entitySet; EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); - // null-able property Height is not selected, thus should not be restored. // null-able property Address is selected, thus should be restored. // Property Education is null-able. const string payloadWithWildcardInQueryOption = @"{ - ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20Education/Id,%20Education/OrgId,%20Address)/$entity"", - ""@odata.id"":""http://mytest"", + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20Education/Id,%20Education/OrgId,%20Address)/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Education"":{""Id"":1} + }"; + + List entries = new List(); + List nestedResourceInfos = new List(); + this.ReadEntryPayload(model, payloadWithWildcardInQueryOption, 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); + nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) + .Should().NotBeNull(); + + // Education + ODataResource edu = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(1); + + // selected null-able property from base type should be restored to null if omitted. + if (bNullValuesOmitted) + { + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "OrgId", StringComparison.Ordinal)) + .Value.Should().BeNull(); + } + else + { + edu.Properties.Any(s => string.Equals(s.Name, "OrgId", StringComparison.Ordinal)).Should().BeFalse(); + } + + // not selected property should not be restored. + edu.Properties.Any(s => string.Equals(s.Name, "OrgCategory", StringComparison.Ordinal)).Should().BeFalse(); + edu.Properties.Any(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Should().BeFalse(); + edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingWithSelectExpandClauseShouldRestoreOmittedNullValuesWithSelectedPropertiesPartialSubTree( + bool bNullValuesOmitted) + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + // null-able property Height is not selected, thus should not be restored. + // Property Education is null-able. + const string payloadWithSelectExpandClauseInQueryOption = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,Education/Id,Education/SchoolName,Company/Id,Company/Name)/$entity"", + ""@odata.id"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(0)"", + ""Id"":0, + ""Education"":{""Id"":1}, + ""Company"":{ + ""@odata.id"":""http://www.example.com/$metadata#EntityNs.MyContainer.Companies(11)"", + ""Id"":11} + }"; + + List entries = new List(); + List nestedResourceInfos = new List(); + this.ReadEntryPayload(model, payloadWithSelectExpandClauseInQueryOption, 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); + + nestedResourceInfos.Count.Should().Be(2); + ODataNestedResourceInfo eduNestedResouce = + nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)); + + // Edu is complex type, its Url should be null. + eduNestedResouce.Should().NotBeNull(); + eduNestedResouce.Url.Should().BeNull(); + // Navigation link for Company entity should be non-null. + ODataNestedResourceInfo companyLink = + nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Company", StringComparison.Ordinal)); + companyLink.Should().NotBeNull(); + companyLink.Url.Should().NotBeNull(); + + entries.Count.Should().Be(3); + // Education + ODataResource edu = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(1); + if (bNullValuesOmitted) + { + // null-able property value should be restored. + edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)) + .Value.Should().BeNull(); + } + else + { + edu.Properties.Any(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Should().BeFalse(); + } + // not selected property should not be restored. + edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); + + // Company + ODataResource company = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Cmpny", StringComparison.Ordinal)); + company.Should().NotBeNull(); + company.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(11); + if (bNullValuesOmitted) + { + company.Properties.FirstOrDefault(s => string.Equals(s.Name, "Name", StringComparison.Ordinal)) + .Value.Should().BeNull(); + } + else + { + company.Properties.Any(s => string.Equals(s.Name, "Name", StringComparison.Ordinal)).Should().BeFalse(); + } + + // Person + ODataResource person = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); + person.Should().NotBeNull(); + person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) + .Value.Should().Be(0); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingWithSelectExpandClauseShouldHaveNoContentForNullNavigationProperty( + bool bNullValuesOmitted) + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + // Context URL with subtree of navigation entity selected. + const string payloadWithSelectExpandClauseInQueryOption1 = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,Education/Id,Education/SchoolName,Company/Id,Company/Name)/$entity"", + ""@odata.id"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(0)"", + ""Id"":0, + ""Education"":{""Id"":1} + }"; + + // Context URL with entire navigation entity selected. + const string payloadWithSelectExpandClauseInQueryOption2 = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,Education/Id,Education/SchoolName,Company)/$entity"", + ""@odata.id"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(0)"", ""Id"":0, ""Education"":{""Id"":1} }"; - foreach (string payload in new string[] { payloadWithWildcardInQueryOption }) + foreach (string payload in new string[] + { + payloadWithSelectExpandClauseInQueryOption1, + payloadWithSelectExpandClauseInQueryOption2 + }) { List entries = new List(); - List navigationLinks = new List(); + List nestedResourceInfos = new List(); this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { @@ -1057,31 +1317,27 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLinks.Add(reader.Item as ODataNestedResourceInfo); + nestedResourceInfos.Add(reader.Item as ODataNestedResourceInfo); break; default: break; } }, - nullValuesOmitted: true); + nullValuesOmitted: bNullValuesOmitted); entries.Count.Should().Be(2); - navigationLinks.Count.Should().Be(1); - navigationLinks.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) - .Should().NotBeNull(); - - // Education - ODataResource edu = - entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); - edu.Should().NotBeNull(); - edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) - .Value.Should().Be(1); - // selected null-able property from base type should be restored to null if omitted. - edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "OrgId", StringComparison.Ordinal)) - .Value.Should().BeNull(); - // not selected property should not be restored. - edu.Properties.Any(s => string.Equals(s.Name, "SchoolName", StringComparison.Ordinal)).Should().BeFalse(); - edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); + // Expanded Company entity via null navigation link is not restored. + // see example: http://services.odata.org/V4/TripPinService/People('ronaldmundy')?$expand=Photo + entries.Any(s => string.Equals(s.TypeName, "NS.Cmpny", StringComparison.Ordinal)).Should().BeFalse(); + + nestedResourceInfos.Count.Should().Be(2); + // There should be a navigation link reported as missing from payload. + ODataNestedResourceInfo companyLink = + nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Company", StringComparison.Ordinal)); + companyLink.Should().NotBeNull(); + // Reported missing/un-processed navigation link has URL constructed from metadata and + // @odata.id annotation of navigation source. + companyLink.Url.Should().NotBeNull(); } } @@ -1186,7 +1442,7 @@ private EdmModel BuildEdmModelForOmittedNullValuesTestCases(out EdmEntityType en EdmModel model = new EdmModel(); entityType = new EdmEntityType("NS", "Person"); - entityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32); + entityType.AddKeys(entityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32)); EdmTypeDefinition weightType = new EdmTypeDefinition("NS", "Wgt", EdmPrimitiveTypeKind.Double); EdmTypeDefinitionReference weightTypeRef = new EdmTypeDefinitionReference(weightType, false); @@ -1230,13 +1486,35 @@ private EdmModel BuildEdmModelForOmittedNullValuesTestCases(out EdmEntityType en entityType.AddStructuralProperty("Education", educationTypeRef); + // Company entity type. + EdmEntityType companyEntityType = new EdmEntityType("NS", "Cmpny"); + entityType.AddKeys(companyEntityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32)); + companyEntityType.AddStructuralProperty("Name", + new EdmTypeDefinitionReference( + new EdmTypeDefinition("NS", "CmpyName", EdmPrimitiveTypeKind.String), + true)); + companyEntityType.AddStructuralProperty("Address", addressTypeRef); + + EdmNavigationPropertyInfo companyNav = new EdmNavigationPropertyInfo() + { + Name = "Company", + ContainsTarget = true, + Target = companyEntityType, + TargetMultiplicity = EdmMultiplicity.ZeroOrOne + }; + + // Add navigation link from Person to Company. + entityType.AddUnidirectionalNavigation(companyNav); + model.AddElement(weightType); model.AddElement(heightType); model.AddElement(addressType); model.AddElement(entityType); + model.AddElement(companyEntityType); EdmEntityContainer container = new EdmEntityContainer("EntityNs", "MyContainer"); entitySet = container.AddEntitySet("People", entityType); + container.AddEntitySet("Companies", companyEntityType); model.AddElement(container); return model; diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeserializerTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeserializerTests.cs index b99f2cf566..f8947eed50 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeserializerTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightDeserializerTests.cs @@ -13,6 +13,7 @@ using Microsoft.OData.Json; using Microsoft.OData.JsonLight; using Microsoft.OData.Edm; +using Microsoft.Test.OData.Utils.ODataLibTest; using Xunit; using ErrorStrings = Microsoft.OData.Strings; @@ -867,6 +868,58 @@ public void ParsingInstanceAnnotationsInTopLevelPropertyShouldSkipBaseOnSettings property.InstanceAnnotations.Count.Should().Be(0); } + [Fact] + public void ParsingPrimitiveTypePropertyInsideComplextPropertyShouldSucceed() + { + EdmModel model = this.CreateEdmModelWithEntity(); + + // Add complex type to model. + EdmComplexType complexType = new EdmComplexType("TestNamespace", "Addr"); + complexType.AddStructuralProperty("CountryRegion", EdmPrimitiveTypeKind.String); + IEdmComplexTypeReference complexTypeReference = new EdmComplexTypeReference(complexType, true); + + EdmEntityType entityType = model.GetEntityType("TestNamespace.Customer"); + entityType.AddStructuralProperty("Address", complexTypeReference); + + IEdmTypeReference primitiveTypeRef = + ((IEdmComplexTypeReference) (entityType.GetProperty("Address").Type)).FindProperty( + "CountryRegion").Type; + + string payload = @"{ + ""@odata.context"":""http://odata.org/test/$metadata#Customers(1)/Address/CountryRegion"", + ""value"":""US"" + }"; + + ODataJsonLightPropertyAndValueDeserializer deserializer = new ODataJsonLightPropertyAndValueDeserializer(this.CreateJsonLightInputContext(payload, model)); + ODataProperty property = deserializer.ReadTopLevelProperty(primitiveTypeRef); + property.Value.Should().Be("US"); + } + + [Fact] + public void ParsingTopLevelComplextPropertyShouldFail() + { + EdmModel model = this.CreateEdmModelWithEntity(); + + // Add complex type to model. + EdmComplexType complexType = new EdmComplexType("TestNamespace", "Addr"); + complexType.AddStructuralProperty("CountryRegion", EdmPrimitiveTypeKind.String); + IEdmComplexTypeReference complexTypeReference = new EdmComplexTypeReference(complexType, true); + + EdmEntityType entityType = model.GetEntityType("TestNamespace.Customer"); + entityType.AddStructuralProperty("Address", complexTypeReference); + + string payload = @"{ + ""@odata.context"":""http://odata.org/test/$metadata#Customers(1)/Address"", + ""value"":{""CountryRegion"":""US""} + }"; + + ODataJsonLightPropertyAndValueDeserializer deserializer = new ODataJsonLightPropertyAndValueDeserializer(this.CreateJsonLightInputContext(payload, model)); + + // Currently using de-serializer to read top-level complex property directly is not supported. Need to use ODataReader.Read API instead. + Action action = () => deserializer.ReadTopLevelProperty(complexTypeReference); + action.ShouldThrow().WithMessage( + "An internal error 'ODataJsonLightPropertyAndValueDeserializer_ReadPropertyValue' occurred."); + } #endregion #region Complex properties instance annotation @@ -1078,7 +1131,7 @@ public void ParsingExpectedComplexPropertyActualNotShouldThrow() var complexTypeRef = new EdmComplexTypeReference(complexType, false); var odataReader = this.CreateJsonLightInputContext("\"CountryRegion\":\"China\"", model, false) .CreateResourceReader(null, complexType); - Action action = () => + Action action = () => { while (odataReader.Read()) { diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs index a03a0671c3..c999486576 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs @@ -592,7 +592,7 @@ internal SelectedPropertiesNodeAssertions(SelectedPropertiesNode node) : base(no internal AndConstraint HaveProperties(IEdmEntityType entityType, params string[] propertyNames) { - this.Subject.As().GetSelectedProperties(entityType).Keys.Should().BeEquivalentTo(propertyNames); + this.Subject.As().GetSelectedProperties(entityType).Select(p => p.Name).Should().BeEquivalentTo(propertyNames); return new AndConstraint(this); } diff --git a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs index e96750b44b..585ae784c5 100644 --- a/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs +++ b/test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/TestWriterUtils.cs @@ -175,9 +175,9 @@ internal static void WriteActionAndVerifyODataPayload(Action(Action Date: Sun, 13 May 2018 23:59:56 -0700 Subject: [PATCH 05/16] 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 c860b285e2..d3b5db4d04 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 7c0ca67898..452a705ec4 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 6fe1609fad..2407f768ab 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 5f455f8809..b6c99f1408 100644 --- a/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs +++ b/src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs @@ -1628,14 +1628,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 6eb5d6c209..966aec727b 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -445,10 +445,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) @@ -473,6 +474,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)] From a75d1814779aec34eb1455c34a6d1e85ae6e4a10 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Thu, 31 May 2018 17:13:49 -0700 Subject: [PATCH 06/16] Don't omit null values for expanded navigation properties during serialization, and updates for deserialization side accordingly. --- .../JsonLight/ODataJsonLightReader.cs | 44 ++--------------- .../JsonLight/ODataJsonLightWriter.cs | 20 ++------ .../WriteJsonWithoutModelTests.cs | 26 ++++++---- ...AndValueJsonLightReaderIntegrationTests.cs | 48 +++++++++++++------ 4 files changed, 59 insertions(+), 79 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index d3b5db4d04..07b4d1c5e2 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -2304,11 +2304,11 @@ private void RestoreOmittedNullValues() { Debug.Assert(currentProperty.Type != null, "currentProperty.Type != null"); - // 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) + // Skip declared properties that are not null-able types. + // Skip navigation properties (no navigation link means link value is null) + // Skip properties that have been read (primitive or structural). + if (!currentProperty.Type.IsNullable + || currentProperty.PropertyKind == EdmPropertyKind.Navigation || resource.Properties.Any(p => p.Name.Equals(currentProperty.Name, StringComparison.Ordinal)) || resourceState.NavigationPropertiesRead.Contains(currentProperty.Name)) { @@ -2322,40 +2322,6 @@ private void RestoreOmittedNullValues() } } - 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. /// diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs index 38f5e78528..2d76f3ac28 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs @@ -270,27 +270,15 @@ protected override void StartResource(ODataResource resource) } this.jsonLightResourceSerializer.InstanceAnnotationWriter.WriteInstanceAnnotations(parentNavLink.GetInstanceAnnotations(), 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); } + + // Write the property name of an expanded navigation property to start the value. + this.jsonWriter.WriteName(parentNavLink.Name); } if (resource == null) { - if (!this.ShouldOmitNullValues()) - { - this.jsonWriter.WriteValue((string)null); - } - + this.jsonWriter.WriteValue((string)null); return; } diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs index 288d38bc7f..21b7a6b388 100644 --- a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs @@ -220,20 +220,34 @@ public void Mikep_ReadWriteNullNavigationTest() expectedNulls = Regex.Replace(expectedNulls, @"\s*", string.Empty, RegexOptions.Multiline); Assert.IsTrue(expectedNulls.Contains(serviceUri.ToString())); + // Requirement A: Omit null value for nested resource if omit-values=nulls is specified. // 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"" }"; + */ + + + // Requirement B: NEVER omit null value for nested resource regardless of omit-values=nulls header + // string expectedNoNulls = "{\"@odata.context\":\"" + serviceUri + "$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity\",\"Name\":\"Fred\",\"Title@test.annotation\":\"annotationValue\",\"DynamicAnnotated@test.annotation\":\"annotationValue\",\"Address\":null,\"Manager\":null}"; + string expectedNoNulls = @"{ + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"", + ""DynamicAnnotated@test.annotation"":""annotationValue"", + ""Address"":null,""Manager"":null + }"; 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 }) + foreach (bool omitNulls in new bool[] { /*false,*/ true }) { // validate writing a null navigation property value var stream = new MemoryStream(); @@ -335,16 +349,10 @@ public void Mikep_ReadWriteNullNavigationTest() } } - if (!omitNulls) - { - Assert.IsTrue(managerReturned, "Manager not returned when {0}omitting nulls", omitNulls ? "" : "not "); - } + 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.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"); 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 5aec9c75fc..636e42d545 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 @@ -384,21 +384,31 @@ public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues(bool bNul ""@odata.id"":""http://mytest"", ""Id"":0, ""Weight"":60.5, - ""Address"":{""CountryRegion"":""US"", ""City"":""Redmond""} + ""Address"":{""CountryRegion"":""US"", ""City"":""Redmond""}, + ""Education"":null }"; List entries = new List(); List nestedResourceInfos = new List(); + string reading = "init"; this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { switch (reader.State) { case ODataReaderState.ResourceStart: - entries.Add(reader.Item as ODataResource); + if (reading.Equals("Education", StringComparison.Ordinal)) + { + reader.Item.Should().BeNull(); + } + else + { + entries.Add(reader.Item as ODataResource); + } break; case ODataReaderState.NestedResourceInfoStart: nestedResourceInfos.Add((ODataNestedResourceInfo)reader.Item); + reading = (reader.Item as ODataNestedResourceInfo).Name; break; default: break; @@ -414,8 +424,6 @@ public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues(bool bNul person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)) .Value.Should().Be(60.5); - 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)) .Value.Should().Be("US"); @@ -433,10 +441,10 @@ public void ReadingTypeDefinitionPayloadJsonLightWithOmittedNullValues(bool bNul address.Properties.Any(s => string.Equals(s.Name, "ZipCode", StringComparison.Ordinal)).Should().BeFalse(); } - nestedResourceInfos.Count().Should().Be(2); - nestedResourceInfos[0].Name.Should().Be("Address"); - // Navigation link missing from payload. - nestedResourceInfos[1].Name.Should().Be("Company"); + nestedResourceInfos.Count().Should().Be(3); + nestedResourceInfos.Any(info => info.Name.Equals("Address")).Should().BeTrue(); + nestedResourceInfos.Any(info => info.Name.Equals("Education")).Should().BeTrue(); + nestedResourceInfos.Any(info => info.Name.Equals("Company")).Should().BeTrue(); } [Fact] @@ -950,29 +958,40 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,UnknownPropX,Education,Address)/$entity"", ""@odata.id"":""http://mytest"", ""Id"":0, - ""Education"":{""Id"":1} + ""Education"":{""Id"":1}, + ""Address"":null }"; const string payloadWithWildcardInQueryOption = @"{ ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,UnknownPropX,Education/*,Address)/$entity"", ""@odata.id"":""http://mytest"", ""Id"":0, - ""Education"":{""Id"":1} + ""Education"":{""Id"":1}, + ""Address"":null }"; foreach (string payload in new string[] {payloadWithQueryOption, payloadWithWildcardInQueryOption}) { List entries = new List(); List nestedResourceInfos = new List(); + string reading = "init"; this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { switch (reader.State) { case ODataReaderState.ResourceStart: - entries.Add(reader.Item as ODataResource); + if (reading.Equals("Address", StringComparison.Ordinal)) + { + reader.Item.Should().BeNull(); + } + else + { + entries.Add(reader.Item as ODataResource); + } break; case ODataReaderState.NestedResourceInfoStart: nestedResourceInfos.Add(reader.Item as ODataNestedResourceInfo); + reading = (reader.Item as ODataNestedResourceInfo).Name; break; default: break; @@ -981,9 +1000,11 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu nullValuesOmitted: bNullValuesOmitted); entries.Count.Should().Be(2); - nestedResourceInfos.Count.Should().Be(1); + nestedResourceInfos.Count.Should().Be(2); nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Education", StringComparison.Ordinal)) .Should().NotBeNull(); + nestedResourceInfos.FirstOrDefault(s => string.Equals(s.Name, "Address", StringComparison.Ordinal)) + .Should().NotBeNull(); // Education ODataResource edu = @@ -1017,9 +1038,6 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) .Value.Should().Be(0); - // 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(); person.Properties.Any(s => string.Equals(s.Name, "Weight", StringComparison.Ordinal)).Should().BeFalse(); From 3f9908a6efd3beac42d3361dbec7409273321ee2 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Thu, 19 Jul 2018 11:38:28 -0700 Subject: [PATCH 07/16] Another round of CR comments: Remove IsOpenType check to include undefined properties for null value restoration. Add filter check for dynamic properties restoration. --- .../JsonLight/ODataJsonLightReader.cs | 25 ++----- .../SelectedPropertiesNode.cs | 2 +- ...AndValueJsonLightReaderIntegrationTests.cs | 73 ++++++++++++++++++- 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index 07b4d1c5e2..ad70218da7 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -2286,7 +2286,7 @@ private void RestoreOmittedNullValues() if (resourceState.SelectedProperties == SelectedPropertiesNode.EntireSubtree) { - selectedProperties = GetSelfAndBaseTypeProperties(edmStructuredType); + selectedProperties = edmStructuredType.Properties(); } else { @@ -2294,9 +2294,15 @@ private void RestoreOmittedNullValues() selectedProperties = resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType); } - // All dynamic properties: null value restoration. + // All dynamic properties: null value restoration for properties that are not present. foreach (string name in resourceState.SelectedProperties.GetSelectedDynamicProperties(edmStructuredType)) { + if (resource.Properties.Any(p => p.Name.Equals(name, StringComparison.Ordinal)) + || resourceState.NavigationPropertiesRead.Contains(name)) + { + continue; + } + RestoreNullODataProperty(name, resourceState); } @@ -2355,21 +2361,6 @@ private static void RestoreNullODataProperty(string nullPropertyName, IODataJson } } - /// - /// Get all properties defined by the EDM structural type and its base types. - /// - /// The EDM structural type. - /// All the properties of this type. - private static IEnumerable GetSelfAndBaseTypeProperties(IEdmStructuredType edmStructuredType) - { - if (edmStructuredType == null) - { - return Enumerable.Empty(); - } - - return edmStructuredType.DeclaredProperties.Concat(GetSelfAndBaseTypeProperties(edmStructuredType.BaseType)); - } - /// /// Add info resolved from context url to current scope. /// diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index 966aec727b..3abcd6b964 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -483,7 +483,7 @@ internal IEnumerable GetSelectedDynamicProperties(IEdmStructuredType str { IEnumerable dynamicProperties = Enumerable.Empty(); - if (structuredType.IsOpen && this.selectedProperties != null && this.selectedProperties.Count > 0) + if ( this.selectedProperties != null && this.selectedProperties.Count > 0) { dynamicProperties = this.selectedProperties.Select( prop => structuredType.FindProperty(prop) == null 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 636e42d545..4d4f384ddb 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 @@ -1032,8 +1032,16 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Person", StringComparison.Ordinal)); person.Should().NotBeNull(); - // Verify that unknown property doesn't cause anomaly and is not restored. - person.Properties.Any(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Should().BeFalse(); + // Verify that unknown property doesn't cause anomaly. And it is restored only when omit-values=nulls is specified. + if (bNullValuesOmitted) + { + person.Properties.Single(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Value + .Should().BeNull(); + } + else + { + person.Properties.Any(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Should().BeFalse(); + } person.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) .Value.Should().Be(0); @@ -1092,7 +1100,18 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu ODataResource edu = entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); edu.Should().NotBeNull(); - edu.Properties.Any(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Should().BeFalse(); + + // Verify that unknown property doesn't cause anomaly. And it is restored only when omit-values=nulls is specified. + if (bNullValuesOmitted) + { + edu.Properties.Single(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Value + .Should().BeNull(); + } + else + { + edu.Properties.Any(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)) + .Should().BeFalse(); + } edu.Properties.FirstOrDefault(s => string.Equals(s.Name, "Id", StringComparison.Ordinal)) .Value.Should().Be(1); @@ -1111,6 +1130,54 @@ public void ReadingPropertyInTopLevelOrInComplexTypeShouldRestoreOmittedNullValu edu.Properties.Any(s => string.Equals(s.Name, "Campuses", StringComparison.Ordinal)).Should().BeFalse(); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ReadingUnknownPropertyOfNonOpenTypeShouldWork(bool bNullValuesOmitted) + { + EdmEntityType entityType; + EdmEntitySet entitySet; + EdmModel model = BuildEdmModelForOmittedNullValuesTestCases(out entityType, out entitySet); + + // null-able property Height is not selected, thus should not be restored. + // null-able property Address is selected, thus should be restored. + // Property Education is null-able. + const string payloadWithSelectedPropertiesPartialSubTreeInQueryOption = @"{ + ""@odata.context"":""http://www.example.com/$metadata#EntityNs.MyContainer.People(Id,%20Education/Id,%20Education/SchoolName,%20Education/UnknownPropX,%20Address)/$entity"", + ""@odata.id"":""http://mytest"", + ""Id"":0, + ""Education"":{""Id"":1, ""UnknownPropX"": ""pX""} + }"; + + List entries = new List(); + List nestedResourceInfos = new List(); + this.ReadEntryPayload(model, payloadWithSelectedPropertiesPartialSubTreeInQueryOption, 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); + + // Education + ODataResource edu = + entries.FirstOrDefault(s => string.Equals(s.TypeName, "NS.Edu", StringComparison.Ordinal)); + edu.Should().NotBeNull(); + + // Verify that unknown property on non-open type doesn't cause anomaly. + edu.Properties.Single(s => string.Equals(s.Name, "UnknownPropX", StringComparison.Ordinal)).Value + .Should().Equals("pX"); + } + [Fact] public void ReadingNormalDynamicCollectionPropertyInOpenStructuralTypeShouldWork() { From f8e687a132ecd9897d3b830a08a9805c5dc55263 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Wed, 29 Aug 2018 16:57:49 -0700 Subject: [PATCH 08/16] Addressing further CR comments. --- .../JsonLight/ODataJsonLightReader.cs | 14 ++------------ src/Microsoft.OData.Core/SelectedPropertiesNode.cs | 10 +++++----- .../SelectedPropertiesNodeTests.cs | 8 ++++---- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index ad70218da7..dff1ef716f 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -2282,17 +2282,7 @@ private void RestoreOmittedNullValues() { ODataResourceBase resource = resourceState.Resource; - IEnumerable selectedProperties; - - if (resourceState.SelectedProperties == SelectedPropertiesNode.EntireSubtree) - { - selectedProperties = edmStructuredType.Properties(); - } - else - { - // Partial subtree. - selectedProperties = resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType); - } + IEnumerable selectedProperties = resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType); // All dynamic properties: null value restoration for properties that are not present. foreach (string name in resourceState.SelectedProperties.GetSelectedDynamicProperties(edmStructuredType)) @@ -2311,7 +2301,7 @@ private void RestoreOmittedNullValues() Debug.Assert(currentProperty.Type != null, "currentProperty.Type != null"); // Skip declared properties that are not null-able types. - // Skip navigation properties (no navigation link means link value is null) + // Skip navigation properties (no navigation links need to be restored as null.) // Skip properties that have been read (primitive or structural). if (!currentProperty.Type.IsNullable || currentProperty.PropertyKind == EdmPropertyKind.Navigation diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index 3abcd6b964..22f4dbcab7 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -465,25 +465,25 @@ internal IEnumerable GetSelectedProperties(IEdmStructuredType stru if (this.selectionType == SelectionType.EntireSubtree || this.hasWildcard) { - return structuredType.DeclaredProperties; + return structuredType.Properties(); } Debug.Assert(this.selectedProperties != null, "selectedProperties != null"); - // Get declared properties selected, and filter out unrecognized properties. + // Get properties selected, and filter out unrecognized properties. return this.selectedProperties.Select(structuredType.FindProperty).OfType(); } /// - /// Return names of dynamic properties if current structured node is open type. + /// Return names of dynamic properties. /// /// The current structured type. - /// The names of dynamic properties, empty if the current structured type is not open type. + /// The names of dynamic properties. internal IEnumerable GetSelectedDynamicProperties(IEdmStructuredType structuredType) { IEnumerable dynamicProperties = Enumerable.Empty(); - if ( this.selectedProperties != null && this.selectedProperties.Count > 0) + if (this.selectedProperties != null && this.selectedProperties.Count > 0) { dynamicProperties = this.selectedProperties.Select( prop => structuredType.FindProperty(prop) == null diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs index c999486576..fc903e1264 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/SelectedPropertiesNodeTests.cs @@ -117,7 +117,7 @@ public void WildcardShouldSelectAllPropertiesParsingTest() public void WildcardShouldSelectAllProperties() { SelectedPropertiesNode.Create("*").Should() - .HaveProperties(this.cityType, "Photo", "Districts") + .HaveProperties(this.cityType, "Id", "Name", "Size", "Photo", "Districts") .And.HaveNavigations(this.cityType, "Districts") .And.HaveChild(this.cityType, "Districts", c => c.Should().BeSameAsEmpty()); } @@ -153,7 +153,7 @@ public void SpecifyingAWildCardShouldNotCauseDuplicates() SelectedPropertiesNode.Create("Districts,*,Photo").Should() .HaveStreams(this.cityType, "Photo") - .And.HaveProperties(this.cityType, "Photo", "Districts") + .And.HaveProperties(this.cityType, "Id", "Name", "Size", "Photo", "Districts") .And.HaveNavigations(this.cityType, "Districts") .And.HaveChild(this.cityType, "Districts", c => c.Should().HaveEntireSubtree()); } @@ -181,7 +181,7 @@ public void DeepAndWideSelection1() // 2) 'Districts/*' should not override 'Districts' SelectedPropertiesNode.Create("*,Districts,Districts/*").Should() .HaveStreams(this.cityType, "Photo") - .And.HaveProperties(this.cityType, "Photo", "Districts") + .And.HaveProperties(this.cityType, "Id", "Name", "Size", "Photo", "Districts") .And.HaveNavigations(this.cityType, "Districts") .And.HaveChild(this.cityType, "Districts", c => c.Should().HaveEntireSubtree()); } @@ -352,7 +352,7 @@ public void CombiningPropertyWithWildcardShouldBeWildcard() SelectedPropertiesNode right = SelectedPropertiesNode.Create("Fake"); this.VerifyCombination(left, right, n => n.Should().HaveStreams(this.cityType, "Photo") - .And.HaveProperties(this.cityType, "Districts", "Photo") + .And.HaveProperties(this.cityType, "Id", "Name", "Size", "Photo", "Districts") .And.HaveNavigations(this.cityType, "Districts")); } From df084b6eb92bcc26940b549e7863d95b36fe9ecf Mon Sep 17 00:00:00 2001 From: Biao Li Date: Tue, 23 Oct 2018 10:31:50 -0700 Subject: [PATCH 09/16] For expanded entity: write omit-nulls; read restore nulls. --- ...DataConventionalResourceMetadataBuilder.cs | 23 + .../ODataResourceMetadataBuilder.cs | 17 + .../JsonLight/ODataJsonLightReader.cs | 15 + .../JsonLight/ODataJsonLightWriter.cs | 20 +- src/Microsoft.OData.Core/ODataResource.cs | 5 + .../SelectedPropertiesNode.cs | 16 + .../Microsoft.Test.OData.Tests.Client.csproj | 1 + .../ReadOmitNullsResponseTests.cs | 521 ++++++++++++++++++ .../WriteJsonWithoutModelTests.cs | 199 ------- 9 files changed, 614 insertions(+), 203 deletions(-) create mode 100644 test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs diff --git a/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs b/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs index 92af0e3581..6f01a58f08 100644 --- a/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs +++ b/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs @@ -33,6 +33,8 @@ internal class ODataConventionalResourceMetadataBuilder : ODataResourceMetadataB /// The list of nested info that have been processed. Here navigation property and complex will both be marked for convenience. protected readonly HashSet ProcessedNestedResourceInfos; + protected readonly HashSet ProcessedExpandedEntityNames; + /// The enumerator for unprocessed navigation links. private IEnumerator unprocessedNavigationLinks; @@ -69,6 +71,7 @@ internal ODataConventionalResourceMetadataBuilder(IODataResourceMetadataContext this.UriBuilder = uriBuilder; this.MetadataContext = metadataContext; this.ProcessedNestedResourceInfos = new HashSet(StringComparer.Ordinal); + this.ProcessedExpandedEntityNames = new HashSet(StringComparer.Ordinal); this.resource = resourceMetadataContext.Resource; } @@ -365,6 +368,26 @@ internal override void MarkNestedResourceInfoProcessed(string navigationProperty this.ProcessedNestedResourceInfos.Add(navigationPropertyName); } + /// + /// Marks the given expanded entity as processed. + /// + /// The name of the expanded entity we've already processed. + internal override void MarkExpandedEntityProcessed(string expandedEntityName) + { + Debug.Assert(!string.IsNullOrEmpty(expandedEntityName), "!string.IsNullOrEmpty(expandedEntityName)"); + Debug.Assert(this.ProcessedExpandedEntityNames != null, "this.ProcessedExpandedEntityNames != null"); + this.ProcessedExpandedEntityNames.Add(expandedEntityName); + } + + /// + /// Return whether the given expanded entity of specified name has been processed. + /// + /// The name of the expanded entity. + internal override bool IsExpandedEntityProcessed(string name) + { + return this.ProcessedExpandedEntityNames.Contains(name); + } + /// /// Returns the next unprocessed navigation link or null if there's no more navigation links to process. /// diff --git a/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs b/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs index a8bfbfb9e8..a45fcf9032 100644 --- a/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs +++ b/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs @@ -156,6 +156,23 @@ internal virtual void MarkNestedResourceInfoProcessed(string navigationPropertyN { } + /// + /// Return whether the given expanded entity of specified name has been processed. + /// + /// The name of the expanded entity. + internal virtual bool IsExpandedEntityProcessed(string name) + { + return false; + } + + /// + /// Marks the given expanded entity as processed. + /// + /// The name of the expanded entity we've already processed. + internal virtual void MarkExpandedEntityProcessed(string expandedEntityName) + { + } + /// /// Returns the next unprocessed nested resource info or null if there's no more navigation links to process. /// diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index dff1ef716f..459a3b6a8b 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -1658,6 +1658,12 @@ private void ReadExpandedNestedResourceInfoEnd(bool isCollection) IODataJsonLightReaderResourceState parentResourceState = (IODataJsonLightReaderResourceState)this.ParentScope; parentResourceState.NavigationPropertiesRead.Add(this.CurrentNestedResourceInfo.Name); + if (parentResourceState.SelectedProperties.NamesOfSelectedExpandedEntities.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); } @@ -2313,6 +2319,15 @@ private void RestoreOmittedNullValues() RestoreNullODataProperty(currentProperty.Name, resourceState); } + + // Restore null Expanded entities, which are specified as "isExpandedNavigationProperty" the child nodes. + foreach (string name in resourceState.SelectedProperties.NamesOfSelectedExpandedEntities) + { + if (!resource.IsExpandedEntityProcessed(name)) + { + RestoreNullODataProperty(name, resourceState); + } + } } } } diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs index 2d76f3ac28..a8653efc90 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 of null 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/ODataResource.cs b/src/Microsoft.OData.Core/ODataResource.cs index caf60ae470..01c4da80c1 100644 --- a/src/Microsoft.OData.Core/ODataResource.cs +++ b/src/Microsoft.OData.Core/ODataResource.cs @@ -186,6 +186,11 @@ public IEnumerable Properties set { this.properties = value; } } + internal bool IsExpandedEntityProcessed(string name) + { + return this.MetadataBuilder.IsExpandedEntityProcessed(name); + } + /// Gets or sets the type name of the resource. /// The type name of the resource. public string TypeName diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index 22f4dbcab7..b592cc769d 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -4,6 +4,9 @@ // //--------------------------------------------------------------------- +using System.Collections; +using System.Runtime.InteropServices.ComTypes; + namespace Microsoft.OData { #region Namespaces @@ -207,6 +210,19 @@ internal enum SelectionType PartialSubtree = 2, } + /// + /// Gets names of the expanded entity nodes at current level. + /// + internal IEnumerable NamesOfSelectedExpandedEntities + { + get + { + return this.children == null + ? Enumerable.Empty() + : this.children.Where(_ => _.Value.isExpandedNavigationProperty).Select(c => c.Key); + } + } + /// /// Creates a node from the given raw $select query option value, structural type information and service model. /// diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/Microsoft.Test.OData.Tests.Client.csproj b/test/EndToEndTests/Tests/Client/Build.Desktop/Microsoft.Test.OData.Tests.Client.csproj index e578da7c44..c3607a897f 100644 --- a/test/EndToEndTests/Tests/Client/Build.Desktop/Microsoft.Test.OData.Tests.Client.csproj +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/Microsoft.Test.OData.Tests.Client.csproj @@ -205,6 +205,7 @@ + diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs new file mode 100644 index 0000000000..a02815af99 --- /dev/null +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs @@ -0,0 +1,521 @@ +//--------------------------------------------------------------------- +// +// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information. +// +//--------------------------------------------------------------------- + +namespace Microsoft.Test.OData.Tests.Client.WriteJsonPayloadTests +{ + using System; + using System.IO; + using System.Linq; + using System.Text.RegularExpressions; + using Microsoft.OData; + 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; + + /// + /// Generate response by writing various payload with/without omit-value=nulls preference, + /// and read the response on client side. + /// Based on sample test provided by Mike Pizzo as omit-values=nulls requirement test cases + /// related to dynamic properties and annotation-only properties. + /// + [TestClass] + public class ReadOmitNullsResponseTest : EndToEndTestBase + { + public ReadOmitNullsResponseTest() + : base(ServiceDescriptors.AstoriaDefaultService) + { + } + + public override void CustomTestInitialize() + { + WritePayloadHelper.CustomTestInitialize(this.ServiceUri); + } + + + + [TestMethod] + public void ReadWriteNullExpandedNavigationTest_omitNulls_projectedEntity() + { + // Expanded entity with projection. + ReadWriteNullExpandedNavigationTest_omitNulls( /*nestedSelect*/true); + } + + [TestMethod] + public void ReadWriteNullExpandedNavigationTest_omitNulls() + { + // Expanded entity without projection. + ReadWriteNullExpandedNavigationTest_omitNulls( /*nestedSelect*/false); + } + + [TestMethod] + public void ReadWriteNullExpandedNavigationTest_notOmitNulls() + { + // set up model + EdmModel model = null; + EdmEntityType employeeType = null; + EdmEntitySet employeeSet = null; + + SetupModel(out model, out employeeType, out employeeSet); + + Uri serviceUri = new Uri("http://test/"); + + string expectedNulls = @"{ + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend())/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"",""Title"":null, + ""Dynamic"":null, + ""DynamicAnnotated@test.annotation"":""annotationValue"",""DynamicAnnotated"":null, + ""Address"":null,""Manager"":null, + ""Friend"": {""Name"":""FriendOfFred""} + }"; + expectedNulls = Regex.Replace(expectedNulls, @"\s*", string.Empty, RegexOptions.Multiline); + Assert.IsTrue(expectedNulls.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,Friend", UriKind.Relative)).ParseUri(); + + // validate writing a null navigation property value + var stream = new MemoryStream(); + var responseMessage = new StreamResponseMessage(stream); + responseMessage.SetHeader("Preference-Applied", "odata.include-annotations=\"*\""); + var messageWriter = new ODataMessageWriter(responseMessage, writeSettings, model); + var writer = messageWriter.CreateODataResourceWriter(employeeSet, employeeType); + + WriteResponse(writer); + + stream.Flush(); + + // compare written stream to expected stream + stream.Seek(0, SeekOrigin.Begin); + var streamReader = new StreamReader(stream); + Assert.AreEqual(expectedNulls, streamReader.ReadToEnd(), "Did not generate expected string when not omitting nulls"); + + // 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 = null; + bool managerReturned = false; + bool addressReturned = false; + int employeeEntityCount = 0; + ODataResource employeeResource = null; + ODataResource managerResource = null; + ODataResource friendResource = null; + ODataResource addressResource = null; + while (reader.Read()) + { + switch (reader.State) + { + case ODataReaderState.ResourceEnd: + switch (reading) + { + case "Manager": + // Primitive null value for nested resource "Manager" is reported here. + managerResource = reader.Item as ODataResource; + managerReturned = true; + break; + case "Address": + // Primitive null value for nested complex value "Address" is reported here. + addressResource = reader.Item as ODataResource; + addressReturned = true; + break; + case "Employee": + ++employeeEntityCount; + + // Nested resource "Friend" reading is completed before the reading of top level resource "Employee". + if (employeeEntityCount == 1) + { + friendResource = reader.Item as ODataResource; + } + else if (employeeEntityCount == 2) + { + employeeResource = reader.Item as ODataResource; + } + else + { + Assert.Fail($"Employee resource entry count is {employeeEntityCount}, should not be more than 2."); + } + break; + } + break; + case ODataReaderState.NestedResourceInfoStart: + reading = ((ODataNestedResourceInfo)reader.Item).Name; + break; + case ODataReaderState.NestedResourceInfoEnd: + reading = "Employee"; + break; + } + } + + Assert.IsTrue(employeeEntityCount == 2, $"employeeEntityCount: Expected: 2, Actual: {employeeEntityCount}"); + + Assert.IsTrue(addressReturned, "Address is not returned but it should be."); + Assert.IsNull(addressResource, "Address resource is not null but it should be."); + + Assert.IsTrue(managerReturned, "Manager is not returned but it should be, because payload has its value of null."); + Assert.IsNull(managerResource, "Manager resource is not null but it should be, because payload has its value of null."); + + Assert.IsNotNull(friendResource, "Friend resource should not be null."); + + VerifyAdditionalProperties(employeeResource); + } + + [TestMethod] + public void ReadWriteNullNavigationTest_omitNulls() + { + // set up model + EdmModel model = null; + EdmEntityType employeeType = null; + EdmEntitySet employeeSet = null; + + SetupModel(out model, out employeeType, out employeeSet); + + Uri serviceUri = new Uri("http://test/"); + + // Requirement: Omit null value for nested resource if omit-values=nulls is specified. + string expectedNoNulls = @"{ + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager,Friend())/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"", + ""DynamicAnnotated@test.annotation"":""annotationValue"", + ""Friend"": {""Name"":""FriendOfFred""} + }"; + + 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,Manager&$expand=Friend", UriKind.Relative)).ParseUri(); + + // validate writing a null navigation property value + var stream = new MemoryStream(); + var responseMessage = new StreamResponseMessage(stream); + responseMessage.SetHeader("Preference-Applied", "omit-values=nulls,odata.include-annotations=\"*\""); + + var messageWriter = new ODataMessageWriter(responseMessage, writeSettings, model); + var writer = messageWriter.CreateODataResourceWriter(employeeSet, employeeType); + + WriteResponse(writer); + + stream.Flush(); + + // compare written stream to expected stream + stream.Seek(0, SeekOrigin.Begin); + var streamReader = new StreamReader(stream); + Assert.AreEqual(expectedNoNulls, streamReader.ReadToEnd(), "Did not generate expected string when omitting nulls"); + + // 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 = null; + bool addressReturned = false; + bool managerReturned = false; + int employeeEntityCount = 0; + ODataResource employeeResource = null; + ODataResource friendResource = null; + while (reader.Read()) + { + switch (reader.State) + { + case ODataReaderState.ResourceEnd: + switch (reading) + { + case "Address": + addressReturned = true; + break; + case "Manager": + managerReturned = true; + break; + case "Employee": + ++employeeEntityCount; + + // Nested resource "Friend" is completed first, and the top level resource "Employee" is completed later. + if (employeeEntityCount == 1) + { + friendResource = reader.Item as ODataResource; + } + else if (employeeEntityCount == 2) + { + employeeResource = reader.Item as ODataResource; + } + else + { + Assert.Fail($"Employee resource entry count is {employeeEntityCount}, should not be more than 2."); + } + break; + } + break; + case ODataReaderState.NestedResourceInfoStart: + reading = ((ODataNestedResourceInfo)reader.Item).Name; + break; + case ODataReaderState.NestedResourceInfoEnd: + reading = "Employee"; + break; + } + } + + Assert.IsTrue(employeeEntityCount == 2, $"employeeEntityCount: Expected: 2, Actual: {employeeEntityCount}"); + Assert.IsFalse(addressReturned, "Address is returned but it should not be."); + Assert.IsFalse(managerReturned, "Manager is returned but it should not be, because it is navigation link and its value is omitted in payload."); + Assert.IsNotNull(friendResource, "Friend resource is null but it should not be."); + VerifyAdditionalProperties(employeeResource); + Assert.IsTrue(employeeResource.Properties != null); + Assert.AreEqual(employeeResource.Properties.Count(), 5); + Assert.IsTrue(employeeResource.Properties.Any(p => p.Name.Equals("Address") && (p.Value == null))); + Assert.IsFalse(employeeResource.Properties.Any(p => p.Name.Equals("Manager"))); + } + + private void ReadWriteNullExpandedNavigationTest_omitNulls(bool nestedSelect) + { + // set up model + EdmModel model = null; + EdmEntityType employeeType = null; + EdmEntitySet employeeSet = null; + + SetupModel(out model, out employeeType, out employeeSet); + + Uri serviceUri = new Uri("http://test/"); + + // Requirement: Omit null value for nested resource if omit-values=nulls is specified. + string expectedNoNulls = nestedSelect + ? @"{ + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend(Name,Title))/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"", + ""DynamicAnnotated@test.annotation"":""annotationValue"", + ""Friend"": {""Name"":""FriendOfFred""} + }" + : @"{ + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend())/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"", + ""DynamicAnnotated@test.annotation"":""annotationValue"", + ""Friend"": {""Name"":""FriendOfFred""} + }"; + + expectedNoNulls = Regex.Replace(expectedNoNulls, @"\s*", string.Empty, RegexOptions.Multiline); + Assert.IsTrue(expectedNoNulls.Contains(serviceUri.ToString())); + + var writeSettings = new ODataMessageWriterSettings() { BaseUri = serviceUri }; + + Uri uri = nestedSelect + ? new Uri("employees?$select=Name,Title,Dynamic,Address&$expand=Manager,Friend($select=Name,Title)", UriKind.Relative) + : new Uri("employees?$select=Name,Title,Dynamic,Address&$expand=Manager,Friend", UriKind.Relative); + writeSettings.ODataUri = new ODataUriParser(model, serviceUri, uri).ParseUri(); + + // validate writing a null navigation property value + var stream = new MemoryStream(); + var responseMessage = new StreamResponseMessage(stream); + responseMessage.SetHeader("Preference-Applied", "omit-values=nulls,odata.include-annotations=\"*\""); + + var messageWriter = new ODataMessageWriter(responseMessage, writeSettings, model); + var writer = messageWriter.CreateODataResourceWriter(employeeSet, employeeType); + + WriteResponse(writer); + + stream.Flush(); + + // compare written stream to expected stream + stream.Seek(0, SeekOrigin.Begin); + var streamReader = new StreamReader(stream); + Assert.AreEqual(expectedNoNulls, streamReader.ReadToEnd(), "Did not generate expected string when omitting nulls"); + + // 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 = null; + bool addressReturned = false; + bool managerReturned = false; + int employeeEntityCount = 0; + ODataResource employeeResource = null; + ODataResource friendResource = null; + while (reader.Read()) + { + switch (reader.State) + { + case ODataReaderState.ResourceEnd: + switch (reading) + { + case "Address": + addressReturned = true; + break; + case "Manager": + managerReturned = true; + break; + case "Employee": + case "Friend": + ++employeeEntityCount; + + // Nested resource "Friend" is completed first, and the top level resource "Employee" is completed later. + if (employeeEntityCount == 1) + { + friendResource = reader.Item as ODataResource; + } + else if (employeeEntityCount == 2) + { + employeeResource = reader.Item as ODataResource; + } + else + { + Assert.Fail($"Employee resource entry count is {employeeEntityCount}, should not be more than 2."); + } + break; + } + break; + case ODataReaderState.NestedResourceInfoStart: + reading = ((ODataNestedResourceInfo)reader.Item).Name; + break; + case ODataReaderState.NestedResourceInfoEnd: + reading = "Employee"; + break; + } + } + + Assert.IsTrue(employeeEntityCount == 2, $"employeeEntityCount: Expected: 2, Actual: {employeeEntityCount}"); + Assert.IsFalse(addressReturned, "Address is returned but it should not be."); + Assert.IsFalse(managerReturned, "Manager is returned but it should not be, because value is omitted in payload."); + + Assert.IsNotNull(friendResource, "Friend resource is null but it should not be."); + Assert.IsTrue(friendResource.Properties.Count() == (nestedSelect ? 2 : 3)); + Assert.IsTrue(friendResource.Properties.Any(p => (p.Name.Equals("Name", StringComparison.Ordinal) && p.Value.Equals("FriendOfFred")))); + Assert.IsTrue(friendResource.Properties.Any(p => (p.Name.Equals("Title", StringComparison.Ordinal) && p.Value == null))); + Assert.IsTrue(nestedSelect + ? !friendResource.Properties.Any(p => p.Name.Equals("Address", StringComparison.Ordinal)) + : friendResource.Properties.Any(p => (p.Name.Equals("Address", StringComparison.Ordinal) && p.Value == null))); + + + Assert.IsNotNull(employeeResource); + VerifyAdditionalProperties(employeeResource); + Assert.IsTrue(employeeResource.Properties != null && employeeResource.Properties.Any(p => p.Name.Equals("Manager") && (p.Value == null)), + "Expanded null entity should be restored if omit-values=nulls."); + } + + private void SetupModel(out EdmModel model, out EdmEntityType employeeType, out EdmEntitySet employeeSet) + { + // set up model + model = new EdmModel(); + var addressType = model.AddComplexType("test", "Address"); + addressType.AddStructuralProperty("city", EdmPrimitiveTypeKind.String, true); + 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 + } + ); + + employeeType.AddUnidirectionalNavigation( + new EdmNavigationPropertyInfo + { + Name = "Friend", + TargetMultiplicity = EdmMultiplicity.ZeroOrOne, + Target = employeeType + } + ); + + var container = model.AddEntityContainer("test", "service"); + + employeeSet = container.AddEntitySet("employees", employeeType); + } + + private void WriteResponse(ODataWriter writer) + { + 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(); + writer.WriteEnd(); // manager nested info + + // write friend + writer.WriteStart(new ODataNestedResourceInfo + { + Name = "Friend", + IsCollection = false + }); + writer.WriteStart(new ODataResource + { + Properties = new ODataProperty[] + { + new ODataProperty { Name = "Name", Value = "FriendOfFred" } + } + }); + writer.WriteEnd(); + writer.WriteEnd(); // friend nested info + + writer.WriteEnd(); // resource + } + + private void VerifyAdditionalProperties(ODataResource employeeResource) + { + Assert.IsNotNull(employeeResource, "Employee should not be null"); + Assert.IsNotNull(employeeResource.Properties.FirstOrDefault(p => p.Name.Equals("Name", StringComparison.Ordinal))); + var titleProperty = employeeResource.Properties.FirstOrDefault(p => p.Name.Equals("Title", StringComparison.Ordinal)); + 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.Equals("test.annotation", StringComparison.Ordinal)); + 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.Equals("Dynamic", StringComparison.Ordinal)); + 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.Equals("DynamicAnnotated", StringComparison.Ordinal)); + 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.Equals("test.annotation", StringComparison.Ordinal)); + Assert.IsNotNull(dynamicAnnotation, "DynamicAnnotated property missing the test.annotation"); + Assert.AreEqual(((ODataPrimitiveValue)dynamicAnnotation.Value).Value.ToString(), "annotationValue"); + } + } +} + + diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs index 21b7a6b388..2eccad2d04 100644 --- a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/WriteJsonWithoutModelTests.cs @@ -4,8 +4,6 @@ // //--------------------------------------------------------------------- -using System.Runtime.CompilerServices; - namespace Microsoft.Test.OData.Tests.Client.WriteJsonPayloadTests { using System; @@ -17,8 +15,6 @@ 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. @@ -179,201 +175,6 @@ 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())); - - // Requirement A: Omit null value for nested resource if omit-values=nulls is specified. - // 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"" - }"; - */ - - - // Requirement B: NEVER omit null value for nested resource regardless of omit-values=nulls header - // string expectedNoNulls = "{\"@odata.context\":\"" + serviceUri + "$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity\",\"Name\":\"Fred\",\"Title@test.annotation\":\"annotationValue\",\"DynamicAnnotated@test.annotation\":\"annotationValue\",\"Address\":null,\"Manager\":null}"; - string expectedNoNulls = @"{ - ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager)/$entity"", - ""Name"":""Fred"", - ""Title@test.annotation"":""annotationValue"", - ""DynamicAnnotated@test.annotation"":""annotationValue"", - ""Address"":null,""Manager"":null - }"; - 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; - } - } - - 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 "); - - 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 /// From 7f83fe568759b52d26dad4486213042eda5e5c81 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Tue, 6 Nov 2018 10:00:49 -0800 Subject: [PATCH 10/16] Lastest round of CR comments from Mike. --- ...DataConventionalResourceMetadataBuilder.cs | 23 -------- .../ODataResourceMetadataBuilder.cs | 11 +++- .../JsonLight/ODataJsonLightReader.cs | 56 ++++++++++++------- .../JsonLight/ODataJsonLightWriter.cs | 4 ++ src/Microsoft.OData.Core/ODataResource.cs | 5 -- .../ReadOnlyEnumerableExtensions.cs | 15 +++++ .../ReadOnlyEnumerableOfT.cs | 17 ++++++ .../SelectedPropertiesNode.cs | 4 +- 8 files changed, 83 insertions(+), 52 deletions(-) diff --git a/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs b/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs index 6f01a58f08..92af0e3581 100644 --- a/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs +++ b/src/Microsoft.OData.Core/Evaluation/ODataConventionalResourceMetadataBuilder.cs @@ -33,8 +33,6 @@ internal class ODataConventionalResourceMetadataBuilder : ODataResourceMetadataB /// The list of nested info that have been processed. Here navigation property and complex will both be marked for convenience. protected readonly HashSet ProcessedNestedResourceInfos; - protected readonly HashSet ProcessedExpandedEntityNames; - /// The enumerator for unprocessed navigation links. private IEnumerator unprocessedNavigationLinks; @@ -71,7 +69,6 @@ internal ODataConventionalResourceMetadataBuilder(IODataResourceMetadataContext this.UriBuilder = uriBuilder; this.MetadataContext = metadataContext; this.ProcessedNestedResourceInfos = new HashSet(StringComparer.Ordinal); - this.ProcessedExpandedEntityNames = new HashSet(StringComparer.Ordinal); this.resource = resourceMetadataContext.Resource; } @@ -368,26 +365,6 @@ internal override void MarkNestedResourceInfoProcessed(string navigationProperty this.ProcessedNestedResourceInfos.Add(navigationPropertyName); } - /// - /// Marks the given expanded entity as processed. - /// - /// The name of the expanded entity we've already processed. - internal override void MarkExpandedEntityProcessed(string expandedEntityName) - { - Debug.Assert(!string.IsNullOrEmpty(expandedEntityName), "!string.IsNullOrEmpty(expandedEntityName)"); - Debug.Assert(this.ProcessedExpandedEntityNames != null, "this.ProcessedExpandedEntityNames != null"); - this.ProcessedExpandedEntityNames.Add(expandedEntityName); - } - - /// - /// Return whether the given expanded entity of specified name has been processed. - /// - /// The name of the expanded entity. - internal override bool IsExpandedEntityProcessed(string name) - { - return this.ProcessedExpandedEntityNames.Contains(name); - } - /// /// Returns the next unprocessed navigation link or null if there's no more navigation links to process. /// diff --git a/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs b/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs index a45fcf9032..5aae167de3 100644 --- a/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs +++ b/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs @@ -4,6 +4,8 @@ // //--------------------------------------------------------------------- +using System.Diagnostics; + #if ODATA_CLIENT namespace Microsoft.OData.Client #else @@ -23,6 +25,9 @@ namespace Microsoft.OData.Evaluation /// internal abstract class ODataResourceMetadataBuilder { + /// The expanded entities that have been processed. + protected readonly HashSet ProcessedExpandedEntityNames = new HashSet(StringComparer.Ordinal); + #if !ODATA_CLIENT /// /// Gets an instance of the metadata builder which never returns anything other than nulls. @@ -36,6 +41,7 @@ internal static ODataResourceMetadataBuilder Null } #endif + /// /// Gets instance of the metadata builder which belongs to the parent odata resource /// @@ -162,7 +168,7 @@ internal virtual void MarkNestedResourceInfoProcessed(string navigationPropertyN /// The name of the expanded entity. internal virtual bool IsExpandedEntityProcessed(string name) { - return false; + return this.ProcessedExpandedEntityNames.Contains(name); } /// @@ -171,6 +177,9 @@ internal virtual bool IsExpandedEntityProcessed(string name) /// The name of the expanded entity we've already processed. 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); } /// diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index 459a3b6a8b..83783c35b8 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -1658,7 +1658,7 @@ private void ReadExpandedNestedResourceInfoEnd(bool isCollection) IODataJsonLightReaderResourceState parentResourceState = (IODataJsonLightReaderResourceState)this.ParentScope; parentResourceState.NavigationPropertiesRead.Add(this.CurrentNestedResourceInfo.Name); - if (parentResourceState.SelectedProperties.NamesOfSelectedExpandedEntities.Contains(this.CurrentNestedResourceInfo.Name)) + if (parentResourceState.SelectedProperties.SelectedExpandedEntities.Contains(this.CurrentNestedResourceInfo.Name)) { // Record that we read the expanded entity parentResourceState.MetadataBuilder.MarkExpandedEntityProcessed(this.CurrentNestedResourceInfo.Name); @@ -2284,9 +2284,11 @@ private void RestoreOmittedNullValues() } else { - if (resourceState.Resource != null && edmStructuredType != null) + if (resourceState.Resource != null) { + Debug.Assert(edmStructuredType != null, "edmStructuredType != null"); ODataResourceBase resource = resourceState.Resource; + List nullPropertyNames = new List(); IEnumerable selectedProperties = resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType); @@ -2299,7 +2301,7 @@ private void RestoreOmittedNullValues() continue; } - RestoreNullODataProperty(name, resourceState); + AddNullPropertyName(name, nullPropertyNames); } foreach (IEdmProperty currentProperty in selectedProperties) @@ -2317,52 +2319,64 @@ private void RestoreOmittedNullValues() continue; } - RestoreNullODataProperty(currentProperty.Name, resourceState); + AddNullPropertyName(currentProperty.Name, nullPropertyNames); } // Restore null Expanded entities, which are specified as "isExpandedNavigationProperty" the child nodes. - foreach (string name in resourceState.SelectedProperties.NamesOfSelectedExpandedEntities) + foreach (string name in resourceState.SelectedProperties.SelectedExpandedEntities) { - if (!resource.IsExpandedEntityProcessed(name)) + if (!resource.MetadataBuilder.IsExpandedEntityProcessed(name)) { - RestoreNullODataProperty(name, resourceState); + AddNullPropertyName(name, nullPropertyNames); } } + + RestoreNullODataProperties(nullPropertyNames); } } } } /// - /// Restore the null property value in the resource. + /// Adds the item to the list of names for null properties. + /// Also updates the current resource state for processed items. /// - /// Name of the property to restore the null value. - /// The resource state. - private static void RestoreNullODataProperty(string nullPropertyName, IODataJsonLightReaderResourceState resourceState) + /// The item to be added to be list. + /// The list keeping track of names of null properties. + private void AddNullPropertyName(string name, List nullPropertyNames) { - // Add null value to omitted property declared in the type - ODataProperty property = new ODataProperty - { - Name = nullPropertyName, - Value = new ODataNullValue() - }; + nullPropertyNames.Add(name); // Mark as processed, will throw if duplicate is detected. - resourceState.PropertyAndAnnotationCollector.MarkPropertyAsProcessed(property.Name); - ODataResourceBase resource = resourceState.Resource; + this.CurrentResourceState.PropertyAndAnnotationCollector.MarkPropertyAsProcessed(name); + } + + /// + /// Restore the null property values in the resource. + /// + /// The list of names of the properties to be restored with null values. + private void RestoreNullODataProperties(List nullPropertyNames) + { + IList properties = nullPropertyNames.Select(name => new ODataProperty() + { + Name = name, + Value = new ODataNullValue() + }).ToList(); + + ODataResourceBase resource = this.CurrentResourceState.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); + resource.Properties.ConcatToReadOnlyEnumerable("Properties", properties); } else { // For entity resource, concatenate the property to original enumerable. resource.Properties = - resource.Properties.Concat(new List() { property }); + resource.Properties.Concat(properties); } } diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs index a8653efc90..281972040d 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs @@ -275,6 +275,10 @@ protected override void StartResource(ODataResource resource) { // 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 diff --git a/src/Microsoft.OData.Core/ODataResource.cs b/src/Microsoft.OData.Core/ODataResource.cs index 01c4da80c1..caf60ae470 100644 --- a/src/Microsoft.OData.Core/ODataResource.cs +++ b/src/Microsoft.OData.Core/ODataResource.cs @@ -186,11 +186,6 @@ public IEnumerable Properties set { this.properties = value; } } - internal bool IsExpandedEntityProcessed(string name) - { - return this.MetadataBuilder.IsExpandedEntityProcessed(name); - } - /// Gets or sets the type name of the resource. /// The type name of the resource. public string TypeName diff --git a/src/Microsoft.OData.Core/ReadOnlyEnumerableExtensions.cs b/src/Microsoft.OData.Core/ReadOnlyEnumerableExtensions.cs index ec2888ada7..5a983b662f 100644 --- a/src/Microsoft.OData.Core/ReadOnlyEnumerableExtensions.cs +++ b/src/Microsoft.OData.Core/ReadOnlyEnumerableExtensions.cs @@ -83,5 +83,20 @@ internal static ReadOnlyEnumerable ConcatToReadOnlyEnumerable(this IEnumer readOnlyEnumerable.AddToSourceList(item); return readOnlyEnumerable; } + + /// + /// Returns a ReadOnlyEnumerableOfT that is the result of plus . + /// + /// The element type of the enumerable. + /// The source enumerable to concatenate. + /// The name of the collection to report in case there's an error. + /// Items to be concatenated to the source enumerable. + /// Returns a ReadOnlyEnumerableOfT that is the result of plus . + internal static ReadOnlyEnumerable ConcatToReadOnlyEnumerable(this IEnumerable source, string collectionName, IList items) + { + ReadOnlyEnumerable readOnlyEnumerable = source.GetOrCreateReadOnlyEnumerable(collectionName); + readOnlyEnumerable.AddToSourceList(items); + return readOnlyEnumerable; + } } } diff --git a/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs b/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs index 9cbe2fa077..179cca447f 100644 --- a/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs +++ b/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs @@ -4,6 +4,8 @@ // //--------------------------------------------------------------------- +using System.Linq; + namespace Microsoft.OData { #region Namespaces @@ -79,5 +81,20 @@ internal void AddToSourceList(T itemToAdd) this.sourceList.Add(itemToAdd); } + + /// + /// This internal method adds to the wrapped source list. From the public's perspective, this enumerable is still readonly. + /// + /// Items to be added to the source list. + internal void AddToSourceList(IList itemsToAdd) + { + Debug.Assert(this.sourceList != null, "this.sourceList != null"); + Debug.Assert(itemsToAdd != null, nameof(itemsToAdd) + " != null"); + + foreach (var item in itemsToAdd) + { + AddToSourceList(item); + } + } } } diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index b592cc769d..9ada729bcc 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -213,13 +213,13 @@ internal enum SelectionType /// /// Gets names of the expanded entity nodes at current level. /// - internal IEnumerable NamesOfSelectedExpandedEntities + internal IEnumerable SelectedExpandedEntities { get { return this.children == null ? Enumerable.Empty() - : this.children.Where(_ => _.Value.isExpandedNavigationProperty).Select(c => c.Key); + : this.children.Where(node => node.Value.isExpandedNavigationProperty).Select(c => c.Key); } } From 6af64eaf1572fb6d3c8d3e806b5b4b1de53578b3 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Wed, 7 Nov 2018 09:46:32 -0800 Subject: [PATCH 11/16] Optimiztion: resolution of declared and dynamic properties. --- .../JsonLight/ODataJsonLightReader.cs | 79 +++++-------------- .../SelectedPropertiesNode.cs | 70 ++++++++++++++++ 2 files changed, 90 insertions(+), 59 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs index 83783c35b8..c95c3590ec 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs @@ -2277,85 +2277,46 @@ private void RestoreOmittedNullValues() { IODataJsonLightReaderResourceState resourceState = this.CurrentResourceState; IEdmStructuredType edmStructuredType = resourceState.ResourceType; + SelectedPropertiesNode selectedProperties = resourceState.SelectedProperties; - if (resourceState.SelectedProperties == SelectedPropertiesNode.Empty) + if (selectedProperties == SelectedPropertiesNode.Empty) { return; } - else + else if (resourceState.Resource != null) { - if (resourceState.Resource != null) - { - Debug.Assert(edmStructuredType != null, "edmStructuredType != null"); - ODataResourceBase resource = resourceState.Resource; - List nullPropertyNames = new List(); + Debug.Assert(edmStructuredType != null, "edmStructuredType != null"); + ODataResourceBase resource = resourceState.Resource; - IEnumerable selectedProperties = resourceState.SelectedProperties.GetSelectedProperties(edmStructuredType); - // All dynamic properties: null value restoration for properties that are not present. - foreach (string name in resourceState.SelectedProperties.GetSelectedDynamicProperties(edmStructuredType)) - { - if (resource.Properties.Any(p => p.Name.Equals(name, StringComparison.Ordinal)) - || resourceState.NavigationPropertiesRead.Contains(name)) - { - continue; - } - - AddNullPropertyName(name, nullPropertyNames); - } - - foreach (IEdmProperty currentProperty in selectedProperties) - { - Debug.Assert(currentProperty.Type != null, "currentProperty.Type != null"); - - // Skip declared properties that are not null-able types. - // Skip navigation properties (no navigation links need to be restored as null.) - // Skip properties that have been read (primitive or structural). - if (!currentProperty.Type.IsNullable - || currentProperty.PropertyKind == EdmPropertyKind.Navigation - || resource.Properties.Any(p => p.Name.Equals(currentProperty.Name, StringComparison.Ordinal)) - || resourceState.NavigationPropertiesRead.Contains(currentProperty.Name)) - { - continue; - } - - AddNullPropertyName(currentProperty.Name, nullPropertyNames); - } + IList nullPropertyNames = + selectedProperties.GetSelectedNullValueProperties(edmStructuredType, resourceState); - // Restore null Expanded entities, which are specified as "isExpandedNavigationProperty" the child nodes. - foreach (string name in resourceState.SelectedProperties.SelectedExpandedEntities) + // Restore null Expanded entities, which are specified as "isExpandedNavigationProperty" the child nodes. + foreach (string name in selectedProperties.SelectedExpandedEntities) + { + if (!resource.MetadataBuilder.IsExpandedEntityProcessed(name)) { - if (!resource.MetadataBuilder.IsExpandedEntityProcessed(name)) - { - AddNullPropertyName(name, nullPropertyNames); - } + nullPropertyNames.Add(name); } + } - RestoreNullODataProperties(nullPropertyNames); + // Mark as processed, will throw if duplicate is detected. + foreach (string name in nullPropertyNames) + { + this.CurrentResourceState.PropertyAndAnnotationCollector.MarkPropertyAsProcessed(name); } + + RestoreNullODataProperties(nullPropertyNames); } } } - /// - /// Adds the item to the list of names for null properties. - /// Also updates the current resource state for processed items. - /// - /// The item to be added to be list. - /// The list keeping track of names of null properties. - private void AddNullPropertyName(string name, List nullPropertyNames) - { - nullPropertyNames.Add(name); - - // Mark as processed, will throw if duplicate is detected. - this.CurrentResourceState.PropertyAndAnnotationCollector.MarkPropertyAsProcessed(name); - } - /// /// Restore the null property values in the resource. /// /// The list of names of the properties to be restored with null values. - private void RestoreNullODataProperties(List nullPropertyNames) + private void RestoreNullODataProperties(IList nullPropertyNames) { IList properties = nullPropertyNames.Select(name => new ODataProperty() { diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index 9ada729bcc..42fedf2e10 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -6,6 +6,7 @@ using System.Collections; using System.Runtime.InteropServices.ComTypes; +using Microsoft.OData.JsonLight; namespace Microsoft.OData { @@ -511,6 +512,75 @@ internal IEnumerable GetSelectedDynamicProperties(IEdmStructuredType str return dynamicProperties; } + /// + /// Gets the list of names for selected declared or dynamic properties having values of null. + /// + /// The current structured type. + /// The current resource state. + /// The list of property names with null values. + internal IList GetSelectedNullValueProperties(IEdmStructuredType structuredType, + IODataJsonLightReaderResourceState resourceState) + { + IList result = new List(); + + // Nothing is selected. + // Or we cannot determine the selected properties without the user model. This means we won't be computing the missing properties. + if (this.selectionType == SelectionType.Empty || structuredType == null) + { + return result; + } + + if (this.selectionType == SelectionType.EntireSubtree || this.hasWildcard) + { + // Combine the list of declared properties with selected properties w/o wildcard, + IEnumerable properties = structuredType.Properties() + .Where(p => p.Type.IsNullable && + p.PropertyKind != EdmPropertyKind.Navigation) + .Select(p => p.Name); + + if (this.selectedProperties != null) + { + properties = properties.Concat( + this.selectedProperties.Where(name => !name.Equals(StarSegment))); + } + + // Get distinct items that haven't been read. + result = properties.Where(name => + !resourceState.Resource.Properties.Any(pRead => pRead.Name.Equals(name, StringComparison.Ordinal)) && + !resourceState.NavigationPropertiesRead.Contains(name)) + .Distinct().ToList(); + } + else + { + foreach (string selected in this.selectedProperties) + { + // Skip properties that have been read (primitive or structural). + bool alreadyRead = resourceState.Resource.Properties.Any(p => p.Name.Equals(selected, StringComparison.Ordinal)) + || resourceState.NavigationPropertiesRead.Contains(selected); + + if (alreadyRead) + { + continue; + } + + IEdmProperty property = structuredType.FindProperty(selected); + if (property != null + && (!property.Type.IsNullable + || property.PropertyKind == EdmPropertyKind.Navigation)) + { + // When resolved to declared property successfully, + // Skip declared properties that are not null-able types. + // Skip navigation properties (no navigation links need to be restored as null.) + continue; + } + + result.Add(selected); + } + } + + return result; + } + /// /// Gets the selected stream properties for the current node. /// From c1012d14e7a2a3c0f0417a47c4f13ac1beff17f0 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Wed, 7 Nov 2018 11:42:19 -0800 Subject: [PATCH 12/16] non-functional cleanup. --- .../Evaluation/ODataResourceMetadataBuilder.cs | 3 ++- src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs | 4 ++-- src/Microsoft.OData.Core/ODataContextUrlInfo.cs | 2 -- src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs | 2 +- src/Microsoft.OData.Core/Uri/NodeToStringBuilder.cs | 2 +- src/Microsoft.OData.Core/UriParser/Binders/SelectBinder.cs | 1 - .../UriParser/Binders/SelectExpandClauseFinisher.cs | 1 - 7 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs b/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs index 5aae167de3..caab1247e4 100644 --- a/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs +++ b/src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataBuilder.cs @@ -166,6 +166,7 @@ internal virtual void MarkNestedResourceInfoProcessed(string navigationPropertyN /// Return whether the given expanded entity of specified name has been processed. /// /// The name of the expanded entity. + /// True if the expanded entity has been processed; False otherwise. internal virtual bool IsExpandedEntityProcessed(string name) { return this.ProcessedExpandedEntityNames.Contains(name); @@ -174,7 +175,7 @@ internal virtual bool IsExpandedEntityProcessed(string name) /// /// Marks the given expanded entity as processed. /// - /// The name of the expanded entity we've already processed. + /// The name of the expanded entity we've already processed. internal virtual void MarkExpandedEntityProcessed(string expandedEntityName) { Debug.Assert(!string.IsNullOrEmpty(expandedEntityName), "!string.IsNullOrEmpty(expandedEntityName)"); diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs index 281972040d..327160d2a4 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs @@ -277,7 +277,7 @@ protected override void StartResource(ODataResource resource) this.jsonWriter.WriteName(parentNavLink.Name); // Optimization: write null and return directly. - this.jsonWriter.WriteValue((string) null); + this.jsonWriter.WriteValue((string)null); return; } } @@ -292,7 +292,7 @@ protected override void StartResource(ODataResource resource) { if (!this.ShouldOmitNullValues()) { - this.jsonWriter.WriteValue((string) null); + this.jsonWriter.WriteValue((string)null); } return; diff --git a/src/Microsoft.OData.Core/ODataContextUrlInfo.cs b/src/Microsoft.OData.Core/ODataContextUrlInfo.cs index f4ecf5f596..5d9665aeda 100644 --- a/src/Microsoft.OData.Core/ODataContextUrlInfo.cs +++ b/src/Microsoft.OData.Core/ODataContextUrlInfo.cs @@ -401,7 +401,6 @@ private static string CreateSelectExpandContextUriSegment(SelectExpandClause sel /// The generated expand string. private static string ProcessSubExpand(string expandNode, string subExpand) { - return expandNode + ODataConstants.ContextUriProjectionStart + subExpand + ODataConstants.ContextUriProjectionEnd; } @@ -415,7 +414,6 @@ private static string CombineSelectAndExpandResult(IList selectList, ILi if (selectList.Any()) { - currentExpandClause += String.Join(ODataConstants.ContextUriProjectionPropertySeparator, selectList.ToArray()); } diff --git a/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs b/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs index 179cca447f..f40ed36db7 100644 --- a/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs +++ b/src/Microsoft.OData.Core/ReadOnlyEnumerableOfT.cs @@ -85,7 +85,7 @@ internal void AddToSourceList(T itemToAdd) /// /// This internal method adds to the wrapped source list. From the public's perspective, this enumerable is still readonly. /// - /// Items to be added to the source list. + /// Items to be added to the source list. internal void AddToSourceList(IList itemsToAdd) { Debug.Assert(this.sourceList != null, "this.sourceList != null"); diff --git a/src/Microsoft.OData.Core/Uri/NodeToStringBuilder.cs b/src/Microsoft.OData.Core/Uri/NodeToStringBuilder.cs index 042e8ab6fd..b42cab6c6f 100644 --- a/src/Microsoft.OData.Core/Uri/NodeToStringBuilder.cs +++ b/src/Microsoft.OData.Core/Uri/NodeToStringBuilder.cs @@ -10,10 +10,10 @@ namespace Microsoft.OData using System.Collections.Generic; using System.Diagnostics; using System.Globalization; + using System.Text; using System.Text.RegularExpressions; using Microsoft.OData.Edm; using Microsoft.OData.UriParser; - using System.Text; /// /// Build QueryNode to String Representation diff --git a/src/Microsoft.OData.Core/UriParser/Binders/SelectBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/SelectBinder.cs index 41e909a4af..44f5e3ce7a 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/SelectBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/SelectBinder.cs @@ -45,7 +45,6 @@ public SelectBinder(IEdmModel model, IEdmStructuredType edmType, int maxDepth, S /// A new SelectExpandClause decorated with the information from the selectToken public SelectExpandClause Bind(SelectToken tokenIn) { - if (tokenIn == null || !tokenIn.Properties.Any()) { // if there are no properties selected for this level, then by default we select diff --git a/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandClauseFinisher.cs b/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandClauseFinisher.cs index c80755e2c1..54047087ba 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandClauseFinisher.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandClauseFinisher.cs @@ -30,7 +30,6 @@ public static void AddExplicitNavPropLinksWhereNecessary(SelectExpandClause clau IEnumerable selectedPaths = selectItems.OfType().Select(item => item.SelectedPath); foreach (ExpandedNavigationSelectItem navigationSelect in selectItems.Where(I => I.GetType() == typeof(ExpandedNavigationSelectItem))) { - AddExplicitNavPropLinksWhereNecessary(navigationSelect.SelectAndExpand); } From 2a5b9b1aef0da0d824cdd26aa560a9105ce0ede4 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Thu, 8 Nov 2018 14:25:15 -0800 Subject: [PATCH 13/16] Revert the optimization related to throwOnUndeclaredProperty. --- .../ODataJsonLightPropertySerializer.cs | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index f0b7fa6871..ca8c96e935 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -211,23 +211,19 @@ private void WriteProperty( 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. - // For very wide and sparse outputs it allows to avoid a lot of dictionary lookups + WriterValidationUtils.ValidatePropertyDefined(this.currentPropertyInfo, this.MessageWriterSettings.ThrowOnUndeclaredPropertyForNonOpenType); + + duplicatePropertyNameChecker.ValidatePropertyUniqueness(property); + bool isNullValue = (value == null || value is ODataNullValue); if (isNullValue && omitNullValues) { - if (!this.currentPropertyInfo.IsTopLevel && !this.MessageWriterSettings.ThrowIfTypeConflictsWithMetadata) + if (!this.currentPropertyInfo.IsTopLevel) { return; } } - WriterValidationUtils.ValidatePropertyDefined(this.currentPropertyInfo, this.MessageWriterSettings.ThrowOnUndeclaredPropertyForNonOpenType); - - duplicatePropertyNameChecker.ValidatePropertyUniqueness(property); - // handle ODataUntypedValue ODataUntypedValue untypedValue = value as ODataUntypedValue; if (untypedValue != null) @@ -253,7 +249,7 @@ private void WriteProperty( if (isNullValue) { - this.WriteNullProperty(property, omitNullValues); + this.WriteNullProperty(property); return; } @@ -380,10 +376,8 @@ private void WriteStreamReferenceProperty(string propertyName, ODataStreamRefere /// Writes a Null property. /// /// The property to write out. - /// Whether to omit the writing of this null property value. private void WriteNullProperty( - ODataProperty property, - bool omitNullValues) + ODataProperty property) { this.WriterValidator.ValidateNullPropertyValue( this.currentPropertyInfo.MetadataType.TypeReference, property.Name, @@ -408,7 +402,7 @@ private void WriteNullProperty( throw new ODataException(Strings.ODataMessageWriter_CannotWriteTopLevelNull); } } - else if (!omitNullValues) + else { this.JsonWriter.WriteName(property.Name); this.JsonLightValueSerializer.WriteNullValue(); From fd97a4455c69cd357d7e0f39b347dd76baf5b62e Mon Sep 17 00:00:00 2001 From: Biao Li Date: Fri, 9 Nov 2018 11:43:24 -0800 Subject: [PATCH 14/16] Don't omit-nulls for annotated properties. --- .../ODataJsonLightPropertySerializer.cs | 3 +- .../JsonLight/ODataJsonLightWriter.cs | 3 +- .../ShippingAssemblyAttributes.cs | 1 + .../ReadOmitNullsResponseTests.cs | 69 +++++++++++-------- 4 files changed, 44 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index ca8c96e935..0403495fa4 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -124,7 +124,8 @@ internal void WriteProperties( owningType, false /* isTopLevel */, !isComplexValue, - omitNullValues, + // Annotated properties won't be omitted even if it is null. + omitNullValues && property.InstanceAnnotations.Count == 0, duplicatePropertyNameChecker); } } diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs index 327160d2a4..a5d64bfb8b 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightWriter.cs @@ -271,7 +271,8 @@ protected override void StartResource(ODataResource resource) this.jsonLightResourceSerializer.InstanceAnnotationWriter.WriteInstanceAnnotations(parentNavLink.GetInstanceAnnotations(), parentNavLink.Name); - if (!this.ShouldOmitNullValues()) + // 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); diff --git a/src/Microsoft.OData.Core/ShippingAssemblyAttributes.cs b/src/Microsoft.OData.Core/ShippingAssemblyAttributes.cs index 8dd911d958..271a4cf883 100644 --- a/src/Microsoft.OData.Core/ShippingAssemblyAttributes.cs +++ b/src/Microsoft.OData.Core/ShippingAssemblyAttributes.cs @@ -19,6 +19,7 @@ [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.Test.Taupo.OData" + AssemblyRef.TestPublicKey)] [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.OData.Core.Tests" + AssemblyRef.TestPublicKey)] [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.OData.Client.Tests" + AssemblyRef.TestPublicKey)] +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.Test.OData.Tests.Client" + AssemblyRef.TestPublicKey)] [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.Test.OData.DependencyInjection.NetCore" + AssemblyRef.TestPublicKey)] [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.OData.Core.Tests.NetCore" + AssemblyRef.TestPublicKey)] #pragma warning restore 436 diff --git a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs index a02815af99..2f61d7751c 100644 --- a/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs +++ b/test/EndToEndTests/Tests/Client/Build.Desktop/WriteJsonPayloadTests/ReadOmitNullsResponseTests.cs @@ -65,14 +65,15 @@ public void ReadWriteNullExpandedNavigationTest_notOmitNulls() Uri serviceUri = new Uri("http://test/"); string expectedNulls = @"{ - ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend())/$entity"", - ""Name"":""Fred"", - ""Title@test.annotation"":""annotationValue"",""Title"":null, - ""Dynamic"":null, - ""DynamicAnnotated@test.annotation"":""annotationValue"",""DynamicAnnotated"":null, - ""Address"":null,""Manager"":null, - ""Friend"": {""Name"":""FriendOfFred""} - }"; + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend())/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"",""Title"":null, + ""Dynamic"":null, + ""DynamicAnnotated@test.annotation"":""annotationValue"",""DynamicAnnotated"":null, + ""Address@test.annotation"":""InsufficientPrivileges"",""Address"":null, + ""Manager"":null, + ""Friend"": {""Name"":""FriendOfFred""} + }"; expectedNulls = Regex.Replace(expectedNulls, @"\s*", string.Empty, RegexOptions.Multiline); Assert.IsTrue(expectedNulls.Contains(serviceUri.ToString())); @@ -180,12 +181,13 @@ public void ReadWriteNullNavigationTest_omitNulls() // Requirement: Omit null value for nested resource if omit-values=nulls is specified. string expectedNoNulls = @"{ - ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager,Friend())/$entity"", - ""Name"":""Fred"", - ""Title@test.annotation"":""annotationValue"", - ""DynamicAnnotated@test.annotation"":""annotationValue"", - ""Friend"": {""Name"":""FriendOfFred""} - }"; + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager,Friend())/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"",""Title"":null, + ""DynamicAnnotated@test.annotation"":""annotationValue"",""DynamicAnnotated"":null, + ""Address@test.annotation"":""InsufficientPrivileges"",""Address"":null, + ""Friend"": {""Name"":""FriendOfFred""} + }"; expectedNoNulls = Regex.Replace(expectedNoNulls, @"\s*", string.Empty, RegexOptions.Multiline); Assert.IsTrue(expectedNoNulls.Contains(serviceUri.ToString())); @@ -263,13 +265,13 @@ public void ReadWriteNullNavigationTest_omitNulls() } Assert.IsTrue(employeeEntityCount == 2, $"employeeEntityCount: Expected: 2, Actual: {employeeEntityCount}"); - Assert.IsFalse(addressReturned, "Address is returned but it should not be."); + Assert.IsTrue(addressReturned, "Address is returned but it should not be."); Assert.IsFalse(managerReturned, "Manager is returned but it should not be, because it is navigation link and its value is omitted in payload."); Assert.IsNotNull(friendResource, "Friend resource is null but it should not be."); VerifyAdditionalProperties(employeeResource); Assert.IsTrue(employeeResource.Properties != null); - Assert.AreEqual(employeeResource.Properties.Count(), 5); - Assert.IsTrue(employeeResource.Properties.Any(p => p.Name.Equals("Address") && (p.Value == null))); + Assert.AreEqual(4, employeeResource.Properties.Count()); + Assert.IsFalse(employeeResource.Properties.Any(p => p.Name.Equals("Address"))); Assert.IsFalse(employeeResource.Properties.Any(p => p.Name.Equals("Manager"))); } @@ -287,18 +289,20 @@ private void ReadWriteNullExpandedNavigationTest_omitNulls(bool nestedSelect) // Requirement: Omit null value for nested resource if omit-values=nulls is specified. string expectedNoNulls = nestedSelect ? @"{ - ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend(Name,Title))/$entity"", - ""Name"":""Fred"", - ""Title@test.annotation"":""annotationValue"", - ""DynamicAnnotated@test.annotation"":""annotationValue"", - ""Friend"": {""Name"":""FriendOfFred""} + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend(Name,Title))/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"",""Title"":null, + ""DynamicAnnotated@test.annotation"":""annotationValue"",""DynamicAnnotated"":null, + ""Address@test.annotation"":""InsufficientPrivileges"",""Address"":null, + ""Friend"": {""Name"":""FriendOfFred""} }" : @"{ - ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend())/$entity"", - ""Name"":""Fred"", - ""Title@test.annotation"":""annotationValue"", - ""DynamicAnnotated@test.annotation"":""annotationValue"", - ""Friend"": {""Name"":""FriendOfFred""} + ""@odata.context"":""http://test/$metadata#employees(Name,Title,Dynamic,Address,Manager(),Friend())/$entity"", + ""Name"":""Fred"", + ""Title@test.annotation"":""annotationValue"",""Title"":null, + ""DynamicAnnotated@test.annotation"":""annotationValue"",""DynamicAnnotated"":null, + ""Address@test.annotation"":""InsufficientPrivileges"",""Address"":null, + ""Friend"": {""Name"":""FriendOfFred""} }"; expectedNoNulls = Regex.Replace(expectedNoNulls, @"\s*", string.Empty, RegexOptions.Multiline); @@ -382,7 +386,7 @@ private void ReadWriteNullExpandedNavigationTest_omitNulls(bool nestedSelect) } Assert.IsTrue(employeeEntityCount == 2, $"employeeEntityCount: Expected: 2, Actual: {employeeEntityCount}"); - Assert.IsFalse(addressReturned, "Address is returned but it should not be."); + Assert.IsTrue(addressReturned, "Address is annotated, should be returned but isn't."); Assert.IsFalse(managerReturned, "Manager is returned but it should not be, because value is omitted in payload."); Assert.IsNotNull(friendResource, "Friend resource is null but it should not be."); @@ -457,11 +461,16 @@ private void WriteResponse(ODataWriter writer) }); // write address - writer.WriteStart(new ODataNestedResourceInfo + ODataNestedResourceInfo addrNestedResourceInfo = new ODataNestedResourceInfo { Name = "Address", IsCollection = false - }); + }; + addrNestedResourceInfo.SetInstanceAnnotations(new ODataInstanceAnnotation[] + { new ODataInstanceAnnotation("test.annotation", new ODataPrimitiveValue("InsufficientPrivileges")) } + ); + + writer.WriteStart(addrNestedResourceInfo); writer.WriteStart((ODataResource)null); writer.WriteEnd(); //address writer.WriteEnd(); //address nestedInfo From 455f5bd4b4c97fae6acdbcbc2ea34a2a90130df5 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Tue, 13 Nov 2018 14:23:38 -0800 Subject: [PATCH 15/16] CR comments. --- .../ODataJsonLightPropertySerializer.cs | 9 +++---- .../SelectedPropertiesNode.cs | 25 ++----------------- 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs index 0403495fa4..40dad0c29e 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs @@ -186,8 +186,6 @@ private void WriteProperty( { WriterValidationUtils.ValidatePropertyNotNull(property); - ODataValue value = property.ODataValue; - string propertyName = property.Name; if (this.JsonLightOutputContext.MessageWriterSettings.Validations != ValidationKinds.None) @@ -204,6 +202,9 @@ private void WriteProperty( this.currentPropertyInfo = this.JsonLightOutputContext.PropertyCacheHandler.GetProperty(propertyName, owningType); } + WriterValidationUtils.ValidatePropertyDefined(this.currentPropertyInfo, this.MessageWriterSettings.ThrowOnUndeclaredPropertyForNonOpenType); + + duplicatePropertyNameChecker.ValidatePropertyUniqueness(property); if (currentPropertyInfo.MetadataType.IsUndeclaredProperty) { @@ -212,9 +213,7 @@ private void WriteProperty( WriteInstanceAnnotation(property, isTopLevel, currentPropertyInfo.MetadataType.IsUndeclaredProperty); - WriterValidationUtils.ValidatePropertyDefined(this.currentPropertyInfo, this.MessageWriterSettings.ThrowOnUndeclaredPropertyForNonOpenType); - - duplicatePropertyNameChecker.ValidatePropertyUniqueness(property); + ODataValue value = property.ODataValue; bool isNullValue = (value == null || value is ODataNullValue); if (isNullValue && omitNullValues) diff --git a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs index 42fedf2e10..1e1b2e5b76 100644 --- a/src/Microsoft.OData.Core/SelectedPropertiesNode.cs +++ b/src/Microsoft.OData.Core/SelectedPropertiesNode.cs @@ -492,32 +492,11 @@ internal IEnumerable GetSelectedProperties(IEdmStructuredType stru } /// - /// Return names of dynamic properties. - /// - /// The current structured type. - /// The names of dynamic properties. - internal IEnumerable GetSelectedDynamicProperties(IEdmStructuredType structuredType) - { - IEnumerable dynamicProperties = Enumerable.Empty(); - - if (this.selectedProperties != null && this.selectedProperties.Count > 0) - { - dynamicProperties = this.selectedProperties.Select( - prop => structuredType.FindProperty(prop) == null - ? prop - : null) - .OfType(); - } - - return dynamicProperties; - } - - /// - /// Gets the list of names for selected declared or dynamic properties having values of null. + /// Gets the list of names for selected nullable declared or dynamic properties missing from payload. /// /// The current structured type. /// The current resource state. - /// The list of property names with null values. + /// The list of property names missing from payload. internal IList GetSelectedNullValueProperties(IEdmStructuredType structuredType, IODataJsonLightReaderResourceState resourceState) { From db86b90c155e388ae1e1314344626ed0564a1d99 Mon Sep 17 00:00:00 2001 From: Biao Li Date: Wed, 14 Nov 2018 15:23:58 -0800 Subject: [PATCH 16/16] CR comment: renamed navigationLink to nestedResourceInfo for original tests in PropertyAndValueJsonLightReaderIntegrationTests.cs --- ...AndValueJsonLightReaderIntegrationTests.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) 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 4d4f384ddb..1d2079472c 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 @@ -71,7 +71,7 @@ public void ReadingPayloadInt64SingleDoubleDecimalWithoutSuffix() IEdmModel mainModel = TestUtils.WrapReferencedModelsToMainModel("EntityNs", "MyContainer", model); List entries = new List(); - ODataNestedResourceInfo navigationLink; + ODataNestedResourceInfo nestedResourceInfo; this.ReadEntryPayload(mainModel, payload, entitySet, entityType, reader => { @@ -81,7 +81,7 @@ public void ReadingPayloadInt64SingleDoubleDecimalWithoutSuffix() entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLink = (ODataNestedResourceInfo)reader.Item; + nestedResourceInfo = (ODataNestedResourceInfo)reader.Item; break; default: break; @@ -341,7 +341,7 @@ public void ReadingTypeDefinitionPayloadJsonLight() "}"; List entries = new List(); - ODataNestedResourceInfo navigationLink = null; + ODataNestedResourceInfo nestedResourceInfo = null; this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { @@ -351,7 +351,7 @@ public void ReadingTypeDefinitionPayloadJsonLight() entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLink = (ODataNestedResourceInfo)reader.Item; + nestedResourceInfo = (ODataNestedResourceInfo)reader.Item; break; default: break; @@ -364,7 +364,7 @@ public void ReadingTypeDefinitionPayloadJsonLight() propertyList[1].Name.Should().Be("Weight"); propertyList[1].Value.Should().Be(60.5); - navigationLink.Name.Should().Be("Address"); + nestedResourceInfo.Name.Should().Be("Address"); var address = entries[1]; address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.OrdinalIgnoreCase)).Value.Should().Be("China"); } @@ -491,7 +491,7 @@ public void ReadingTypeDefinitionPayloadWithTypeAnnotationJsonLight() "}"; List entries = new List(); - ODataNestedResourceInfo navigationLink = null; + ODataNestedResourceInfo nestedResourceInfo = null; this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { @@ -501,7 +501,7 @@ public void ReadingTypeDefinitionPayloadWithTypeAnnotationJsonLight() entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLink = (ODataNestedResourceInfo)reader.Item; + nestedResourceInfo = (ODataNestedResourceInfo)reader.Item; break; default: break; @@ -516,7 +516,7 @@ public void ReadingTypeDefinitionPayloadWithTypeAnnotationJsonLight() propertyList[2].Name.Should().Be("Height"); propertyList[2].Value.Should().Be(180); - navigationLink.Name.Should().Be("Address"); + nestedResourceInfo.Name.Should().Be("Address"); var address = entries[1]; address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.OrdinalIgnoreCase)).Value.Should().Be("China"); } @@ -568,7 +568,7 @@ public void ReadingTypeDefinitionPayloadWithMultipleTypeDefinitionJsonLight() "}"; List entries = new List(); - ODataNestedResourceInfo navigationLink = null; + ODataNestedResourceInfo nestedResourceInfo = null; this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { @@ -578,7 +578,7 @@ public void ReadingTypeDefinitionPayloadWithMultipleTypeDefinitionJsonLight() entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLink = (ODataNestedResourceInfo)reader.Item; + nestedResourceInfo = (ODataNestedResourceInfo)reader.Item; break; default: break; @@ -591,7 +591,7 @@ public void ReadingTypeDefinitionPayloadWithMultipleTypeDefinitionJsonLight() propertyList[1].Name.Should().Be("Weight"); propertyList[1].Value.Should().Be(60.5); - navigationLink.Name.Should().Be("Address"); + nestedResourceInfo.Name.Should().Be("Address"); var address = entries[1]; address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.OrdinalIgnoreCase)).Value.Should().Be("China"); } @@ -640,7 +640,7 @@ public void ReadingTypeDefinitionPayloadWithEdmTypeAnnotationJsonLight() "}"; List entries = new List(); - ODataNestedResourceInfo navigationLink = null; + ODataNestedResourceInfo nestedResourceInfo = null; this.ReadEntryPayload(model, payload, entitySet, entityType, reader => { @@ -650,7 +650,7 @@ public void ReadingTypeDefinitionPayloadWithEdmTypeAnnotationJsonLight() entries.Add(reader.Item as ODataResource); break; case ODataReaderState.NestedResourceInfoStart: - navigationLink = (ODataNestedResourceInfo)reader.Item; + nestedResourceInfo = (ODataNestedResourceInfo)reader.Item; break; default: break; @@ -663,7 +663,7 @@ public void ReadingTypeDefinitionPayloadWithEdmTypeAnnotationJsonLight() propertyList[1].Name.Should().Be("Weight"); propertyList[1].Value.Should().Be(60.5); - navigationLink.Name.Should().Be("Address"); + nestedResourceInfo.Name.Should().Be("Address"); var address = entries[1]; address.Properties.FirstOrDefault(s => string.Equals(s.Name, "CountryRegion", StringComparison.OrdinalIgnoreCase)).Value.Should().Be("China"); }