Disable fast path serialization for types with properties using custo…

…m converters. (#58291)
dotnet · Sep 2, 2021 · 7b32a1a · 7b32a1a
1 parent 2d3ad72
commit 7b32a1a
Show file tree

Hide file tree

Showing 9 changed files with 116 additions and 5 deletions.
diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
@@ -790,7 +790,7 @@ private string GenerateFastPathFuncForObject(TypeGenerationSpec typeGenSpec)
                     out Dictionary<string, PropertyGenerationSpec>? serializableProperties,
                     out bool castingRequiredForProps))
                 {
-                    string exceptionMessage = @$"""Invalid serializable-property configuration specified for type '{typeRef}'. For more information, use 'JsonSourceGenerationMode.Serialization'.""";
+                    string exceptionMessage = @$"""Invalid serializable-property configuration specified for type '{typeRef}'. For more information, see 'JsonSourceGenerationMode.Serialization'.""";
 
                     return GenerateFastPathFuncForType(
                         serializeMethodName,

diff --git a/src/libraries/System.Text.Json/gen/TypeGenerationSpec.cs b/src/libraries/System.Text.Json/gen/TypeGenerationSpec.cs
@@ -217,7 +217,8 @@ private bool FastPathIsSupported()
                 {
                     if (property.TypeGenerationSpec.Type.IsObjectType() ||
                         property.NumberHandling == JsonNumberHandling.AllowNamedFloatingPointLiterals ||
-                        property.NumberHandling == JsonNumberHandling.WriteAsString)
+                        property.NumberHandling == JsonNumberHandling.WriteAsString ||
+                        property.ConverterInstantiationLogic is not null)
                     {
                         return false;
                     }

diff --git a/...ibraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/ContextClasses.cs b/...ibraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/ContextClasses.cs
@@ -9,6 +9,8 @@ namespace System.Text.Json.SourceGeneration.Tests
 {
     public interface ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode { get; }
+
         public JsonTypeInfo<Location> Location { get; }
         public JsonTypeInfo<NumberTypes> NumberTypes { get; }
         public JsonTypeInfo<RepeatedTypes.Location> RepeatedLocation { get; }
@@ -31,6 +33,8 @@ public interface ITestContext
         public JsonTypeInfo<RealWorldContextTests.ClassWithEnumAndNullable> ClassWithEnumAndNullable { get; }
         public JsonTypeInfo<ClassWithCustomConverter> ClassWithCustomConverter { get; }
         public JsonTypeInfo<StructWithCustomConverter> StructWithCustomConverter { get; }
+        public JsonTypeInfo<ClassWithCustomConverterProperty> ClassWithCustomConverterProperty { get; }
+        public JsonTypeInfo<StructWithCustomConverterProperty> StructWithCustomConverterProperty { get; }
         public JsonTypeInfo<ClassWithBadCustomConverter> ClassWithBadCustomConverter { get; }
         public JsonTypeInfo<StructWithBadCustomConverter> StructWithBadCustomConverter { get; }
     }

diff --git a/...son/tests/System.Text.Json.SourceGeneration.Tests/MetadataAndSerializationContextTests.cs b/...son/tests/System.Text.Json.SourceGeneration.Tests/MetadataAndSerializationContextTests.cs
@@ -29,10 +29,13 @@ namespace System.Text.Json.SourceGeneration.Tests
     [JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable))]
     [JsonSerializable(typeof(ClassWithCustomConverter))]
     [JsonSerializable(typeof(StructWithCustomConverter))]
+    [JsonSerializable(typeof(ClassWithCustomConverterProperty))]
+    [JsonSerializable(typeof(StructWithCustomConverterProperty))]
     [JsonSerializable(typeof(ClassWithBadCustomConverter))]
     [JsonSerializable(typeof(StructWithBadCustomConverter))]
     internal partial class MetadataAndSerializationContext : JsonSerializerContext, ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode => JsonSourceGenerationMode.Default;
     }
 
     public sealed class MetadataAndSerializationContextTests : RealWorldContextTests
