diff --git a/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj b/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj index f4d65766..12aa4e1f 100644 --- a/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj +++ b/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj @@ -5,8 +5,8 @@ 4 - 2 - 1 + 3 + 0 diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj index 4068f7b4..d195cfa8 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj @@ -4,8 +4,8 @@ 4 - 2 - 1 + 3 + 0 diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 44b2bcc3..78161707 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -23,6 +23,7 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP // IFeatureDefinitionProviderCacheable interface is only used to mark this provider as cacheable. This allows our test suite's // provider to be marked for caching as well. private readonly IConfiguration _configuration; + private readonly ConfigurationFeatureDefinitionProviderOptions _options; private IEnumerable _dotnetFeatureDefinitionSections; private IEnumerable _microsoftFeatureDefinitionSections; private readonly ConcurrentDictionary> _definitions; @@ -37,9 +38,13 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP /// Creates a configuration feature definition provider. /// /// The configuration of feature definitions. - public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) + /// The options for the configuration feature definition provider. + public ConfigurationFeatureDefinitionProvider( + IConfiguration configuration, + ConfigurationFeatureDefinitionProviderOptions options = null) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); + _options = options ?? new ConfigurationFeatureDefinitionProviderOptions(); _definitions = new ConcurrentDictionary>(); _changeSubscription = ChangeToken.OnChange( @@ -229,6 +234,13 @@ private IEnumerable GetDotnetFeatureDefinitionSections() private IEnumerable GetMicrosoftFeatureDefinitionSections() { + if (!_options.CustomConfigurationMergingEnabled) + { + return _configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName) + .GetChildren(); + } + var featureDefinitionSections = new List(); FindFeatureFlags(_configuration, featureDefinitionSections); diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProviderOptions.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProviderOptions.cs new file mode 100644 index 00000000..3893edc4 --- /dev/null +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProviderOptions.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +namespace Microsoft.FeatureManagement +{ + /// + /// Options that control the behavior of the . + /// + public class ConfigurationFeatureDefinitionProviderOptions + { + /// + /// Controls whether to enable the custom configuration merging logic for Microsoft schema feature flags or fall back to .NET's native configuration merging behavior. + /// + /// + /// This option only affects Microsoft schema feature flags (e.g. feature_management:feature_flags arrays). .NET schema feature flags are not affected by this setting. + /// + /// The uses custom configuration merging logic for Microsoft schema feature flags to ensure that + /// feature flags with the same ID from different configuration sources are merged correctly based on their logical identity rather than array position. + /// By default, the provider bypasses .NET's native array merging behavior which merges arrays by index position and can lead to unexpected results when feature flags are defined across multiple configuration sources. + /// + /// Consider the following configuration sources: + /// Configuration Source 1: + /// { + /// "feature_management": { + /// "feature_flags": [ + /// { + /// "id": "feature1", + /// "enabled": true + /// }, + /// { + /// "id": "feature2", + /// "enabled": false + /// } + /// ] + /// } + /// } + /// + /// Configuration Source 2: + /// { + /// "feature_management": { + /// "feature_flags": [ + /// { + /// "id": "feature2", + /// "enabled": true + /// } + /// ] + /// } + /// } + /// + /// With custom merging: + /// - feature1: enabled = true + /// - feature2: enabled = true (last declaration wins) + /// + /// With native .NET merging: + /// - feature1 would be overwritten by feature2 from source 2 (index-based merging, e.g. feature_flags:0:id) + /// - feature2: enabled = false (from source 1, index 1) + /// + public bool CustomConfigurationMergingEnabled { get; set; } + } +} diff --git a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj index 264473ca..560a6c07 100644 --- a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj +++ b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj @@ -5,8 +5,8 @@ 4 - 2 - 1 + 3 + 0 diff --git a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs index 602dd20c..806c66fa 100644 --- a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs +++ b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs @@ -36,17 +36,18 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec AddCommonFeatureManagementServices(services); - services.AddSingleton(sp => new FeatureManager( - sp.GetRequiredService(), - sp.GetRequiredService>().Value) - { - FeatureFilters = sp.GetRequiredService>(), - SessionManagers = sp.GetRequiredService>(), - Cache = sp.GetRequiredService(), - Logger = sp.GetRequiredService().CreateLogger(), - TargetingContextAccessor = sp.GetService(), - AssignerOptions = sp.GetRequiredService>().Value - }); + services.AddSingleton(sp => + new FeatureManager( + sp.GetRequiredService(), + sp.GetRequiredService>().Value) + { + FeatureFilters = sp.GetRequiredService>(), + SessionManagers = sp.GetRequiredService>(), + Cache = sp.GetRequiredService(), + Logger = sp.GetRequiredService().CreateLogger(), + TargetingContextAccessor = sp.GetService(), + AssignerOptions = sp.GetRequiredService>().Value + }); services.TryAddSingleton(sp => sp.GetRequiredService()); @@ -70,7 +71,9 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec } services.AddSingleton(sp => - new ConfigurationFeatureDefinitionProvider(configuration) + new ConfigurationFeatureDefinitionProvider( + configuration, + sp.GetRequiredService>().Value) { RootConfigurationFallbackEnabled = true, Logger = sp.GetRequiredService().CreateLogger() @@ -96,17 +99,18 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService AddCommonFeatureManagementServices(services); - services.AddScoped(sp => new FeatureManager( - sp.GetRequiredService(), - sp.GetRequiredService>().Value) - { - FeatureFilters = sp.GetRequiredService>(), - SessionManagers = sp.GetRequiredService>(), - Cache = sp.GetRequiredService(), - Logger = sp.GetRequiredService().CreateLogger(), - TargetingContextAccessor = sp.GetService(), - AssignerOptions = sp.GetRequiredService>().Value - }); + services.AddScoped(sp => + new FeatureManager( + sp.GetRequiredService(), + sp.GetRequiredService>().Value) + { + FeatureFilters = sp.GetRequiredService>(), + SessionManagers = sp.GetRequiredService>(), + Cache = sp.GetRequiredService(), + Logger = sp.GetRequiredService().CreateLogger(), + TargetingContextAccessor = sp.GetService(), + AssignerOptions = sp.GetRequiredService>().Value + }); services.TryAddScoped(sp => sp.GetRequiredService()); @@ -130,7 +134,9 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService } services.AddSingleton(sp => - new ConfigurationFeatureDefinitionProvider(configuration) + new ConfigurationFeatureDefinitionProvider( + configuration, + sp.GetRequiredService>().Value) { RootConfigurationFallbackEnabled = true, Logger = sp.GetRequiredService().CreateLogger() @@ -145,7 +151,11 @@ private static void AddCommonFeatureManagementServices(IServiceCollection servic services.AddMemoryCache(); - services.TryAddSingleton(); + services.TryAddSingleton(sp => + new ConfigurationFeatureDefinitionProvider( + sp.GetRequiredService(), + sp.GetRequiredService>().Value) + ); services.AddScoped(); diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index 5c71a60b..1b8ae34b 100644 --- a/tests/Tests.FeatureManagement/FeatureManagementTest.cs +++ b/tests/Tests.FeatureManagement/FeatureManagementTest.cs @@ -398,6 +398,11 @@ public async Task LastFeatureFlagWins() [Fact] public async Task MergesFeatureFlagsFromDifferentConfigurationSources() { + var mergeOptions = new ConfigurationFeatureDefinitionProviderOptions() + { + CustomConfigurationMergingEnabled = true + }; + /* * appsettings1.json * Feature1: true @@ -425,18 +430,99 @@ public async Task MergesFeatureFlagsFromDifferentConfigurationSources() .AddJsonFile("appsettings3.json") .Build(); - var featureManager1 = new FeatureManager(new ConfigurationFeatureDefinitionProvider(configuration1)); + var featureManager1 = new FeatureManager(new ConfigurationFeatureDefinitionProvider(configuration1, mergeOptions)); Assert.True(await featureManager1.IsEnabledAsync("FeatureA")); Assert.True(await featureManager1.IsEnabledAsync("FeatureB")); Assert.True(await featureManager1.IsEnabledAsync("Feature1")); Assert.False(await featureManager1.IsEnabledAsync("Feature2")); // appsettings2 should override appsettings1 - var featureManager2 = new FeatureManager(new ConfigurationFeatureDefinitionProvider(configuration2)); + var featureManager2 = new FeatureManager(new ConfigurationFeatureDefinitionProvider(configuration2, mergeOptions)); Assert.True(await featureManager2.IsEnabledAsync("FeatureA")); Assert.True(await featureManager2.IsEnabledAsync("FeatureB")); Assert.True(await featureManager2.IsEnabledAsync("FeatureC")); Assert.False(await featureManager2.IsEnabledAsync("Feature1")); // appsettings3 should override previous settings Assert.False(await featureManager2.IsEnabledAsync("Feature2")); // appsettings3 should override previous settings + + // + // default behavior + var featureManager3 = new FeatureManager(new ConfigurationFeatureDefinitionProvider(configuration1)); + Assert.False(await featureManager3.IsEnabledAsync("FeatureA")); // it will be overridden by FeatureB + Assert.True(await featureManager3.IsEnabledAsync("FeatureB")); + Assert.True(await featureManager3.IsEnabledAsync("Feature1")); + Assert.False(await featureManager3.IsEnabledAsync("Feature2")); // appsettings2 should override appsettings1 + + IConfiguration configuration3 = new ConfigurationBuilder() + .AddJsonFile("appsettings1.json") + .AddInMemoryCollection(new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["feature_management:feature_flags:0:enabled"] = bool.FalseString, + ["feature_management:feature_flags:1:enabled"] = bool.FalseString, + }) + .Build(); + var featureManager4 = new FeatureManager(new ConfigurationFeatureDefinitionProvider(configuration3)); + Assert.False(await featureManager4.IsEnabledAsync("Feature1")); + Assert.False(await featureManager4.IsEnabledAsync("Feature2")); + Assert.True(await featureManager4.IsEnabledAsync("FeatureA")); + + // + // DI usage + var services1 = new ServiceCollection(); + services1 + .AddSingleton(configuration2) + .AddFeatureManagement(); + services1.Configure(o => + { + o.CustomConfigurationMergingEnabled = true; + }); + ServiceProvider serviceProvider1 = services1.BuildServiceProvider(); + IFeatureManager featureManager5 = serviceProvider1.GetRequiredService(); + + Assert.True(await featureManager5.IsEnabledAsync("FeatureA")); + Assert.True(await featureManager5.IsEnabledAsync("FeatureB")); + Assert.True(await featureManager5.IsEnabledAsync("FeatureC")); + Assert.False(await featureManager5.IsEnabledAsync("Feature1")); + Assert.False(await featureManager5.IsEnabledAsync("Feature2")); + + var services2 = new ServiceCollection(); + services2 + .AddSingleton(configuration2) + .AddFeatureManagement(); + ServiceProvider serviceProvider2 = services2.BuildServiceProvider(); + IFeatureManager featureManager6 = serviceProvider2.GetRequiredService(); + + Assert.False(await featureManager6.IsEnabledAsync("FeatureA")); + Assert.False(await featureManager6.IsEnabledAsync("FeatureB")); + Assert.True(await featureManager6.IsEnabledAsync("FeatureC")); + Assert.False(await featureManager6.IsEnabledAsync("Feature1")); + Assert.False(await featureManager6.IsEnabledAsync("Feature2")); + + var services3 = new ServiceCollection(); + services3 + .AddFeatureManagement(configuration2); + services3.Configure(o => + { + o.CustomConfigurationMergingEnabled = true; + }); + ServiceProvider serviceProvider3 = services3.BuildServiceProvider(); + IFeatureManager featureManager7 = serviceProvider3.GetRequiredService(); + + Assert.True(await featureManager7.IsEnabledAsync("FeatureA")); + Assert.True(await featureManager7.IsEnabledAsync("FeatureB")); + Assert.True(await featureManager7.IsEnabledAsync("FeatureC")); + Assert.False(await featureManager7.IsEnabledAsync("Feature1")); + Assert.False(await featureManager7.IsEnabledAsync("Feature2")); + + var services4 = new ServiceCollection(); + services4 + .AddFeatureManagement(configuration2); + ServiceProvider serviceProvider4 = services4.BuildServiceProvider(); + IFeatureManager featureManager8 = serviceProvider4.GetRequiredService(); + + Assert.False(await featureManager8.IsEnabledAsync("FeatureA")); + Assert.False(await featureManager8.IsEnabledAsync("FeatureB")); + Assert.True(await featureManager8.IsEnabledAsync("FeatureC")); + Assert.False(await featureManager8.IsEnabledAsync("Feature1")); + Assert.False(await featureManager8.IsEnabledAsync("Feature2")); } }