From 1c2c4109e8fa882c2fcdd7d5fdb86133d1adb85d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=B3genes=20Ferreira?= Date: Fri, 13 May 2022 14:58:41 +0200 Subject: [PATCH 1/3] Implement a default/global feature definition --- ...yNameFeatureFilterConfigurationComparer.cs | 24 ++++++ .../ConfigurationFeatureDefinitionProvider.cs | 44 +++++++++-- .../FeatureManagement.cs | 24 +++++- .../appsettings-with-default.json | 75 +++++++++++++++++++ 4 files changed, 160 insertions(+), 7 deletions(-) create mode 100644 src/Microsoft.FeatureManagement/Comparers/CompareByNameFeatureFilterConfigurationComparer.cs create mode 100644 tests/Tests.FeatureManagement/appsettings-with-default.json diff --git a/src/Microsoft.FeatureManagement/Comparers/CompareByNameFeatureFilterConfigurationComparer.cs b/src/Microsoft.FeatureManagement/Comparers/CompareByNameFeatureFilterConfigurationComparer.cs new file mode 100644 index 00000000..ca394748 --- /dev/null +++ b/src/Microsoft.FeatureManagement/Comparers/CompareByNameFeatureFilterConfigurationComparer.cs @@ -0,0 +1,24 @@ +using System.Collections.Generic; + +namespace Microsoft.FeatureManagement.Comparers +{ + /// + /// Compares two FeatureFilterConfiguration using only the name + /// + public class CompareByNameFeatureFilterConfigurationComparer : IEqualityComparer + { + public bool Equals(FeatureFilterConfiguration x, FeatureFilterConfiguration y) + { + if (ReferenceEquals(x, y)) return true; + if (ReferenceEquals(x, null)) return false; + if (ReferenceEquals(y, null)) return false; + if (x.GetType() != y.GetType()) return false; + return x.Name == y.Name; + } + + public int GetHashCode(FeatureFilterConfiguration obj) + { + return (obj.Name != null ? obj.Name.GetHashCode() : 0); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs index 8763b850..7ff118c8 100644 --- a/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs +++ b/src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // + using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Primitives; using System; @@ -9,6 +10,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.FeatureManagement.Comparers; namespace Microsoft.FeatureManagement { @@ -18,6 +20,7 @@ namespace Microsoft.FeatureManagement sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionProvider, IDisposable { private const string FeatureFiltersSectionName = "EnabledFor"; + private const string DefaultDefinitionSectionName = "Default"; private readonly IConfiguration _configuration; private readonly ConcurrentDictionary _definitions; private IDisposable _changeSubscription; @@ -56,6 +59,11 @@ public Task GetFeatureDefinitionAsync(string featureName) // Query by feature name FeatureDefinition definition = _definitions.GetOrAdd(featureName, (name) => ReadFeatureDefinition(name)); + FeatureDefinition defaultDefinition = + _definitions.GetOrAdd(DefaultDefinitionSectionName, (name) => ReadFeatureDefinition(name)); + + definition = MergeDefinitions(definition, defaultDefinition); + return Task.FromResult(definition); } @@ -77,14 +85,14 @@ public async IAsyncEnumerable GetAllFeatureDefinitionsAsync() { // // Underlying IConfigurationSection data is dynamic so latest feature definitions are returned - yield return _definitions.GetOrAdd(featureSection.Key, (_) => ReadFeatureDefinition(featureSection)); + yield return _definitions.GetOrAdd(featureSection.Key, (_) => ReadFeatureDefinition(featureSection)); } } private FeatureDefinition ReadFeatureDefinition(string featureName) { IConfigurationSection configuration = GetFeatureDefinitionSections() - .FirstOrDefault(section => section.Key.Equals(featureName, StringComparison.OrdinalIgnoreCase)); + .FirstOrDefault(section => section.Key.Equals(featureName, StringComparison.OrdinalIgnoreCase)); if (configuration == null) { @@ -149,14 +157,16 @@ We support } else { - IEnumerable filterSections = configurationSection.GetSection(FeatureFiltersSectionName).GetChildren(); + IEnumerable filterSections = + configurationSection.GetSection(FeatureFiltersSectionName).GetChildren(); foreach (IConfigurationSection section in filterSections) { // // Arrays in json such as "myKey": [ "some", "values" ] // Are accessed through the configuration system by using the array index as the property name, e.g. "myKey": { "0": "some", "1": "values" } - if (int.TryParse(section.Key, out int i) && !string.IsNullOrEmpty(section[nameof(FeatureFilterConfiguration.Name)])) + if (int.TryParse(section.Key, out int i) && + !string.IsNullOrEmpty(section[nameof(FeatureFilterConfiguration.Name)])) { enabledFor.Add(new FeatureFilterConfiguration() { @@ -178,7 +188,8 @@ private IEnumerable GetFeatureDefinitionSections() { const string FeatureManagementSectionName = "FeatureManagement"; - if (_configuration.GetChildren().Any(s => s.Key.Equals(FeatureManagementSectionName, StringComparison.OrdinalIgnoreCase))) + if (_configuration.GetChildren().Any(s => + s.Key.Equals(FeatureManagementSectionName, StringComparison.OrdinalIgnoreCase))) { // // Look for feature definitions under the "FeatureManagement" section @@ -189,5 +200,26 @@ private IEnumerable GetFeatureDefinitionSections() return _configuration.GetChildren(); } } + + private FeatureDefinition MergeDefinitions(FeatureDefinition targetDefinition, + FeatureDefinition otherDefinition) + { + if (targetDefinition is null) + { + return null; + } + + if (otherDefinition is null) + { + return targetDefinition; + } + + return new FeatureDefinition + { + Name = targetDefinition.Name, + EnabledFor = targetDefinition.EnabledFor.Union(otherDefinition.EnabledFor, + new CompareByNameFeatureFilterConfigurationComparer()) + }; + } } -} +} \ No newline at end of file diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index af67fe8e..28d0ebcb 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -415,7 +415,8 @@ public async Task UsesContext() serviceCollection.AddSingleton(config) .AddFeatureManagement() - .AddFeatureFilter(); + .AddFeatureFilter() + .AddFeatureFilter(); ServiceProvider provider = serviceCollection.BuildServiceProvider(); @@ -669,6 +670,27 @@ public async Task ThreadsafeSnapshot() } } + [Fact] + public async Task DefaultFeatureDefinition() + { + IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings-with-default.json").Build(); + + var services = new ServiceCollection(); + + services + .AddSingleton(config) + .AddFeatureManagement() + .AddFeatureFilter(); + + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + var featureDefinitionProvider = serviceProvider.GetRequiredService(); + + var definition = await featureDefinitionProvider.GetFeatureDefinitionAsync(nameof(Features.OffTestFeature)); + + Assert.Collection(definition.EnabledFor, configuration => configuration.Name.Equals("Test")); + } + private static void DisableEndpointRouting(MvcOptions options) { #if NET5_0 || NETCOREAPP3_1 diff --git a/tests/Tests.FeatureManagement/appsettings-with-default.json b/tests/Tests.FeatureManagement/appsettings-with-default.json new file mode 100644 index 00000000..6f239909 --- /dev/null +++ b/tests/Tests.FeatureManagement/appsettings-with-default.json @@ -0,0 +1,75 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Warning" + } + }, + "AllowedHosts": "*", + + "FeatureManagement": { + "Default": { + "EnabledFor": [ + { + "name": "Test" + } + ] + }, + "OnTestFeature": true, + "OffTestFeature": false, + "TargetingTestFeature": { + "EnabledFor": [ + { + "Name": "Targeting", + "Parameters": { + "Audience": { + "Users": [ + "Jeff", + "Alicia" + ], + "Groups": [ + { + "Name": "Ring0", + "RolloutPercentage": 100 + }, + { + "Name": "Ring1", + "RolloutPercentage": 50 + } + ], + "DefaultRolloutPercentage": 20 + } + } + } + ] + }, + "ConditionalFeature": { + "EnabledFor": [ + { + "Name": "Test", + "Parameters": { + "P1": "V1" + } + } + ] + }, + "ConditionalFeature2": { + "EnabledFor": [ + { + "Name": "Test" + } + ] + }, + "ContextualFeature": { + "EnabledFor": [ + { + "Name": "ContextualTest", + "Parameters": { + "AllowedAccounts": [ + "abc" + ] + } + } + ] + } + } +} From dd2487356ed6672a3ba8418490319aac83ce4ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=B3genes=20Ferreira?= Date: Fri, 13 May 2022 15:02:15 +0200 Subject: [PATCH 2/3] remove unnecessary setup filter injection --- tests/Tests.FeatureManagement/FeatureManagement.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 28d0ebcb..076571fe 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -415,8 +415,7 @@ public async Task UsesContext() serviceCollection.AddSingleton(config) .AddFeatureManagement() - .AddFeatureFilter() - .AddFeatureFilter(); + .AddFeatureFilter(); ServiceProvider provider = serviceCollection.BuildServiceProvider(); From 87bd3a38328c0e9f3e4a3df90b21214b04e914d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=B3genes=20Ferreira?= Date: Fri, 13 May 2022 15:03:07 +0200 Subject: [PATCH 3/3] cleanup appsettings-with-default.json --- .../appsettings-with-default.json | 58 +------------------ 1 file changed, 1 insertion(+), 57 deletions(-) diff --git a/tests/Tests.FeatureManagement/appsettings-with-default.json b/tests/Tests.FeatureManagement/appsettings-with-default.json index 6f239909..d17e5830 100644 --- a/tests/Tests.FeatureManagement/appsettings-with-default.json +++ b/tests/Tests.FeatureManagement/appsettings-with-default.json @@ -14,62 +14,6 @@ } ] }, - "OnTestFeature": true, - "OffTestFeature": false, - "TargetingTestFeature": { - "EnabledFor": [ - { - "Name": "Targeting", - "Parameters": { - "Audience": { - "Users": [ - "Jeff", - "Alicia" - ], - "Groups": [ - { - "Name": "Ring0", - "RolloutPercentage": 100 - }, - { - "Name": "Ring1", - "RolloutPercentage": 50 - } - ], - "DefaultRolloutPercentage": 20 - } - } - } - ] - }, - "ConditionalFeature": { - "EnabledFor": [ - { - "Name": "Test", - "Parameters": { - "P1": "V1" - } - } - ] - }, - "ConditionalFeature2": { - "EnabledFor": [ - { - "Name": "Test" - } - ] - }, - "ContextualFeature": { - "EnabledFor": [ - { - "Name": "ContextualTest", - "Parameters": { - "AllowedAccounts": [ - "abc" - ] - } - } - ] - } + "OffTestFeature": false } }