Attempting to Get a configuration section using a type defined as a record will duplicate collection values.

[vsfeedback]

Recent status can be found in the comment #83803 (comment)

Edited by @tarekgh
This issue claimed the problem is not occurring on .NET 6.0 which is not true. The comment #83803 (comment) clarified that. In short, this issue is not a regression from .NET 6.0. Actually, .NET 7.0 behavior is not throwing.
End of @tarekgh edit

Edit by @tarekgh: additional case to cover when fixing
The same duplication also happens with a class (not a record) when a constructor parameter name differs only by case from a collection property, including a class that uses a params collection constructor (for example a parameter instances and a property Instances). The reflection based runtime binder fix for that case is tracked by #129760 and PR #129775. When fixing this issue in the configuration source generator, the fix should be extended to cover both the record case described above and this class case so the reflection binder and the source generator behave the same.
End of @tarekgh edit

This issue has been moved from a ticket on Developer Community.

---

[severity:It bothers me. A fix would be nice]
The behaviour has changed unexpectedly from how .Net 6.0 used to work.

/* Environment Variables
"MonitorConfig__Services__0__Name": "Service A"
"MonitorConfig__Services__0__Url": "http://localhost:5201/health",
"MonitorConfig__Services__1__Name": "Service B",
"MonitorConfig__Services__1__Url": "http://localhost:5202/health"
*/

/* defined Records types
public record MonitorConfig(ServiceConfig[] Services = null);

public record ServiceConfig(string Name, string Url);
*/

var builder = WebApplication.CreateBuilder(args);

builder.Configuration.AddEnvironmentVariables();

var monitorConfig = builder.Configuration.GetSection("MonitorConfig").Get();

monitorConfig will contain 4 ServiceConfig entries with duplicated instances of both ServiceA and ServceB declarations.

The expectation should be two entries as defined by the environment variables.

If I convert to use class types instead of record types, the behaviour works as expected.

The use of records should be supported and work as it did for .Net 6.0 using the same code, as I should not have to go and rewrite records to as classes after porting to .Net 7.

---

Original Comments

Feedback Bot on 3/8/2023, 05:37 PM:

(private comment, text removed)

---

Original Solutions

Chris Westermann solved on 3/9/2023, 05:05 AM, 0 votes:

I found that if I override the default constructor, then I get the behaviour I would expect

public record MonitorConfig(ServiceConfig[] Services)
{
public MonitorConfig()
: this((ServiceConfig[])null)
{ }
}

While this is a workaround, it seems odd I would need to do this since, conceptually, setting the default value of the positional parameter to null should produce the same behaviour and not cause the actual instance to have twice as many array entries as expected.

[mitchdenny]

Without the workaround of adding a default public constructor, this code actually throws an exception in .NET 6.0 - but the original posted said it used to work. Here is the behaviors based on my experimentation:

Runtime	Has default public constructor	Behavior
.NET 6.0	Yes	Runs - creates two ServiceConfig elements
.NET 7.0	Yes	Runs - creates two ServicConfig elements
.NET 6.0	No	Runtime exception about lack of public constructor
.NET 7.0	No	Runs - creates four ServiceConfig elements

It looks like logic was added in .NET 7.0 to support non-default constructors where the parameters line up with properties (basically record types). But it's possible that there is a bug in there somewhere.

[mitchdenny]

I transferred this over to the runtime repo because I think that is where the bug is.

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This issue has been moved from a ticket on Developer Community.

---

[severity:It bothers me. A fix would be nice]
The behaviour has changed unexpectedly from how .Net 6.0 used to work.

/* Environment Variables
"MonitorConfig__Services__0__Name": "Service A"
"MonitorConfig__Services__0__Url": "http://localhost:5201/health",
"MonitorConfig__Services__1__Name": "Service B",
"MonitorConfig__Services__1__Url": "http://localhost:5202/health"
*/

/* defined Records types
public record MonitorConfig(ServiceConfig[] Services = null);

public record ServiceConfig(string Name, string Url);
*/

var builder = WebApplication.CreateBuilder(args);

builder.Configuration.AddEnvironmentVariables();

var monitorConfig = builder.Configuration.GetSection("MonitorConfig").Get();

monitorConfig will contain 4 ServiceConfig entries with duplicated instances of both ServiceA and ServceB declarations.

The expectation should be two entries as defined by the environment variables.

If I convert to use class types instead of record types, the behaviour works as expected.

The use of records should be supported and work as it did for .Net 6.0 using the same code, as I should not have to go and rewrite records to as classes after porting to .Net 7.

---

Original Comments

