Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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);
Comment thread
tarekgh marked this conversation as resolved.
}
}
}

/// <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)
Expand Down Expand Up @@ -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)
{
Expand All @@ -401,7 +436,7 @@ private static void BindInstance(
}
else
{
bindingPoint.SetValue(CreateInstance(type, config, options));
bindingPoint.SetValue(CreateInstance(type, config, options, out constructorParameters));
}
}

Expand All @@ -424,7 +459,7 @@ private static void BindInstance(
}
else
{
BindProperties(bindingPoint.Value, config, options);
BindProperties(bindingPoint.Value, config, options, constructorParameters);
}
}
}
Expand All @@ -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.")]
Expand All @@ -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));
Expand Down Expand Up @@ -495,6 +547,8 @@ private static object CreateInstance(
parameterValues[index] = BindParameter(parameters[index], type, config, options);
}

constructorParameters = parameters;

return constructor.Invoke(parameterValues);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ public string Color
}
}

public record RecordWithArrayParameter(string[] Array);

public readonly record struct ReadonlyRecordStructTypeOptions(string Color, int Length);

public class ContainerWithNestedImmutableObject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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",
Expand Down