From ec5f02d93a622153ee06f95fbde1a02ba5c1e0aa Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Sun, 7 Mar 2021 07:56:09 -0800 Subject: [PATCH 1/4] Support prefix filtering and trimming from feature flags --- .../AzureAppConfigurationOptions.cs | 36 +- .../FeatureManagement/FeatureFlagOptions.cs | 6 + .../FeatureManagementKeyValueAdapter.cs | 18 +- .../FeatureManagementTests.cs | 434 ++++++++++++++++++ 4 files changed, 488 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index 04a4026d..8fed076e 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -4,6 +4,7 @@ using Azure.Core; using Azure.Data.AppConfiguration; using Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault; +using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions; using Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement; using Microsoft.Extensions.Configuration.AzureAppConfiguration.Models; using System; @@ -34,6 +35,7 @@ public class AzureAppConfigurationOptions private IConfigurationRefresher _refresher = new AzureAppConfigurationRefresher(); private SortedSet _keyPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.InvariantCultureIgnoreCase))); + private SortedSet _featureFlagPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.InvariantCultureIgnoreCase))); /// /// The connection string to use to connect to Azure App Configuration. @@ -154,25 +156,34 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action c string.Format(ErrorMessages.CacheExpirationTimeTooShort, MinimumFeatureFlagsCacheExpirationInterval.TotalMilliseconds)); } - if (!(_kvSelectors.Any(selector => selector.KeyFilter.StartsWith(FeatureManagementConstants.FeatureFlagMarker) && selector.LabelFilter.Equals(options.Label)))) + string fullPrefix = FeatureManagementConstants.FeatureFlagMarker + options.Prefix; + + if (!(_kvSelectors.Any(selector => selector.KeyFilter.StartsWith(fullPrefix) && selector.LabelFilter.NormalizeNull() == options.Label.NormalizeNull()))) { - Select(FeatureManagementConstants.FeatureFlagMarker + "*", options.Label); + Select(fullPrefix + "*", options.Label); } if (!_adapters.Any(a => a is FeatureManagementKeyValueAdapter)) { - _adapters.Add(new FeatureManagementKeyValueAdapter()); + _adapters.Add(new FeatureManagementKeyValueAdapter(_featureFlagPrefixes)); } - if (!_multiKeyWatchers.Any(kw => kw.Key.Equals(FeatureManagementConstants.FeatureFlagMarker))) + var multiKeyWatcher = _multiKeyWatchers.FirstOrDefault(kw => kw.Key.Equals(fullPrefix) && kw.Label.NormalizeNull() == options.Label.NormalizeNull()); + + if (multiKeyWatcher == null) { _multiKeyWatchers.Add(new KeyValueWatcher { - Key = FeatureManagementConstants.FeatureFlagMarker, + Key = fullPrefix, Label = options.Label, CacheExpirationInterval = options.CacheExpirationInterval }); } + else + { + // If UseFeatureFlags is called multiple times for the same prefix+label, last cache expiration time wins + multiKeyWatcher.CacheExpirationInterval = options.CacheExpirationInterval; + } return this; } @@ -245,6 +256,21 @@ public AzureAppConfigurationOptions TrimKeyPrefix(string prefix) return this; } + /// + /// Trims the provided prefix from the keys of all feature flags retrieved from Azure App Configuration. + /// + /// The prefix to be trimmed from all feature flags retrieved from Azure App Configuration. + public AzureAppConfigurationOptions TrimFeatureFlagPrefix(string prefix) + { + if (string.IsNullOrEmpty(prefix)) + { + throw new ArgumentNullException(nameof(prefix)); + } + + _featureFlagPrefixes.Add(prefix); + return this; + } + /// /// Configure the client used to communicate with Azure App Configuration. /// diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs index b50ea61e..fa15f495 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs @@ -19,5 +19,11 @@ public class FeatureFlagOptions /// The time after which the cached values of the feature flags expire. Must be greater than or equal to 1 second. /// public TimeSpan CacheExpirationInterval { get; set; } = AzureAppConfigurationOptions.DefaultFeatureFlagsCacheExpirationInterval; + + /// + /// Selectively load only those feature flags which begin with this prefix. + /// The characters asterisk (*), comma (,) and backslash (\) are reserved and must be escaped using a backslash (\). + /// + public string Prefix { get; set; } = string.Empty; } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs index 230cb0e0..e8aaf181 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs @@ -13,6 +13,13 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManage { internal class FeatureManagementKeyValueAdapter : IKeyValueAdapter { + private readonly IEnumerable _featureFlagPrefixes; + + public FeatureManagementKeyValueAdapter(IEnumerable featureFlagPrefixes) + { + _featureFlagPrefixes = featureFlagPrefixes ?? throw new ArgumentNullException(nameof(featureFlagPrefixes)); + } + public Task>> ProcessKeyValue(ConfigurationSetting setting, CancellationToken cancellationToken) { FeatureFlag featureFlag; @@ -25,8 +32,17 @@ public Task>> ProcessKeyValue(Configura throw new FormatException(setting.Key, e); } + foreach (string prefix in _featureFlagPrefixes) + { + if (featureFlag.Id.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + featureFlag.Id = featureFlag.Id.Substring(prefix.Length); + break; + } + } + var keyValues = new List>(); - + if (featureFlag.Enabled) { //if (featureFlag.Conditions?.ClientFilters == null) diff --git a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs index d0a2075e..edeabffb 100644 --- a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs +++ b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs @@ -81,6 +81,101 @@ public class FeatureManagementTests contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")); + List _featureFlagCollection = new List + { + ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "App1_Feature1", + value: @" + { + ""id"": ""App1_Feature1"", + ""enabled"": true, + ""conditions"": { + ""client_filters"": [] + } + } + ", + label: "App1_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")), + + ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "App1_Feature2", + value: @" + { + ""id"": ""App1_Feature2"", + ""enabled"": false, + ""conditions"": { + ""client_filters"": [] + } + } + ", + label: "App1_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")), + + ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "Feature1", + value: @" + { + ""id"": ""Feature1"", + ""enabled"": false, + ""conditions"": { + ""client_filters"": [] + } + } + ", + label: "App1_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")), + + ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "App2_Feature1", + value: @" + { + ""id"": ""App2_Feature1"", + ""enabled"": false, + ""conditions"": { + ""client_filters"": [] + } + } + ", + label: "App2_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")), + + ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "App2_Feature2", + value: @" + { + ""id"": ""App2_Feature2"", + ""enabled"": true, + ""conditions"": { + ""client_filters"": [] + } + } + ", + label: "App2_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")), + + ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "Feature1", + value: @" + { + ""id"": ""Feature1"", + ""enabled"": true, + ""conditions"": { + ""client_filters"": [] + } + } + ", + label: "App2_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")), + }; + + ConfigurationSetting FirstFeatureFlag => _featureFlagCollection.First(); + [Fact] public void UsesFeatureFlags() { @@ -332,5 +427,344 @@ public void UsesEtagForFeatureFlagRefresh() refresher.TryRefreshAsync().Wait(); mockClient.Verify(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny()), Times.Exactly(2)); } + + [Fact] + public void PrefixFilterFeatureFlags() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix = "App1"; + var label = "App1_Label"; + var cacheExpiration = TimeSpan.FromSeconds(1); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(_featureFlagCollection.Where(s => s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix) && s.Label == label).ToList())); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = testClient; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Label = label; + ff.Prefix = prefix; + }); + }) + .Build(); + + Assert.Equal("True", config["FeatureManagement:App1_Feature1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + + // Verify that the feature flag that did not start with the specified prefix was not loaded + Assert.Null(config["FeatureManagement:Feature1"]); + + // Verify that the feature flag that did not match the specified label was not loaded + Assert.Null(config["FeatureManagement:App2_Feature1"]); + Assert.Null(config["FeatureManagement:App2_Feature2"]); + } + + [Fact] + public void MultipleCallsToUseFeatureFlags() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix1 = "App1"; + var prefix2 = "App2"; + var label1 = "App1_Label"; + var label2 = "App2_Label"; + var cacheExpiration = TimeSpan.FromSeconds(1); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(() => + { + return new MockAsyncPageable(_featureFlagCollection.Where(s => + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix1) && s.Label == label1) || + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix2) && s.Label == label2)).ToList()); + }); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = testClient; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Label = label1; + ff.Prefix = prefix1; + }); + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Label = label2; + ff.Prefix = prefix2; + }); + }) + .Build(); + + Assert.Equal("True", config["FeatureManagement:App1_Feature1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + Assert.Equal("False", config["FeatureManagement:App2_Feature1"]); + Assert.Equal("True", config["FeatureManagement:App2_Feature2"]); + + // Verify that the feature flag that did not start with the specified prefix was not loaded + Assert.Null(config["FeatureManagement:Feature1"]); + } + + [Fact] + public void DifferentCacheExpirationsForMultipleFeatureFlagRegistrations() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix1 = "App1"; + var prefix2 = "App2"; + var label1 = "App1_Label"; + var label2 = "App2_Label"; + var cacheExpiration1 = TimeSpan.FromSeconds(1); + var cacheExpiration2 = TimeSpan.FromSeconds(60); + IConfigurationRefresher refresher = null; + var featureFlagCollection = new List(_featureFlagCollection); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(() => + { + return new MockAsyncPageable(featureFlagCollection.Where(s => + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix1) && s.Label == label1) || + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix2) && s.Label == label2 && s.Key != FeatureManagementConstants.FeatureFlagMarker + "App2_Feature3")).ToList()); + }); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration1; + ff.Label = label1; + ff.Prefix = prefix1; + }); + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration2; + ff.Label = label2; + ff.Prefix = prefix2; + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal("True", config["FeatureManagement:App1_Feature1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + Assert.Equal("False", config["FeatureManagement:App2_Feature1"]); + Assert.Equal("True", config["FeatureManagement:App2_Feature2"]); + + // update the value of App1_Feature1 feature flag with label1 + featureFlagCollection[0] = ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "App1_Feature1", + value: @" + { + ""id"": ""App1_Feature1"", + ""enabled"": true, + ""conditions"": { + ""client_filters"": [ + { + ""name"": ""Browser"", + ""parameters"": { + ""AllowedBrowsers"": [ ""Chrome"", ""Edge"" ] + } + } + ] + } + } + ", + label: "App1_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1" + "f")); + + // add new feature flag with label2 + featureFlagCollection.Add(ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "App2_Feature3", + value: @" + { + ""id"": ""App2_Feature3"", + ""enabled"": true, + ""conditions"": { + ""client_filters"": [] + } + } + ", + label: "App2_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1" + "f"))); + + // Sleep to let the cache for feature flag with label1 expire + Thread.Sleep(cacheExpiration1); + refresher.RefreshAsync().Wait(); + + Assert.Equal("Browser", config["FeatureManagement:App1_Feature1:EnabledFor:0:Name"]); + Assert.Equal("Chrome", config["FeatureManagement:App1_Feature1:EnabledFor:0:Parameters:AllowedBrowsers:0"]); + Assert.Equal("Edge", config["FeatureManagement:App1_Feature1:EnabledFor:0:Parameters:AllowedBrowsers:1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + Assert.Equal("False", config["FeatureManagement:App2_Feature1"]); + Assert.Equal("True", config["FeatureManagement:App2_Feature2"]); + + // even though App2_Feature3 feature flag has been added, its value should not be loaded in config because label2 cache has not expired + Assert.Null(config["FeatureManagement:App2_Feature3"]); + } + + [Fact] + public void DifferentCacheExpirationsForSameFeatureFlagRegistrations() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var cacheExpiration1 = TimeSpan.FromSeconds(1); + var cacheExpiration2 = TimeSpan.FromSeconds(60); + IConfigurationRefresher refresher = null; + var featureFlagCollection = new List(_featureFlagCollection); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(featureFlagCollection)); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration1; + }); + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration2; + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal("True", config["FeatureManagement:App1_Feature1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + Assert.Equal("False", config["FeatureManagement:App2_Feature1"]); + Assert.Equal("True", config["FeatureManagement:App2_Feature2"]); + Assert.Equal("True", config["FeatureManagement:Feature1"]); + + // update the value of App1_Feature1 feature flag with label1 + featureFlagCollection[0] = ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "App1_Feature1", + value: @" + { + ""id"": ""App1_Feature1"", + ""enabled"": true, + ""conditions"": { + ""client_filters"": [ + { + ""name"": ""Browser"", + ""parameters"": { + ""AllowedBrowsers"": [ ""Chrome"", ""Edge"" ] + } + } + ] + } + } + ", + label: "App1_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1" + "f")); + + Thread.Sleep(cacheExpiration1); + refresher.RefreshAsync().Wait(); + + // The cache expiration time for feature flags was overwritten by second call to UseFeatureFlags. + // Sleeping for cacheExpiration1 time should not update feature flags. + Assert.Equal("True", config["FeatureManagement:App1_Feature1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + Assert.Equal("False", config["FeatureManagement:App2_Feature1"]); + Assert.Equal("True", config["FeatureManagement:App2_Feature2"]); + Assert.Equal("True", config["FeatureManagement:Feature1"]); + } + + [Fact] + public void TrimFeatureFlags() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var label1 = "App1_Label"; + var cacheExpiration = TimeSpan.FromSeconds(1); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(new MockAsyncPageable(_featureFlagCollection.Where(s => s.Label == label1).ToList())); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = testClient; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Label = label1; + }); + options.TrimFeatureFlagPrefix("App1_"); + }) + .Build(); + + // Verify that prefix was trimmed from feature flag ID and in case of same ID after trimming, last feature flag value was present in config + Assert.Equal("False", config["FeatureManagement:Feature1"]); + Assert.Equal("False", config["FeatureManagement:Feature2"]); + } + + [Fact] + public void MultipleTrimFeatureFlags() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix1 = "App1"; + var prefix2 = "App2"; + var label1 = "App1_Label"; + var label2 = "App2_Label"; + var cacheExpiration = TimeSpan.FromSeconds(1); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(() => + { + return new MockAsyncPageable(_featureFlagCollection.Where(s => + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix1) && s.Label == label1) || + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix2) && s.Label == label2)).ToList()); + }); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = testClient; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Label = label1; + ff.Prefix = prefix1; + }); + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Label = label2; + ff.Prefix = prefix2; + }); + options.TrimFeatureFlagPrefix("App1_"); + options.TrimFeatureFlagPrefix("App2_"); + + }) + .Build(); + + // Verify that prefix was trimmed from feature flag ID and in case of same ID after trimming, last feature flag value was present in config + Assert.Equal("False", config["FeatureManagement:Feature1"]); + Assert.Equal("True", config["FeatureManagement:Feature2"]); + } } } From 6a95b957b20bd82ad748486b1fcd33761454d52f Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Wed, 17 Mar 2021 19:09:24 -0700 Subject: [PATCH 2/4] Updating design to introduce Select API for feature flags --- .../AzureAppConfigurationOptions.cs | 83 +++--- .../AzureAppConfigurationProvider.cs | 24 +- .../ConfigurationClientExtensions.cs | 14 +- .../FeatureManagement/FeatureFlagOptions.cs | 79 +++++- .../FeatureManagementTests.cs | 260 ++++++++++++++++-- 5 files changed, 379 insertions(+), 81 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index 8fed076e..eabce37d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -34,8 +34,8 @@ public class AzureAppConfigurationOptions private List _kvSelectors = new List(); private IConfigurationRefresher _refresher = new AzureAppConfigurationRefresher(); - private SortedSet _keyPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.InvariantCultureIgnoreCase))); - private SortedSet _featureFlagPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.InvariantCultureIgnoreCase))); + private SortedSet _keyPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.OrdinalIgnoreCase))); + private List _featureFlagPrefixes = new List(); /// /// The connection string to use to connect to Azure App Configuration. @@ -156,33 +156,63 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action c string.Format(ErrorMessages.CacheExpirationTimeTooShort, MinimumFeatureFlagsCacheExpirationInterval.TotalMilliseconds)); } - string fullPrefix = FeatureManagementConstants.FeatureFlagMarker + options.Prefix; - - if (!(_kvSelectors.Any(selector => selector.KeyFilter.StartsWith(fullPrefix) && selector.LabelFilter.NormalizeNull() == options.Label.NormalizeNull()))) + if (options.FeatureFlagSelectors.Count() != 0 && options.Label != null) + { + throw new InvalidOperationException($"Please select feature flags by either the {nameof(options.Select)} method or by setting the {nameof(options.Label)}, not both."); + } + + if (options.FeatureFlagSelectors.Count() == 0 && options.Label == null) { - Select(fullPrefix + "*", options.Label); + // No Select clause and the Label property is not set + options.FeatureFlagSelectors.Add(new KeyValueSelector + { + KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*", + LabelFilter = LabelFilter.Null + }); } + else if (options.FeatureFlagSelectors.Count() == 0 && options.Label != null) + { + // No Select clause but the Label property is set + options.FeatureFlagSelectors.Add(new KeyValueSelector + { + KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*", + LabelFilter = options.Label + }); + } + + _featureFlagPrefixes.AddRange(options.FeatureFlagPrefixes); if (!_adapters.Any(a => a is FeatureManagementKeyValueAdapter)) { _adapters.Add(new FeatureManagementKeyValueAdapter(_featureFlagPrefixes)); } - var multiKeyWatcher = _multiKeyWatchers.FirstOrDefault(kw => kw.Key.Equals(fullPrefix) && kw.Label.NormalizeNull() == options.Label.NormalizeNull()); - - if (multiKeyWatcher == null) + foreach (var featureFlagSelector in options.FeatureFlagSelectors) { - _multiKeyWatchers.Add(new KeyValueWatcher + var featureFlagFilter = featureFlagSelector.KeyFilter; + var labelFilter = featureFlagSelector.LabelFilter; + + if (!_kvSelectors.Any(selector => selector.KeyFilter == featureFlagFilter && selector.LabelFilter == labelFilter)) { - Key = fullPrefix, - Label = options.Label, - CacheExpirationInterval = options.CacheExpirationInterval - }); - } - else - { - // If UseFeatureFlags is called multiple times for the same prefix+label, last cache expiration time wins - multiKeyWatcher.CacheExpirationInterval = options.CacheExpirationInterval; + Select(featureFlagFilter, labelFilter); + } + + var multiKeyWatcher = _multiKeyWatchers.FirstOrDefault(kw => kw.Key.Equals(featureFlagFilter) && kw.Label.NormalizeNull() == labelFilter.NormalizeNull()); + + if (multiKeyWatcher == null) + { + _multiKeyWatchers.Add(new KeyValueWatcher + { + Key = featureFlagFilter, + Label = labelFilter, + CacheExpirationInterval = options.CacheExpirationInterval + }); + } + else + { + // If UseFeatureFlags is called multiple times for the same key and label filters, last cache expiration time wins + multiKeyWatcher.CacheExpirationInterval = options.CacheExpirationInterval; + } } return this; @@ -256,21 +286,6 @@ public AzureAppConfigurationOptions TrimKeyPrefix(string prefix) return this; } - /// - /// Trims the provided prefix from the keys of all feature flags retrieved from Azure App Configuration. - /// - /// The prefix to be trimmed from all feature flags retrieved from Azure App Configuration. - public AzureAppConfigurationOptions TrimFeatureFlagPrefix(string prefix) - { - if (string.IsNullOrEmpty(prefix)) - { - throw new ArgumentNullException(nameof(prefix)); - } - - _featureFlagPrefixes.Add(prefix); - return this; - } - /// /// Configure the client used to communicate with Azure App Configuration. /// diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index e95b07e1..83ec5e50 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -14,6 +14,7 @@ using System.Net; using System.Security; using System.Text.Json; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; @@ -462,10 +463,27 @@ private async Task RefreshKeyValueCollections() try { - IEnumerable currentKeyValues = _applicationSettings.Values.Where(kv => + IEnumerable currentKeyValues; + + // Check if the user-provided feature flag filter ends with an unescaped * character: + // (? + { + return kv.Key.StartsWith(changeWatcher.Key.Substring(0, changeWatcher.Key.Length - 1)) && kv.Label == changeWatcher.Label.NormalizeNull(); + }); + } + else + { + currentKeyValues = _applicationSettings.Values.Where(kv => + { + return kv.Key.Equals(changeWatcher.Key) && kv.Label == changeWatcher.Label.NormalizeNull(); + }); + } IEnumerable keyValueChanges = await _client.GetKeyValueChangeCollection(currentKeyValues, new GetKeyValueChangeCollectionOptions { diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs index bfa2fb87..9dcc5e81 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs @@ -77,21 +77,11 @@ public static async Task> GetKeyValueChangeCollectio options.Prefix = string.Empty; } - if (options.Prefix.Contains('*')) - { - throw new ArgumentException("The prefix cannot contain '*'", $"{nameof(options)}.{nameof(options.Prefix)}"); - } - if (keyValues.Any(k => string.IsNullOrEmpty(k.Key))) { throw new ArgumentNullException($"{nameof(keyValues)}[].{nameof(ConfigurationSetting.Key)}"); } - if (!string.IsNullOrEmpty(options.Prefix) && keyValues.Any(k => !k.Key.StartsWith(options.Prefix))) - { - throw new ArgumentException("All key-values registered for refresh must start with the provided prefix.", $"{nameof(keyValues)}[].{nameof(ConfigurationSetting.Key)}"); - } - if (keyValues.Any(k => !string.Equals(k.Label.NormalizeNull(), options.Label.NormalizeNull()))) { throw new ArgumentException("All key-values registered for refresh must use the same label.", $"{nameof(keyValues)}[].{nameof(ConfigurationSetting.Label)}"); @@ -105,7 +95,7 @@ public static async Task> GetKeyValueChangeCollectio var hasKeyValueCollectionChanged = false; var selector = new SettingSelector { - KeyFilter = options.Prefix + "*", + KeyFilter = options.Prefix, LabelFilter = string.IsNullOrEmpty(options.Label) ? LabelFilter.Null : options.Label, Fields = SettingFields.ETag | SettingFields.Key }; @@ -142,7 +132,7 @@ await TracingUtils.CallWithRequestTracing(options.RequestTracingEnabled, Request { selector = new SettingSelector { - KeyFilter = options.Prefix + "*", + KeyFilter = options.Prefix, LabelFilter = string.IsNullOrEmpty(options.Label) ? LabelFilter.Null : options.Label }; diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs index fa15f495..ed6f069e 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs @@ -1,7 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // +using Microsoft.Extensions.Configuration.AzureAppConfiguration.Models; using System; +using System.Collections.Generic; +using System.Linq; namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement { @@ -10,10 +13,22 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManage /// public class FeatureFlagOptions { + private SortedSet _featureFlagPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.OrdinalIgnoreCase))); + + /// + /// A collection of key prefixes to be trimmed. + /// + internal IEnumerable FeatureFlagPrefixes => _featureFlagPrefixes; + + /// + /// A collection of . + /// + internal List FeatureFlagSelectors = new List(); + /// /// The label that feature flags will be selected from. /// - public string Label { get; set; } = LabelFilter.Null; + public string Label { get; set; } /// /// The time after which the cached values of the feature flags expire. Must be greater than or equal to 1 second. @@ -21,9 +36,67 @@ public class FeatureFlagOptions public TimeSpan CacheExpirationInterval { get; set; } = AzureAppConfigurationOptions.DefaultFeatureFlagsCacheExpirationInterval; /// - /// Selectively load only those feature flags which begin with this prefix. + /// Specify what feature flags to include in the configuration provider. + /// can be called multiple times to include multiple sets of feature flags. + /// + /// + /// The filter to apply to feature flag names when querying Azure App Configuration for feature flags. + /// For example, you can select all feature flags that begin with "MyApp" by setting the featureflagFilter to "MyApp*". /// The characters asterisk (*), comma (,) and backslash (\) are reserved and must be escaped using a backslash (\). + /// Built-in feature flag filter options: . + /// + /// + /// The label filter to apply when querying Azure App Configuration for feature flags. By default the null label will be used. Built-in label filter options: + /// The characters asterisk (*) and comma (,) are not supported. Backslash (\) character is reserved and must be escaped using another backslash (\). + /// + public FeatureFlagOptions Select(string featureFlagFilter, string labelFilter = LabelFilter.Null) + { + if (string.IsNullOrEmpty(featureFlagFilter)) + { + throw new ArgumentNullException(nameof(featureFlagFilter)); + } + + if (labelFilter == null) + { + labelFilter = LabelFilter.Null; + } + + // Do not support * and , for label filter for now. + if (labelFilter.Contains('*') || labelFilter.Contains(',')) + { + throw new ArgumentException("The characters '*' and ',' are not supported in label filters.", nameof(labelFilter)); + } + + string featureFlagPrefix = FeatureManagementConstants.FeatureFlagMarker + featureFlagFilter; + + if (!FeatureFlagSelectors.Any(s => s.KeyFilter.Equals(featureFlagPrefix) && s.LabelFilter.Equals(labelFilter))) + { + FeatureFlagSelectors.Add(new KeyValueSelector + { + KeyFilter = featureFlagPrefix, + LabelFilter = labelFilter + }); + } + + return this; + } + + /// + /// Trims the provided prefix from the keys of all feature flags retrieved from Azure App Configuration. /// - public string Prefix { get; set; } = string.Empty; + /// + /// For example, you can trim the prefix "MyApp" from all feature flags by setting the prefix to "MyApp". + /// + /// The prefix to be trimmed. + public FeatureFlagOptions TrimFeatureFlagPrefix(string prefix) + { + if (string.IsNullOrEmpty(prefix)) + { + throw new ArgumentNullException(nameof(prefix)); + } + + _featureFlagPrefixes.Add(prefix); + return this; + } } } diff --git a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs index edeabffb..529cb9f2 100644 --- a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs +++ b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs @@ -174,8 +174,6 @@ public class FeatureManagementTests eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1")), }; - ConfigurationSetting FirstFeatureFlag => _featureFlagCollection.First(); - [Fact] public void UsesFeatureFlags() { @@ -429,16 +427,16 @@ public void UsesEtagForFeatureFlagRefresh() } [Fact] - public void PrefixFilterFeatureFlags() + public void SelectFeatureFlags() { var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); - var prefix = "App1"; - var label = "App1_Label"; + var featureFlagPrefix = "App1"; + var labelFilter = "App1_Label"; var cacheExpiration = TimeSpan.FromSeconds(1); mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) - .Returns(new MockAsyncPageable(_featureFlagCollection.Where(s => s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix) && s.Label == label).ToList())); + .Returns(new MockAsyncPageable(_featureFlagCollection.Where(s => s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + featureFlagPrefix) && s.Label == labelFilter).ToList())); var testClient = mockClient.Object; @@ -449,8 +447,7 @@ public void PrefixFilterFeatureFlags() options.UseFeatureFlags(ff => { ff.CacheExpirationInterval = cacheExpiration; - ff.Label = label; - ff.Prefix = prefix; + ff.Select(featureFlagPrefix + "*", labelFilter); }); }) .Build(); @@ -466,6 +463,64 @@ public void PrefixFilterFeatureFlags() Assert.Null(config["FeatureManagement:App2_Feature2"]); } + [Fact] + public void MultipleSelectsInSameUseFeatureFlags() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix1 = "App1"; + var prefix2 = "App2"; + var label1 = "App1_Label"; + var label2 = "App2_Label"; + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(() => + { + return new MockAsyncPageable(_featureFlagCollection.Where(s => + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix1) && s.Label == label1) || + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix2) && s.Label == label2)).ToList()); + }); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = testClient; + options.UseFeatureFlags(ff => + { + ff.Select(prefix1 + "*", label1); + ff.Select(prefix2 + "*", label2); + }); + }) + .Build(); + + Assert.Equal("True", config["FeatureManagement:App1_Feature1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + Assert.Equal("False", config["FeatureManagement:App2_Feature1"]); + Assert.Equal("True", config["FeatureManagement:App2_Feature2"]); + + // Verify that the feature flag that did not start with the specified prefix was not loaded + Assert.Null(config["FeatureManagement:Feature1"]); + } + + [Fact] + public void UseFeatureFlagsThrowsIfBothSelectAndLabelPresent() + { + void action() => new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.UseFeatureFlags(ff => + { + ff.Select("MyApp*", "Label1"); + ff.Label = "Label1"; + }); + }) + .Build(); + + Assert.Throws(action); + } + [Fact] public void MultipleCallsToUseFeatureFlags() { @@ -475,7 +530,6 @@ public void MultipleCallsToUseFeatureFlags() var prefix2 = "App2"; var label1 = "App1_Label"; var label2 = "App2_Label"; - var cacheExpiration = TimeSpan.FromSeconds(1); mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) .Returns(() => @@ -493,15 +547,11 @@ public void MultipleCallsToUseFeatureFlags() options.Client = testClient; options.UseFeatureFlags(ff => { - ff.CacheExpirationInterval = cacheExpiration; - ff.Label = label1; - ff.Prefix = prefix1; + ff.Select(prefix1 + "*", label1); }); options.UseFeatureFlags(ff => { - ff.CacheExpirationInterval = cacheExpiration; - ff.Label = label2; - ff.Prefix = prefix2; + ff.Select(prefix2 + "*", label2); }); }) .Build(); @@ -515,6 +565,50 @@ public void MultipleCallsToUseFeatureFlags() Assert.Null(config["FeatureManagement:Feature1"]); } + [Fact] + public void MultipleCallsToUseFeatureFlagsWithSelectAndLabel() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix1 = "App1"; + var label1 = "App1_Label"; + var label2 = "App2_Label"; + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(() => + { + return new MockAsyncPageable(_featureFlagCollection.Where(s => + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix1) && s.Label == label1) || + (s.Label == label2)).ToList()); + }); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = testClient; + options.UseFeatureFlags(ff => + { + ff.Select(prefix1 + "*", label1); + }); + options.UseFeatureFlags(ff => + { + ff.Label = label2; + }); + }) + .Build(); + + // Loaded from prefix1 and label1 + Assert.Equal("True", config["FeatureManagement:App1_Feature1"]); + Assert.Equal("False", config["FeatureManagement:App1_Feature2"]); + + // Loaded from label2 + Assert.Equal("False", config["FeatureManagement:App2_Feature1"]); + Assert.Equal("True", config["FeatureManagement:App2_Feature2"]); + Assert.Equal("True", config["FeatureManagement:Feature1"]); + } + [Fact] public void DifferentCacheExpirationsForMultipleFeatureFlagRegistrations() { @@ -544,14 +638,12 @@ public void DifferentCacheExpirationsForMultipleFeatureFlagRegistrations() options.UseFeatureFlags(ff => { ff.CacheExpirationInterval = cacheExpiration1; - ff.Label = label1; - ff.Prefix = prefix1; + ff.Select(prefix1 + "*", label1); }); options.UseFeatureFlags(ff => { ff.CacheExpirationInterval = cacheExpiration2; - ff.Label = label2; - ff.Prefix = prefix2; + ff.Select(prefix2 + "*", label2); }); refresher = options.GetRefresher(); @@ -618,7 +710,7 @@ public void DifferentCacheExpirationsForMultipleFeatureFlagRegistrations() } [Fact] - public void DifferentCacheExpirationsForSameFeatureFlagRegistrations() + public void OverwrittenCacheExpirationForSameFeatureFlagRegistrations() { var mockResponse = new Mock(); var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); @@ -636,10 +728,14 @@ public void DifferentCacheExpirationsForSameFeatureFlagRegistrations() options.Client = mockClient.Object; options.UseFeatureFlags(ff => { + ff.Select("*", "App1_Label"); + ff.Select("*", "App2_Label"); ff.CacheExpirationInterval = cacheExpiration1; }); options.UseFeatureFlags(ff => { + ff.Select("*", "App1_Label"); + ff.Select("*", "App2_Label"); ff.CacheExpirationInterval = cacheExpiration2; }); @@ -709,8 +805,8 @@ public void TrimFeatureFlags() { ff.CacheExpirationInterval = cacheExpiration; ff.Label = label1; + ff.TrimFeatureFlagPrefix("App1_"); }); - options.TrimFeatureFlagPrefix("App1_"); }) .Build(); @@ -747,24 +843,130 @@ public void MultipleTrimFeatureFlags() options.UseFeatureFlags(ff => { ff.CacheExpirationInterval = cacheExpiration; - ff.Label = label1; - ff.Prefix = prefix1; + ff.Select(prefix1 + "*", label1); + ff.Select(prefix2 + "*", label2); + ff.TrimFeatureFlagPrefix("App1_"); + ff.TrimFeatureFlagPrefix("App2_"); + }); + }) + .Build(); + + // Verify that both prefixes were trimmed from feature flag IDs and feature flags + // with label2 overwrote label1 values due to key name conflict after trimming + Assert.Equal("False", config["FeatureManagement:Feature1"]); + Assert.Equal("True", config["FeatureManagement:Feature2"]); + } + + [Fact] + public void TrimFeatureFlagPrefixHasGlobalScope() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix1 = "App1"; + var prefix2 = "App2"; + var label1 = "App1_Label"; + var label2 = "App2_Label"; + var cacheExpiration = TimeSpan.FromSeconds(1); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(() => + { + return new MockAsyncPageable(_featureFlagCollection.Where(s => + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix1) && s.Label == label1) || + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix2) && s.Label == label2)).ToList()); + }); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = testClient; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Select(prefix1 + "*", label1); + ff.TrimFeatureFlagPrefix("App2_"); }); options.UseFeatureFlags(ff => { ff.CacheExpirationInterval = cacheExpiration; - ff.Label = label2; - ff.Prefix = prefix2; + ff.Select(prefix2 + "*", label2); + ff.TrimFeatureFlagPrefix("App1_"); }); - options.TrimFeatureFlagPrefix("App1_"); - options.TrimFeatureFlagPrefix("App2_"); - }) .Build(); - // Verify that prefix was trimmed from feature flag ID and in case of same ID after trimming, last feature flag value was present in config + // Verify that both prefixes were trimmed from feature flag IDs and feature flags + // with label2 overwrote label1 values due to key name conflict after trimming Assert.Equal("False", config["FeatureManagement:Feature1"]); Assert.Equal("True", config["FeatureManagement:Feature2"]); } + + [Fact] + public void SelectAndRefreshSingleFeatureFlag() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict, TestHelpers.CreateMockEndpointString()); + var prefix1 = "Feature1"; + var label1 = "App1_Label"; + var cacheExpiration = TimeSpan.FromSeconds(1); + IConfigurationRefresher refresher = null; + var featureFlagCollection = new List(_featureFlagCollection); + + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny(), It.IsAny())) + .Returns(() => + { + return new MockAsyncPageable(featureFlagCollection.Where(s => + s.Key.Equals(FeatureManagementConstants.FeatureFlagMarker + prefix1) && s.Label == label1).ToList()); + }); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.Client = mockClient.Object; + options.UseFeatureFlags(ff => + { + ff.CacheExpirationInterval = cacheExpiration; + ff.Select(prefix1, label1); + }); + + refresher = options.GetRefresher(); + }) + .Build(); + + Assert.Equal("False", config["FeatureManagement:Feature1"]); + + // update the value of Feature1 feature flag with App1_Label + featureFlagCollection[2] = ConfigurationModelFactory.ConfigurationSetting( + key: FeatureManagementConstants.FeatureFlagMarker + "Feature1", + value: @" + { + ""id"": ""Feature1"", + ""enabled"": true, + ""conditions"": { + ""client_filters"": [ + { + ""name"": ""Browser"", + ""parameters"": { + ""AllowedBrowsers"": [ ""Chrome"", ""Edge"" ] + } + } + ] + } + } + ", + label: "App1_Label", + contentType: FeatureManagementConstants.ContentType + ";charset=utf-8", + eTag: new ETag("c3c231fd-39a0-4cb6-3237-4614474b92c1" + "f")); + + // Sleep to let the cache for feature flag with label1 expire + Thread.Sleep(cacheExpiration); + refresher.RefreshAsync().Wait(); + + Assert.Equal("Browser", config["FeatureManagement:Feature1:EnabledFor:0:Name"]); + Assert.Equal("Chrome", config["FeatureManagement:Feature1:EnabledFor:0:Parameters:AllowedBrowsers:0"]); + Assert.Equal("Edge", config["FeatureManagement:Feature1:EnabledFor:0:Parameters:AllowedBrowsers:1"]); + } } } From 5acf31ac882378677efd23787e8b7c2d05b2697b Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Fri, 19 Mar 2021 17:58:46 -0700 Subject: [PATCH 3/4] Resolving PR comments --- .../AzureAppConfigurationOptions.cs | 35 ++++++++----------- .../AzureAppConfigurationProvider.cs | 8 ++--- .../ConfigurationClientExtensions.cs | 8 ++--- .../FeatureManagement/FeatureFlagOptions.cs | 11 +++--- .../GetKeyValueChangeCollectionOptions.cs | 2 +- .../FeatureManagementTests.cs | 17 +++++++++ 6 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index eabce37d..86090086 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -34,8 +34,10 @@ public class AzureAppConfigurationOptions private List _kvSelectors = new List(); private IConfigurationRefresher _refresher = new AzureAppConfigurationRefresher(); + // The following sets are sorted in descending order. + // Since multiple prefixes could start with the same characters, we need to trim the longest prefix first. private SortedSet _keyPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.OrdinalIgnoreCase))); - private List _featureFlagPrefixes = new List(); + private SortedSet _featureFlagPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.OrdinalIgnoreCase))); /// /// The connection string to use to connect to Azure App Configuration. @@ -158,34 +160,18 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action c if (options.FeatureFlagSelectors.Count() != 0 && options.Label != null) { - throw new InvalidOperationException($"Please select feature flags by either the {nameof(options.Select)} method or by setting the {nameof(options.Label)}, not both."); + throw new InvalidOperationException($"Please select feature flags by either the {nameof(options.Select)} method or by setting the {nameof(options.Label)} property, not both."); } - if (options.FeatureFlagSelectors.Count() == 0 && options.Label == null) + if (options.FeatureFlagSelectors.Count() == 0) { - // No Select clause and the Label property is not set + // Select clause is not present options.FeatureFlagSelectors.Add(new KeyValueSelector { KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*", - LabelFilter = LabelFilter.Null + LabelFilter = options.Label == null ? LabelFilter.Null : options.Label }); } - else if (options.FeatureFlagSelectors.Count() == 0 && options.Label != null) - { - // No Select clause but the Label property is set - options.FeatureFlagSelectors.Add(new KeyValueSelector - { - KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*", - LabelFilter = options.Label - }); - } - - _featureFlagPrefixes.AddRange(options.FeatureFlagPrefixes); - - if (!_adapters.Any(a => a is FeatureManagementKeyValueAdapter)) - { - _adapters.Add(new FeatureManagementKeyValueAdapter(_featureFlagPrefixes)); - } foreach (var featureFlagSelector in options.FeatureFlagSelectors) { @@ -215,6 +201,13 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action c } } + options.FeatureFlagPrefixes.ForEach(prefix => _featureFlagPrefixes.Add(prefix)); + + if (!_adapters.Any(a => a is FeatureManagementKeyValueAdapter)) + { + _adapters.Add(new FeatureManagementKeyValueAdapter(_featureFlagPrefixes)); + } + return this; } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index 83ec5e50..090cff85 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -465,11 +465,7 @@ private async Task RefreshKeyValueCollections() { IEnumerable currentKeyValues; - // Check if the user-provided feature flag filter ends with an unescaped * character: - // (? @@ -487,7 +483,7 @@ private async Task RefreshKeyValueCollections() IEnumerable keyValueChanges = await _client.GetKeyValueChangeCollection(currentKeyValues, new GetKeyValueChangeCollectionOptions { - Prefix = changeWatcher.Key, + KeyFilter = changeWatcher.Key, Label = changeWatcher.Label.NormalizeNull(), RequestTracingEnabled = _requestTracingEnabled, HostType = _hostType diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs index 9dcc5e81..e8d2333a 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ConfigurationClientExtensions.cs @@ -72,9 +72,9 @@ public static async Task> GetKeyValueChangeCollectio keyValues = Enumerable.Empty(); } - if (options.Prefix == null) + if (options.KeyFilter == null) { - options.Prefix = string.Empty; + options.KeyFilter = string.Empty; } if (keyValues.Any(k => string.IsNullOrEmpty(k.Key))) @@ -95,7 +95,7 @@ public static async Task> GetKeyValueChangeCollectio var hasKeyValueCollectionChanged = false; var selector = new SettingSelector { - KeyFilter = options.Prefix, + KeyFilter = options.KeyFilter, LabelFilter = string.IsNullOrEmpty(options.Label) ? LabelFilter.Null : options.Label, Fields = SettingFields.ETag | SettingFields.Key }; @@ -132,7 +132,7 @@ await TracingUtils.CallWithRequestTracing(options.RequestTracingEnabled, Request { selector = new SettingSelector { - KeyFilter = options.Prefix, + KeyFilter = options.KeyFilter, LabelFilter = string.IsNullOrEmpty(options.Label) ? LabelFilter.Null : options.Label }; diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs index ed6f069e..24f0491d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs @@ -13,12 +13,10 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManage /// public class FeatureFlagOptions { - private SortedSet _featureFlagPrefixes = new SortedSet(Comparer.Create((k1, k2) => -string.Compare(k1, k2, StringComparison.OrdinalIgnoreCase))); - /// /// A collection of key prefixes to be trimmed. /// - internal IEnumerable FeatureFlagPrefixes => _featureFlagPrefixes; + internal List FeatureFlagPrefixes = new List(); /// /// A collection of . @@ -56,6 +54,11 @@ public FeatureFlagOptions Select(string featureFlagFilter, string labelFilter = throw new ArgumentNullException(nameof(featureFlagFilter)); } + if (featureFlagFilter.EndsWith(@"\*")) + { + throw new ArgumentException(@"Feature flag filter should not end with '\*'.", nameof(featureFlagFilter)); + } + if (labelFilter == null) { labelFilter = LabelFilter.Null; @@ -95,7 +98,7 @@ public FeatureFlagOptions TrimFeatureFlagPrefix(string prefix) throw new ArgumentNullException(nameof(prefix)); } - _featureFlagPrefixes.Add(prefix); + FeatureFlagPrefixes.Add(prefix); return this; } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/GetKeyValueChangeCollectionOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/GetKeyValueChangeCollectionOptions.cs index 156bfbe3..117a1d7c 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/GetKeyValueChangeCollectionOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/GetKeyValueChangeCollectionOptions.cs @@ -5,7 +5,7 @@ namespace Microsoft.Extensions.Configuration.AzureAppConfiguration { internal class GetKeyValueChangeCollectionOptions { - public string Prefix { get; set; } + public string KeyFilter { get; set; } public string Label { get; set; } public bool RequestTracingEnabled { get; set; } public HostType HostType { get; set; } diff --git a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs index 529cb9f2..6472c698 100644 --- a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs +++ b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs @@ -521,6 +521,23 @@ public void UseFeatureFlagsThrowsIfBothSelectAndLabelPresent() Assert.Throws(action); } + [Fact] + public void UseFeatureFlagsThrowsIfFeatureFlagFilterIsInvalid() + { + void action() => new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.UseFeatureFlags(ff => + { + ff.Select(@"MyApp\*", "Label1"); + ff.Label = "Label1"; + }); + }) + .Build(); + + Assert.Throws(action); + } + [Fact] public void MultipleCallsToUseFeatureFlags() { From 4dcb85446c264102bf075be50113eb6acd028afa Mon Sep 17 00:00:00 2001 From: Avani Gupta Date: Thu, 25 Mar 2021 12:50:28 -0700 Subject: [PATCH 4/4] Resolve comments --- .../AzureAppConfigurationProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs index 090cff85..6a561b58 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs @@ -14,7 +14,6 @@ using System.Net; using System.Security; using System.Text.Json; -using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; @@ -468,9 +467,10 @@ private async Task RefreshKeyValueCollections() if (changeWatcher.Key.EndsWith("*")) { // Get current application settings starting with changeWatcher.Key, excluding the last * character + var keyPrefix = changeWatcher.Key.Substring(0, changeWatcher.Key.Length - 1); currentKeyValues = _applicationSettings.Values.Where(kv => { - return kv.Key.StartsWith(changeWatcher.Key.Substring(0, changeWatcher.Key.Length - 1)) && kv.Label == changeWatcher.Label.NormalizeNull(); + return kv.Key.StartsWith(keyPrefix) && kv.Label == changeWatcher.Label.NormalizeNull(); }); } else