From 85b401e4c912715c07687190303da8a24e1cbdd9 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 21 Sep 2023 17:36:41 -0700 Subject: [PATCH 1/4] Adjusts the configuration provider to no longer check top level for feature flags --- .../ConfigurationFeatureDefinitionProvider.cs | 22 +++++++++++----- .../ServiceCollectionExtensions.cs | 2 +- .../FeatureManagement.cs | 26 +++++++++++++++++++ .../Tests.FeatureManagement.csproj | 2 +- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 4774fd5e..64912d23 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. // using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; using System; using System.Collections.Concurrent; @@ -23,14 +24,19 @@ sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider private const string FeatureFiltersSectionName = "EnabledFor"; private const string RequirementTypeKeyword = "RequirementType"; + private const string FeatureManagementSectionName = "FeatureManagement"; private readonly IConfiguration _configuration; private readonly ConcurrentDictionary _definitions; private IDisposable _changeSubscription; + private readonly ILogger _logger; private int _stale = 0; - public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) + const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'."; + + public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); + _logger = loggerFactory.CreateLogger(); _definitions = new ConcurrentDictionary(); _changeSubscription = ChangeToken.OnChange( @@ -199,17 +205,19 @@ We support private IEnumerable GetFeatureDefinitionSections() { - const string FeatureManagementSectionName = "FeatureManagement"; + // + // Look for feature definitions under the "FeatureManagement" section + IConfigurationSection featureManagementConfigurationSection = _configuration.GetSection(FeatureManagementSectionName); - if (_configuration.GetChildren().Any(s => s.Key.Equals(FeatureManagementSectionName, StringComparison.OrdinalIgnoreCase))) + if (featureManagementConfigurationSection.Exists()) { - // - // Look for feature definitions under the "FeatureManagement" section - return _configuration.GetSection(FeatureManagementSectionName).GetChildren(); + return featureManagementConfigurationSection.GetChildren(); } else { - return _configuration.GetChildren(); + _logger.LogWarning($"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 507f1394..5808a6a9 100644 --- a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs +++ b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs @@ -48,7 +48,7 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec throw new ArgumentNullException(nameof(configuration)); } - services.AddSingleton(new ConfigurationFeatureDefinitionProvider(configuration)); + services.AddSingleton(sp => new ConfigurationFeatureDefinitionProvider(configuration, sp.GetRequiredService())); return services.AddFeatureManagement(); } diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 9ecd3e7f..84f18b5e 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -12,9 +12,11 @@ using Microsoft.FeatureManagement.FeatureFilters; using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Net; using System.Net.Http; +using System.Text; using System.Threading.Tasks; using Xunit; @@ -71,6 +73,30 @@ public async Task ReadsConfiguration() Assert.True(called); } + [Fact] + public async Task ReadsOnlyFeatureManagementSection() + { + MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes("{\"AllowedHosts\": \"*\"}")); + IConfiguration config = new ConfigurationBuilder().AddJsonStream(stream).Build(); + + var services = new ServiceCollection(); + + services + .AddSingleton(config) + .AddFeatureManagement() + .AddFeatureFilter(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + await foreach (string featureName in featureManager.GetFeatureNamesAsync()) + { + // Fail, as no features should be found + Assert.True(false); + } + } + [Fact] public async Task Integrates() { diff --git a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj index 7501c259..83063e1c 100644 --- a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj +++ b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj @@ -16,7 +16,7 @@ - + From c55a5d2b656023e75d0b4138ff09e84912208ff1 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 28 Sep 2023 16:44:46 -0700 Subject: [PATCH 2/4] Adding missing using and removes unused else --- .../ConfigurationFeatureDefinitionProvider.cs | 8 +++----- .../ServiceCollectionExtensions.cs | 1 + 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 64912d23..32ca4d28 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -213,12 +213,10 @@ private IEnumerable GetFeatureDefinitionSections() { return featureManagementConfigurationSection.GetChildren(); } - else - { - _logger.LogWarning($"No configuration section named '{FeatureManagementSectionName}' was found."); - return Enumerable.Empty(); - } + _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 5808a6a9..04cbb499 100644 --- a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs +++ b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging; using System; namespace Microsoft.FeatureManagement From 3d1414f25f158f9cecb338a3e6eeb028cad919f7 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 28 Sep 2023 17:10:40 -0700 Subject: [PATCH 3/4] Remove unused string --- .../ConfigurationFeatureDefinitionProvider.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 32ca4d28..7b5015ec 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -31,8 +31,6 @@ sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider private readonly ILogger _logger; private int _stale = 0; - const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'."; - public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); From 56133ec8f0cc1346ac2b04071929d528ca132fed Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Fri, 29 Sep 2023 10:39:09 -0700 Subject: [PATCH 4/4] Add null check to constructor --- .../ConfigurationFeatureDefinitionProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 7b5015ec..276f4822 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -34,7 +34,7 @@ sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider public ConfigurationFeatureDefinitionProvider(IConfiguration configuration, ILoggerFactory loggerFactory) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); - _logger = loggerFactory.CreateLogger(); + _logger = loggerFactory?.CreateLogger() ?? throw new ArgumentNullException(nameof(loggerFactory)); _definitions = new ConcurrentDictionary(); _changeSubscription = ChangeToken.OnChange(