Honor custom number handling only when property/type is a number/collection of number...

* Honor custom number handling only when property/type is a number/collection of numbers

* Verify typeof(object) + custom number handling behavior

* Simplify setting of per-property number handling

* Clean up number-handling-applicable check

* Fix issue with number-handling + boxed non-number types

diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx
index 4562c9a..a14cc26 100644 (file)
--- a/src/libraries/System.Text.Json/src/Resources/Strings.resx
+++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx
@@ -540,7 +540,7 @@
     <value>'JsonNumberHandlingAttribute' cannot be placed on a property, field, or type that is handled by a custom converter. See usage(s) of converter '{0}' on type '{1}'.</value>
   </data>
   <data name="NumberHandlingOnPropertyTypeMustBeNumberOrCollection" xml:space="preserve">
-    <value>When 'JsonNumberHandlingAttribute' is placed on a property or field, the property or field must be a number or a collection. See member '{0}' on type '{1}'.</value>
+    <value>When 'JsonNumberHandlingAttribute' is placed on a property or field, the property or field must be a number or a collection of numbers. See member '{0}' on type '{1}'.</value>
   </data>
   <data name="ConverterCanConvertNullableRedundant" xml:space="preserve">
     <value>The converter '{0}' handles type '{1}' but is being asked to convert type '{2}'. Either create a separate converter for type '{2}' or change the converter's 'CanConvert' method to only return 'true' for a single type.</value>

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs
index a4a1d34..a9feb74 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ObjectConverter.cs
@@ -5,6 +5,11 @@ namespace System.Text.Json.Serialization.Converters
 {
     internal sealed class ObjectConverter : JsonConverter<object>
     {
+        public ObjectConverter()
+        {
+            IsInternalConverterForNumberType = true;
+        }
+
         public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
             using (JsonDocument document = JsonDocument.ParseValue(ref reader))
@@ -37,5 +42,13 @@ namespace System.Text.Json.Serialization.Converters
 
             return runtimeConverter;
         }
+
+        internal override object ReadNumberWithCustomHandling(ref Utf8JsonReader reader, JsonNumberHandling handling)
+        {
+            using (JsonDocument document = JsonDocument.ParseValue(ref reader))
+            {
+                return document.RootElement.Clone();
+            }
+        }
     }
 }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
