From 8be45cdd629a14605ceca5c9581703e389b8d24e Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 25 Sep 2024 12:22:31 -0700 Subject: [PATCH 01/15] Resolves snapshot breaking change and adjusts feature tag helper as well --- .../TagHelpers/FeatureTagHelper.cs | 11 +- .../FeatureManagerSnapshot.cs | 47 +++-- .../ServiceCollectionExtensions.cs | 94 ++++------ .../FeatureManagementAspNetCore.cs | 69 ++++++++ .../Tests.FeatureManagement.AspNetCore.csproj | 1 + .../FeatureManagementTest.cs | 164 ++++++++++++------ .../Tests.FeatureManagement.csproj | 1 + 7 files changed, 260 insertions(+), 127 deletions(-) diff --git a/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs b/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs index f65e62e8..2cc897b8 100644 --- a/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs +++ b/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs @@ -14,7 +14,8 @@ namespace Microsoft.FeatureManagement.Mvc.TagHelpers /// public class FeatureTagHelper : TagHelper { - private readonly IVariantFeatureManager _featureManager; + private readonly IFeatureManager _featureManager; + private readonly IVariantFeatureManager _variantFeatureManager; /// /// A feature name, or comma separated list of feature names, for which the content should be rendered. By default, all specified features must be enabled to render the content, but this requirement can be controlled by the property. @@ -38,12 +39,14 @@ public class FeatureTagHelper : TagHelper public string Variant { get; set; } /// - /// Creates a feature tag helper. + /// Creates a feature tag helper. Takes both a feature manager and a variant feature manager for backwards compatibility. /// /// The feature manager snapshot to use to evaluate feature state. - public FeatureTagHelper(IVariantFeatureManagerSnapshot featureManager) + /// The variant feature manager snapshot to use to evaluate feature state. + public FeatureTagHelper(IFeatureManagerSnapshot featureManager, IVariantFeatureManagerSnapshot variantFeatureManager) { _featureManager = featureManager ?? throw new ArgumentNullException(nameof(featureManager)); + _variantFeatureManager = variantFeatureManager ?? throw new ArgumentNullException(nameof(variantFeatureManager)); } /// @@ -84,7 +87,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu enabled = await variants.Any( async variant => { - Variant assignedVariant = await _featureManager.GetVariantAsync(features.First()).ConfigureAwait(false); + Variant assignedVariant = await _variantFeatureManager.GetVariantAsync(features.First()).ConfigureAwait(false); return variant == assignedVariant?.Name; }); diff --git a/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs b/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs index 384bb2b1..a324ca6a 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs @@ -16,19 +16,38 @@ namespace Microsoft.FeatureManagement /// class FeatureManagerSnapshot : IFeatureManagerSnapshot, IVariantFeatureManagerSnapshot { - private readonly IVariantFeatureManager _featureManager; - private readonly ConcurrentDictionary> _flagCache = new ConcurrentDictionary>(); + private readonly IFeatureManager _featureManager; + private readonly IVariantFeatureManager _variantFeatureManager; + private readonly ConcurrentDictionary> _flagCache = new ConcurrentDictionary>(); + private readonly ConcurrentDictionary> _variantFlagCache = new ConcurrentDictionary>(); private readonly ConcurrentDictionary _variantCache = new ConcurrentDictionary(); private IEnumerable _featureNames; - public FeatureManagerSnapshot(IVariantFeatureManager featureManager) + // Takes both a feature manager and a variant feature manager for backwards compatibility. + public FeatureManagerSnapshot(IFeatureManager featureManager, IVariantFeatureManager variantFeatureManager) { _featureManager = featureManager ?? throw new ArgumentNullException(nameof(featureManager)); + _variantFeatureManager = variantFeatureManager ?? throw new ArgumentNullException(nameof(variantFeatureManager)); } - public IAsyncEnumerable GetFeatureNamesAsync() + public async IAsyncEnumerable GetFeatureNamesAsync() { - return GetFeatureNamesAsync(CancellationToken.None); + if (_featureNames == null) + { + var featureNames = new List(); + + await foreach (string featureName in _featureManager.GetFeatureNamesAsync().ConfigureAwait(false)) + { + featureNames.Add(featureName); + } + + _featureNames = featureNames; + } + + foreach (string featureName in _featureNames) + { + yield return featureName; + } } public async IAsyncEnumerable GetFeatureNamesAsync([EnumeratorCancellation] CancellationToken cancellationToken) @@ -37,7 +56,7 @@ public async IAsyncEnumerable GetFeatureNamesAsync([EnumeratorCancellati { var featureNames = new List(); - await foreach (string featureName in _featureManager.GetFeatureNamesAsync(cancellationToken).ConfigureAwait(false)) + await foreach (string featureName in _variantFeatureManager.GetFeatureNamesAsync(cancellationToken).ConfigureAwait(false)) { featureNames.Add(featureName); } @@ -55,28 +74,28 @@ public Task IsEnabledAsync(string feature) { return _flagCache.GetOrAdd( feature, - (key) => _featureManager.IsEnabledAsync(key, CancellationToken.None)).AsTask(); + (key) => _featureManager.IsEnabledAsync(key)); } public Task IsEnabledAsync(string feature, TContext context) { return _flagCache.GetOrAdd( feature, - (key) => _featureManager.IsEnabledAsync(key, context, CancellationToken.None)).AsTask(); + (key) => _featureManager.IsEnabledAsync(key, context)); } public ValueTask IsEnabledAsync(string feature, CancellationToken cancellationToken) { - return _flagCache.GetOrAdd( + return _variantFlagCache.GetOrAdd( feature, - (key) => _featureManager.IsEnabledAsync(key, cancellationToken)); + (key) => _variantFeatureManager.IsEnabledAsync(key, cancellationToken)); } public ValueTask IsEnabledAsync(string feature, TContext context, CancellationToken cancellationToken) { - return _flagCache.GetOrAdd( + return _variantFlagCache.GetOrAdd( feature, - (key) => _featureManager.IsEnabledAsync(key, context, cancellationToken)); + (key) => _variantFeatureManager.IsEnabledAsync(key, context, cancellationToken)); } public async ValueTask GetVariantAsync(string feature, CancellationToken cancellationToken) @@ -90,7 +109,7 @@ public async ValueTask GetVariantAsync(string feature, CancellationToke return _variantCache[cacheKey]; } - Variant variant = await _featureManager.GetVariantAsync(feature, cancellationToken).ConfigureAwait(false); + Variant variant = await _variantFeatureManager.GetVariantAsync(feature, cancellationToken).ConfigureAwait(false); _variantCache[cacheKey] = variant; @@ -108,7 +127,7 @@ public async ValueTask GetVariantAsync(string feature, ITargetingContex return _variantCache[cacheKey]; } - Variant variant = await _featureManager.GetVariantAsync(feature, context, cancellationToken).ConfigureAwait(false); + Variant variant = await _variantFeatureManager.GetVariantAsync(feature, context, cancellationToken).ConfigureAwait(false); _variantCache[cacheKey] = variant; diff --git a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs index df7d3d00..602dd20c 100644 --- a/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs +++ b/src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs @@ -34,13 +34,7 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec "Scoped feature management has been registered."); } - services.AddLogging(); - - services.AddMemoryCache(); - - // - // Add required services - services.TryAddSingleton(); + AddCommonFeatureManagementServices(services); services.AddSingleton(sp => new FeatureManager( sp.GetRequiredService(), @@ -58,27 +52,7 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec services.TryAddSingleton(sp => sp.GetRequiredService()); - services.AddScoped(); - - services.TryAddScoped(sp => sp.GetRequiredService()); - - services.TryAddScoped(sp => sp.GetRequiredService()); - - var builder = new FeatureManagementBuilder(services); - - // - // Add built-in feature filters - builder.AddFeatureFilter(); - - builder.AddFeatureFilter(sp => - new TimeWindowFilter() - { - Cache = sp.GetRequiredService() - }); - - builder.AddFeatureFilter(); - - return builder; + return GetFeatureManagementBuilder(services); } /// @@ -120,13 +94,7 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService "Singleton feature management has been registered."); } - services.AddLogging(); - - services.AddMemoryCache(); - - // - // Add required services - services.TryAddSingleton(); + AddCommonFeatureManagementServices(services); services.AddScoped(sp => new FeatureManager( sp.GetRequiredService(), @@ -144,27 +112,7 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService services.TryAddScoped(sp => sp.GetRequiredService()); - services.AddScoped(); - - services.TryAddScoped(sp => sp.GetRequiredService()); - - services.TryAddScoped(sp => sp.GetRequiredService()); - - var builder = new FeatureManagementBuilder(services); - - // - // Add built-in feature filters - builder.AddFeatureFilter(); - - builder.AddFeatureFilter(sp => - new TimeWindowFilter() - { - Cache = sp.GetRequiredService() - }); - - builder.AddFeatureFilter(); - - return builder; + return GetFeatureManagementBuilder(services); } /// @@ -190,5 +138,39 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService return services.AddScopedFeatureManagement(); } + + private static void AddCommonFeatureManagementServices(IServiceCollection services) + { + services.AddLogging(); + + services.AddMemoryCache(); + + services.TryAddSingleton(); + + services.AddScoped(); + + services.TryAddScoped(sp => sp.GetRequiredService()); + + services.TryAddScoped(sp => sp.GetRequiredService()); + } + + private static IFeatureManagementBuilder GetFeatureManagementBuilder(IServiceCollection services) + { + var builder = new FeatureManagementBuilder(services); + + // + // Add built-in feature filters + builder.AddFeatureFilter(); + + builder.AddFeatureFilter(sp => + new TimeWindowFilter() + { + Cache = sp.GetRequiredService() + }); + + builder.AddFeatureFilter(); + + return builder; + } } } diff --git a/tests/Tests.FeatureManagement.AspNetCore/FeatureManagementAspNetCore.cs b/tests/Tests.FeatureManagement.AspNetCore/FeatureManagementAspNetCore.cs index dce12cfe..79352588 100644 --- a/tests/Tests.FeatureManagement.AspNetCore/FeatureManagementAspNetCore.cs +++ b/tests/Tests.FeatureManagement.AspNetCore/FeatureManagementAspNetCore.cs @@ -5,10 +5,12 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Razor.TagHelpers; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.FeatureManagement; +using Microsoft.FeatureManagement.Mvc.TagHelpers; using System.Collections.Generic; using System.Linq; using System.Net; @@ -181,4 +183,71 @@ private static void DisableEndpointRouting(MvcOptions options) options.EnableEndpointRouting = false; } } + + public class CustomImplementationsFeatureManagementTests + { + public class CustomIFeatureManager : IFeatureManager + { + public IAsyncEnumerable GetFeatureNamesAsync() + { + return new string[1] { "Test" }.ToAsyncEnumerable(); + } + + public async Task IsEnabledAsync(string feature) + { + return await Task.FromResult(feature == "Test"); + } + + public async Task IsEnabledAsync(string feature, TContext context) + { + return await Task.FromResult(feature == "Test"); + } + } + + [Fact] + public async Task CustomIFeatureManagerAspNetCoreTest() + { + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + var services = new ServiceCollection(); + + services.AddSingleton(config) + .AddSingleton() + .AddFeatureManagement(); // Shouldn't override + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + Assert.True(await featureManager.IsEnabledAsync("Test")); + Assert.False(await featureManager.IsEnabledAsync("NotTest")); + + // FeatureTagHelper should use available IFeatureManager + FeatureTagHelper featureTagHelper = new FeatureTagHelper(serviceProvider.GetRequiredService(), serviceProvider.GetRequiredService()); + TagHelperOutput tagHelperOutput = new TagHelperOutput("TestTag", new TagHelperAttributeList(), (aBool, aHtmlEncoder) => { return null; }); + + // Test returns true, so it shouldn't be modified + featureTagHelper.Name = "Test"; + Assert.False(tagHelperOutput.IsContentModified); + await featureTagHelper.ProcessAsync(null, tagHelperOutput); + Assert.False(tagHelperOutput.IsContentModified); + + tagHelperOutput.Reinitialize("TestTag", TagMode.StartTagAndEndTag); + + // NotTest returns false, so it should be modified + featureTagHelper.Name = "NotTest"; + Assert.False(tagHelperOutput.IsContentModified); + await featureTagHelper.ProcessAsync(null, tagHelperOutput); + Assert.True(tagHelperOutput.IsContentModified); + + tagHelperOutput.Reinitialize("TestTag", TagMode.StartTagAndEndTag); + + // When variant is used, Test flag should no longer exist and return false + featureTagHelper.Name = "Test"; + featureTagHelper.Variant = "Something"; + Assert.False(tagHelperOutput.IsContentModified); + await featureTagHelper.ProcessAsync(null, tagHelperOutput); + Assert.True(tagHelperOutput.IsContentModified); + } + } } diff --git a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj index 72916ae8..398c1588 100644 --- a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj +++ b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj @@ -15,6 +15,7 @@ + diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index d12fe51c..989fc11c 100644 --- a/tests/Tests.FeatureManagement/FeatureManagementTest.cs +++ b/tests/Tests.FeatureManagement/FeatureManagementTest.cs @@ -375,59 +375,6 @@ public void AddsScopedFeatureManagement() Assert.Equal($"Singleton feature management has been registered.", ex.Message); } - [Fact] - public async Task CustomFeatureDefinitionProvider() - { - FeatureDefinition testFeature = new FeatureDefinition - { - Name = Features.ConditionalFeature, - EnabledFor = new List() - { - new FeatureFilterConfiguration - { - Name = "Test", - Parameters = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary() - { - { "P1", "V1" }, - }).Build() - } - } - }; - - var services = new ServiceCollection(); - - services.AddSingleton(new InMemoryFeatureDefinitionProvider(new FeatureDefinition[] { testFeature })) - .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 = (evaluationContext) => - { - called = true; - - Assert.Equal("V1", evaluationContext.Parameters["P1"]); - - Assert.Equal(Features.ConditionalFeature, evaluationContext.FeatureName); - - return Task.FromResult(true); - }; - - await featureManager.IsEnabledAsync(Features.ConditionalFeature); - - Assert.True(called); - } - [Fact] public async Task LastFeatureFlagWins() { @@ -1918,4 +1865,115 @@ public async Task TelemetryPublishing() Assert.True(result); } } + + public class CustomImplementationsFeatureManagementTests + { + public class CustomIFeatureManager : IFeatureManager + { + public IAsyncEnumerable GetFeatureNamesAsync() + { + return new string[1] { "Test" }.ToAsyncEnumerable(); + } + + public async Task IsEnabledAsync(string feature) + { + return await Task.FromResult(feature == "Test"); + } + + public async Task IsEnabledAsync(string feature, TContext context) + { + return await Task.FromResult(feature == "Test"); + } + } + + [Fact] + public async Task CustomIFeatureManagerTest() + { + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + var services = new ServiceCollection(); + + services.AddSingleton(config) + .AddSingleton() + .AddFeatureManagement(); // Shouldn't override + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IFeatureManager featureManager = serviceProvider.GetRequiredService(); + + Assert.True(await featureManager.IsEnabledAsync("Test")); + Assert.False(await featureManager.IsEnabledAsync("NotTest")); + + // Provider shouldn't be affected + IFeatureDefinitionProvider featureDefinitionProvider = serviceProvider.GetRequiredService(); + + Assert.True(await featureDefinitionProvider.GetAllFeatureDefinitionsAsync().AnyAsync()); + Assert.NotNull(await featureDefinitionProvider.GetFeatureDefinitionAsync("OnTestFeature")); + + // Snapshot should use available IFeatureManager + FeatureManagerSnapshot featureManagerSnapshot = serviceProvider.GetRequiredService(); + + Assert.True(await featureManagerSnapshot.IsEnabledAsync("Test")); + Assert.False(await featureManagerSnapshot.IsEnabledAsync("NotTest")); + Assert.False(await featureManagerSnapshot.IsEnabledAsync("OnTestFeature")); + + // But use IVariantFeatureManager when using new interface + Assert.False(await featureManagerSnapshot.IsEnabledAsync("Test", CancellationToken.None)); + Assert.False(await featureManagerSnapshot.IsEnabledAsync("NotTest", CancellationToken.None)); + Assert.True(await featureManagerSnapshot.IsEnabledAsync("OnTestFeature", CancellationToken.None)); + } + + [Fact] + public async Task CustomIFeatureDefinitionProvider() + { + FeatureDefinition testFeature = new FeatureDefinition + { + Name = Features.ConditionalFeature, + EnabledFor = new List() + { + new FeatureFilterConfiguration + { + Name = "Test", + Parameters = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary() + { + { "P1", "V1" }, + }).Build() + } + } + }; + + var services = new ServiceCollection(); + + services.AddSingleton(new InMemoryFeatureDefinitionProvider(new FeatureDefinition[] { testFeature })) + .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 = (evaluationContext) => + { + called = true; + + Assert.Equal("V1", evaluationContext.Parameters["P1"]); + + Assert.Equal(Features.ConditionalFeature, evaluationContext.FeatureName); + + return Task.FromResult(true); + }; + + await featureManager.IsEnabledAsync(Features.ConditionalFeature); + + Assert.True(called); + } + } } diff --git a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj index 829caf87..daf20490 100644 --- a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj +++ b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj @@ -10,6 +10,7 @@ + From ea8fea117c3de6b2e199b4764bf97b7e5b0482de Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 25 Sep 2024 15:34:13 -0700 Subject: [PATCH 02/15] Moves comment out of summary --- .../TagHelpers/FeatureTagHelper.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs b/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs index 2cc897b8..cddcf129 100644 --- a/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs +++ b/src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs @@ -39,12 +39,13 @@ public class FeatureTagHelper : TagHelper public string Variant { get; set; } /// - /// Creates a feature tag helper. Takes both a feature manager and a variant feature manager for backwards compatibility. + /// Creates a feature tag helper. /// /// The feature manager snapshot to use to evaluate feature state. /// The variant feature manager snapshot to use to evaluate feature state. public FeatureTagHelper(IFeatureManagerSnapshot featureManager, IVariantFeatureManagerSnapshot variantFeatureManager) { + // Takes both a feature manager and a variant feature manager for backwards compatibility. _featureManager = featureManager ?? throw new ArgumentNullException(nameof(featureManager)); _variantFeatureManager = variantFeatureManager ?? throw new ArgumentNullException(nameof(variantFeatureManager)); } From 3e075227074cb2975fcec7ed5125000b47805ec8 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 25 Sep 2024 16:37:44 -0700 Subject: [PATCH 03/15] Adjusted snapshot to use a shared IsEnabled cache --- .../FeatureManagerSnapshot.cs | 11 +++++------ .../Tests.FeatureManagement/FeatureManagementTest.cs | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs b/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs index a324ca6a..12004aea 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs @@ -18,8 +18,7 @@ class FeatureManagerSnapshot : IFeatureManagerSnapshot, IVariantFeatureManagerSn { private readonly IFeatureManager _featureManager; private readonly IVariantFeatureManager _variantFeatureManager; - private readonly ConcurrentDictionary> _flagCache = new ConcurrentDictionary>(); - private readonly ConcurrentDictionary> _variantFlagCache = new ConcurrentDictionary>(); + private readonly ConcurrentDictionary> _flagCache = new ConcurrentDictionary>(); private readonly ConcurrentDictionary _variantCache = new ConcurrentDictionary(); private IEnumerable _featureNames; @@ -74,26 +73,26 @@ public Task IsEnabledAsync(string feature) { return _flagCache.GetOrAdd( feature, - (key) => _featureManager.IsEnabledAsync(key)); + (key) => new ValueTask(_featureManager.IsEnabledAsync(key))).AsTask(); } public Task IsEnabledAsync(string feature, TContext context) { return _flagCache.GetOrAdd( feature, - (key) => _featureManager.IsEnabledAsync(key, context)); + (key) => new ValueTask(_featureManager.IsEnabledAsync(key, context))).AsTask(); } public ValueTask IsEnabledAsync(string feature, CancellationToken cancellationToken) { - return _variantFlagCache.GetOrAdd( + return _flagCache.GetOrAdd( feature, (key) => _variantFeatureManager.IsEnabledAsync(key, cancellationToken)); } public ValueTask IsEnabledAsync(string feature, TContext context, CancellationToken cancellationToken) { - return _variantFlagCache.GetOrAdd( + return _flagCache.GetOrAdd( feature, (key) => _variantFeatureManager.IsEnabledAsync(key, context, cancellationToken)); } diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index 989fc11c..af7d161a 100644 --- a/tests/Tests.FeatureManagement/FeatureManagementTest.cs +++ b/tests/Tests.FeatureManagement/FeatureManagementTest.cs @@ -1917,10 +1917,10 @@ public async Task CustomIFeatureManagerTest() Assert.False(await featureManagerSnapshot.IsEnabledAsync("NotTest")); Assert.False(await featureManagerSnapshot.IsEnabledAsync("OnTestFeature")); - // But use IVariantFeatureManager when using new interface - Assert.False(await featureManagerSnapshot.IsEnabledAsync("Test", CancellationToken.None)); + // Use snapshot results even though IVariantFeatureManager would be called here + Assert.True(await featureManagerSnapshot.IsEnabledAsync("Test", CancellationToken.None)); Assert.False(await featureManagerSnapshot.IsEnabledAsync("NotTest", CancellationToken.None)); - Assert.True(await featureManagerSnapshot.IsEnabledAsync("OnTestFeature", CancellationToken.None)); + Assert.False(await featureManagerSnapshot.IsEnabledAsync("OnTestFeature", CancellationToken.None)); } [Fact] From 4fc399f7aa6cef02dc0bfad960608d9a30bbffb0 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 17 Oct 2024 11:52:09 -0700 Subject: [PATCH 04/15] Cleans up allocations for critical paths --- examples/ConsoleApp/appsettings.json | 2 +- .../FeatureGateAttribute.cs | 2 +- .../ConfigurationFeatureDefinitionProvider.cs | 41 ++++- .../FeatureManager.cs | 161 +++++++++--------- .../FeatureManagementTest.cs | 53 +++++- 5 files changed, 162 insertions(+), 97 deletions(-) diff --git a/examples/ConsoleApp/appsettings.json b/examples/ConsoleApp/appsettings.json index cb01b08e..86c556fa 100644 --- a/examples/ConsoleApp/appsettings.json +++ b/examples/ConsoleApp/appsettings.json @@ -17,4 +17,4 @@ } ] } -} \ No newline at end of file +} diff --git a/src/Microsoft.FeatureManagement.AspNetCore/FeatureGateAttribute.cs b/src/Microsoft.FeatureManagement.AspNetCore/FeatureGateAttribute.cs index caf30a28..98ba2096 100644 --- a/src/Microsoft.FeatureManagement.AspNetCore/FeatureGateAttribute.cs +++ b/src/Microsoft.FeatureManagement.AspNetCore/FeatureGateAttribute.cs @@ -95,7 +95,7 @@ public FeatureGateAttribute(RequirementType requirementType, params object[] fea public RequirementType RequirementType { get; } /// - /// Performs controller action pre-procesing to ensure that any or all of the specified features are enabled. + /// Performs controller action pre-processing to ensure that any or all of the specified features are enabled. /// /// The context of the MVC action. /// The action delegate. diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index e5d4d1b1..10d90cc2 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -24,7 +24,7 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP // provider to be marked for caching as well. private readonly IConfiguration _configuration; - private readonly ConcurrentDictionary _definitions; + private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; @@ -37,7 +37,7 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) { _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); - _definitions = new ConcurrentDictionary(); + _definitions = new ConcurrentDictionary>(); _changeSubscription = ChangeToken.OnChange( () => _configuration.GetReloadToken(), @@ -86,10 +86,13 @@ public Task GetFeatureDefinitionAsync(string featureName) _definitions.Clear(); } - return Task.FromResult( - _definitions.GetOrAdd( - featureName, - (_) => GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName))); + if (!_definitions.ContainsKey(featureName)) + { + _definitions[featureName] = + Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName)); + } + + return _definitions[featureName]; } /// @@ -98,7 +101,7 @@ public Task GetFeatureDefinitionAsync(string featureName) /// An enumerator which provides asynchronous iteration over feature definitions. // // The async key word is necessary for creating IAsyncEnumerable. - // The need to disable this warning occurs when implementaing async stream synchronously. + // The need to disable this warning occurs when implementing async stream synchronously. #pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() #pragma warning restore CS1998 @@ -121,7 +124,17 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition = _definitions.GetOrAdd(featureName, (_) => ParseMicrosoftSchemaFeatureDefinition(featureSection)); + FeatureDefinition definition; + + if (!_definitions.ContainsKey(featureName)) + { + definition = ParseMicrosoftSchemaFeatureDefinition(featureSection); + _definitions[featureName] = Task.FromResult(definition); + } + else + { + definition = _definitions[featureName].Result; + } if (definition != null) { @@ -142,7 +155,17 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition = _definitions.GetOrAdd(featureName, (_) => ParseDotnetSchemaFeatureDefinition(featureSection)); + FeatureDefinition definition; + + if (!_definitions.ContainsKey(featureName)) + { + definition = ParseDotnetSchemaFeatureDefinition(featureSection); + _definitions[featureName] = Task.FromResult(definition); + } + else + { + definition = _definitions[featureName].Result; + } if (definition != null) { diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index 2661bf88..f42e9b0b 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -23,6 +23,13 @@ namespace Microsoft.FeatureManagement /// public sealed class FeatureManager : IFeatureManager, IVariantFeatureManager { + private const string FeatureDefinitionNotFoundError = "The feature definition for the feature '{featureName}' was not found."; + private const string FeatureFilterNotFoundError = "The feature filter '{featureFilterName}' specified for feature '{featureName}' was not found."; + + private const string AlwaysOnFilterName = "AlwaysOn"; + private const string OnFilterName = "On"; + private const string FilterSuffix = "filter"; + private readonly TimeSpan ParametersCacheSlidingExpiration = TimeSpan.FromMinutes(5); private readonly TimeSpan ParametersCacheAbsoluteExpirationRelativeToNow = TimeSpan.FromDays(1); @@ -61,9 +68,8 @@ public FeatureManager( _options = options ?? new FeatureManagementOptions(); _filterMetadataCache = new ConcurrentDictionary(); _contextualFeatureFilterCache = new ConcurrentDictionary(); - _featureFilters = Enumerable.Empty(); - _sessionManagers = Enumerable.Empty(); _assignerOptions = new TargetingEvaluationOptions(); + _featureFilters = Enumerable.Empty(); } /// @@ -86,7 +92,15 @@ public IEnumerable FeatureFilters /// Thrown if it is set to null. public IEnumerable SessionManagers { - get => _sessionManagers; + get + { + if (_sessionManagers == null) + { + _sessionManagers = Enumerable.Empty(); + } + + return _sessionManagers; + } set { @@ -130,9 +144,7 @@ public TargetingEvaluationOptions AssignerOptions /// True if the feature is enabled, otherwise false. public async Task IsEnabledAsync(string feature) { - EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, CancellationToken.None); - - return evaluationEvent.Enabled; + return (await EvaluateFeature(feature, context: null, useContext: false, CancellationToken.None).ConfigureAwait(false)).Enabled; } /// @@ -143,9 +155,7 @@ public async Task IsEnabledAsync(string feature) /// True if the feature is enabled, otherwise false. public async Task IsEnabledAsync(string feature, TContext appContext) { - EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: appContext, useContext: true, CancellationToken.None); - - return evaluationEvent.Enabled; + return (await EvaluateFeature(feature, context: appContext, useContext: true, CancellationToken.None).ConfigureAwait(false)).Enabled; } /// @@ -156,9 +166,7 @@ public async Task IsEnabledAsync(string feature, TContext appCon /// True if the feature is enabled, otherwise false. public async ValueTask IsEnabledAsync(string feature, CancellationToken cancellationToken = default) { - EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, cancellationToken); - - return evaluationEvent.Enabled; + return (await EvaluateFeature(feature, context: null, useContext: false, cancellationToken).ConfigureAwait(false)).Enabled; } /// @@ -170,9 +178,7 @@ public async ValueTask IsEnabledAsync(string feature, CancellationToken ca /// True if the feature is enabled, otherwise false. public async ValueTask IsEnabledAsync(string feature, TContext appContext, CancellationToken cancellationToken = default) { - EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: appContext, useContext: true, cancellationToken); - - return evaluationEvent.Enabled; + return (await EvaluateFeature(feature, context: appContext, useContext: true, cancellationToken).ConfigureAwait(false)).Enabled; } /// @@ -211,9 +217,7 @@ public async ValueTask GetVariantAsync(string feature, CancellationToke throw new ArgumentNullException(nameof(feature)); } - EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, cancellationToken); - - return evaluationEvent.Variant; + return (await EvaluateFeature(feature, context: null, useContext: false, cancellationToken).ConfigureAwait(false)).Variant; } /// @@ -235,9 +239,7 @@ public async ValueTask GetVariantAsync(string feature, ITargetingContex throw new ArgumentNullException(nameof(context)); } - EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context, useContext: true, cancellationToken); - - return evaluationEvent.Variant; + return (await EvaluateFeature(feature, context, useContext: true, cancellationToken).ConfigureAwait(false)).Variant; } private async ValueTask EvaluateFeature(string feature, TContext context, bool useContext, CancellationToken cancellationToken) @@ -247,46 +249,40 @@ private async ValueTask EvaluateFeature(string featur FeatureDefinition = await GetFeatureDefinition(feature).ConfigureAwait(false) }; - bool telemetryEnabled = evaluationEvent.FeatureDefinition?.Telemetry?.Enabled ?? false; - - // - // Only start an activity if telemetry is enabled for the feature - using Activity activity = telemetryEnabled - ? ActivitySource.StartActivity("FeatureEvaluation") - : null; - - // - // Determine Targeting Context - TargetingContext targetingContext = null; - - if (!useContext) - { - targetingContext = await ResolveTargetingContextAsync(cancellationToken).ConfigureAwait(false); - } - else if (context is ITargetingContext targetingInfo) + if (evaluationEvent.FeatureDefinition != null) { - targetingContext = new TargetingContext - { - UserId = targetingInfo.UserId, - Groups = targetingInfo.Groups - }; - } + bool telemetryEnabled = evaluationEvent.FeatureDefinition.Telemetry?.Enabled ?? false; - evaluationEvent.TargetingContext = targetingContext; + // + // Only start an activity if telemetry is enabled for the feature + using Activity activity = telemetryEnabled + ? ActivitySource.StartActivity("FeatureEvaluation") + : null; - if (evaluationEvent.FeatureDefinition != null) - { // // Determine IsEnabled evaluationEvent.Enabled = await IsEnabledAsync(evaluationEvent.FeatureDefinition, context, useContext, cancellationToken).ConfigureAwait(false); // // Determine Variant - VariantDefinition variantDefinition = null; - if (evaluationEvent.FeatureDefinition.Variants != null && evaluationEvent.FeatureDefinition.Variants.Any()) { + VariantDefinition variantDefinition = null; + + if (!useContext) + { + evaluationEvent.TargetingContext = await ResolveTargetingContextAsync(cancellationToken).ConfigureAwait(false); + } + else if (context is ITargetingContext targetingInfo) + { + evaluationEvent.TargetingContext = new TargetingContext + { + UserId = targetingInfo.UserId, + Groups = targetingInfo.Groups + }; + } + if (evaluationEvent.FeatureDefinition.Allocation == null) { evaluationEvent.VariantAssignmentReason = evaluationEvent.Enabled @@ -307,29 +303,25 @@ private async ValueTask EvaluateFeature(string featur } else { - if (targetingContext == null) + if (evaluationEvent.TargetingContext == null) { - string message; - if (useContext) { - message = $"The context of type {context.GetType().Name} does not implement {nameof(ITargetingContext)} for variant assignment."; + Logger?.LogWarning("The context of type {contextType} does not implement {targetingContextInterface} for variant assignment.", context.GetType().Name, nameof(ITargetingContext)); } else if (TargetingContextAccessor == null) { - message = $"No instance of {nameof(ITargetingContextAccessor)} could be found for variant assignment."; + Logger?.LogWarning("No instance of {targetingContextAccessorClass} could be found for variant assignment.", nameof(ITargetingContextAccessor)); } else { - message = $"No instance of {nameof(TargetingContext)} could be found using {nameof(ITargetingContextAccessor)} for variant assignment."; + Logger?.LogWarning("No instance of {targetingContextClass} could be found using {targetingContextAccessorClass} for variant assignment.", nameof(TargetingContext), nameof(ITargetingContextAccessor)); } - - Logger?.LogWarning(message); } - if (targetingContext != null && evaluationEvent.FeatureDefinition.Allocation != null) + if (evaluationEvent.TargetingContext != null && evaluationEvent.FeatureDefinition.Allocation != null) { - variantDefinition = await AssignVariantAsync(evaluationEvent, targetingContext, cancellationToken).ConfigureAwait(false); + variantDefinition = await AssignVariantAsync(evaluationEvent, evaluationEvent.TargetingContext, cancellationToken).ConfigureAwait(false); } if (evaluationEvent.VariantAssignmentReason == VariantAssignmentReason.None) @@ -363,9 +355,12 @@ private async ValueTask EvaluateFeature(string featur } } - foreach (ISessionManager sessionManager in _sessionManagers) + if (_sessionManagers != null) { - await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.Enabled).ConfigureAwait(false); + foreach (ISessionManager sessionManager in _sessionManagers) + { + await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.Enabled).ConfigureAwait(false); + } } // Only add an activity event if telemetry is enabled for the feature and the activity is valid @@ -409,7 +404,7 @@ private void AddEvaluationActivityEvent(EvaluationEvent evaluationEvent) { if (tags.ContainsKey(kvp.Key)) { - Logger?.LogWarning($"{kvp.Key} from telemetry metadata will be ignored, as it would override an existing key."); + Logger?.LogWarning("{key} from telemetry metadata will be ignored, as it would override an existing key.", kvp.Key); continue; } @@ -427,13 +422,16 @@ private async ValueTask IsEnabledAsync(FeatureDefinition feature { Debug.Assert(featureDefinition != null); - foreach (ISessionManager sessionManager in _sessionManagers) + if (_sessionManagers != null) { - bool? readSessionResult = await sessionManager.GetAsync(featureDefinition.Name).ConfigureAwait(false); - - if (readSessionResult.HasValue) + foreach (ISessionManager sessionManager in _sessionManagers) { - return readSessionResult.Value; + bool? readSessionResult = await sessionManager.GetAsync(featureDefinition.Name).ConfigureAwait(false); + + if (readSessionResult.HasValue) + { + return readSessionResult.Value; + } } } @@ -478,8 +476,8 @@ private async ValueTask IsEnabledAsync(FeatureDefinition feature // // Handle AlwaysOn and On filters - if (string.Equals(featureFilterConfiguration.Name, "AlwaysOn", StringComparison.OrdinalIgnoreCase) || - string.Equals(featureFilterConfiguration.Name, "On", StringComparison.OrdinalIgnoreCase)) + if (string.Equals(featureFilterConfiguration.Name, AlwaysOnFilterName, StringComparison.OrdinalIgnoreCase) || + string.Equals(featureFilterConfiguration.Name, OnFilterName, StringComparison.OrdinalIgnoreCase)) { if (featureDefinition.RequirementType == RequirementType.Any) { @@ -513,14 +511,12 @@ private async ValueTask IsEnabledAsync(FeatureDefinition feature continue; } - string errorMessage = $"The feature filter '{featureFilterConfiguration.Name}' specified for feature '{featureDefinition.Name}' was not found."; - if (!_options.IgnoreMissingFeatureFilters) { - throw new FeatureManagementException(FeatureManagementError.MissingFeatureFilter, errorMessage); + throw new FeatureManagementException(FeatureManagementError.MissingFeatureFilter, string.Format(FeatureFilterNotFoundError, featureFilterConfiguration.Name, featureDefinition.Name)); } - Logger?.LogWarning(errorMessage); + Logger?.LogWarning(FeatureFilterNotFoundError, featureFilterConfiguration.Name, featureDefinition.Name); continue; } @@ -573,14 +569,15 @@ private async ValueTask GetFeatureDefinition(string feature) if (featureDefinition == null) { - string errorMessage = $"The feature definition for the feature '{feature}' was not found."; - if (!_options.IgnoreMissingFeatures) { - throw new FeatureManagementException(FeatureManagementError.MissingFeature, errorMessage); + throw new FeatureManagementException(FeatureManagementError.MissingFeature, string.Format(FeatureDefinitionNotFoundError, feature)); } - Logger?.LogDebug(errorMessage); + if (Logger?.IsEnabled(LogLevel.Debug) == true) + { + Logger.LogDebug(FeatureDefinitionNotFoundError, feature); + } } return featureDefinition; @@ -618,7 +615,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati { if (string.IsNullOrEmpty(user.Variant)) { - Logger?.LogWarning($"Missing variant name for user allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + Logger?.LogWarning("Missing variant name for user allocation in feature {featureName}", evaluationEvent.FeatureDefinition.Name); return new ValueTask((VariantDefinition)null); } @@ -644,7 +641,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati { if (string.IsNullOrEmpty(group.Variant)) { - Logger?.LogWarning($"Missing variant name for group allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + Logger?.LogWarning("Missing variant name for group allocation in feature {featureName}", evaluationEvent.FeatureDefinition.Name); return new ValueTask((VariantDefinition)null); } @@ -675,7 +672,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati { if (string.IsNullOrEmpty(percentile.Variant)) { - Logger?.LogWarning($"Missing variant name for percentile allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + Logger?.LogWarning("Missing variant name for percentile allocation in feature {featureName}", evaluationEvent.FeatureDefinition.Name); return new ValueTask((VariantDefinition)null); } @@ -788,13 +785,11 @@ private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName, Type private bool IsMatchingName(Type filterType, string filterName) { - const string filterSuffix = "filter"; - string name = ((FilterAliasAttribute)Attribute.GetCustomAttribute(filterType, typeof(FilterAliasAttribute)))?.Alias; if (name == null) { - name = filterType.Name.EndsWith(filterSuffix, StringComparison.OrdinalIgnoreCase) ? filterType.Name.Substring(0, filterType.Name.Length - filterSuffix.Length) : filterType.Name; + name = filterType.Name.EndsWith(FilterSuffix, StringComparison.OrdinalIgnoreCase) ? filterType.Name.Substring(0, filterType.Name.Length - FilterSuffix.Length) : filterType.Name; } // diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index af7d161a..26c083db 100644 --- a/tests/Tests.FeatureManagement/FeatureManagementTest.cs +++ b/tests/Tests.FeatureManagement/FeatureManagementTest.cs @@ -281,7 +281,7 @@ public async Task ThrowsForMissingFeatures() } [Fact] - public async Task ThreadsafeSnapshot() + public async Task ThreadSafeSnapshot() { IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); @@ -793,7 +793,7 @@ public async Task BindsFeatureFlagSettings() } } - public class FeatureManagementBuiltinFeatureFilterTest + public class FeatureManagementBuiltInFeatureFilterTest { [Fact] public async Task TimeWindow() @@ -1242,7 +1242,7 @@ public async Task CustomFeatureDefinitionProvider() } [Fact] - public async Task ThreadsafeSnapshot() + public async Task ThreadSafeSnapshot() { IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); @@ -1976,4 +1976,51 @@ public async Task CustomIFeatureDefinitionProvider() Assert.True(called); } } + + public class PerformanceTests + { + [Fact] + public async Task BooleanFlagManyTimes() + { + var services = new ServiceCollection(); + + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + services.AddSingleton(config) + .AddFeatureManagement(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IVariantFeatureManager featureManager = serviceProvider.GetRequiredService(); + + bool result; + + for (int i = 0; i < 100000; i++) + { + result = await featureManager.IsEnabledAsync("OnTestFeature"); + } + } + + [Fact] + public async Task MissingFlagManyTimes() + { + var services = new ServiceCollection(); + + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build(); + + services.AddSingleton(config) + .AddFeatureManagement(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + IVariantFeatureManager featureManager = serviceProvider.GetRequiredService(); + + bool result; + + for (int i = 0; i < 100000; i++) + { + result = await featureManager.IsEnabledAsync("DoesNotExist"); + } + } + } } From e428511e27bf21e52490aa8d71a521d6d79a20bd Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 17 Oct 2024 12:00:04 -0700 Subject: [PATCH 05/15] Vulnerable package updates for test and examples --- examples/ConsoleApp/ConsoleApp.csproj | 6 +++--- .../TargetingConsoleApp/TargetingConsoleApp.csproj | 6 +++--- .../Tests.FeatureManagement.AspNetCore.csproj | 12 ++++++------ .../Tests.FeatureManagement.csproj | 12 ++++++------ 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/examples/ConsoleApp/ConsoleApp.csproj b/examples/ConsoleApp/ConsoleApp.csproj index f2e84724..2482f7c0 100644 --- a/examples/ConsoleApp/ConsoleApp.csproj +++ b/examples/ConsoleApp/ConsoleApp.csproj @@ -7,9 +7,9 @@ - - - + + + diff --git a/examples/TargetingConsoleApp/TargetingConsoleApp.csproj b/examples/TargetingConsoleApp/TargetingConsoleApp.csproj index f2e84724..2482f7c0 100644 --- a/examples/TargetingConsoleApp/TargetingConsoleApp.csproj +++ b/examples/TargetingConsoleApp/TargetingConsoleApp.csproj @@ -7,9 +7,9 @@ - - - + + + diff --git a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj index 398c1588..99922e3f 100644 --- a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj +++ b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj @@ -14,29 +14,29 @@ - + - + - - + + - + - + diff --git a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj index daf20490..495cbc51 100644 --- a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj +++ b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj @@ -9,16 +9,16 @@ - + - + - - + + @@ -30,14 +30,14 @@ - + - + From 9ec9d8958693a552d6db230c5d5121036d3d336a Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 17 Oct 2024 12:04:37 -0700 Subject: [PATCH 06/15] Adds pull request template --- .github/pull_request_template.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..22f9ff10 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,7 @@ +## Why this PR? + +-motivation for making this change- + +## Visible Changes + +-changes that are visible to developers using this library- From bd7ed2242339fbcb9660618286d935c96793ea2c Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 17 Oct 2024 12:52:35 -0700 Subject: [PATCH 07/15] Fix error message format --- src/Microsoft.FeatureManagement/FeatureManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index f42e9b0b..dfe794ec 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -23,8 +23,8 @@ namespace Microsoft.FeatureManagement /// public sealed class FeatureManager : IFeatureManager, IVariantFeatureManager { - private const string FeatureDefinitionNotFoundError = "The feature definition for the feature '{featureName}' was not found."; - private const string FeatureFilterNotFoundError = "The feature filter '{featureFilterName}' specified for feature '{featureName}' was not found."; + private const string FeatureDefinitionNotFoundError = "The feature definition for the feature '{0}' was not found."; + private const string FeatureFilterNotFoundError = "The feature filter '{0}' specified for feature '{1}' was not found."; private const string AlwaysOnFilterName = "AlwaysOn"; private const string OnFilterName = "On"; From 26f215c53f4a6bdaef8fd310cf57ece01d80528a Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Tue, 29 Oct 2024 11:38:25 -0700 Subject: [PATCH 08/15] Adjusts to still use GetOrAdd to be thread safe --- .../ConfigurationFeatureDefinitionProvider.cs | 83 +++++++------------ 1 file changed, 32 insertions(+), 51 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 10d90cc2..37226988 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -27,6 +27,7 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; + public Func> schemaReadFunc; const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'."; @@ -42,6 +43,11 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) _changeSubscription = ChangeToken.OnChange( () => _configuration.GetReloadToken(), () => _stale = 1); + + schemaReadFunc = (featureName) => + { + return Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName, _configuration) ?? GetDotnetSchemaFeatureDefinition(featureName, _configuration, RootConfigurationFallbackEnabled)); + }; } /// @@ -69,6 +75,7 @@ public void Dispose() /// /// The name of the feature to retrieve the definition for. /// The feature's definition. + public Task GetFeatureDefinitionAsync(string featureName) { if (featureName == null) @@ -86,13 +93,7 @@ public Task GetFeatureDefinitionAsync(string featureName) _definitions.Clear(); } - if (!_definitions.ContainsKey(featureName)) - { - _definitions[featureName] = - Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName)); - } - - return _definitions[featureName]; + return _definitions.GetOrAdd(featureName, schemaReadFunc); } /// @@ -111,7 +112,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() _definitions.Clear(); } - IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); + IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(_configuration); foreach (IConfigurationSection featureSection in microsoftFeatureDefinitionSections) { @@ -124,17 +125,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition; - - if (!_definitions.ContainsKey(featureName)) - { - definition = ParseMicrosoftSchemaFeatureDefinition(featureSection); - _definitions[featureName] = Task.FromResult(definition); - } - else - { - definition = _definitions[featureName].Result; - } + FeatureDefinition definition = _definitions.GetOrAdd(featureName, schemaReadFunc).Result; if (definition != null) { @@ -142,7 +133,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() } } - IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); + IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(_configuration, RootConfigurationFallbackEnabled); foreach (IConfigurationSection featureSection in dotnetFeatureDefinitionSections) { @@ -155,17 +146,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition; - - if (!_definitions.ContainsKey(featureName)) - { - definition = ParseDotnetSchemaFeatureDefinition(featureSection); - _definitions[featureName] = Task.FromResult(definition); - } - else - { - definition = _definitions[featureName].Result; - } + FeatureDefinition definition = _definitions.GetOrAdd(featureName, schemaReadFunc).Result; if (definition != null) { @@ -174,11 +155,11 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() } } - private FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName) + private static FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName, IConfiguration configuration, bool rootConfigurationFallbackEnabled) { - IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); + IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(configuration, rootConfigurationFallbackEnabled); - IConfigurationSection configuration = dotnetFeatureDefinitionSections + IConfigurationSection dotnetFeatureDefinitionConfiguration = dotnetFeatureDefinitionSections .FirstOrDefault(section => string.Equals(section.Key, featureName, StringComparison.OrdinalIgnoreCase)); @@ -187,14 +168,14 @@ private FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName) return null; } - return ParseDotnetSchemaFeatureDefinition(configuration); + return ParseDotnetSchemaFeatureDefinition(dotnetFeatureDefinitionConfiguration); } - private FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName) + private static FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName, IConfiguration configuration) { - IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); + IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(configuration); - IConfigurationSection configuration = microsoftFeatureDefinitionSections + IConfigurationSection microsoftFeatureDefinitionConfiguration = microsoftFeatureDefinitionSections .LastOrDefault(section => string.Equals(section[MicrosoftFeatureManagementFields.Id], featureName, StringComparison.OrdinalIgnoreCase)); @@ -203,12 +184,12 @@ private FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName return null; } - return ParseMicrosoftSchemaFeatureDefinition(configuration); + return ParseMicrosoftSchemaFeatureDefinition(microsoftFeatureDefinitionConfiguration); } - private IEnumerable GetDotnetFeatureDefinitionSections() + private static IEnumerable GetDotnetFeatureDefinitionSections(IConfiguration configuration, bool rootConfigurationFallbackEnabled) { - IConfigurationSection featureManagementConfigurationSection = _configuration.GetSection(DotnetFeatureManagementFields.FeatureManagementSectionName); + IConfigurationSection featureManagementConfigurationSection = configuration.GetSection(DotnetFeatureManagementFields.FeatureManagementSectionName); if (featureManagementConfigurationSection.Exists()) { @@ -218,25 +199,25 @@ private IEnumerable GetDotnetFeatureDefinitionSections() // // Root configuration fallback only applies to .NET schema. // If Microsoft schema can be found, root configuration fallback will not be effective. - if (RootConfigurationFallbackEnabled && - !_configuration.GetChildren() + if (rootConfigurationFallbackEnabled && + !configuration.GetChildren() .Any(section => string.Equals(section.Key, MicrosoftFeatureManagementFields.FeatureManagementSectionName, StringComparison.OrdinalIgnoreCase))) { - return _configuration.GetChildren(); + return configuration.GetChildren(); } return Enumerable.Empty(); } - private IEnumerable GetMicrosoftFeatureDefinitionSections() + private static IEnumerable GetMicrosoftFeatureDefinitionSections(IConfiguration configuration) { - return _configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + return configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName) .GetChildren(); } - private FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) + private static FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) { /* @@ -327,7 +308,7 @@ We support }; } - private FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSection configurationSection) + private static FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSection configurationSection) { /* @@ -541,7 +522,7 @@ private FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSe }; } - private T ParseEnum(string feature, string rawValue, string fieldKeyword) + private static T ParseEnum(string feature, string rawValue, string fieldKeyword) where T : struct, Enum { Debug.Assert(!string.IsNullOrEmpty(rawValue)); @@ -556,7 +537,7 @@ private T ParseEnum(string feature, string rawValue, string fieldKeyword) return value; } - private double ParseDouble(string feature, string rawValue, string fieldKeyword) + private static double ParseDouble(string feature, string rawValue, string fieldKeyword) { Debug.Assert(!string.IsNullOrEmpty(rawValue)); @@ -570,7 +551,7 @@ private double ParseDouble(string feature, string rawValue, string fieldKeyword) return value; } - private bool ParseBool(string feature, string rawValue, string fieldKeyword) + private static bool ParseBool(string feature, string rawValue, string fieldKeyword) { Debug.Assert(!string.IsNullOrEmpty(rawValue)); From ba44f08b13e72f9076809c958eb0b98d80008b68 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Tue, 29 Oct 2024 12:05:58 -0700 Subject: [PATCH 09/15] Missed config name change --- .../ConfigurationFeatureDefinitionProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 37226988..f6de7aa7 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -163,7 +163,7 @@ private static FeatureDefinition GetDotnetSchemaFeatureDefinition(string feature .FirstOrDefault(section => string.Equals(section.Key, featureName, StringComparison.OrdinalIgnoreCase)); - if (configuration == null) + if (dotnetFeatureDefinitionConfiguration == null) { return null; } @@ -179,7 +179,7 @@ private static FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string feat .LastOrDefault(section => string.Equals(section[MicrosoftFeatureManagementFields.Id], featureName, StringComparison.OrdinalIgnoreCase)); - if (configuration == null) + if (microsoftFeatureDefinitionConfiguration == null) { return null; } From cc9d33a1eeac1ea7c5aee2fe524e83dfa1b84878 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 30 Oct 2024 10:59:53 -0700 Subject: [PATCH 10/15] Drops .net7 and adds .net9 as targets --- ...Microsoft.FeatureManagement.AspNetCore.csproj | 2 +- .../Tests.FeatureManagement.AspNetCore.csproj | 10 +++++----- .../Tests.FeatureManagement.csproj | 16 ++++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj b/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj index 81429a96..50a0e702 100644 --- a/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj +++ b/src/Microsoft.FeatureManagement.AspNetCore/Microsoft.FeatureManagement.AspNetCore.csproj @@ -12,7 +12,7 @@ - net6.0;net7.0;net8.0 + net6.0;net8.0;net9.0 true false ..\..\build\Microsoft.FeatureManagement.snk diff --git a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj index 398c1588..52f17cf2 100644 --- a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj +++ b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj @@ -1,7 +1,7 @@ - net6.0;net7.0;net8.0 + net6.0;net8.0;net9.0 false 8.0 True @@ -26,14 +26,14 @@ - - - + + + - + diff --git a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj index daf20490..f9b354fd 100644 --- a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj +++ b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj @@ -1,7 +1,7 @@ - net48;net6.0;net7.0;net8.0 + net48;net6.0;net8.0;net9.0 false 8.0 True @@ -22,19 +22,19 @@ - + - - - - + + + + - - + + From 4624acf521ef4cbb25e4615f1bffe4af5b848bb9 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 30 Oct 2024 11:11:43 -0700 Subject: [PATCH 11/15] Removes static from instance methods --- .../ConfigurationFeatureDefinitionProvider.cs | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index f6de7aa7..6922c8ad 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -27,7 +27,7 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; - public Func> schemaReadFunc; + private Func> getFeatureDefinitionFunc; const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'."; @@ -44,9 +44,9 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) () => _configuration.GetReloadToken(), () => _stale = 1); - schemaReadFunc = (featureName) => + getFeatureDefinitionFunc = (featureName) => { - return Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName, _configuration) ?? GetDotnetSchemaFeatureDefinition(featureName, _configuration, RootConfigurationFallbackEnabled)); + return Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName)); }; } @@ -93,7 +93,7 @@ public Task GetFeatureDefinitionAsync(string featureName) _definitions.Clear(); } - return _definitions.GetOrAdd(featureName, schemaReadFunc); + return _definitions.GetOrAdd(featureName, getFeatureDefinitionFunc); } /// @@ -112,7 +112,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() _definitions.Clear(); } - IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(_configuration); + IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); foreach (IConfigurationSection featureSection in microsoftFeatureDefinitionSections) { @@ -125,7 +125,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition = _definitions.GetOrAdd(featureName, schemaReadFunc).Result; + FeatureDefinition definition = _definitions.GetOrAdd(featureName, getFeatureDefinitionFunc).Result; if (definition != null) { @@ -133,7 +133,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() } } - IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(_configuration, RootConfigurationFallbackEnabled); + IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); foreach (IConfigurationSection featureSection in dotnetFeatureDefinitionSections) { @@ -146,7 +146,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition = _definitions.GetOrAdd(featureName, schemaReadFunc).Result; + FeatureDefinition definition = _definitions.GetOrAdd(featureName, getFeatureDefinitionFunc).Result; if (definition != null) { @@ -155,9 +155,9 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() } } - private static FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName, IConfiguration configuration, bool rootConfigurationFallbackEnabled) + private FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName) { - IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(configuration, rootConfigurationFallbackEnabled); + IEnumerable dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(); IConfigurationSection dotnetFeatureDefinitionConfiguration = dotnetFeatureDefinitionSections .FirstOrDefault(section => @@ -171,9 +171,9 @@ private static FeatureDefinition GetDotnetSchemaFeatureDefinition(string feature return ParseDotnetSchemaFeatureDefinition(dotnetFeatureDefinitionConfiguration); } - private static FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName, IConfiguration configuration) + private FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName) { - IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(configuration); + IEnumerable microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(); IConfigurationSection microsoftFeatureDefinitionConfiguration = microsoftFeatureDefinitionSections .LastOrDefault(section => @@ -187,9 +187,9 @@ private static FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string feat return ParseMicrosoftSchemaFeatureDefinition(microsoftFeatureDefinitionConfiguration); } - private static IEnumerable GetDotnetFeatureDefinitionSections(IConfiguration configuration, bool rootConfigurationFallbackEnabled) + private IEnumerable GetDotnetFeatureDefinitionSections() { - IConfigurationSection featureManagementConfigurationSection = configuration.GetSection(DotnetFeatureManagementFields.FeatureManagementSectionName); + IConfigurationSection featureManagementConfigurationSection = _configuration.GetSection(DotnetFeatureManagementFields.FeatureManagementSectionName); if (featureManagementConfigurationSection.Exists()) { @@ -199,25 +199,25 @@ private static IEnumerable GetDotnetFeatureDefinitionSect // // Root configuration fallback only applies to .NET schema. // If Microsoft schema can be found, root configuration fallback will not be effective. - if (rootConfigurationFallbackEnabled && - !configuration.GetChildren() + if (RootConfigurationFallbackEnabled && + !_configuration.GetChildren() .Any(section => string.Equals(section.Key, MicrosoftFeatureManagementFields.FeatureManagementSectionName, StringComparison.OrdinalIgnoreCase))) { - return configuration.GetChildren(); + return _configuration.GetChildren(); } return Enumerable.Empty(); } - private static IEnumerable GetMicrosoftFeatureDefinitionSections(IConfiguration configuration) + private IEnumerable GetMicrosoftFeatureDefinitionSections() { - return configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) + return _configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName) .GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName) .GetChildren(); } - private static FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) + private FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection) { /* @@ -308,7 +308,7 @@ We support }; } - private static FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSection configurationSection) + private FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSection configurationSection) { /* From de563eb548c0543095bcbc9584df71464d3530b1 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 30 Oct 2024 11:13:39 -0700 Subject: [PATCH 12/15] Naming of private field --- .../ConfigurationFeatureDefinitionProvider.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 6922c8ad..a5a5b8ac 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -27,7 +27,7 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; - private Func> getFeatureDefinitionFunc; + private Func> _getFeatureDefinitionFunc; const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'."; @@ -44,7 +44,7 @@ public ConfigurationFeatureDefinitionProvider(IConfiguration configuration) () => _configuration.GetReloadToken(), () => _stale = 1); - getFeatureDefinitionFunc = (featureName) => + _getFeatureDefinitionFunc = (featureName) => { return Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName)); }; @@ -75,7 +75,6 @@ public void Dispose() /// /// The name of the feature to retrieve the definition for. /// The feature's definition. - public Task GetFeatureDefinitionAsync(string featureName) { if (featureName == null) @@ -93,7 +92,7 @@ public Task GetFeatureDefinitionAsync(string featureName) _definitions.Clear(); } - return _definitions.GetOrAdd(featureName, getFeatureDefinitionFunc); + return _definitions.GetOrAdd(featureName, _getFeatureDefinitionFunc); } /// @@ -125,7 +124,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition = _definitions.GetOrAdd(featureName, getFeatureDefinitionFunc).Result; + FeatureDefinition definition = _definitions.GetOrAdd(featureName, _getFeatureDefinitionFunc).Result; if (definition != null) { @@ -146,7 +145,7 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - FeatureDefinition definition = _definitions.GetOrAdd(featureName, getFeatureDefinitionFunc).Result; + FeatureDefinition definition = _definitions.GetOrAdd(featureName, _getFeatureDefinitionFunc).Result; if (definition != null) { From 60b0e3407c1338ab06b30bf4626e9235ef3ec7df Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 30 Oct 2024 11:25:51 -0700 Subject: [PATCH 13/15] Resolves remaining deprecation warnings --- .../Tests.FeatureManagement.AspNetCore.csproj | 1 + tests/Tests.FeatureManagement/RecurrenceEvaluation.cs | 3 ++- tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj | 4 +++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj index 99922e3f..05e16daa 100644 --- a/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj +++ b/tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj @@ -23,6 +23,7 @@ + diff --git a/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs b/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs index 184f3612..c65c1abb 100644 --- a/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs +++ b/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs @@ -7,6 +7,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Xunit; namespace Tests.FeatureManagement @@ -1614,7 +1615,7 @@ public void FindWeeklyClosestStartTest() } [Fact] - public async void RecurrenceEvaluationThroughCacheTest() + public async Task RecurrenceEvaluationThroughCacheTest() { OnDemandClock mockedTimeProvider = new OnDemandClock(); diff --git a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj index 495cbc51..66f72503 100644 --- a/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj +++ b/tests/Tests.FeatureManagement/Tests.FeatureManagement.csproj @@ -18,12 +18,14 @@ + - + + From ffec9d6cfe3586d16fd9652a1a145429bfe607af Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 30 Oct 2024 12:01:46 -0700 Subject: [PATCH 14/15] Adds missing readonly --- .../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 a5a5b8ac..507bd1b1 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -27,7 +27,7 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP private readonly ConcurrentDictionary> _definitions; private IDisposable _changeSubscription; private int _stale = 0; - private Func> _getFeatureDefinitionFunc; + private readonly Func> _getFeatureDefinitionFunc; const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'."; From a077fc11ee73bbfeb0283ffc852ad3e340f3bded Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Thu, 31 Oct 2024 13:13:30 -0700 Subject: [PATCH 15/15] Adds .net 9 build to pipeline --- build/install-dotnet.ps1 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build/install-dotnet.ps1 b/build/install-dotnet.ps1 index b5fb7a5d..355cb882 100644 --- a/build/install-dotnet.ps1 +++ b/build/install-dotnet.ps1 @@ -7,4 +7,6 @@ &([scriptblock]::Create((Invoke-WebRequest -UseBasicParsing 'https://dot.net/v1/dotnet-install.ps1'))) -Channel 7.0 -&([scriptblock]::Create((Invoke-WebRequest -UseBasicParsing 'https://dot.net/v1/dotnet-install.ps1'))) -Channel 8.0 \ No newline at end of file +&([scriptblock]::Create((Invoke-WebRequest -UseBasicParsing 'https://dot.net/v1/dotnet-install.ps1'))) -Channel 8.0 + +&([scriptblock]::Create((Invoke-WebRequest -UseBasicParsing 'https://dot.net/v1/dotnet-install.ps1'))) -Channel 9.0