Skip to content

Commit

Permalink
adapted to pr review
Browse files Browse the repository at this point in the history 
Signed-off-by: Florian Bacher <[email protected]>
  • Loading branch information
@bacherfl
bacherfl committed Jan 10, 2024
1 parent 4f1cec8 commit 672fa32
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 66 deletions.
9 changes: 9 additions & 0 deletions src/OpenFeature/Api.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,5 +220,14 @@ public void RemoveHandler(ProviderEventTypes type, EventHandlerDelegate handler)
{
this.EventExecutor.RemoveApiLevelHandler(type, handler);
}

/// <summary>
/// Sets the logger for the API
/// </summary>
/// <param name="logger">The logger to be used</param>
public void SetLogger(ILogger logger)
{
this.EventExecutor.Logger = logger;
}
}
}
11 changes: 7 additions & 4 deletions src/OpenFeature/EventExecutor.cs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;
using System.Threading.Channels;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using OpenFeature.Constant;
using OpenFeature.Model;

Expand All @@ -27,8 +27,11 @@ internal class EventExecutor
private readonly Dictionary<ProviderEventTypes, List<EventHandlerDelegate>> _apiHandlers = new Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>();
private readonly Dictionary<string, Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>> _clientHandlers = new Dictionary<string, Dictionary<ProviderEventTypes, List<EventHandlerDelegate>>>();

internal ILogger Logger { get; set; }

public EventExecutor()
{
this.Logger = new Logger<EventExecutor>(new NullLoggerFactory());
this._shutdownDelegate = this.SignalShutdownAsync;
var eventProcessing = new Thread(this.ProcessEventAsync);
eventProcessing.Start();
Expand Down Expand Up @@ -208,14 +211,14 @@ private void EmitOnRegistration(FeatureProviderReference provider, ProviderEvent
{
handler.Invoke(new ProviderEventPayload
{
ProviderName = provider.Provider.GetMetadata().Name,
ProviderName = provider.Provider?.GetMetadata()?.Name,
Type = eventType,
Message = message
});
}
catch (Exception exc)
{
Debug.WriteLine(exc);
this.Logger?.LogError("Error running handler: " + exc);
}
}
}
Expand Down Expand Up @@ -320,7 +323,7 @@ private void InvokeEventHandler(EventHandlerDelegate eventHandler, Event e)
}
catch (Exception exc)
{
Debug.WriteLine(exc);
this.Logger?.LogError("Error running handler: " + exc);
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/OpenFeature/Model/ProviderEvents.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,22 @@ public class ProviderEventPayload
/// Name of the provider.
/// </summary>
public string ProviderName { get; set; }

/// <summary>
/// Type of the event
/// </summary>
public ProviderEventTypes Type { get; set; }

/// <summary>
/// A message providing more information about the event.
/// </summary>
public string Message { get; set; }

/// <summary>
/// A List of flags that have been changed.
/// </summary>
public List<string> FlagChanges { get; set; }
public List<string> FlagsChanged { get; set; }

/// <summary>
/// Metadata information for the event.
/// </summary>
Expand Down
149 changes: 88 additions & 61 deletions test/OpenFeature.Tests/OpenFeatureEventTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -20,18 +21,23 @@ public async Task Event_Executor_Should_Propagate_Events_ToGlobal_Handler()
{
var eventHandler = Substitute.For<EventHandlerDelegate>();

eventHandler.Invoke(Arg.Any<ProviderEventPayload>());

var eventExecutor = new EventExecutor();

eventExecutor.AddApiLevelHandler(ProviderEventTypes.ProviderReady, eventHandler);
eventExecutor.AddApiLevelHandler(ProviderEventTypes.ProviderConfigurationChanged, eventHandler);

var eventPayload = new Event { EventPayload = new ProviderEventPayload { Type = ProviderEventTypes.ProviderReady } };
eventExecutor.EventChannel.Writer.TryWrite(eventPayload);
var eventMetadata = new Dictionary<string, object> { { "foo", "bar" } };
var myEvent = new Event { EventPayload = new ProviderEventPayload
{
Type = ProviderEventTypes.ProviderConfigurationChanged,
Message = "The provider is ready",
EventMetadata = eventMetadata,
FlagsChanged = new List<string>{ "flag1", "flag2" }
} };
eventExecutor.EventChannel.Writer.TryWrite(myEvent);

Thread.Sleep(1000);

eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.Type == ProviderEventTypes.ProviderReady));
eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload == myEvent.EventPayload));

