Skip to content

refactor: Simplify Provider Repository #515

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jul 28, 2025
Merged

refactor: Simplify Provider Repository #515

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24f240a
refactor: simplify initialization and shutdown methods with cancellat…
askpt Jul 7, 2025
1376f6b
refactor: change visibility of GetProvider and ShutdownAsync methods …
askpt Jul 7, 2025
a9ebf4f
fix: correct logger instance type in ProviderRepository
askpt Jul 7, 2025
30acf3f
fix: update preprocessor directives for compatibility with .NET Standard
askpt Jul 7, 2025
b6d45ea
fix: update SetProviderAsync method to enforce non-null domain parameter
askpt Jul 7, 2025
c071451
fix: enhance documentation for GetProvider and FeatureClient construc…
askpt Jul 8, 2025
08c556b
Merge branch 'main' into askpt/chore-provider-repository
askpt Jul 13, 2025
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
61 changes: 35 additions & 26 deletions src/OpenFeature/ProviderRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,18 @@
using OpenFeature.Constant;
using OpenFeature.Model;


namespace OpenFeature;

/// <summary>
/// This class manages the collection of providers, both default and named, contained by the API.
/// </summary>
internal sealed partial class ProviderRepository : IAsyncDisposable
{
private ILogger _logger = NullLogger<EventExecutor>.Instance;
private ILogger _logger = NullLogger<ProviderRepository>.Instance;

private FeatureProvider _defaultProvider = new NoOpFeatureProvider();

private readonly ConcurrentDictionary<string, FeatureProvider> _featureProviders =
new ConcurrentDictionary<string, FeatureProvider>();
private readonly ConcurrentDictionary<string, FeatureProvider> _featureProviders = new();

/// The reader/writer locks is not disposed because the singleton instance should never be disposed.
///
Expand All @@ -29,7 +27,7 @@ internal sealed partial class ProviderRepository : IAsyncDisposable
/// The second is that a concurrent collection doesn't provide any ordering, so we could check a provider
/// as it was being added or removed such as two concurrent calls to SetProvider replacing multiple instances
/// of that provider under different names.
private readonly ReaderWriterLockSlim _providersLock = new ReaderWriterLockSlim();
private readonly ReaderWriterLockSlim _providersLock = new();

public async ValueTask DisposeAsync()
{
Expand All @@ -53,11 +51,13 @@ public async ValueTask DisposeAsync()
/// called if an error happens during the initialization of the provider, only called if the provider needed
/// initialization
/// </param>
public async Task SetProviderAsync(
/// <param name="cancellationToken">a cancellation token to cancel the operation</param>
internal async Task SetProviderAsync(
FeatureProvider? featureProvider,
EvaluationContext context,
Func<FeatureProvider, Task>? afterInitSuccess = null,
Func<FeatureProvider, Exception, Task>? afterInitError = null)
Func<FeatureProvider, Exception, Task>? afterInitError = null,
CancellationToken cancellationToken = default)
{
// Cannot unset the feature provider.
if (featureProvider == null)
Expand All @@ -79,22 +79,23 @@ public async Task SetProviderAsync(
this._defaultProvider = featureProvider;
// We want to allow shutdown to happen concurrently with initialization, and the caller to not
// wait for it.
_ = this.ShutdownIfUnusedAsync(oldProvider);
_ = this.ShutdownIfUnusedAsync(oldProvider, cancellationToken);
}
finally
{
this._providersLock.ExitWriteLock();
}

await InitProviderAsync(this._defaultProvider, context, afterInitSuccess, afterInitError)
await InitProviderAsync(this._defaultProvider, context, afterInitSuccess, afterInitError, cancellationToken)
.ConfigureAwait(false);
}

private static async Task InitProviderAsync(
FeatureProvider? newProvider,
EvaluationContext context,
Func<FeatureProvider, Task>? afterInitialization,
Func<FeatureProvider, Exception, Task>? afterError)
Func<FeatureProvider, Exception, Task>? afterError,
CancellationToken cancellationToken = default)
{
if (newProvider == null)
{
Expand All @@ -104,7 +105,7 @@ private static async Task InitProviderAsync(
{
try
{
await newProvider.InitializeAsync(context).ConfigureAwait(false);
await newProvider.InitializeAsync(context, cancellationToken).ConfigureAwait(false);
if (afterInitialization != null)
{
await afterInitialization.Invoke(newProvider).ConfigureAwait(false);
Expand Down Expand Up @@ -134,18 +135,26 @@ private static async Task InitProviderAsync(
/// initialization
/// </param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>
public async Task SetProviderAsync(string? domain,
internal async Task SetProviderAsync(string? domain,
FeatureProvider? featureProvider,
EvaluationContext context,
Func<FeatureProvider, Task>? afterInitSuccess = null,
Func<FeatureProvider, Exception, Task>? afterInitError = null,
CancellationToken cancellationToken = default)
{
// Cannot set a provider for a null domain.
#if NETFRAMEWORK || NETSTANDARD
// This is a workaround for the issue in .NET Framework where string.IsNullOrEmpty is not nullable compatible.
if (domain == null)
{
return;
}
#else
if (string.IsNullOrWhiteSpace(domain))
{
return;
}
#endif

this._providersLock.EnterWriteLock();

Expand All @@ -166,21 +175,21 @@ public async Task SetProviderAsync(string? domain,

// We want to allow shutdown to happen concurrently with initialization, and the caller to not
// wait for it.
_ = this.ShutdownIfUnusedAsync(oldProvider);
_ = this.ShutdownIfUnusedAsync(oldProvider, cancellationToken);
}
finally
{
this._providersLock.ExitWriteLock();
}

await InitProviderAsync(featureProvider, context, afterInitSuccess, afterInitError).ConfigureAwait(false);
await InitProviderAsync(featureProvider, context, afterInitSuccess, afterInitError, cancellationToken).ConfigureAwait(false);
}

/// <remarks>
/// Shutdown the feature provider if it is unused. This must be called within a write lock of the _providersLock.
/// </remarks>
private async Task ShutdownIfUnusedAsync(
FeatureProvider? targetProvider)
FeatureProvider? targetProvider, CancellationToken cancellationToken = default)
{
if (ReferenceEquals(this._defaultProvider, targetProvider))
{
Expand All @@ -192,7 +201,7 @@ private async Task ShutdownIfUnusedAsync(
return;
}

await this.SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);
await this.SafeShutdownProviderAsync(targetProvider, cancellationToken).ConfigureAwait(false);
}

/// <remarks>
Expand All @@ -204,7 +213,7 @@ private async Task ShutdownIfUnusedAsync(
/// it would not be meaningful to emit an error.
/// </para>
/// </remarks>
private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider)
private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider, CancellationToken cancellationToken = default)
{
if (targetProvider == null)
{
Expand All @@ -213,15 +222,15 @@ private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider)

try
{
await targetProvider.ShutdownAsync().ConfigureAwait(false);
await targetProvider.ShutdownAsync(cancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
this.ErrorShuttingDownProvider(targetProvider.GetMetadata()?.Name, ex);
}
}

public FeatureProvider GetProvider()
internal FeatureProvider GetProvider()
{
this._providersLock.EnterReadLock();
try
Expand All @@ -234,16 +243,16 @@ public FeatureProvider GetProvider()
}
}

public FeatureProvider GetProvider(string? domain)
internal FeatureProvider GetProvider(string? domain)
{
#if NET6_0_OR_GREATER
if (string.IsNullOrEmpty(domain))
#if NETFRAMEWORK || NETSTANDARD
// This is a workaround for the issue in .NET Framework where string.IsNullOrEmpty is not nullable compatible.
if (domain == null)
{
return this.GetProvider();
}
#else
// This is a workaround for the issue in .NET Framework where string.IsNullOrEmpty is not nullable compatible.
if (domain == null || string.IsNullOrEmpty(domain))
if (string.IsNullOrWhiteSpace(domain))
{
return this.GetProvider();
}
Expand All @@ -254,7 +263,7 @@ public FeatureProvider GetProvider(string? domain)
: this.GetProvider();
}

public async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError = null, CancellationToken cancellationToken = default)
internal async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError = null, CancellationToken cancellationToken = default)
{
var providers = new HashSet<FeatureProvider>();
this._providersLock.EnterWriteLock();
Expand All @@ -278,7 +287,7 @@ public async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError =
foreach (var targetProvider in providers)
{
// We don't need to take any actions after shutdown.
await this.SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);
await this.SafeShutdownProviderAsync(targetProvider, cancellationToken).ConfigureAwait(false);
}
}

Expand Down