Skip to content

Commit

Permalink
fixup: review feedback
Browse files Browse the repository at this point in the history 
Signed-off-by: Todd Baert <[email protected]>
  • Loading branch information
@toddbaert
toddbaert committed Jun 28, 2024
1 parent 35644d9 commit 18311fb
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 19 deletions.
14 changes: 7 additions & 7 deletions src/OpenFeature/Api.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ private Api() { }
public async Task SetProviderAsync(FeatureProvider featureProvider)
{
this._eventExecutor.RegisterDefaultFeatureProvider(featureProvider);
await this._repository.SetProviderAsync(featureProvider, this.GetContext(), afterInitialization, afterError).ConfigureAwait(false);
await this._repository.SetProviderAsync(featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -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(), afterInitialization, afterError).ConfigureAwait(false);
await this._repository.SetProviderAsync(clientName, featureProvider, this.GetContext(), AfterInitialization, AfterError).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -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)
Expand All @@ -269,8 +270,7 @@ internal void RemoveClientHandler(string client, ProviderEventTypes eventType, E
/// <summary>
/// Update the provider state to READY and emit an READY after successful init.
/// </summary>
private void AfterInitialization(FeatureProvider provider)

private async Task AfterInitialization(FeatureProvider provider)
{
provider.Status = ProviderStatus.Ready;
var eventPayload = new ProviderEventPayload
Expand All @@ -280,13 +280,13 @@ private void AfterInitialization(FeatureProvider provider)
ProviderName = provider.GetMetadata().Name,
};

this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload });
await this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false);
}

/// <summary>
/// Update the provider state to ERROR and emit an ERROR after failed init.
/// </summary>
private void AfterError(FeatureProvider provider, Exception ex)
private async Task AfterError(FeatureProvider provider, Exception ex)

{
provider.Status = typeof(ProviderFatalException) == ex.GetType() ? ProviderStatus.Fatal : ProviderStatus.Error;
Expand All @@ -297,7 +297,7 @@ private void AfterError(FeatureProvider provider, Exception ex)
ProviderName = provider.GetMetadata()?.Name,
};

this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload });
await this._eventExecutor.EventChannel.Writer.WriteAsync(new Event { Provider = provider, EventPayload = eventPayload }).ConfigureAwait(false);
}
}
}
2 changes: 1 addition & 1 deletion src/OpenFeature/EventExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ private void UpdateProviderStatus(FeatureProvider provider, ProviderEventPayload
provider.Status = ProviderStatus.Stale;
break;
case ProviderEventTypes.ProviderError:
provider.Status = eventPayload.errorType == ErrorType.ProviderFatal ? ProviderStatus.Fatal : ProviderStatus.Error;
provider.Status = eventPayload.ErrorType == ErrorType.ProviderFatal ? ProviderStatus.Fatal : ProviderStatus.Error;
break;
default: break;
}
Expand Down
36 changes: 27 additions & 9 deletions src/OpenFeature/ProviderRepository.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -14,6 +16,8 @@ namespace OpenFeature
/// </summary>
internal sealed class ProviderRepository : IAsyncDisposable
{
private ILogger _logger;

private FeatureProvider _defaultProvider = new NoOpFeatureProvider();

private readonly ConcurrentDictionary<string, FeatureProvider> _featureProviders =
Expand All @@ -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<EventExecutor>.Instance;
}

public async ValueTask DisposeAsync()
{
using (this._providersLock)
Expand All @@ -39,6 +48,8 @@ public async ValueTask DisposeAsync()
}
}

internal void SetLogger(ILogger logger) => this._logger = logger;