index 9ce34a8..38bd9cb 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
@@ -163,7 +163,7 @@ namespace System.Text.Json.Serialization
                 // For performance, only perform validation on internal converters on debug builds.
                 if (IsInternalConverter)
                 {
-                    if (IsInternalConverterForNumberType && state.Current.NumberHandling != null)
+                    if (state.Current.NumberHandling != null)
                     {
                         value = ReadNumberWithCustomHandling(ref reader, state.Current.NumberHandling.Value);
                     }
@@ -179,7 +179,7 @@ namespace System.Text.Json.Serialization
                     int originalPropertyDepth = reader.CurrentDepth;
                     long originalPropertyBytesConsumed = reader.BytesConsumed;
 
-                    if (IsInternalConverterForNumberType && state.Current.NumberHandling != null)
+                    if (state.Current.NumberHandling != null)
                     {
                         value = ReadNumberWithCustomHandling(ref reader, state.Current.NumberHandling.Value);
                     }
@@ -356,7 +356,7 @@ namespace System.Text.Json.Serialization
 
                 int originalPropertyDepth = writer.CurrentDepth;
 
-                if (IsInternalConverterForNumberType && state.Current.NumberHandling != null)
+                if (state.Current.NumberHandling != null && IsInternalConverterForNumberType)
                 {
                     WriteNumberWithCustomHandling(writer, value, state.Current.NumberHandling.Value);
                 }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
index 10b92e2..4d7d9f4 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
@@ -184,6 +184,8 @@ namespace System.Text.Json
 
         private void DetermineNumberHandling(JsonNumberHandling? parentTypeNumberHandling)
         {
+            bool numberHandlingIsApplicable = ConverterBase.IsInternalConverterForNumberType || TypeIsCollectionOfNumbersWithInternalConverter();
+
             if (IsForClassInfo)
             {
                 if (parentTypeNumberHandling != null && !ConverterBase.IsInternalConverter)
@@ -191,45 +193,80 @@ namespace System.Text.Json
                     ThrowHelper.ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(this);
                 }
 
-                // Priority 1: Get handling from the type (parent type in this case is the type itself).
-                NumberHandling = parentTypeNumberHandling;
-
-                // Priority 2: Get handling from JsonSerializerOptions instance.
-                if (!NumberHandling.HasValue && Options.NumberHandling != JsonNumberHandling.Strict)
+                if (numberHandlingIsApplicable)
                 {
-                    NumberHandling = Options.NumberHandling;
+                    // This logic is to honor JsonNumberHandlingAttribute placed on
+                    // custom collections e.g. public class MyNumberList : List<int>.
+
+                    // Priority 1: Get handling from the type (parent type in this case is the type itself).
+                    NumberHandling = parentTypeNumberHandling;
+
+                    // Priority 2: Get handling from JsonSerializerOptions instance.
+                    if (!NumberHandling.HasValue && Options.NumberHandling != JsonNumberHandling.Strict)
+                    {
+                        NumberHandling = Options.NumberHandling;
+                    }
                 }
             }
             else
             {
-                JsonNumberHandling? handling = null;
+                Debug.Assert(MemberInfo != null);
+
+                JsonNumberHandlingAttribute? attribute = GetAttribute<JsonNumberHandlingAttribute>(MemberInfo);
+                if (attribute != null && !numberHandlingIsApplicable)
+                {
+                    ThrowHelper.ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(this);
+                }
 
-                // Priority 1: Get handling from attribute on property or field.
-                if (MemberInfo != null)
+                if (numberHandlingIsApplicable)
                 {
-                    JsonNumberHandlingAttribute? attribute = GetAttribute<JsonNumberHandlingAttribute>(MemberInfo);
+                    // Priority 1: Get handling from attribute on property or field.
+                    JsonNumberHandling? handling = attribute?.Handling;
+
+                    // Priority 2: Get handling from attribute on parent class type.
+                    handling ??= parentTypeNumberHandling;
 
-                    if (attribute != null &&
-                        !ConverterBase.IsInternalConverterForNumberType &&
-                        ((ClassType.Enumerable | ClassType.Dictionary) & ClassType) == 0)
+                    // Priority 3: Get handling from JsonSerializerOptions instance.
+                    if (!handling.HasValue && Options.NumberHandling != JsonNumberHandling.Strict)
                     {
-                        ThrowHelper.ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(this);
+                        handling = Options.NumberHandling;
                     }
 
-                    handling = attribute?.Handling;
+                    NumberHandling = handling;
                 }
+            }
+        }
 
-                // Priority 2: Get handling from attribute on parent class type.
-                handling ??= parentTypeNumberHandling;
-
-                // Priority 3: Get handling from JsonSerializerOptions instance.
-                if (!handling.HasValue && Options.NumberHandling != JsonNumberHandling.Strict)
-                {
-                    handling = Options.NumberHandling;
-                }
+        private bool TypeIsCollectionOfNumbersWithInternalConverter()
+        {
+            if (!ConverterBase.IsInternalConverter ||
+                ((ClassType.Enumerable | ClassType.Dictionary) & ClassType) == 0)
+            {
+                return false;
+            }
 
-                NumberHandling = handling;
+            Type? elementType = ConverterBase.ElementType;
+            Debug.Assert(elementType != null);
+
+            elementType = Nullable.GetUnderlyingType(elementType) ?? elementType;
+
+            if (elementType == typeof(byte) ||
+                elementType == typeof(decimal) ||
+                elementType == typeof(double) ||
+                elementType == typeof(short) ||
+                elementType == typeof(int) ||
+                elementType == typeof(long) ||
+                elementType == typeof(sbyte) ||
+                elementType == typeof(float) ||
+                elementType == typeof(ushort) ||
+                elementType == typeof(uint) ||
+                elementType == typeof(ulong) ||
+                elementType == JsonClassInfo.ObjectType)
+            {
+                return true;
             }
+
+            return false;
         }
 
         public static TAttribute? GetAttribute<TAttribute>(MemberInfo memberInfo) where TAttribute : Attribute