// shut down the event executor
await eventExecutor.Shutdown();
Expand All @@ -55,8 +61,6 @@ public async Task API_Level_Event_Handlers_Should_Be_Registered()
{
var eventHandler = Substitute.For<EventHandlerDelegate>();

eventHandler.Invoke(Arg.Any<ProviderEventPayload>());

Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
Api.Instance.AddHandler(ProviderEventTypes.ProviderConfigurationChanged, eventHandler);
Api.Instance.AddHandler(ProviderEventTypes.ProviderError, eventHandler);
Expand Down Expand Up @@ -131,7 +135,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_
[Specification("5.2.4", "The `handler function` MUST accept a `event details` parameter.")]
[Specification("5.3.2", "If the provider's `initialize` function terminates abnormally, `PROVIDER_ERROR` handlers MUST run.")]
[Specification("5.3.3", "Handlers attached after the provider is already in the associated state, MUST run immediately.")]
public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_After_Registering_Provider_Error()
public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Error_State_After_Registering_Provider_Error()
{
var eventHandler = Substitute.For<EventHandlerDelegate>();

Expand All @@ -157,7 +161,7 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_
[Specification("5.2.3", "The `event details` MUST contain the `provider name` associated with the event.")]
[Specification("5.2.4", "The `handler function` MUST accept a `event details` parameter.")]
[Specification("5.3.3", "Handlers attached after the provider is already in the associated state, MUST run immediately.")]
public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_After_Registering_Provider_Stale()
public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Stale_State_After_Registering_Provider_Stale()
{
var eventHandler = Substitute.For<EventHandlerDelegate>();

Expand Down Expand Up @@ -188,15 +192,21 @@ public async Task API_Level_Event_Handlers_Should_Be_Exchangeable()
var eventHandler = Substitute.For<EventHandlerDelegate>();

Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
Api.Instance.AddHandler(ProviderEventTypes.ProviderConfigurationChanged, eventHandler);

var testProvider = new TestProvider();
await Api.Instance.SetProvider(testProvider);

testProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);

var newTestProvider = new TestProvider();
await Api.Instance.SetProvider(newTestProvider);

newTestProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);

Thread.Sleep(1000);
eventHandler.Received(2).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
eventHandler.Received(2).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderReady));
eventHandler.Received(2).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));
}

[Fact]
Expand Down Expand Up @@ -230,32 +240,22 @@ public async Task API_Level_Event_Handlers_Should_Be_Removable()
[Specification("5.2.5", "If a `handler function` terminates abnormally, other `handler` functions MUST run.")]
public async Task API_Level_Event_Handlers_Should_Be_Executed_When_Other_Handler_Fails()
{
try
{
var fixture = new Fixture();

var failingEventHandler = Substitute.For<EventHandlerDelegate>();
var eventHandler = Substitute.For<EventHandlerDelegate>();
var fixture = new Fixture();

failingEventHandler.When(x => x.Invoke(Arg.Any<ProviderEventPayload>()))
.Do(x => throw new Exception());
var failingEventHandler = Substitute.For<EventHandlerDelegate>();
var eventHandler = Substitute.For<EventHandlerDelegate>();

eventHandler.Invoke(Arg.Any<ProviderEventPayload>());
failingEventHandler.When(x => x.Invoke(Arg.Any<ProviderEventPayload>()))
.Do(x => throw new Exception());

Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, failingEventHandler);
Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, failingEventHandler);
Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

var testProvider = new TestProvider(fixture.Create<string>());
await Api.Instance.SetProvider(testProvider);
var testProvider = new TestProvider(fixture.Create<string>());
await Api.Instance.SetProvider(testProvider);

failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
}
catch (Exception e)
{
// make sure the exception is passed on correctly
Assert.NotNull(e);
}
failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
}

[Fact]
Expand All @@ -268,8 +268,6 @@ public async Task Client_Level_Event_Handlers_Should_Be_Registered()
var fixture = new Fixture();
var eventHandler = Substitute.For<EventHandlerDelegate>();

eventHandler.Invoke(Arg.Any<ProviderEventPayload>());

var myClient = Api.Instance.GetClient(fixture.Create<string>());