/// <summary>
/// Set the default provider
/// </summary>
Expand All @@ -54,8 +65,8 @@ public async ValueTask DisposeAsync()
public async Task SetProviderAsync(
FeatureProvider? featureProvider,
EvaluationContext context,
Action<FeatureProvider>? afterInitSuccess = null,
Action<FeatureProvider, Exception>? afterInitError = null)
Func<FeatureProvider, Task>? afterInitSuccess = null,
Func<FeatureProvider, Exception, Task>? afterInitError = null)
{
// Cannot unset the feature provider.
if (featureProvider == null)
Expand Down Expand Up @@ -91,8 +102,8 @@ await InitProviderAsync(this._defaultProvider, context, afterInitSuccess, afterI
private static async Task InitProviderAsync(
FeatureProvider? newProvider,
EvaluationContext context,
Action<FeatureProvider>? afterInitialization,
Action<FeatureProvider, Exception>? afterError)
Func<FeatureProvider, Task>? afterInitialization,
Func<FeatureProvider, Exception, Task>? afterError)
{
if (newProvider == null)
{
Expand All @@ -103,11 +114,17 @@ private static async Task InitProviderAsync(
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);
}
}
}
}
Expand All @@ -129,8 +146,8 @@ private static async Task InitProviderAsync(
public async Task SetProviderAsync(string? clientName,
FeatureProvider? featureProvider,
EvaluationContext context,
Action<FeatureProvider>? afterInitSuccess = null,
Action<FeatureProvider, Exception>? afterInitError = null,
Func<FeatureProvider, Task>? afterInitSuccess = null,
Func<FeatureProvider, Exception, Task>? afterInitError = null,
CancellationToken cancellationToken = default)
{
// Cannot set a provider for a null clientName.
Expand Down Expand Up @@ -207,8 +224,9 @@ private async Task SafeShutdownProviderAsync(FeatureProvider? targetProvider)
{
await targetProvider.ShutdownAsync().ConfigureAwait(false);
}
catch (Exception)
catch (Exception ex)
{
this._logger.LogError(ex, $"Error shutting down provider: {targetProvider.GetMetadata().Name}");
}
}

Expand Down
16 changes: 14 additions & 2 deletions test/OpenFeature.Tests/ProviderRepositoryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ await repository.SetProviderAsync(providerMock, context, afterInitSuccess: (theP
{
Assert.Equal(providerMock, theProvider);
callCount++;
return Task.CompletedTask;
});
Assert.Equal(1, callCount);
}
Expand All @@ -67,6 +68,7 @@ await repository.SetProviderAsync(providerMock, context, afterInitError: (thePro
Assert.Equal(providerMock, theProvider);
callCount++;
receivedError = error;
return Task.CompletedTask;
});
Assert.Equal("BAD THINGS", receivedError?.Message);
Assert.Equal(1, callCount);
Expand Down Expand Up @@ -97,7 +99,11 @@ internal async Task AfterInitialize_Is_Not_Called_For_Ready_Provider(ProviderSta
providerMock.Status.Returns(status);
var context = new EvaluationContextBuilder().Build();
var callCount = 0;
await repository.SetProviderAsync(providerMock, context, afterInitSuccess: provider => { callCount++; });
await repository.SetProviderAsync(providerMock, context, afterInitSuccess: provider =>
{
callCount++;
return Task.CompletedTask;
});
Assert.Equal(0, callCount);
}

Expand Down Expand Up @@ -153,6 +159,7 @@ await repository.SetProviderAsync("the-name", providerMock, context, afterInitSu
{
Assert.Equal(providerMock, theProvider);
callCount++;
return Task.CompletedTask;
});
Assert.Equal(1, callCount);
}
Expand All @@ -172,6 +179,7 @@ await repository.SetProviderAsync("the-provider", providerMock, context, afterIn
Assert.Equal(providerMock, theProvider);
callCount++;
receivedError = error;
return Task.CompletedTask;
});
Assert.Equal("BAD THINGS", receivedError?.Message);
Assert.Equal(1, callCount);
Expand Down Expand Up @@ -203,7 +211,11 @@ internal async Task AfterInitialize_Is_Not_Called_For_Ready_Named_Provider(Provi
var context = new EvaluationContextBuilder().Build();
var callCount = 0;
await repository.SetProviderAsync("the-name", providerMock, context,
afterInitSuccess: provider => { callCount++; });
afterInitSuccess: provider =>
{
callCount++;
return Task.CompletedTask;
});
Assert.Equal(0, callCount);
}

Expand Down

0 comments on commit 18311fb

Please sign in to comment.