Honor dictionary key policy when serializing (dotnet/corefx#39032)

* Honor dictionary key policy when serializing

* Address review feedback

Commit migrated from https://github.com/dotnet/corefx/commit/00b49d189519677732bcde79a91bee955f3f7a82

diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx
index 404f978..b40292d 100644 (file)
--- a/src/libraries/System.Text.Json/src/Resources/Strings.resx
+++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx
@@ -402,6 +402,9 @@
   <data name="SerializationConverterWrite" xml:space="preserve">
     <value>The converter '{0}' wrote too much or not enough.</value>
   </data>
+  <data name="SerializerDictionaryKeyNull" xml:space="preserve">
+    <value>The dictionary key policy '{0}' cannot return a null key.</value>
+  </data>
   <data name="SerializationDuplicateAttribute" xml:space="preserve">
     <value>The attribute '{0}' cannot exist more than once on '{1}'.</value>
   </data>

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs
index 7d8ab23..987dc80 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs
@@ -31,6 +31,13 @@ namespace System.Text.Json
                 if (options.DictionaryKeyPolicy != null)
                 {
                     keyName = options.DictionaryKeyPolicy.ConvertName(keyName);
+
+                    if (keyName == null)
+                    {
+                        ThrowHelper.ThrowInvalidOperationException_SerializerDictionaryKeyNull(options.DictionaryKeyPolicy.GetType());
+                    }
+
+                    keyName = options.DictionaryKeyPolicy.ConvertName(keyName);
                 }
 
                 if (state.Current.IsDictionary || state.Current.IsIDictionaryConstructible)

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs
index a09c721..6d426aa 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleDictionary.cs
@@ -143,6 +143,16 @@ namespace System.Text.Json
             }
             else
             {
+                if (options.DictionaryKeyPolicy != null)
+                {
+                    key = options.DictionaryKeyPolicy.ConvertName(key);
+
+                    if (key == null)
+                    {
+                        ThrowHelper.ThrowInvalidOperationException_SerializerDictionaryKeyNull(options.DictionaryKeyPolicy.GetType());
+                    }
+                }
+
                 JsonEncodedText escapedKey = JsonEncodedText.Encode(key);
                 writer.WritePropertyName(escapedKey);
                 converter.Write(writer, value, options);

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
index 905d751..74e2534 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
@@ -115,6 +115,12 @@ namespace System.Text.Json
             throw new InvalidOperationException(SR.Format(SR.SerializerPropertyNameNull, parentType, jsonPropertyInfo.PropertyInfo.Name));
         }
 
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        public static void ThrowInvalidOperationException_SerializerDictionaryKeyNull(Type policyType)
+        {
+            throw new InvalidOperationException(SR.Format(SR.SerializerDictionaryKeyNull, policyType));
+        }
+
         public static void ThrowJsonException_DeserializeDataRemaining(long length, long bytesRemaining)
         {
             throw new JsonException(SR.Format(SR.DeserializeDataRemaining, length, bytesRemaining), path: null, lineNumber: null, bytePositionInLine: null);

diff --git a/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs
new file mode 100644 (file)
index 0000000..cf3d3fc
--- /dev/null
+++ b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs
@@ -0,0 +1,138 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.Runtime.Serialization;
+using Newtonsoft.Json;
+using Xunit;
+
+namespace System.Text.Json.Serialization.Tests
+{
+    public static class DictionaryKeyPolicyTests
+    {
+        [Fact]
+        public static void CamelCaseDeserialize()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase
+            };
+
+            const string JsonString = @"[{""Key1"":1,""Key2"":2},{""Key1"":3,""Key2"":4}]";
+            Dictionary<string, int>[] obj = JsonSerializer.Deserialize<Dictionary<string, int>[]>(JsonString, options);
+
+            Assert.Equal(2, obj.Length);
+            Assert.Equal(1, obj[0]["key1"]);
+            Assert.Equal(2, obj[0]["key2"]);
+            Assert.Equal(3, obj[1]["key1"]);
+            Assert.Equal(4, obj[1]["key2"]);
+        }
+
+        [Fact]
+        public static void CamelCaseSerialize()
+        {
+            var options = new JsonSerializerOptions()
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase
+            };
+
+            Dictionary<string, int>[] obj = new Dictionary<string, int>[]
+            {
+                new Dictionary<string, int>() { { "Key1", 1 }, { "Key2", 2 } },
+                new Dictionary<string, int>() { { "Key1", 3 }, { "Key2", 4 } },
+            };
+
+            const string Json = @"[{""Key1"":1,""Key2"":2},{""Key1"":3,""Key2"":4}]";
+            const string JsonCamel = @"[{""key1"":1,""key2"":2},{""key1"":3,""key2"":4}]";
+
+            // 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()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = new UppercaseNamingPolicy()
+            };
+
+            Dictionary<string, int> obj = JsonSerializer.Deserialize<Dictionary<string, int>>(@"{""myint"":1}", options);
+            Assert.Equal(1, obj["MYINT"]);
+        }
+
+        [Fact]
+        public static void CustomNameSerialize()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = new UppercaseNamingPolicy()
+            };
+
+            Dictionary<string, int> obj = new Dictionary<string, int> { { "myint1", 1 }, { "myint2", 2 } };
+
+            const string Json = @"{""myint1"":1,""myint2"":2}";
+            const string JsonCustomKey = @"{""MYINT1"":1,""MYINT2"":2}";
+
+            // 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 honoring the custom key policy.
+            json = JsonSerializer.Serialize<object>(obj, options);
+            Assert.Equal(JsonCustomKey, json);
+        }
+
+        [Fact]
+        public static void NullNamePolicy()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = new NullNamingPolicy()
+            };
+
+            // A policy that returns null is not allowed.
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<Dictionary<string, int>>(@"{""onlyKey"": 1}", options));
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new Dictionary<string, int> { { "onlyKey", 1 } }, options));
+        }
+
+        [Fact]
+        public static void KeyConflictDeserialize_LastValueWins()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase
+            };
+
+            // The camel case policy resolves two keys to the same output key.
+            string json = @"{""myInt"":1,""MyInt"":2}";
+            Dictionary<string, int> obj = JsonSerializer.Deserialize<Dictionary<string, int>>(json, options);
+
+            // Check that the last value wins.
+            Assert.Equal(2, obj["myInt"]);
+            Assert.Equal(1, obj.Count);
+        }
+
+        [Fact]
+        public static void KeyConflictSerialize_WriteAll()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase
+            };
+
+            // The camel case policy resolves two keys to the same output key.
+            Dictionary<string, int> obj = new Dictionary<string, int> { { "myInt", 1 }, { "MyInt", 2 } };
+            string json = JsonSerializer.Serialize(obj, options);
+
+            // Check that we write all.
+            Assert.Equal(@"{""myInt"":1,""myInt"":2}", json);
+        }
+    }
+}

