From 3e9e4e294a876dd1dda3e0c34a5ffc03ef0ae7af Mon Sep 17 00:00:00 2001 From: Biao Li Date: Mon, 5 Mar 2018 02:22:44 -0800 Subject: [PATCH] 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 9e696a393c..2ccc2cbbcd 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 7ff95fa04f..a8cd92527c 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 58eeb7df5c..df52b2e549 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 4fd5dd42f5..acefee6af5 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 6b8c8d3c61..74fe5d2081 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 @@ -4536,6 +4536,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" } [ @@ -5178,7 +5179,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; }