Throw NotSupportedException when applying JsonObjectHandling.Populate on types with...

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

diff --git a/src/libraries/System.Text.Json/src/Resources/Strings.resx b/src/libraries/System.Text.Json/src/Resources/Strings.resx
index f091984..0ebab3e 100644 (file)
--- a/src/libraries/System.Text.Json/src/Resources/Strings.resx
+++ b/src/libraries/System.Text.Json/src/Resources/Strings.resx
@@ -696,6 +696,9 @@
   <data name="ObjectCreationHandlingPropertyCannotAllowReferenceHandling" xml:space="preserve">
     <value>JsonObjectCreationHandling.Populate is incompatible with reference handling.</value>
   </data>
+  <data name="ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors" xml:space="preserve">
+    <value>JsonObjectCreationHandling.Populate is currently not supported in types with parameterized constructors.</value>
+  </data>
   <data name="FormatInt128" xml:space="preserve">
     <value>Either the JSON value is not in a supported format, or is out of bounds for an Int128.</value>
   </data>

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs
index 304ca0a..7c5d6aa 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs
@@ -25,10 +25,10 @@ namespace System.Text.Json.Serialization.Converters
         {
             JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;
 
-            if (jsonTypeInfo.CreateObject != null || state.Current.IsPopulating)
+            if (!jsonTypeInfo.UsesParameterizedConstructor || state.Current.IsPopulating)
             {
                 // Fall back to default object converter in following cases:
-                // - if user has set a default constructor delegate with contract customization
+                // - if user configuration has invalidated the parameterized constructor
                 // - we're continuing populating an object.
                 return base.OnTryRead(ref reader, typeToConvert, options, ref state, out value);
             }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
index 8ee9f3d..c654a92 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
@@ -38,12 +38,20 @@ namespace System.Text.Json.Serialization.Metadata
         private static JsonTypeInfo CreateTypeInfoCore(Type type, JsonConverter converter, JsonSerializerOptions options)
         {
             JsonTypeInfo typeInfo = JsonTypeInfo.CreateJsonTypeInfo(type, converter, options);
-            typeInfo.NumberHandling = GetNumberHandlingForType(typeInfo.Type);
-            typeInfo.PreferredPropertyObjectCreationHandling = GetObjectCreationHandlingForType(typeInfo.Type);
 
-            if (typeInfo.Kind == JsonTypeInfoKind.Object)
+            if (GetNumberHandlingForType(typeInfo.Type) is { } numberHandling)
             {
-                typeInfo.UnmappedMemberHandling = GetUnmappedMemberHandling(typeInfo.Type);
+                typeInfo.NumberHandling = numberHandling;
+            }
+
+            if (GetObjectCreationHandlingForType(typeInfo.Type) is { } creationHandling)
+            {
+                typeInfo.PreferredPropertyObjectCreationHandling = creationHandling;
+            }
+
+            if (GetUnmappedMemberHandling(typeInfo.Type) is { } unmappedMemberHandling)
+            {
+                typeInfo.UnmappedMemberHandling = unmappedMemberHandling;
             }
 
             typeInfo.PopulatePolymorphismMetadata();

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
index 0ab5c08..e223409 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
@@ -495,9 +495,17 @@ namespace System.Text.Json.Serialization.Metadata
             Debug.Assert(ParentTypeInfo != null, "We should have ensured parent is assigned in JsonTypeInfo");
             Debug.Assert(!IsConfigured, "Should not be called post-configuration.");
 
+            JsonObjectCreationHandling effectiveObjectCreationHandling = JsonObjectCreationHandling.Replace;
             if (ObjectCreationHandling == null)
             {
-                JsonObjectCreationHandling preferredCreationHandling = ParentTypeInfo.PreferredPropertyObjectCreationHandling ?? Options.PreferredObjectCreationHandling;
+                // Consult type-level configuration, then global configuration.
+                // Ignore global configuration if we're using a parameterized constructor.
+                JsonObjectCreationHandling preferredCreationHandling =
+                    ParentTypeInfo.PreferredPropertyObjectCreationHandling
+                    ?? (ParentTypeInfo.DetermineUsesParameterizedConstructor()
+                        ? JsonObjectCreationHandling.Replace
+                        : Options.PreferredObjectCreationHandling);
+
                 bool canPopulate =
                     preferredCreationHandling == JsonObjectCreationHandling.Populate &&
                     EffectiveConverter.CanPopulate &&
@@ -506,7 +514,7 @@ namespace System.Text.Json.Serialization.Metadata
                     !ParentTypeInfo.SupportsPolymorphicDeserialization &&
                     !(Set == null && IgnoreReadOnlyMember);
 
-                EffectiveObjectCreationHandling = canPopulate ? JsonObjectCreationHandling.Populate : JsonObjectCreationHandling.Replace;
+                effectiveObjectCreationHandling = canPopulate ? JsonObjectCreationHandling.Populate : JsonObjectCreationHandling.Replace;
             }
             else if (ObjectCreationHandling == JsonObjectCreationHandling.Populate)
             {
@@ -537,18 +545,24 @@ namespace System.Text.Json.Serialization.Metadata
                     ThrowHelper.ThrowInvalidOperationException_ObjectCreationHandlingPropertyCannotAllowReadOnlyMember(this);
                 }
 
-                EffectiveObjectCreationHandling = JsonObjectCreationHandling.Populate;
-            }
-            else
-            {
-                Debug.Assert(EffectiveObjectCreationHandling == JsonObjectCreationHandling.Replace);
+                effectiveObjectCreationHandling = JsonObjectCreationHandling.Populate;
             }
 
-            if (EffectiveObjectCreationHandling == JsonObjectCreationHandling.Populate &&
-                Options.ReferenceHandlingStrategy != ReferenceHandlingStrategy.None)
+            if (effectiveObjectCreationHandling is JsonObjectCreationHandling.Populate)
             {
-                ThrowHelper.ThrowInvalidOperationException_ObjectCreationHandlingPropertyCannotAllowReferenceHandling();
+                if (ParentTypeInfo.DetermineUsesParameterizedConstructor())
+                {
+                    ThrowHelper.ThrowNotSupportedException_ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors();
+                }
+
+                if (Options.ReferenceHandlingStrategy != ReferenceHandlingStrategy.None)
+                {
+                    ThrowHelper.ThrowInvalidOperationException_ObjectCreationHandlingPropertyCannotAllowReferenceHandling();
+                }
             }
+
+            // Validation complete, commit configuration.
+            EffectiveObjectCreationHandling = effectiveObjectCreationHandling;
         }
 
         private bool NumberHandingIsApplicable()

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs
index 86a7af2..5a901fb 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs
@@ -35,6 +35,14 @@ namespace System.Text.Json.Serialization.Metadata
         // All of the serializable parameters on a POCO constructor keyed on parameter name.
         // Only parameters which bind to properties are cached.
         internal JsonPropertyDictionary<JsonParameterInfo>? ParameterCache { get; private set; }
+        internal bool UsesParameterizedConstructor
+        {
+            get
+            {
+                Debug.Assert(IsConfigured);
+                return ParameterCache != null;
+            }
+        }
 
         // All of the serializable properties on a POCO (except the optional extension property) keyed on property name.
         internal JsonPropertyDictionary<JsonPropertyInfo>? PropertyCache { get; private set; }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
index b9e9fe6..a1a184f 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
@@ -552,17 +552,14 @@ namespace System.Text.Json.Serialization.Metadata
             {
                 VerifyMutable();
 
-                if (value is not null)
+                if (Kind != JsonTypeInfoKind.Object)
                 {
-                    if (Kind != JsonTypeInfoKind.Object)
-                    {
-                        ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
-                    }
+                    ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
+                }
 
-                    if (!JsonSerializer.IsValidCreationHandlingValue(value.Value))
-                    {
-                        throw new ArgumentOutOfRangeException(nameof(value));
-                    }
+                if (value is not null && !JsonSerializer.IsValidCreationHandlingValue(value.Value))
+                {
+                    throw new ArgumentOutOfRangeException(nameof(value));
                 }
 
                 _preferredPropertyObjectCreationHandling = value;
@@ -684,7 +681,7 @@ namespace System.Text.Json.Serialization.Metadata
             {
                 ConfigureProperties();
 
-                if (Converter.ConstructorIsParameterized)
+                if (DetermineUsesParameterizedConstructor())
                 {
                     ConfigureConstructorParameters();
                 }
@@ -808,6 +805,12 @@ namespace System.Text.Json.Serialization.Metadata
         /// </summary>
         private bool IsCompatibleWithCurrentOptions { get; set; } = true;
 
+        /// <summary>
+        /// Determine if the current configuration is compatible with using a parameterized constructor.
+        /// </summary>
+        internal bool DetermineUsesParameterizedConstructor()
+            => Converter.ConstructorIsParameterized && CreateObject is null;
+
 #if DEBUG
         internal string GetPropertyDebugInfo(ReadOnlySpan<byte> unescapedPropertyName)
         {
@@ -1107,7 +1110,7 @@ namespace System.Text.Json.Serialization.Metadata
         internal void ConfigureConstructorParameters()
         {
             Debug.Assert(Kind == JsonTypeInfoKind.Object);
-            Debug.Assert(Converter.ConstructorIsParameterized);
+            Debug.Assert(DetermineUsesParameterizedConstructor());
             Debug.Assert(PropertyCache is not null);
             Debug.Assert(ParameterCache is null);
 

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs
index 59a47bc..25c067c 100644 (file)
--- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs
+++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs
@@ -386,20 +386,20 @@ namespace System.Text.Json
 
             for (int i = 0; i < _count - 1; i++)
             {
-                if (_stack[i].JsonTypeInfo.Converter.ConstructorIsParameterized)
+                if (_stack[i].JsonTypeInfo.UsesParameterizedConstructor)
                 {
                     return _stack[i].JsonTypeInfo;
                 }
             }
 
-            Debug.Assert(Current.JsonTypeInfo.Converter.ConstructorIsParameterized);
+            Debug.Assert(Current.JsonTypeInfo.UsesParameterizedConstructor);
             return Current.JsonTypeInfo;
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void SetConstructorArgumentState()
         {
-            if (Current.JsonTypeInfo.Converter.ConstructorIsParameterized)
+            if (Current.JsonTypeInfo.UsesParameterizedConstructor)
             {
                 Current.CtorArgumentState ??= new();
             }

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 7072d9e..5b05ff2 100644 (file)
--- 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
@@ -100,6 +100,12 @@ namespace System.Text.Json
         }
 
         [DoesNotReturn]
+        public static void ThrowNotSupportedException_ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors()
+        {
+            throw new NotSupportedException(SR.ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors);
+        }
+
+        [DoesNotReturn]
         public static void ThrowJsonException_SerializationConverterRead(JsonConverter? converter)
         {
             throw new JsonException(SR.Format(SR.SerializationConverterRead, converter)) { AppendPathInformation = true };

diff --git a/src/libraries/System.Text.Json/tests/Common/JsonCreationHandlingTests.Object.cs b/src/libraries/System.Text.Json/tests/Common/JsonCreationHandlingTests.Object.cs
index aae9da6..e25adff 100644 (file)
--- a/src/libraries/System.Text.Json/tests/Common/JsonCreationHandlingTests.Object.cs
+++ b/src/libraries/System.Text.Json/tests/Common/JsonCreationHandlingTests.Object.cs
@@ -1165,4 +1165,52 @@ public abstract partial class JsonCreationHandlingTests : SerializerTests
         [JsonObjectCreationHandling((JsonObjectCreationHandling)(-1))]
         public List<int> Property { get; }
     }
+
+    [Theory]
+    [InlineData(typeof(ClassWithParameterizedConstructorWithPopulateProperty))]
+    [InlineData(typeof(ClassWithParameterizedConstructorWithPopulateType))]
+    public async Task ClassWithParameterizedCtor_UsingPopulateConfiguration_ThrowsNotSupportedException(Type type)
+    {
+        object instance = Activator.CreateInstance(type, "Jim");
+        string json = """{"Username":"Jim","PhoneNumbers":["123456"]}""";
+
+        await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.SerializeWrapper(instance, type));
+        await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.DeserializeWrapper(json, type));
+        Assert.Throws<NotSupportedException>(() => Serializer.GetTypeInfo(type));
+    }
+
+    public class ClassWithParameterizedConstructorWithPopulateProperty(string name)
+    {
+        public string Name { get; } = name;
+
+        [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)]
+        public List<string> PhoneNumbers { get; } = new();
+    }
+
+    [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)]
+    public class ClassWithParameterizedConstructorWithPopulateType(string name)
+    {
+        public string Name { get; } = name;
+
+        public List<string> PhoneNumbers { get; } = new();
+    }
+
+    [Fact]
+    public async Task ClassWithParameterizedCtor_NoPopulateConfiguration_WorksWithGlobalPopulateConfiguration()
+    {
+        string json = """{"Username":"Jim","PhoneNumbers":["123456"]}""";
+
+        JsonSerializerOptions options = Serializer.CreateOptions(makeReadOnly: false);
+        options.PreferredObjectCreationHandling = JsonObjectCreationHandling.Populate;
+
+        ClassWithParameterizedConstructorNoPopulate result = await Serializer.DeserializeWrapper<ClassWithParameterizedConstructorNoPopulate>(json, options);
+        Assert.Empty(result.PhoneNumbers);
+    }
+
+    public class ClassWithParameterizedConstructorNoPopulate(string name)
+    {
+        public string Name { get; } = name;
+
+        public List<string> PhoneNumbers { get; } = new();
+    }
 }

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/JsonCreationHandlingTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/JsonCreationHandlingTests.cs
index eab6f93..5862387 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/JsonCreationHandlingTests.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/JsonCreationHandlingTests.cs
@@ -278,6 +278,9 @@ namespace System.Text.Json.SourceGeneration.Tests
     [JsonSerializable(typeof(SimpleClassWitNonPopulatableProperty))]
     [JsonSerializable(typeof(ClassWithInvalidTypeAnnotation))]
     [JsonSerializable(typeof(ClassWithInvalidPropertyAnnotation))]
