Edit: We're not going to do most of what is here, but allowing configuration to be bound to options that are immutable through exposing a constructor and read-only properties.
In some scenarios it might be good to have either an interface that doesn't expose mutability or an implementation of the configuration interfaces that throws when changes are attempted. The application could pass instances of these to components that it trusts enough to read configuration but not to modify it.
Note that this is a separate feature from persistence covered in dotnet/extensions#386. Currently configurations are not persistable but they can be modified in memory so they are not read-only either.
:+1: It would be great to see more strict immutability, plus the option for persistence (depending on the outcome of dotnet/extensions#386).
Clearing milestone for visibility
I think it should not be like "in some scenarios, someone might need to enforce readonlyness", but "the overwhelming majority of configuration access is readonly, so this should be enforced statically to prevent mistakes unless opted-out to support complicated RW scenarios".
I was kind of disappointed when I saw that your design is mutable-everything because otherwise it's such a great solution. It unfortunately sets bad example for object oriented design and it's difficult to evangelize developers to not promise stuff they don't mean to fulfill in their interfaces when the standard library requires them to write useless setters.
So, it would be great if you allow this:
public interface ISomeKindOfNetworkConfig
{
int TimeoutInMs { get; }
int Port { get; }
}
static void Main(string[] args)
{
var c = new ConfigurationBuilder()
.AddInMemoryCollection(new List<KeyValuePair<string, string>> {
new KeyValuePair<string, string>("Port", "8080"),
new KeyValuePair<string, string>("Timeout", "60000")})
.Build();
var networkConfig = c.Get<ISomeKindOfNetworkConfig>();
Console.WriteLine(networkConfig.Port);
}
@divega I was bothered about this too, so I wrote a little library that works for my use case.
NuGet: Microsoft.Extensions.Configuration.ImmutableBinder
GitHub: MartinJohns/ConfigurationContrib
It adds a new extension method ImmutableBind<T> which will create an instance and bind the configuration using a constructor. This allows for your option types to have only getter-only auto-properties. Also the library supports binding to IReadOnlyCollection<> and IReadOnlyDictionary<,> (actually it does not currently support other collection or dictionary values). It's first rough version.
Note: I'm not certain if I'm allowed to name my package this way. If there will be any feedback from Microsoft or NuGet I will rename the package.
Had dotnet/extensions#593 closed as a dupe of this, so I will comment here. In dotnet/extensions#593 I asked for binding support to basic immutable POCOs via constructor parameters.
It sounds like what is being suggested/discussed here is instead to make a mutable configuration class _and_ an immutable interface that it implements. This seems over-patterned, and not DRY.
I made an extension method that does what I am describing, and published it at:
Github: halforbit/extensions-configuration
Nuget: Halforbit.Extensions.Configuration
Internally it interprets the enumerated key/values from an IConfigurationSection into a JObject, then uses the JObject.ToObject<>() method to construct the result POCO. This technique has very stable and mature support for creating constructor-based immutable POCOs, as well as anything else that Json.Net supports, which is quite a lot.
I can use my extension method to do what I want to do today, but I know the prescribed approach tends to include Options<>. I am speculating here, but I expect the .Get() method here is probably what is underpinning Options<> functionality, so that would mean it doesn't support immutable POCO either, and at that point my custom extension method would not be used.
Edit: We're not going to do most of what is here
@divega I'm not sure what you mean by this. I hope it doesn't mean this feature won't be implemented.
I would be willing to put together a PR applying the above technique against the Microsoft.Extensions.Configuration source if we can agree it is worthwhile and can be admitted.
@halforbit it was @ajcvickers who added this paragraph at the top the initial comment:
Edit: We're not going to do most of what is here, but allowing configuration to be bound to options that are immutable through exposing a constructor and read-only properties.
So the issue now is being used to track binding to options with read-only properties and constructor parameters.
This is a practical example of this issue:
The issue #https://github.com/IdentityServer/IdentityServer4/issues/2573 is related to this one.
In IdentityServer4 when the client configuration is stored in appsettings.json, the claims of the client cannot be read because the class System.Security.Claims.Claim is immutable and does not have a parameterless constructor.
I think this is pretty important now considering C# 8 will complain about nulls with getters and setters.
Next step is to prepare API proposal and examples using guidelines in https://github.com/dotnet/runtime/blob/master/docs/project/api-review-process.md
Most helpful comment
I think this is pretty important now considering C# 8 will complain about nulls with getters and setters.