Runtime: Empty objects loaded via ConfigurationBuilder.AddJsonFile() are ignored

Created on 31 Jul 2020  路  19Comments  路  Source: dotnet/runtime

Description

Empty objects loaded via ConfigurationBuilder.AddJsonFile() are ignored.


Repro

config.json
{
    "a": "b",
    "c": {
        "d": "e"
    },
    "f": "",
    "g": null,
    "h": {},
    "i": {
        "k": {}
    } 
}
Program.cs
using System;
using Microsoft.Extensions.Configuration;

namespace EmptyConfigSection
{
    class Program
    {
        static void Main(string[] args)
        {
            var config = new ConfigurationBuilder()
                .AddJsonFile("config.json")
                .Build();

            Console.WriteLine("{");

            foreach (var child in config.GetChildren())
            {
                PrintConfigSection(child, padding: "    ");
            }

            Console.WriteLine("}");
        }

        static void PrintConfigSection(IConfigurationSection configSection, string padding)
        {
            if (configSection.Value is null)
            {
                Console.WriteLine($@"{padding}""{configSection.Key}"": {{");

                foreach (var child in configSection.GetChildren())
                {
                    PrintConfigSection(child, $"    {padding}");
                }

                Console.WriteLine($"{padding}}},");
            }
            else
            {
                Console.WriteLine($@"{padding}""{configSection.Key}"": ""{configSection.Value}"",");
            }
        }
    }
}
EmptyConfigSection.csproj
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="5.0.0-preview.7.20364.11" />
  </ItemGroup>

  <ItemGroup>
    <Content Include="config.json">
      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
    </Content>
  </ItemGroup>

</Project>

Expected output

{
    "a": "b",
    "c": {
        "d": "e",
    },
    "f": "",
    "g": "",
    "h": {
    },
    "i": {
        "k": {
        },
    },
}

Actual output

{
    "a": "b",
    "c": {
        "d": "e",
    },
    "f": "",
    "g": "",
}

"h", "i" and "k" are not present in the actual output even though they should be.

The top-level call to IConfiguration.GetChildren() in Program.Main() should return an IEnumerable containing 6 top-level IConfigurationSections: "a", "c", "f", "g", "h" and "k". Instead it returns 4 top-level IConfiguration sections omitting both "h" and "i".

The repro program never even makes a call to GetChildren() that should return the "k" subsection, because it never sees "k"'s parent "i" in the first place.

Configuration

F:\dev> dotnet --info
.NET SDK (reflecting any global.json):
 Version:   5.0.100-preview.7.20330.3
 Commit:    eeb77e1a55

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.20161
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   F:\dev\aspnet\AspNetCore\.dotnet\sdk\5.0.100-preview.7.20330.3\

Host (useful for support):
  Version: 5.0.0-rc.1.20370.4
  Commit:  0e0e648770

I do not think this issue is specific to my configuration however.

Regression?

  • I don't think this is a regression, but I cannot say for sure.
area-Extensions-Configuration up-for-grabs

Most helpful comment

Yeah, seems reasonable to fix the behavior so the config hierarchy gets empty sections so you don't have to do wacky stuff like add random junk to your sections for them to exist

All 19 comments

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

@HaoK do you remember if this was intentional?

I don't think this is intentional, I'd imagine this is going to depend on the behavior of the json parser code: https://github.com/dotnet/runtime/blob/master/src/libraries/Microsoft.Extensions.Configuration.Json/src/JsonConfigurationFileParser.cs

One thing to try @halter73 , compare against the old Newtonsoft provider:

https://github.com/dotnet/extensions/blob/master/src/Configuration/Config.NewtonsoftJson/src/NewtonsoftJsonConfigurationExtensions.cs#L37

Looking at the code: https://github.com/dotnet/runtime/blob/master/src/libraries/Microsoft.Extensions.Configuration.Json/src/JsonConfigurationFileParser.cs#L47

Doesn't look like empty objects are handled, so might just be a bug?

Basically maybe there needs to be a special case in there that inserts an empty child section, right now looks like empty objects are just dropped with no entry into config (maybe the old json provider did the same thing though...) as the code is similar: https://github.com/dotnet/extensions/blob/master/src/Configuration/Config.NewtonsoftJson/src/NewtonsoftJsonConfigurationFileParser.cs#L41

I could see how maybe this is intentional, as the only real functional difference between omitting empty sections would be for code that is looking at the hierarchy of config, as you would never get anything out of empty sections...

I wasn't involved in the initial designs of config, so the actual design/intent for this code was before my time

I could see how maybe this is intentional, as the only real functional difference between omitting empty sections would be for code that is looking at the hierarchy of config, as you would never get anything out of empty sections...

And that's exactly why I noticed this now. See https://github.com/dotnet/aspnetcore/pull/24286.

Yeah, seems reasonable to fix the behavior so the config hierarchy gets empty sections so you don't have to do wacky stuff like add random junk to your sections for them to exist

If this is up from grabs, I could try to look into it.

What is the expected behaviour?
Should it only add an empty child section when it is not already there?
If it already exists, it will not be clearing out the section from all the properties, right?

What's the normal behavior when multiple sources define the same config section?

I assume we'd want to do what we normally do when multiple sources define the same section. If sections with the same path from multiple sources normally get merged, then clearing out sections because the JSON source defined those sections as empty objects seems wrong. If it's the last source wins, and a JSON file defines sections as empty objects, then clearing out those sections seems right.

I don't know if this will be up for grabs. It hasn't been labeled that way yet.

I believe you can think of sections as a union of key/values. So an empty section is basically a no-op, any other section that defines key/values would win in an union. But when it comes to a specific value for a key, that's where overriding comes into play, so foo.bar="baz" and foo.bar="", the last one will win, so you can 'delete' values.

This is just a natural result of how configuration works. Each provider can define different configuration values for a section, i.e. json defines foo.bar = "yes", xml defines foo.baz = "no". The end result is foo.bar and foo.baz show up. That's just how the config provider model is intended to work.

That makes a lot of sense. The alternative would be each top-level section only being defined by one config source which doesn't seem very useful.

I agree this makes sense and it match my expectations. It is also the smaller change as it will not change the current behaviour when a section already exists, as it will only add it if missing.

I was asking as currently there is no easy way to clean up a config section, unless you add an extra json file to clear the section (setting it to null) before your actual json file the set the only properties you want. A good example would be to have a environment specific json settings override where you want to clean up any sort of logging overrides in the app specific appsettings.json. But I guess that is a different issue to discuss.

Going back on topic, if this gets added, I think it would make sense to add support to create empty sections in all configuration providers.
Currently a way to achieve something close to this behaviour that is compatible with all providers is to add a property named empty string, however then you currently need logic to exclude empty string keys if you enumerate the children. Maybe an option would be to exclude the ability to have a empty string property name and use this to syntax to express the empty block in the other providers.

e.g.
environment variable: foo__=1
cli: --foo:=1
json: { "foo": { "" : 1 } }
All these currently lead to a section foo with a child "":"1".

cc @ericstj @maryamariyan

@alefranz were you still up to taking a PR up for this?

Sure, I'd like to! I've missed it was up for grabs, I guess github doesn't send notifications when labels are added.

Created PR #40829

Was this page helpful?
0 / 5 - 0 ratings

Related issues

omajid picture omajid  路  3Comments

GitAntoinee picture GitAntoinee  路  3Comments

btecu picture btecu  路  3Comments

iCodeWebApps picture iCodeWebApps  路  3Comments

noahfalk picture noahfalk  路  3Comments