@@ -64,6 +67,8 @@ public override void EnsureFastPathGeneratedAsExpected()
             Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithEnumAndNullable.Serialize);
             Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithCustomConverter);
             Assert.NotNull(MetadataAndSerializationContext.Default.StructWithCustomConverter);
+            Assert.NotNull(MetadataAndSerializationContext.Default.ClassWithCustomConverterProperty);
+            Assert.NotNull(MetadataAndSerializationContext.Default.StructWithCustomConverterProperty);
             Assert.Throws<InvalidOperationException>(() => MetadataAndSerializationContext.Default.ClassWithBadCustomConverter);
             Assert.Throws<InvalidOperationException>(() => MetadataAndSerializationContext.Default.StructWithBadCustomConverter);
         }

diff --git a/...es/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/MetadataContextTests.cs b/...es/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/MetadataContextTests.cs
@@ -30,10 +30,13 @@ namespace System.Text.Json.SourceGeneration.Tests
     [JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
     [JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
     [JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
+    [JsonSerializable(typeof(ClassWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
+    [JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata)]
     [JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
     [JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata)]
     internal partial class MetadataWithPerTypeAttributeContext : JsonSerializerContext, ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode => JsonSourceGenerationMode.Metadata;
     }
 
     public sealed class MetadataWithPerTypeAttributeContextTests : RealWorldContextTests
@@ -91,10 +94,13 @@ public override void EnsureFastPathGeneratedAsExpected()
     [JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable))]
     [JsonSerializable(typeof(ClassWithCustomConverter))]
     [JsonSerializable(typeof(StructWithCustomConverter))]
+    [JsonSerializable(typeof(ClassWithCustomConverterProperty))]
+    [JsonSerializable(typeof(StructWithCustomConverterProperty))]
     [JsonSerializable(typeof(ClassWithBadCustomConverter))]
     [JsonSerializable(typeof(StructWithBadCustomConverter))]
     internal partial class MetadataContext : JsonSerializerContext, ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode => JsonSourceGenerationMode.Metadata;
     }
 
     public sealed class MetadataContextTests : RealWorldContextTests
@@ -126,6 +132,8 @@ public override void EnsureFastPathGeneratedAsExpected()
             Assert.Null(MetadataContext.Default.ClassWithEnumAndNullable.Serialize);
             Assert.Null(MetadataContext.Default.ClassWithCustomConverter.Serialize);
             Assert.Null(MetadataContext.Default.StructWithCustomConverter.Serialize);
+            Assert.Null(MetadataContext.Default.ClassWithCustomConverterProperty.Serialize);
+            Assert.Null(MetadataContext.Default.StructWithCustomConverterProperty.Serialize);
             Assert.Throws<InvalidOperationException>(() => MetadataContext.Default.ClassWithBadCustomConverter.Serialize);
             Assert.Throws<InvalidOperationException>(() => MetadataContext.Default.StructWithBadCustomConverter.Serialize);
         }

diff --git a/...s/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/MixedModeContextTests.cs b/...s/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/MixedModeContextTests.cs
@@ -28,10 +28,13 @@ namespace System.Text.Json.SourceGeneration.Tests
     [JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
+    [JsonSerializable(typeof(ClassWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
+    [JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization)]
     internal partial class MixedModeContext : JsonSerializerContext, ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode => JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization;
     }
 
     public sealed class MixedModeContextTests : RealWorldContextTests
@@ -62,6 +65,8 @@ public override void EnsureFastPathGeneratedAsExpected()
             Assert.NotNull(MixedModeContext.Default.ClassWithEnumAndNullable.Serialize);
             Assert.Null(MixedModeContext.Default.ClassWithCustomConverter.Serialize);
             Assert.Null(MixedModeContext.Default.StructWithCustomConverter.Serialize);
+            Assert.Null(MixedModeContext.Default.ClassWithCustomConverterProperty.Serialize);
+            Assert.Null(MixedModeContext.Default.StructWithCustomConverterProperty.Serialize);
             Assert.Throws<InvalidOperationException>(() => MixedModeContext.Default.ClassWithBadCustomConverter.Serialize);
             Assert.Throws<InvalidOperationException>(() => MixedModeContext.Default.StructWithBadCustomConverter.Serialize);
         }

diff --git a/...s/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/RealWorldContextTests.cs b/...s/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/RealWorldContextTests.cs
@@ -144,6 +144,60 @@ public virtual void RoundTripWithCustomConverter_Struct()
             Assert.Equal(42, obj.MyInt);
         }
 
