From fa14ce5fc698bb1d7ec3c268f415d53c82767f66 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Thu, 31 Oct 2019 19:27:18 -0700 Subject: [PATCH] Honor dictionary key policy when entry value is null (dotnet/corefx#42267) * Honor dictionary key policy when entry value is null * Address review feedback * Remove reverse Json checks Commit migrated from https://github.com/dotnet/corefx/commit/a24db0ba5666f46f50649810fe698ad267b60331 --- .../Json/Serialization/JsonPropertyInfoNullable.cs | 25 +++++--- .../JsonSerializer.Write.HandleDictionary.cs | 25 ++++---- .../Serialization/DictionaryTests.KeyPolicy.cs | 73 ++++++++++++++++++++++ 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNullable.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNullable.cs index 98b88aa..3605750 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNullable.cs +++ b/src/libraries/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 { @@ -109,22 +110,26 @@ namespace System.Text.Json 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/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 d221a5d..d39ea99 100644 --- 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 @@ -145,23 +145,26 @@ namespace System.Text.Json 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/libraries/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs index 4aaeb18..47f1595 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.KeyPolicy.cs @@ -47,6 +47,29 @@ namespace System.Text.Json.Serialization.Tests } [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(@"{""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 ExtensionData { get; set; } + } + + [Fact] public static void CamelCaseSerialize() { var options = new JsonSerializerOptions() @@ -73,6 +96,56 @@ namespace System.Text.Json.Serialization.Tests } [Fact] + public static void CamelCaseSerialize_Null_Values() + { + var options = new JsonSerializerOptions() + { + DictionaryKeyPolicy = JsonNamingPolicy.CamelCase // e.g. Key1 -> key1. + }; + + Dictionary[] obj = new Dictionary[] + { + new Dictionary() { { "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(obj); + Assert.Equal(Json, json); + + // With key policy option, serialize keys with camel casing. + json = JsonSerializer.Serialize(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[] obj = new Dictionary[] + { + new Dictionary() { { "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(obj); + Assert.Equal(Json, json); + + // With key policy option, serialize keys with camel casing. + json = JsonSerializer.Serialize(obj, options); + Assert.Equal(JsonCamel, json); + } + + [Fact] public static void CustomNameDeserialize() { var options = new JsonSerializerOptions -- 2.7.4