Feedback Bot on 3/8/2023, 05:37 PM:

(private comment, text removed)

---

Original Solutions

Chris Westermann solved on 3/9/2023, 05:05 AM, 0 votes:

I found that if I override the default constructor, then I get the behaviour I would expect

public record MonitorConfig(ServiceConfig[] Services)
{
public MonitorConfig()
: this((ServiceConfig[])null)
{ }
}

While this is a workaround, it seems odd I would need to do this since, conceptually, setting the default value of the positional parameter to null should produce the same behaviour and not cause the actual instance to have twice as many array entries as expected.

Author:	vsfeedback
Assignees:	-
Labels:	untriaged, area-Extensions-Configuration, Author: Migration Bot :robot:
Milestone:	-

[tarekgh]

The Issue here is during the binding to types (like record), we try to create instance of the type:

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

Line 385 in 6246ce3

bindingPoint.SetValue(CreateInstance(type, config, options));

CreateInstance finds out the type doesn't have the parameter-less constructor and then tries to do binding through the constructor having parameters.

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

Line 468 in 6246ce3

parameterValues[index] = BindParameter(parameters[index], type, config, options);

Will bind the configuration to the parameters and send call the constructor with the initialized parameters. The constructor will set the parameters values to the properties.

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

Line 471 in 6246ce3

return constructor.Invoke(parameterValues);

After that will continue the binding process on the created object by going through binding its properties

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

Line 408 in 6246ce3

BindProperties(bindingPoint.Value, config, options);

This will cause binding again the property which was already set during the constructor initialization which will duplicate the entries inside the created list as the property is of type Array.

This is why when having a parameter-less constructor fix the issue because it will be used and will not do any config binding during creating the object (through CreateInstance).

[heldersousa-planetpayment]

@tarekgh are there any plans for this issue?

By the way, this is not specific to records. If a class is used instead of a record, and that class has a constructor that receives an array as a parameter, the same behaviour (duplicate entries in the array) will happen.

There is even more bizarre behaviour, even if there is a parameter-less constructor. If the array Config2 in example below is initialized with a default value, the result will contain the default values plus the values defined in the configuration.

Example:

public class MyConfig
{
   public string Config1 { get; set; } = "a"
   public string[] Config2 { get; set; } = ["A"]
}

{
  "MyConfig" : {
    "Config1": "b"
    "Config2": ["B", "C"]
  }
}

After binding the config to the object, the array Config2 will have 3 strings ["A", "B", "C"] instead of overwriting the default value with ["B", "C"]. The value for Config1 is overwritten with the value "b" as expected.

Maybe this is an old behaviour and it happens even in .NET 6.0 (I didn't check to be honest), but I can't think of a situation where appending to the array would make more sense than overwriting the array with the values in the config. For me, this looks like a bug that should be fixed as soon as possible.

[tarekgh]

@heldersousa-planetpayment What you are explaining regarding the class differs from the original issue at hand. The initial problem revolved around a property binding occurring twice in the configuration. Conversely, your concern pertains to the distinction between extending and mutating the original collection. In .NET 7.0, a breaking change was introduced, primarily addressing dictionaries but applicable to other collections like arrays. It's not clear why extending array values instead of overriding them might be perceived as unconventional. Both approaches have valid arguments, and I recall encountering scenarios where extending the collection was deemed necessary.

[heldersousa-planetpayment]

@tarekgh Thank you so much for your quick reply.

Our issue is also related to the original issue reported in this ticket and it also applies to classes when there is a non parameter-less constructor without the parameter-less constructor, not just records. I only mentioned the second behaviour as something that doesn't feel natural.

I appreciate there may be cases where the example provided in Binding config to dictionary extends values makes sense, but when loading the configuration the following way and MyConfig is a record, no one is expecting the append behaviour on collections because records are supposed to define immutable properties, i.e, the array won't grow or shrink after it is initialized.

public record MyConfig(string Config1, string[] Config2);

(...)

var myConfig = configuration.GetRequiredSection("MyConfig").Get<MyConfig>();

In my opinion, if a config value is set on a property when calling the non parameter-less constructor (which is the case when working with records), that property should be excluded from BindProperties logic. From what I saw in the code, it looks like the logic is parsing the config and setting the config value twice, once in the constructor and a second time when doing the BindProperties logic. This behaviour is innocuous while parsing and setting primitive values, but if the config value is another class with a big and complex structure, it may be costly to do it twice.

If the BindProperties logic is excluded for properties used in the constructor, it would fix the original issue of duplicated entries in the array/list. This is the bug that I think needs to be fixed.