Add feature to bind a single element to an array on ConfigurationBinder.

[safern]

Background and motivation

Currently with the JSON Configuration Provider you can do:

{
   "tvshow": {
        "metadata": [
            {
               "name": "PrisonBreak"
            }
         ]
    }
}

and then bind to without any problem:

public class TvShow
{
  public Metadata[] Metadata { get; set; }
}

public class Metadata
{
  public string Name { get; set; }
}

However if we want to bind to this from the xml configuration provider using:

<TvShow>
  <Metadata>
    <Name>Prison Break</Name>
  </Metadata>
</TvShow>

We don't have a way to know that TvShows can contain multiple Metadata elements and that is bound to an array, so we fail to bind when a single element is provided as the key doesn't contain any index in the target array as the binder expects when binding to an array, and we return a Metadata[] with a null object.

In order to fix this we would need to change the binder to account for a key without the index suffix when the child elements don't contain any index (a single element); but this would be breaking for other providers where this is not expected. Consider changing the original JSON so that tvshows is an object with a single metadata, this would bind even though json is not specifying an array:

{
   "tvshow": {
        "metadata": {
            "name": "PrisonBreak"
          }
    }

We want to introduce a new flag to enable this behavior.

API Proposal

namespace Microsoft.Extensions.Configuration
{
     public class BinderOptions
     {
          public bool BindSingleElementsToArray { get; set; }
     }
}

API Usage

With the given xml example above.

var configuration = new ConfigurationBuilder().AddXmlFile("MyFile.xml").Build();

var tvShow = new tvShow();
configuration.Bind(tvShow, o => { o.BindSingleElementsToArray = true });

Risks

This would be a breaking change and could impact perf a little bit but only if the flag is set to true.

[dotnet-issue-labeler[bot]]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @maryamariyan, @safern See info in area-owners.md if you want to be subscribed.

Issue Details

---

### Background and motivation Currently with the JSON Configuration Provider you can do: ```json { "tvshow": { "metadata": [ { "name": "PrisonBreak" } ] } } ``` and then bind to without any problem: ```cs public class TvShow { public Metadata[] Metadata { get; set; } } public class Metadata { public string Name { get; set; } } ``` However if we want to bind to this from the xml configuration provider using: ```xml Prison Break ``` We don't have a way to know that `TvShows` can contain multiple `Metadata` elements and that is bound to an array, so we fail to bind when a single element is provided as the key doesn't contain any `index` in the target array as the binder expects when binding to an array, and we return a `Metadata[]` with a `null` object. In order to fix this we would need to change the binder to account for a `key` without the `index` suffix when the child elements don't contain any index (a single element); but this would be breaking for other providers where this is not expected. Consider changing the original JSON so that `tvshows` is an object with a single `metadata`, this would bind even though json is not specifying an array: ```json { "tvshow": { "metadata": { "name": "PrisonBreak" } } ``` We want to introduce a new flag to enable this behavior. ### API Proposal ```C# namespace Microsoft.Extensions.Configuration { public class BinderOptions { public bool BindSingleElementsToArray { get; set; } } } ``` ### API Usage With the given xml example above. ```C# var configuration = new ConfigurationBuilder().AddXmlFile("MyFile.xml").Build(); var tvShow = new tvShow(); configuration.Bind(tvShow, o => { o.BindSingleElementsToArray = true }); ``` ### Risks This would be a breaking change and could impact perf a little bit but only if the flag is set to `true`.

Author:	safern
Assignees:	-
Labels:	`api-suggestion`, `blocking-release`, `area-Extensions-Configuration`
Milestone:	6.0.0

[safern]

cc: @maryamariyan @eerhardt @davidfowl

[safern]

I marked it as blocking release as a couple of 1st party costumers need this in order to be able to consume this and move to .NET 6.

[bartonjs]

Video

We feel that the behavior isn't something that people really want to switch on/off, this is just for compatibility across an upgrade boundary.

Either just make the breaking change, or use an AppContext switch to provide "temporary" access to the previous behavior. (API can be added if we're proven wrong).

[safern]

Reopening as we reverted the change introduced in: https://github.com/dotnet/runtime/pull/57204.

Let's try to tackle this on 7.0.0 and consider the two issues this change caused: https://github.com/dotnet/runtime/issues/58330 and https://github.com/dotnet/runtime/issues/58852

[maryamariyan]

Use cases to consider including as test cases for this issue:

Two use cases shown in comments under: https://github.com/dotnet/runtime/issues/58330#issue-982143010 and https://github.com/dotnet/runtime/issues/58330#issuecomment-912745146

And from original PR https://github.com/dotnet/runtime/pull/57204/files here

And another use case is when binding to empty value: https://github.com/dotnet/runtime/issues/58852#issue-991813251

[maryamariyan]

Either just make the breaking change, or use an AppContext switch to provide "temporary" access to the previous behavior. (API can be added if we're proven wrong).

Update on this issue, to fix this in release 7.0, we decided to go with the BinderOptions flag as proposed in the description.

[bartonjs]

Video

The feature area didn't seem convinced that this is the approach they wanted to take, marking as needs-work until they are.

[halter73]

Related: #64840

[SiliconFiend]

Until this is solved upstream, I came up with a workaround. Assume an XML config like this:

<root>
    <parameter_set>
        <parameter>
            <name>Test1</name>
        </parameter>
        <parameter>
            <name>Test2</name>
        </parameter>
    </parameter_set>
</root>

where <parameter> could be one or more entries. Then you define your POCOs/Option pattern objects as such:

class Parameter_Set
{
    [ConfigurationKeyName("parameter")]
    public Parameter ParameterSingle { get; set; } = new Parameter();
    [ConfigurationKeyName("parameter")]
    public Parameter[] Parameters { get; set; } = Array.Empty<Parameter>();
}

class Parameter
{
    public string Name { get; set; } = string.Empty;
    ...
}

Retrieve the Parameter_Set object as follows:

var paramSet = config.GetSection("parameter_set").Get<Parameter_Set>();

The part that makes this work is the duplicated attribute [ConfigurationKeyName(...)], which will either map to the array or the single instance, depending on the source configuration structure. When there is more than one <parameter> elements nested under <parameter_set>, the Parameter_Set.ParameterSingle will be a default instance of Parameter (this is true even if ParameterSingle is declared nullable and is null by default), and Parameter_Set.Parameters will have a length equal to the number of <parameter> elements. Alternatively, if there is only one <parameter> element, the Parameter_Set.Parameters array will have a length of 1 where element 0 (eezo?) is a default instance of Parameter, and Parameter_Set.ParameterSingle will have the actual configuration values . If there are no <parameter> elements, the Parameter_Set.Parameters array will have a length of 0.

It's not awesome and the resultant structure requires special handling by the IConfiguration consumers, but it does work and allows you to leverage the Configuration infrastructure instead of completely rolling your own config reader, etc.

[ThorstenReichert]

Alternatively, if there is only one <parameter> element, the Parameter_Set.Parameters array will have a length of 1 where element 0 (eezo?) is a default instance of Parameter, and Parameter_Set.ParameterSingle will have the actual configuration values .

After playing around with this a bit, it seems that in the single-element case, Parameter_Set.Parameters contains as many default-instances of Parameter, as there are child nodes in <parameter>. So for example the XML

<?xml version="1.0" encoding="UTF-8"?>
<root>
    <parameter_set>
        <parameter>
            <name>Test1</name>
            <data1>1</data1>
            <data2>2</data2>
        </parameter>
    </parameter_set>
</root>

will cause Parameter_Set.Parameters to contain three default-instances of Parameter. Might be because the config provider now has three entries under parameter_set:parameter:*?

parameter_set
parameter_set:parameter
parameter_set:parameter:data1
parameter_set:parameter:data2
parameter_set:parameter:name

@safern, @maryamariyan this behavior certainly looks weird, is this intentional, simply not supported, or a bug?

[horato]

I belive the issue comes from the XML parser which has no information about the structure of the xml file and so the only way to recognize a collection is to count the number of elements with the same name. https://github.com/dotnet/runtime/blob/9906682033b4fee6aadc12ac878ffa8fde1edfe1/src/libraries/Microsoft.Extensions.Configuration.Xml/src/XmlStreamConfigurationProvider.cs#L334

This could be solved (at the very least as a workaround) by providing a way to tell the the parser which elements are to be treated as a collection. Simple string collection in the format of "a:b:c" would be more than enough

            bool IsCollection(Prefix prefix, ICollection<XmlConfigurationElement> collection, ICollection<string> collectionDefinitions)
            {
                if (collection.Count > 1)
                    return true;
                if (collection.Count != 1)
                    return false;

                var child = collection.Single();
                var elementFullPath = prefix.AsString + ConfigurationPath.KeyDelimiter + child.ElementName;
                return collectionDefinitions.Contains(elementFullPath);
            }

Or by providing a way to override the parser's default behavior, so that we could implement the detection ourselves.