From 2a7699d12692176a00de0d3892960d383a13e2c5 Mon Sep 17 00:00:00 2001 From: Dmitry Vlasov Date: Wed, 16 Sep 2020 12:43:13 +0500 Subject: [PATCH 1/5] fix reading json with naming policy --- .../Converters/Value/EnumConverter.cs | 13 +++ .../EnumConverterWithNamingPolicyTests.cs | 96 +++++++++++++++++++ .../tests/System.Text.Json.Tests.csproj | 1 + 3 files changed, 110 insertions(+) create mode 100644 src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 109ecd539a95b..6101da87ecedd 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -24,6 +24,9 @@ internal class EnumConverter : JsonConverter private readonly JsonNamingPolicy? _namingPolicy; private readonly ConcurrentDictionary _nameCache; + private readonly ConcurrentDictionary _sourceNameCache; + private readonly bool _needRestoreSourceName; + private readonly JsonSerializerOptions _serializerOptions; // This is used to prevent flooding the cache due to exponential bitwise combinations of flags. // Since multiple threads can add to the cache, a few more values might be added. @@ -43,7 +46,9 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na { _converterOptions = converterOptions; _namingPolicy = namingPolicy; + _serializerOptions = serializerOptions; _nameCache = new ConcurrentDictionary(); + _sourceNameCache = new ConcurrentDictionary(); string[] names = Enum.GetNames(TypeToConvert); Array values = Enum.GetValues(TypeToConvert); @@ -67,7 +72,12 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na namingPolicy == null ? JsonEncodedText.Encode(name, encoder) : FormatEnumValue(name, encoder)); + + if (namingPolicy != null) + _sourceNameCache.TryAdd(FormatEnumValue(name, encoder), name); } + + _needRestoreSourceName = namingPolicy != null; } public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) @@ -313,6 +323,9 @@ internal override T ReadWithQuotes(ref Utf8JsonReader reader) { string? enumString = reader.GetString(); + if (_needRestoreSourceName && enumString != null) + _sourceNameCache.TryGetValue(FormatEnumValue(enumString, _serializerOptions.Encoder), out enumString); + // Try parsing case sensitive first if (!Enum.TryParse(enumString, out T value) && !Enum.TryParse(enumString, ignoreCase: true, out value)) diff --git a/src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs new file mode 100644 index 0000000000000..89bc6c262d6d9 --- /dev/null +++ b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs @@ -0,0 +1,96 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace System.Text.Json.Serialization.Tests +{ + public class EnumConverterWithNamingPolicyTests + { + private readonly ITestOutputHelper _outputHelper; + + public EnumConverterWithNamingPolicyTests(ITestOutputHelper outputHelper) + { + _outputHelper = outputHelper; + } + + public class SnakeCaseNamingPolicy : JsonNamingPolicy + { + public override string ConvertName(string name) + { + if (name == null) + { + throw new ArgumentNullException(nameof(name)); + } + var result = new StringBuilder(); + for (var i = 0; i < name.Length; i++) + { + var c = name[i]; + if (i == 0) + { + result.Append(char.ToLower(c)); + } + else + { + if (char.IsUpper(c)) + { + result.Append('_'); + result.Append(char.ToLower(c)); + } + else + { + result.Append(c); + } + } + } + return result.ToString(); + } + + } + public enum TestType + { + None, + ValueOne, + ValueTwo, + } + + public class ObjectWithEnumProperty + { + public TestType TestType { get; set; } + } + + [Fact] + public void TestEnumCase() + { + var namingPolicy = new SnakeCaseNamingPolicy(); + + var opts = new JsonSerializerOptions() + { + PropertyNamingPolicy = namingPolicy, + DictionaryKeyPolicy = namingPolicy, + Converters = + { + new JsonStringEnumConverter(namingPolicy) + } + }; + + var enumValues = Enum.GetValues(typeof(TestType)).Cast(); + foreach (var v in enumValues) + { + var sourceObject = new ObjectWithEnumProperty() + { + TestType = v + }; + + var json = JsonSerializer.Serialize(sourceObject, opts); + _outputHelper.WriteLine(json); + var deserializedObject = JsonSerializer.Deserialize(json, opts); + Assert.Equal(sourceObject.TestType, deserializedObject.TestType); + } + } + } +} 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 6e1e66c690248..01444d4da0670 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests.csproj @@ -88,6 +88,7 @@ + From b8a66770f205691745445890509c0f06537313a9 Mon Sep 17 00:00:00 2001 From: Dmitry Vlasov Date: Thu, 17 Sep 2020 17:34:23 +0500 Subject: [PATCH 2/5] add test case for flags --- .../Converters/Value/EnumConverter.cs | 39 ++++++++++++++----- .../EnumConverterWithNamingPolicyTests.cs | 33 ++++++++++++++++ 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 6101da87ecedd..a2bbf59c3cd8d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -24,9 +24,8 @@ internal class EnumConverter : JsonConverter private readonly JsonNamingPolicy? _namingPolicy; private readonly ConcurrentDictionary _nameCache; - private readonly ConcurrentDictionary _sourceNameCache; + private ConcurrentDictionary? _sourceNameCache; private readonly bool _needRestoreSourceName; - private readonly JsonSerializerOptions _serializerOptions; // This is used to prevent flooding the cache due to exponential bitwise combinations of flags. // Since multiple threads can add to the cache, a few more values might be added. @@ -46,9 +45,7 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na { _converterOptions = converterOptions; _namingPolicy = namingPolicy; - _serializerOptions = serializerOptions; _nameCache = new ConcurrentDictionary(); - _sourceNameCache = new ConcurrentDictionary(); string[] names = Enum.GetNames(TypeToConvert); Array values = Enum.GetValues(TypeToConvert); @@ -74,7 +71,12 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na : FormatEnumValue(name, encoder)); if (namingPolicy != null) - _sourceNameCache.TryAdd(FormatEnumValue(name, encoder), name); + { + if(_sourceNameCache == null) + _sourceNameCache = new ConcurrentDictionary(); + + _sourceNameCache.TryAdd(FormatEnumValueToString(name, null), name); + } } _needRestoreSourceName = namingPolicy != null; @@ -288,14 +290,33 @@ private JsonEncodedText FormatEnumValue(string value, JavaScriptEncoder? encoder return JsonEncodedText.Encode(formatted, encoder); } - private string FormatEnumValueToString(string value, JavaScriptEncoder? encoder) + private string FormatNamingPolicy(string value, bool isWrite) + { + Debug.Assert(_namingPolicy != null); + + if (isWrite) + { + return _namingPolicy.ConvertName(value); + } + else + { + if (_sourceNameCache.ContainsKey(value)) + return _sourceNameCache[value]; + + ThrowHelper.ThrowJsonException(); + } + + return default; + } + + private string FormatEnumValueToString(string value, JavaScriptEncoder? encoder, bool isWrite = true) { Debug.Assert(_namingPolicy != null); string converted; if (!value.Contains(ValueSeparator)) { - converted = _namingPolicy.ConvertName(value); + converted = FormatNamingPolicy(value, isWrite); } else { @@ -310,7 +331,7 @@ private string FormatEnumValueToString(string value, JavaScriptEncoder? encoder) for (int i = 0; i < enumValues.Length; i++) { - enumValues[i] = _namingPolicy.ConvertName(enumValues[i]); + enumValues[i] = FormatNamingPolicy(enumValues[i], isWrite); } converted = string.Join(ValueSeparator, enumValues); @@ -324,7 +345,7 @@ internal override T ReadWithQuotes(ref Utf8JsonReader reader) string? enumString = reader.GetString(); if (_needRestoreSourceName && enumString != null) - _sourceNameCache.TryGetValue(FormatEnumValue(enumString, _serializerOptions.Encoder), out enumString); + enumString = FormatEnumValueToString(enumString, null, false); // Try parsing case sensitive first if (!Enum.TryParse(enumString, out T value) diff --git a/src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs index 89bc6c262d6d9..c26948d00656b 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/EnumConverterWithNamingPolicyTests.cs @@ -51,6 +51,8 @@ public override string ConvertName(string name) } } + + [Flags] public enum TestType { None, @@ -92,5 +94,36 @@ public void TestEnumCase() Assert.Equal(sourceObject.TestType, deserializedObject.TestType); } } + + + [Fact] + public void TestFlagsEnumValuesWithNamingPolicy() + { + var namingPolicy = new SnakeCaseNamingPolicy(); + + var opts = new JsonSerializerOptions() + { + PropertyNamingPolicy = namingPolicy, + DictionaryKeyPolicy = namingPolicy, + Converters = + { + new JsonStringEnumConverter(namingPolicy) + } + }; + + var sourceObject = new ObjectWithEnumProperty() + { + TestType = TestType.ValueOne | TestType.ValueTwo + }; + + var json = JsonSerializer.Serialize(sourceObject, opts); + _outputHelper.WriteLine(json); + + Assert.Equal(@"{""test_type"":""value_one, value_two""}", json); + + var restoredObject = JsonSerializer.Deserialize(json, opts); + + Assert.Equal(sourceObject.TestType, restoredObject.TestType); + } } } From 098cb5d2d76c8e9582cb7f52f3996b61c68ccdf6 Mon Sep 17 00:00:00 2001 From: Dmitry Vlasov Date: Thu, 17 Sep 2020 18:53:55 +0500 Subject: [PATCH 3/5] fix warnings --- .../Text/Json/Serialization/Converters/Value/EnumConverter.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index a2bbf59c3cd8d..265fe7a4e312c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -72,7 +72,7 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na if (namingPolicy != null) { - if(_sourceNameCache == null) + if (_sourceNameCache == null) _sourceNameCache = new ConcurrentDictionary(); _sourceNameCache.TryAdd(FormatEnumValueToString(name, null), name); @@ -293,6 +293,7 @@ private JsonEncodedText FormatEnumValue(string value, JavaScriptEncoder? encoder private string FormatNamingPolicy(string value, bool isWrite) { Debug.Assert(_namingPolicy != null); + Debug.Assert(_sourceNameCache != null); if (isWrite) { From 7c8642181c236baee9b0b675c049d70111bbcb4a Mon Sep 17 00:00:00 2001 From: Dmitry Vlasov Date: Thu, 17 Sep 2020 19:03:35 +0500 Subject: [PATCH 4/5] fix debug asserts --- .../Json/Serialization/Converters/Value/EnumConverter.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 265fe7a4e312c..4051b8b626e85 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -292,15 +292,14 @@ private JsonEncodedText FormatEnumValue(string value, JavaScriptEncoder? encoder private string FormatNamingPolicy(string value, bool isWrite) { - Debug.Assert(_namingPolicy != null); - Debug.Assert(_sourceNameCache != null); - if (isWrite) { + Debug.Assert(_namingPolicy != null); return _namingPolicy.ConvertName(value); } else { + Debug.Assert(_sourceNameCache != null); if (_sourceNameCache.ContainsKey(value)) return _sourceNameCache[value]; From 04cbc89d0327f7bac5e31f94142e2cd8eed48a87 Mon Sep 17 00:00:00 2001 From: Dmitry Vlasov Date: Thu, 17 Sep 2020 21:31:09 +0500 Subject: [PATCH 5/5] use check cache initialized --- .../Json/Serialization/Converters/Value/EnumConverter.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 4051b8b626e85..b844bd6a731de 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -25,7 +25,6 @@ internal class EnumConverter : JsonConverter private readonly ConcurrentDictionary _nameCache; private ConcurrentDictionary? _sourceNameCache; - private readonly bool _needRestoreSourceName; // This is used to prevent flooding the cache due to exponential bitwise combinations of flags. // Since multiple threads can add to the cache, a few more values might be added. @@ -78,8 +77,6 @@ public EnumConverter(EnumConverterOptions converterOptions, JsonNamingPolicy? na _sourceNameCache.TryAdd(FormatEnumValueToString(name, null), name); } } - - _needRestoreSourceName = namingPolicy != null; } public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) @@ -344,7 +341,7 @@ internal override T ReadWithQuotes(ref Utf8JsonReader reader) { string? enumString = reader.GetString(); - if (_needRestoreSourceName && enumString != null) + if (_sourceNameCache != null && enumString != null) enumString = FormatEnumValueToString(enumString, null, false); // Try parsing case sensitive first