refactor: Simplify Provider Repository (#515)

askpt · web-flow · commit 2547a574e0d0 · 2025-07-28T16:26:12.000Z
* refactor: simplify initialization and shutdown methods with cancellation support

Signed-off-by: André Silva &lt;2493377+askpt@users.noreply.github.com&gt;

* refactor: change visibility of GetProvider and ShutdownAsync methods to internal

Signed-off-by: André Silva &lt;2493377+askpt@users.noreply.github.com&gt;

* fix: correct logger instance type in ProviderRepository

Signed-off-by: André Silva &lt;2493377+askpt@users.noreply.github.com&gt;

* fix: update preprocessor directives for compatibility with .NET Standard

Signed-off-by: André Silva &lt;2493377+askpt@users.noreply.github.com&gt;

* fix: update SetProviderAsync method to enforce non-null domain parameter

Signed-off-by: André Silva &lt;2493377+askpt@users.noreply.github.com&gt;

* fix: enhance documentation for GetProvider and FeatureClient constructor parameters

Signed-off-by: André Silva &lt;2493377+askpt@users.noreply.github.com&gt;

---------

Signed-off-by: André Silva &lt;2493377+askpt@users.noreply.github.com&gt;
diff --git a/src/OpenFeature/Api.cs b/src/OpenFeature/Api.cs
@@ -85,7 +85,7 @@ public FeatureProvider GetProvider()
     /// Gets the feature provider with given domain
     /// </summary>
     /// <param name="domain">An identifier which logically binds clients with providers</param>
-    /// <returns>A provider associated with the given domain, if domain is empty or doesn't
+    /// <returns>A provider associated with the given domain, if domain is empty, null, whitespace or doesn't
     /// have a corresponding provider the default provider will be returned</returns>
     public FeatureProvider GetProvider(string domain)
     {
@@ -114,7 +114,7 @@ public FeatureProvider GetProvider(string domain)
     /// <summary>
     /// Create a new instance of <see cref="FeatureClient"/> using the current provider
     /// </summary>
-    /// <param name="name">Name of client</param>
+    /// <param name="name">Name of client, if the <paramref name="name"/> is not provided a default name will be used</param>
     /// <param name="version">Version of client</param>
     /// <param name="logger">Logger instance used by client</param>
     /// <param name="context">Context given to this client</param>
diff --git a/src/OpenFeature/ProviderRepository.cs b/src/OpenFeature/ProviderRepository.cs
@@ -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.
     ///
@@ -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()
     {
@@ -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)
@@ -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)
         {
@@ -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);
@@ -134,15 +135,15 @@ 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 (domain == null)
+        if (string.IsNullOrWhiteSpace(domain))
         {
             return;
         }
@@ -166,21 +167,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))
         {
@@ -192,7 +193,7 @@ private async Task ShutdownIfUnusedAsync(
             return;
         }
 
-        await this.SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);
+        await this.SafeShutdownProviderAsync(targetProvider, cancellationToken).ConfigureAwait(false);
     }
 
     /// <remarks>
@@ -204,7 +205,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)
         {
@@ -213,15 +214,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
@@ -234,16 +235,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();
         }
@@ -254,7 +255,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();
@@ -278,7 +279,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);
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -4,20 +4,18 @@`
`4`	`4`	`using OpenFeature.Constant;`
`5`	`5`	`using OpenFeature.Model;`
`6`	`6`
`7`		`-`
`8`	`7`	`namespace OpenFeature;`
`9`	`8`
`10`	`9`	`/// <summary>`
`11`	`10`	`/// This class manages the collection of providers, both default and named, contained by the API.`
`12`	`11`	`/// </summary>`
`13`	`12`	`internal sealed partial class ProviderRepository : IAsyncDisposable`
`14`	`13`	`{`
`15`		`- private ILogger _logger = NullLogger<EventExecutor>.Instance;`
	`14`	`+ private ILogger _logger = NullLogger<ProviderRepository>.Instance;`
`16`	`15`
`17`	`16`	`private FeatureProvider _defaultProvider = new NoOpFeatureProvider();`
`18`	`17`
`19`		`- private readonly ConcurrentDictionary<string, FeatureProvider> _featureProviders =`
`20`		`- new ConcurrentDictionary<string, FeatureProvider>();`
	`18`	`+ private readonly ConcurrentDictionary<string, FeatureProvider> _featureProviders = new();`
`21`	`19`
`22`	`20`	`/// The reader/writer locks is not disposed because the singleton instance should never be disposed.`
`23`	`21`	`///`
`@@ -29,7 +27,7 @@ internal sealed partial class ProviderRepository : IAsyncDisposable`
`29`	`27`	`/// The second is that a concurrent collection doesn't provide any ordering, so we could check a provider`
`30`	`28`	`/// as it was being added or removed such as two concurrent calls to SetProvider replacing multiple instances`
`31`	`29`	`/// of that provider under different names.`
`32`		`- private readonly ReaderWriterLockSlim _providersLock = new ReaderWriterLockSlim();`
	`30`	`+ private readonly ReaderWriterLockSlim _providersLock = new();`
`33`	`31`
`34`	`32`	`public async ValueTask DisposeAsync()`
`35`	`33`	`{`
`@@ -53,11 +51,13 @@ public async ValueTask DisposeAsync()`
`53`	`51`	`/// called if an error happens during the initialization of the provider, only called if the provider needed`
`54`	`52`	`/// initialization`
`55`	`53`	`/// </param>`
`56`		`- public async Task SetProviderAsync(`
	`54`	`+ /// <param name="cancellationToken">a cancellation token to cancel the operation</param>`
	`55`	`+ internal async Task SetProviderAsync(`
`57`	`56`	`FeatureProvider? featureProvider,`
`58`	`57`	`EvaluationContext context,`
`59`	`58`	`Func<FeatureProvider, Task>? afterInitSuccess = null,`
`60`		`- Func<FeatureProvider, Exception, Task>? afterInitError = null)`
	`59`	`+ Func<FeatureProvider, Exception, Task>? afterInitError = null,`
	`60`	`+ CancellationToken cancellationToken = default)`
`61`	`61`	`{`
`62`	`62`	`// Cannot unset the feature provider.`
`63`	`63`	`if (featureProvider == null)`
`@@ -79,22 +79,23 @@ public async Task SetProviderAsync(`
`79`	`79`	`this._defaultProvider = featureProvider;`
`80`	`80`	`// We want to allow shutdown to happen concurrently with initialization, and the caller to not`
`81`	`81`	`// wait for it.`
`82`		`- _ = this.ShutdownIfUnusedAsync(oldProvider);`
	`82`	`+ _ = this.ShutdownIfUnusedAsync(oldProvider, cancellationToken);`
`83`	`83`	`}`
`84`	`84`	`finally`
`85`	`85`	`{`
`86`	`86`	`this._providersLock.ExitWriteLock();`
`87`	`87`	`}`
`88`	`88`
`89`		`- await InitProviderAsync(this._defaultProvider, context, afterInitSuccess, afterInitError)`
	`89`	`+ await InitProviderAsync(this._defaultProvider, context, afterInitSuccess, afterInitError, cancellationToken)`
`90`	`90`	`.ConfigureAwait(false);`
`91`	`91`	`}`
`92`	`92`
`93`	`93`	`private static async Task InitProviderAsync(`
`94`	`94`	`FeatureProvider? newProvider,`
`95`	`95`	`EvaluationContext context,`
`96`	`96`	`Func<FeatureProvider, Task>? afterInitialization,`
`97`		`- Func<FeatureProvider, Exception, Task>? afterError)`
	`97`	`+ Func<FeatureProvider, Exception, Task>? afterError,`
	`98`	`+ CancellationToken cancellationToken = default)`
`98`	`99`	`{`
`99`	`100`	`if (newProvider == null)`
`100`	`101`	`{`
`@@ -104,7 +105,7 @@ private static async Task InitProviderAsync(`
`104`	`105`	`{`
`105`	`106`	`try`
`106`	`107`	`{`
`107`		`- await newProvider.InitializeAsync(context).ConfigureAwait(false);`
	`108`	`+ await newProvider.InitializeAsync(context, cancellationToken).ConfigureAwait(false);`
`108`	`109`	`if (afterInitialization != null)`
`109`	`110`	`{`
`110`	`111`	`await afterInitialization.Invoke(newProvider).ConfigureAwait(false);`
`@@ -134,15 +135,15 @@ private static async Task InitProviderAsync(`
`134`	`135`	`/// initialization`
`135`	`136`	`/// </param>`
`136`	`137`	`/// <param name="cancellationToken">The <see cref="CancellationToken"/> to cancel any async side effects.</param>`
`137`		`- public async Task SetProviderAsync(string? domain,`
	`138`	`+ internal async Task SetProviderAsync(string domain,`
`138`	`139`	`FeatureProvider? featureProvider,`
`139`	`140`	`EvaluationContext context,`
`140`	`141`	`Func<FeatureProvider, Task>? afterInitSuccess = null,`
`141`	`142`	`Func<FeatureProvider, Exception, Task>? afterInitError = null,`
`142`	`143`	`CancellationToken cancellationToken = default)`
`143`	`144`	`{`
`144`	`145`	`// Cannot set a provider for a null domain.`
`145`		`- if (domain == null)`
	`146`	`+ if (string.IsNullOrWhiteSpace(domain))`
`146`	`147`	`{`
`147`	`148`	`return;`
`148`	`149`	`}`
`@@ -166,21 +167,21 @@ public async Task SetProviderAsync(string? domain,`
`166`	`167`
`167`	`168`	`// We want to allow shutdown to happen concurrently with initialization, and the caller to not`
`168`	`169`	`// wait for it.`
`169`		`- _ = this.ShutdownIfUnusedAsync(oldProvider);`
	`170`	`+ _ = this.ShutdownIfUnusedAsync(oldProvider, cancellationToken);`
`170`	`171`	`}`
`171`	`172`	`finally`
`172`	`173`	`{`
`173`	`174`	`this._providersLock.ExitWriteLock();`
`174`	`175`	`}`
`175`	`176`
`176`		`- await InitProviderAsync(featureProvider, context, afterInitSuccess, afterInitError).ConfigureAwait(false);`
	`177`	`+ await InitProviderAsync(featureProvider, context, afterInitSuccess, afterInitError, cancellationToken).ConfigureAwait(false);`
`177`	`178`	`}`
`178`	`179`
`179`	`180`	`/// <remarks>`
`180`	`181`	`/// Shutdown the feature provider if it is unused. This must be called within a write lock of the _providersLock.`
`181`	`182`	`/// </remarks>`
`182`	`183`	`private async Task ShutdownIfUnusedAsync(`
`183`		`- FeatureProvider? targetProvider)`
	`184`	`+ FeatureProvider? targetProvider, CancellationToken cancellationToken = default)`
`184`	`185`	`{`
`185`	`186`	`if (ReferenceEquals(this._defaultProvider, targetProvider))`
`186`	`187`	`{`
`@@ -192,7 +193,7 @@ private async Task ShutdownIfUnusedAsync(`
`192`	`193`	`return;`
`193`	`194`	`}`
`194`	`195`
`195`		`- await this.SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);`
	`196`	`+ await this.SafeShutdownProviderAsync(targetProvider, cancellationToken).ConfigureAwait(false);`
`196`	`197`	`}`
`197`	`198`
`198`	`199`	`/// <remarks>`
`@@ -204,7 +205,7 @@ private async Task ShutdownIfUnusedAsync(`
`204`	`205`	`/// it would not be meaningful to emit an error.`
`205`	`206`	`/// </para>`
`206`	`207`	`/// </remarks>`
`207`		`- private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider)`
	`208`	`+ private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider, CancellationToken cancellationToken = default)`
`208`	`209`	`{`
`209`	`210`	`if (targetProvider == null)`
`210`	`211`	`{`
`@@ -213,15 +214,15 @@ private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider)`
`213`	`214`
`214`	`215`	`try`
`215`	`216`	`{`
`216`		`- await targetProvider.ShutdownAsync().ConfigureAwait(false);`
	`217`	`+ await targetProvider.ShutdownAsync(cancellationToken).ConfigureAwait(false);`
`217`	`218`	`}`
`218`	`219`	`catch (Exception ex)`
`219`	`220`	`{`
`220`	`221`	`this.ErrorShuttingDownProvider(targetProvider.GetMetadata()?.Name, ex);`
`221`	`222`	`}`
`222`	`223`	`}`
`223`	`224`
`224`		`- public FeatureProvider GetProvider()`
	`225`	`+ internal FeatureProvider GetProvider()`
`225`	`226`	`{`
`226`	`227`	`this._providersLock.EnterReadLock();`
`227`	`228`	`try`
`@@ -234,16 +235,16 @@ public FeatureProvider GetProvider()`
`234`	`235`	`}`
`235`	`236`	`}`
`236`	`237`
`237`		`- public FeatureProvider GetProvider(string? domain)`
	`238`	`+ internal FeatureProvider GetProvider(string? domain)`
`238`	`239`	`{`
`239`		`-#if NET6_0_OR_GREATER`
`240`		`- if (string.IsNullOrEmpty(domain))`
	`240`	`+#if NETFRAMEWORK \|\| NETSTANDARD`
	`241`	`+ // This is a workaround for the issue in .NET Framework where string.IsNullOrEmpty is not nullable compatible.`
	`242`	`+ if (domain == null)`
`241`	`243`	`{`
`242`	`244`	`return this.GetProvider();`
`243`	`245`	`}`
`244`	`246`	`#else`
`245`		`- // This is a workaround for the issue in .NET Framework where string.IsNullOrEmpty is not nullable compatible.`
`246`		`- if (domain == null \|\| string.IsNullOrEmpty(domain))`
	`247`	`+ if (string.IsNullOrWhiteSpace(domain))`
`247`	`248`	`{`
`248`	`249`	`return this.GetProvider();`
`249`	`250`	`}`
`@@ -254,7 +255,7 @@ public FeatureProvider GetProvider(string? domain)`
`254`	`255`	`: this.GetProvider();`
`255`	`256`	`}`
`256`	`257`
`257`		`- public async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError = null, CancellationToken cancellationToken = default)`
	`258`	`+ internal async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError = null, CancellationToken cancellationToken = default)`
`258`	`259`	`{`
`259`	`260`	`var providers = new HashSet<FeatureProvider>();`
`260`	`261`	`this._providersLock.EnterWriteLock();`
`@@ -278,7 +279,7 @@ public async Task ShutdownAsync(Action<FeatureProvider, Exception>? afterError =`
`278`	`279`	`foreach (var targetProvider in providers)`
`279`	`280`	`{`
`280`	`281`	`// We don't need to take any actions after shutdown.`
`281`		`- await this.SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false);`
	`282`	`+ await this.SafeShutdownProviderAsync(targetProvider, cancellationToken).ConfigureAwait(false);`
`282`	`283`	`}`
`283`	`284`	`}`
`284`	`285`