Code updates for walk-thru comments.

src/Microsoft.OData.Core/JsonLight/ODataJsonLightPropertySerializer.cs

@@ -98,26 +98,6 @@ internal void WriteTopLevelProperty(ODataProperty property)
                 });
         }
 
-        /// <summary>
-        /// This is the backward-compatible method to write property names and value pairs
-        /// without omitting null property values.
-        /// </summary>
-        /// <param name="owningType">The <see cref="IEdmStructuredType"/> of the resource (or null if not metadata is available).</param>
-        /// <param name="properties">The enumeration of properties to write out.</param>
-        /// <param name="isComplexValue">
-        /// Whether the properties are being written for complex value. Also used for detecting whether stream properties
-        /// are allowed as named stream properties should only be defined on ODataResource instances
-        /// </param>
-        /// <param name="duplicatePropertyNameChecker">The DuplicatePropertyNameChecker to use.</param>
-        internal void WriteProperties(
-            IEdmStructuredType owningType,
-            IEnumerable<ODataProperty> properties,
-            bool isComplexValue,
-            IDuplicatePropertyNameChecker duplicatePropertyNameChecker)
-        {
-            this.WriteProperties(owningType, properties, isComplexValue, /*omitNullValues*/ false, duplicatePropertyNameChecker);
-        }
-
         /// <summary>
         /// Writes property names and value pairs.
         /// </summary>
@@ -171,7 +151,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
@@ -194,6 +174,7 @@ private bool IsOpenProperty(ODataProperty property)
         /// <param name="isTopLevel">true when writing a top-level property; false for nested properties.</param>
         /// <param name="allowStreamProperty">Should pass in true if we are writing a property of an ODataResource instance, false otherwise.
         /// Named stream properties should only be defined on ODataResource instances.</param>
+        /// <param name="omitNullValues">whether to omit null property values for writing.</param>
         /// <param name="duplicatePropertyNameChecker">The DuplicatePropertyNameChecker to use.</param>
         [SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Splitting the code would make the logic harder to understand; class coupling is only slightly above threshold.")]
         private void WriteProperty(
@@ -387,6 +368,7 @@ private void WriteStreamReferenceProperty(string propertyName, ODataStreamRefere
         /// Writes a Null property.
         /// </summary>
         /// <param name="property">The property to write out.</param>
+        /// <param name="omitNullValues">Whether to omit the writing of this null property value.</param>
         private void WriteNullProperty(
             ODataProperty property,
             bool omitNullValues)

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<ODataProperty>() {property});
+                                        resource.Properties.Concat(new List<ODataProperty>() { property });
                                 }
                             }
                         }

src/Microsoft.OData.Core/ODataConstants.cs

@@ -59,6 +59,19 @@ internal static class ODataInternalConstants
         /// </summary>
         public const string ContentIdHeader = "Content-ID";
 
+        #region Preference Headers
+        /// <summary>
+        /// Valid value for <code>OmitValuesPreferenceToken</code> indicating nulls are omitted.
+        /// </summary>
+        public const string OmitValuesNulls = "nulls";
+
+        /// <summary>
+        /// Valid value for <code>OmitValuesPreferenceToken</code> indicating defaults are omitted.
+        /// Internal access only since "defaults" is not supported yet.
+        /// </summary>
+        internal const string OmitValuesDefaults = "defaults";
+        #endregion Preference Headers
+
         /// <summary>
         /// Name of the Content-Length HTTP header.
         /// </summary>
@@ -201,17 +214,5 @@ internal static class ODataInternalConstants
         internal const string ContextUriDeletedLink = UriSegmentSeparator + DeletedLink;
 
         #endregion Context URL
-
-        #region Preference Headers
-            /// <summary>
-            /// Valid value for <code>OmitValuesPreferenceToken</code> indicating nulls are omitted.
-            /// </summary>
-            internal const string OmitValuesNulls = "nulls";
-
-            /// <summary>
-            /// Valid value for <code>OmitValuesPreferenceToken</code> indicating defaults are omitted.
-            /// </summary>
-            internal const string OmitValuesDefaults = "defaults";
-        #endregion Preference Headers
     }
 }

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

src/Microsoft.OData.Core/ODataMessageWriterSettings.cs

@@ -183,7 +183,7 @@ public ODataUri ODataUri
         /// <remarks>
         /// Default value is false, that means serialize null values.
         /// </remarks>
-        public bool OmitNullValues { get; set; }
+        internal bool OmitNullValues { get; set; }
 
         /// <summary>
         /// Returns whether ThrowOnUndeclaredPropertyForNonOpenType validation setting is enabled.

src/Microsoft.OData.Core/ODataOutputContext.cs

@@ -437,6 +437,15 @@ public virtual Task WriteErrorAsync(ODataError odataError, bool includeDebugInfo
         }
 #endif
 
+        /// <summary>
+        /// Returns whether properties of null values should be omitted when serializing the payload.
+        /// </summary>
+        /// <returns>Return true to omit null-value properties; false otherwise.</returns>
+        internal bool ShouldOmitNullValues()
+        {
+            return this.WritingResponse && this.MessageWriterSettings.OmitNullValues;
+        }
+
         /// <summary>
         /// Writes an <see cref="ODataError"/> into the message payload.
         /// </summary>

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.
             }
         }
 

src/Microsoft.OData.Core/ODataWriterCore.cs

@@ -1150,11 +1150,8 @@ protected void ValidateNoDeltaLinkForExpandedResourceSet(ODataResourceSet resour
         /// <returns>Return true to omit null-value properties; false otherwise.</returns>
         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();
         }
 
         /// <summary>

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();
 

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();
 

test/FunctionalTests/Tests/DataOData/Tests/OData.Common.Tests/PublicApi/PublicApi.bsl

@@ -4503,6 +4503,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"
 }
 
 [
@@ -5143,7 +5144,6 @@ public sealed class Microsoft.OData.ODataMessageWriterSettings {
 	string JsonPCallback  { 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; }
 

test/FunctionalTests/Tests/DataOData/Tests/OData.Writer.Tests/Microsoft.Test.Taupo.OData.Writer.Tests.csproj

@@ -182,7 +182,7 @@
     <Content Include="JsonLight\JsonLightEntryWriterTests.OpenPropertiesInEntryTest.approved.txt">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
-    <Content Include="JsonLight\JsonLightEntryWriterTests.IgnoreNullPropertiesInEntryTest.approved.txt">
+    <Content Include="JsonLight\JsonLightEntryWriterTests.OmitNullPropertiesInEntryTest.approved.txt">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
     <Content Include="JsonLight\JsonLightPropertyWriterTests.WriteFloatingPointValue.approved.txt">

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;
             }