From ce23cdc6444091a324831e9bf16fcc6d4642f6a8 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 3 Jul 2024 15:47:50 -0400 Subject: [PATCH] feat!: internally maintain provider status (#276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR implements a few things from spec 0.8.0: - implements internal provider status (already implemented in JS) - the provider no longer updates its status to READY/ERROR, etc after init (the SDK does this automatically) - the provider's state is updated according to the last event it fired - adds `PROVIDER_FATAL` error and code - adds "short circuit" feature when evaluations are skipped if provider is `NOT_READY` or `FATAL` - removes some deprecations that were making the work harder since we already have pending breaking changes. Fixes: https://github.com/open-feature/dotnet-sdk/issues/250 --------- Signed-off-by: Todd Baert Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com> Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Signed-off-by: Artyom Tonoyan --- src/OpenFeature/Api.cs | 44 ++- src/OpenFeature/Constant/ErrorType.cs | 5 + src/OpenFeature/Constant/ProviderStatus.cs | 7 +- .../Error/ProviderFatalException.cs | 23 ++ .../Error/ProviderNotReadyException.cs | 2 +- src/OpenFeature/EventExecutor.cs | 21 +- src/OpenFeature/FeatureProvider.cs | 24 +- src/OpenFeature/IFeatureClient.cs | 7 + src/OpenFeature/Model/ProviderEvents.cs | 5 + src/OpenFeature/OpenFeatureClient.cs | 18 +- src/OpenFeature/ProviderRepository.cs | 100 +++---- .../OpenFeatureClientTests.cs | 94 +++++- .../OpenFeatureEventTests.cs | 25 +- test/OpenFeature.Tests/OpenFeatureTests.cs | 16 +- .../ProviderRepositoryTests.cs | 269 ++++-------------- test/OpenFeature.Tests/TestImplementations.cs | 43 +-- 16 files changed, 365 insertions(+), 338 deletions(-) create mode 100644 src/OpenFeature/Error/ProviderFatalException.cs diff --git a/src/OpenFeature/Api.cs b/src/OpenFeature/Api.cs index 6f13cac2..5440151f 100644 --- a/src/OpenFeature/Api.cs +++ b/src/OpenFeature/Api.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.Logging; using OpenFeature.Constant; +using OpenFeature.Error; using OpenFeature.Model; namespace OpenFeature @@ -37,7 +38,7 @@ static Api() { } private Api() { } /// - /// Sets the feature provider. In order to wait for the provider to be set, and initialization to complete, + /// Sets the default feature provider. In order to wait for the provider to be set, and initialization to complete, /// await the returned task. /// /// The provider cannot be set to null. Attempting to set the provider to null has no effect. @@ -45,10 +46,9 @@ private Api() { } public async Task SetProviderAsync(FeatureProvider featureProvider) { this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider); - await this._repository.SetProviderAsync(featureProvider, this.GetContext()).ConfigureAwait(false); + await this._repository.SetProviderAsync(featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false); } - /// /// Sets the feature provider to given clientName. In order to wait for the provider to be set, and /// initialization to complete, await the returned task. @@ -62,7 +62,7 @@ public async Task SetProviderAsync(string clientName, FeatureProvider featurePro throw new ArgumentNullException(nameof(clientName)); } this._eventExecutor.RegisterClientFeatureProvider(clientName, featureProvider); - await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext()).ConfigureAwait(false); + await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false); } /// @@ -121,7 +121,7 @@ public FeatureProvider GetProvider(string clientName) /// public FeatureClient GetClient(string? name = null, string? version = null, ILogger? logger = null, EvaluationContext? context = null) => - new FeatureClient(name, version, logger, context); + new FeatureClient(() => _repository.GetProvider(name), name, version, logger, context); /// /// Appends list of hooks to global hooks list @@ -258,6 +258,7 @@ public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler) public void SetLogger(ILogger logger) { this._eventExecutor.SetLogger(logger); + this._repository.SetLogger(logger); } internal void AddClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler) @@ -265,5 +266,38 @@ internal void AddClientHandler(string client, ProviderEventTypes eventType, Even internal void RemoveClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler) => this._eventExecutor.RemoveClientHandler(client, eventType, handler); + + /// + /// Update the provider state to READY and emit a READY event after successful init. + /// + private async Task AfterInitialization(FeatureProvider provider) + { + provider.Status = ProviderStatus.Ready; + var eventPayload = new ProviderEventPayload + { + Type = ProviderEventTypes.ProviderReady, + Message = "Provider initialization complete", + ProviderName = provider.GetMetadata().Name, + }; + + await this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false); + } + + /// + /// Update the provider state to ERROR and emit an ERROR after failed init. + /// + private async Task AfterError(FeatureProvider provider, Exception ex) + + { + provider.Status = typeof(ProviderFatalException) == ex.GetType() ? ProviderStatus.Fatal : ProviderStatus.Error; + var eventPayload = new ProviderEventPayload + { + Type = ProviderEventTypes.ProviderError, + Message = $"Provider initialization error: {ex?.Message}", + ProviderName = provider.GetMetadata()?.Name, + }; + + await this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false); + } } } diff --git a/src/OpenFeature/Constant/ErrorType.cs b/src/OpenFeature/Constant/ErrorType.cs index 232a57cb..4660e41a 100644 --- a/src/OpenFeature/Constant/ErrorType.cs +++ b/src/OpenFeature/Constant/ErrorType.cs @@ -47,5 +47,10 @@ public enum ErrorType /// Context does not contain a targeting key and the provider requires one. /// [Description("TARGETING_KEY_MISSING")] TargetingKeyMissing, + + /// + /// The provider has entered an irrecoverable error state. + /// + [Description("PROVIDER_FATAL")] ProviderFatal, } } diff --git a/src/OpenFeature/Constant/ProviderStatus.cs b/src/OpenFeature/Constant/ProviderStatus.cs index e56c6c95..16dbd024 100644 --- a/src/OpenFeature/Constant/ProviderStatus.cs +++ b/src/OpenFeature/Constant/ProviderStatus.cs @@ -26,6 +26,11 @@ public enum ProviderStatus /// /// The provider is in an error state and unable to evaluate flags. /// - [Description("ERROR")] Error + [Description("ERROR")] Error, + + /// + /// The provider has entered an irrecoverable error state. + /// + [Description("FATAL")] Fatal, } } diff --git a/src/OpenFeature/Error/ProviderFatalException.cs b/src/OpenFeature/Error/ProviderFatalException.cs new file mode 100644 index 00000000..fae8712a --- /dev/null +++ b/src/OpenFeature/Error/ProviderFatalException.cs @@ -0,0 +1,23 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using OpenFeature.Constant; + +namespace OpenFeature.Error +{ + /// the + /// An exception that signals the provider has entered an irrecoverable error state. + /// + [ExcludeFromCodeCoverage] + public class ProviderFatalException : FeatureProviderException + { + /// + /// Initialize a new instance of the class + /// + /// Exception message + /// Optional inner exception + public ProviderFatalException(string? message = null, Exception? innerException = null) + : base(ErrorType.ProviderFatal, message, innerException) + { + } + } +} diff --git a/src/OpenFeature/Error/ProviderNotReadyException.cs b/src/OpenFeature/Error/ProviderNotReadyException.cs index ca509692..b66201d7 100644 --- a/src/OpenFeature/Error/ProviderNotReadyException.cs +++ b/src/OpenFeature/Error/ProviderNotReadyException.cs @@ -5,7 +5,7 @@ namespace OpenFeature.Error { /// - /// Provider has yet been initialized when evaluating a flag. + /// Provider has not yet been initialized when evaluating a flag. /// [ExcludeFromCodeCoverage] public class ProviderNotReadyException : FeatureProviderException diff --git a/src/OpenFeature/EventExecutor.cs b/src/OpenFeature/EventExecutor.cs index 886a47b6..5dfd7dbe 100644 --- a/src/OpenFeature/EventExecutor.cs +++ b/src/OpenFeature/EventExecutor.cs @@ -184,7 +184,7 @@ private void EmitOnRegistration(FeatureProvider? provider, ProviderEventTypes ev { return; } - var status = provider.GetStatus(); + var status = provider.Status; var message = ""; if (status == ProviderStatus.Ready && eventType == ProviderEventTypes.ProviderReady) @@ -234,6 +234,7 @@ private async void ProcessFeatureProviderEventsAsync(object? providerRef) switch (item) { case ProviderEventPayload eventPayload: + this.UpdateProviderStatus(typedProviderRef, eventPayload); await this.EventChannel.Writer.WriteAsync(new Event { Provider = typedProviderRef, EventPayload = eventPayload }).ConfigureAwait(false); break; } @@ -307,6 +308,24 @@ private async void ProcessEventAsync() } } + // map events to provider status as per spec: https://openfeature.dev/specification/sections/events/#requirement-535 + private void UpdateProviderStatus(FeatureProvider provider, ProviderEventPayload eventPayload) + { + switch (eventPayload.Type) + { + case ProviderEventTypes.ProviderReady: + provider.Status = ProviderStatus.Ready; + break; + case ProviderEventTypes.ProviderStale: + provider.Status = ProviderStatus.Stale; + break; + case ProviderEventTypes.ProviderError: + provider.Status = eventPayload.ErrorType == ErrorType.ProviderFatal ? ProviderStatus.Fatal : ProviderStatus.Error; + break; + default: break; + } + } + private void InvokeEventHandler(EventHandlerDelegate eventHandler, Event e) { try diff --git a/src/OpenFeature/FeatureProvider.cs b/src/OpenFeature/FeatureProvider.cs index 62976f53..32635d95 100644 --- a/src/OpenFeature/FeatureProvider.cs +++ b/src/OpenFeature/FeatureProvider.cs @@ -1,10 +1,12 @@ using System.Collections.Immutable; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Channels; using System.Threading.Tasks; using OpenFeature.Constant; using OpenFeature.Model; +[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] // required to allow NSubstitute mocking of internal methods namespace OpenFeature { /// @@ -94,22 +96,17 @@ public abstract Task> ResolveStructureValueAsync(string EvaluationContext? context = null, CancellationToken cancellationToken = default); /// - /// Get the status of the provider. + /// Internally-managed provider status. + /// The SDK uses this field to track the status of the provider. + /// Not visible outside OpenFeature assembly /// - /// The current - /// - /// If a provider does not override this method, then its status will be assumed to be - /// . If a provider implements this method, and supports initialization, - /// then it should start in the status . If the status is - /// , then the Api will call the when the - /// provider is set. - /// - public virtual ProviderStatus GetStatus() => ProviderStatus.Ready; + internal virtual ProviderStatus Status { get; set; } = ProviderStatus.NotReady; /// /// /// This method is called before a provider is used to evaluate flags. Providers can overwrite this method, /// if they have special initialization needed prior being called for flag evaluation. + /// When this method completes, the provider will be considered ready for use. /// /// /// @@ -117,12 +114,7 @@ public abstract Task> ResolveStructureValueAsync(string /// A task that completes when the initialization process is complete. /// /// - /// A provider which supports initialization should override this method as well as - /// . - /// - /// - /// The provider should return or from - /// the method after initialization is complete. + /// Providers not implementing this method will be considered ready immediately. /// /// public virtual Task InitializeAsync(EvaluationContext context, CancellationToken cancellationToken = default) diff --git a/src/OpenFeature/IFeatureClient.cs b/src/OpenFeature/IFeatureClient.cs index 4a09c5e8..f39b7f52 100644 --- a/src/OpenFeature/IFeatureClient.cs +++ b/src/OpenFeature/IFeatureClient.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using OpenFeature.Constant; using OpenFeature.Model; namespace OpenFeature @@ -53,6 +54,12 @@ public interface IFeatureClient : IEventBus /// Client metadata ClientMetadata GetMetadata(); + /// + /// Returns the current status of the associated provider. + /// + /// + ProviderStatus ProviderStatus { get; } + /// /// Resolves a boolean feature flag /// diff --git a/src/OpenFeature/Model/ProviderEvents.cs b/src/OpenFeature/Model/ProviderEvents.cs index 5c48fc19..bdae057e 100644 --- a/src/OpenFeature/Model/ProviderEvents.cs +++ b/src/OpenFeature/Model/ProviderEvents.cs @@ -28,6 +28,11 @@ public class ProviderEventPayload /// public string? Message { get; set; } + /// + /// Optional error associated with the event. + /// + public ErrorType? ErrorType { get; set; } + /// /// A List of flags that have been changed. /// diff --git a/src/OpenFeature/OpenFeatureClient.cs b/src/OpenFeature/OpenFeatureClient.cs index 674b78a7..767e8b11 100644 --- a/src/OpenFeature/OpenFeatureClient.cs +++ b/src/OpenFeature/OpenFeatureClient.cs @@ -21,6 +21,7 @@ public sealed partial class FeatureClient : IFeatureClient private readonly ClientMetadata _metadata; private readonly ConcurrentStack _hooks = new ConcurrentStack(); private readonly ILogger _logger; + private readonly Func _providerAccessor; private EvaluationContext _evaluationContext; private readonly object _evaluationContextLock = new object(); @@ -48,6 +49,9 @@ public sealed partial class FeatureClient : IFeatureClient return (method(provider), provider); } + /// + public ProviderStatus ProviderStatus => this._providerAccessor.Invoke().Status; + /// public EvaluationContext GetContext() { @@ -69,16 +73,18 @@ public void SetContext(EvaluationContext? context) /// /// Initializes a new instance of the class. /// + /// Function to retrieve current provider /// Name of client /// Version of client /// Logger used by client /// Context given to this client /// Throws if any of the required parameters are null - public FeatureClient(string? name, string? version, ILogger? logger = null, EvaluationContext? context = null) + internal FeatureClient(Func providerAccessor, string? name, string? version, ILogger? logger = null, EvaluationContext? context = null) { this._metadata = new ClientMetadata(name, version); this._logger = logger ?? NullLogger.Instance; this._evaluationContext = context ?? EvaluationContext.Empty; + this._providerAccessor = providerAccessor; } /// @@ -246,6 +252,16 @@ private async Task> EvaluateFlagAsync( { var contextFromHooks = await this.TriggerBeforeHooksAsync(allHooks, hookContext, options, cancellationToken).ConfigureAwait(false); + // short circuit evaluation entirely if provider is in a bad state + if (provider.Status == ProviderStatus.NotReady) + { + throw new ProviderNotReadyException("Provider has not yet completed initialization."); + } + else if (provider.Status == ProviderStatus.Fatal) + { + throw new ProviderFatalException("Provider is in an irrecoverable error state."); + } + evaluation = (await resolveValueDelegate.Invoke(flagKey, defaultValue, contextFromHooks.EvaluationContext, cancellationToken).ConfigureAwait(false)) .ToFlagEvaluationDetails(); diff --git a/src/OpenFeature/ProviderRepository.cs b/src/OpenFeature/ProviderRepository.cs index 7934da1c..1656fdd3 100644 --- a/src/OpenFeature/ProviderRepository.cs +++ b/src/OpenFeature/ProviderRepository.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using OpenFeature.Constant; using OpenFeature.Model; @@ -14,6 +16,8 @@ namespace OpenFeature /// internal sealed class ProviderRepository : IAsyncDisposable { + private ILogger _logger; + private FeatureProvider _defaultProvider = new NoOpFeatureProvider(); private readonly ConcurrentDictionary _featureProviders = @@ -31,6 +35,11 @@ internal sealed class ProviderRepository : IAsyncDisposable /// of that provider under different names.. private readonly ReaderWriterLockSlim _providersLock = new ReaderWriterLockSlim(); + public ProviderRepository() + { + this._logger = NullLogger.Instance; + } + public async ValueTask DisposeAsync() { using (this._providersLock) @@ -39,36 +48,25 @@ public async ValueTask DisposeAsync() } } + internal void SetLogger(ILogger logger) => this._logger = logger; + /// /// Set the default provider /// /// the provider to set as the default, passing null has no effect /// the context to initialize the provider with - /// - /// - /// Called after the provider is set, but before any actions are taken on it. - /// - /// This can be used for tasks such as registering event handlers. It should be noted that this can be called - /// several times for a single provider. For instance registering a provider with multiple names or as the - /// default and named provider. - /// - /// - /// - /// + /// /// called after the provider has initialized successfully, only called if the provider needed initialization /// - /// + /// /// called if an error happens during the initialization of the provider, only called if the provider needed /// initialization /// - /// called after a provider is shutdown, can be used to remove event handlers public async Task SetProviderAsync( FeatureProvider? featureProvider, EvaluationContext context, - Action? afterSet = null, - Action? afterInitialization = null, - Action? afterError = null, - Action? afterShutdown = null) + Func? afterInitSuccess = null, + Func? afterInitError = null) { // Cannot unset the feature provider. if (featureProvider == null) @@ -88,42 +86,45 @@ public async Task SetProviderAsync( var oldProvider = this._defaultProvider; this._defaultProvider = featureProvider; - afterSet?.Invoke(featureProvider); // We want to allow shutdown to happen concurrently with initialization, and the caller to not // wait for it. -#pragma warning disable CS4014 - this.ShutdownIfUnusedAsync(oldProvider, afterShutdown, afterError); -#pragma warning restore CS4014 + _ = this.ShutdownIfUnusedAsync(oldProvider); } finally { this._providersLock.ExitWriteLock(); } - await InitProviderAsync(this._defaultProvider, context, afterInitialization, afterError) + await InitProviderAsync(this._defaultProvider, context, afterInitSuccess, afterInitError) .ConfigureAwait(false); } private static async Task InitProviderAsync( FeatureProvider? newProvider, EvaluationContext context, - Action? afterInitialization, - Action? afterError) + Func? afterInitialization, + Func? afterError) { if (newProvider == null) { return; } - if (newProvider.GetStatus() == ProviderStatus.NotReady) + if (newProvider.Status == ProviderStatus.NotReady) { try { await newProvider.InitializeAsync(context).ConfigureAwait(false); - afterInitialization?.Invoke(newProvider); + if (afterInitialization != null) + { + await afterInitialization.Invoke(newProvider).ConfigureAwait(false); + } } catch (Exception ex) { - afterError?.Invoke(newProvider, ex); + if (afterError != null) + { + await afterError.Invoke(newProvider, ex).ConfigureAwait(false); + } } } } @@ -134,32 +135,19 @@ private static async Task InitProviderAsync( /// the name to associate with the provider /// the provider to set as the default, passing null has no effect /// the context to initialize the provider with - /// - /// - /// Called after the provider is set, but before any actions are taken on it. - /// - /// This can be used for tasks such as registering event handlers. It should be noted that this can be called - /// several times for a single provider. For instance registering a provider with multiple names or as the - /// default and named provider. - /// - /// - /// - /// + /// /// called after the provider has initialized successfully, only called if the provider needed initialization /// - /// + /// /// called if an error happens during the initialization of the provider, only called if the provider needed /// initialization /// - /// called after a provider is shutdown, can be used to remove event handlers /// The to cancel any async side effects. - public async Task SetProviderAsync(string clientName, + public async Task SetProviderAsync(string? clientName, FeatureProvider? featureProvider, EvaluationContext context, - Action? afterSet = null, - Action? afterInitialization = null, - Action? afterError = null, - Action? afterShutdown = null, + Func? afterInitSuccess = null, + Func? afterInitError = null, CancellationToken cancellationToken = default) { // Cannot set a provider for a null clientName. @@ -177,7 +165,6 @@ public async Task SetProviderAsync(string clientName, { this._featureProviders.AddOrUpdate(clientName, featureProvider, (key, current) => featureProvider); - afterSet?.Invoke(featureProvider); } else { @@ -188,25 +175,21 @@ public async Task SetProviderAsync(string clientName, // We want to allow shutdown to happen concurrently with initialization, and the caller to not // wait for it. -#pragma warning disable CS4014 - this.ShutdownIfUnusedAsync(oldProvider, afterShutdown, afterError); -#pragma warning restore CS4014 + _ = this.ShutdownIfUnusedAsync(oldProvider); } finally { this._providersLock.ExitWriteLock(); } - await InitProviderAsync(featureProvider, context, afterInitialization, afterError).ConfigureAwait(false); + await InitProviderAsync(featureProvider, context, afterInitSuccess, afterInitError).ConfigureAwait(false); } /// /// Shutdown the feature provider if it is unused. This must be called within a write lock of the _providersLock. /// private async Task ShutdownIfUnusedAsync( - FeatureProvider? targetProvider, - Action? afterShutdown, - Action? afterError) + FeatureProvider? targetProvider) { if (ReferenceEquals(this._defaultProvider, targetProvider)) { @@ -218,7 +201,7 @@ private async Task ShutdownIfUnusedAsync( return; } - await SafeShutdownProviderAsync(targetProvider, afterShutdown, afterError).ConfigureAwait(false); + await SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false); } /// @@ -230,9 +213,7 @@ private async Task ShutdownIfUnusedAsync( /// it would not be meaningful to emit an error. /// /// - private static async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider, - Action? afterShutdown, - Action? afterError) + private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider) { if (targetProvider == null) { @@ -242,11 +223,10 @@ private static async Task SafeShutdownProviderAsync(FeatureProvider? targetProvi try { await targetProvider.ShutdownAsync().ConfigureAwait(false); - afterShutdown?.Invoke(targetProvider); } catch (Exception ex) { - afterError?.Invoke(targetProvider, ex); + this._logger.LogError(ex, $"Error shutting down provider: {targetProvider.GetMetadata().Name}"); } } @@ -307,7 +287,7 @@ public async Task ShutdownAsync(Action? afterError = foreach (var targetProvider in providers) { // We don't need to take any actions after shutdown. - await SafeShutdownProviderAsync(targetProvider, null, afterError).ConfigureAwait(false); + await SafeShutdownProviderAsync(targetProvider).ConfigureAwait(false); } } } diff --git a/test/OpenFeature.Tests/OpenFeatureClientTests.cs b/test/OpenFeature.Tests/OpenFeatureClientTests.cs index e7c76d75..925de66a 100644 --- a/test/OpenFeature.Tests/OpenFeatureClientTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureClientTests.cs @@ -185,6 +185,92 @@ public async Task OpenFeatureClient_Should_Return_DefaultValue_When_Type_Mismatc mockedLogger.Received(1).IsEnabled(LogLevel.Error); } + [Fact] + [Specification("1.7.3", "The client's provider status accessor MUST indicate READY if the initialize function of the associated provider terminates normally.")] + [Specification("1.7.1", "The client MUST define a provider status accessor which indicates the readiness of the associated provider, with possible values NOT_READY, READY, STALE, ERROR, or FATAL.")] + public async Task Provider_Status_Should_Be_Ready_If_Init_Succeeds() + { + var name = "1.7.3"; + // provider which succeeds initialization + var provider = new TestProvider(); + FeatureClient client = Api.Instance.GetClient(name); + Assert.Equal(ProviderStatus.NotReady, provider.Status); + await Api.Instance.SetProviderAsync(name, provider); + + // after init fails fatally, status should be READY + Assert.Equal(ProviderStatus.Ready, client.ProviderStatus); + } + + [Fact] + [Specification("1.7.4", "The client's provider status accessor MUST indicate ERROR if the initialize function of the associated provider terminates abnormally.")] + [Specification("1.7.1", "The client MUST define a provider status accessor which indicates the readiness of the associated provider, with possible values NOT_READY, READY, STALE, ERROR, or FATAL.")] + public async Task Provider_Status_Should_Be_Error_If_Init_Fails() + { + var name = "1.7.4"; + // provider which fails initialization + var provider = new TestProvider("some-name", new GeneralException("fake")); + FeatureClient client = Api.Instance.GetClient(name); + Assert.Equal(ProviderStatus.NotReady, provider.Status); + await Api.Instance.SetProviderAsync(name, provider); + + // after init fails fatally, status should be ERROR + Assert.Equal(ProviderStatus.Error, client.ProviderStatus); + } + + [Fact] + [Specification("1.7.5", "The client's provider status accessor MUST indicate FATAL if the initialize function of the associated provider terminates abnormally and indicates error code PROVIDER_FATAL.")] + [Specification("1.7.1", "The client MUST define a provider status accessor which indicates the readiness of the associated provider, with possible values NOT_READY, READY, STALE, ERROR, or FATAL.")] + public async Task Provider_Status_Should_Be_Fatal_If_Init_Fatal() + { + var name = "1.7.5"; + // provider which fails initialization fatally + var provider = new TestProvider(name, new ProviderFatalException("fatal")); + FeatureClient client = Api.Instance.GetClient(name); + Assert.Equal(ProviderStatus.NotReady, provider.Status); + await Api.Instance.SetProviderAsync(name, provider); + + // after init fails fatally, status should be FATAL + Assert.Equal(ProviderStatus.Fatal, client.ProviderStatus); + } + + [Fact] + [Specification("1.7.6", "The client MUST default, run error hooks, and indicate an error if flag resolution is attempted while the provider is in NOT_READY.")] + public async Task Must_Short_Circuit_Not_Ready() + { + var name = "1.7.6"; + var defaultStr = "123-default"; + + // provider which is never ready (ready after maxValue) + var provider = new TestProvider(name, null, int.MaxValue); + FeatureClient client = Api.Instance.GetClient(name); + Assert.Equal(ProviderStatus.NotReady, provider.Status); + _ = Api.Instance.SetProviderAsync(name, provider); + + var details = await client.GetStringDetailsAsync("some-flag", defaultStr); + Assert.Equal(defaultStr, details.Value); + Assert.Equal(ErrorType.ProviderNotReady, details.ErrorType); + Assert.Equal(Reason.Error, details.Reason); + } + + [Fact] + [Specification("1.7.7", "The client MUST default, run error hooks, and indicate an error if flag resolution is attempted while the provider is in NOT_READY.")] + public async Task Must_Short_Circuit_Fatal() + { + var name = "1.7.6"; + var defaultStr = "456-default"; + + // provider which immediately fails fatally + var provider = new TestProvider(name, new ProviderFatalException("fake")); + FeatureClient client = Api.Instance.GetClient(name); + Assert.Equal(ProviderStatus.NotReady, provider.Status); + _ = Api.Instance.SetProviderAsync(name, provider); + + var details = await client.GetStringDetailsAsync("some-flag", defaultStr); + Assert.Equal(defaultStr, details.Value); + Assert.Equal(ErrorType.ProviderFatal, details.ErrorType); + Assert.Equal(Reason.Error, details.Reason); + } + [Fact] public async Task Should_Resolve_BooleanValue() { @@ -358,15 +444,15 @@ public async Task Cancellation_Token_Added_Is_Passed_To_Provider() var cts = new CancellationTokenSource(); - var featureProviderMock = Substitute.For(); - featureProviderMock.ResolveStringValueAsync(flagName, defaultString, Arg.Any(), Arg.Any()).Returns(async args => + var featureProviderMock = Substitute.For(); + featureProviderMock.ResolveStringValueAsync(flagName, defaultString, Arg.Any(), Arg.Any()).Returns(async args => { var token = args.ArgAt(3); - while (!token.IsCancellationRequested) + while (!token.IsCancellationRequested) { await Task.Delay(10); // artificially delay until cancelled } - return new ResolutionDetails(flagName, defaultString, ErrorType.None, cancelledReason); + return new ResolutionDetails(flagName, defaultString, ErrorType.None, cancelledReason); }); featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create())); featureProviderMock.GetProviderHooks().Returns(ImmutableList.Empty); diff --git a/test/OpenFeature.Tests/OpenFeatureEventTests.cs b/test/OpenFeature.Tests/OpenFeatureEventTests.cs index 384928d6..a7bcd2e7 100644 --- a/test/OpenFeature.Tests/OpenFeatureEventTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureEventTests.cs @@ -147,9 +147,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_ var eventHandler = Substitute.For(); var testProvider = new TestProvider(); -#pragma warning disable CS0618// Type or member is obsolete await Api.Instance.SetProviderAsync(testProvider); -#pragma warning restore CS0618// Type or member is obsolete Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler); @@ -175,7 +173,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Error_State_ var testProvider = new TestProvider(); await Api.Instance.SetProviderAsync(testProvider); - testProvider.SetStatus(ProviderStatus.Error); + testProvider.Status = ProviderStatus.Error; Api.Instance.AddHandler(ProviderEventTypes.ProviderError, eventHandler); @@ -200,7 +198,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Stale_State_ var testProvider = new TestProvider(); await Api.Instance.SetProviderAsync(testProvider); - testProvider.SetStatus(ProviderStatus.Stale); + testProvider.Status = ProviderStatus.Stale; Api.Instance.AddHandler(ProviderEventTypes.ProviderStale, eventHandler); @@ -476,7 +474,11 @@ public async Task Client_Level_Event_Handlers_Should_Be_Removable() await Api.Instance.SetProviderAsync(myClient.GetMetadata().Name!, testProvider); // wait for the first event to be received - await Utils.AssertUntilAsync(_ => myClient.RemoveHandler(ProviderEventTypes.ProviderReady, eventHandler)); + await Utils.AssertUntilAsync( + _ => eventHandler.Received(1).Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)) + ); + + myClient.RemoveHandler(ProviderEventTypes.ProviderReady, eventHandler); // send another event from the provider - this one should not be received await testProvider.SendEventAsync(ProviderEventTypes.ProviderReady); @@ -501,5 +503,18 @@ public void RegisterClientFeatureProvider_WhenCalledWithNullProvider_DoesNotThro // Assert Assert.Null(exception); } + + [Theory] + [InlineData(ProviderEventTypes.ProviderError, ProviderStatus.Error)] + [InlineData(ProviderEventTypes.ProviderReady, ProviderStatus.Ready)] + [InlineData(ProviderEventTypes.ProviderStale, ProviderStatus.Stale)] + [Specification("5.3.5", "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.")] + public async Task Provider_Events_Should_Update_ProviderStatus(ProviderEventTypes type, ProviderStatus status) + { + var provider = new TestProvider(); + await Api.Instance.SetProviderAsync("5.3.5", provider); + _ = provider.SendEventAsync(type); + await Utils.AssertUntilAsync(_ => Assert.True(provider.Status == status)); + } } } diff --git a/test/OpenFeature.Tests/OpenFeatureTests.cs b/test/OpenFeature.Tests/OpenFeatureTests.cs index 673c183d..1df3c976 100644 --- a/test/OpenFeature.Tests/OpenFeatureTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureTests.cs @@ -28,13 +28,13 @@ public void OpenFeature_Should_Be_Singleton() public async Task OpenFeature_Should_Initialize_Provider() { var providerMockDefault = Substitute.For(); - providerMockDefault.GetStatus().Returns(ProviderStatus.NotReady); + providerMockDefault.Status.Returns(ProviderStatus.NotReady); await Api.Instance.SetProviderAsync(providerMockDefault); await providerMockDefault.Received(1).InitializeAsync(Api.Instance.GetContext()); var providerMockNamed = Substitute.For(); - providerMockNamed.GetStatus().Returns(ProviderStatus.NotReady); + providerMockNamed.Status.Returns(ProviderStatus.NotReady); await Api.Instance.SetProviderAsync("the-name", providerMockNamed); await providerMockNamed.Received(1).InitializeAsync(Api.Instance.GetContext()); @@ -46,26 +46,26 @@ public async Task OpenFeature_Should_Initialize_Provider() public async Task OpenFeature_Should_Shutdown_Unused_Provider() { var providerA = Substitute.For(); - providerA.GetStatus().Returns(ProviderStatus.NotReady); + providerA.Status.Returns(ProviderStatus.NotReady); await Api.Instance.SetProviderAsync(providerA); await providerA.Received(1).InitializeAsync(Api.Instance.GetContext()); var providerB = Substitute.For(); - providerB.GetStatus().Returns(ProviderStatus.NotReady); + providerB.Status.Returns(ProviderStatus.NotReady); await Api.Instance.SetProviderAsync(providerB); await providerB.Received(1).InitializeAsync(Api.Instance.GetContext()); await providerA.Received(1).ShutdownAsync(); var providerC = Substitute.For(); - providerC.GetStatus().Returns(ProviderStatus.NotReady); + providerC.Status.Returns(ProviderStatus.NotReady); await Api.Instance.SetProviderAsync("named", providerC); await providerC.Received(1).InitializeAsync(Api.Instance.GetContext()); var providerD = Substitute.For(); - providerD.GetStatus().Returns(ProviderStatus.NotReady); + providerD.Status.Returns(ProviderStatus.NotReady); await Api.Instance.SetProviderAsync("named", providerD); await providerD.Received(1).InitializeAsync(Api.Instance.GetContext()); @@ -77,10 +77,10 @@ public async Task OpenFeature_Should_Shutdown_Unused_Provider() public async Task OpenFeature_Should_Support_Shutdown() { var providerA = Substitute.For(); - providerA.GetStatus().Returns(ProviderStatus.NotReady); + providerA.Status.Returns(ProviderStatus.NotReady); var providerB = Substitute.For(); - providerB.GetStatus().Returns(ProviderStatus.NotReady); + providerB.Status.Returns(ProviderStatus.NotReady); await Api.Instance.SetProviderAsync(providerA); await Api.Instance.SetProviderAsync("named", providerB); diff --git a/test/OpenFeature.Tests/ProviderRepositoryTests.cs b/test/OpenFeature.Tests/ProviderRepositoryTests.cs index ccec89bd..e88de6e9 100644 --- a/test/OpenFeature.Tests/ProviderRepositoryTests.cs +++ b/test/OpenFeature.Tests/ProviderRepositoryTests.cs @@ -2,7 +2,6 @@ using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; using NSubstitute; -using NSubstitute.ExceptionExtensions; using OpenFeature.Constant; using OpenFeature.Model; using Xunit; @@ -25,28 +24,12 @@ public async Task Default_Provider_Is_Set_Without_Await() Assert.Equal(provider, repository.GetProvider()); } - [Fact] - public async Task AfterSet_Is_Invoked_For_Setting_Default_Provider() - { - var repository = new ProviderRepository(); - var provider = new NoOpFeatureProvider(); - var context = new EvaluationContextBuilder().Build(); - var callCount = 0; - // The setting of the provider is synchronous, so the afterSet should be as well. - await repository.SetProviderAsync(provider, context, afterSet: (theProvider) => - { - callCount++; - Assert.Equal(provider, theProvider); - }); - Assert.Equal(1, callCount); - } - [Fact] public async Task Initialization_Provider_Method_Is_Invoked_For_Setting_Default_Provider() { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(ProviderStatus.NotReady); + providerMock.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync(providerMock, context); providerMock.Received(1).InitializeAsync(context); @@ -58,13 +41,14 @@ public async Task AfterInitialization_Is_Invoked_For_Setting_Default_Provider() { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(ProviderStatus.NotReady); + providerMock.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); var callCount = 0; - await repository.SetProviderAsync(providerMock, context, afterInitialization: (theProvider) => + await repository.SetProviderAsync(providerMock, context, afterInitSuccess: (theProvider) => { Assert.Equal(providerMock, theProvider); callCount++; + return Task.CompletedTask; }); Assert.Equal(1, callCount); } @@ -74,16 +58,17 @@ public async Task AfterError_Is_Invoked_If_Initialization_Errors_Default_Provide { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(ProviderStatus.NotReady); + providerMock.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); providerMock.When(x => x.InitializeAsync(context)).Throw(new Exception("BAD THINGS")); var callCount = 0; Exception? receivedError = null; - await repository.SetProviderAsync(providerMock, context, afterError: (theProvider, error) => + await repository.SetProviderAsync(providerMock, context, afterInitError: (theProvider, error) => { Assert.Equal(providerMock, theProvider); callCount++; receivedError = error; + return Task.CompletedTask; }); Assert.Equal("BAD THINGS", receivedError?.Message); Assert.Equal(1, callCount); @@ -93,11 +78,11 @@ await repository.SetProviderAsync(providerMock, context, afterError: (theProvide [InlineData(ProviderStatus.Ready)] [InlineData(ProviderStatus.Stale)] [InlineData(ProviderStatus.Error)] - public async Task Initialize_Is_Not_Called_For_Ready_Provider(ProviderStatus status) + internal async Task Initialize_Is_Not_Called_For_Ready_Provider(ProviderStatus status) { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(status); + providerMock.Status.Returns(status); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync(providerMock, context); providerMock.DidNotReceive().InitializeAsync(context); @@ -107,14 +92,18 @@ public async Task Initialize_Is_Not_Called_For_Ready_Provider(ProviderStatus sta [InlineData(ProviderStatus.Ready)] [InlineData(ProviderStatus.Stale)] [InlineData(ProviderStatus.Error)] - public async Task AfterInitialize_Is_Not_Called_For_Ready_Provider(ProviderStatus status) + internal async Task AfterInitialize_Is_Not_Called_For_Ready_Provider(ProviderStatus status) { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(status); + providerMock.Status.Returns(status); var context = new EvaluationContextBuilder().Build(); var callCount = 0; - await repository.SetProviderAsync(providerMock, context, afterInitialization: provider => { callCount++; }); + await repository.SetProviderAsync(providerMock, context, afterInitSuccess: provider => + { + callCount++; + return Task.CompletedTask; + }); Assert.Equal(0, callCount); } @@ -123,10 +112,10 @@ public async Task Replaced_Default_Provider_Is_Shutdown() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync(provider1, context); @@ -135,52 +124,6 @@ public async Task Replaced_Default_Provider_Is_Shutdown() provider2.DidNotReceive().ShutdownAsync(); } - [Fact] - public async Task AfterShutdown_Is_Called_For_Shutdown_Provider() - { - var repository = new ProviderRepository(); - var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); - - var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); - - var context = new EvaluationContextBuilder().Build(); - await repository.SetProviderAsync(provider1, context); - var callCount = 0; - await repository.SetProviderAsync(provider2, context, afterShutdown: provider => - { - Assert.Equal(provider, provider1); - callCount++; - }); - Assert.Equal(1, callCount); - } - - [Fact] - public async Task AfterError_Is_Called_For_Shutdown_That_Throws() - { - var repository = new ProviderRepository(); - var provider1 = Substitute.For(); - provider1.ShutdownAsync().Throws(new Exception("SHUTDOWN ERROR")); - provider1.GetStatus().Returns(ProviderStatus.NotReady); - - var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); - - var context = new EvaluationContextBuilder().Build(); - await repository.SetProviderAsync(provider1, context); - var callCount = 0; - Exception? errorThrown = null; - await repository.SetProviderAsync(provider2, context, afterError: (provider, ex) => - { - Assert.Equal(provider, provider1); - errorThrown = ex; - callCount++; - }); - Assert.Equal(1, callCount); - Assert.Equal("SHUTDOWN ERROR", errorThrown?.Message); - } - [Fact] public async Task Named_Provider_Provider_Is_Set_Without_Await() { @@ -192,28 +135,12 @@ public async Task Named_Provider_Provider_Is_Set_Without_Await() Assert.Equal(provider, repository.GetProvider("the-name")); } - [Fact] - public async Task AfterSet_Is_Invoked_For_Setting_Named_Provider() - { - var repository = new ProviderRepository(); - var provider = new NoOpFeatureProvider(); - var context = new EvaluationContextBuilder().Build(); - var callCount = 0; - // The setting of the provider is synchronous, so the afterSet should be as well. - await repository.SetProviderAsync("the-name", provider, context, afterSet: (theProvider) => - { - callCount++; - Assert.Equal(provider, theProvider); - }); - Assert.Equal(1, callCount); - } - [Fact] public async Task Initialization_Provider_Method_Is_Invoked_For_Setting_Named_Provider() { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(ProviderStatus.NotReady); + providerMock.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync("the-name", providerMock, context); providerMock.Received(1).InitializeAsync(context); @@ -225,13 +152,14 @@ public async Task AfterInitialization_Is_Invoked_For_Setting_Named_Provider() { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(ProviderStatus.NotReady); + providerMock.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); var callCount = 0; - await repository.SetProviderAsync("the-name", providerMock, context, afterInitialization: (theProvider) => + await repository.SetProviderAsync("the-name", providerMock, context, afterInitSuccess: (theProvider) => { Assert.Equal(providerMock, theProvider); callCount++; + return Task.CompletedTask; }); Assert.Equal(1, callCount); } @@ -241,16 +169,17 @@ public async Task AfterError_Is_Invoked_If_Initialization_Errors_Named_Provider( { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(ProviderStatus.NotReady); + providerMock.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); providerMock.When(x => x.InitializeAsync(context)).Throw(new Exception("BAD THINGS")); var callCount = 0; Exception? receivedError = null; - await repository.SetProviderAsync("the-provider", providerMock, context, afterError: (theProvider, error) => + await repository.SetProviderAsync("the-provider", providerMock, context, afterInitError: (theProvider, error) => { Assert.Equal(providerMock, theProvider); callCount++; receivedError = error; + return Task.CompletedTask; }); Assert.Equal("BAD THINGS", receivedError?.Message); Assert.Equal(1, callCount); @@ -260,11 +189,11 @@ await repository.SetProviderAsync("the-provider", providerMock, context, afterEr [InlineData(ProviderStatus.Ready)] [InlineData(ProviderStatus.Stale)] [InlineData(ProviderStatus.Error)] - public async Task Initialize_Is_Not_Called_For_Ready_Named_Provider(ProviderStatus status) + internal async Task Initialize_Is_Not_Called_For_Ready_Named_Provider(ProviderStatus status) { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(status); + providerMock.Status.Returns(status); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync("the-name", providerMock, context); providerMock.DidNotReceive().InitializeAsync(context); @@ -274,15 +203,19 @@ public async Task Initialize_Is_Not_Called_For_Ready_Named_Provider(ProviderStat [InlineData(ProviderStatus.Ready)] [InlineData(ProviderStatus.Stale)] [InlineData(ProviderStatus.Error)] - public async Task AfterInitialize_Is_Not_Called_For_Ready_Named_Provider(ProviderStatus status) + internal async Task AfterInitialize_Is_Not_Called_For_Ready_Named_Provider(ProviderStatus status) { var repository = new ProviderRepository(); var providerMock = Substitute.For(); - providerMock.GetStatus().Returns(status); + providerMock.Status.Returns(status); var context = new EvaluationContextBuilder().Build(); var callCount = 0; await repository.SetProviderAsync("the-name", providerMock, context, - afterInitialization: provider => { callCount++; }); + afterInitSuccess: provider => + { + callCount++; + return Task.CompletedTask; + }); Assert.Equal(0, callCount); } @@ -291,10 +224,10 @@ public async Task Replaced_Named_Provider_Is_Shutdown() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync("the-name", provider1, context); @@ -303,61 +236,15 @@ public async Task Replaced_Named_Provider_Is_Shutdown() provider2.DidNotReceive().ShutdownAsync(); } - [Fact] - public async Task AfterShutdown_Is_Called_For_Shutdown_Named_Provider() - { - var repository = new ProviderRepository(); - var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); - - var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); - - var context = new EvaluationContextBuilder().Build(); - await repository.SetProviderAsync("the-provider", provider1, context); - var callCount = 0; - await repository.SetProviderAsync("the-provider", provider2, context, afterShutdown: provider => - { - Assert.Equal(provider, provider1); - callCount++; - }); - Assert.Equal(1, callCount); - } - - [Fact] - public async Task AfterError_Is_Called_For_Shutdown_Named_Provider_That_Throws() - { - var repository = new ProviderRepository(); - var provider1 = Substitute.For(); - provider1.ShutdownAsync().Throws(new Exception("SHUTDOWN ERROR")); - provider1.GetStatus().Returns(ProviderStatus.NotReady); - - var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); - - var context = new EvaluationContextBuilder().Build(); - await repository.SetProviderAsync("the-name", provider1, context); - var callCount = 0; - Exception? errorThrown = null; - await repository.SetProviderAsync("the-name", provider2, context, afterError: (provider, ex) => - { - Assert.Equal(provider, provider1); - errorThrown = ex; - callCount++; - }); - Assert.Equal(1, callCount); - Assert.Equal("SHUTDOWN ERROR", errorThrown?.Message); - } - [Fact] public async Task In_Use_Provider_Named_And_Default_Is_Not_Shutdown() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); @@ -374,10 +261,10 @@ public async Task In_Use_Provider_Two_Named_Is_Not_Shutdown() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); @@ -394,10 +281,10 @@ public async Task When_All_Instances_Are_Removed_Shutdown_Is_Called() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); @@ -415,10 +302,10 @@ public async Task Can_Get_Providers_By_Name() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); @@ -434,10 +321,10 @@ public async Task Replaced_Named_Provider_Gets_Latest_Set() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); @@ -452,13 +339,13 @@ public async Task Can_Shutdown_All_Providers() { var repository = new ProviderRepository(); var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); + provider1.Status.Returns(ProviderStatus.NotReady); var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); + provider2.Status.Returns(ProviderStatus.NotReady); var provider3 = Substitute.For(); - provider3.GetStatus().Returns(ProviderStatus.NotReady); + provider3.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); @@ -475,62 +362,12 @@ public async Task Can_Shutdown_All_Providers() provider3.Received(1).ShutdownAsync(); } - [Fact] - public async Task Errors_During_Shutdown_Propagate() - { - var repository = new ProviderRepository(); - var provider1 = Substitute.For(); - provider1.GetStatus().Returns(ProviderStatus.NotReady); - provider1.ShutdownAsync().Throws(new Exception("SHUTDOWN ERROR 1")); - - var provider2 = Substitute.For(); - provider2.GetStatus().Returns(ProviderStatus.NotReady); - provider2.ShutdownAsync().Throws(new Exception("SHUTDOWN ERROR 2")); - - var provider3 = Substitute.For(); - provider3.GetStatus().Returns(ProviderStatus.NotReady); - - var context = new EvaluationContextBuilder().Build(); - - await repository.SetProviderAsync(provider1, context); - await repository.SetProviderAsync("provider1", provider1, context); - await repository.SetProviderAsync("provider2", provider2, context); - await repository.SetProviderAsync("provider2a", provider2, context); - await repository.SetProviderAsync("provider3", provider3, context); - - var callCountShutdown1 = 0; - var callCountShutdown2 = 0; - var totalCallCount = 0; - await repository.ShutdownAsync(afterError: (provider, exception) => - { - totalCallCount++; - if (provider == provider1) - { - callCountShutdown1++; - Assert.Equal("SHUTDOWN ERROR 1", exception.Message); - } - - if (provider == provider2) - { - callCountShutdown2++; - Assert.Equal("SHUTDOWN ERROR 2", exception.Message); - } - }); - Assert.Equal(2, totalCallCount); - Assert.Equal(1, callCountShutdown1); - Assert.Equal(1, callCountShutdown2); - - provider1.Received(1).ShutdownAsync(); - provider2.Received(1).ShutdownAsync(); - provider3.Received(1).ShutdownAsync(); - } - [Fact] public async Task Setting_Same_Default_Provider_Has_No_Effect() { var repository = new ProviderRepository(); var provider = Substitute.For(); - provider.GetStatus().Returns(ProviderStatus.NotReady); + provider.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync(provider, context); await repository.SetProviderAsync(provider, context); @@ -545,7 +382,7 @@ public async Task Setting_Null_Default_Provider_Has_No_Effect() { var repository = new ProviderRepository(); var provider = Substitute.For(); - provider.GetStatus().Returns(ProviderStatus.NotReady); + provider.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync(provider, context); await repository.SetProviderAsync(null, context); @@ -561,10 +398,10 @@ public async Task Setting_Null_Named_Provider_Removes_It() var repository = new ProviderRepository(); var namedProvider = Substitute.For(); - namedProvider.GetStatus().Returns(ProviderStatus.NotReady); + namedProvider.Status.Returns(ProviderStatus.NotReady); var defaultProvider = Substitute.For(); - defaultProvider.GetStatus().Returns(ProviderStatus.NotReady); + defaultProvider.Status.Returns(ProviderStatus.NotReady); var context = new EvaluationContextBuilder().Build(); await repository.SetProviderAsync(defaultProvider, context); diff --git a/test/OpenFeature.Tests/TestImplementations.cs b/test/OpenFeature.Tests/TestImplementations.cs index c949b373..a4fe51a4 100644 --- a/test/OpenFeature.Tests/TestImplementations.cs +++ b/test/OpenFeature.Tests/TestImplementations.cs @@ -40,24 +40,30 @@ public class TestProvider : FeatureProvider public static string DefaultName = "test-provider"; - public string Name { get; set; } - - private ProviderStatus _status; + public string? Name { get; set; } public void AddHook(Hook hook) => this._hooks.Add(hook); public override IImmutableList GetProviderHooks() => this._hooks.ToImmutableList(); + private Exception? initException = null; + private int initDelay = 0; public TestProvider() { - this._status = ProviderStatus.NotReady; this.Name = DefaultName; } - public TestProvider(string name) + /// + /// A provider used for testing. + /// + /// the name of the provider. + /// Optional exception to throw during init. + /// + public TestProvider(string? name, Exception? initException = null, int initDelay = 0) { - this._status = ProviderStatus.NotReady; - this.Name = name; + this.Name = string.IsNullOrEmpty(name) ? DefaultName : name; + this.initException = initException; + this.initDelay = initDelay; } public override Metadata GetMetadata() @@ -95,26 +101,23 @@ public override Task> ResolveStructureValueAsync(string return Task.FromResult(new ResolutionDetails(flagKey, defaultValue)); } - public override ProviderStatus GetStatus() - { - return this._status; - } - - public void SetStatus(ProviderStatus status) + public override async Task InitializeAsync(EvaluationContext context, CancellationToken cancellationToken = default) { - this._status = status; + await Task.Delay(initDelay).ConfigureAwait(false); + if (initException != null) + { + throw initException; + } } - public override async Task InitializeAsync(EvaluationContext context, CancellationToken cancellationToken = default) + internal ValueTask SendEventAsync(ProviderEventTypes eventType, CancellationToken cancellationToken = default) { - this._status = ProviderStatus.Ready; - await this.EventChannel.Writer.WriteAsync(new ProviderEventPayload { Type = ProviderEventTypes.ProviderReady, ProviderName = this.GetMetadata().Name }, cancellationToken).ConfigureAwait(false); - await base.InitializeAsync(context, cancellationToken).ConfigureAwait(false); + return this.EventChannel.Writer.WriteAsync(new ProviderEventPayload { Type = eventType, ProviderName = this.GetMetadata().Name, }, cancellationToken); } - internal ValueTask SendEventAsync(ProviderEventTypes eventType, CancellationToken cancellationToken = default) + internal ValueTask SendEventAsync(ProviderEventPayload payload, CancellationToken cancellationToken = default) { - return this.EventChannel.Writer.WriteAsync(new ProviderEventPayload { Type = eventType, ProviderName = this.GetMetadata().Name }, cancellationToken); + return this.EventChannel.Writer.WriteAsync(payload, cancellationToken); } } }