From a9f744bef54fe515fafea2a218faf74cc4cb9c06 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Wed, 14 May 2025 16:44:59 +0800 Subject: [PATCH 1/5] update dependency --- .../Microsoft.FeatureManagement.csproj | 7 ++++--- .../Tests.FeatureManagement.AspNetCore.csproj | 4 ++-- .../Tests.FeatureManagement/Tests.FeatureManagement.csproj | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj index dfd5ccc8..08d0b314 100644 --- a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj +++ b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj @@ -41,9 +41,10 @@ - - - + + + + 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/Tests.FeatureManagement.csproj b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj index 4a915a4f..6ebf54ac 100644 --- a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj +++ b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj @@ -18,21 +18,21 @@ - + - + - + From bedd802431f94a28038e252923cae0204dbb2324 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 15 May 2025 23:04:04 +0800 Subject: [PATCH 2/5] merge feature flags from configuration providers --- .../ConfigurationFeatureDefinitionProvider.cs | 78 +++++++++++++++---- .../OnDemandConfigurationProvider.cs | 13 ++++ 2 files changed, 75 insertions(+), 16 deletions(-) create mode 100644 src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 507bd1b1..6c505659 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -9,6 +9,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -22,8 +23,10 @@ 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 static readonly PropertyInfo _propertyInfo = typeof(ConfigurationProvider).GetProperty("Data", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); private readonly IConfiguration _configuration; + private IEnumerable _dotnetFeatureDefinitionSections; + private IEnumerable _microsoftFeatureDefinitionSections; private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; @@ -48,6 +51,10 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) { return Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName)); }; + + _dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); + + _microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); } /// @@ -90,6 +97,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 +120,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 +145,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() } } - IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); - - foreach (IConfigurationSection featureSection in dotnetFeatureDefinitionSections) + foreach (IConfigurationSection featureSection in _dotnetFeatureDefinitionSections) { string featureName = featureSection.Key; @@ -156,9 +167,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 +181,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 +218,48 @@ private IEnumerable GetDotnetFeatureDefinitionSections() private IEnumerable GetMicrosoftFeatureDefinitionSections() { - return _configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) - .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName) - .GetChildren(); + var configurationRoot = _configuration as IConfigurationRoot; + if (configurationRoot == null) + { + // Fallback to current behavior if not an IConfigurationRoot + return _configuration + .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName) + .GetChildren(); + } + + var mergedSections = new List(); + + foreach (IConfigurationProvider provider in configurationRoot.Providers) + { + if (provider is ConfigurationProvider configProvider) + { + var data = _propertyInfo.GetValue(configProvider) as IDictionary; + + // + // Cannot use the original provider directly as its reload token is subscribed + var configurationProvider = new OnDemandConfigurationProvider(data); + + var onDemandConfigurationRoot = new ConfigurationRoot(new[] { configurationProvider }); + + IConfigurationSection featureFlagsSection = onDemandConfigurationRoot + .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName); + + if (featureFlagsSection.Exists()) + { + mergedSections.AddRange(featureFlagsSection.GetChildren()); + } + } + else if (provider is ChainedConfigurationProvider chainedProvider) + { + IConfigurationSection featureFlagsSection = chainedProvider.Configuration + .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName); + } + } + + return mergedSections; } private FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) diff --git a/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs b/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs new file mode 100644 index 00000000..2a551da2 --- /dev/null +++ b/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs @@ -0,0 +1,13 @@ +using Microsoft.Extensions.Configuration; +using System.Collections.Generic; + +namespace Microsoft.FeatureManagement +{ + internal class OnDemandConfigurationProvider : ConfigurationProvider + { + public OnDemandConfigurationProvider(IDictionary data) + { + Data = data; + } + } +} From af4843ef705ffdeafed48a9d62bf103ac4195eaf Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 20 May 2025 11:35:19 +0800 Subject: [PATCH 3/5] add testcases --- .../ConfigurationFeatureDefinitionProvider.cs | 35 +++++++++------ .../FeatureManagementTest.cs | 44 +++++++++++++++++++ .../Tests.FeatureManagement.csproj | 11 ++++- .../Tests.FeatureManagement/appsettings1.json | 18 ++++++++ .../Tests.FeatureManagement/appsettings2.json | 18 ++++++++ .../Tests.FeatureManagement/appsettings3.json | 18 ++++++++ 6 files changed, 129 insertions(+), 15 deletions(-) create mode 100644 tests/Tests.FeatureManagement/appsettings1.json create mode 100644 tests/Tests.FeatureManagement/appsettings2.json create mode 100644 tests/Tests.FeatureManagement/appsettings3.json diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 6c505659..d0b90af8 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -218,17 +218,28 @@ private IEnumerable GetDotnetFeatureDefinitionSections() private IEnumerable GetMicrosoftFeatureDefinitionSections() { - var configurationRoot = _configuration as IConfigurationRoot; - if (configurationRoot == null) + var featureDefinitionSections = new List(); + + FindFeatureFlags(_configuration, featureDefinitionSections); + + return featureDefinitionSections; + } + + private void FindFeatureFlags(IConfiguration configuration, List featureDefinitionSections) + { + if (!(configuration is IConfigurationRoot configurationRoot)) { - // Fallback to current behavior if not an IConfigurationRoot - return _configuration + IConfigurationSection featureFlagsSection = configuration .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) - .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName) - .GetChildren(); - } + .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName); - var mergedSections = new List(); + if (featureFlagsSection.Exists()) + { + featureDefinitionSections.AddRange(featureFlagsSection.GetChildren()); + } + + return; + } foreach (IConfigurationProvider provider in configurationRoot.Providers) { @@ -248,18 +259,14 @@ private IEnumerable GetMicrosoftFeatureDefinitionSections if (featureFlagsSection.Exists()) { - mergedSections.AddRange(featureFlagsSection.GetChildren()); + featureDefinitionSections.AddRange(featureFlagsSection.GetChildren()); } } else if (provider is ChainedConfigurationProvider chainedProvider) { - IConfigurationSection featureFlagsSection = chainedProvider.Configuration - .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) - .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName); + FindFeatureFlags(chainedProvider.Configuration, featureDefinitionSections); } } - - return mergedSections; } private FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index fe2e2ac5..e4a39176 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 6ebf54ac..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 @@ -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 + } + ] + } +} From 1d3897b9c517a0c526e2ed496349aae1426e9740 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 21 May 2025 09:27:14 +0800 Subject: [PATCH 4/5] update --- .../ConfigurationFeatureDefinitionProvider.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index d0b90af8..103f8ab7 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -227,7 +227,9 @@ private IEnumerable GetMicrosoftFeatureDefinitionSections private void FindFeatureFlags(IConfiguration configuration, List featureDefinitionSections) { - if (!(configuration is IConfigurationRoot configurationRoot)) + if (!(configuration is IConfigurationRoot configurationRoot) || + configurationRoot.Providers.Any(provider => + !(provider is ConfigurationProvider) && !(provider is ChainedConfigurationProvider))) { IConfigurationSection featureFlagsSection = configuration .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) From 1770e0a3989d4e267b330a2de8c175c558009e6e Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 29 May 2025 12:36:37 +0800 Subject: [PATCH 5/5] update reflection using nameof(Data) --- .../ConfigurationFeatureDefinitionProvider.cs | 10 +++------- .../OnDemandConfigurationProvider.cs | 7 ++++++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 103f8ab7..d8dbd996 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -9,7 +9,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -23,7 +22,6 @@ 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 static readonly PropertyInfo _propertyInfo = typeof(ConfigurationProvider).GetProperty("Data", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); private readonly IConfiguration _configuration; private IEnumerable _dotnetFeatureDefinitionSections; private IEnumerable _microsoftFeatureDefinitionSections; @@ -245,15 +243,13 @@ private void FindFeatureFlags(IConfiguration configuration, List; - // // Cannot use the original provider directly as its reload token is subscribed - var configurationProvider = new OnDemandConfigurationProvider(data); + var onDemandConfigurationProvider = new OnDemandConfigurationProvider(configurationProvider); - var onDemandConfigurationRoot = new ConfigurationRoot(new[] { configurationProvider }); + var onDemandConfigurationRoot = new ConfigurationRoot(new[] { onDemandConfigurationProvider }); IConfigurationSection featureFlagsSection = onDemandConfigurationRoot .GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) diff --git a/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs b/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs index 2a551da2..1170e636 100644 --- a/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs +++ b/src/Microsoft.FeatureManagement/OnDemandConfigurationProvider.cs @@ -1,12 +1,17 @@ using Microsoft.Extensions.Configuration; using System.Collections.Generic; +using System.Reflection; namespace Microsoft.FeatureManagement { internal class OnDemandConfigurationProvider : ConfigurationProvider { - public OnDemandConfigurationProvider(IDictionary data) + 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; } }