Skip to content

Commit

Permalink
dedup same selectors with original precedence (#494)
Browse files Browse the repository at this point in the history
  • Loading branch information
Eskibear authored Dec 13, 2023
1 parent dcd8626 commit 5434fb3
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand All @@ -222,43 +215,32 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action<FeatureFlagOptions> 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
options.FeatureFlagSelectors.Add(new KeyValueSelector
{
KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*",
LabelFilter = options.Label == null ? LabelFilter.Null : options.Label
});
});
}

foreach (var featureFlagSelector in options.FeatureFlagSelectors)
{
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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<T>(this List<T> 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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </summary>
public string SnapshotName { get; set; }

/// <summary>
/// Determines whether the specified object is equal to the current object.
/// </summary>
/// <param name="obj">The object to compare with the current object.</param>
/// <returns>true if the specified object is equal to the current object; otherwise, false.</returns>
public override bool Equals(object obj)
{
if (obj is KeyValueSelector selector)
{
return KeyFilter == selector.KeyFilter
&& LabelFilter == selector.LabelFilter
&& SnapshotName == selector.SnapshotName;
}

return false;
}

/// <summary>
/// Serves as the hash function.
/// </summary>
/// <returns>A hash code for the current object.</returns>
public override int GetHashCode()
{
return (KeyFilter?.GetHashCode() ?? 0) ^
(LabelFilter?.GetHashCode() ?? 1) ^
(SnapshotName?.GetHashCode() ?? 2);
}
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand Down
35 changes: 35 additions & 0 deletions tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,41 @@ public void MultipleSelectsInSameUseFeatureFlags()
Assert.Null(config["FeatureManagement:Feature1"]);
}

[Fact]
public void KeepSelectorPrecedenceAfterDedup()
{
var mockResponse = new Mock<Response>();
var mockClient = new Mock<ConfigurationClient>(MockBehavior.Strict);
var prefix = "Feature1";
var label1 = "App1_Label";
var label2 = "App2_Label";

mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.IsAny<SettingSelector>(), It.IsAny<CancellationToken>()))
.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()
{
Expand Down
31 changes: 31 additions & 0 deletions tests/Tests.AzureAppConfiguration/Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Response>();
var mockClient = new Mock<ConfigurationClient>(MockBehavior.Strict);
var kvOfDevLabel = new List<ConfigurationSetting>
{
ConfigurationModelFactory.ConfigurationSetting("message", "message from dev label", "dev")
};
var kvOfProdLabel = new List<ConfigurationSetting>
{
ConfigurationModelFactory.ConfigurationSetting("message", "message from prod label", "prod")
};
mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.Is<SettingSelector>(s => s.LabelFilter == "dev"), It.IsAny<CancellationToken>()))
.Returns(new MockAsyncPageable(kvOfDevLabel));
mockClient.Setup(c => c.GetConfigurationSettingsAsync(It.Is<SettingSelector>(s => s.LabelFilter == "prod"), It.IsAny<CancellationToken>()))
.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");
}
}
}

0 comments on commit 5434fb3

Please sign in to comment.