Add DictionaryKeyPolicy support for EnumConverter [#47765] (#54429)
authorSychev Vadim <svddroid@gmail.com>
Tue, 3 Aug 2021 03:30:14 +0000 (06:30 +0300)
committerGitHub <noreply@github.com>
Tue, 3 Aug 2021 03:30:14 +0000 (20:30 -0700)
* Add DictionaryKeyPolicy support for EnumConverter [#47765]

* Moved DictionaryKeyPolicy parsing code to WriteWithQuotes [#47765]

* Refactored the bugfix in accordance to the comments (#47765)

* Made few minor corrections (#47765)

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs
src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Dictionary.KeyPolicy.cs

index 64e891b..b4e666b 100644 (file)
@@ -25,6 +25,8 @@ namespace System.Text.Json.Serialization.Converters
 
         private readonly ConcurrentDictionary<ulong, JsonEncodedText> _nameCache;
 
+        private ConcurrentDictionary<ulong, JsonEncodedText>? _dictionaryKeyPolicyCache;
+
         // 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.
         private const int NameCacheSizeSoftLimit = 64;
@@ -325,35 +327,82 @@ namespace System.Text.Json.Serialization.Converters
 
             ulong key = ConvertToUInt64(value);
 
-            if (_nameCache.TryGetValue(key, out JsonEncodedText formatted))
+            // Try to obtain values from caches
+            if (options.DictionaryKeyPolicy != null)
+            {
+                Debug.Assert(!state.Current.IgnoreDictionaryKeyPolicy);
+
+                if (_dictionaryKeyPolicyCache != null && _dictionaryKeyPolicyCache.TryGetValue(key, out JsonEncodedText formatted))
+                {
+                    writer.WritePropertyName(formatted);
+                    return;
+                }
+            }
+            else if (_nameCache.TryGetValue(key, out JsonEncodedText formatted))
             {
                 writer.WritePropertyName(formatted);
                 return;
             }
 
+
+            // if there are not cached values
             string original = value.ToString();
             if (IsValidIdentifier(original))
             {
-                // We are dealing with a combination of flag constants since
-                // all constant values were cached during warm-up.
-                JavaScriptEncoder? encoder = options.Encoder;
-
-                if (_nameCache.Count < NameCacheSizeSoftLimit)
+                if (options.DictionaryKeyPolicy != null)
                 {
-                    formatted = JsonEncodedText.Encode(original, encoder);
+                    original = options.DictionaryKeyPolicy.ConvertName(original);
 
-                    writer.WritePropertyName(formatted);
+                    if (original == null)
+                    {
+                        ThrowHelper.ThrowInvalidOperationException_NamingPolicyReturnNull(options.DictionaryKeyPolicy);
+                    }
+
+                    _dictionaryKeyPolicyCache ??= new ConcurrentDictionary<ulong, JsonEncodedText>();
+
+                    if (_dictionaryKeyPolicyCache.Count < NameCacheSizeSoftLimit)
+                    {
+                        JavaScriptEncoder? encoder = options.Encoder;
+
+                        JsonEncodedText formatted = JsonEncodedText.Encode(original, encoder);
+
+                        writer.WritePropertyName(formatted);
+
+                        _dictionaryKeyPolicyCache.TryAdd(key, formatted);
+                    }
+                    else
+                    {
+                        // We also do not create a JsonEncodedText instance here because passing the string
+                        // directly to the writer is cheaper than creating one and not caching it for reuse.
+                        writer.WritePropertyName(original);
+                    }
 
-                    _nameCache.TryAdd(key, formatted);
+                    return;
                 }
                 else
                 {
-                    // We also do not create a JsonEncodedText instance here because passing the string
-                    // directly to the writer is cheaper than creating one and not caching it for reuse.
-                    writer.WritePropertyName(original);
-                }
+                    // We might be dealing with a combination of flag constants since all constant values were
+                    // likely cached during warm - up(assuming the number of constants <= NameCacheSizeSoftLimit).
 
-                return;
+                    JavaScriptEncoder? encoder = options.Encoder;
+
+                    if (_nameCache.Count < NameCacheSizeSoftLimit)
+                    {
+                        JsonEncodedText formatted = JsonEncodedText.Encode(original, encoder);
+
+                        writer.WritePropertyName(formatted);
+
+                        _nameCache.TryAdd(key, formatted);
+                    }
+                    else
+                    {
+                        // We also do not create a JsonEncodedText instance here because passing the string
+                        // directly to the writer is cheaper than creating one and not caching it for reuse.
+                        writer.WritePropertyName(original);
+                    }
+
+                    return;
+                }
             }
 
             switch (s_enumTypeCode)
index 1a90f20..bb00474 100644 (file)
@@ -188,6 +188,101 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Equal(JsonCustomKey, json);
         }
 
+        public enum ETestEnum
+        {
+            TestValue1 = 1,
+            TestValue2 = 2,
+        }
+      
+        [Fact]
+        public static void EnumSerialization_DictionaryPolicy_Honored_CamelCase()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase,
+            };
+
+            Dictionary<ETestEnum, ETestEnum> dict = new Dictionary<ETestEnum, ETestEnum> { [ETestEnum.TestValue1] = ETestEnum.TestValue1 };
+            string value = JsonSerializer.Serialize(dict, options);
+            Assert.Equal("{\"testValue1\":1}", value);
+
+            dict = new Dictionary<ETestEnum, ETestEnum> { [ETestEnum.TestValue2] = ETestEnum.TestValue2 };
+            value = JsonSerializer.Serialize(dict, options);
+            Assert.Equal("{\"testValue2\":2}", value);
+
+            dict = new Dictionary<ETestEnum, ETestEnum> { [ETestEnum.TestValue1] = ETestEnum.TestValue1, [ETestEnum.TestValue2] = ETestEnum.TestValue2 };
+            value = JsonSerializer.Serialize(dict, options);
+            Assert.Equal("{\"testValue1\":1,\"testValue2\":2}", value);
+        }
+
+        [Fact]
+        public static void EnumSerializationAsDictKey_NoDictionaryKeyPolicy()
+        {
+            Dictionary<ETestEnum, ETestEnum> dict = new Dictionary<ETestEnum, ETestEnum> { [ETestEnum.TestValue1] = ETestEnum.TestValue1 };
+            string value = JsonSerializer.Serialize(dict);
+            Assert.Equal("{\"TestValue1\":1}", value);
+
+            dict = new Dictionary<ETestEnum, ETestEnum> { [ETestEnum.TestValue2] = ETestEnum.TestValue2 };
+            value = JsonSerializer.Serialize(dict);
+            Assert.Equal("{\"TestValue2\":2}", value);
+
+            dict = new Dictionary<ETestEnum, ETestEnum> { [ETestEnum.TestValue1] = ETestEnum.TestValue1, [ETestEnum.TestValue2] = ETestEnum.TestValue2 };
+            value = JsonSerializer.Serialize(dict);
+            Assert.Equal("{\"TestValue1\":1,\"TestValue2\":2}", value);
+        }
+
+        public class ClassWithEnumProperties
+        {
+            public ETestEnum TestEnumProperty1 { get; } = ETestEnum.TestValue2;
+            public DayOfWeek TestEnumProperty2 { get; } = DayOfWeek.Monday;
+        }
+
+        [Fact]
+        public static void EnumSerialization_DictionaryPolicy_NotApplied_WhenEnumsAreSerialized()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase,
+            };
+
+            string value = JsonSerializer.Serialize(DayOfWeek.Friday, options);
+
+            Assert.Equal("5", value);
+
+            value = JsonSerializer.Serialize(ETestEnum.TestValue2, options);
+
+            Assert.Equal("2", value);
+
+
+            value = JsonSerializer.Serialize(new ClassWithEnumProperties(), options);
+
+            Assert.Equal("{\"TestEnumProperty1\":2,\"TestEnumProperty2\":1}", value);
+
+            value = JsonSerializer.Serialize(new List<DayOfWeek> { DayOfWeek.Sunday, DayOfWeek.Monday, DayOfWeek.Tuesday, DayOfWeek.Wednesday, DayOfWeek.Thursday, DayOfWeek.Friday, DayOfWeek.Saturday}, options);
+
+            Assert.Equal("[0,1,2,3,4,5,6]", value);
+        }
+
+        public class CustomJsonNamingPolicy : JsonNamingPolicy
+        {
+            public override string ConvertName(string name) => null;
+        }
+
+        [Fact]
+        public static void EnumSerialization_DictionaryPolicy_ThrowsException_WhenNamingPolicyReturnsNull()
+        {
+            var options = new JsonSerializerOptions
+            {
+                DictionaryKeyPolicy = new CustomJsonNamingPolicy(),
+            };
+
+            Dictionary<ETestEnum, ETestEnum> dict = new Dictionary<ETestEnum, ETestEnum> { [ETestEnum.TestValue1] = ETestEnum.TestValue1 };
+
+            InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(dict, options));
+
+            Assert.Contains(typeof(CustomJsonNamingPolicy).ToString(), ex.Message);
+        }
+
         [Fact]
         public async Task NullNamePolicy()
         {