-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
ConfigurationBuilder add elements to initialized array instead of replacing it #36569
Comments
Judging by the existence of tests that verify exactly that the binder behaves that way, I would assume that this behavior is fully intended. |
Such behavior doesn't seem to be obvious... I really expected 100% equal objects in config and in runtime. May be there should be an option? Count this as suggestion. |
This is really bothering me as well. If I write something in my config.json, I expect it to overwrite default values, not add to it. |
+1 to fix this behavior. |
As this would be a breaking change, can we consider it in asp.net core 3.0 that is expected to ship this fall? |
This is causing me a lot of grief. What is the plan for this? |
I am also having trouble because of this behavior. A fairly simple fix should be to add a BinderOptions flag that, if set, calls For anybody else having trouble with this in the meantime, creating a shim class inheriting from |
I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label. |
@tvanbaak would you be interested in providing API proposal with sample usage for this API addition? Guideline for API proposal is here |
@maryamariyan Sure, I was thinking of forking the configuration library repo and checking if the solution is as easy as I think it is. I can put an official API proposal in when I have the time to do that in the next month or so. |
Sure. Sounds good. The API proposal would be there to cover motivation for change, sample usage, and any risk involved with making the new changes. If you wanted to get more guidance check out api-review-process.md.
@tvanbaak please note, this issue was transferred from dotnet/extensions and some of extensions source code moved to dotnet/runtime earlier in May. The note below is just FYI in case you are keen on testing things out or plan on making future contributions in this repo. In case you are new to this repo and interested to contribute:If you wanted to play around with (e.g. Microsoft.Extensions.Configuration library) extensions source code or any other libraries in runtime, your first stop is to check workflow/requirements for dotnet/runtime. Once you've made sure your machine has the requirements to build, then perhaps keep the building instructions with you as cheatsheet. That would help you while you do some prototypes on e.g. Microsoft.Extensions.Configuration, And then in a sample console or web application point to your local copy of Microsoft.Extensions.Configuration by: <Reference Include="C:\runtimepath\artifacts\bin\Microsoft.Extensions.Configuration\netstandard2.0-Debug\Microsoft.Extensions.Configuration.dll" /> If the work involves API addition, then follow updating-ref-source.md section |
as a proposal ...
|
as a proposal (alternative way) ... var appModules = new List<string>();
cfg.GetSection("Modules").Bind(appModules, o => o.ArrayMergeMode = ConfigurationArrayMergeMode.Override); public enum ConfigurationArrayMergeMode
{
Override,
Merge,
Append
} However I deem that the option Update 30/03/2022I'd like to clarify things how I see it, based on my experience. 1. Override (default)The default behaviour (IMHO) should be to replace the existing array. For example: "Modules": [
"Module1",
"Module2",
"Module3"
] appsettings.development.json: "Modules": [
"Module4",
"Module5"
] Results in Please consider include other methods: 2. MergeThis is the current (wrong) behavior I never used and I probably will never ever use it. For example: "Modules": [
"Module1",
"Module2",
"Module3"
] appsettings.development.json: "Modules": [
"Module4",
"Module5"
] Results in 3. AppendThis one is interesting behavior can be useful sometimes. For example: "Modules": [
"Module1",
"Module2",
"Module3"
] appsettings.development.json: "Modules": [
"Module4",
"Module5"
] Results in People are asking too: https://stackoverflow.com/questions/51614792/how-to-override-an-asp-net-core-configuration-array-setting-reducing-length-of-t/69676630 |
@Eilon FYI |
This merge/additive behavior is by design. Closing as for the moment we don't have immediate plans to add this feature. |
This is very counter intuitive and very easily lead to errors and wasted time. |
I understand the frustration here but you have to understand that this issue does not happen at bind time. Configuration and binding are actually two separate concepts which operate separately from each other. So even if the binder was able to control whether to append or replace items (which wouldn’t be too difficult to implement, e.g. with a custom collection type), the issue is that the underlying configuration does not support this. I explain this issue also in this Stack Overflow answer but the basic idea is that configuration is loaded as a list of key/value pairs. This happens regardless of what your input format is. So a JSON configuration file is actually flattened into a format where you have flat keys and values. And the hierarchy within JSON is just represented by a key which has a “path” with a colon as the “path separator“. So taking one of the examples above, a JSON
And if there’s now another JSON file that contains
At that time, the information that this is JSON is gone. So when the configuration system now overlays the configuration providers on top of each other, you will get these values:
And of course, if you now run the configuration binder on this, then it shouldn’t be a surprise how the result comes to be. So the underlying format of the configuration system simply does not allow a different behavior since the providers overwrite each other on the key/value items. What theoretically could work is if the binder would bind each configuration provider individually and then apply its own logic in merging this. But the system currently does not allow for this. And changing it now would be a major breaking change considering that this is how the system has been behaving (by design!) for a little over six years. |
I see. Thanks for the explanation. So, the The only workaround I can see today is to use a single string with comma separated values. appsettings.json: {
"Modules": "Module1,Module2,Module3",
} appsettings.development.json: {
"Modules": "Module4,Module5",
} |
Thanks for the engagement, closing, |
From @LordXaosa on Thursday, 18 October 2018 09:39:00
Code:
in appsettings.json:
I expect after config loading there would be only 2 items: "3" and "4"
But there would be all 4. "123","test1","3","4"
Consider this is a bug or there should be an option for replacing array values.
Copied from original issue: dotnet/aspnetcore#3666
The text was updated successfully, but these errors were encountered: