From 5434fb3338085aa90db3b08a5f56c740517be417 Mon Sep 17 00:00:00 2001 From: Yan Zhang <2351748+Eskibear@users.noreply.github.com> Date: Wed, 13 Dec 2023 10:21:27 +0800 Subject: [PATCH] dedup same selectors with original precedence (#494) --- .../AzureAppConfigurationOptions.cs | 50 ++++++------------- .../Extensions/ListExtensions.cs | 31 ++++++++++++ .../FeatureManagement/FeatureFlagOptions.cs | 12 ++--- .../Models/KeyValueSelector.cs | 28 +++++++++++ .../Models/KeyValueWatcher.cs | 3 +- .../FeatureManagementTests.cs | 35 +++++++++++++ tests/Tests.AzureAppConfiguration/Tests.cs | 31 ++++++++++++ 7 files changed, 148 insertions(+), 42 deletions(-) create mode 100644 src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ListExtensions.cs diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs index 02ceb495..046233f6 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs @@ -166,15 +166,11 @@ public AzureAppConfigurationOptions Select(string keyFilter, string labelFilter throw new ArgumentException("The characters '*' and ',' are not supported in label filters.", nameof(labelFilter)); } - if (!_kvSelectors.Any(s => string.Equals(s.KeyFilter, keyFilter) && string.Equals(s.LabelFilter, labelFilter))) + _kvSelectors.AppendUnique(new KeyValueSelector { - _kvSelectors.Add(new KeyValueSelector - { - KeyFilter = keyFilter, - LabelFilter = labelFilter - }); - } - + KeyFilter = keyFilter, + LabelFilter = labelFilter + }); return this; } @@ -190,13 +186,10 @@ public AzureAppConfigurationOptions SelectSnapshot(string name) throw new ArgumentNullException(nameof(name)); } - if (!_kvSelectors.Any(s => string.Equals(s.SnapshotName, name))) + _kvSelectors.AppendUnique(new KeyValueSelector { - _kvSelectors.Add(new KeyValueSelector - { - SnapshotName = name - }); - } + SnapshotName = name + }); return this; } @@ -222,7 +215,7 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action c { 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) { // Select clause is not present @@ -230,7 +223,7 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action c { KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*", LabelFilter = options.Label == null ? LabelFilter.Null : options.Label - }); + }); } foreach (var featureFlagSelector in options.FeatureFlagSelectors) @@ -238,27 +231,16 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action c var featureFlagFilter = featureFlagSelector.KeyFilter; var labelFilter = featureFlagSelector.LabelFilter; - if (!_kvSelectors.Any(selector => selector.KeyFilter == featureFlagFilter && selector.LabelFilter == labelFilter)) - { - Select(featureFlagFilter, labelFilter); - } - - var multiKeyWatcher = _multiKeyWatchers.FirstOrDefault(kw => kw.Key.Equals(featureFlagFilter) && kw.Label.NormalizeNull() == labelFilter.NormalizeNull()); + Select(featureFlagFilter, labelFilter); - if (multiKeyWatcher == null) - { - _multiKeyWatchers.Add(new KeyValueWatcher - { - Key = featureFlagFilter, - Label = labelFilter, - CacheExpirationInterval = options.CacheExpirationInterval - }); - } - else + _multiKeyWatchers.AppendUnique(new KeyValueWatcher { + Key = featureFlagFilter, + Label = labelFilter, // If UseFeatureFlags is called multiple times for the same key and label filters, last cache expiration time wins - multiKeyWatcher.CacheExpirationInterval = options.CacheExpirationInterval; - } + CacheExpirationInterval = options.CacheExpirationInterval + }); + } return this; diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ListExtensions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ListExtensions.cs new file mode 100644 index 00000000..06bb5a29 --- /dev/null +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ListExtensions.cs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions +{ + internal static class ListExtensions + { + public static void AppendUnique(this List items, T item) + { + if (item == null) + { + throw new ArgumentNullException(nameof(item)); + } + + T existingItem = items.FirstOrDefault(s => Equals(s, item)); + + if (existingItem != null) + { + // Remove duplicate item if existing. + items.Remove(existingItem); + } + + // Append to the end, keeping precedence. + items.Add(item); + } + } +} diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs index 856a7bd6..202f4055 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureFlagOptions.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. // using Microsoft.Extensions.Configuration.AzureAppConfiguration.Models; +using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions; using System; using System.Collections.Generic; using System.Linq; @@ -67,14 +68,11 @@ public FeatureFlagOptions Select(string featureFlagFilter, string labelFilter = string featureFlagPrefix = FeatureManagementConstants.FeatureFlagMarker + featureFlagFilter; - if (!FeatureFlagSelectors.Any(s => s.KeyFilter.Equals(featureFlagPrefix) && s.LabelFilter.Equals(labelFilter))) + FeatureFlagSelectors.AppendUnique(new KeyValueSelector { - FeatureFlagSelectors.Add(new KeyValueSelector - { - KeyFilter = featureFlagPrefix, - LabelFilter = labelFilter - }); - } + KeyFilter = featureFlagPrefix, + LabelFilter = labelFilter + }); return this; } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueSelector.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueSelector.cs index 49b60661..5491d04d 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueSelector.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueSelector.cs @@ -23,5 +23,33 @@ public class KeyValueSelector /// The name of the Azure App Configuration snapshot to use when selecting key-values for the configuration provider. /// public string SnapshotName { get; set; } + + /// + /// Determines whether the specified object is equal to the current object. + /// + /// The object to compare with the current object. + /// true if the specified object is equal to the current object; otherwise, false. + public override bool Equals(object obj) + { + if (obj is KeyValueSelector selector) + { + return KeyFilter == selector.KeyFilter + && LabelFilter == selector.LabelFilter + && SnapshotName == selector.SnapshotName; + } + + return false; + } + + /// + /// Serves as the hash function. + /// + /// A hash code for the current object. + public override int GetHashCode() + { + return (KeyFilter?.GetHashCode() ?? 0) ^ + (LabelFilter?.GetHashCode() ?? 1) ^ + (SnapshotName?.GetHashCode() ?? 2); + } } } diff --git a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueWatcher.cs b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueWatcher.cs index 85afaa3d..121a35be 100644 --- a/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueWatcher.cs +++ b/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueWatcher.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // +using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions; using System; namespace Microsoft.Extensions.Configuration.AzureAppConfiguration.Models @@ -36,7 +37,7 @@ public override bool Equals(object obj) { if (obj is KeyValueWatcher kvWatcher) { - return Key == kvWatcher.Key && Label == kvWatcher.Label; + return Key == kvWatcher.Key && Label.NormalizeNull() == kvWatcher.Label.NormalizeNull(); } return false; diff --git a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs index bc1e74f4..4eb83a90 100644 --- a/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs +++ b/tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs @@ -518,6 +518,41 @@ public void MultipleSelectsInSameUseFeatureFlags() Assert.Null(config["FeatureManagement:Feature1"]); } + [Fact] + public void KeepSelectorPrecedenceAfterDedup() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict); + var prefix = "Feature1"; + 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 + prefix) && s.Label == label1) || + (s.Key.StartsWith(FeatureManagementConstants.FeatureFlagMarker + prefix) && s.Label == label2)).ToList()); + }); + + var testClient = mockClient.Object; + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(testClient); + options.UseFeatureFlags(ff => + { + ff.Select(prefix + "*", label1); // to be deduped + ff.Select(prefix + "*", label2); // lower precedence + ff.Select(prefix + "*", label1); // higher precedence, taking effect + }); + }) + .Build(); + // label: App1_Label has higher precedence + Assert.Equal("True", config["FeatureManagement:Feature1"]); + } + [Fact] public void UseFeatureFlagsThrowsIfBothSelectAndLabelPresent() { diff --git a/tests/Tests.AzureAppConfiguration/Tests.cs b/tests/Tests.AzureAppConfiguration/Tests.cs index d45ba103..b7e61978 100644 --- a/tests/Tests.AzureAppConfiguration/Tests.cs +++ b/tests/Tests.AzureAppConfiguration/Tests.cs @@ -315,5 +315,36 @@ public void TestTurnOffRequestTracing() // Delete the azure function environment variable Environment.SetEnvironmentVariable(RequestTracingConstants.AzureFunctionEnvironmentVariable, null); } + + [Fact] + public void TestKeepSelectorPrecedenceAfterDedup() + { + var mockResponse = new Mock(); + var mockClient = new Mock(MockBehavior.Strict); + var kvOfDevLabel = new List + { + ConfigurationModelFactory.ConfigurationSetting("message", "message from dev label", "dev") + }; + var kvOfProdLabel = new List + { + ConfigurationModelFactory.ConfigurationSetting("message", "message from prod label", "prod") + }; + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.Is(s => s.LabelFilter == "dev"), It.IsAny())) + .Returns(new MockAsyncPageable(kvOfDevLabel)); + mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.Is(s => s.LabelFilter == "prod"), It.IsAny())) + .Returns(new MockAsyncPageable(kvOfProdLabel)); + + var config = new ConfigurationBuilder() + .AddAzureAppConfiguration(options => + { + options.ClientManager = TestHelpers.CreateMockedConfigurationClientManager(mockClient.Object); + options.Select("message", "dev"); + options.Select("message", "prod"); + options.Select("message", "dev"); + }) + .Build(); + + Assert.True(config["message"] == "message from dev label"); + } } }