diff --git a/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj b/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj index f0c31e0e..821ff397 100644 --- a/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj +++ b/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj @@ -4,7 +4,7 @@ 2 - 3 + 4 0 diff --git a/src/Microsoft.FeatureManagement/FeatureFilters/PercentageFilter.cs b/src/Microsoft.FeatureManagement/FeatureFilters/PercentageFilter.cs index c63441cc..63f6aa01 100644 --- a/src/Microsoft.FeatureManagement/FeatureFilters/PercentageFilter.cs +++ b/src/Microsoft.FeatureManagement/FeatureFilters/PercentageFilter.cs @@ -48,7 +48,7 @@ public Task EvaluateAsync(FeatureFilterEvaluationContext context, Cancella if (result) { - result = (RandomGenerator.NextDouble() * 100) <= settings.Value; + result = (RandomGenerator.NextDouble() * 100) < settings.Value; } return Task.FromResult(result); diff --git a/src/Microsoft.FeatureManagement/FeatureManagementError.cs b/src/Microsoft.FeatureManagement/FeatureManagementError.cs index 60f8a81d..8eb6208a 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagementError.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagementError.cs @@ -18,6 +18,11 @@ public enum FeatureManagementError /// AmbiguousFeatureFilter, + /// + /// A feature that was requested for evaluation was not found. + /// + MissingFeature, + /// /// A feature filter being used in feature evaluation is invalid. /// @@ -43,11 +48,6 @@ public enum FeatureManagementError /// InvalidFeatureVariantAssigner, - /// - /// A feature that was requested for evaluation was not found. - /// - MissingFeature, - /// /// A dynamic feature does not have any feature variants registered. /// diff --git a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs index 9322934e..e425c054 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs @@ -11,7 +11,15 @@ public class FeatureManagementOptions /// /// Controls the behavior of feature evaluation when dependent feature filters are missing. /// 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. + /// The default value is false. /// public bool IgnoreMissingFeatureFilters { get; set; } + + /// + /// Controls the behavior of feature evaluation when the target feature is missing. + /// If missing features are not ignored an exception will be thrown when attempting to evaluate them. + /// The default value is true. + /// + public bool IgnoreMissingFeatures { get; set; } = true; } } diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index b119d4f8..2ebd611c 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -161,6 +161,19 @@ private async Task IsEnabledAsync(string feature, TContext appCo } } } + else + { + string errorMessage = $"The feature declaration for the feature '{feature}' was not found."; + + if (!_options.IgnoreMissingFeatures) + { + throw new FeatureManagementException(FeatureManagementError.MissingFeature, errorMessage); + } + else + { + _logger.LogWarning(errorMessage); + } + } foreach (ISessionManager sessionManager in _sessionManagers) { diff --git a/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs b/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs index 2abea404..e61dbe49 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. // using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Runtime.CompilerServices; using System.Threading; @@ -15,7 +16,7 @@ namespace Microsoft.FeatureManagement class FeatureManagerSnapshot : IFeatureManagerSnapshot { private readonly IFeatureManager _featureManager; - private readonly IDictionary _flagCache = new Dictionary(); + private readonly ConcurrentDictionary> _flagCache = new ConcurrentDictionary>(); private IEnumerable _featureFlagNames; public FeatureManagerSnapshot(IFeatureManager featureManager) @@ -43,36 +44,18 @@ public async IAsyncEnumerable GetFeatureFlagNamesAsync([EnumeratorCancel } } - public async Task IsEnabledAsync(string feature, CancellationToken cancellationToken) + public Task IsEnabledAsync(string feature, CancellationToken cancellationToken) { - // - // First, check local cache - if (_flagCache.ContainsKey(feature)) - { - return _flagCache[feature]; - } - - bool enabled = await _featureManager.IsEnabledAsync(feature, cancellationToken).ConfigureAwait(false); - - _flagCache[feature] = enabled; - - return enabled; + return _flagCache.GetOrAdd( + feature, + (key) => _featureManager.IsEnabledAsync(key, cancellationToken)); } - public async Task IsEnabledAsync(string feature, TContext context, CancellationToken cancellationToken) + public Task IsEnabledAsync(string feature, TContext context, CancellationToken cancellationToken) { - // - // First, check local cache - if (_flagCache.ContainsKey(feature)) - { - return _flagCache[feature]; - } - - bool enabled = await _featureManager.IsEnabledAsync(feature, context, cancellationToken).ConfigureAwait(false); - - _flagCache[feature] = enabled; - - return enabled; + return _flagCache.GetOrAdd( + feature, + (key) => _featureManager.IsEnabledAsync(key, context, cancellationToken)); } } } diff --git a/src/Microsoft.FeatureManagement/FilterAliasAttribute.cs b/src/Microsoft.FeatureManagement/FilterAliasAttribute.cs index 42f5571a..30f9d997 100644 --- a/src/Microsoft.FeatureManagement/FilterAliasAttribute.cs +++ b/src/Microsoft.FeatureManagement/FilterAliasAttribute.cs @@ -18,7 +18,7 @@ public FilterAliasAttribute(string alias) { if (string.IsNullOrEmpty(alias)) { - throw new ArgumentNullException(alias); + throw new ArgumentNullException(nameof(alias)); } Alias = alias; diff --git a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj index 878778f0..620a1264 100644 --- a/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj +++ b/src/Microsoft.FeatureManagement/Microsoft.FeatureManagement.csproj @@ -3,10 +3,9 @@ - 3 - 0 + 2 + 4 0 - -preview diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 4f14944f..4a935744 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -48,9 +48,9 @@ public async Task ReadsConfiguration() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - Assert.True(await featureManager.IsEnabledAsync(OnFeature, CancellationToken.None)); + Assert.True(await featureManager.IsEnabledAsync(OnFeature)); - Assert.False(await featureManager.IsEnabledAsync(OffFeature, CancellationToken.None)); + Assert.False(await featureManager.IsEnabledAsync(OffFeature)); IEnumerable featureFilters = serviceProvider.GetRequiredService>(); @@ -68,10 +68,10 @@ public async Task ReadsConfiguration() Assert.Equal(ConditionalFeature, evaluationContext.FeatureName); - return true; + return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(ConditionalFeature, CancellationToken.None); + await featureManager.IsEnabledAsync(ConditionalFeature); Assert.True(called); @@ -155,7 +155,7 @@ public async Task ReadsV1Configuration() Assert.Equal(ConditionalFeature, evaluationContext.FeatureName); - return true; + return Task.FromResult(true); }; await featureManager.IsEnabledAsync(ConditionalFeature, CancellationToken.None); @@ -195,7 +195,7 @@ public async Task AllowsSuffix() { called = true; - return true; + return Task.FromResult(true); }; await featureManager.IsEnabledAsync(WithSuffixFeature, CancellationToken.None); @@ -244,14 +244,14 @@ public async Task Integrates() TestFilter testFeatureFilter = (TestFilter)featureFilters.First(f => f is TestFilter); - testFeatureFilter.Callback = _ => true; + testFeatureFilter.Callback = _ => Task.FromResult(true); HttpResponseMessage res = await testServer.CreateClient().GetAsync(""); Assert.True(res.Headers.Contains(nameof(MvcFilter))); Assert.True(res.Headers.Contains(nameof(RouterMiddleware))); - testFeatureFilter.Callback = _ => false; + testFeatureFilter.Callback = _ => Task.FromResult(false); res = await testServer.CreateClient().GetAsync(""); @@ -281,7 +281,7 @@ public async Task GatesFeatures() // // Enable all features - testFeatureFilter.Callback = ctx => true; + testFeatureFilter.Callback = ctx => Task.FromResult(true); HttpResponseMessage gateAllResponse = await testServer.CreateClient().GetAsync("gateAll"); HttpResponseMessage gateAnyResponse = await testServer.CreateClient().GetAsync("gateAny"); @@ -291,7 +291,7 @@ public async Task GatesFeatures() // // Enable 1/2 features - testFeatureFilter.Callback = ctx => ctx.FeatureName == Features.ConditionalFeature; + testFeatureFilter.Callback = ctx => Task.FromResult(ctx.FeatureName == Features.ConditionalFeature); gateAllResponse = await testServer.CreateClient().GetAsync("gateAll"); gateAnyResponse = await testServer.CreateClient().GetAsync("gateAny"); @@ -301,7 +301,7 @@ public async Task GatesFeatures() // // Enable no - testFeatureFilter.Callback = ctx => false; + testFeatureFilter.Callback = ctx => Task.FromResult(false); gateAllResponse = await testServer.CreateClient().GetAsync("gateAll"); gateAnyResponse = await testServer.CreateClient().GetAsync("gateAny"); @@ -342,10 +342,10 @@ public async Task TimeWindow() IFeatureManager featureManager = provider.GetRequiredService(); - Assert.True(await featureManager.IsEnabledAsync(feature1, CancellationToken.None)); - Assert.False(await featureManager.IsEnabledAsync(feature2, CancellationToken.None)); - Assert.True(await featureManager.IsEnabledAsync(feature3, CancellationToken.None)); - Assert.False(await featureManager.IsEnabledAsync(feature4, CancellationToken.None)); + Assert.True(await featureManager.IsEnabledAsync(feature1)); + Assert.False(await featureManager.IsEnabledAsync(feature2)); + Assert.True(await featureManager.IsEnabledAsync(feature3)); + Assert.False(await featureManager.IsEnabledAsync(feature4)); } [Fact] @@ -372,7 +372,7 @@ public async Task Percentage() for (int i = 0; i < 10; i++) { - if (await featureManager.IsEnabledAsync(feature1, CancellationToken.None)) + if (await featureManager.IsEnabledAsync(feature1)) { enabledCount++; } @@ -410,21 +410,21 @@ public async Task Targeting() Assert.True(await featureManager.IsEnabledAsync(targetingTestFeature, new TargetingContext { UserId = "Jeff" - }, CancellationToken.None)); + })); // // Not targeted by user id, but targeted by default rollout Assert.True(await featureManager.IsEnabledAsync(targetingTestFeature, new TargetingContext { UserId = "Anne" - }, CancellationToken.None)); + })); // // Not targeted by user id or default rollout Assert.False(await featureManager.IsEnabledAsync(targetingTestFeature, new TargetingContext { UserId = "Patty" - }, CancellationToken.None)); + })); // // Targeted by group rollout @@ -432,7 +432,7 @@ public async Task Targeting() { UserId = "Patty", Groups = new List() { "Ring1" } - }, CancellationToken.None)); + })); // // Not targeted by user id, default rollout or group rollout @@ -440,7 +440,7 @@ public async Task Targeting() { UserId = "Isaac", Groups = new List() { "Ring1" } - }, CancellationToken.None)); + })); } [Fact] @@ -623,7 +623,7 @@ public async Task TargetingAccessor() UserId = "Jeff" }; - Assert.True(await featureManager.IsEnabledAsync(beta, CancellationToken.None)); + Assert.True(await featureManager.IsEnabledAsync(beta)); // // Not targeted by user id or default rollout @@ -632,7 +632,7 @@ public async Task TargetingAccessor() UserId = "Patty" }; - Assert.False(await featureManager.IsEnabledAsync(beta, CancellationToken.None)); + Assert.False(await featureManager.IsEnabledAsync(beta)); } [Fact] @@ -665,11 +665,11 @@ public async Task UsesContext() context.AccountId = "NotEnabledAccount"; - Assert.False(await featureManager.IsEnabledAsync(ContextualFeature, context, CancellationToken.None)); + Assert.False(await featureManager.IsEnabledAsync(ContextualFeature, context)); context.AccountId = "abc"; - Assert.True(await featureManager.IsEnabledAsync(ContextualFeature, context, CancellationToken.None)); + Assert.True(await featureManager.IsEnabledAsync(ContextualFeature, context)); } [Fact] @@ -851,11 +851,36 @@ public async Task SwallowsExceptionForMissingFeatureFilter() IFeatureManager featureManager = serviceProvider.GetRequiredService(); - var isEnabled = await featureManager.IsEnabledAsync(ConditionalFeature, CancellationToken.None); + var isEnabled = await featureManager.IsEnabledAsync(ConditionalFeature); Assert.False(isEnabled); } + [Fact] + public async Task ThrowsForMissingFeatures() + { + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + var services = new ServiceCollection(); + + services + .Configure(options => + { + options.IgnoreMissingFeatures = false; + }); + + services + .AddSingleton(config) + .AddFeatureManagement(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + FeatureManagementException fme = await Assert.ThrowsAsync(() => + featureManager.IsEnabledAsync("NonExistentFeature")); + } + [Fact] public async Task CustomFeatureDefinitionProvider() { @@ -952,10 +977,10 @@ public async Task CustomFeatureDefinitionProvider() Assert.Equal(ConditionalFeature, evaluationContext.FeatureName); - return true; + return Task.FromResult(true); }; - await featureManager.IsEnabledAsync(ConditionalFeature, CancellationToken.None); + await featureManager.IsEnabledAsync(ConditionalFeature); Assert.True(called); @@ -997,6 +1022,58 @@ public async Task CustomFeatureDefinitionProvider() Assert.True(called); } + [Fact] + public async Task ThreadsafeSnapshot() + { + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + var services = new ServiceCollection(); + + services + .AddSingleton(config) + .AddFeatureManagement() + .AddFeatureFilter(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + IEnumerable featureFilters = serviceProvider.GetRequiredService>(); + + // + // Sync filter + TestFilter testFeatureFilter = (TestFilter)featureFilters.First(f => f is TestFilter); + + bool called = false; + + testFeatureFilter.Callback = async (evaluationContext) => + { + called = true; + + await Task.Delay(10); + + return new Random().Next(0, 100) > 50; + }; + + var tasks = new List>(); + + for (int i = 0; i < 1000; i++) + { + tasks.Add(featureManager.IsEnabledAsync(ConditionalFeature)); + } + + Assert.True(called); + + await Task.WhenAll(tasks); + + bool result = tasks.First().Result; + + foreach (Task t in tasks) + { + Assert.Equal(result, t.Result); + } + } + private static void DisableEndpointRouting(MvcOptions options) { #if NET5_0 || NETCOREAPP3_1 diff --git a/tests/Tests.FeatureManagement/InMemoryFeatureDefinitionProvider.cs b/tests/Tests.FeatureManagement/InMemoryFeatureDefinitionProvider.cs index 01e347ab..ee0cfa9b 100644 --- a/tests/Tests.FeatureManagement/InMemoryFeatureDefinitionProvider.cs +++ b/tests/Tests.FeatureManagement/InMemoryFeatureDefinitionProvider.cs @@ -45,7 +45,7 @@ public Task GetDynamicFeatureDefinitionAsync(string fe } #pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously - public async IAsyncEnumerable GetAllDynamicFeatureDefinitionsAsync(CancellationToken cancellationToken = default) + public async IAsyncEnumerable GetAllDynamicFeatureDefinitionsAsync([EnumeratorCancellation] CancellationToken cancellationToken = default) #pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously { foreach (DynamicFeatureDefinition definition in _dynamicFeatureDefinitions) diff --git a/tests/Tests.FeatureManagement/TestFilter.cs b/tests/Tests.FeatureManagement/TestFilter.cs index 5ec056d0..6d71464f 100644 --- a/tests/Tests.FeatureManagement/TestFilter.cs +++ b/tests/Tests.FeatureManagement/TestFilter.cs @@ -10,11 +10,11 @@ namespace Tests.FeatureManagement { class TestFilter : IFeatureFilter { - public Func Callback { get; set; } + public Func> Callback { get; set; } public Task EvaluateAsync(FeatureFilterEvaluationContext context, CancellationToken cancellationToken) { - return Task.FromResult(Callback?.Invoke(context) ?? false); + return Callback?.Invoke(context) ?? Task.FromResult(false); } } }