diff --git a/src/OpenFeature/Api.cs b/src/OpenFeature/Api.cs index d9dc583e..77a990c2 100644 --- a/src/OpenFeature/Api.cs +++ b/src/OpenFeature/Api.cs @@ -208,15 +208,16 @@ public async Task Shutdown() await this._repository.Shutdown().ConfigureAwait(false); } + /// public void AddHandler(ProviderEventTypes type, EventHandlerDelegate handler) { this.EventExecutor.AddApiLevelHandler(type, handler); } + /// public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler) { - // TODO - throw new System.NotImplementedException(); + this.EventExecutor.RemoveApiLevelHandler(type, handler); } } } diff --git a/src/OpenFeature/EventExecutor.cs b/src/OpenFeature/EventExecutor.cs index 90483e37..c258fd34 100644 --- a/src/OpenFeature/EventExecutor.cs +++ b/src/OpenFeature/EventExecutor.cs @@ -11,75 +11,98 @@ namespace OpenFeature { public class EventExecutor { + private Mutex _mutex = new Mutex(); public readonly Channel eventChannel = Channel.CreateBounded(1); private FeatureProviderReference _defaultProvider; private readonly Dictionary _namedProviderReferences = new Dictionary(); private readonly List _activeSubscriptions = new List(); private readonly SemaphoreSlim _shutdownSemaphore = new SemaphoreSlim(0); - private readonly Dictionary> _apiLevelHandlers = new Dictionary>(); - private readonly Dictionary>> _scopedApiHandlers = new Dictionary>>(); + private readonly Dictionary> _apiHandlers = new Dictionary>(); + private readonly Dictionary>> _clientHandlers = new Dictionary>>(); public EventExecutor() { this.ProcessEventAsync(); } - + internal void AddApiLevelHandler(ProviderEventTypes eventType, EventHandlerDelegate handler) { - if (!this._apiLevelHandlers.TryGetValue(eventType, out var eventHandlers)) + this._mutex.WaitOne(); + if (!this._apiHandlers.TryGetValue(eventType, out var eventHandlers)) { eventHandlers = new List(); - this._apiLevelHandlers[eventType] = eventHandlers; + this._apiHandlers[eventType] = eventHandlers; } - + eventHandlers.Add(handler); this.EmitOnRegistration(this._defaultProvider, eventType, handler); + this._mutex.ReleaseMutex(); } - - internal void RemoveGlobalHandler(ProviderEventTypes type, EventHandlerDelegate handler) + + internal void RemoveApiLevelHandler(ProviderEventTypes type, EventHandlerDelegate handler) { - if (this._apiLevelHandlers.TryGetValue(type, out var eventHandlers)) + this._mutex.WaitOne(); + if (this._apiHandlers.TryGetValue(type, out var eventHandlers)) { eventHandlers.Remove(handler); } + this._mutex.ReleaseMutex(); } - - internal void AddNamedHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler) + + internal void AddClientHandler(string client, ProviderEventTypes eventType, EventHandlerDelegate handler) { + this._mutex.WaitOne(); // check if there is already a list of handlers for the given client and event type - if (!this._scopedApiHandlers.TryGetValue(client, out var registry)) + if (!this._clientHandlers.TryGetValue(client, out var registry)) { registry = new Dictionary>(); - this._scopedApiHandlers[client] = registry; + this._clientHandlers[client] = registry; } - if (!this._scopedApiHandlers[client].TryGetValue(eventType, out var eventHandlers)) + if (!this._clientHandlers[client].TryGetValue(eventType, out var eventHandlers)) { eventHandlers = new List(); - this._scopedApiHandlers[client][eventType] = eventHandlers; + this._clientHandlers[client][eventType] = eventHandlers; } - this._scopedApiHandlers[client][eventType].Add(handler); + this._clientHandlers[client][eventType].Add(handler); if (this._namedProviderReferences.TryGetValue(client, out var clientProviderReference)) { this.EmitOnRegistration(clientProviderReference, eventType, handler); } + this._mutex.ReleaseMutex(); + } + + internal void RemoveClientHandler(string client, ProviderEventTypes type, EventHandlerDelegate handler) + { + this._mutex.WaitOne(); + if (this._clientHandlers.TryGetValue(client, out var clientEventHandlers)) + { + if (clientEventHandlers.TryGetValue(type, out var eventHandlers)) + { + eventHandlers.Remove(handler); + } + } + this._mutex.ReleaseMutex(); } internal void RegisterDefaultFeatureProvider(FeatureProvider provider) { + this._mutex.WaitOne(); var oldProvider = this._defaultProvider; this._defaultProvider = new FeatureProviderReference(provider); this.StartListeningAndShutdownOld(this._defaultProvider, oldProvider); + this._mutex.ReleaseMutex(); } internal void RegisterClientFeatureProvider(string client, FeatureProvider provider) { + this._mutex.WaitOne(); var newProvider = new FeatureProviderReference(provider); FeatureProviderReference oldProvider = null; if (this._namedProviderReferences.TryGetValue(client, out var foundOldProvider)) @@ -87,9 +110,10 @@ internal void RegisterClientFeatureProvider(string client, FeatureProvider provi oldProvider = foundOldProvider; } - this._namedProviderReferences.Add(client, newProvider); + this._namedProviderReferences[client] = newProvider; this.StartListeningAndShutdownOld(newProvider, oldProvider); + this._mutex.ReleaseMutex(); } private void StartListeningAndShutdownOld(FeatureProviderReference newProvider, FeatureProviderReference oldProvider) @@ -141,19 +165,21 @@ private void EmitOnRegistration(FeatureProviderReference provider, ProviderEvent if (status == ProviderStatus.Ready && eventType == ProviderEventTypes.PROVIDER_READY) { message = "Provider is ready"; - } else if (status == ProviderStatus.Error && eventType == ProviderEventTypes.PROVIDER_ERROR) + } + else if (status == ProviderStatus.Error && eventType == ProviderEventTypes.PROVIDER_ERROR) { message = "Provider is in error state"; - } else if (status == ProviderStatus.Stale && eventType == ProviderEventTypes.PROVIDER_STALE) + } + else if (status == ProviderStatus.Stale && eventType == ProviderEventTypes.PROVIDER_STALE) { message = "Provider is in stale state"; } if (message != "") { - handler.Invoke(new ProviderEventPayload + handler(new ProviderEventPayload { - ProviderName = provider.ToString(), + ProviderName = provider.Provider.GetMetadata().Name, Type = eventType, Message = message, }); @@ -169,7 +195,7 @@ private async Task ProcessFeatureProviderEventsAsync(FeatureProviderReference pr switch (item) { case ProviderEventPayload eventPayload: - this.eventChannel.Writer.TryWrite(new Event{ Provider = providerRef, EventPayload = eventPayload }); + this.eventChannel.Writer.TryWrite(new Event { Provider = providerRef, EventPayload = eventPayload }); break; case ShutdownSignal _: providerRef.ShutdownSemaphore.Release(); @@ -183,45 +209,47 @@ private async Task ProcessEventAsync() { while (true) { - var item = await this.eventChannel.Reader.ReadAsync().ConfigureAwait(false); - - switch (item) - { - case Event e: - if (this._apiLevelHandlers.TryGetValue(e.EventPayload.Type, out var eventHandlers)) - { - foreach (var eventHandler in eventHandlers) - { - eventHandler.Invoke(e.EventPayload); - } - } - - // look for client handlers and call invoke method there - foreach (var keyAndValue in this._namedProviderReferences) - { - if (keyAndValue.Value == e.Provider) - { - if (this._scopedApiHandlers.TryGetValue(keyAndValue.Key, out var clientRegistry)) - { - if (clientRegistry.TryGetValue(e.EventPayload.Type, out var clientEventHandlers)) - { - foreach (var eventHandler in clientEventHandlers) - { - eventHandler.Invoke(e.EventPayload); - } - } - } - } - } - break; - case ShutdownSignal _: - this._shutdownSemaphore.Release(); - return; - } - + var item = await this.eventChannel.Reader.ReadAsync().ConfigureAwait(false); + + switch (item) + { + case Event e: + this._mutex.WaitOne(); + if (this._apiHandlers.TryGetValue(e.EventPayload.Type, out var eventHandlers)) + { + foreach (var eventHandler in eventHandlers) + { + eventHandler.Invoke(e.EventPayload); + } + } + + // look for client handlers and call invoke method there + foreach (var keyAndValue in this._namedProviderReferences) + { + if (keyAndValue.Value == e.Provider) + { + if (this._clientHandlers.TryGetValue(keyAndValue.Key, out var clientRegistry)) + { + if (clientRegistry.TryGetValue(e.EventPayload.Type, out var clientEventHandlers)) + { + foreach (var eventHandler in clientEventHandlers) + { + eventHandler.Invoke(e.EventPayload); + } + } + } + } + } + this._mutex.ReleaseMutex(); + break; + case ShutdownSignal _: + this._shutdownSemaphore.Release(); + return; + } + } } - + // Method to signal shutdown public async Task SignalShutdownAsync() { diff --git a/src/OpenFeature/FeatureProvider.cs b/src/OpenFeature/FeatureProvider.cs index c1f3074a..78a9e0d3 100644 --- a/src/OpenFeature/FeatureProvider.cs +++ b/src/OpenFeature/FeatureProvider.cs @@ -1,8 +1,8 @@ using System.Collections.Immutable; +using System.Threading.Channels; using System.Threading.Tasks; using OpenFeature.Constant; using OpenFeature.Model; -using System.Threading.Channels; namespace OpenFeature { diff --git a/src/OpenFeature/IEventBus.cs b/src/OpenFeature/IEventBus.cs index da4b26cc..e11213d3 100644 --- a/src/OpenFeature/IEventBus.cs +++ b/src/OpenFeature/IEventBus.cs @@ -3,8 +3,19 @@ namespace OpenFeature { - public interface IEventBus { + public interface IEventBus + { + /// + /// Adds an Event Handler for the given event type. + /// + /// The type of the event + /// Implementation of the void AddHandler(ProviderEventTypes type, EventHandlerDelegate handler); + /// + /// Removes an Event Handler for the given event type. + /// + /// The type of the event + /// Implementation of the void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler); - } -} \ No newline at end of file + } +} diff --git a/src/OpenFeature/IFeatureClient.cs b/src/OpenFeature/IFeatureClient.cs index bb848c72..1d2e6dfb 100644 --- a/src/OpenFeature/IFeatureClient.cs +++ b/src/OpenFeature/IFeatureClient.cs @@ -8,7 +8,7 @@ namespace OpenFeature /// /// Interface used to resolve flags of varying types. /// - public interface IFeatureClient + public interface IFeatureClient : IEventBus { /// /// Appends hooks to client @@ -19,16 +19,6 @@ public interface IFeatureClient /// A list of Hooks that implement the interface void AddHooks(IEnumerable hooks); - /// - /// Adds an Event Handler for the client - /// - /// The appending operation will be atomic. - /// - /// - /// The event type - /// An object that implements the interface - void AddHandler(ProviderEventTypes eventType, EventHandlerDelegate handler); - /// /// Enumerates the global hooks. /// diff --git a/src/OpenFeature/Model/ProviderEvents.cs b/src/OpenFeature/Model/ProviderEvents.cs index 3062b786..3592e1a4 100644 --- a/src/OpenFeature/Model/ProviderEvents.cs +++ b/src/OpenFeature/Model/ProviderEvents.cs @@ -11,7 +11,7 @@ public class ProviderEventPayload /// Name of the provider /// public string ProviderName { get; set; } - public ProviderEventTypes Type {get; set; } + public ProviderEventTypes Type { get; set; } public string Message { get; set; } public List FlagChanges { get; set; } public Dictionary EventMetadata { get; set; } diff --git a/src/OpenFeature/OpenFeatureClient.cs b/src/OpenFeature/OpenFeatureClient.cs index 6ce0bdd2..9ea9b13a 100644 --- a/src/OpenFeature/OpenFeatureClient.cs +++ b/src/OpenFeature/OpenFeatureClient.cs @@ -93,17 +93,16 @@ public FeatureClient(string name, string version, ILogger logger = null, Evaluat /// Hook that implements the interface public void AddHooks(Hook hook) => this._hooks.Push(hook); - /// - /// Adds an Event Handler for the client - /// - /// The appending operation will be atomic. - /// - /// - /// The event type - /// An object that implements the interface + /// public void AddHandler(ProviderEventTypes eventType, EventHandlerDelegate handler) { - Api.Instance.EventExecutor.AddNamedHandler(this._metadata.Name, eventType, handler); + Api.Instance.EventExecutor.AddClientHandler(this._metadata.Name, eventType, handler); + } + + /// + public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler) + { + Api.Instance.EventExecutor.RemoveClientHandler(this._metadata.Name, type, handler); } /// diff --git a/test/OpenFeature.Tests/OpenFeatureEventTests.cs b/test/OpenFeature.Tests/OpenFeatureEventTests.cs index 05abc71f..fe68cfa0 100644 --- a/test/OpenFeature.Tests/OpenFeatureEventTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureEventTests.cs @@ -33,6 +33,7 @@ public async Task Event_Executor_Should_Propagate_Events_ToGlobal_Handler() var eventPayload = new Event { EventPayload = new ProviderEventPayload { Type = ProviderEventTypes.PROVIDER_READY }}; eventExecutor.eventChannel.Writer.TryWrite(eventPayload); + eventHandler.Received().Invoke(Arg.Is(payload => payload.Type == ProviderEventTypes.PROVIDER_READY)); // shut down the event executor await eventExecutor.SignalShutdownAsync(); @@ -44,11 +45,7 @@ public async Task Event_Executor_Should_Propagate_Events_ToGlobal_Handler() eventHandler.DidNotReceive().Invoke(newEventPayload); - // verify the event handler received one event - Received.InOrder(async () => - { - eventHandler.Invoke(Arg.Any()); - }); + eventHandler.DidNotReceive().Invoke(Arg.Is(payload => payload.Type == ProviderEventTypes.PROVIDER_STALE)); } [Fact] @@ -63,36 +60,21 @@ public async Task API_Level_Event_Handlers_Should_Be_Registered() var testProvider = new TestProvider(); await Api.Instance.SetProvider(testProvider); - Received.InOrder(async () => - { - // first one due to NoOpProvider being in ready state - eventHandler.Invoke(Arg.Any()); - // second one for the testProvider - eventHandler.Invoke(Arg.Any()); - }); - - Thread.Sleep(10000); + eventHandler.Received().Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)); } [Fact] - public async Task API_Level_Event_Handlers_Should_Be_Informed_Aabout_Ready_State_After_Registering() + public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_After_Registering() { var eventHandler = Substitute.For(); - eventHandler.Invoke(Arg.Any()); - var testProvider = new TestProvider(); await Api.Instance.SetProvider(testProvider); Api.Instance.AddHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); - Received.InOrder(async () => - { - // first one due to NoOpProvider being in ready state - eventHandler.Invoke(Arg.Any()); - // second one for the testProvider - eventHandler.Invoke(Arg.Any()); - }); + Thread.Sleep(1000); + eventHandler.Received().Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)); } [Fact] @@ -100,47 +82,102 @@ public async Task API_Level_Event_Handlers_Should_Be_Exchangeable() { var eventHandler = Substitute.For(); - //eventHandler.Invoke(Arg.Any()); + Api.Instance.AddHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); + + var testProvider = new TestProvider(); + await Api.Instance.SetProvider(testProvider); + + var newTestProvider = new TestProvider(); + await Api.Instance.SetProvider(newTestProvider); + + Thread.Sleep(1000); + eventHandler.Received(2).Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)); + } + + [Fact] + public async Task API_Level_Event_Handlers_Should_Be_Removable() + { + var eventHandler = Substitute.For(); Api.Instance.AddHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); var testProvider = new TestProvider(); await Api.Instance.SetProvider(testProvider); + Thread.Sleep(1000); + + eventHandler.Received().Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)); + + Api.Instance.RemoveHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); + var newTestProvider = new TestProvider(); await Api.Instance.SetProvider(newTestProvider); - Received.InOrder(async () => - { - // first one due to NoOpProvider being in ready state - eventHandler.Invoke(Arg.Any()); - // second one for the testProvider - eventHandler.Invoke(Arg.Any()); - // third one for the new testProvider - eventHandler.Invoke(Arg.Any()); - }); + // now we should not receive any event since we have removed the event handler + Received.InOrder(async () => {}); } [Fact] public async Task Client_Level_Event_Handlers_Should_Be_Registered() { + var fixture = new Fixture(); + var eventHandler = Substitute.For(); + + eventHandler.Invoke(Arg.Any()); + + var myClient = Api.Instance.GetClient(fixture.Create()); + + var testProvider = new TestProvider(); + await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider); + + myClient.AddHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); + + eventHandler.Received().Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)); + } + + [Fact] + public async Task Client_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_After_Registering() + { + var fixture = new Fixture(); var eventHandler = Substitute.For(); eventHandler.Invoke(Arg.Any()); - var myClient = Api.Instance.GetClient("my-client"); + var myClient = Api.Instance.GetClient(fixture.Create()); + + var testProvider = new TestProvider(); + await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider); + + // add the event handler after the provider has already transitioned into the ready state + myClient.AddHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); + + eventHandler.Received().Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)); + } + + [Fact] + public async Task Client_Level_Event_Handlers_Should_Be_Removable() + { + var fixture = new Fixture(); + + var eventHandler = Substitute.For(); + + var myClient = Api.Instance.GetClient(fixture.Create()); myClient.AddHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); var testProvider = new TestProvider(); await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider); - Received.InOrder(async () => - { - eventHandler.Invoke(Arg.Any()); - }); + // wait for the first event to be received + Thread.Sleep(1000); + myClient.RemoveHandler(ProviderEventTypes.PROVIDER_READY, eventHandler); + + var newTestProvider = new TestProvider(); + await Api.Instance.SetProvider(myClient.GetMetadata().Name, newTestProvider); - Thread.Sleep(10000); + // wait a bit and make sure we only have received the first event, but nothing after removing the event handler + Thread.Sleep(1000); + eventHandler.Received(1).Invoke(Arg.Is(payload => payload.ProviderName == testProvider.GetMetadata().Name)); } } } \ No newline at end of file diff --git a/test/OpenFeature.Tests/TestImplementations.cs b/test/OpenFeature.Tests/TestImplementations.cs index 14cf001f..9fc0f1c3 100644 --- a/test/OpenFeature.Tests/TestImplementations.cs +++ b/test/OpenFeature.Tests/TestImplementations.cs @@ -90,8 +90,13 @@ public override ProviderStatus GetStatus() public override Task Initialize(EvaluationContext context) { this._status = ProviderStatus.Ready; - this._eventChannel.Writer.TryWrite(new ProviderEventPayload { Type = ProviderEventTypes.PROVIDER_READY }); + this._eventChannel.Writer.TryWrite(new ProviderEventPayload { Type = ProviderEventTypes.PROVIDER_READY, ProviderName = this.GetMetadata().Name}); return base.Initialize(context); } + + internal void SendEvent(ProviderEventTypes eventType) + { + this._eventChannel.Writer.TryWrite(new ProviderEventPayload { Type = eventType, ProviderName = this.GetMetadata().Name }); + } } }