Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert ConfigurationClient factory changes, add RegisterAll #619

Merged
merged 2 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
//
using Azure.Core;
using Azure.Data.AppConfiguration;
using Microsoft.Extensions.Azure;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.AzureKeyVault;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement;
Expand All @@ -24,12 +23,13 @@ public class AzureAppConfigurationOptions
private const int MaxRetries = 2;
private static readonly TimeSpan MaxRetryDelay = TimeSpan.FromMinutes(1);

private List<KeyValueWatcher> _changeWatchers = new List<KeyValueWatcher>();
private List<KeyValueWatcher> _multiKeyWatchers = new List<KeyValueWatcher>();
private List<KeyValueWatcher> _individualKvWatchers = new List<KeyValueWatcher>();
private List<KeyValueWatcher> _ffWatchers = new List<KeyValueWatcher>();
private List<IKeyValueAdapter> _adapters;
private List<Func<ConfigurationSetting, ValueTask<ConfigurationSetting>>> _mappers = new List<Func<ConfigurationSetting, ValueTask<ConfigurationSetting>>>();
private List<KeyValueSelector> _kvSelectors = new List<KeyValueSelector>();
private List<KeyValueSelector> _selectors;
private IConfigurationRefresher _refresher = new AzureAppConfigurationRefresher();
private bool _selectCalled = false;

// The following set is sorted in descending order.
// Since multiple prefixes could start with the same characters, we need to trim the longest prefix first.
Expand Down Expand Up @@ -63,19 +63,29 @@ public class AzureAppConfigurationOptions
internal TokenCredential Credential { get; private set; }

/// <summary>
/// A collection of <see cref="KeyValueSelector"/>.
/// A collection of <see cref="KeyValueSelector"/> specified by user.
/// </summary>
internal IEnumerable<KeyValueSelector> KeyValueSelectors => _kvSelectors;
internal IEnumerable<KeyValueSelector> Selectors => _selectors;

/// <summary>
/// Indicates if <see cref="AzureAppConfigurationRefreshOptions.RegisterAll"/> was called.
/// </summary>
internal bool RegisterAllEnabled { get; private set; }

/// <summary>
/// Refresh interval for selected key-value collections when <see cref="AzureAppConfigurationRefreshOptions.RegisterAll"/> is called.
/// </summary>
internal TimeSpan KvCollectionRefreshInterval { get; private set; }

/// <summary>
/// A collection of <see cref="KeyValueWatcher"/>.
/// </summary>
internal IEnumerable<KeyValueWatcher> ChangeWatchers => _changeWatchers;
internal IEnumerable<KeyValueWatcher> IndividualKvWatchers => _individualKvWatchers;

/// <summary>
/// A collection of <see cref="KeyValueWatcher"/>.
/// </summary>
internal IEnumerable<KeyValueWatcher> MultiKeyWatchers => _multiKeyWatchers;
internal IEnumerable<KeyValueWatcher> FeatureFlagWatchers => _ffWatchers;

/// <summary>
/// A collection of <see cref="IKeyValueAdapter"/>.
Expand All @@ -97,11 +107,15 @@ internal IEnumerable<IKeyValueAdapter> Adapters
internal IEnumerable<string> KeyPrefixes => _keyPrefixes;

/// <summary>
/// An optional configuration client manager that can be used to provide clients to communicate with Azure App Configuration.
/// For use in tests only. An optional configuration client manager that can be used to provide clients to communicate with Azure App Configuration.
/// </summary>
/// <remarks>This property is used only for unit testing.</remarks>
internal IConfigurationClientManager ClientManager { get; set; }

/// <summary>
/// For use in tests only. An optional class used to process pageable results from Azure App Configuration.
/// </summary>
internal IConfigurationSettingPageIterator ConfigurationSettingPageIterator { get; set; }

/// <summary>
/// An optional timespan value to set the minimum backoff duration to a value other than the default.
/// </summary>
Expand Down Expand Up @@ -132,11 +146,6 @@ internal IEnumerable<IKeyValueAdapter> Adapters
/// </summary>
internal StartupOptions Startup { get; set; } = new StartupOptions();

/// <summary>
/// Client factory that is responsible for creating instances of ConfigurationClient.
/// </summary>
internal IAzureClientFactory<ConfigurationClient> ClientFactory { get; private set; }

/// <summary>
/// Initializes a new instance of the <see cref="AzureAppConfigurationOptions"/> class.
/// </summary>
Expand All @@ -148,17 +157,9 @@ public AzureAppConfigurationOptions()
new JsonKeyValueAdapter(),
new FeatureManagementKeyValueAdapter(FeatureFlagTracing)
};
}

/// <summary>
/// Sets the client factory used to create ConfigurationClient instances.
/// </summary>
/// <param name="factory">The client factory.</param>
/// <returns>The current <see cref="AzureAppConfigurationOptions"/> instance.</returns>
public AzureAppConfigurationOptions SetClientFactory(IAzureClientFactory<ConfigurationClient> factory)
{
ClientFactory = factory ?? throw new ArgumentNullException(nameof(factory));
return this;
// Adds the default query to App Configuration if <see cref="Select"/> and <see cref="SelectSnapshot"/> are never called.
_selectors = new List<KeyValueSelector> { new KeyValueSelector { KeyFilter = KeyFilter.Any, LabelFilter = LabelFilter.Null } };
}

/// <summary>
Expand Down Expand Up @@ -187,22 +188,30 @@ public AzureAppConfigurationOptions Select(string keyFilter, string labelFilter
throw new ArgumentNullException(nameof(keyFilter));
}

// Do not support * and , for label filter for now.
if (labelFilter != null && (labelFilter.Contains('*') || labelFilter.Contains(',')))
{
throw new ArgumentException("The characters '*' and ',' are not supported in label filters.", nameof(labelFilter));
}

if (string.IsNullOrWhiteSpace(labelFilter))
{
labelFilter = LabelFilter.Null;
}

// Do not support * and , for label filter for now.
if (labelFilter.Contains('*') || labelFilter.Contains(','))
if (!_selectCalled)
{
throw new ArgumentException("The characters '*' and ',' are not supported in label filters.", nameof(labelFilter));
_selectors.Clear();

_selectCalled = true;
}

_kvSelectors.AppendUnique(new KeyValueSelector
_selectors.AppendUnique(new KeyValueSelector
{
KeyFilter = keyFilter,
LabelFilter = labelFilter
});

return this;
}

Expand All @@ -218,7 +227,14 @@ public AzureAppConfigurationOptions SelectSnapshot(string name)
throw new ArgumentNullException(nameof(name));
}

_kvSelectors.AppendUnique(new KeyValueSelector
if (!_selectCalled)
{
_selectors.Clear();

_selectCalled = true;
}

_selectors.AppendUnique(new KeyValueSelector
{
SnapshotName = name
});
Expand All @@ -229,7 +245,7 @@ public AzureAppConfigurationOptions SelectSnapshot(string name)
/// <summary>
/// Configures options for Azure App Configuration feature flags that will be parsed and transformed into feature management configuration.
/// If no filtering is specified via the <see cref="FeatureFlagOptions"/> then all feature flags with no label are loaded.
/// All loaded feature flags will be automatically registered for refresh on an individual flag level.
/// All loaded feature flags will be automatically registered for refresh as a collection.
/// </summary>
/// <param name="configure">A callback used to configure feature flag options.</param>
public AzureAppConfigurationOptions UseFeatureFlags(Action<FeatureFlagOptions> configure = null)
Expand All @@ -254,25 +270,22 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action<FeatureFlagOptions> c
options.FeatureFlagSelectors.Add(new KeyValueSelector
{
KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*",
LabelFilter = options.Label == null ? LabelFilter.Null : options.Label
LabelFilter = string.IsNullOrWhiteSpace(options.Label) ? LabelFilter.Null : options.Label,
IsFeatureFlagSelector = true
});
}

foreach (var featureFlagSelector in options.FeatureFlagSelectors)
foreach (KeyValueSelector featureFlagSelector in options.FeatureFlagSelectors)
{
var featureFlagFilter = featureFlagSelector.KeyFilter;
var labelFilter = featureFlagSelector.LabelFilter;
_selectors.AppendUnique(featureFlagSelector);

Select(featureFlagFilter, labelFilter);

_multiKeyWatchers.AppendUnique(new KeyValueWatcher
_ffWatchers.AppendUnique(new KeyValueWatcher
{
Key = featureFlagFilter,
Label = labelFilter,
Key = featureFlagSelector.KeyFilter,
Label = featureFlagSelector.LabelFilter,
// If UseFeatureFlags is called multiple times for the same key and label filters, last refresh interval wins
RefreshInterval = options.RefreshInterval
});

}

return this;
Expand Down Expand Up @@ -393,18 +406,41 @@ public AzureAppConfigurationOptions ConfigureClientOptions(Action<ConfigurationC
/// <param name="configure">A callback used to configure Azure App Configuration refresh options.</param>
public AzureAppConfigurationOptions ConfigureRefresh(Action<AzureAppConfigurationRefreshOptions> configure)
{
if (RegisterAllEnabled)
{
throw new InvalidOperationException($"{nameof(ConfigureRefresh)}() cannot be invoked multiple times when {nameof(AzureAppConfigurationRefreshOptions.RegisterAll)} has been invoked.");
}

var refreshOptions = new AzureAppConfigurationRefreshOptions();
configure?.Invoke(refreshOptions);

if (!refreshOptions.RefreshRegistrations.Any())
bool isRegisterCalled = refreshOptions.RefreshRegistrations.Any();
RegisterAllEnabled = refreshOptions.RegisterAllEnabled;

if (!isRegisterCalled && !RegisterAllEnabled)
{
throw new InvalidOperationException($"{nameof(ConfigureRefresh)}() must call either {nameof(AzureAppConfigurationRefreshOptions.Register)}()" +
$" or {nameof(AzureAppConfigurationRefreshOptions.RegisterAll)}()");
}

// Check if both register methods are called at any point
if (RegisterAllEnabled && (_individualKvWatchers.Any() || isRegisterCalled))
{
throw new ArgumentException($"{nameof(ConfigureRefresh)}() must have at least one key-value registered for refresh.");
throw new InvalidOperationException($"Cannot call both {nameof(AzureAppConfigurationRefreshOptions.RegisterAll)} and "
+ $"{nameof(AzureAppConfigurationRefreshOptions.Register)}.");
}

foreach (var item in refreshOptions.RefreshRegistrations)
if (RegisterAllEnabled)
{
item.RefreshInterval = refreshOptions.RefreshInterval;
_changeWatchers.Add(item);
KvCollectionRefreshInterval = refreshOptions.RefreshInterval;
}
else
{
foreach (KeyValueWatcher item in refreshOptions.RefreshRegistrations)
{
item.RefreshInterval = refreshOptions.RefreshInterval;
_individualKvWatchers.Add(item);
}
}

return this;
Expand Down
Loading