diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index c0434440df015e..c966e522be1517 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/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 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); + } } } + /// + /// 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. + /// + /// The property to reset. + /// The instance to reset the property on. + /// The binder options. + /// + /// 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. + /// + [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 _)); } } } + /// + /// Create an instance of the specified type. + /// + /// The type to create an instance of. + /// The configuration to bind to the instance. + /// The binder options. + /// The parameters of the constructor used to create the instance. + /// The created instance. + /// If the type cannot be created. + /// + /// 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. + /// [RequiresDynamicCode(DynamicCodeWarningMessage)] [RequiresUnreferencedCode( "In case type is a Nullable, 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); } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 17ec28e06f358f..0e6e09e637c0e0 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/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 diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index 8ad298190073a3..406738478d1c55 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/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); } + /// + /// This test to ensure the binding of the constructor/property array is done once and not duplicating values in the array. + /// + [Fact] + public void CanBindOnParametersAndProperties_RecordWithArrayConstructorParameter() + { + var dic = new Dictionary + { + { "Array:0", "a" }, + { "Array:1", "b" }, + { "Array:2", "c" }, + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + + var options = config.Get(); + 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",