From e7134511e7c91048cd14d9f3b67b902b16d59a83 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Wed, 7 May 2025 19:48:17 +0100 Subject: [PATCH 01/10] Add extension method for adding Handler via Dependency Injection Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../Internal/EventHandlerDelegateWrapper.cs | 17 +++++ .../Internal/FeatureLifecycleManager.cs | 6 ++ .../OpenFeatureBuilderExtensions.cs | 71 +++++++++++++++++++ .../OpenFeatureOptions.cs | 12 ++++ 4 files changed, 106 insertions(+) create mode 100644 src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs diff --git a/src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs b/src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs new file mode 100644 index 000000000..bf5037708 --- /dev/null +++ b/src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs @@ -0,0 +1,17 @@ +using OpenFeature.Constant; +using OpenFeature.Model; + +namespace OpenFeature.DependencyInjection.Internal; + +internal record EventHandlerDelegateWrapper +{ + public ProviderEventTypes ProviderEventType { get; } + + public EventHandlerDelegate EventHandlerDelegate { get; } + + public EventHandlerDelegateWrapper(ProviderEventTypes providerEventTypes, EventHandlerDelegate eventHandlerDelegate) + { + ProviderEventType = providerEventTypes; + EventHandlerDelegate = eventHandlerDelegate; + } +} diff --git a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs index f2c914f23..c08f07c50 100644 --- a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs +++ b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs @@ -43,6 +43,12 @@ public async ValueTask EnsureInitializedAsync(CancellationToken cancellationToke } _featureApi.AddHooks(hooks); + + foreach (var handlerName in options.HandlerNames) + { + var handler = _serviceProvider.GetRequiredKeyedService(handlerName); + _featureApi.AddHandler(handler.ProviderEventType, handler.EventHandlerDelegate); + } } /// diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index 8f79f3946..fcdc8ff35 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -1,7 +1,9 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; +using OpenFeature.Constant; using OpenFeature.DependencyInjection; +using OpenFeature.DependencyInjection.Internal; using OpenFeature.Model; namespace OpenFeature; @@ -303,4 +305,73 @@ public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, return builder; } + + /// + /// Add a to allow you to react to state changes in the provider or underlying flag management system, such as flag definition changes, provider readiness, or error conditions + /// + /// The instance. + /// The type to handle. + /// The handler which reacts to . + /// + public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) + { + return AddHandler(builder, GenerateRandomName(), type, sp => eventHandlerDelegate); + } + + /// + /// Add a to allow you to react to state changes in the provider or underlying flag management system, such as flag definition changes, provider readiness, or error conditions + /// + /// The instance. + /// The name of the . + /// The type to handle. + /// The handler which reacts to . + /// + public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) + { + return AddHandler(builder, handlerName, type, sp => eventHandlerDelegate); + } + + /// + /// Add a to allow you to react to state changes in the provider or underlying flag management system, such as flag definition changes, provider readiness, or error conditions + /// + /// The instance. + /// The type to handle. + /// The handler factory for creating a handler which reacts to . + /// + public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, Func implementationFactory) + { + return AddHandler(builder, GenerateRandomName(), type, implementationFactory); + } + + /// + /// Add a to allow you to react to state changes in the provider or underlying flag management system, such as flag definition changes, provider readiness, or error conditions + /// + /// The instance. + /// The name of the . + /// The type to handle. + /// The handler factory for creating a handler which reacts to . + /// + public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, Func implementationFactory) + { + var key = string.Join(":", handlerName, type.ToString()); + + builder.Services.PostConfigure(options => options.AddHandlerName(key)); + + builder.Services.TryAddKeyedSingleton(key, (serviceProvider, _) => + { + var handler = implementationFactory(serviceProvider); + return new EventHandlerDelegateWrapper(type, handler); + }); + + return builder; + } + + static readonly Random _random = new(); + private static string GenerateRandomName() + { + const string characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + const int length = 4; + + return new string([.. Enumerable.Range(0, length).Select(_ => characters[_random.Next(characters.Length)])]); + } } diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs index e9cc3cb12..fd37b33cc 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs @@ -58,4 +58,16 @@ internal void AddHookName(string name) _hookNames.Add(name); } } + + private readonly HashSet _handlerNames = []; + + internal IReadOnlyCollection HandlerNames => _handlerNames; + + internal void AddHandlerName(string name) + { + lock (_handlerNames) + { + _handlerNames.Add(name); + } + } } From 1a4e979400c200534185d1f2f9d0e0f04307042e Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Wed, 7 May 2025 20:15:46 +0100 Subject: [PATCH 02/10] Add unit tests for new Extension method Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../FeatureLifecycleManagerTests.cs | 24 ++++++ .../OpenFeatureBuilderExtensionsTests.cs | 84 +++++++++++++++++++ .../FeatureFlagIntegrationTest.cs | 34 +++++++- 3 files changed, 141 insertions(+), 1 deletion(-) diff --git a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs index 47cc7df5c..b5e092e28 100644 --- a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs @@ -1,7 +1,9 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging.Abstractions; +using OpenFeature.Constant; using OpenFeature.DependencyInjection.Internal; +using OpenFeature.Model; using Xunit; namespace OpenFeature.DependencyInjection.Tests; @@ -81,4 +83,26 @@ public async Task EnsureInitializedAsync_ShouldSetHook_WhenHooksAreRegistered() var actual = Api.Instance.GetHooks().FirstOrDefault(); Assert.Equal(hook, actual); } + + [Fact] + public async Task EnsureInitializedAsync_ShouldSetHandler_WhenHandlersAreRegistered() + { + // Arrange + EventHandlerDelegate eventHandlerDelegate = (_) => { }; + var featureProvider = new NoOpFeatureProvider(); + var handler = new EventHandlerDelegateWrapper(ProviderEventTypes.ProviderReady, eventHandlerDelegate); + + _serviceCollection.AddSingleton(featureProvider) + .AddKeyedSingleton("test:ProviderReady", (_, key) => handler) + .Configure(options => + { + options.AddHandlerName("test:ProviderReady"); + }); + + var serviceProvider = _serviceCollection.BuildServiceProvider(); + var sut = new FeatureLifecycleManager(Api.Instance, serviceProvider, NullLogger.Instance); + + // Act + await sut.EnsureInitializedAsync().ConfigureAwait(true); + } } diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index 07597703a..5af523b51 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using OpenFeature.DependencyInjection.Internal; using OpenFeature.Model; using Xunit; @@ -301,4 +302,87 @@ public void AddHook_WithSpecifiedNameAndImplementationFactory_AsKeyedService() // Assert Assert.NotNull(hook); } + + [Fact] + public void AddHandler_AddsEventHandlerDelegateWrapperAsKeyedService() + { + // Arrange + EventHandlerDelegate eventHandler = (eventDetails) => { }; + _systemUnderTest.AddHandler("test", Constant.ProviderEventTypes.ProviderReady, eventHandler); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var handler = serviceProvider.GetKeyedService("test:ProviderReady"); + + // Assert + Assert.NotNull(handler); + Assert.Equal(eventHandler, handler.EventHandlerDelegate); + } + + [Fact] + public void AddHandler_SetsHandlerNameInOpenFeatureOptions() + { + // Arrange + EventHandlerDelegate eventHandler = (eventDetails) => { }; + _systemUnderTest.AddHandler("test", Constant.ProviderEventTypes.ProviderReady, eventHandler); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var options = serviceProvider.GetService>(); + var openFeatureOptions = options!.Value; + + // Assert + Assert.Contains("test:ProviderReady", openFeatureOptions.HandlerNames); + } + + [Fact] + public void AddHandler_WithoutName_AddsEventHandlerDelegateWrapperAsKeyedService() + { + // Arrange + EventHandlerDelegate eventHandler = (eventDetails) => { }; + _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, eventHandler); + + // Act + var handlers = _services.Where(s => s.ServiceType == typeof(EventHandlerDelegateWrapper)).ToList(); + var handler = handlers.First(); + + // Assert + Assert.True(handler.IsKeyedService); + Assert.Equal(ServiceLifetime.Singleton, handler.Lifetime); + } + + [Fact] + public void AddHandler_WithImplementationFactory_AddsEventHandlerDelegateWrapperAsKeyedService() + { + // Arrange + EventHandlerDelegate eventHandler = (eventDetails) => { }; + _systemUnderTest.AddHandler("test", Constant.ProviderEventTypes.ProviderReady, sp => eventHandler); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var handler = serviceProvider.GetKeyedService("test:ProviderReady"); + + // Assert + Assert.NotNull(handler); + Assert.Equal(eventHandler, handler.EventHandlerDelegate); + } + + [Fact] + public void AddHandlerWithImplementationFactory_WithoutName_AddsEventHandlerDelegateWrapperAsKeyedService() + { + // Arrange + EventHandlerDelegate eventHandler = (eventDetails) => { }; + _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, sp => eventHandler); + + // Act + var handlers = _services.Where(s => s.ServiceType == typeof(EventHandlerDelegateWrapper)).ToList(); + var handler = handlers.First(); + + // Assert + Assert.True(handler.IsKeyedService); + Assert.Equal(ServiceLifetime.Singleton, handler.Lifetime); + } } diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs index 9e1f4bca9..25c1636f0 100644 --- a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -6,9 +6,11 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging.Testing; +using OpenFeature.Constant; using OpenFeature.DependencyInjection.Providers.Memory; using OpenFeature.Hooks; using OpenFeature.IntegrationTests.Services; +using OpenFeature.Model; using OpenFeature.Providers.Memory; namespace OpenFeature.IntegrationTests; @@ -89,8 +91,33 @@ public async Task VerifyLoggingHookIsRegisteredAsync() }); } + [Fact] + public async Task VerifyHandlerIsRegisteredAsync() + { + // Arrange + var logger = new FakeLogger(); + Action configureServices = services => + { + services.AddTransient(); + }; + var handlerSuccess = false; + + using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, configureServices, (_) => { handlerSuccess = true; }) + .ConfigureAwait(true); + + var client = server.CreateClient(); + var requestUri = $"/features/{TestUserId}/flags/{FeatureA}"; + + // Act + var response = await client.GetAsync(requestUri).ConfigureAwait(true); + + // Assert + Assert.True(handlerSuccess); + } + private static async Task CreateServerAsync(ServiceLifetime serviceLifetime, FakeLogger logger, - Action? configureServices = null) + Action? configureServices = null, + EventHandlerDelegate? eventHandlerDelegate = null) { var builder = WebApplication.CreateBuilder(); builder.WebHost.UseTestServer(); @@ -126,6 +153,11 @@ private static async Task CreateServerAsync(ServiceLifetime serviceL } }); cfg.AddHook(serviceProvider => new LoggingHook(logger)); + + if (eventHandlerDelegate is not null) + { + cfg.AddHandler(ProviderEventTypes.ProviderReady, eventHandlerDelegate); + } }); var app = builder.Build(); From 51acc27f271edb03d62eda7c6abdb06a45845712 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Wed, 7 May 2025 20:22:57 +0100 Subject: [PATCH 03/10] Update README and add examples for injecting Handlers Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- README.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b0063d3a4..d21c111e6 100644 --- a/README.md +++ b/README.md @@ -433,9 +433,18 @@ For a basic configuration, you can use the InMemoryProvider. This provider is si builder.Services.AddOpenFeature(featureBuilder => { featureBuilder .AddHostedFeatureLifecycle() // From Hosting package + .AddInMemoryProvider(); +}); +``` + +You can add EvaluationContext, hooks, and handlers at a global/API level as needed. + +```csharp +builder.Services.AddOpenFeature(featureBuilder => { + featureBuilder .AddContext((contextBuilder, serviceProvider) => { /* Custom context configuration */ }) .AddHook() - .AddInMemoryProvider(); + .AddHandler(ProviderEventTypes.ProviderReady, (eventDetails) => { /* Handle event */ }); }); ``` From 0fd4057904b27a93aeb145a4656a4390a87739d5 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Wed, 7 May 2025 22:03:26 +0100 Subject: [PATCH 04/10] Swap to use Guid.NewGuid() instead of random string generator * Add check if passed in handler name is an empty or null string Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../OpenFeatureBuilderExtensions.cs | 16 +++++----------- .../OpenFeatureBuilderExtensionsTests.cs | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index fcdc8ff35..7b25685e6 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -315,7 +315,7 @@ public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, /// public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) { - return AddHandler(builder, GenerateRandomName(), type, sp => eventHandlerDelegate); + return AddHandler(builder, string.Empty, type, sp => eventHandlerDelegate); } /// @@ -340,7 +340,7 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, str /// public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, Func implementationFactory) { - return AddHandler(builder, GenerateRandomName(), type, implementationFactory); + return AddHandler(builder, string.Empty, type, implementationFactory); } /// @@ -353,6 +353,9 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, Pro /// public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, Func implementationFactory) { + if (string.IsNullOrEmpty(handlerName)) + handlerName = Guid.NewGuid().ToString(); + var key = string.Join(":", handlerName, type.ToString()); builder.Services.PostConfigure(options => options.AddHandlerName(key)); @@ -365,13 +368,4 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, str return builder; } - - static readonly Random _random = new(); - private static string GenerateRandomName() - { - const string characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; - const int length = 4; - - return new string([.. Enumerable.Range(0, length).Select(_ => characters[_random.Next(characters.Length)])]); - } } diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index 5af523b51..eef99b75b 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -353,6 +353,24 @@ public void AddHandler_WithoutName_AddsEventHandlerDelegateWrapperAsKeyedService Assert.Equal(ServiceLifetime.Singleton, handler.Lifetime); } + [Theory] + [InlineData("")] + [InlineData(null)] + public void AddHandler_WithEmptyName_AddsEventHandlerDelegateWrapperAsKeyedService(string? handlerName) + { + // Arrange + EventHandlerDelegate eventHandler = (eventDetails) => { }; + _systemUnderTest.AddHandler(handlerName!, Constant.ProviderEventTypes.ProviderReady, eventHandler); + + // Act + var handlers = _services.Where(s => s.ServiceType == typeof(EventHandlerDelegateWrapper)).ToList(); + var handler = handlers.First(); + + // Assert + Assert.True(handler.IsKeyedService); + Assert.Equal(ServiceLifetime.Singleton, handler.Lifetime); + } + [Fact] public void AddHandler_WithImplementationFactory_AddsEventHandlerDelegateWrapperAsKeyedService() { From e439ab13b6eb606aca31652a9c3d0b827ba7d1a5 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Fri, 9 May 2025 19:27:26 +0100 Subject: [PATCH 05/10] Address PR comments * Ensure that calling AddHandler multiple times will actually add all instances to the underlying OpenFeature API * Add unit tests to cover this behaviour Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../Internal/FeatureLifecycleManager.cs | 7 ++++-- .../OpenFeatureBuilderExtensions.cs | 10 ++++---- .../FeatureLifecycleManagerTests.cs | 25 +++++++++++++++++++ .../OpenFeatureBuilderExtensionsTests.cs | 21 ++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs index c08f07c50..a5d40949c 100644 --- a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs +++ b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs @@ -46,8 +46,11 @@ public async ValueTask EnsureInitializedAsync(CancellationToken cancellationToke foreach (var handlerName in options.HandlerNames) { - var handler = _serviceProvider.GetRequiredKeyedService(handlerName); - _featureApi.AddHandler(handler.ProviderEventType, handler.EventHandlerDelegate); + var handlers = _serviceProvider.GetKeyedServices(handlerName); + foreach (var handler in handlers) + { + _featureApi.AddHandler(handler.ProviderEventType, handler.EventHandlerDelegate); + } } } diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index 7b25685e6..0901317d8 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -315,7 +315,7 @@ public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, /// public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) { - return AddHandler(builder, string.Empty, type, sp => eventHandlerDelegate); + return AddHandler(builder, typeof(EventHandlerDelegate).Name, type, sp => eventHandlerDelegate); } /// @@ -340,7 +340,7 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, str /// public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, Func implementationFactory) { - return AddHandler(builder, string.Empty, type, implementationFactory); + return AddHandler(builder, typeof(EventHandlerDelegate).Name, type, implementationFactory); } /// @@ -353,14 +353,14 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, Pro /// public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, Func implementationFactory) { - if (string.IsNullOrEmpty(handlerName)) - handlerName = Guid.NewGuid().ToString(); + if (string.IsNullOrWhiteSpace(handlerName)) + handlerName = string.Empty; var key = string.Join(":", handlerName, type.ToString()); builder.Services.PostConfigure(options => options.AddHandlerName(key)); - builder.Services.TryAddKeyedSingleton(key, (serviceProvider, _) => + builder.Services.AddKeyedSingleton(key, (serviceProvider, _) => { var handler = implementationFactory(serviceProvider); return new EventHandlerDelegateWrapper(type, handler); diff --git a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs index b5e092e28..1b00241e3 100644 --- a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs @@ -105,4 +105,29 @@ public async Task EnsureInitializedAsync_ShouldSetHandler_WhenHandlersAreRegiste // Act await sut.EnsureInitializedAsync().ConfigureAwait(true); } + + [Fact] + public async Task EnsureInitializedAsync_ShouldSetHandler_WhenMultipleHandlersAreRegistered() + { + // Arrange + EventHandlerDelegate eventHandlerDelegate1 = (_) => { }; + EventHandlerDelegate eventHandlerDelegate2 = (_) => { }; + var featureProvider = new NoOpFeatureProvider(); + var handler1 = new EventHandlerDelegateWrapper(ProviderEventTypes.ProviderReady, eventHandlerDelegate1); + var handler2 = new EventHandlerDelegateWrapper(ProviderEventTypes.ProviderReady, eventHandlerDelegate2); + + _serviceCollection.AddSingleton(featureProvider) + .AddKeyedSingleton("test:ProviderReady", (_, key) => handler1) + .AddKeyedSingleton("test:ProviderReady", (_, key) => handler2) + .Configure(options => + { + options.AddHandlerName("test:ProviderReady"); + }); + + var serviceProvider = _serviceCollection.BuildServiceProvider(); + var sut = new FeatureLifecycleManager(Api.Instance, serviceProvider, NullLogger.Instance); + + // Act + await sut.EnsureInitializedAsync().ConfigureAwait(true); + } } diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index eef99b75b..1649168e9 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -320,6 +320,26 @@ public void AddHandler_AddsEventHandlerDelegateWrapperAsKeyedService() Assert.Equal(eventHandler, handler.EventHandlerDelegate); } + [Fact] + public void AddHandlerTwice_MultipleEventHandlerDelegateWrappersAsKeyedServices() + { + // Arrange + EventHandlerDelegate eventHandler1 = (eventDetails) => { }; + EventHandlerDelegate eventHandler2 = (eventDetails) => { }; + _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, eventHandler1); + _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, eventHandler2); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var handler = serviceProvider.GetKeyedServices("EventHandlerDelegate:ProviderReady"); + + // Assert + Assert.NotEmpty(handler); + Assert.Equal(eventHandler1, handler.ElementAt(0).EventHandlerDelegate); + Assert.Equal(eventHandler2, handler.ElementAt(1).EventHandlerDelegate); + } + [Fact] public void AddHandler_SetsHandlerNameInOpenFeatureOptions() { @@ -355,6 +375,7 @@ public void AddHandler_WithoutName_AddsEventHandlerDelegateWrapperAsKeyedService [Theory] [InlineData("")] + [InlineData(" ")] [InlineData(null)] public void AddHandler_WithEmptyName_AddsEventHandlerDelegateWrapperAsKeyedService(string? handlerName) { From 217294885c259ee36761ba55192cd0bb19fc607b Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Fri, 9 May 2025 22:49:56 +0100 Subject: [PATCH 06/10] Address PR comments * Throw ArgumentException when HandlerName is null or empty * Use primary constructor in EventHandlerDelegateWrapper Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../Internal/EventHandlerDelegateWrapper.cs | 15 +++------------ .../OpenFeatureBuilderExtensions.cs | 12 +++++++----- .../OpenFeatureBuilderExtensionsTests.cs | 9 ++++----- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs b/src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs index bf5037708..d31b3355c 100644 --- a/src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs +++ b/src/OpenFeature.DependencyInjection/Internal/EventHandlerDelegateWrapper.cs @@ -3,15 +3,6 @@ namespace OpenFeature.DependencyInjection.Internal; -internal record EventHandlerDelegateWrapper -{ - public ProviderEventTypes ProviderEventType { get; } - - public EventHandlerDelegate EventHandlerDelegate { get; } - - public EventHandlerDelegateWrapper(ProviderEventTypes providerEventTypes, EventHandlerDelegate eventHandlerDelegate) - { - ProviderEventType = providerEventTypes; - EventHandlerDelegate = eventHandlerDelegate; - } -} +internal record EventHandlerDelegateWrapper( + ProviderEventTypes ProviderEventType, + EventHandlerDelegate EventHandlerDelegate); diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index 0901317d8..ccde2df76 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -312,7 +312,7 @@ public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, /// The instance. /// The type to handle. /// The handler which reacts to . - /// + /// The instance. public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) { return AddHandler(builder, typeof(EventHandlerDelegate).Name, type, sp => eventHandlerDelegate); @@ -325,7 +325,8 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, Pro /// The name of the . /// The type to handle. /// The handler which reacts to . - /// + /// The instance. + /// Thrown when is null or empty public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) { return AddHandler(builder, handlerName, type, sp => eventHandlerDelegate); @@ -337,7 +338,7 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, str /// The instance. /// The type to handle. /// The handler factory for creating a handler which reacts to . - /// + /// The instance. public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, Func implementationFactory) { return AddHandler(builder, typeof(EventHandlerDelegate).Name, type, implementationFactory); @@ -350,11 +351,12 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, Pro /// The name of the . /// The type to handle. /// The handler factory for creating a handler which reacts to . - /// + /// The instance. + /// Thrown when is null or empty public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, Func implementationFactory) { if (string.IsNullOrWhiteSpace(handlerName)) - handlerName = string.Empty; + throw new ArgumentException("Null or empty Handler name", nameof(handlerName)); var key = string.Join(":", handlerName, type.ToString()); diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index 1649168e9..8b21b9554 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -381,15 +381,14 @@ public void AddHandler_WithEmptyName_AddsEventHandlerDelegateWrapperAsKeyedServi { // Arrange EventHandlerDelegate eventHandler = (eventDetails) => { }; - _systemUnderTest.AddHandler(handlerName!, Constant.ProviderEventTypes.ProviderReady, eventHandler); // Act - var handlers = _services.Where(s => s.ServiceType == typeof(EventHandlerDelegateWrapper)).ToList(); - var handler = handlers.First(); + Action act = () => _systemUnderTest.AddHandler(handlerName!, Constant.ProviderEventTypes.ProviderReady, eventHandler); // Assert - Assert.True(handler.IsKeyedService); - Assert.Equal(ServiceLifetime.Singleton, handler.Lifetime); + var ex = Assert.Throws(act); + Assert.Equal("handlerName", ex.ParamName); + Assert.StartsWith("Null or empty Handler name", ex.Message); } [Fact] From cd44977e1a3c6a73e5073c764d35f55a6e20257c Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Sat, 10 May 2025 19:34:35 +0100 Subject: [PATCH 07/10] Adopt Guard.ThrowIfNullOrWhiteSpace Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../OpenFeatureBuilderExtensions.cs | 3 +-- .../OpenFeatureBuilderExtensionsTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index ccde2df76..b20ffbcfb 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -355,8 +355,7 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, Pro /// Thrown when is null or empty public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, Func implementationFactory) { - if (string.IsNullOrWhiteSpace(handlerName)) - throw new ArgumentException("Null or empty Handler name", nameof(handlerName)); + Guard.ThrowIfNullOrWhiteSpace(handlerName); var key = string.Join(":", handlerName, type.ToString()); diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index 8b21b9554..e26d58804 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -386,9 +386,9 @@ public void AddHandler_WithEmptyName_AddsEventHandlerDelegateWrapperAsKeyedServi Action act = () => _systemUnderTest.AddHandler(handlerName!, Constant.ProviderEventTypes.ProviderReady, eventHandler); // Assert - var ex = Assert.Throws(act); + var ex = Assert.Throws(act); Assert.Equal("handlerName", ex.ParamName); - Assert.StartsWith("Null or empty Handler name", ex.Message); + Assert.StartsWith("Value cannot be null.", ex.Message); } [Fact] From 9b7268efc3e0eed612153ad8d83bc1b9a3a46f99 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Sat, 10 May 2025 23:51:44 +0100 Subject: [PATCH 08/10] Update integration tests * Ensure when multiple handlers are registered that they are both registered * Tweak CreateServerAsync to make it easier to extend dependency injection tests Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../FeatureFlagIntegrationTest.cs | 46 +++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs index 25c1636f0..dec5d0fd6 100644 --- a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.Metrics; using System.Net.Http.Json; using System.Text.Json; using Microsoft.AspNetCore.Builder; @@ -7,6 +8,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging.Testing; using OpenFeature.Constant; +using OpenFeature.DependencyInjection; using OpenFeature.DependencyInjection.Providers.Memory; using OpenFeature.Hooks; using OpenFeature.IntegrationTests.Services; @@ -100,9 +102,14 @@ public async Task VerifyHandlerIsRegisteredAsync() { services.AddTransient(); }; + var handlerSuccess = false; + Action openFeatureBuilder = cfg => + { + cfg.AddHandler(ProviderEventTypes.ProviderReady, (_) => { handlerSuccess = true; }); + }; - using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, configureServices, (_) => { handlerSuccess = true; }) + using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, configureServices, openFeatureBuilder) .ConfigureAwait(true); var client = server.CreateClient(); @@ -115,9 +122,40 @@ public async Task VerifyHandlerIsRegisteredAsync() Assert.True(handlerSuccess); } + [Fact] + public async Task VerifyMultipleHandlersAreRegisteredAsync() + { + // Arrange + var logger = new FakeLogger(); + Action configureServices = services => + { + services.AddTransient(); + }; + + var @lock = new Lock(); + var counter = 0; + Action openFeatureBuilder = cfg => + { + cfg.AddHandler(ProviderEventTypes.ProviderReady, (_) => { lock (@lock) { counter++; } }); + cfg.AddHandler(ProviderEventTypes.ProviderReady, (_) => { lock (@lock) { counter++; } }); + }; + + using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, configureServices, openFeatureBuilder) + .ConfigureAwait(true); + + var client = server.CreateClient(); + var requestUri = $"/features/{TestUserId}/flags/{FeatureA}"; + + // Act + var response = await client.GetAsync(requestUri).ConfigureAwait(true); + + // Assert + Assert.Equal(2, counter); + } + private static async Task CreateServerAsync(ServiceLifetime serviceLifetime, FakeLogger logger, Action? configureServices = null, - EventHandlerDelegate? eventHandlerDelegate = null) + Action? openFeatureBuilder = null) { var builder = WebApplication.CreateBuilder(); builder.WebHost.UseTestServer(); @@ -154,9 +192,9 @@ private static async Task CreateServerAsync(ServiceLifetime serviceL }); cfg.AddHook(serviceProvider => new LoggingHook(logger)); - if (eventHandlerDelegate is not null) + if (openFeatureBuilder is not null) { - cfg.AddHandler(ProviderEventTypes.ProviderReady, eventHandlerDelegate); + openFeatureBuilder(cfg); } }); From 550d245e51f146dfc34e26312c8733bd164a3f01 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Sat, 10 May 2025 23:53:58 +0100 Subject: [PATCH 09/10] Remove unnecessary using statements Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs index dec5d0fd6..a53f6c299 100644 --- a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -1,4 +1,3 @@ -using System.Diagnostics.Metrics; using System.Net.Http.Json; using System.Text.Json; using Microsoft.AspNetCore.Builder; @@ -12,7 +11,6 @@ using OpenFeature.DependencyInjection.Providers.Memory; using OpenFeature.Hooks; using OpenFeature.IntegrationTests.Services; -using OpenFeature.Model; using OpenFeature.Providers.Memory; namespace OpenFeature.IntegrationTests; From 51645c2638be72c1a0675bb87b7f6b74be9b9a78 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Sun, 11 May 2025 17:39:58 +0100 Subject: [PATCH 10/10] Remove AddHandler with name extension methods * Remove any unnecessary unit tests * Update integration tests to use openFeatureBuilder for easily adjusting the Arrange part of each test Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../Internal/FeatureLifecycleManager.cs | 9 +-- .../OpenFeatureBuilderExtensions.cs | 38 +-------- .../OpenFeatureOptions.cs | 12 --- .../FeatureLifecycleManagerTests.cs | 14 +--- .../OpenFeatureBuilderExtensionsTests.cs | 77 ++----------------- .../FeatureFlagIntegrationTest.cs | 60 ++++++++++++--- 6 files changed, 63 insertions(+), 147 deletions(-) diff --git a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs index a5d40949c..1ecac4349 100644 --- a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs +++ b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs @@ -44,13 +44,10 @@ public async ValueTask EnsureInitializedAsync(CancellationToken cancellationToke _featureApi.AddHooks(hooks); - foreach (var handlerName in options.HandlerNames) + var handlers = _serviceProvider.GetServices(); + foreach (var handler in handlers) { - var handlers = _serviceProvider.GetKeyedServices(handlerName); - foreach (var handler in handlers) - { - _featureApi.AddHandler(handler.ProviderEventType, handler.EventHandlerDelegate); - } + _featureApi.AddHandler(handler.ProviderEventType, handler.EventHandlerDelegate); } } diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index b20ffbcfb..317589606 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -315,21 +315,7 @@ public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, /// The instance. public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) { - return AddHandler(builder, typeof(EventHandlerDelegate).Name, type, sp => eventHandlerDelegate); - } - - /// - /// Add a to allow you to react to state changes in the provider or underlying flag management system, such as flag definition changes, provider readiness, or error conditions - /// - /// The instance. - /// The name of the . - /// The type to handle. - /// The handler which reacts to . - /// The instance. - /// Thrown when is null or empty - public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, EventHandlerDelegate eventHandlerDelegate) - { - return AddHandler(builder, handlerName, type, sp => eventHandlerDelegate); + return AddHandler(builder, type, _ => eventHandlerDelegate); } /// @@ -341,27 +327,7 @@ public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, str /// The instance. public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, ProviderEventTypes type, Func implementationFactory) { - return AddHandler(builder, typeof(EventHandlerDelegate).Name, type, implementationFactory); - } - - /// - /// Add a to allow you to react to state changes in the provider or underlying flag management system, such as flag definition changes, provider readiness, or error conditions - /// - /// The instance. - /// The name of the . - /// The type to handle. - /// The handler factory for creating a handler which reacts to . - /// The instance. - /// Thrown when is null or empty - public static OpenFeatureBuilder AddHandler(this OpenFeatureBuilder builder, string handlerName, ProviderEventTypes type, Func implementationFactory) - { - Guard.ThrowIfNullOrWhiteSpace(handlerName); - - var key = string.Join(":", handlerName, type.ToString()); - - builder.Services.PostConfigure(options => options.AddHandlerName(key)); - - builder.Services.AddKeyedSingleton(key, (serviceProvider, _) => + builder.Services.AddSingleton((serviceProvider) => { var handler = implementationFactory(serviceProvider); return new EventHandlerDelegateWrapper(type, handler); diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs index fd37b33cc..e9cc3cb12 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs @@ -58,16 +58,4 @@ internal void AddHookName(string name) _hookNames.Add(name); } } - - private readonly HashSet _handlerNames = []; - - internal IReadOnlyCollection HandlerNames => _handlerNames; - - internal void AddHandlerName(string name) - { - lock (_handlerNames) - { - _handlerNames.Add(name); - } - } } diff --git a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs index 1b00241e3..db9ac4e00 100644 --- a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs @@ -93,11 +93,7 @@ public async Task EnsureInitializedAsync_ShouldSetHandler_WhenHandlersAreRegiste var handler = new EventHandlerDelegateWrapper(ProviderEventTypes.ProviderReady, eventHandlerDelegate); _serviceCollection.AddSingleton(featureProvider) - .AddKeyedSingleton("test:ProviderReady", (_, key) => handler) - .Configure(options => - { - options.AddHandlerName("test:ProviderReady"); - }); + .AddSingleton(_ => handler); var serviceProvider = _serviceCollection.BuildServiceProvider(); var sut = new FeatureLifecycleManager(Api.Instance, serviceProvider, NullLogger.Instance); @@ -117,12 +113,8 @@ public async Task EnsureInitializedAsync_ShouldSetHandler_WhenMultipleHandlersAr var handler2 = new EventHandlerDelegateWrapper(ProviderEventTypes.ProviderReady, eventHandlerDelegate2); _serviceCollection.AddSingleton(featureProvider) - .AddKeyedSingleton("test:ProviderReady", (_, key) => handler1) - .AddKeyedSingleton("test:ProviderReady", (_, key) => handler2) - .Configure(options => - { - options.AddHandlerName("test:ProviderReady"); - }); + .AddSingleton(_ => handler1) + .AddSingleton(_ => handler2); var serviceProvider = _serviceCollection.BuildServiceProvider(); var sut = new FeatureLifecycleManager(Api.Instance, serviceProvider, NullLogger.Instance); diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index e26d58804..f742f98d8 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -308,12 +308,12 @@ public void AddHandler_AddsEventHandlerDelegateWrapperAsKeyedService() { // Arrange EventHandlerDelegate eventHandler = (eventDetails) => { }; - _systemUnderTest.AddHandler("test", Constant.ProviderEventTypes.ProviderReady, eventHandler); + _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, eventHandler); var serviceProvider = _services.BuildServiceProvider(); // Act - var handler = serviceProvider.GetKeyedService("test:ProviderReady"); + var handler = serviceProvider.GetService(); // Assert Assert.NotNull(handler); @@ -332,7 +332,7 @@ public void AddHandlerTwice_MultipleEventHandlerDelegateWrappersAsKeyedServices( var serviceProvider = _services.BuildServiceProvider(); // Act - var handler = serviceProvider.GetKeyedServices("EventHandlerDelegate:ProviderReady"); + var handler = serviceProvider.GetServices(); // Assert Assert.NotEmpty(handler); @@ -340,87 +340,20 @@ public void AddHandlerTwice_MultipleEventHandlerDelegateWrappersAsKeyedServices( Assert.Equal(eventHandler2, handler.ElementAt(1).EventHandlerDelegate); } - [Fact] - public void AddHandler_SetsHandlerNameInOpenFeatureOptions() - { - // Arrange - EventHandlerDelegate eventHandler = (eventDetails) => { }; - _systemUnderTest.AddHandler("test", Constant.ProviderEventTypes.ProviderReady, eventHandler); - - var serviceProvider = _services.BuildServiceProvider(); - - // Act - var options = serviceProvider.GetService>(); - var openFeatureOptions = options!.Value; - - // Assert - Assert.Contains("test:ProviderReady", openFeatureOptions.HandlerNames); - } - - [Fact] - public void AddHandler_WithoutName_AddsEventHandlerDelegateWrapperAsKeyedService() - { - // Arrange - EventHandlerDelegate eventHandler = (eventDetails) => { }; - _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, eventHandler); - - // Act - var handlers = _services.Where(s => s.ServiceType == typeof(EventHandlerDelegateWrapper)).ToList(); - var handler = handlers.First(); - - // Assert - Assert.True(handler.IsKeyedService); - Assert.Equal(ServiceLifetime.Singleton, handler.Lifetime); - } - - [Theory] - [InlineData("")] - [InlineData(" ")] - [InlineData(null)] - public void AddHandler_WithEmptyName_AddsEventHandlerDelegateWrapperAsKeyedService(string? handlerName) - { - // Arrange - EventHandlerDelegate eventHandler = (eventDetails) => { }; - - // Act - Action act = () => _systemUnderTest.AddHandler(handlerName!, Constant.ProviderEventTypes.ProviderReady, eventHandler); - - // Assert - var ex = Assert.Throws(act); - Assert.Equal("handlerName", ex.ParamName); - Assert.StartsWith("Value cannot be null.", ex.Message); - } - [Fact] public void AddHandler_WithImplementationFactory_AddsEventHandlerDelegateWrapperAsKeyedService() { // Arrange EventHandlerDelegate eventHandler = (eventDetails) => { }; - _systemUnderTest.AddHandler("test", Constant.ProviderEventTypes.ProviderReady, sp => eventHandler); + _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, _ => eventHandler); var serviceProvider = _services.BuildServiceProvider(); // Act - var handler = serviceProvider.GetKeyedService("test:ProviderReady"); + var handler = serviceProvider.GetService(); // Assert Assert.NotNull(handler); Assert.Equal(eventHandler, handler.EventHandlerDelegate); } - - [Fact] - public void AddHandlerWithImplementationFactory_WithoutName_AddsEventHandlerDelegateWrapperAsKeyedService() - { - // Arrange - EventHandlerDelegate eventHandler = (eventDetails) => { }; - _systemUnderTest.AddHandler(Constant.ProviderEventTypes.ProviderReady, sp => eventHandler); - - // Act - var handlers = _services.Where(s => s.ServiceType == typeof(EventHandlerDelegateWrapper)).ToList(); - var handler = handlers.First(); - - // Assert - Assert.True(handler.IsKeyedService); - Assert.Equal(ServiceLifetime.Singleton, handler.Lifetime); - } } diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs index a53f6c299..d911135e6 100644 --- a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; using OpenFeature.Constant; using OpenFeature.DependencyInjection; @@ -31,8 +32,7 @@ public class FeatureFlagIntegrationTest public async Task VerifyFeatureFlagBehaviorAcrossServiceLifetimesAsync(string userId, bool expectedResult, ServiceLifetime serviceLifetime) { // Arrange - var logger = new FakeLogger(); - using var server = await CreateServerAsync(serviceLifetime, logger, services => + using var server = await CreateServerAsync(serviceLifetime, services => { switch (serviceLifetime) { @@ -69,10 +69,17 @@ public async Task VerifyLoggingHookIsRegisteredAsync() { // Arrange var logger = new FakeLogger(); - using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, services => + Action configureServices = services => { services.AddTransient(); - }).ConfigureAwait(true); + }; + + Action openFeatureBuilder = cfg => + { + cfg.AddHook(_ => new LoggingHook(logger)); + }; + + using var server = await CreateServerAsync(ServiceLifetime.Transient, configureServices, openFeatureBuilder).ConfigureAwait(true); var client = server.CreateClient(); var requestUri = $"/features/{TestUserId}/flags/{FeatureA}"; @@ -95,7 +102,6 @@ public async Task VerifyLoggingHookIsRegisteredAsync() public async Task VerifyHandlerIsRegisteredAsync() { // Arrange - var logger = new FakeLogger(); Action configureServices = services => { services.AddTransient(); @@ -107,7 +113,7 @@ public async Task VerifyHandlerIsRegisteredAsync() cfg.AddHandler(ProviderEventTypes.ProviderReady, (_) => { handlerSuccess = true; }); }; - using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, configureServices, openFeatureBuilder) + using var server = await CreateServerAsync(ServiceLifetime.Transient, configureServices, openFeatureBuilder) .ConfigureAwait(true); var client = server.CreateClient(); @@ -117,6 +123,7 @@ public async Task VerifyHandlerIsRegisteredAsync() var response = await client.GetAsync(requestUri).ConfigureAwait(true); // Assert + Assert.True(response.IsSuccessStatusCode, "Expected HTTP status code 200 OK."); Assert.True(handlerSuccess); } @@ -124,7 +131,6 @@ public async Task VerifyHandlerIsRegisteredAsync() public async Task VerifyMultipleHandlersAreRegisteredAsync() { // Arrange - var logger = new FakeLogger(); Action configureServices = services => { services.AddTransient(); @@ -138,7 +144,7 @@ public async Task VerifyMultipleHandlersAreRegisteredAsync() cfg.AddHandler(ProviderEventTypes.ProviderReady, (_) => { lock (@lock) { counter++; } }); }; - using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, configureServices, openFeatureBuilder) + using var server = await CreateServerAsync(ServiceLifetime.Transient, configureServices, openFeatureBuilder) .ConfigureAwait(true); var client = server.CreateClient(); @@ -148,10 +154,45 @@ public async Task VerifyMultipleHandlersAreRegisteredAsync() var response = await client.GetAsync(requestUri).ConfigureAwait(true); // Assert + Assert.True(response.IsSuccessStatusCode, "Expected HTTP status code 200 OK."); Assert.Equal(2, counter); } - private static async Task CreateServerAsync(ServiceLifetime serviceLifetime, FakeLogger logger, + [Fact] + public async Task VerifyHandlersAreRegisteredWithServiceProviderAsync() + { + // Arrange + var logs = string.Empty; + Action configureServices = services => + { + services.AddFakeLogging(a => a.OutputSink = log => logs = string.Join('|', logs, log)); + services.AddTransient(); + }; + + Action openFeatureBuilder = cfg => + { + cfg.AddHandler(ProviderEventTypes.ProviderReady, sp => (@event) => + { + var innerLoger = sp.GetService>(); + innerLoger!.LogInformation("Handler invoked from builder!"); + }); + }; + + using var server = await CreateServerAsync(ServiceLifetime.Transient, configureServices, openFeatureBuilder) + .ConfigureAwait(true); + + var client = server.CreateClient(); + var requestUri = $"/features/{TestUserId}/flags/{FeatureA}"; + + // Act + var response = await client.GetAsync(requestUri).ConfigureAwait(true); + + // Assert + Assert.True(response.IsSuccessStatusCode, "Expected HTTP status code 200 OK."); + Assert.Contains("Handler invoked from builder!", logs); + } + + private static async Task CreateServerAsync(ServiceLifetime serviceLifetime, Action? configureServices = null, Action? openFeatureBuilder = null) { @@ -188,7 +229,6 @@ private static async Task CreateServerAsync(ServiceLifetime serviceL return flagService.GetFlags(); } }); - cfg.AddHook(serviceProvider => new LoggingHook(logger)); if (openFeatureBuilder is not null) {