From 084c70961f606def226cebc463fb2365213d2e57 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 14 Sep 2020 20:01:07 -0700 Subject: [PATCH 1/2] Ensure Read/WriteWithNumberHandling() not called for collection number-elements with custom converters --- .../src/Resources/Strings.resx | 10 +-- .../Json/Serialization/JsonPropertyInfo.cs | 25 ++----- .../Text/Json/ThrowHelper.Serialization.cs | 24 +++--- .../Serialization/NumberHandlingTests.cs | 74 ++++++++++++++++++- 4 files changed, 94 insertions(+), 39 deletions(-) diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index a14cc263f1eb57..cee4615601f5da 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -536,11 +536,11 @@ The ignore condition 'JsonIgnoreCondition.WhenWritingNull' is not valid on value-type member '{0}' on type '{1}'. Consider using 'JsonIgnoreCondition.WhenWritingDefault'. - - '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}'. + + Number handling is only applicable to number types or collections of numbers. When applied to a property or field, the member cannot be handled by a custom converter. If the member is a collection of numbers, the number elements cannot be handled by a custom converter. See type '{0}'. - - 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}'. + + Number handling is only applicable to number types or collections of numbers. When applied to a property or field, the member cannot be handled by a custom converter. If the member is a collection of numbers, the number elements cannot be handled by a custom converter. 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. @@ -554,4 +554,4 @@ Unable to assign 'null' to the property or field of type '{0}'. - + \ No newline at end of file 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 4d7d9f40f2c816..4e07b6f0601aba 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 @@ -248,25 +248,16 @@ private bool TypeIsCollectionOfNumbersWithInternalConverter() 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) + try { - return true; + JsonConverter converter = Options.GetConverter(elementType); + return converter.IsInternalConverterForNumberType; + } + catch (NotSupportedException) + { + // Allow NotSupportedException to be thrown with correct path information during (de)serialization. + return false; } - - return false; } public static TAttribute? GetAttribute(MemberInfo memberInfo) where TAttribute : Attribute diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index 842b5dc3033158..2978c381d09012 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -243,22 +243,20 @@ public static void ThrowInvalidOperationException_IgnoreConditionOnValueTypeInva [MethodImpl(MethodImplOptions.NoInlining)] public static void ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(JsonPropertyInfo jsonPropertyInfo) { - MemberInfo? memberInfo = jsonPropertyInfo.MemberInfo; - - if (!jsonPropertyInfo.ConverterBase.IsInternalConverter) + if (jsonPropertyInfo.IsForClassInfo) { - throw new InvalidOperationException(SR.Format( - SR.NumberHandlingConverterMustBeBuiltIn, - jsonPropertyInfo.ConverterBase.GetType(), - jsonPropertyInfo.IsForClassInfo ? jsonPropertyInfo.DeclaredPropertyType : memberInfo!.DeclaringType)); + throw new InvalidOperationException(SR.Format(SR.NumberHandlingOnTypeInvalid, jsonPropertyInfo.DeclaredPropertyType)); } + else + { + MemberInfo? memberInfo = jsonPropertyInfo.MemberInfo; + Debug.Assert(memberInfo != null); - // This exception is only thrown for object properties. - Debug.Assert(!jsonPropertyInfo.IsForClassInfo && memberInfo != null); - throw new InvalidOperationException(SR.Format( - SR.NumberHandlingOnPropertyTypeMustBeNumberOrCollection, - memberInfo.Name, - memberInfo.DeclaringType)); + throw new InvalidOperationException(SR.Format( + SR.NumberHandlingOnPropertyInvalid, + memberInfo.Name, + memberInfo.DeclaringType)); + } } [DoesNotReturn] diff --git a/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs b/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs index f306e3a3287484..b3ad9855b108af 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs @@ -1524,12 +1524,12 @@ public static void Attribute_NotAllowed_On_Property_WithCustomConverter() string json = @""; InvalidOperationException ex = Assert.Throws(() => JsonSerializer.Deserialize(json)); string exAsStr = ex.ToString(); - Assert.Contains(typeof(ConverterForInt32).ToString(), exAsStr); + Assert.Contains("MyProp", exAsStr); Assert.Contains(typeof(ClassWith_NumberHandlingOn_Property_WithCustomConverter).ToString(), exAsStr); ex = Assert.Throws(() => JsonSerializer.Serialize(new ClassWith_NumberHandlingOn_Property_WithCustomConverter())); exAsStr = ex.ToString(); - Assert.Contains(typeof(ConverterForInt32).ToString(), exAsStr); + Assert.Contains("MyProp", exAsStr); Assert.Contains(typeof(ClassWith_NumberHandlingOn_Property_WithCustomConverter).ToString(), exAsStr); } @@ -1546,12 +1546,10 @@ public static void Attribute_NotAllowed_On_Type_WithCustomConverter() string json = @""; InvalidOperationException ex = Assert.Throws(() => JsonSerializer.Deserialize(json)); string exAsStr = ex.ToString(); - Assert.Contains(typeof(ConverterForMyType).ToString(), exAsStr); Assert.Contains(typeof(ClassWith_NumberHandlingOn_Type_WithCustomConverter).ToString(), exAsStr); ex = Assert.Throws(() => JsonSerializer.Serialize(new ClassWith_NumberHandlingOn_Type_WithCustomConverter())); exAsStr = ex.ToString(); - Assert.Contains(typeof(ConverterForMyType).ToString(), exAsStr); Assert.Contains(typeof(ClassWith_NumberHandlingOn_Type_WithCustomConverter).ToString(), exAsStr); } @@ -1639,5 +1637,73 @@ public static void JsonNumberHandling_ArgOutOfRangeFail() Assert.Throws( () => new JsonNumberHandlingAttribute((JsonNumberHandling)(8))); } + + [Fact] + public static void InternalCollectionConverter_CustomNumberConverter() + { + var list = new List { 1 }; + var options = new JsonSerializerOptions(s_optionReadAndWriteFromStr) + { + Converters = { new ConverterForInt32() } + }; + + // Assert converter methods are called and not Read/WriteWithNumberHandling (which would throw InvalidOperationException). + Assert.Throws(() => JsonSerializer.Serialize(list, options)); + Assert.Equal(25, JsonSerializer.Deserialize>(@"[""1""]", options)[0]); + + var list2 = new List { 1 }; + Assert.Throws(() => JsonSerializer.Serialize(list2, options)); + Assert.Equal(25, JsonSerializer.Deserialize>(@"[""1""]", options)[0]); + + // Invalid to set number handling for number collection property when number is handled with custom converter. + var ex = Assert.Throws(() => JsonSerializer.Deserialize("", options)); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWithListPropAndAttribute(), options)); + + Assert.Throws(() => JsonSerializer.Deserialize("", options)); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWithDictPropAndAttribute(), options)); + } + + private class ClassWithListPropAndAttribute + { + [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] + public List IntProp { get; set; } + } + + private class ClassWithDictPropAndAttribute + { + [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] + public Dictionary IntProp { get; set; } + } + + [Fact] + public static void InternalCollectionConverter_CustomNullableNumberConverter() + { + var dict = new Dictionary { [1] = 1 }; + var options = new JsonSerializerOptions(s_optionReadAndWriteFromStr) + { + Converters = { new ConverterForNullableInt32() } + }; + + // Assert converter methods are called and not Read/WriteWithNumberHandling (which would throw InvalidOperationException). + Assert.Throws(() => JsonSerializer.Serialize(dict, options)); + Assert.Equal(25, JsonSerializer.Deserialize> (@"{""1"":""1""}", options)[1]); + + // Invalid to set number handling for number collection property when number is handled with custom converter. + Assert.Throws(() => JsonSerializer.Deserialize("", options)); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWithDictPropAndAttribute(), options)); + } + + public class ConverterForNullableInt32 : JsonConverter + { + public override int? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return 25; + } + + public override void Write(Utf8JsonWriter writer, int? value, JsonSerializerOptions options) + { + throw new NotImplementedException(); + } + } } } From 603caf2b941fa4d59f9a033043b87770c2afe468 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Sat, 19 Sep 2020 17:36:52 -0700 Subject: [PATCH 2/2] Remove restrictions on custom converters --- .../src/Resources/Strings.resx | 7 +- .../Json/Serialization/JsonConverterOfT.cs | 4 +- .../Json/Serialization/JsonPropertyInfo.cs | 66 +++++++------- .../Text/Json/ThrowHelper.Serialization.cs | 17 +--- .../Serialization/NumberHandlingTests.cs | 91 +++++++++++++------ 5 files changed, 104 insertions(+), 81 deletions(-) diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx index cee4615601f5da..62937bf75a4145 100644 --- a/src/libraries/System.Text.Json/src/Resources/Strings.resx +++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx @@ -536,11 +536,8 @@ The ignore condition 'JsonIgnoreCondition.WhenWritingNull' is not valid on value-type member '{0}' on type '{1}'. Consider using 'JsonIgnoreCondition.WhenWritingDefault'. - - Number handling is only applicable to number types or collections of numbers. When applied to a property or field, the member cannot be handled by a custom converter. If the member is a collection of numbers, the number elements cannot be handled by a custom converter. See type '{0}'. - - Number handling is only applicable to number types or collections of numbers. When applied to a property or field, the member cannot be handled by a custom converter. If the member is a collection of numbers, the number elements cannot be handled by a custom converter. See member '{0}' on type '{1}'. + 'JsonNumberHandlingAttribute' is only valid on a number or a collection of numbers when applied to a property or field. 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. @@ -554,4 +551,4 @@ Unable to assign 'null' to the property or field of type '{0}'. - \ No newline at end of file + 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 38bd9cb480f8b5..854eb76429c3eb 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 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali // For performance, only perform validation on internal converters on debug builds. if (IsInternalConverter) { - if (state.Current.NumberHandling != null) + if (state.Current.NumberHandling != null && IsInternalConverterForNumberType) { value = ReadNumberWithCustomHandling(ref reader, state.Current.NumberHandling.Value); } @@ -179,7 +179,7 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali int originalPropertyDepth = reader.CurrentDepth; long originalPropertyBytesConsumed = reader.BytesConsumed; - if (state.Current.NumberHandling != null) + if (state.Current.NumberHandling != null && IsInternalConverterForNumberType) { value = ReadNumberWithCustomHandling(ref reader, 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 4e07b6f0601aba..4f27bb9e1e55ba 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,15 +184,10 @@ private void DetermineIgnoreCondition(JsonIgnoreCondition? ignoreCondition, bool private void DetermineNumberHandling(JsonNumberHandling? parentTypeNumberHandling) { - bool numberHandlingIsApplicable = ConverterBase.IsInternalConverterForNumberType || TypeIsCollectionOfNumbersWithInternalConverter(); + bool numberHandlingIsApplicable = NumberHandlingIsApplicable(); if (IsForClassInfo) { - if (parentTypeNumberHandling != null && !ConverterBase.IsInternalConverter) - { - ThrowHelper.ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(this); - } - if (numberHandlingIsApplicable) { // This logic is to honor JsonNumberHandlingAttribute placed on @@ -211,53 +206,60 @@ private void DetermineNumberHandling(JsonNumberHandling? parentTypeNumberHandlin else { Debug.Assert(MemberInfo != null); - JsonNumberHandlingAttribute? attribute = GetAttribute(MemberInfo); - if (attribute != null && !numberHandlingIsApplicable) + + if (!numberHandlingIsApplicable) { - ThrowHelper.ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(this); + if (attribute != null) + { + ThrowHelper.ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(this); + } } - - if (numberHandlingIsApplicable) + else { // Priority 1: Get handling from attribute on property or field. - JsonNumberHandling? handling = attribute?.Handling; + NumberHandling = attribute?.Handling; // Priority 2: Get handling from attribute on parent class type. - handling ??= parentTypeNumberHandling; + NumberHandling ??= parentTypeNumberHandling; // Priority 3: Get handling from JsonSerializerOptions instance. - if (!handling.HasValue && Options.NumberHandling != JsonNumberHandling.Strict) + if (!NumberHandling.HasValue && Options.NumberHandling != JsonNumberHandling.Strict) { - handling = Options.NumberHandling; + NumberHandling = Options.NumberHandling; } - - NumberHandling = handling; } } } - private bool TypeIsCollectionOfNumbersWithInternalConverter() + private bool NumberHandlingIsApplicable() { - if (!ConverterBase.IsInternalConverter || - ((ClassType.Enumerable | ClassType.Dictionary) & ClassType) == 0) - { - return false; - } - - Type? elementType = ConverterBase.ElementType; - Debug.Assert(elementType != null); + Type potentialNumberType; - try + if (((ClassType.Enumerable | ClassType.Dictionary) & ClassType) == 0) { - JsonConverter converter = Options.GetConverter(elementType); - return converter.IsInternalConverterForNumberType; + potentialNumberType = DeclaredPropertyType; } - catch (NotSupportedException) + else { - // Allow NotSupportedException to be thrown with correct path information during (de)serialization. - return false; + Debug.Assert(ConverterBase.ElementType != null); + potentialNumberType = ConverterBase.ElementType; } + + potentialNumberType = Nullable.GetUnderlyingType(potentialNumberType) ?? potentialNumberType; + + return potentialNumberType == typeof(byte) || + potentialNumberType == typeof(decimal) || + potentialNumberType == typeof(double) || + potentialNumberType == typeof(short) || + potentialNumberType == typeof(int) || + potentialNumberType == typeof(long) || + potentialNumberType == typeof(sbyte) || + potentialNumberType == typeof(float) || + potentialNumberType == typeof(ushort) || + potentialNumberType == typeof(uint) || + potentialNumberType == typeof(ulong) || + potentialNumberType == JsonClassInfo.ObjectType; } public static TAttribute? GetAttribute(MemberInfo memberInfo) where TAttribute : Attribute diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index 2978c381d09012..607f627be8e2af 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -243,20 +243,11 @@ public static void ThrowInvalidOperationException_IgnoreConditionOnValueTypeInva [MethodImpl(MethodImplOptions.NoInlining)] public static void ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(JsonPropertyInfo jsonPropertyInfo) { - if (jsonPropertyInfo.IsForClassInfo) - { - throw new InvalidOperationException(SR.Format(SR.NumberHandlingOnTypeInvalid, jsonPropertyInfo.DeclaredPropertyType)); - } - else - { - MemberInfo? memberInfo = jsonPropertyInfo.MemberInfo; - Debug.Assert(memberInfo != null); + MemberInfo? memberInfo = jsonPropertyInfo.MemberInfo; + Debug.Assert(memberInfo != null); + Debug.Assert(!jsonPropertyInfo.IsForClassInfo); - throw new InvalidOperationException(SR.Format( - SR.NumberHandlingOnPropertyInvalid, - memberInfo.Name, - memberInfo.DeclaringType)); - } + throw new InvalidOperationException(SR.Format(SR.NumberHandlingOnPropertyInvalid, memberInfo.Name, memberInfo.DeclaringType)); } [DoesNotReturn] diff --git a/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs b/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs index b3ad9855b108af..12e825616a20d4 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/NumberHandlingTests.cs @@ -1519,38 +1519,31 @@ public class ClassWith_NumberHandlingOn_ObjectProperty } [Fact] - public static void Attribute_NotAllowed_On_Property_WithCustomConverter() + public static void Attribute_Ignored_On_Property_WithCustomConverter() { - string json = @""; - InvalidOperationException ex = Assert.Throws(() => JsonSerializer.Deserialize(json)); - string exAsStr = ex.ToString(); - Assert.Contains("MyProp", exAsStr); - Assert.Contains(typeof(ClassWith_NumberHandlingOn_Property_WithCustomConverter).ToString(), exAsStr); + string json = @"{""Prop"":1}"; - ex = Assert.Throws(() => JsonSerializer.Serialize(new ClassWith_NumberHandlingOn_Property_WithCustomConverter())); - exAsStr = ex.ToString(); - Assert.Contains("MyProp", exAsStr); - Assert.Contains(typeof(ClassWith_NumberHandlingOn_Property_WithCustomConverter).ToString(), exAsStr); + // Ensure custom converter is honored. + var obj = JsonSerializer.Deserialize(json); + Assert.Equal(25, obj.Prop); + Assert.Throws(() => JsonSerializer.Serialize(obj)); } public class ClassWith_NumberHandlingOn_Property_WithCustomConverter { [JsonNumberHandling(JsonNumberHandling.Strict)] [JsonConverter(typeof(ConverterForInt32))] - public int MyProp { get; set; } + public int Prop { get; set; } } [Fact] - public static void Attribute_NotAllowed_On_Type_WithCustomConverter() + public static void Attribute_Ignored_On_Type_WithCustomConverter() { - string json = @""; - InvalidOperationException ex = Assert.Throws(() => JsonSerializer.Deserialize(json)); - string exAsStr = ex.ToString(); - Assert.Contains(typeof(ClassWith_NumberHandlingOn_Type_WithCustomConverter).ToString(), exAsStr); + string json = @"{}"; - ex = Assert.Throws(() => JsonSerializer.Serialize(new ClassWith_NumberHandlingOn_Type_WithCustomConverter())); - exAsStr = ex.ToString(); - Assert.Contains(typeof(ClassWith_NumberHandlingOn_Type_WithCustomConverter).ToString(), exAsStr); + // Assert regular Read/Write methods on custom converter are called. + Assert.Throws(() => JsonSerializer.Deserialize(json)); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWith_NumberHandlingOn_Type_WithCustomConverter())); } [JsonNumberHandling(JsonNumberHandling.Strict)] @@ -1639,7 +1632,7 @@ public static void JsonNumberHandling_ArgOutOfRangeFail() } [Fact] - public static void InternalCollectionConverter_CustomNumberConverter() + public static void InternalCollectionConverter_CustomNumberConverter_GlobalOption() { var list = new List { 1 }; var options = new JsonSerializerOptions(s_optionReadAndWriteFromStr) @@ -1655,26 +1648,66 @@ public static void InternalCollectionConverter_CustomNumberConverter() Assert.Throws(() => JsonSerializer.Serialize(list2, options)); Assert.Equal(25, JsonSerializer.Deserialize>(@"[""1""]", options)[0]); - // Invalid to set number handling for number collection property when number is handled with custom converter. - var ex = Assert.Throws(() => JsonSerializer.Deserialize("", options)); - Assert.Throws(() => JsonSerializer.Serialize(new ClassWithListPropAndAttribute(), options)); + // Okay to set number handling for number collection property when number is handled with custom converter; + // converter Read/Write methods called. + ClassWithListPropAndAttribute obj1 = JsonSerializer.Deserialize(@"{""Prop"":[""1""]}", options); + Assert.Equal(25, obj1.Prop[0]); + Assert.Throws(() => JsonSerializer.Serialize(obj1, options)); - Assert.Throws(() => JsonSerializer.Deserialize("", options)); - Assert.Throws(() => JsonSerializer.Serialize(new ClassWithDictPropAndAttribute(), options)); + ClassWithDictPropAndAttribute obj2 = JsonSerializer.Deserialize(@"{""Prop"":{""1"":""1""}}", options); + Assert.Equal(25, obj2.Prop[1]); + Assert.Throws(() => JsonSerializer.Serialize(obj2, options)); } private class ClassWithListPropAndAttribute { [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] - public List IntProp { get; set; } + public List Prop { get; set; } } private class ClassWithDictPropAndAttribute { [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] + public Dictionary Prop { get; set; } + } + + [Fact] + public static void InternalCollectionConverter_CustomNumberConverter_OnProperty() + { + // Invalid to set number handling for number collection property when number is handled with custom converter. + var ex = Assert.Throws(() => JsonSerializer.Deserialize("")); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWithListPropAndAttribute_ConverterOnProp())); + + Assert.Throws(() => JsonSerializer.Deserialize("")); + Assert.Throws(() => JsonSerializer.Serialize(new ClassWithDictPropAndAttribute_ConverterOnProp())); + } + + private class ClassWithListPropAndAttribute_ConverterOnProp + { + [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] + [JsonConverter(typeof(ListOfIntConverter))] + public List IntProp { get; set; } + } + + private class ClassWithDictPropAndAttribute_ConverterOnProp + { + [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] + [JsonConverter(typeof(ClassWithDictPropAndAttribute_ConverterOnProp))] public Dictionary IntProp { get; set; } } + public class ListOfIntConverter : JsonConverter> + { + public override List Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotImplementedException(); + public override void Write(Utf8JsonWriter writer, List value, JsonSerializerOptions options) => throw new NotImplementedException(); + } + + public class DictionaryOfNullableIntConverter : JsonConverter> + { + public override Dictionary Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotImplementedException(); + public override void Write(Utf8JsonWriter writer, Dictionary value, JsonSerializerOptions options) => throw new NotImplementedException(); + } + [Fact] public static void InternalCollectionConverter_CustomNullableNumberConverter() { @@ -1688,9 +1721,9 @@ public static void InternalCollectionConverter_CustomNullableNumberConverter() Assert.Throws(() => JsonSerializer.Serialize(dict, options)); Assert.Equal(25, JsonSerializer.Deserialize> (@"{""1"":""1""}", options)[1]); - // Invalid to set number handling for number collection property when number is handled with custom converter. - Assert.Throws(() => JsonSerializer.Deserialize("", options)); - Assert.Throws(() => JsonSerializer.Serialize(new ClassWithDictPropAndAttribute(), options)); + var obj = JsonSerializer.Deserialize(@"{""Prop"":{""1"":""1""}}", options); + Assert.Equal(25, obj.Prop[1]); + Assert.Throws(() => JsonSerializer.Serialize(obj, options)); } public class ConverterForNullableInt32 : JsonConverter