Honor converters for underlying types of Nullable<T> specified with JsonConverterAttr...

* FIxed exception being thrown when JsonConverterAttribute is used on Nullable<T> when converter can handle T.

* Expanded test coverage.

* Code review feedback.

* Code review feedback.

* Updated to use JsonValueConverterNullableFactory directly instead of fall-through logic.

* Null response is no longer possible.

* Code review.

* Updated for refactoring.

* Code review.

* Code review #2.

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs
index ffa7b6f..796bd86 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs
@@ -26,14 +26,17 @@ namespace System.Text.Json.Serialization
                 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(NullableConverter<>).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 c17282b..2de29b4 100644 (file)
--- 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,6 +217,13 @@ namespace System.Text.Json
             Debug.Assert(converter != null);
             if (!converter.CanConvert(typeToConvert))
             {
+                Type? underlyingType = Nullable.GetUnderlyingType(typeToConvert);
+                if (underlyingType != null && converter.CanConvert(underlyingType))
+                {
+                    // Allow nullable handling to forward to the underlying type's converter.
+                    return NullableConverterFactory.CreateValueConverter(underlyingType, converter);
+                }
+
                 ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, propertyInfo, typeToConvert);
             }
 

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 (file)
index 0000000..ffc7c45
--- /dev/null
+++ b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.NullableTypes.cs
@@ -0,0 +1,129 @@
+// 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<TestStruct>
+        {
+            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<TestStruct>
+        {
+            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<TestStruct>("1", options);
+                Assert.Equal(1, myStruct.InnerValue);
+            }
+
+            {
+                TestStruct? myStruct = JsonSerializer.Deserialize<TestStruct?>("null", options);
+                Assert.False(myStruct.HasValue);
+            }
+
+            {
+                TestStruct? myStruct = JsonSerializer.Deserialize<TestStruct?>("1", options);
+                Assert.Equal(1, myStruct.Value.InnerValue);
+            }
+        }
+
+        [Fact]
+        public static void NullableCustomValueTypeUsingAttributes()
+        {
+            {
+                TestStructClass myStructClass = JsonSerializer.Deserialize<TestStructClass>(@"{""MyStruct"":null}");
+                Assert.False(myStructClass.MyStruct.HasValue);
+            }
+
+            {
+                TestStructClass myStructClass = JsonSerializer.Deserialize<TestStructClass>(@"{""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<NotSupportedException>(() => JsonSerializer.Deserialize<TestStruct?>("1", options));
+
+            // Chooses JsonTestStructConverter on attribute, which will not throw.
+            TestStructClass myStructClass = JsonSerializer.Deserialize<TestStructClass>(@"{""MyStruct"":null}", options);
+            Assert.False(myStructClass.MyStruct.HasValue);
+        }
+
+        [Fact]
+        public static void NullableCustomValueTypeNegativeTest()
+        {
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<TestStructInvalidClass>(@"{""MyInt"":null}"));
+            Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize<TestStructInvalidClass>(@"{""MyInt"":1}"));
+        }
+
+        [Fact]
+        public static void NullableStandardValueTypeTest()
+        {
+            {
+                int? myInt = JsonSerializer.Deserialize<int?>("null");
+                Assert.False(myInt.HasValue);
+            }
+
+            {
+                int? myInt = JsonSerializer.Deserialize<int?>("1");
+                Assert.Equal(1, myInt.Value);
+            }
+        }
+    }
+}

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 d101e9f..61e78a9 100644 (file)
--- 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 @@
     <Compile Include="Serialization\CustomConverterTests.Exceptions.cs" />
     <Compile Include="Serialization\CustomConverterTests.Int32.cs" />
     <Compile Include="Serialization\CustomConverterTests.List.cs" />
+    <Compile Include="Serialization\CustomConverterTests.NullableTypes.cs" />
     <Compile Include="Serialization\CustomConverterTests.NullValueType.cs" />
     <Compile Include="Serialization\CustomConverterTests.Object.cs" />
     <Compile Include="Serialization\CustomConverterTests.Point.cs" />