-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Description
Currently in our configuration provider implementation if the value of a configuration key is set to null, the value of the dictionary entry is set to null, but the key still remains in the dictionary, so checking for existence on the parent keys will return wrong (positive) results.
I think we can fix this by simply changing the behavior of the setter to remove the key from the dictionary.
This should unblock dotnet/extensions#314. From one of my comments there:
The following test attempts to demonstrate why it is desirable that setting a key to null removes it from the tree reachable through GetChildren() and AsEnumerable().
For brevity I used the Exists() extension method which was contribute by @andrewlock in aspnet/Configuration#521 and got merged today. IMO the behavior of this method is completely consistent with the semantics of IConfiguration: a section exists only if it has a value or if it has children sections.
[Fact]
public void ConfigurationRemoveTest()
{
var config = new ConfigurationBuilder()
.AddInMemoryCollection()
.Build();
var section = config.GetSection("a:b:c"); // represents an arbitrary child key in the key-space
Assert.Null(section.Value); // does not have a value
Assert.False(section.Exists()); // hence it doesn't exists yet
Assert.Equal(config.AsEnumerable().Count(), 0); // in fact the configuration is still empty
section.Value = "x"; // same as config["a:b:c"] = "x";
Assert.True(config.GetSection("a").Exists()); // the grand parent exists because it has a grand child
Assert.True(config.GetSection("a:b").Exists()); // the parent exists because it has a child
Assert.True(config.GetSection("a:b:c").Exists()); // and the child exists because it has a value
// Unfortunately things start getting inconsistent if you try to set the value to null
config["a:b:c"] = null;
Assert.True(config.GetSection("a").Exists()); // the grand parent exists because it has a grand child
Assert.True(config.GetSection("a:b").Exists()); // the parent exists because it has a child
Assert.False(config.GetSection("a:b:c").Exists()); // but the child doesn't actually exist!!!
}