From 13a092bddba0727cb17ce46cfeea006c568feb9e Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sun, 9 Feb 2020 16:50:19 -0800 Subject: [PATCH 01/10] FIxed exception being thrown when JsonConverterAttribute is used on Nullable when converter can handle T. --- .../JsonSerializerOptions.Converters.cs | 4 +- .../CustomConverterTests.Int32.cs | 53 ++++++++++++++----- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 161c1345c8af7b..c59e8db8259461 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -189,7 +189,7 @@ internal bool HasConverter(Type typeToConvert) return GetConverter(typeToConvert) != null; } - private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, PropertyInfo? propertyInfo) + private JsonConverter? GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, PropertyInfo? propertyInfo) { JsonConverter? converter; @@ -217,6 +217,8 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter Debug.Assert(converter != null); if (!converter.CanConvert(typeToConvert)) { + if (Nullable.GetUnderlyingType(typeToConvert) != null) + return null; ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, propertyInfo, typeToConvert); } diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs index d194bc27b0800c..37f9d75833ed5e 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs @@ -11,30 +11,45 @@ public static partial class CustomConverterTests /// /// Allow both string and number values on deserialize. /// - private class Int32Converter : JsonConverter + private class Int32Converter : JsonConverterFactory { - public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + public override bool CanConvert(Type typeToConvert) + => typeToConvert == typeof(int); + + public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options) + => new InternalInt32Converter(); + + private class InternalInt32Converter : JsonConverter { - if (reader.TokenType == JsonTokenType.String) + public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - string stringValue = reader.GetString(); - if (int.TryParse(stringValue, out int value)) + if (reader.TokenType == JsonTokenType.String) { - return value; + string stringValue = reader.GetString(); + if (int.TryParse(stringValue, out int value)) + { + return value; + } } + else if (reader.TokenType == JsonTokenType.Number) + { + return reader.GetInt32(); + } + + throw new JsonException(); } - else if (reader.TokenType == JsonTokenType.Number) + + public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options) { - return reader.GetInt32(); + writer.WriteNumberValue(value); } - - throw new JsonException(); } + } - public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options) - { - writer.WriteNumberValue(value); - } + private class Int32Class + { + [JsonConverter(typeof(Int32Converter))] + public int? MyInt { get; set; } } [Fact] @@ -52,6 +67,16 @@ public static void OverrideDefaultConverter() int myInt = JsonSerializer.Deserialize(@"""1""", options); Assert.Equal(1, myInt); } + + { + Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":null}"); + Assert.False(myIntClass.MyInt.HasValue); + } + + { + Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":1}"); + Assert.Equal(1, myIntClass.MyInt.Value); + } } } } From e91a0b8ce6b0bed19fc431fea9d85e199d831285 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sun, 9 Feb 2020 16:56:34 -0800 Subject: [PATCH 02/10] Expanded test coverage. --- .../tests/Serialization/CustomConverterTests.Int32.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs index 37f9d75833ed5e..045326fbb8ae99 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs @@ -68,6 +68,16 @@ public static void OverrideDefaultConverter() Assert.Equal(1, myInt); } + { + int? myInt = JsonSerializer.Deserialize("null", options); + Assert.False(myInt.HasValue); + } + + { + int? myInt = JsonSerializer.Deserialize("1", options); + Assert.Equal(1, myInt.Value); + } + { Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":null}"); Assert.False(myIntClass.MyInt.HasValue); From 947159d9d398c03900ae65a0690d7768f8b77280 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 10 Feb 2020 09:28:19 -0800 Subject: [PATCH 03/10] Code review feedback. --- .../Json/Serialization/JsonSerializerOptions.Converters.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index c59e8db8259461..3427038c3f98d7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -218,7 +218,11 @@ internal bool HasConverter(Type typeToConvert) if (!converter.CanConvert(typeToConvert)) { if (Nullable.GetUnderlyingType(typeToConvert) != null) + { + // Allow nullable handling to forward to the underlying type's converter. return null; + } + ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, propertyInfo, typeToConvert); } From 44d65ebb06f55f49c19996c623cf06439e17dc13 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 10 Feb 2020 11:51:01 -0800 Subject: [PATCH 04/10] Code review feedback. --- .../CustomConverterTests.Int32.cs | 39 +++++++------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs index 045326fbb8ae99..0b3434bcfd0253 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs @@ -11,38 +11,29 @@ public static partial class CustomConverterTests /// /// Allow both string and number values on deserialize. /// - private class Int32Converter : JsonConverterFactory + private class Int32Converter : JsonConverter { - public override bool CanConvert(Type typeToConvert) - => typeToConvert == typeof(int); - - public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options) - => new InternalInt32Converter(); - - private class InternalInt32Converter : JsonConverter + public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + if (reader.TokenType == JsonTokenType.String) { - if (reader.TokenType == JsonTokenType.String) + string stringValue = reader.GetString(); + if (int.TryParse(stringValue, out int value)) { - string stringValue = reader.GetString(); - if (int.TryParse(stringValue, out int value)) - { - return value; - } + return value; } - else if (reader.TokenType == JsonTokenType.Number) - { - return reader.GetInt32(); - } - - throw new JsonException(); } - - public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options) + else if (reader.TokenType == JsonTokenType.Number) { - writer.WriteNumberValue(value); + return reader.GetInt32(); } + + throw new JsonException(); + } + + public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options) + { + writer.WriteNumberValue(value); } } From 07348b2ab3d013279ba56dc1960f0a3691a2bf08 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 12 Feb 2020 22:59:36 -0800 Subject: [PATCH 05/10] Updated to use JsonValueConverterNullableFactory directly instead of fall-through logic. --- .../JsonValueConverterNullableFactory.cs | 9 ++- .../JsonSerializerOptions.Converters.cs | 5 +- .../CustomConverterTests.Int32.cs | 7 +++ .../CustomConverterTests.NullValueType.cs | 60 +++++++++++++++++++ 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonValueConverterNullableFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonValueConverterNullableFactory.cs index 742b1843f9f74a..934c1b9e55c860 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonValueConverterNullableFactory.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/JsonValueConverterNullableFactory.cs @@ -26,14 +26,17 @@ public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializer ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(valueTypeToConvert); } - JsonConverter converter = (JsonConverter)Activator.CreateInstance( + return CreateValueConverter(valueTypeToConvert, valueConverter); + } + + public static JsonConverter CreateValueConverter(Type valueTypeToConvert, JsonConverter valueConverter) + { + return (JsonConverter)Activator.CreateInstance( typeof(JsonValueConverterNullable<>).MakeGenericType(valueTypeToConvert), BindingFlags.Instance | BindingFlags.Public, binder: null, args: new object[] { valueConverter }, culture: null)!; - - return converter; } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index bc1bc392840b36..5dd8ba55384dac 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -217,10 +217,11 @@ private static List GetDefaultConverters() Debug.Assert(converter != null); if (!converter.CanConvert(typeToConvert)) { - if (Nullable.GetUnderlyingType(typeToConvert) != null) + Type? underlyingType = Nullable.GetUnderlyingType(typeToConvert); + if (underlyingType != null) { // Allow nullable handling to forward to the underlying type's converter. - return null; + return JsonValueConverterNullableFactory.CreateValueConverter(underlyingType, converter); } ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, propertyInfo, typeToConvert); diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs index 0b3434bcfd0253..e018006e836178 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs @@ -76,6 +76,13 @@ public static void OverrideDefaultConverter() { Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":1}"); + Assert.True(myIntClass.MyInt.HasValue); + Assert.Equal(1, myIntClass.MyInt.Value); + } + + { + Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":""1""}"); + Assert.True(myIntClass.MyInt.HasValue); Assert.Equal(1, myIntClass.MyInt.Value); } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs index 50ecaaef31ee54..3db77d06c7121c 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs @@ -222,5 +222,65 @@ public static void ConverterForClassThatCanBeNullDependingOnContent() obj = JsonSerializer.Deserialize(@"{""MyInt"":0}"); Assert.Null(obj); } + + private class JsonTestStructConverter : JsonConverter + { + public override TestStruct Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return new TestStruct + { + InnerValue = reader.GetInt32() + }; + } + + public override void Write(Utf8JsonWriter writer, TestStruct value, JsonSerializerOptions options) + { + writer.WriteNumberValue(value.InnerValue); + } + } + + private struct TestStruct + { + public int InnerValue { get; set; } + } + + private class TestStructClass + { + [JsonConverter(typeof(JsonTestStructConverter))] + public TestStruct? MyStruct { get; set; } + } + + [Fact] + public static void NullableCustomValueType() + { + var options = new JsonSerializerOptions(); + options.Converters.Add(new JsonTestStructConverter()); + + { + TestStruct myStruct = JsonSerializer.Deserialize("1", options); + Assert.Equal(1, myStruct.InnerValue); + } + + { + TestStruct? myStruct = JsonSerializer.Deserialize("null", options); + Assert.False(myStruct.HasValue); + } + + { + TestStruct? myStruct = JsonSerializer.Deserialize("1", options); + Assert.Equal(1, myStruct.Value.InnerValue); + } + + { + TestStructClass myStructClass = JsonSerializer.Deserialize(@"{""MyStruct"":null}"); + Assert.False(myStructClass.MyStruct.HasValue); + } + + { + TestStructClass myStructClass = JsonSerializer.Deserialize(@"{""MyStruct"":1}"); + Assert.True(myStructClass.MyStruct.HasValue); + Assert.Equal(1, myStructClass.MyStruct.Value.InnerValue); + } + } } } From 8251e338b3c3b73b0d2782314e7645c9ca058cd2 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 12 Feb 2020 23:06:34 -0800 Subject: [PATCH 06/10] Null response is no longer possible. --- .../Text/Json/Serialization/JsonSerializerOptions.Converters.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 5dd8ba55384dac..bcd7fc017e1f4d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -189,7 +189,7 @@ private static List GetDefaultConverters() return converter; } - private JsonConverter? GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, PropertyInfo? propertyInfo) + private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, PropertyInfo? propertyInfo) { JsonConverter? converter; From ab723f529637409459606a3ff3a7b8c82762c24c Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 13 Feb 2020 10:25:00 -0800 Subject: [PATCH 07/10] Code review. --- .../JsonSerializerOptions.Converters.cs | 2 +- .../CustomConverterTests.NullValueType.cs | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index bcd7fc017e1f4d..374eee393a966e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -218,7 +218,7 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter if (!converter.CanConvert(typeToConvert)) { Type? underlyingType = Nullable.GetUnderlyingType(typeToConvert); - if (underlyingType != null) + if (underlyingType != null && converter.CanConvert(underlyingType)) { // Allow nullable handling to forward to the underlying type's converter. return JsonValueConverterNullableFactory.CreateValueConverter(underlyingType, converter); diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs index 3db77d06c7121c..d246814da28212 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs @@ -250,6 +250,13 @@ private class TestStructClass public TestStruct? MyStruct { get; set; } } + private class TestStructInvalidClass + { + // Note: JsonTestStructConverter does not convert int, this is for negative testing. + [JsonConverter(typeof(JsonTestStructConverter))] + public int? MyInt { get; set; } + } + [Fact] public static void NullableCustomValueType() { @@ -282,5 +289,12 @@ public static void NullableCustomValueType() Assert.Equal(1, myStructClass.MyStruct.Value.InnerValue); } } + + [Fact] + public static void NullableCustomValueTypeNegativeTest() + { + Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":null}")); + Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":1}")); + } } } From 96857fef7a4084e80dd0ca425576f5e82ffdaf28 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Sun, 16 Feb 2020 11:02:40 -0800 Subject: [PATCH 08/10] Updated for refactoring. --- .../Text/Json/Serialization/JsonSerializerOptions.Converters.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 7445ce04f5a08d..2de29b4f5358c4 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -221,7 +221,7 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter if (underlyingType != null && converter.CanConvert(underlyingType)) { // Allow nullable handling to forward to the underlying type's converter. - return JsonValueConverterNullableFactory.CreateValueConverter(underlyingType, converter); + return NullableConverterFactory.CreateValueConverter(underlyingType, converter); } ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, propertyInfo, typeToConvert); From 1aee7fb987868185da07d1d868f18aba15c5b292 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 27 Feb 2020 12:06:10 -0800 Subject: [PATCH 09/10] Code review. --- .../CustomConverterTests.Int32.cs | 23 ---- .../CustomConverterTests.NullValueType.cs | 74 ----------- .../CustomConverterTests.NullableTypes.cs | 115 ++++++++++++++++++ .../tests/System.Text.Json.Tests.csproj | 1 + 4 files changed, 116 insertions(+), 97 deletions(-) create mode 100644 src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs index e018006e836178..86adf7cf274564 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs @@ -37,12 +37,6 @@ public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptio } } - private class Int32Class - { - [JsonConverter(typeof(Int32Converter))] - public int? MyInt { get; set; } - } - [Fact] public static void OverrideDefaultConverter() { @@ -68,23 +62,6 @@ public static void OverrideDefaultConverter() int? myInt = JsonSerializer.Deserialize("1", options); Assert.Equal(1, myInt.Value); } - - { - Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":null}"); - Assert.False(myIntClass.MyInt.HasValue); - } - - { - Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":1}"); - Assert.True(myIntClass.MyInt.HasValue); - Assert.Equal(1, myIntClass.MyInt.Value); - } - - { - Int32Class myIntClass = JsonSerializer.Deserialize(@"{""MyInt"":""1""}"); - Assert.True(myIntClass.MyInt.HasValue); - Assert.Equal(1, myIntClass.MyInt.Value); - } } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs index d246814da28212..50ecaaef31ee54 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs @@ -222,79 +222,5 @@ public static void ConverterForClassThatCanBeNullDependingOnContent() obj = JsonSerializer.Deserialize(@"{""MyInt"":0}"); Assert.Null(obj); } - - private class JsonTestStructConverter : JsonConverter - { - public override TestStruct Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) - { - return new TestStruct - { - InnerValue = reader.GetInt32() - }; - } - - public override void Write(Utf8JsonWriter writer, TestStruct value, JsonSerializerOptions options) - { - writer.WriteNumberValue(value.InnerValue); - } - } - - private struct TestStruct - { - public int InnerValue { get; set; } - } - - private class TestStructClass - { - [JsonConverter(typeof(JsonTestStructConverter))] - public TestStruct? MyStruct { get; set; } - } - - private class TestStructInvalidClass - { - // Note: JsonTestStructConverter does not convert int, this is for negative testing. - [JsonConverter(typeof(JsonTestStructConverter))] - public int? MyInt { get; set; } - } - - [Fact] - public static void NullableCustomValueType() - { - var options = new JsonSerializerOptions(); - options.Converters.Add(new JsonTestStructConverter()); - - { - TestStruct myStruct = JsonSerializer.Deserialize("1", options); - Assert.Equal(1, myStruct.InnerValue); - } - - { - TestStruct? myStruct = JsonSerializer.Deserialize("null", options); - Assert.False(myStruct.HasValue); - } - - { - TestStruct? myStruct = JsonSerializer.Deserialize("1", options); - Assert.Equal(1, myStruct.Value.InnerValue); - } - - { - TestStructClass myStructClass = JsonSerializer.Deserialize(@"{""MyStruct"":null}"); - Assert.False(myStructClass.MyStruct.HasValue); - } - - { - TestStructClass myStructClass = JsonSerializer.Deserialize(@"{""MyStruct"":1}"); - Assert.True(myStructClass.MyStruct.HasValue); - Assert.Equal(1, myStructClass.MyStruct.Value.InnerValue); - } - } - - [Fact] - public static void NullableCustomValueTypeNegativeTest() - { - Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":null}")); - Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":1}")); - } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs new file mode 100644 index 00000000000000..333e5f4e5026a3 --- /dev/null +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs @@ -0,0 +1,115 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +using Xunit; + +namespace System.Text.Json.Serialization.Tests +{ + public static partial class CustomConverterTests + { + private class JsonTestStructConverter : JsonConverter + { + public override TestStruct Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return new TestStruct + { + InnerValue = reader.GetInt32() + }; + } + + public override void Write(Utf8JsonWriter writer, TestStruct value, JsonSerializerOptions options) + { + writer.WriteNumberValue(value.InnerValue); + } + } + + private class JsonTestStructThrowingConverter : JsonConverter + { + public override TestStruct Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + throw new NotSupportedException(); + } + + public override void Write(Utf8JsonWriter writer, TestStruct value, JsonSerializerOptions options) + { + throw new NotSupportedException(); + } + } + + private struct TestStruct + { + public int InnerValue { get; set; } + } + + private class TestStructClass + { + [JsonConverter(typeof(JsonTestStructConverter))] + public TestStruct? MyStruct { get; set; } + } + + private class TestStructInvalidClass + { + // Note: JsonTestStructConverter does not convert int, this is for negative testing. + [JsonConverter(typeof(JsonTestStructConverter))] + public int? MyInt { get; set; } + } + + [Fact] + public static void NullableCustomValueTypeUsingOptions() + { + var options = new JsonSerializerOptions(); + options.Converters.Add(new JsonTestStructConverter()); + + { + TestStruct myStruct = JsonSerializer.Deserialize("1", options); + Assert.Equal(1, myStruct.InnerValue); + } + + { + TestStruct? myStruct = JsonSerializer.Deserialize("null", options); + Assert.False(myStruct.HasValue); + } + + { + TestStruct? myStruct = JsonSerializer.Deserialize("1", options); + Assert.Equal(1, myStruct.Value.InnerValue); + } + } + + [Fact] + public static void NullableCustomValueTypeUsingAttributes() + { + { + TestStructClass myStructClass = JsonSerializer.Deserialize(@"{""MyStruct"":null}"); + Assert.False(myStructClass.MyStruct.HasValue); + } + + { + TestStructClass myStructClass = JsonSerializer.Deserialize(@"{""MyStruct"":1}"); + Assert.True(myStructClass.MyStruct.HasValue); + Assert.Equal(1, myStructClass.MyStruct.Value.InnerValue); + } + } + + [Fact] + public static void NullableCustomValueTypeChoosesAttributeOverOptions() + { + var options = new JsonSerializerOptions(); + options.Converters.Add(new JsonTestStructThrowingConverter()); + + // Chooses JsonTestStructThrowingConverter on options, which will throw. + Assert.Throws(() => JsonSerializer.Deserialize("1", options)); + + // Chooses JsonTestStructConverter on attribute, which will not throw. + TestStructClass myStructClass = JsonSerializer.Deserialize(@"{""MyStruct"":null}", options); + Assert.False(myStructClass.MyStruct.HasValue); + } + + [Fact] + public static void NullableCustomValueTypeNegativeTest() + { + Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":null}")); + Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":1}")); + } + } +} 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 d101e9fe03f8b7..61e78a9182db8a 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 @@ -50,6 +50,7 @@ + From 07d457ede0b4a936065ecfdac151f0378d4c5b8d Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 27 Feb 2020 12:09:49 -0800 Subject: [PATCH 10/10] Code review #2. --- .../Serialization/CustomConverterTests.Int32.cs | 10 ---------- .../CustomConverterTests.NullableTypes.cs | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs index 86adf7cf274564..d194bc27b0800c 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Int32.cs @@ -52,16 +52,6 @@ public static void OverrideDefaultConverter() int myInt = JsonSerializer.Deserialize(@"""1""", options); Assert.Equal(1, myInt); } - - { - int? myInt = JsonSerializer.Deserialize("null", options); - Assert.False(myInt.HasValue); - } - - { - int? myInt = JsonSerializer.Deserialize("1", options); - Assert.Equal(1, myInt.Value); - } } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs index 333e5f4e5026a3..ffc7c4569263d9 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs @@ -111,5 +111,19 @@ public static void NullableCustomValueTypeNegativeTest() Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":null}")); Assert.Throws(() => JsonSerializer.Deserialize(@"{""MyInt"":1}")); } + + [Fact] + public static void NullableStandardValueTypeTest() + { + { + int? myInt = JsonSerializer.Deserialize("null"); + Assert.False(myInt.HasValue); + } + + { + int? myInt = JsonSerializer.Deserialize("1"); + Assert.Equal(1, myInt.Value); + } + } } }