From a60ddc5f9865ea9b45c632f029044b180c94df67 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 14 Nov 2023 10:45:58 +0800 Subject: [PATCH 1/6] add option for no feature management section found --- .../ConfigurationFeatureDefinitionProvider.cs | 16 +++++++++++++--- .../FeatureManagementOptions.cs | 7 +++++++ .../ServiceCollectionExtensions.cs | 11 ++++++++++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 5b30f56e..c16c909c 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -3,6 +3,7 @@ // using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using System; using System.Collections.Concurrent; @@ -29,12 +30,14 @@ sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider private readonly ConcurrentDictionary _definitions; private IDisposable _changeSubscription; private readonly ILogger _logger; + private readonly FeatureManagementOptions _options; private int _stale = 0; - public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory) + public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory, IOptions options) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); _logger = loggerFactory?.CreateLogger() ?? throw new ArgumentNullException(nameof(loggerFactory)); + _options = options?.Value ?? throw new ArgumentNullException(nameof(options)); _definitions = new ConcurrentDictionary(); _changeSubscription = ChangeToken.OnChange( @@ -217,9 +220,16 @@ private IEnumerable GetFeatureDefinitionSections() return featureManagementConfigurationSection.GetChildren(); } - _logger.LogDebug($"No configuration section named '{FeatureManagementSectionName}' was found."); + // + // There is no "FeatureManagement" section in the configuration + if (_options.RequireFeatureManagementSection) + { + _logger.LogDebug($"No configuration section named '{FeatureManagementSectionName}' was found."); + + return Enumerable.Empty(); + } - return Enumerable.Empty(); + return _configuration.GetChildren(); } } } diff --git a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs index 46658786..02624968 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs @@ -22,5 +22,12 @@ public class FeatureManagementOptions /// The default value is true. /// public bool IgnoreMissingFeatures { get; set; } = true; + + /// + /// Controls the behavior of feature definition provider when "FeatureManagement" section in the configuration is missing. + /// If missing configuration section is not ignored, the feature definition provider will fall back to the root of the provided configuration. + /// The default value is false. + /// + public bool RequireFeatureManagementSection { get; set; } = true; } } diff --git a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs index 09307583..57826991 100644 --- a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs +++ b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Microsoft.FeatureManagement.FeatureFilters; using System; @@ -60,7 +61,15 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec throw new ArgumentNullException(nameof(configuration)); } - services.AddSingleton(sp => new ConfigurationFeatureDefinitionProvider(configuration, sp.GetRequiredService())); + services.Configure(options => + { + options.RequireFeatureManagementSection = false; + }); + + services.AddSingleton(sp => new ConfigurationFeatureDefinitionProvider( + configuration, + sp.GetRequiredService(), + sp.GetRequiredService>())); return services.AddFeatureManagement(); } From 585c02aaf0ef480d034fd290daffff286057771f Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 15 Nov 2023 16:24:10 +0800 Subject: [PATCH 2/6] decouple ConfigurationFeatureDefinitionProvider from FeatureManagementOption --- .../ConfigurationFeatureDefinitionProvider.cs | 32 +++++++++++++------ .../ServiceCollectionExtensions.cs | 21 +++++++++--- .../FeatureManagement.cs | 23 +++++++++++-- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index c16c909c..102bccc1 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -3,7 +3,6 @@ // using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using System; using System.Collections.Concurrent; @@ -28,16 +27,15 @@ sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider private const string FeatureManagementSectionName = "FeatureManagement"; private readonly IConfiguration _configuration; private readonly ConcurrentDictionary _definitions; - private IDisposable _changeSubscription; + private readonly bool _useTopLevelConfiguration = false; private readonly ILogger _logger; - private readonly FeatureManagementOptions _options; + private IDisposable _changeSubscription; private int _stale = 0; - public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory, IOptions options) + public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); _logger = loggerFactory?.CreateLogger() ?? throw new ArgumentNullException(nameof(loggerFactory)); - _options = options?.Value ?? throw new ArgumentNullException(nameof(options)); _definitions = new ConcurrentDictionary(); _changeSubscription = ChangeToken.OnChange( @@ -45,6 +43,20 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILog () => _stale = 1); } + /// + /// The collection of feature filter metadata. + /// + /// Thrown if it is set to null. + public bool UseTopLevelConfiguration + { + get => _useTopLevelConfiguration; + + init + { + _useTopLevelConfiguration = value; + } + } + public void Dispose() { _changeSubscription?.Dispose(); @@ -222,14 +234,14 @@ private IEnumerable GetFeatureDefinitionSections() // // There is no "FeatureManagement" section in the configuration - if (_options.RequireFeatureManagementSection) + if (_useTopLevelConfiguration) { - _logger.LogDebug($"No configuration section named '{FeatureManagementSectionName}' was found."); - - return Enumerable.Empty(); + return _configuration.GetChildren(); } - return _configuration.GetChildren(); + _logger.LogDebug($"No configuration section named '{FeatureManagementSectionName}' was found."); + + return Enumerable.Empty(); } } } diff --git a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs index 5b85c469..f246db6b 100644 --- a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs +++ b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs @@ -69,6 +69,7 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec /// /// Adds singleton and other required feature management services. + /// The registered will use the provided configuration and read from the top level if no "FeatureManagement" section can be found. /// /// The service collection that feature management services are added to. /// A specific instance that will be used to obtain feature settings. @@ -85,10 +86,13 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec options.RequireFeatureManagementSection = false; }); - services.AddSingleton(sp => new ConfigurationFeatureDefinitionProvider( - configuration, - sp.GetRequiredService(), - sp.GetRequiredService>())); + services.AddSingleton(sp => + new ConfigurationFeatureDefinitionProvider( + configuration, + sp.GetRequiredService()) + { + UseTopLevelConfiguration = true + }); return services.AddFeatureManagement(); } @@ -143,6 +147,7 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService /// /// Adds scoped and other required feature management services. + /// The registered will use the provided configuration and read from the top level if no "FeatureManagement" section can be found. /// /// The service collection that feature management services are added to. /// A specific instance that will be used to obtain feature settings. @@ -154,7 +159,13 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService throw new ArgumentNullException(nameof(configuration)); } - services.AddSingleton(sp => new ConfigurationFeatureDefinitionProvider(configuration, sp.GetRequiredService())); + services.AddSingleton(sp => + new ConfigurationFeatureDefinitionProvider( + configuration, + sp.GetRequiredService()) + { + UseTopLevelConfiguration = true + }); return services.AddScopedFeatureManagement(); } diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 810043fe..b29976d4 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -72,8 +72,7 @@ public async Task ReadsOnlyFeatureManagementSection() services .AddSingleton(config) - .AddFeatureManagement() - .AddFeatureFilter(); + .AddFeatureManagement(); ServiceProvider serviceProvider = services.BuildServiceProvider(); @@ -87,6 +86,26 @@ public async Task ReadsOnlyFeatureManagementSection() } } + [Fact] + public async Task ReadsTopLevelConfiguration() + { + const string feature = "FeatureX"; + + var stream = new MemoryStream(Encoding.UTF8.GetBytes($"{{\"AllowedHosts\": \"*\", \"FeatureFlags\": {{\"{feature}\": true}}}}")); + + IConfiguration config = new ConfigurationBuilder().AddJsonStream(stream).Build(); + + var services = new ServiceCollection(); + + services.AddFeatureManagement(config.GetSection("FeatureFlags")); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + Assert.True(await featureManager.IsEnabledAsync(feature)); + } + [Fact] public void AddsScopedFeatureManagement() { From 1e58de01a58dca1a1a96295c28291c2f7c278cbb Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 15 Nov 2023 16:28:13 +0800 Subject: [PATCH 3/6] update comments --- .../ConfigurationFeatureDefinitionProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 102bccc1..97b0a94c 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -44,9 +44,8 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILog } /// - /// The collection of feature filter metadata. + /// The option that controls the behavior when "FeatureManagement" section in the configuration is missing. /// - /// Thrown if it is set to null. public bool UseTopLevelConfiguration { get => _useTopLevelConfiguration; From 06070e65c4131696580255b45f581db743c73e98 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 16 Nov 2023 08:58:17 +0800 Subject: [PATCH 4/6] remove redundant code --- .../FeatureManagementOptions.cs | 7 ------- .../ServiceCollectionExtensions.cs | 5 ----- 2 files changed, 12 deletions(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs index 02624968..46658786 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs @@ -22,12 +22,5 @@ public class FeatureManagementOptions /// The default value is true. /// public bool IgnoreMissingFeatures { get; set; } = true; - - /// - /// Controls the behavior of feature definition provider when "FeatureManagement" section in the configuration is missing. - /// If missing configuration section is not ignored, the feature definition provider will fall back to the root of the provided configuration. - /// The default value is false. - /// - public bool RequireFeatureManagementSection { get; set; } = true; } } diff --git a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs index f246db6b..bbd4a09e 100644 --- a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs +++ b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs @@ -81,11 +81,6 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec throw new ArgumentNullException(nameof(configuration)); } - services.Configure(options => - { - options.RequireFeatureManagementSection = false; - }); - services.AddSingleton(sp => new ConfigurationFeatureDefinitionProvider( configuration, From abcc25163424b07e1345edb236293895a54b77f8 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 16 Nov 2023 09:00:00 +0800 Subject: [PATCH 5/6] resolve comments --- .../ConfigurationFeatureDefinitionProvider.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 97b0a94c..8d7e92fc 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -46,15 +46,7 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILog /// /// The option that controls the behavior when "FeatureManagement" section in the configuration is missing. /// - public bool UseTopLevelConfiguration - { - get => _useTopLevelConfiguration; - - init - { - _useTopLevelConfiguration = value; - } - } + public bool UseTopLevelConfiguration { get; init; } public void Dispose() { From 2e7ef12427fd229a57d889b06e09f191590cdb4b Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 16 Nov 2023 10:16:52 +0800 Subject: [PATCH 6/6] fix bug --- .../ConfigurationFeatureDefinitionProvider.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 8d7e92fc..15b69a62 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -27,7 +27,6 @@ sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider private const string FeatureManagementSectionName = "FeatureManagement"; private readonly IConfiguration _configuration; private readonly ConcurrentDictionary _definitions; - private readonly bool _useTopLevelConfiguration = false; private readonly ILogger _logger; private IDisposable _changeSubscription; private int _stale = 0; @@ -225,7 +224,7 @@ private IEnumerable GetFeatureDefinitionSections() // // There is no "FeatureManagement" section in the configuration - if (_useTopLevelConfiguration) + if (UseTopLevelConfiguration) { return _configuration.GetChildren(); }