diff --git a/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs
index a4f907d..715c20a 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs
+++ b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs
@@ -688,29 +688,6 @@ namespace System.Text.Json.Serialization.Tests
         }
 
         [Fact]
-        public static void CamelCaseOption()
-        {
-            var options = new JsonSerializerOptions();
-            options.DictionaryKeyPolicy = JsonNamingPolicy.CamelCase;
-
-            const string JsonString = @"[{""Key1"":1,""Key2"":2},{""Key1"":3,""Key2"":4}]";
-            Dictionary<string, int>[] obj = JsonSerializer.Deserialize<Dictionary<string, int>[]>(JsonString, options);
-
-            Assert.Equal(2, obj.Length);
-            Assert.Equal(1, obj[0]["key1"]);
-            Assert.Equal(2, obj[0]["key2"]);
-            Assert.Equal(3, obj[1]["key1"]);
-            Assert.Equal(4, obj[1]["key2"]);
-
-            const string JsonCamel = @"[{""key1"":1,""key2"":2},{""key1"":3,""key2"":4}]";
-            string jsonCamel = JsonSerializer.Serialize<object>(obj);
-            Assert.Equal(JsonCamel, jsonCamel);
-
-            jsonCamel = JsonSerializer.Serialize<object>(obj, options);
-            Assert.Equal(JsonCamel, jsonCamel);
-        }
-
-        [Fact]
         public static void UnicodePropertyNames()
         {
             {

diff --git a/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs b/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs
index 016a896..0afe52f 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs
+++ b/src/libraries/System.Text.Json/tests/Serialization/PropertyNameTests.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Generic;
 using Xunit;
 
 namespace System.Text.Json.Serialization.Tests
@@ -64,6 +65,7 @@ namespace System.Text.Json.Serialization.Tests
 
             // A policy that returns null is not allowed.
             Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<SimpleTestClass>(@"{}", options));
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new SimpleTestClass(), options));
         }
 
         [Fact]
@@ -257,6 +259,21 @@ namespace System.Text.Json.Serialization.Tests
                 Assert.Equal(1, obj.Aѧ34567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890);
             }
         }
+
+        [Fact]
+        public static void ExtensionDataDictionarySerialize_DoesNotHonor()
+        {
+            var options = new JsonSerializerOptions
+            {
+                PropertyNamingPolicy = JsonNamingPolicy.CamelCase
+            };
+
+            EmptyClassWithExtensionProperty obj = JsonSerializer.Deserialize<EmptyClassWithExtensionProperty>(@"{""Key1"": 1}", options);
+
+            // Ignore naming policy for extension data properties by default.
+            Assert.False(obj.MyOverflow.ContainsKey("key1"));
+            Assert.Equal(1, obj.MyOverflow["Key1"].GetInt32());
+        }
     }
 
     public class OverridePropertyNameDesignTime_TestClass
@@ -316,4 +333,10 @@ namespace System.Text.Json.Serialization.Tests
             return null;
         }
     }
+
+    public class EmptyClassWithExtensionProperty
+    {
+        [JsonExtensionData]
+        public IDictionary<string, JsonElement> MyOverflow { get; set; }
+    }
 }

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj
index 5ec95fe..c436702 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj
@@ -51,6 +51,7 @@
     <Compile Include="Serialization\CustomConverterTests.Callback.cs" />
     <Compile Include="Serialization\CyclicTests.cs" />
     <Compile Include="Serialization\DictionaryTests.cs" />
+    <Compile Include="Serialization\DictionaryTests.KeyPolicy.cs" />
     <Compile Include="Serialization\EnumConverterTests.cs" />
     <Compile Include="Serialization\EnumTests.cs" />
     <Compile Include="Serialization\ExceptionTests.cs" />