dotnet/runtime

Add support for clearing ConfigurationManager sources (was possible in version 5)

fredjeck opened this issue ยท 13 comments

Problem description

Prior to version 6.0 with the old style application startup it was possible for an application to define how configuration file should be loaded by clearing up the configuration sources and by adding the configuration items in the desired order.
A use case is for instance a shared library with a baseline configuration which needs to be "injected" into the Configuration before the appsettings.* files. so that developers just need to implement minimum overrides in their appsettings files.

With version 6 it is now impossible to

  • Set the Configuration of a WebApplicationBuilder instance (readonly property)
  • Access the Sources of the ConfigurationManager (IConfigurationBuilder.Sources) property as it is private

Proposal

As it is always possible to add new configuration sources to the ConfigurationManager class, the cleanest approach which would not break the current encapsulation would be to add a clear() method to the ConfigurationManager class.

This is the workaround:

var builder = WebApplication.CreateBuilder(args);

((IConfigurationBuilder)builder.Configuration).Sources.Clear();

Moving this to the runtime

cc @halter73

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

Issue Details

Problem description

Prior to version 6.0 with the old style application startup it was possible for an application to define how configuration file should be loaded by clearing up the configuration sources and by adding the configuration items in the desired order.
A use case is for instance a shared library with a baseline configuration which needs to be "injected" into the Configuration before the appsettings.* files. so that developers just need to implement minimum overrides in their appsettings files.

With version 6 it is now impossible to

  • Set the Configuration of a WebApplicationBuilder instance (readonly property)
  • Access the Sources of the ConfigurationManager (IConfigurationBuilder.Sources) property as it is private

Proposal

As it is always possible to add new configuration sources to the ConfigurationManager class, the cleanest approach which would not break the current encapsulation would be to add a clear() method to the ConfigurationManager class.

Author: fredjeck
Assignees: -
Labels:

untriaged, area-Extensions-Configuration

Milestone: -

Thanks a lot for the workaround !

@halter73 - any thoughts on this?

I see three options:

  1. Make ConfigurationManager.Sources public rather than an explicit interface implementation of IConfigurationBuilder.Sources.
  2. Add a new public method like ConfigurationManager.ClearSources(). This could also be an extension method on IConfigurationBuilder, but it doesn't seem that helpful for ConfigurationBuilder so we probably wouldn't do it that way.
  3. Leave things as they are and force people to call ((IConfigurationBuilder)builder.Configuration).Sources.Clear() to clear sources.

I'm leaning towards option 1. The only reason we didn't make Sources public to begin with is that ConfigurationManager also used to read config and the extra visible property could be confusing, but I'm not sure it's any more confusing than a new ConfigurationManager.ClearSources() method which would be less flexible anyway.

The argument for option 3 is that removing sources is far less common than adding them so the vast majority of interaction with the Sources property is done with extension methods like .AddJsonFile(string path). I am somewhat sympathetic to that argument, but I'm not sure I buy that the Sources property is that confusing.

I like 1

namespace Microsoft.Extensions.Configuration
{
    public sealed class ConfigurationManager : IConfigurationBuilder, IConfigurationRoot, IDisposable
    {
-         IList<IConfigurationSource> IConfigurationBuilder.Sources { get; }
+         IList<IConfigurationSource> Sources { get; }

@maryamariyan I moved this to the 7.0.0 milestone and marked it ready for review.

Video

  • Looks good as proposed
    • Should we also expose IConfigurationBuilder.Properties? What happens if Properties aren't cleared? Would there a benefit to add a Clear method that clears both?
namespace Microsoft.Extensions.Configuration
{
    public partial sealed class ConfigurationManager
    {
        // Currently explicit:
        // IList<IConfigurationSource> IConfigurationBuilder.Sources { get; }

        // Now implicit:
        public IList<IConfigurationSource> Sources { get; }
    }
}

Should we also expose IConfigurationBuilder.Properties? What happens if Properties aren't cleared? Would there a benefit to add a Clear method that clears both?

I don't think clearing both is helpful. IConfigurationBuilder.Properties is primarily used by extension methods that need to load config from files with relative paths. If the IConfiguartionBuilder comes from the host, it will usually call SetBasePath for you which sets a property. If you want to use a different base path for sources, you can call SetBasePath yourself to overwrite the property.

It could be used by third-party extension methods to track other state, but I think most people who clear Sources would not want Properties to be cleared because it would make it harder to load new sources from files with relative paths.

@maryamariyan should this be marked up for grabs in case @fredjeck or someone else is interested in doing it before we get to it?

marked up for grabs

Resolved by #66485