From 149b235d6792be337ac6d9151a9f8ee9844c3e2f Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Tue, 8 Sep 2020 08:39:46 -0700 Subject: [PATCH] Honor custom number handling only when property/type is a number/collection of numbers (#41679) * 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 --- .../System.Text.Json/src/Resources/Strings.resx | 2 +- .../Converters/Value/ObjectConverter.cs | 13 ++ .../Text/Json/Serialization/JsonConverterOfT.cs | 6 +- .../Text/Json/Serialization/JsonPropertyInfo.cs | 85 +++++-- .../tests/Serialization/NumberHandlingTests.cs | 245 ++++++++++++++++++++- 5 files changed, 315 insertions(+), 36 deletions(-) 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 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -540,7 +540,7 @@ '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}'. - 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}'. + 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}'. 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. 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 --- 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 { + 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 --- 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 --- 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. + + // 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(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(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(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 --- 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(serialized); + + JsonElement el = Assert.IsType(obj.MyInt); + Assert.Equal(JsonValueKind.String, el.ValueKind); + Assert.Equal("1", el.GetString()); + + el = Assert.IsType(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 { @int }; + string serialized = JsonSerializer.Serialize(obj, s_optionReadAndWriteFromStr); + Assert.Equal(expected, serialized); + + obj = JsonSerializer.Deserialize>(serialized, s_optionReadAndWriteFromStr); + + JsonElement el = Assert.IsType(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(serialized, s_optionReadAndWriteFromStr); + + el = Assert.IsType(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 { @int }, + MyNullableFloats = new object[] { nullableFloat } + }; + + string serialized = JsonSerializer.Serialize(obj); + JsonTestHelper.AssertJsonEqual(expected, serialized); + + obj = JsonSerializer.Deserialize(serialized); + + JsonElement el = Assert.IsType(obj.MyInts[0]); + Assert.Equal(JsonValueKind.String, el.ValueKind); + Assert.Equal("2", el.GetString()); + + el = Assert.IsType(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 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(serialized); + + JsonElement el = Assert.IsType(obj.MyDateTime); + Assert.Equal(JsonValueKind.String, el.ValueKind); + Assert.Equal(dateTime, el.GetDateTime()); + + el = Assert.IsType(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 { dateTime }; + string serialized = JsonSerializer.Serialize(obj, s_optionReadAndWriteFromStr); + Assert.Equal(expected, serialized); + + obj = JsonSerializer.Deserialize>(serialized, s_optionReadAndWriteFromStr); + + JsonElement el = Assert.IsType(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(serialized, s_optionReadAndWriteFromStr); + + el = Assert.IsType(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 { dateTime }, + MyNullableGuids = new object[] { nullableGuid } + }; + + string serialized = JsonSerializer.Serialize(obj); + JsonTestHelper.AssertJsonEqual(expected, serialized); + + obj = JsonSerializer.Deserialize(serialized); + + JsonElement el = Assert.IsType(obj.MyDateTimes[0]); + Assert.Equal(JsonValueKind.String, el.ValueKind); + Assert.Equal(dateTime, el.GetDateTime()); + + el = Assert.IsType(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 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(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Deserialize(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 { 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 MyList { get; set; } + } + + [Fact] + public static void NumberHandlingAttribute_NotAllowedOn_CollectionOfNonNumbers() + { + Assert.Throws(() => JsonSerializer.Deserialize("")); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWith_AttributeOnComplexListProperty())); + Assert.Throws(() => JsonSerializer.Deserialize("")); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWith_AttributeOnComplexDictionaryProperty())); + } + + public class ClassWith_AttributeOnComplexListProperty + { [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] public List MyList { get; set; } } + public class ClassWith_AttributeOnComplexDictionaryProperty + { + [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString)] + public Dictionary 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(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Deserialize(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 { ["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 MyDictionary { get; set; } } -- 2.7.4