Honor dictionary key policy when entry value is null (#42267) (#42284)

* Honor dictionary key policy when entry value is null * Address review feedback * Remove reverse Json checks
dotnet · Nov 1, 2019 · 281e5da · 281e5da
1 parent 94e453f
commit 281e5da
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 21 deletions.
diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNullable.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNullable.cs
@@ -5,6 +5,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Text.Json.Serialization;
 
 namespace System.Text.Json
 {
@@ -99,22 +100,26 @@ protected override void OnWriteDictionary(ref WriteStackFrame current, Utf8JsonW
 
             Debug.Assert(key != null);
 
+            if (Options.DictionaryKeyPolicy != null)
+            {
+                // We should not be in the Nullable-value implementation branch for extension data.
+                // (TValue should be typeof(object) or typeof(JsonElement)).
+                Debug.Assert(current.ExtensionDataStatus != ExtensionDataWriteStatus.Writing);
+
+                key = Options.DictionaryKeyPolicy.ConvertName(key);
+
+                if (key == null)
+                {
+                    ThrowHelper.ThrowInvalidOperationException_SerializerDictionaryKeyNull(Options.DictionaryKeyPolicy.GetType());
+                }
+            }
+
             if (value == null)
             {
                 writer.WriteNull(key);
             }
             else
             {
-                if (Options.DictionaryKeyPolicy != null)
-                {
-                    key = Options.DictionaryKeyPolicy.ConvertName(key);
-
-                    if (key == null)
-                    {
-                        ThrowHelper.ThrowInvalidOperationException_SerializerDictionaryKeyNull(Options.DictionaryKeyPolicy.GetType());
-                    }
-                }
-
                 writer.WritePropertyName(key);
                 Converter.Write(writer, value.GetValueOrDefault(), Options);
             }

diff --git a/...tem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs b/...tem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs
@@ -145,23 +145,26 @@ internal static void WriteDictionary<TProperty>(
                     current.JsonPropertyInfo.PropertyInfo);
             }
 
+            Debug.Assert(key != null);
+
+            if (options.DictionaryKeyPolicy != null &&
+                // We do not convert extension data.
+                current.ExtensionDataStatus != ExtensionDataWriteStatus.Writing)
+            {
+                key = options.DictionaryKeyPolicy.ConvertName(key);
+
+                if (key == null)
+                {
+                    ThrowHelper.ThrowInvalidOperationException_SerializerDictionaryKeyNull(options.DictionaryKeyPolicy.GetType());
+                }
+            }
+
             if (value == null)
             {
                 writer.WriteNull(key);
             }
             else
             {
-                if (options.DictionaryKeyPolicy != null && 
-                    current.ExtensionDataStatus != ExtensionDataWriteStatus.Writing) // We do not convert extension data.
-                {
-                    key = options.DictionaryKeyPolicy.ConvertName(key);
-
-                    if (key == null)
-                    {
-                        ThrowHelper.ThrowInvalidOperationException_SerializerDictionaryKeyNull(options.DictionaryKeyPolicy.GetType());
-                    }
-                }
-
                 writer.WritePropertyName(key);
                 converter.Write(writer, value, options);
             }

diff --git a/src/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs b/src/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs
@@ -46,6 +46,29 @@ public static void CamelCaseDeserialize()
             Assert.Equal(4, obj[1]["Key2"]);
         }
 
+        [Fact]
+        public static void IgnoreKeyPolicyForExtensionData()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase // e.g. Key1 -> key1.
+            };
+
+            // Ensure we ignore key policy for extension data and deserialize keys as they are.
+            ClassWithExtensionData myClass = JsonSerializer.Deserialize<ClassWithExtensionData>(@"{""Key1"":1, ""Key2"":2}", options);
+            Assert.Equal(1, (myClass.ExtensionData["Key1"]).GetInt32());
+            Assert.Equal(2, (myClass.ExtensionData["Key2"]).GetInt32());
+
+            // Ensure we ignore key policy for extension data and serialize keys as they are.
+            Assert.Equal(@"{""Key1"":1,""Key2"":2}", JsonSerializer.Serialize(myClass, options));
+        }
+
+        public class ClassWithExtensionData
+        {
+            [JsonExtensionData]
+            public Dictionary<string, JsonElement> ExtensionData { get; set; }
+        }
+
         [Fact]
         public static void CamelCaseSerialize()
         {
@@ -72,6 +95,56 @@ public static void CamelCaseSerialize()
             Assert.Equal(JsonCamel, json);
         }
 
+        [Fact]
+        public static void CamelCaseSerialize_Null_Values()
+        {
+            var options = new JsonSerializerOptions()
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase // e.g. Key1 -> key1.
+            };
+
+            Dictionary<string, string>[] obj = new Dictionary<string, string>[]
+            {
+                new Dictionary<string, string>() { { "Key1", null }, { "Key2", null } },
+            };
+
+            const string Json = @"[{""Key1"":null,""Key2"":null}]";
+            const string JsonCamel = @"[{""key1"":null,""key2"":null}]";
+
+            // Without key policy option, serialize keys as they are.
+            string json = JsonSerializer.Serialize<object>(obj);
+            Assert.Equal(Json, json);
+
+            // With key policy option, serialize keys with camel casing.
+            json = JsonSerializer.Serialize<object>(obj, options);
+            Assert.Equal(JsonCamel, json);
+        }
+
+        [Fact]
+        public static void CamelCaseSerialize_Null_Nullable_Values()
+        {
+            var options = new JsonSerializerOptions()
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase // e.g. Key1 -> key1.
+            };
+
+            Dictionary<string, int?>[] obj = new Dictionary<string, int?>[]
+            {
+                new Dictionary<string, int?>() { { "Key1", null }, { "Key2", null } },
+            };
+
+            const string Json = @"[{""Key1"":null,""Key2"":null}]";
+            const string JsonCamel = @"[{""key1"":null,""key2"":null}]";
+
+            // Without key policy option, serialize keys as they are.
+            string json = JsonSerializer.Serialize<object>(obj);
+            Assert.Equal(Json, json);
+
+            // With key policy option, serialize keys with camel casing.
+            json = JsonSerializer.Serialize<object>(obj, options);
+            Assert.Equal(JsonCamel, json);
+        }
+
         [Fact]
         public static void CustomNameDeserialize()
         {