+        [Fact]
+        public virtual void RoundtripWithCustomConverterProperty_Class()
+        {
+            const string ExpectedJson = "{\"Property\":42}";
+
+            ClassWithCustomConverterProperty obj = new()
+            {
+                Property = new ClassWithCustomConverterProperty.NestedPoco { Value = 42 }
+            };
+
+            // Types with properties in custom converters do not support fast path serialization.
+            Assert.True(DefaultContext.ClassWithCustomConverterProperty.Serialize is null);
+
+            if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
+            {
+                Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterProperty));
+            }
+            else
+            {
+                string json = JsonSerializer.Serialize(obj, DefaultContext.ClassWithCustomConverterProperty);
+                Assert.Equal(ExpectedJson, json);
+            }
+
+            obj = JsonSerializer.Deserialize<ClassWithCustomConverterProperty>(ExpectedJson);
+            Assert.Equal(42, obj.Property.Value);
+        }
+
+        [Fact]
+        public virtual void RoundtripWithCustomConverterProperty_Struct()
+        {
+            const string ExpectedJson = "{\"Property\":42}";
+
+            StructWithCustomConverterProperty obj = new()
+            {
+                Property = new ClassWithCustomConverterProperty.NestedPoco { Value = 42 }
+            };
+
+            // Types with properties in custom converters do not support fast path serialization.
+            Assert.True(DefaultContext.StructWithCustomConverterProperty.Serialize is null);
+
+            if (DefaultContext.JsonSourceGenerationMode == JsonSourceGenerationMode.Serialization)
+            {
+                Assert.Throws<InvalidOperationException>(() => JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterProperty));
+            }
+            else
+            {
+                string json = JsonSerializer.Serialize(obj, DefaultContext.StructWithCustomConverterProperty);
+                Assert.Equal(ExpectedJson, json);
+            }
+
+            obj = JsonSerializer.Deserialize<StructWithCustomConverterProperty>(ExpectedJson);
+            Assert.Equal(42, obj.Property.Value);
+        }
+
         [Fact]
         public virtual void BadCustomConverter_Class()
         {

diff --git a/...stem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationContextTests.cs b/...stem.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationContextTests.cs
@@ -29,10 +29,13 @@ namespace System.Text.Json.SourceGeneration.Tests
     [JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable))]
     [JsonSerializable(typeof(ClassWithCustomConverter))]
     [JsonSerializable(typeof(StructWithCustomConverter))]
+    [JsonSerializable(typeof(ClassWithCustomConverterProperty))]
+    [JsonSerializable(typeof(StructWithCustomConverterProperty))]
     [JsonSerializable(typeof(ClassWithBadCustomConverter))]
     [JsonSerializable(typeof(StructWithBadCustomConverter))]
     internal partial class SerializationContext : JsonSerializerContext, ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode => JsonSourceGenerationMode.Serialization;
     }
 
     [JsonSerializable(typeof(Location), GenerationMode = JsonSourceGenerationMode.Serialization)]
