Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support repeated elements in XML configuration #36561

Closed
amoerie opened this issue Oct 27, 2017 · 13 comments · Fixed by #44608
Closed

Support repeated elements in XML configuration #36561

amoerie opened this issue Oct 27, 2017 · 13 comments · Fixed by #44608
Labels
area-Extensions-Configuration enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@amoerie
Copy link
Contributor

amoerie commented Oct 27, 2017

When repeating an element in appsettings.xml, an exception occurs because a duplicate key will be produced

Functional impact

Configuration that is possible from JSON is not always possible from XML, for example configuring Serilog sinks are configured as an array in JSON. Arrays in XML are not supported by the XmlConfigurationProvider.

Minimal repro steps

  1. Create a .NET Core 2.0 application with an appsettings.xml file
  2. Fill in the appsettings.xml file like this:
          <settings>
            <DefaultConnection>
                <ConnectionString>TestConnectionString1</ConnectionString>
                <Provider>SqlClient1</Provider>
            </DefaultConnection>
            <DefaultConnection>
                <ConnectionString>TestConnectionString2</ConnectionString>
                <Provider>SqlClient2</Provider>
            </DefaultConnection>
          </settings>
  1. Load this file with the following snippet in Program.cs or Startup.cs
new ConfigurationBuilder()
      .SetBasePath(Directory.GetCurrentDirectory())
      .AddXmlFile($"appsettings.xml")
      .Build();
  1. Startup the application

Expected result

Unique keys should be produced for each entry in the XML settings

I would expect the following output:

"DefaultConnection:0:ConnectionString"="TestConnectionString1"
"DefaultConnection:0:Provider"="SqlClient1"
"DefaultConnection:1:ConnectionString"="TestConnectionString2"
"DefaultConnection:1:Provider"="SqlClient2"

Actual result

System.FormatException occurred
  HResult=0x80131537
  Message=A duplicate key 'DefaultConnection:ConnectionString' was found.
  Source=<Cannot evaluate the exception source>

Further technical details

I took the liberty of actually implementing something like this, the full source code is available at https://github.com/amoerie/configuration.xml

I've added extra unit tests to cover the various scenarios (turns out there's a magical 'Name' attribute with some special rules there) and all tests seem to be green.

I refrained from immediately creating a pull request, because nobody (besides myself) was asking for this and it is actually a considerable refactoring of the existing code, so it seemed better to start a conversation first. Thank you for your consideration.

@BradBarnich
Copy link

I just ran into this myself, thank you for the code sample!

@renatotkr
Copy link

nice work @amoerie, I had the same problem, in my case I need a list of nodes, which is not possible right now

I'm converting to json for now

@bdovaz
Copy link

bdovaz commented Nov 11, 2017

I'm also interested in this, please make a pull request!

@amoerie
Copy link
Contributor Author

amoerie commented Nov 11, 2017

I have little faith a PR would be approved if nobody from Microsoft responds..

@CiprianMo
Copy link

This became an issue for me too, there will most likely be more people with the same problem in the future

@amoerie
Copy link
Contributor Author

amoerie commented Jan 26, 2018

If there is interest from Microsoft for this, I am willing to make a PR, but seeing as they've not responded for 3 months I am assuming they are either inundated with issues and PRs or they simply don't care about this. (XML isn't exactly hip anymore 😄 )

Still, should there be interest from Microsoft in this feature, I would love to hear it. We still have to support MSI installers and updating XML files is magnitudes easier than updating JSON from an MSI installer.

@akrobet
Copy link

akrobet commented Feb 3, 2018

Could you maybe create a nuget package from this?

@amoerie
Copy link
Contributor Author

amoerie commented Feb 5, 2018

Well I'm hesitant to distribute this, as I don't really want to be the "owner" of this thing. And when, or if, Microsoft decides to fix this issue, that would immediately make my NuGet package redundant.

Of course you're free to just include the source code + unit tests from the repository I created. For now, that's what I would recommend.

@akrobet
Copy link

akrobet commented Feb 5, 2018

Okay, thanks for the reply, will do that.

@amoerie
Copy link
Contributor Author

amoerie commented Apr 24, 2018

I have further information pertaining to this issue: the suggested workaround on the official docs, which suggest to use the "Name" attribute, actually introduces a new, subtle problem: it becomes impossible to then have a child element called 'Name'

Minimal repro steps

  1. Create a .NET Core 2.0 application with an appsettings.xml file
  2. Fill in the appsettings.xml file like this:
          <settings>
            <Foo Name="0">
                <Name>The first Foo</Name>
            </Foo>
            <Foo Name="1">
                <Name>The second Foo</Name>
            </Foo>
          </settings>
  1. Load this file with the following snippet in Program.cs or Startup.cs
new ConfigurationBuilder()
      .SetBasePath(Directory.GetCurrentDirectory())
      .AddXmlFile($"appsettings.xml")
      .Build();
  1. Startup the application

Expected result

Unique keys should be produced for each entry in the XML settings

I would expect the following output:

"Foo:0:Name"="The first foo"
"Foo:1:Name"="The second foo"

Actual result

System.FormatException occurred
  HResult=0x80131537
  Message=A duplicate key 'Foo:Name' was found.
  Source=<Cannot evaluate the exception source>
  1. Edit the appsettings.xml as suggested by the documentation
          <settings>
            <Foo Name="First">
                <Name>The first Foo</Name>
            </Foo>
            <Foo Name="Second">
                <Name>The second Foo</Name>
            </Foo>
          </settings>

Expected result

Unique keys should be produced for each entry in the XML settings

I would expect the following output:

"Foo:First:Name"="The first foo"
"Foo:Second:Name"="The second foo"

Actual result

An exception occurs. (Not on my dev machine right now)

Real world use case

We encountered this issue today trying to configure Serilog Sinks in an XML file.
Each sink has a Name property which uniquely identifies the type of sink that should be written to.

@amoerie
Copy link
Contributor Author

amoerie commented Apr 25, 2018

In spite of not receiving any sign of life from MS yet, I've optimistically created a pull request that solves this problem: #44608

Edit 2019: Fix link to PR
Edit 2020: Fix link to PR again
Edit 2020 v2: Fix link to PR again

@jez9999
Copy link

jez9999 commented Nov 10, 2018

Adding my support for the PR by @amoerie - a good solution for the problem.

@ajcvickers ajcvickers transferred this issue from aspnet/Configuration Dec 15, 2018
@analogrelay analogrelay transferred this issue from dotnet/extensions May 15, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Runtime untriaged New issue has not been triaged by the area owner labels May 15, 2020
@analogrelay analogrelay added this to the Future milestone May 15, 2020
@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Jul 1, 2020
@maryamariyan maryamariyan modified the milestones: Future, 6.0.0 Aug 8, 2020
@maryamariyan maryamariyan added the enhancement Product code improvement that does NOT require public API changes/additions label Oct 20, 2020
@amoerie
Copy link
Contributor Author

amoerie commented Oct 22, 2020

For future visitors of this issue, the code related to this issue has moved repositories twice already. The last, up to date merge request can now be found here: #44608

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 5, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 10, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Configuration enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
10 participants