Add configurable VariableNameTransformation to EnvironmentVariables config#127503
Add configurable VariableNameTransformation to EnvironmentVariables config#127503svick wants to merge 4 commits intodotnet:mainfrom
Conversation
…onfig (dotnet#125137) Adds: - AddEnvironmentVariables(prefix, variableNameTransformation) overload on IConfigurationBuilder. - EnvironmentVariablesConfigurationSource.VariableNameTransformation: a settable property that, when non-null, replaces the default transformation behavior. - EnvironmentVariablesConfigurationSource.DefaultTransformation: the existing default that replaces __ with the configuration key delimiter (:). - EnvironmentVariablesConfigurationSource.ColonAndDotTransformation: replaces ___ with . and __ with :, allowing dots in configuration keys without the override-by-environment ergonomics being limited to colons. ColonAndDotTransformation is implemented with ValueStringBuilder on .NET and StringBuilder on .NET Framework. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration |
| /// <param name="prefix">A prefix used to filter the environment variables.</param> | ||
| /// <param name="variableNameTransformation">A function that transforms environment variable names. | ||
| /// When <see langword="null"/>, <see cref="EnvironmentVariablesConfigurationSource.DefaultTransformation"/> is used.</param> | ||
| internal EnvironmentVariablesConfigurationProvider(string? prefix, Func<string, string>? variableNameTransformation) |
There was a problem hiding this comment.
I think this constructor should actually be public. I should take it back to API review, right?
There was a problem hiding this comment.
@svick It’s up to you whether to wait and add the new public constructor after the design review, or proceed with the current change and track the constructor addition as a complementary issue.
| var builder = new StringBuilder(name.Length); | ||
| #endif | ||
|
|
||
| // For short strings, this seems to be faster than Append(name.Slice(0, first)). |
There was a problem hiding this comment.
did you measure that? note the builder with every insrt has to check the buffer size before adding it.
There was a problem hiding this comment.
I did arrive at this code by measuring various approaches, but did not recheck. When I measured it now, builder.Append(name.AsSpan(0, first)) ended up being better, so I'm going to change it.
| builder.Append(name[j]); | ||
| } | ||
|
|
||
| int i = first; |
There was a problem hiding this comment.
nit: why not using first instead of adding a new variable? you can rename first to something like index or just i
| <ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Configuration.Abstractions\src\Microsoft.Extensions.Configuration.Abstractions.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'"> |
There was a problem hiding this comment.
Why adding it conditionally. shouldn't ValueStringBuilder work on non .NETCoreApp platforms too?
There was a problem hiding this comment.
I didn't want to add extra dependencies just for this. But I didn't realize that System.Memory is already transitively referenced by this package. So yes, we can use VSB everywhere.
Implements #125137.
API additions
IConfigurationBuilder.AddEnvironmentVariables(string? prefix, Func<string, string>? variableNameTransformation)overload.EnvironmentVariablesConfigurationSource.VariableNameTransformation-- a settableFunc<string, string>?that, when non-null, completely replaces the default transformation.EnvironmentVariablesConfigurationSource.DefaultTransformation-- the existing default that replaces__with the configuration key delimiter (:).EnvironmentVariablesConfigurationSource.ColonAndDotTransformation-- replaces___with.and__with:, so configuration keys containing dots can be set from environment variables.API shape matches @bartonjs's approval comment on the issue.
Implementation notes
ColonAndDotTransformationusesValueStringBuilder+stackalloc char[256]on .NET,StringBuilderon .NET Framework, kept under a single small#if NETregion. No new package references.Prefixis itself passed through the active transformation before comparison, so it should be specified in pre-transformation form (e.g."Logging__"rather than"Logging:"). Documented in<remarks>on thePrefixproperty and the matching overloads.Tests
13 new tests in
EnvironmentVariablesTestcover:[Theory]tables for double/triple/4/5/6 underscores, single-underscore preservation, no-underscore early return).VariableNameTransformationreplacing the default, identity transformation disabling replacement, transformation applied to prefix, source/builder wiring, and a behavioral check thatColonAndDotTransformationis not the default.Note
This PR description and the implementation were drafted with assistance from GitHub Copilot.