Fix config binding with records having collection constructor paramet…

…er (#106158)

src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

@@ -220,7 +220,7 @@ public static void Bind(this IConfiguration configuration, object? instance, Act
 
         [RequiresDynamicCode(DynamicCodeWarningMessage)]
         [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)]
-        private static void BindProperties(object instance, IConfiguration configuration, BinderOptions options)
+        private static void BindProperties(object instance, IConfiguration configuration, BinderOptions options, ParameterInfo[]? constructorParameters)
         {
             List<PropertyInfo> modelProperties = GetAllProperties(instance.GetType());
 
@@ -248,10 +248,43 @@ private static void BindProperties(object instance, IConfiguration configuration
 
             foreach (PropertyInfo property in modelProperties)
             {
-                BindProperty(property, instance, configuration, options);
+                if (constructorParameters is null || !constructorParameters.Any(p => p.Name == property.Name))
+                {
+                    BindProperty(property, instance, configuration, options);
+                }
+                else
+                {
+                    ResetPropertyValue(property, instance, options);
+                }
             }
         }
 
+        /// <summary>
+        /// Reset the property value to the value from the property getter. This is useful for properties that have a getter or setters that perform some logic changing the object state.
+        /// </summary>
+        /// <param name="property">The property to reset.</param>
+        /// <param name="instance">The instance to reset the property on.</param>
+        /// <param name="options">The binder options.</param>
+        /// <remarks>
+        /// This method doesn't do any configuration binding. It just resets the property value to the value from the property getter.
+        /// This method called only when creating an instance using a primary constructor with parameters names match properties names.
+        /// </remarks>
+        [RequiresDynamicCode(DynamicCodeWarningMessage)]
+        [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)]
+        private static void ResetPropertyValue(PropertyInfo property, object instance, BinderOptions options)
+        {
+            // We don't support set only, non public, or indexer properties
+            if (property.GetMethod is null ||
+                property.SetMethod is null ||
+                (!options.BindNonPublicProperties && (!property.GetMethod.IsPublic || !property.SetMethod.IsPublic)) ||
+                property.GetMethod.GetParameters().Length > 0)
+            {
+                return;
+            }
+
+            property.SetValue(instance, property.GetValue(instance));
+        }
+
         [RequiresDynamicCode(DynamicCodeWarningMessage)]
         [RequiresUnreferencedCode(PropertyTrimmingWarningMessage)]
         private static void BindProperty(PropertyInfo property, object instance, IConfiguration config, BinderOptions options)
@@ -381,6 +414,8 @@ private static void BindInstance(
                     return;
                 }
 
+                ParameterInfo[]? constructorParameters = null;
+
                 // If we don't have an instance, try to create one
                 if (bindingPoint.Value is null)
                 {
@@ -401,7 +436,7 @@ private static void BindInstance(
                     }
                     else
                     {
-                        bindingPoint.SetValue(CreateInstance(type, config, options));
+                        bindingPoint.SetValue(CreateInstance(type, config, options, out constructorParameters));
                     }
                 }
 
@@ -424,7 +459,7 @@ private static void BindInstance(
                     }
                     else
                     {
-                        BindProperties(bindingPoint.Value, config, options);
+                        BindProperties(bindingPoint.Value, config, options, constructorParameters);
                     }
                 }
             }
@@ -433,11 +468,25 @@ private static void BindInstance(
                 if (isParentCollection && bindingPoint.Value is null && string.IsNullOrEmpty(configValue))
                 {
                     // If we don't have an instance, try to create one
-                    bindingPoint.TrySetValue(CreateInstance(type, config, options));
+                    bindingPoint.TrySetValue(CreateInstance(type, config, options, out _));
                 }
             }
         }
 
+        /// <summary>
+        /// Create an instance of the specified type.
+        /// </summary>
+        /// <param name="type">The type to create an instance of.</param>
+        /// <param name="config">The configuration to bind to the instance.</param>
+        /// <param name="options">The binder options.</param>
+        /// <param name="constructorParameters">The parameters of the constructor used to create the instance.</param>
+        /// <returns>The created instance.</returns>
+        /// <exception cref="InvalidOperationException">If the type cannot be created.</exception>
+        /// <remarks>
+        /// constructorParameters will not be null only when using a constructor with a parameters which get their values from the configuration
+        /// This happen when using types having properties match the constructor parameter names. `record` types are an example.
+        /// In such cases we need to carry the parameters list to avoid binding the properties again during BindProperties.
+        /// </remarks>
         [RequiresDynamicCode(DynamicCodeWarningMessage)]
         [RequiresUnreferencedCode(
             "In case type is a Nullable<T>, cannot statically analyze what the underlying type is so its members may be trimmed.")]
@@ -446,10 +495,13 @@ private static object CreateInstance(
                                         DynamicallyAccessedMemberTypes.NonPublicConstructors)]
             Type type,
             IConfiguration config,
-            BinderOptions options)
+            BinderOptions options,
+            out ParameterInfo[]? constructorParameters)
         {
             Debug.Assert(!type.IsArray);
 
+            constructorParameters = null;
+
             if (type.IsInterface || type.IsAbstract)
             {
                 throw new InvalidOperationException(SR.Format(SR.Error_CannotActivateAbstractOrInterface, type));
@@ -495,6 +547,8 @@ private static object CreateInstance(
                     parameterValues[index] = BindParameter(parameters[index], type, config, options);
                 }
 
+                constructorParameters = parameters;
+
                 return constructor.Invoke(parameterValues);
             }
 

src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs

@@ -188,6 +188,8 @@ public string Color
             }
         }
 
+        public record RecordWithArrayParameter(string[] Array);
+
         public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length);
 
         public class ContainerWithNestedImmutableObject

src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs

@@ -1523,6 +1523,27 @@ public void CanBindOnParametersAndProperties_PropertiesAreSetAfterTheConstructor
             Assert.Equal("the color is Green", options.Color);
         }
 
+        /// <summary>
+        /// This test to ensure the binding of the constructor/property array is done once and not duplicating values in the array.
+        /// </summary>
+        [Fact]
+        public void CanBindOnParametersAndProperties_RecordWithArrayConstructorParameter()
+        {
+            var dic = new Dictionary<string, string>
+            {
+                { "Array:0", "a" },
+                { "Array:1", "b" },
+                { "Array:2", "c" },
+            };
+
+            var configurationBuilder = new ConfigurationBuilder();
+            configurationBuilder.AddInMemoryCollection(dic);
+            var config = configurationBuilder.Build();
+
+            var options = config.Get<RecordWithArrayParameter>();
+            Assert.Equal(new string[] { "a", "b", "c" }, options.Array);
+        }
+
         [Fact]
         public void CanBindReadonlyRecordStructOptions()
         {
@@ -2525,7 +2546,7 @@ public void CanBindToClassWithNewProperties()
         {
             /// the source generator will bind to the most derived property only.
             /// the reflection binder will bind the same data to all properties (including hidden).
-            
+
             var config = TestHelpers.GetConfigurationFromJsonString("""
                 {
                     "A": "AVal",