Fixed issues with JsonConverter<object>and nullable value-types

devsko · devsko · commit 906b41c88986 · 2020-08-20T12:13:08.000+02:00
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
@@ -274,8 +274,7 @@ public JsonConverter GetConverter(Type typeToConvert)
 
             Type converterTypeToConvert = converter.TypeToConvert;
 
-            if (!converterTypeToConvert.IsAssignableFrom(typeToConvert) &&
-                !typeToConvert.IsAssignableFrom(converterTypeToConvert))
+            if (!IsCompatibleConverter(converterTypeToConvert, typeToConvert))
             {
                 ThrowHelper.ThrowInvalidOperationException_SerializationConverterNotCompatible(converter.GetType(), typeToConvert);
             }
@@ -292,6 +291,36 @@ public JsonConverter GetConverter(Type typeToConvert)
             return converter;
         }
 
+        private static bool IsNullableValueType(Type type)
+        {
+            return type.IsGenericType && type.GetGenericTypeDefinition() == s_nullableOfTType;
+        }
+
+        private static bool IsNullableType(Type type)
+        {
+            return !type.IsValueType || IsNullableValueType(type);
+        }
+
+        internal static bool IsAssignableFrom(Type type, Type from)
+        {
+            // Special case for which Type.IsAssignableFrom returns false
+            //    type: Nullable<T> where T : IInterface
+            //    from: IInterface
+
+            if (IsNullableValueType(from) && type.IsInterface)
+            {
+                return type.IsAssignableFrom(from.GetGenericArguments()[0]);
+            }
+
+            return type.IsAssignableFrom(from);
+        }
+
+        private static bool IsCompatibleConverter(Type converterTypeToConvert, Type typeToConvert)
+        {
+            return IsAssignableFrom(converterTypeToConvert, typeToConvert)
+                || IsAssignableFrom(typeToConvert, converterTypeToConvert);
+        }
+
         private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, MemberInfo? memberInfo)
         {
             JsonConverter? converter;
@@ -361,9 +390,5 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter
             return default;
         }
 
-        private static bool IsNullableType(Type type)
-        {
-            return type.IsGenericType && type.GetGenericTypeDefinition() == s_nullableOfTType;
-        }
     }
 }
diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReflectionEmitMemberAccessor.cs
@@ -231,7 +231,7 @@ private static DynamicMethod CreatePropertyGetter(PropertyInfo propertyInfo, Typ
             Debug.Assert(declaringType != null);
 
             Type declaredPropertyType = propertyInfo.PropertyType;
-            Debug.Assert(runtimePropertyType.IsAssignableFrom(declaredPropertyType));
+            Debug.Assert(JsonSerializerOptions.IsAssignableFrom(runtimePropertyType, declaredPropertyType));
 
             DynamicMethod dynamicMethod = CreateGetterMethod(propertyInfo.Name, runtimePropertyType);
             ILGenerator generator = dynamicMethod.GetILGenerator();
@@ -274,7 +274,7 @@ private static DynamicMethod CreatePropertySetter(PropertyInfo propertyInfo, Typ
             Debug.Assert(declaringType != null);
 
             Type declaredPropertyType = propertyInfo.PropertyType;
-            Debug.Assert(runtimePropertyType.IsAssignableFrom(declaredPropertyType));
+            Debug.Assert(JsonSerializerOptions.IsAssignableFrom(runtimePropertyType, declaredPropertyType));
 
             DynamicMethod dynamicMethod = CreateSetterMethod(propertyInfo.Name, runtimePropertyType);
             ILGenerator generator = dynamicMethod.GetILGenerator();
@@ -321,7 +321,7 @@ private static DynamicMethod CreateFieldGetter(FieldInfo fieldInfo, Type runtime
             Debug.Assert(declaringType != null);
 
             Type declaredFieldType = fieldInfo.FieldType;
-            Debug.Assert(runtimeFieldType.IsAssignableFrom(declaredFieldType));
+            Debug.Assert(JsonSerializerOptions.IsAssignableFrom(runtimeFieldType, declaredFieldType));
 
             DynamicMethod dynamicMethod = CreateGetterMethod(fieldInfo.Name, runtimeFieldType);
             ILGenerator generator = dynamicMethod.GetILGenerator();
@@ -356,7 +356,7 @@ private static DynamicMethod CreateFieldSetter(FieldInfo fieldInfo, Type runtime
             Debug.Assert(declaringType != null);
 
             Type declaredFieldType = fieldInfo.FieldType;
-            Debug.Assert(runtimeFieldType.IsAssignableFrom(declaredFieldType));
+            Debug.Assert(JsonSerializerOptions.IsAssignableFrom(runtimeFieldType, declaredFieldType));
 
             DynamicMethod dynamicMethod = CreateSetterMethod(fieldInfo.Name, runtimeFieldType);
             ILGenerator generator = dynamicMethod.GetILGenerator();
diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.Object.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.Object.cs
@@ -460,7 +460,6 @@ private class ClassWithNullablePrimitives
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/36329")]
         public static void ClassWithNullablePrimitivesObjectConverterDeserialize()
         {
             const string json =
diff --git a/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.StructMember.cs b/src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests/CustomConverterTests.StructMember.cs
@@ -26,14 +26,14 @@ public override IMemberInterface Read(ref Utf8JsonReader reader, Type typeToConv
 
                 string value = reader.GetString();
 
-                return new StructMember(value);
+                return value == null ? null : new StructMember(value);
             }
 
             public override void Write(Utf8JsonWriter writer, IMemberInterface value, JsonSerializerOptions options)
             {
                 WriteCallCount++;
 
-                JsonSerializer.Serialize(writer, value.Value, typeof(string), options);
+                JsonSerializer.Serialize<string>(writer, value == null ? null : value.Value, options);
             }
         }
 
@@ -46,6 +46,7 @@ private class StructToObjectConverter : JsonConverter<object>
 
             public override bool CanConvert(Type typeToConvert)
             {
+                typeToConvert = Nullable.GetUnderlyingType(typeToConvert) ?? typeToConvert;
                 return typeof(IMemberInterface).IsAssignableFrom(typeToConvert);
             }
 
@@ -55,14 +56,14 @@ public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonS
 
                 string value = reader.GetString();
 
-                return new StructMember(value);
+                return value == null ? null : new StructMember(value);
             }
 
             public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
             {
                 WriteCallCount++;
 
-                JsonSerializer.Serialize(writer, ((IMemberInterface)value).Value, typeof(string), options);
+                JsonSerializer.Serialize<string>(writer, value == null ? null : ((IMemberInterface)value).Value, options);
             }
         }
 
@@ -131,7 +132,6 @@ public static void StructMemberToObjectConverter()
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/36329")]
         public static void NullableStructMemberToInterfaceConverter()
         {
             const string expected = @"{""MyStructProperty"":""StructProperty"",""MyClassProperty"":""ClassProperty"",""MyStructField"":""StructField"",""MyClassField"":""ClassField""}";
@@ -164,7 +164,6 @@ public static void NullableStructMemberToInterfaceConverter()
         }
 
         [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/36329")]
         public static void NullableStructMemberToObjectConverter()
         {
             const string expected = @"{""MyStructProperty"":""StructProperty"",""MyClassProperty"":""ClassProperty"",""MyStructField"":""StructField"",""MyClassField"":""ClassField""}";
@@ -196,5 +195,71 @@ public static void NullableStructMemberToObjectConverter()
             }
         }
 
+        [Fact]
+        public static void NullableStructMemberWithNullsToInterfaceConverter()
+        {
+            const string expected = @"{""MyStructProperty"":null,""MyClassProperty"":null,""MyStructField"":null,""MyClassField"":null}";
+
+            var converter = new StructToInterfaceConverter();
+            var options = new JsonSerializerOptions()
+            {
+                IncludeFields = true,
+            };
+            options.Converters.Add(converter);
+
+            string json;
+
+            {
+                var obj = new TestClassWithNullableStructMember();
+                json = JsonSerializer.Serialize(obj, options);
+
+                Assert.Equal(4, converter.WriteCallCount);
+                Assert.Equal(expected, json);
+            }
+
+            {
+                var obj = JsonSerializer.Deserialize<TestClassWithNullableStructMember>(json, options);
+
+                Assert.Equal(4, converter.ReadCallCount);
+                Assert.Null(obj.MyStructProperty);
+                Assert.Null(obj.MyStructField);
+                Assert.Null(obj.MyClassProperty);
+                Assert.Null(obj.MyClassField);
+            }
+        }
+
+        [Fact]
+        public static void NullableStructMemberWithNullsToObjectConverter()
+        {
+            const string expected = @"{""MyStructProperty"":null,""MyClassProperty"":null,""MyStructField"":null,""MyClassField"":null}";
+
+            var converter = new StructToObjectConverter();
+            var options = new JsonSerializerOptions()
+            {
+                IncludeFields = true,
+            };
+            options.Converters.Add(converter);
+
+            string json;
+
+            {
+                var obj = new TestClassWithNullableStructMember();
+                json = JsonSerializer.Serialize(obj, options);
+
+                Assert.Equal(4, converter.WriteCallCount);
+                Assert.Equal(expected, json);
+            }
+
+            {
+                var obj = JsonSerializer.Deserialize<TestClassWithNullableStructMember>(json, options);
+
+                Assert.Equal(4, converter.ReadCallCount);
+                Assert.Null(obj.MyStructProperty);
+                Assert.Null(obj.MyStructField);
+                Assert.Null(obj.MyClassProperty);
+                Assert.Null(obj.MyClassField);
+            }
+        }
+
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -460,7 +460,6 @@ private class ClassWithNullablePrimitives`
`460`	`460`	`}`
`461`	`461`
`462`	`462`	`[Fact]`
`463`		`- [ActiveIssue("https://github.com/dotnet/runtime/issues/36329")]`
`464`	`463`	`public static void ClassWithNullablePrimitivesObjectConverterDeserialize()`
`465`	`464`	`{`
`466`	`465`	`const string json =`