var testProvider = new TestProvider();
Expand All @@ -288,35 +286,24 @@ public async Task Client_Level_Event_Handlers_Should_Be_Registered()
[Specification("5.2.5", "If a `handler function` terminates abnormally, other `handler` functions MUST run.")]
public async Task Client_Level_Event_Handlers_Should_Be_Executed_When_Other_Handler_Fails()
{
try
{
var fixture = new Fixture();

var failingEventHandler = Substitute.For<EventHandlerDelegate>();
var eventHandler = Substitute.For<EventHandlerDelegate>();

failingEventHandler.When(x => x.Invoke(Arg.Any<ProviderEventPayload>()))
.Do(x => throw new Exception());
var fixture = new Fixture();

eventHandler.Invoke(Arg.Any<ProviderEventPayload>());
var failingEventHandler = Substitute.For<EventHandlerDelegate>();
var eventHandler = Substitute.For<EventHandlerDelegate>();

var myClient = Api.Instance.GetClient(fixture.Create<string>());
failingEventHandler.When(x => x.Invoke(Arg.Any<ProviderEventPayload>()))
.Do(x => throw new Exception());

myClient.AddHandler(ProviderEventTypes.ProviderReady, failingEventHandler);
myClient.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
var myClient = Api.Instance.GetClient(fixture.Create<string>());

var testProvider = new TestProvider();
await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
myClient.AddHandler(ProviderEventTypes.ProviderReady, failingEventHandler);
myClient.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);

failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
}
catch (Exception e)
{
// make sure the exception is passed on correctly
Assert.NotNull(e);
}
var testProvider = new TestProvider();
await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);

failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
}

[Fact]
Expand All @@ -331,8 +318,6 @@ public async Task Client_Level_Event_Handlers_Should_Be_Registered_To_Default_Pr
var eventHandler = Substitute.For<EventHandlerDelegate>();
var clientEventHandler = Substitute.For<EventHandlerDelegate>();

eventHandler.Invoke(Arg.Any<ProviderEventPayload>());

var myClientWithNoBoundProvider = Api.Instance.GetClient(fixture.Create<string>());
var myClientWithBoundProvider = Api.Instance.GetClient(fixture.Create<string>());

Expand All @@ -354,6 +339,50 @@ public async Task Client_Level_Event_Handlers_Should_Be_Registered_To_Default_Pr
clientEventHandler.DidNotReceive().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == apiProvider.GetMetadata().Name));
}

[Fact]
[Specification("5.1.2", "When a `provider` signals the occurrence of a particular `event`, the associated `client` and `API` event handlers MUST run.")]
[Specification("5.1.3", "When a `provider` signals the occurrence of a particular `event`, event handlers on clients which are not associated with that provider MUST NOT run.")]
[Specification("5.2.1", "The `client` MUST provide a function for associating `handler functions` with a particular `provider event type`.")]
[Specification("5.2.3", "The `event details` MUST contain the `provider name` associated with the event.")]
[Specification("5.2.4", "The `handler function` MUST accept a `event details` parameter.")]
[Specification("5.2.6", "Event handlers MUST persist across `provider` changes.")]
public async Task Client_Level_Event_Handlers_Should_Be_Receive_Events_From_Named_Provider_Instead_of_Default()
{
var fixture = new Fixture();
var clientEventHandler = Substitute.For<EventHandlerDelegate>();

var client = Api.Instance.GetClient(fixture.Create<string>());

var defaultProvider = new TestProvider(fixture.Create<string>());
var clientProvider = new TestProvider(fixture.Create<string>());

// set the default provider
await Api.Instance.SetProvider(defaultProvider);

client.AddHandler(ProviderEventTypes.ProviderConfigurationChanged, clientEventHandler);

defaultProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);

Thread.Sleep(1000);

// verify that the client received the event from the default provider as there is no named provider registered yet
clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == defaultProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));

// set the other provider specifically for the client
await Api.Instance.SetProvider(client.GetMetadata().Name, clientProvider);

// now, send another event for the default handler
defaultProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);
clientProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);

Thread.Sleep(1000);

// now the client should have received only the event from the named provider
clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == clientProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));
// for the default provider, the number of received events should stay unchanged
clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == defaultProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));
}

[Fact]
[Specification("5.1.2", "When a `provider` signals the occurrence of a particular `event`, the associated `client` and `API` event handlers MUST run.")]
[Specification("5.2.1", "The `client` MUST provide a function for associating `handler functions` with a particular `provider event type`.")]
Expand All @@ -366,8 +395,6 @@ public async Task Client_Level_Event_Handlers_Should_Be_Informed_About_Ready_Sta
var fixture = new Fixture();
var eventHandler = Substitute.For<EventHandlerDelegate>();

eventHandler.Invoke(Arg.Any<ProviderEventPayload>());

var myClient = Api.Instance.GetClient(fixture.Create<string>());

var testProvider = new TestProvider();
Expand Down

0 comments on commit 672fa32

Please sign in to comment.