@@ -57,12 +60,13 @@ internal partial class SerializationContext : JsonSerializerContext, ITestContex
     [JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
-    [JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
-    [JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
+    [JsonSerializable(typeof(ClassWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Serialization)]
+    [JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
     internal partial class SerializationWithPerTypeAttributeContext : JsonSerializerContext, ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode => JsonSourceGenerationMode.Serialization;
     }
 
     [JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)]
@@ -88,10 +92,13 @@ internal partial class SerializationWithPerTypeAttributeContext : JsonSerializer
     [JsonSerializable(typeof(RealWorldContextTests.ClassWithEnumAndNullable), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(ClassWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(StructWithCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
+    [JsonSerializable(typeof(ClassWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Serialization)]
+    [JsonSerializable(typeof(StructWithCustomConverterProperty), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(ClassWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
     [JsonSerializable(typeof(StructWithBadCustomConverter), GenerationMode = JsonSourceGenerationMode.Serialization)]
     internal partial class SerializationContextWithCamelCase : JsonSerializerContext, ITestContext
     {
+        public JsonSourceGenerationMode JsonSourceGenerationMode => JsonSourceGenerationMode.Serialization;
     }
 
     public class SerializationContextTests : RealWorldContextTests
@@ -128,6 +135,8 @@ public override void EnsureFastPathGeneratedAsExpected()
             Assert.NotNull(SerializationContext.Default.ClassWithEnumAndNullable.Serialize);
             Assert.Null(SerializationContext.Default.ClassWithCustomConverter.Serialize);
             Assert.Null(SerializationContext.Default.StructWithCustomConverter.Serialize);
+            Assert.Null(SerializationContext.Default.ClassWithCustomConverterProperty.Serialize);
+            Assert.Null(SerializationContext.Default.StructWithCustomConverterProperty.Serialize);
             Assert.Throws<InvalidOperationException>(() => SerializationContext.Default.ClassWithBadCustomConverter.Serialize);
             Assert.Throws<InvalidOperationException>(() => SerializationContext.Default.StructWithBadCustomConverter.Serialize);
         }
@@ -390,6 +399,8 @@ public override void EnsureFastPathGeneratedAsExpected()
             Assert.NotNull(SerializationWithPerTypeAttributeContext.Default.ClassWithEnumAndNullable.Serialize);
             Assert.Null(SerializationWithPerTypeAttributeContext.Default.ClassWithCustomConverter.Serialize);
             Assert.Null(SerializationWithPerTypeAttributeContext.Default.StructWithCustomConverter.Serialize);
+            Assert.Null(SerializationWithPerTypeAttributeContext.Default.ClassWithCustomConverterProperty.Serialize);
+            Assert.Null(SerializationWithPerTypeAttributeContext.Default.StructWithCustomConverterProperty.Serialize);
             Assert.Throws<InvalidOperationException>(() => SerializationWithPerTypeAttributeContext.Default.ClassWithBadCustomConverter.Serialize);
             Assert.Throws<InvalidOperationException>(() => SerializationWithPerTypeAttributeContext.Default.StructWithBadCustomConverter.Serialize);
         }

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.cs
@@ -199,8 +199,31 @@ public class ClassWithCustomConverter
         public int MyInt { get; set; }
     }
 
+    public class ClassWithCustomConverterProperty
+    {
+        [JsonConverter(typeof(NestedPocoCustomConverter))]
+        public NestedPoco Property { get; set; }
+
+        public class NestedPoco
+        {
+            public int Value { get; set; }
+        }
+
+        public class NestedPocoCustomConverter : JsonConverter<NestedPoco>
+        {
+            public override NestedPoco? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => new NestedPoco { Value = reader.GetInt32() };
+            public override void Write(Utf8JsonWriter writer, NestedPoco value, JsonSerializerOptions options) => writer.WriteNumberValue(value.Value);
+        }
+    }
+
+    public struct StructWithCustomConverterProperty
+    {
+        [JsonConverter(typeof(ClassWithCustomConverterProperty.NestedPocoCustomConverter))]
+        public ClassWithCustomConverterProperty.NestedPoco Property { get; set; }
+    }
+
     [JsonConverter(typeof(CustomConverterForStruct))] // Invalid
-    public struct ClassWithBadCustomConverter
+    public class ClassWithBadCustomConverter
     {
         public int MyInt { get; set; }
     }