diff --git a/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs b/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs
index 60acb54..f306e3a 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs
+++ b/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs
@@ -126,7 +126,7 @@ namespace System.Text.Json.Serialization.Tests
         }
 
         [Fact]
-        public static void Number_AsBoxedRootType()
+        public static void Number_AsBoxed_RootType()
         {
             string numberAsString = @"""2""";
 
@@ -147,6 +147,216 @@ namespace System.Text.Json.Serialization.Tests
         }
 
         [Fact]
+        public static void Number_AsBoxed_Property()
+        {
+            int @int = 1;
+            float? nullableFloat = 2;
+
+            string expected = @"{""MyInt"":""1"",""MyNullableFloat"":""2""}";
+
+            var obj = new Class_With_BoxedNumbers
+            {
+                MyInt = @int,
+                MyNullableFloat = nullableFloat
+            };
+
+            string serialized = JsonSerializer.Serialize(obj);
+            JsonTestHelper.AssertJsonEqual(expected, serialized);
+
+            obj = JsonSerializer.Deserialize<Class_With_BoxedNumbers>(serialized);
+
+            JsonElement el = Assert.IsType<JsonElement>(obj.MyInt);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal("1", el.GetString());
+
+            el = Assert.IsType<JsonElement>(obj.MyNullableFloat);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal("2", el.GetString());
+        }
+
+        public class Class_With_BoxedNumbers
+        {
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public object MyInt { get; set; }
+
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public object MyNullableFloat { get; set; }
+        }
+
+        [Fact]
+        public static void Number_AsBoxed_CollectionRootType_Element()
+        {
+            int @int = 1;
+            float? nullableFloat = 2;
+
+            string expected = @"[""1""]";
+
+            var obj = new List<object> { @int };
+            string serialized = JsonSerializer.Serialize(obj, s_optionReadAndWriteFromStr);
+            Assert.Equal(expected, serialized);
+
+            obj = JsonSerializer.Deserialize<List<object>>(serialized, s_optionReadAndWriteFromStr);
+
+            JsonElement el = Assert.IsType<JsonElement>(obj[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal("1", el.GetString());
+
+            expected = @"[""2""]";
+
+            IList obj2 = new object[] { nullableFloat };
+            serialized = JsonSerializer.Serialize(obj2, s_optionReadAndWriteFromStr);
+            Assert.Equal(expected, serialized);
+
+            obj2 = JsonSerializer.Deserialize<IList>(serialized, s_optionReadAndWriteFromStr);
+
+            el = Assert.IsType<JsonElement>(obj2[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal("2", el.GetString());
+        }
+
+        [Fact]
+        public static void Number_AsBoxed_CollectionProperty_Element()
+        {
+            int @int = 2;
+            float? nullableFloat = 2;
+
+            string expected = @"{""MyInts"":[""2""],""MyNullableFloats"":[""2""]}";
+
+            var obj = new Class_With_ListsOfBoxedNumbers
+            {
+                MyInts = new List<object> { @int },
+                MyNullableFloats = new object[] { nullableFloat }
+            };
+
+            string serialized = JsonSerializer.Serialize(obj);
+            JsonTestHelper.AssertJsonEqual(expected, serialized);
+
+            obj = JsonSerializer.Deserialize<Class_With_ListsOfBoxedNumbers>(serialized);
+
+            JsonElement el = Assert.IsType<JsonElement>(obj.MyInts[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal("2", el.GetString());
+
+            el = Assert.IsType<JsonElement>(obj.MyNullableFloats[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal("2", el.GetString());
+        }
+
+        public class Class_With_ListsOfBoxedNumbers
+        {
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public List<object> MyInts { get; set; }
+
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public IList MyNullableFloats { get; set; }
+        }
+
+        [Fact]
+        public static void NonNumber_AsBoxed_Property()
+        {
+            DateTime dateTime = DateTime.Now;
+            Guid? nullableGuid = Guid.NewGuid();
+
+            string expected = @$"{{""MyDateTime"":{JsonSerializer.Serialize(dateTime)},""MyNullableGuid"":{JsonSerializer.Serialize(nullableGuid)}}}";
+
+            var obj = new Class_With_BoxedNonNumbers
+            {
+                MyDateTime = dateTime,
+                MyNullableGuid = nullableGuid
+            };
+
+            string serialized = JsonSerializer.Serialize(obj);
+            JsonTestHelper.AssertJsonEqual(expected, serialized);
+
+            obj = JsonSerializer.Deserialize<Class_With_BoxedNonNumbers>(serialized);
+
+            JsonElement el = Assert.IsType<JsonElement>(obj.MyDateTime);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal(dateTime, el.GetDateTime());
+
+            el = Assert.IsType<JsonElement>(obj.MyNullableGuid);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal(nullableGuid.Value, el.GetGuid());
+        }
+
+        public class Class_With_BoxedNonNumbers
+        {
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public object MyDateTime { get; set; }
+
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public object MyNullableGuid { get; set; }
+        }
+
+        [Fact]
+        public static void NonNumber_AsBoxed_CollectionRootType_Element()
+        {
+            DateTime dateTime = DateTime.Now;
+            Guid? nullableGuid = Guid.NewGuid();
+
+            string expected = @$"[{JsonSerializer.Serialize(dateTime)}]";
+
+            var obj = new List<object> { dateTime };
+            string serialized = JsonSerializer.Serialize(obj, s_optionReadAndWriteFromStr);
+            Assert.Equal(expected, serialized);
+
+            obj = JsonSerializer.Deserialize<List<object>>(serialized, s_optionReadAndWriteFromStr);
+
+            JsonElement el = Assert.IsType<JsonElement>(obj[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal(dateTime, el.GetDateTime());
+
+            expected = @$"[{JsonSerializer.Serialize(nullableGuid)}]";
+
+            IList obj2 = new object[] { nullableGuid };
+            serialized = JsonSerializer.Serialize(obj2, s_optionReadAndWriteFromStr);
+            Assert.Equal(expected, serialized);
+
+            obj2 = JsonSerializer.Deserialize<IList>(serialized, s_optionReadAndWriteFromStr);
+
+            el = Assert.IsType<JsonElement>(obj2[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal(nullableGuid.Value, el.GetGuid());
+        }
+
+        [Fact]
+        public static void NonNumber_AsBoxed_CollectionProperty_Element()
+        {
+            DateTime dateTime = DateTime.Now;
+            Guid? nullableGuid = Guid.NewGuid();
+
+            string expected = @$"{{""MyDateTimes"":[{JsonSerializer.Serialize(dateTime)}],""MyNullableGuids"":[{JsonSerializer.Serialize(nullableGuid)}]}}";
+
+            var obj = new Class_With_ListsOfBoxedNonNumbers
+            {
+                MyDateTimes = new List<object> { dateTime },
+                MyNullableGuids = new object[] { nullableGuid }
+            };
+
+            string serialized = JsonSerializer.Serialize(obj);
+            JsonTestHelper.AssertJsonEqual(expected, serialized);
+
+            obj = JsonSerializer.Deserialize<Class_With_ListsOfBoxedNonNumbers>(serialized);
+
+            JsonElement el = Assert.IsType<JsonElement>(obj.MyDateTimes[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal(dateTime, el.GetDateTime());
+
+            el = Assert.IsType<JsonElement>(obj.MyNullableGuids[0]);
+            Assert.Equal(JsonValueKind.String, el.ValueKind);
+            Assert.Equal(nullableGuid, el.GetGuid());
+        }
+
+        public class Class_With_ListsOfBoxedNonNumbers
+        {
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public List<object> MyDateTimes { get; set; }
+
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
+            public IList MyNullableGuids { get; set; }
+        }
+
+        [Fact]
         public static void Number_AsCollectionElement_RoundTrip()
         {
             RunAsCollectionElementTest(JsonNumberTestData.Bytes);
@@ -1102,26 +1312,46 @@ namespace System.Text.Json.Serialization.Tests
         }
 
         [Fact]
-        public static void NestedCollectionElementTypeHandling_Overrides_ParentPropertyHandling()
+        public static void NestedCollectionElementTypeHandling_Overrides_GlobalOption()
         {
             // Strict policy on the collection element type overrides read-as-string on the collection property
             string json = @"{""MyList"":[{""Float"":""1""}]}";
-            Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<ClassWithComplexListProperty>(json));
+            Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<ClassWithComplexListProperty>(json, s_optionReadAndWriteFromStr));
 
             // Strict policy on the collection element type overrides write-as-string on the collection property
             var obj = new ClassWithComplexListProperty
             {
                 MyList = new List<ClassWith_StrictAttribute> { new ClassWith_StrictAttribute { Float = 1 } }
             };
-            Assert.Equal(@"{""MyList"":[{""Float"":1}]}", JsonSerializer.Serialize(obj));
+            Assert.Equal(@"{""MyList"":[{""Float"":1}]}", JsonSerializer.Serialize(obj, s_optionReadAndWriteFromStr));
         }
 
         public class ClassWithComplexListProperty
         {
+            public List<ClassWith_StrictAttribute> MyList { get; set; }
+        }
+
+        [Fact]
+        public static void NumberHandlingAttribute_NotAllowedOn_CollectionOfNonNumbers()
+        {
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<ClassWith_AttributeOnComplexListProperty>(""));
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new ClassWith_AttributeOnComplexListProperty()));
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<ClassWith_AttributeOnComplexDictionaryProperty>(""));
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(new ClassWith_AttributeOnComplexDictionaryProperty()));
+        }
+
+        public class ClassWith_AttributeOnComplexListProperty
+        {
             [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)]
             public List<ClassWith_StrictAttribute> MyList { get; set; }
         }
 
+        public class ClassWith_AttributeOnComplexDictionaryProperty
+        {
+            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString)]
+            public Dictionary<string, ClassWith_StrictAttribute> MyDictionary { get; set; }
+        }
+
         [Fact]
         public static void MemberAttributeAppliesToDictionary_SimpleElements()
         {
@@ -1136,23 +1366,22 @@ namespace System.Text.Json.Serialization.Tests
         }
 
         [Fact]
-        public static void NestedDictionaryElementTypeHandling_Overrides_ParentPropertyHandling()
+        public static void NestedDictionaryElementTypeHandling_Overrides_GlobalOption()
         {
             // Strict policy on the dictionary element type overrides read-as-string on the collection property.
             string json = @"{""MyDictionary"":{""Key"":{""Float"":""1""}}}";
-            Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<ClassWithComplexDictionaryProperty>(json));
+            Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<ClassWithComplexDictionaryProperty>(json, s_optionReadFromStr));
 
             // Strict policy on the collection element type overrides write-as-string on the collection property
             var obj = new ClassWithComplexDictionaryProperty
             {
                 MyDictionary = new Dictionary<string, ClassWith_StrictAttribute> { ["Key"] = new ClassWith_StrictAttribute { Float = 1 } }
             };
-            Assert.Equal(@"{""MyDictionary"":{""Key"":{""Float"":1}}}", JsonSerializer.Serialize(obj));
+            Assert.Equal(@"{""MyDictionary"":{""Key"":{""Float"":1}}}", JsonSerializer.Serialize(obj, s_optionReadFromStr));
         }
 
         public class ClassWithComplexDictionaryProperty
         {
-            [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString)]
             public Dictionary<string, ClassWith_StrictAttribute> MyDictionary { get; set; }
         }