diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 507bd1b1..d8dbd996 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -22,8 +22,9 @@ 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 IEnumerable _dotnetFeatureDefinitionSections; + private IEnumerable _microsoftFeatureDefinitionSections; private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; @@ -48,6 +49,10 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) { return Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName)); }; + + _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); + + _microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); } /// @@ -90,6 +95,10 @@ public Task GetFeatureDefinitionAsync(string featureName) if (Interlocked.Exchange(ref _stale, 0) != 0) { _definitions.Clear(); + + _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); + + _microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); } return _definitions.GetOrAdd(featureName, _getFeatureDefinitionFunc); @@ -109,11 +118,13 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() if (Interlocked.Exchange(ref _stale, 0) != 0) { _definitions.Clear(); - } - IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); + _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); + + _microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); + } - foreach (IConfigurationSection featureSection in microsoftFeatureDefinitionSections) + foreach (IConfigurationSection featureSection in _microsoftFeatureDefinitionSections) { string featureName = featureSection[MicrosoftFeatureManagementFields.Id]; @@ -132,9 +143,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() } } - IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); - - foreach (IConfigurationSection featureSection in dotnetFeatureDefinitionSections) + foreach (IConfigurationSection featureSection in _dotnetFeatureDefinitionSections) { string featureName = featureSection.Key; @@ -156,9 +165,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() private FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName) { - IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); - - IConfigurationSection dotnetFeatureDefinitionConfiguration = dotnetFeatureDefinitionSections + IConfigurationSection dotnetFeatureDefinitionConfiguration = _dotnetFeatureDefinitionSections .FirstOrDefault(section => string.Equals(section.Key, featureName, StringComparison.OrdinalIgnoreCase)); @@ -172,9 +179,7 @@ private FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName) private FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName) { - IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); - - IConfigurationSection microsoftFeatureDefinitionConfiguration = microsoftFeatureDefinitionSections + IConfigurationSection microsoftFeatureDefinitionConfiguration = _microsoftFeatureDefinitionSections .LastOrDefault(section => string.Equals(section[MicrosoftFeatureManagementFields.Id], featureName, StringComparison.OrdinalIgnoreCase)); @@ -211,9 +216,55 @@ private IEnumerable GetDotnetFeatureDefinitionSections() private IEnumerable GetMicrosoftFeatureDefinitionSections() { - return _configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) - .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName) - .GetChildren(); + var featureDefinitionSections = new List(); + + FindFeatureFlags(_configuration, featureDefinitionSections); + + return featureDefinitionSections; + } + + private void FindFeatureFlags(IConfiguration configuration, List featureDefinitionSections) + { + if (!(configuration is IConfigurationRoot configurationRoot) || + configurationRoot.Providers.Any(provider => + !(provider is ConfigurationProvider) && !(provider is ChainedConfigurationProvider))) + { + IConfigurationSection featureFlagsSection = configuration + .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName); + + if (featureFlagsSection.Exists()) + { + featureDefinitionSections.AddRange(featureFlagsSection.GetChildren()); + } + + return; + } + + foreach (IConfigurationProvider provider in configurationRoot.Providers) + { + if (provider is ConfigurationProvider configurationProvider) + { + // + // Cannot use the original provider directly as its reload token is subscribed + var onDemandConfigurationProvider = new OnDemandConfigurationProvider(configurationProvider); + + var onDemandConfigurationRoot = new ConfigurationRoot(new[] { onDemandConfigurationProvider }); + + IConfigurationSection featureFlagsSection = onDemandConfigurationRoot + .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName); + + if (featureFlagsSection.Exists()) + { + featureDefinitionSections.AddRange(featureFlagsSection.GetChildren()); + } + } + else if (provider is ChainedConfigurationProvider chainedProvider) + { + FindFeatureFlags(chainedProvider.Configuration, featureDefinitionSections); + } + } } private FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) diff --git a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj index 31a47510..899b6c0c 100644 --- a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj +++ b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj @@ -41,9 +41,10 @@ - - - + + + + diff --git a/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs b/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs new file mode 100644 index 00000000..1170e636 --- /dev/null +++ b/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs @@ -0,0 +1,18 @@ +using Microsoft.Extensions.Configuration; +using System.Collections.Generic; +using System.Reflection; + +namespace Microsoft.FeatureManagement +{ + internal class OnDemandConfigurationProvider : ConfigurationProvider + { + private static readonly PropertyInfo _DataProperty = typeof(ConfigurationProvider).GetProperty(nameof(Data), BindingFlags.NonPublic | BindingFlags.Instance); + + public OnDemandConfigurationProvider(ConfigurationProvider configurationProvider) + { + var data = _DataProperty.GetValue(configurationProvider) as IDictionary; + + Data = data; + } + } +} diff --git a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj index e91b1be6..e5fd29ca 100644 --- a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj +++ b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj @@ -24,14 +24,14 @@ - + - + diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index 1a3b6c5e..b9c3d7c1 100644 --- a/tests/Tests.FeatureManagement/FeatureManagementTest.cs +++ b/tests/Tests.FeatureManagement/FeatureManagementTest.cs @@ -394,6 +394,50 @@ public async Task LastFeatureFlagWins() Assert.True(await featureManager.IsEnabledAsync(Features.DuplicateFlag)); } + + [Fact] + public async Task MergesFeatureFlagsFromDifferentConfigurationSources() + { + /* + * appsettings1.json + * Feature1: true + * Feature2: true + * FeatureA: true + * + * appsettings2.json + * Feature1: true + * Feature2: false + * FeatureB: true + * + * appsettings3.json + * Feature1: false + * Feature2: false + * FeatureC: true + */ + + IConfiguration configuration1 = new ConfigurationBuilder() + .AddJsonFile("appsettings1.json") + .AddJsonFile("appsettings2.json") + .Build(); + + IConfiguration configuration2 = new ConfigurationBuilder() + .AddConfiguration(configuration1) // chained configuration + .AddJsonFile("appsettings3.json") + .Build(); + + var featureManager1 = new FeatureManager(new ConfigurationFeatureDefinitionProvider(configuration1)); + 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)); + 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 + } } public class FeatureManagementFeatureFilterGeneralTest diff --git a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj index 4a915a4f..bed33670 100644 --- a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj +++ b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj @@ -1,4 +1,4 @@ - + net48;net8.0;net9.0 @@ -18,21 +18,21 @@ - + - + - + @@ -43,6 +43,15 @@ Always + + Always + + + Always + + + Always + Always diff --git a/tests/Tests.FeatureManagement/appsettings1.json b/tests/Tests.FeatureManagement/appsettings1.json new file mode 100644 index 00000000..8b75e0b4 --- /dev/null +++ b/tests/Tests.FeatureManagement/appsettings1.json @@ -0,0 +1,18 @@ +{ + "feature_management": { + "feature_flags": [ + { + "id": "Feature1", + "enabled": true + }, + { + "id": "Feature2", + "enabled": true + }, + { + "id": "FeatureA", + "enabled": true + } + ] + } +} diff --git a/tests/Tests.FeatureManagement/appsettings2.json b/tests/Tests.FeatureManagement/appsettings2.json new file mode 100644 index 00000000..d3895ecc --- /dev/null +++ b/tests/Tests.FeatureManagement/appsettings2.json @@ -0,0 +1,18 @@ +{ + "feature_management": { + "feature_flags": [ + { + "id": "Feature1", + "enabled": true + }, + { + "id": "Feature2", + "enabled": false + }, + { + "id": "FeatureB", + "enabled": true + } + ] + } +} diff --git a/tests/Tests.FeatureManagement/appsettings3.json b/tests/Tests.FeatureManagement/appsettings3.json new file mode 100644 index 00000000..72c62d47 --- /dev/null +++ b/tests/Tests.FeatureManagement/appsettings3.json @@ -0,0 +1,18 @@ +{ + "feature_management": { + "feature_flags": [ + { + "id": "Feature1", + "enabled": false + }, + { + "id": "Feature2", + "enabled": false + }, + { + "id": "FeatureC", + "enabled": true + } + ] + } +}