+    [JsonSerializable(typeof(ClassWithParameterizedConstructorWithPopulateProperty))]
+    [JsonSerializable(typeof(ClassWithParameterizedConstructorWithPopulateType))]
+    [JsonSerializable(typeof(ClassWithParameterizedConstructorNoPopulate))]
     internal partial class CreationHandlingTestContext : JsonSerializerContext
     {
     }

diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs
index 06fd59b..bc6e3a2 100644 (file)
--- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs
+++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/MetadataTests/DefaultJsonTypeInfoResolverTests.JsonTypeInfo.cs
@@ -1440,11 +1440,10 @@ namespace System.Text.Json.Serialization.Tests
         {
             JsonTypeInfo jsonTypeInfo = JsonTypeInfo.CreateJsonTypeInfo(type, new());
 
-            // Invalid kinds default to null and can be set to null.
-            Assert.Null(jsonTypeInfo.PreferredPropertyObjectCreationHandling);
-            jsonTypeInfo.PreferredPropertyObjectCreationHandling = null;
+            // Invalid kinds default to null.
             Assert.Null(jsonTypeInfo.PreferredPropertyObjectCreationHandling);
 
+            Assert.Throws<InvalidOperationException>(() => jsonTypeInfo.PreferredPropertyObjectCreationHandling = null);
             Assert.Throws<InvalidOperationException>(() => jsonTypeInfo.PreferredPropertyObjectCreationHandling = JsonObjectCreationHandling.Populate);
             Assert.Throws<InvalidOperationException>(() => jsonTypeInfo.PreferredPropertyObjectCreationHandling = JsonObjectCreationHandling.Replace);
             Assert.Null(jsonTypeInfo.PreferredPropertyObjectCreationHandling);