From 2f2bdb9c08818d9614a8b40009a9d9cac857b343 Mon Sep 17 00:00:00 2001 From: Sotiris Zacharopoulos Date: Thu, 5 Dec 2019 01:06:18 +0000 Subject: [PATCH 1/3] If a feature is configured to be enabled for a specific feature filter and the feature hasn't been registered, an exception will be thrown when the feature will be evaluated. (#13) --- README.md | 9 ++ .../FeatureManager.cs | 8 +- .../FeatureManagerOptions.cs | 16 +++ .../FeatureManagement.cs | 103 ++++++++++++++---- 4 files changed, 115 insertions(+), 21 deletions(-) create mode 100644 src/Microsoft.FeatureManagement/FeatureManagerOptions.cs diff --git a/README.md b/README.md index 9767e82e..fb38ca28 100644 --- a/README.md +++ b/README.md @@ -247,6 +247,15 @@ Creating a feature filter provides a way to enable features based on criteria th Feature filters are registered by the `IFeatureManagementBuilder` when `AddFeatureManagement` is called. These feature filters have access to the services that exist within the service collection that was used to add feature flags. Dependency injection can be used to retrieve these services. +If a feature is configured to be enabled for a specific feature filter and the feature hasn't been registered, an exception will be thrown when the feature will be evaluated. The exception could be silently swallowed, using the feature manager's options. + +``` +services.Configure(options => +{ + options.SwallowExceptionForUnregisteredFilter = true; +}); +``` + ### Parameterized Feature Filters Some feature filters require parameters to decide whether a feature should be turned on or not. For example a browser feature filter may turn on a feature for a certain set of browsers. It may be desired that Edge and Chrome browsers enable a feature, while FireFox does not. To do this a feature filter can be designed to expect parameters. These parameters would be specified in the feature configuration, and in code would be accessible via the `FeatureFilterEvaluationContext` parameter of `IFeatureFilter.EvaluateAsync`. diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index a15a6148..4a4f2ffe 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. // using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -21,8 +22,9 @@ class FeatureManager : IFeatureManager private readonly ILogger _logger; private readonly ConcurrentDictionary _filterMetadataCache; private readonly ConcurrentDictionary _contextualFeatureFilterCache; + private readonly FeatureManagerOptions _options; - public FeatureManager(IFeatureSettingsProvider settingsProvider, IEnumerable featureFilters, IEnumerable sessionManagers, ILoggerFactory loggerFactory) + public FeatureManager(IFeatureSettingsProvider settingsProvider, IEnumerable featureFilters, IEnumerable sessionManagers, ILoggerFactory loggerFactory, IOptionsMonitor optionsAccessor) { _settingsProvider = settingsProvider; _featureFilters = featureFilters ?? throw new ArgumentNullException(nameof(featureFilters)); @@ -30,6 +32,7 @@ public FeatureManager(IFeatureSettingsProvider settingsProvider, IEnumerable(); _filterMetadataCache = new ConcurrentDictionary(); _contextualFeatureFilterCache = new ConcurrentDictionary(); + _options = optionsAccessor.CurrentValue; } public Task IsEnabledAsync(string feature) @@ -78,8 +81,9 @@ private async Task IsEnabledAsync(string feature, TContext appCo if (filter == null) { + if (!_options.SwallowExceptionForUnregisteredFilter) + throw new Exception($"Feature filter '{featureFilterSettings.Name}' specified for feature '{feature}' was not found."); _logger.LogWarning($"Feature filter '{featureFilterSettings.Name}' specified for feature '{feature}' was not found."); - continue; } diff --git a/src/Microsoft.FeatureManagement/FeatureManagerOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagerOptions.cs new file mode 100644 index 00000000..13bfe15a --- /dev/null +++ b/src/Microsoft.FeatureManagement/FeatureManagerOptions.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +namespace Microsoft.FeatureManagement +{ + /// + /// Options that are being used when a feature is being evaluated by the Feature Manager. + /// + public class FeatureManagerOptions + { + /// + /// Is being used to decide if an exception should be thrown or not when a configured feature filter has not been registered. + /// + public bool SwallowExceptionForUnregisteredFilter { get; set; } + } +} \ No newline at end of file diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index b71cb33e..494cb6c2 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -75,18 +75,26 @@ public async Task Integrates() IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); TestServer testServer = new TestServer(WebHost.CreateDefaultBuilder().ConfigureServices(services => + { + services + .Configure(options => + { + options.SwallowExceptionForUnregisteredFilter = true; + }); + + services + .AddSingleton(config) + .AddFeatureManagement() + .AddFeatureFilter(); + + services.AddMvcCore(o => + { + + o.Filters.AddForFeature(ConditionalFeature); + }); + }) + .Configure(app => { - services - .AddSingleton(config) - .AddFeatureManagement() - .AddFeatureFilter(); - - services.AddMvcCore(o => { - - o.Filters.AddForFeature(ConditionalFeature); - }); - }) - .Configure(app => { app.UseForFeature(ConditionalFeature, a => a.Use(async (ctx, next) => { @@ -123,14 +131,20 @@ public async Task GatesFeatures() IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); TestServer testServer = new TestServer(WebHost.CreateDefaultBuilder().ConfigureServices(services => - { - services - .AddSingleton(config) - .AddFeatureManagement() - .AddFeatureFilter(); - - services.AddMvcCore(); - }) + { + services + .Configure(options => + { + options.SwallowExceptionForUnregisteredFilter = true; + }); + + services + .AddSingleton(config) + .AddFeatureManagement() + .AddFeatureFilter(); + + services.AddMvcCore(); + }) .Configure(app => app.UseMvc())); IEnumerable featureFilters = testServer.Host.Services.GetRequiredService>(); @@ -246,6 +260,12 @@ public async Task UsesContext() var serviceCollection = new ServiceCollection(); + serviceCollection + .Configure(options => + { + options.SwallowExceptionForUnregisteredFilter = true; + }); + serviceCollection.AddSingleton(config) .AddFeatureManagement() .AddFeatureFilter(); @@ -297,5 +317,50 @@ public void LimitsFeatureFilterImplementations() .AddFeatureFilter(); }); } + + [Fact] + public void ThrowExceptionForUnregisteredFilter() + { + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + var services = new ServiceCollection(); + + services + .AddSingleton(config) + .AddFeatureManagement(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + Assert.ThrowsAsync(async () => await featureManager.IsEnabledAsync(ConditionalFeature)); + } + + [Fact] + public async Task SwallowExceptionForUnregisteredFilter() + { + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + var services = new ServiceCollection(); + + services + .Configure(options => + { + options.SwallowExceptionForUnregisteredFilter = true; + }); + + services + .AddSingleton(config) + .AddFeatureManagement(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + var isEnabled = await featureManager.IsEnabledAsync(ConditionalFeature); + + Assert.False(isEnabled); + } + } } From e206d1c9a2f2c4e98db1ebcce9a0397751d80f3e Mon Sep 17 00:00:00 2001 From: Jimmy Campbell Date: Fri, 21 Feb 2020 18:06:01 -0500 Subject: [PATCH 2/3] Renamed option for ignoring missing filters. Modified exception type thrown when a filter is missing. --- README.md | 20 ++++++----- .../FeatureManagementError.cs | 13 +++++++ .../FeatureManagementException.cs | 26 ++++++++++++++ .../FeatureManagementOptions.cs | 17 +++++++++ .../FeatureManager.cs | 27 ++++++++++---- .../FeatureManagerOptions.cs | 16 --------- .../FeatureManagement.cs | 35 ++++++------------- .../Tests.FeatureManagement/appsettings.json | 20 ++++++----- 8 files changed, 109 insertions(+), 65 deletions(-) create mode 100644 src/Microsoft.FeatureManagement/FeatureManagementError.cs create mode 100644 src/Microsoft.FeatureManagement/FeatureManagementException.cs create mode 100644 src/Microsoft.FeatureManagement/FeatureManagementOptions.cs delete mode 100644 src/Microsoft.FeatureManagement/FeatureManagerOptions.cs diff --git a/README.md b/README.md index fb38ca28..e241e0a5 100644 --- a/README.md +++ b/README.md @@ -247,15 +247,6 @@ Creating a feature filter provides a way to enable features based on criteria th Feature filters are registered by the `IFeatureManagementBuilder` when `AddFeatureManagement` is called. These feature filters have access to the services that exist within the service collection that was used to add feature flags. Dependency injection can be used to retrieve these services. -If a feature is configured to be enabled for a specific feature filter and the feature hasn't been registered, an exception will be thrown when the feature will be evaluated. The exception could be silently swallowed, using the feature manager's options. - -``` -services.Configure(options => -{ - options.SwallowExceptionForUnregisteredFilter = true; -}); -``` - ### Parameterized Feature Filters Some feature filters require parameters to decide whether a feature should be turned on or not. For example a browser feature filter may turn on a feature for a certain set of browsers. It may be desired that Edge and Chrome browsers enable a feature, while FireFox does not. To do this a feature filter can be designed to expect parameters. These parameters would be specified in the feature configuration, and in code would be accessible via the `FeatureFilterEvaluationContext` parameter of `IFeatureFilter.EvaluateAsync`. @@ -307,6 +298,17 @@ When a feature filter is registered to be used for a feature flag, the alias use This can be overridden through the use of the `FilterAliasAttribute`. A feature filter can be decorated with this attribute to declare the name that should be used in configuration to reference this feature filter within a feature flag. +### Missing Feature Filters + +If a feature is configured to be enabled for a specific feature filter and that feature filter hasn't been registered, then an exception will be thrown when the feature is evaluated. The exception can be disabled by using the feature management options. + +``` +services.Configure(options => +{ + options.IgnoreMissingFeatureFilters = true; +}); +``` + ### Using HttpContext Feature filters can evaluate whether a feature should be enabled based off the properties of an HTTP Request. This is performed by inspecting the HTTP Context. A feature filter can get a reference to the HTTP Context by obtaining an `IHttpContextAccessor` through dependency injection. diff --git a/src/Microsoft.FeatureManagement/FeatureManagementError.cs b/src/Microsoft.FeatureManagement/FeatureManagementError.cs new file mode 100644 index 00000000..ce733063 --- /dev/null +++ b/src/Microsoft.FeatureManagement/FeatureManagementError.cs @@ -0,0 +1,13 @@ +namespace Microsoft.FeatureManagement +{ + /// + /// An error that can occur during feature management. + /// + public enum FeatureManagementError + { + /// + /// A feature filter that was listed for feature evaluation was not found. + /// + MissingFeatureFilter + } +} diff --git a/src/Microsoft.FeatureManagement/FeatureManagementException.cs b/src/Microsoft.FeatureManagement/FeatureManagementException.cs new file mode 100644 index 00000000..405c7b8f --- /dev/null +++ b/src/Microsoft.FeatureManagement/FeatureManagementException.cs @@ -0,0 +1,26 @@ +using System; + +namespace Microsoft.FeatureManagement +{ + /// + /// Represents errors that occur during feature management. + /// + public class FeatureManagementException : Exception + { + /// + /// Initializes a new instance of the class. + /// + /// The feature management error that the exception represents. + /// Error message for the exception. + public FeatureManagementException(FeatureManagementError errorType, string message) + : base(message) + { + Error = errorType; + } + + /// + /// The feature management error that the exception represents. + /// + public FeatureManagementError Error { get; set; } + } +} diff --git a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs new file mode 100644 index 00000000..4f26a6f0 --- /dev/null +++ b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +namespace Microsoft.FeatureManagement +{ + /// + /// Options that control the behavior of the feature management system. + /// + public class FeatureManagementOptions + { + /// + /// Controls the behavior of feature evaluation when dependent feature filters are missing. + /// If missing features filters are not ignored an exception will be thrown when attempting to evaluate a feature that depends on a missing feature filter. + /// + public bool IgnoreMissingFeatureFilters { get; set; } + } +} diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index 4a4f2ffe..baca4304 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -22,9 +22,14 @@ class FeatureManager : IFeatureManager private readonly ILogger _logger; private readonly ConcurrentDictionary _filterMetadataCache; private readonly ConcurrentDictionary _contextualFeatureFilterCache; - private readonly FeatureManagerOptions _options; - - public FeatureManager(IFeatureSettingsProvider settingsProvider, IEnumerable featureFilters, IEnumerable sessionManagers, ILoggerFactory loggerFactory, IOptionsMonitor optionsAccessor) + private readonly FeatureManagementOptions _options; + + public FeatureManager( + IFeatureSettingsProvider settingsProvider, + IEnumerable featureFilters, + IEnumerable sessionManagers, + ILoggerFactory loggerFactory, + IOptions options) { _settingsProvider = settingsProvider; _featureFilters = featureFilters ?? throw new ArgumentNullException(nameof(featureFilters)); @@ -32,7 +37,7 @@ public FeatureManager(IFeatureSettingsProvider settingsProvider, IEnumerable(); _filterMetadataCache = new ConcurrentDictionary(); _contextualFeatureFilterCache = new ConcurrentDictionary(); - _options = optionsAccessor.CurrentValue; + _options = options?.Value ?? throw new ArgumentNullException(nameof(options)); } public Task IsEnabledAsync(string feature) @@ -81,9 +86,17 @@ private async Task IsEnabledAsync(string feature, TContext appCo if (filter == null) { - if (!_options.SwallowExceptionForUnregisteredFilter) - throw new Exception($"Feature filter '{featureFilterSettings.Name}' specified for feature '{feature}' was not found."); - _logger.LogWarning($"Feature filter '{featureFilterSettings.Name}' specified for feature '{feature}' was not found."); + string errorMessage = $"The feature filter '{featureFilterSettings.Name}' specified for feature '{feature}' was not found."; + + if (!_options.IgnoreMissingFeatureFilters) + { + throw new FeatureManagementException(FeatureManagementError.MissingFeatureFilter, errorMessage); + } + else + { + _logger.LogWarning(errorMessage); + } + continue; } diff --git a/src/Microsoft.FeatureManagement/FeatureManagerOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagerOptions.cs deleted file mode 100644 index 13bfe15a..00000000 --- a/src/Microsoft.FeatureManagement/FeatureManagerOptions.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -namespace Microsoft.FeatureManagement -{ - /// - /// Options that are being used when a feature is being evaluated by the Feature Manager. - /// - public class FeatureManagerOptions - { - /// - /// Is being used to decide if an exception should be thrown or not when a configured feature filter has not been registered. - /// - public bool SwallowExceptionForUnregisteredFilter { get; set; } - } -} \ No newline at end of file diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 494cb6c2..6505a327 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -24,6 +24,7 @@ public class FeatureManagement private const string OnFeature = "OnTestFeature"; private const string OffFeature = "OffFeature"; private const string ConditionalFeature = "ConditionalFeature"; + private const string ContextualFeature = "ContextualFeature"; [Fact] public async Task ReadsConfiguration() @@ -76,12 +77,6 @@ public async Task Integrates() TestServer testServer = new TestServer(WebHost.CreateDefaultBuilder().ConfigureServices(services => { - services - .Configure(options => - { - options.SwallowExceptionForUnregisteredFilter = true; - }); - services .AddSingleton(config) .AddFeatureManagement() @@ -132,12 +127,6 @@ public async Task GatesFeatures() TestServer testServer = new TestServer(WebHost.CreateDefaultBuilder().ConfigureServices(services => { - services - .Configure(options => - { - options.SwallowExceptionForUnregisteredFilter = true; - }); - services .AddSingleton(config) .AddFeatureManagement() @@ -260,12 +249,6 @@ public async Task UsesContext() var serviceCollection = new ServiceCollection(); - serviceCollection - .Configure(options => - { - options.SwallowExceptionForUnregisteredFilter = true; - }); - serviceCollection.AddSingleton(config) .AddFeatureManagement() .AddFeatureFilter(); @@ -289,11 +272,11 @@ public async Task UsesContext() context.AccountId = "NotEnabledAccount"; - Assert.False(await featureManager.IsEnabledAsync(ConditionalFeature, context)); + Assert.False(await featureManager.IsEnabledAsync(ContextualFeature, context)); context.AccountId = "abc"; - Assert.True(await featureManager.IsEnabledAsync(ConditionalFeature, context)); + Assert.True(await featureManager.IsEnabledAsync(ContextualFeature, context)); } [Fact] @@ -319,7 +302,7 @@ public void LimitsFeatureFilterImplementations() } [Fact] - public void ThrowExceptionForUnregisteredFilter() + public async Task ThrowsExceptionForMissingFeatureFilter() { IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); @@ -333,20 +316,22 @@ public void ThrowExceptionForUnregisteredFilter() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - Assert.ThrowsAsync(async () => await featureManager.IsEnabledAsync(ConditionalFeature)); + FeatureManagementException e = await Assert.ThrowsAsync(async () => await featureManager.IsEnabledAsync(ConditionalFeature)); + + Assert.Equal(FeatureManagementError.MissingFeatureFilter, e.Error); } [Fact] - public async Task SwallowExceptionForUnregisteredFilter() + public async Task SwallowsExceptionForMissingFeatureFilter() { IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); var services = new ServiceCollection(); services - .Configure(options => + .Configure(options => { - options.SwallowExceptionForUnregisteredFilter = true; + options.IgnoreMissingFeatureFilters = true; }); services diff --git a/tests/Tests.FeatureManagement/appsettings.json b/tests/Tests.FeatureManagement/appsettings.json index ff27ac15..fc3d90bb 100644 --- a/tests/Tests.FeatureManagement/appsettings.json +++ b/tests/Tests.FeatureManagement/appsettings.json @@ -16,14 +16,6 @@ "Parameters": { "P1": "V1" } - }, - { - "Name": "ContextualTest", - "Parameters": { - "AllowedAccounts": [ - "abc" - ] - } } ] }, @@ -33,6 +25,18 @@ "Name": "Test" } ] + }, + "ContextualFeature": { + "EnabledFor": [ + { + "Name": "ContextualTest", + "Parameters": { + "AllowedAccounts": [ + "abc" + ] + } + } + ] } } } From a41ce8ac3daafcaf6bf0a9b6df47ecaf4507d4a2 Mon Sep 17 00:00:00 2001 From: Jimmy Campbell Date: Mon, 24 Feb 2020 12:04:10 -0500 Subject: [PATCH 3/3] Fixed typo. --- src/Microsoft.FeatureManagement/FeatureManagementOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs index 4f26a6f0..94da5900 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs @@ -10,7 +10,7 @@ public class FeatureManagementOptions { /// /// Controls the behavior of feature evaluation when dependent feature filters are missing. - /// If missing features filters are not ignored an exception will be thrown when attempting to evaluate a feature that depends on a missing feature filter. + /// If missing feature filters are not ignored an exception will be thrown when attempting to evaluate a feature that depends on a missing feature filter. /// public bool IgnoreMissingFeatureFilters { get; set; } }