From f6b9b92db81470bd01d5bfaa6fd271f655685f6f Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Tue, 29 Apr 2025 19:51:15 +0100 Subject: [PATCH 1/6] Add AddHook method in DependencyInjection package Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../Internal/FeatureLifecycleManager.cs | 9 ++++++++ .../OpenFeatureBuilderExtensions.cs | 22 +++++++++++++++++++ .../OpenFeatureOptions.cs | 12 ++++++++++ 3 files changed, 43 insertions(+) diff --git a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs index d14d421b6..f2c914f23 100644 --- a/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs +++ b/src/OpenFeature.DependencyInjection/Internal/FeatureLifecycleManager.cs @@ -34,6 +34,15 @@ public async ValueTask EnsureInitializedAsync(CancellationToken cancellationToke var featureProvider = _serviceProvider.GetRequiredKeyedService(name); await _featureApi.SetProviderAsync(name, featureProvider).ConfigureAwait(false); } + + var hooks = new List(); + foreach (var hookName in options.HookNames) + { + var hook = _serviceProvider.GetRequiredKeyedService(hookName); + hooks.Add(hook); + } + + _featureApi.AddHooks(hooks); } /// diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index a9c3f2586..f4ed52cd9 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -262,4 +262,26 @@ public static OpenFeatureBuilder AddPolicyName(this OpenFeatureBuilder /// The configured instance. public static OpenFeatureBuilder AddPolicyName(this OpenFeatureBuilder builder, Action configureOptions) => AddPolicyName(builder, configureOptions); + + /// + /// Adds a feature hook to the service collection using a factory method. Hooks added here are not domain-bound. + /// + /// The type of to be added. + /// The instance. + /// + /// + public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, Func implementationFactory) + where THook : Hook + { + var hookName = typeof(THook).Name; + + builder.Services.PostConfigure(options => options.AddHookName(hookName)); + + builder.Services.AddKeyedSingleton(hookName, (serviceProvider, key) => + { + return implementationFactory(serviceProvider); + }); + + return builder; + } } diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs index b2f15e442..e9cc3cb12 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs @@ -46,4 +46,16 @@ protected internal void AddProviderName(string? name) } } } + + private readonly HashSet _hookNames = []; + + internal IReadOnlyCollection HookNames => _hookNames; + + internal void AddHookName(string name) + { + lock (_hookNames) + { + _hookNames.Add(name); + } + } } From 615008f362f76784c5662befa339305ebdb69cea Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Tue, 29 Apr 2025 20:21:06 +0100 Subject: [PATCH 2/6] Add unit tests for AddHook extension method Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../FeatureLifecycleManagerTests.cs | 65 ++++++++++++------- .../NoOpHook.cs | 26 ++++++++ .../OpenFeatureBuilderExtensionsTests.cs | 30 +++++++++ 3 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 test/OpenFeature.DependencyInjection.Tests/NoOpHook.cs diff --git a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs index c55736047..47cc7df5c 100644 --- a/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/FeatureLifecycleManagerTests.cs @@ -1,7 +1,6 @@ using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; -using NSubstitute; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging.Abstractions; using OpenFeature.DependencyInjection.Internal; using Xunit; @@ -9,27 +8,18 @@ namespace OpenFeature.DependencyInjection.Tests; public class FeatureLifecycleManagerTests { - private readonly FeatureLifecycleManager _systemUnderTest; - private readonly IServiceProvider _mockServiceProvider; + private readonly IServiceCollection _serviceCollection; public FeatureLifecycleManagerTests() { Api.Instance.SetContext(null); Api.Instance.ClearHooks(); - _mockServiceProvider = Substitute.For(); - - var options = new OpenFeatureOptions(); - options.AddDefaultProviderName(); - var optionsMock = Substitute.For>(); - optionsMock.Value.Returns(options); - - _mockServiceProvider.GetService>().Returns(optionsMock); - - _systemUnderTest = new FeatureLifecycleManager( - Api.Instance, - _mockServiceProvider, - Substitute.For>()); + _serviceCollection = new ServiceCollection() + .Configure(options => + { + options.AddDefaultProviderName(); + }); } [Fact] @@ -37,10 +27,13 @@ public async Task EnsureInitializedAsync_ShouldLogAndSetProvider_WhenProviderExi { // Arrange var featureProvider = new NoOpFeatureProvider(); - _mockServiceProvider.GetService(typeof(FeatureProvider)).Returns(featureProvider); + _serviceCollection.AddSingleton(featureProvider); + + var serviceProvider = _serviceCollection.BuildServiceProvider(); + var sut = new FeatureLifecycleManager(Api.Instance, serviceProvider, NullLogger.Instance); // Act - await _systemUnderTest.EnsureInitializedAsync().ConfigureAwait(true); + await sut.EnsureInitializedAsync().ConfigureAwait(true); // Assert Assert.Equal(featureProvider, Api.Instance.GetProvider()); @@ -50,14 +43,42 @@ public async Task EnsureInitializedAsync_ShouldLogAndSetProvider_WhenProviderExi public async Task EnsureInitializedAsync_ShouldThrowException_WhenProviderDoesNotExist() { // Arrange - _mockServiceProvider.GetService(typeof(FeatureProvider)).Returns(null as FeatureProvider); + _serviceCollection.RemoveAll(); + + var serviceProvider = _serviceCollection.BuildServiceProvider(); + var sut = new FeatureLifecycleManager(Api.Instance, serviceProvider, NullLogger.Instance); // Act - var act = () => _systemUnderTest.EnsureInitializedAsync().AsTask(); + var act = () => sut.EnsureInitializedAsync().AsTask(); // Assert var exception = await Assert.ThrowsAsync(act).ConfigureAwait(true); Assert.NotNull(exception); Assert.False(string.IsNullOrWhiteSpace(exception.Message)); } + + [Fact] + public async Task EnsureInitializedAsync_ShouldSetHook_WhenHooksAreRegistered() + { + // Arrange + var featureProvider = new NoOpFeatureProvider(); + var hook = new NoOpHook(); + + _serviceCollection.AddSingleton(featureProvider) + .AddKeyedSingleton("NoOpHook", (_, key) => hook) + .Configure(options => + { + options.AddHookName("NoOpHook"); + }); + + var serviceProvider = _serviceCollection.BuildServiceProvider(); + var sut = new FeatureLifecycleManager(Api.Instance, serviceProvider, NullLogger.Instance); + + // Act + await sut.EnsureInitializedAsync().ConfigureAwait(true); + + // Assert + var actual = Api.Instance.GetHooks().FirstOrDefault(); + Assert.Equal(hook, actual); + } } diff --git a/test/OpenFeature.DependencyInjection.Tests/NoOpHook.cs b/test/OpenFeature.DependencyInjection.Tests/NoOpHook.cs new file mode 100644 index 000000000..cee6ef1df --- /dev/null +++ b/test/OpenFeature.DependencyInjection.Tests/NoOpHook.cs @@ -0,0 +1,26 @@ +using OpenFeature.Model; + +namespace OpenFeature.DependencyInjection.Tests; + +internal class NoOpHook : Hook +{ + public override ValueTask BeforeAsync(HookContext context, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + { + return base.BeforeAsync(context, hints, cancellationToken); + } + + public override ValueTask AfterAsync(HookContext context, FlagEvaluationDetails details, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + { + return base.AfterAsync(context, details, hints, cancellationToken); + } + + public override ValueTask FinallyAsync(HookContext context, FlagEvaluationDetails evaluationDetails, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + { + return base.FinallyAsync(context, evaluationDetails, hints, cancellationToken); + } + + public override ValueTask ErrorAsync(HookContext context, Exception error, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + { + return base.ErrorAsync(context, error, hints, cancellationToken); + } +} diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index 6985125d9..c05a242fa 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -241,4 +241,34 @@ public void AddProvider_ConfiguresPolicyNameAcrossMultipleProviderSetups(int pro Assert.NotNull(provider); Assert.IsType(provider); } + + [Fact] + public void AddHook_AddsHookAsKeyedService() + { + // Arrange + _systemUnderTest.AddHook(sp => new NoOpHook()); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var hook = serviceProvider.GetKeyedService("NoOpHook"); + + // Assert + Assert.NotNull(hook); + } + + [Fact] + public void AddHook_AddsNameNameToOpenFeatureOptions() + { + // Arrange + _systemUnderTest.AddHook(sp => new NoOpHook()); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var options = serviceProvider.GetRequiredService>(); + + // Assert + Assert.Contains(options.Value.HookNames, t => t == "NoOpHook"); + } } From cbd7aa46610a9979073b803a545a71f480bcdb51 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Wed, 30 Apr 2025 19:21:29 +0100 Subject: [PATCH 3/6] Mark AddHook implementationFactory as optional * This will allow consumers to easily add Hooks without forcing them to write a function everytime they do Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../OpenFeatureBuilderExtensions.cs | 17 ++++++++++++----- .../OpenFeatureBuilderExtensionsTests.cs | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index f4ed52cd9..97821b406 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -268,19 +268,26 @@ public static OpenFeatureBuilder AddPolicyName(this OpenFeatureBuilder builder, /// /// The type of to be added. /// The instance. - /// + /// Optional factory for controlling how will be created in the DI container. /// - public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, Func implementationFactory) + public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, Func? implementationFactory = null) where THook : Hook { var hookName = typeof(THook).Name; builder.Services.PostConfigure(options => options.AddHookName(hookName)); - builder.Services.AddKeyedSingleton(hookName, (serviceProvider, key) => + if (implementationFactory is not null) { - return implementationFactory(serviceProvider); - }); + builder.Services.AddKeyedSingleton(hookName, (serviceProvider, key) => + { + return implementationFactory(serviceProvider); + }); + } + else + { + builder.Services.AddKeyedSingleton(typeof(Hook), hookName, typeof(THook)); + } return builder; } diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index c05a242fa..9359a4979 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -246,7 +246,7 @@ public void AddProvider_ConfiguresPolicyNameAcrossMultipleProviderSetups(int pro public void AddHook_AddsHookAsKeyedService() { // Arrange - _systemUnderTest.AddHook(sp => new NoOpHook()); + _systemUnderTest.AddHook(); var serviceProvider = _services.BuildServiceProvider(); From 0138fffed3693df76e22feaae2456256228424c1 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Thu, 1 May 2025 17:59:32 +0100 Subject: [PATCH 4/6] Update README file with new AddHook DI method Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 3e51a690f..b0063d3a4 100644 --- a/README.md +++ b/README.md @@ -434,6 +434,7 @@ builder.Services.AddOpenFeature(featureBuilder => { featureBuilder .AddHostedFeatureLifecycle() // From Hosting package .AddContext((contextBuilder, serviceProvider) => { /* Custom context configuration */ }) + .AddHook() .AddInMemoryProvider(); }); ``` @@ -446,6 +447,7 @@ builder.Services.AddOpenFeature(featureBuilder => { featureBuilder .AddHostedFeatureLifecycle() .AddContext((contextBuilder, serviceProvider) => { /* Custom context configuration */ }) + .AddHook((serviceProvider) => new LoggingHook( /* Custom configuration */ )) .AddInMemoryProvider("name1") .AddInMemoryProvider("name2") .AddPolicyName(options => { From 41bde563e65b8bc066e92701f4bcf651e7a80203 Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Thu, 1 May 2025 22:47:40 +0100 Subject: [PATCH 5/6] Add ability for consumers to specify Hook name * Add unit tests to cover off new AddHook extension method Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../OpenFeatureBuilderExtensions.cs | 20 +++++++++--- .../OpenFeatureBuilderExtensionsTests.cs | 32 ++++++++++++++++++- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index 97821b406..8f79f3946 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -269,24 +269,36 @@ public static OpenFeatureBuilder AddPolicyName(this OpenFeatureBuilder builder, /// The type of to be added. /// The instance. /// Optional factory for controlling how will be created in the DI container. - /// + /// The instance. public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, Func? implementationFactory = null) where THook : Hook { - var hookName = typeof(THook).Name; + return builder.AddHook(typeof(THook).Name, implementationFactory); + } + /// + /// Adds a feature hook to the service collection using a factory method and specified name. Hooks added here are not domain-bound. + /// + /// The type of to be added. + /// The instance. + /// The name of the that is being added. + /// Optional factory for controlling how will be created in the DI container. + /// The instance. + public static OpenFeatureBuilder AddHook(this OpenFeatureBuilder builder, string hookName, Func? implementationFactory = null) + where THook : Hook + { builder.Services.PostConfigure(options => options.AddHookName(hookName)); if (implementationFactory is not null) { - builder.Services.AddKeyedSingleton(hookName, (serviceProvider, key) => + builder.Services.TryAddKeyedSingleton(hookName, (serviceProvider, key) => { return implementationFactory(serviceProvider); }); } else { - builder.Services.AddKeyedSingleton(typeof(Hook), hookName, typeof(THook)); + builder.Services.TryAddKeyedSingleton(hookName); } return builder; diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index 9359a4979..07597703a 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -258,7 +258,7 @@ public void AddHook_AddsHookAsKeyedService() } [Fact] - public void AddHook_AddsNameNameToOpenFeatureOptions() + public void AddHook_AddsHookNameToOpenFeatureOptions() { // Arrange _systemUnderTest.AddHook(sp => new NoOpHook()); @@ -271,4 +271,34 @@ public void AddHook_AddsNameNameToOpenFeatureOptions() // Assert Assert.Contains(options.Value.HookNames, t => t == "NoOpHook"); } + + [Fact] + public void AddHook_WithSpecifiedNameToOpenFeatureOptions() + { + // Arrange + _systemUnderTest.AddHook("my-custom-name"); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var hook = serviceProvider.GetKeyedService("my-custom-name"); + + // Assert + Assert.NotNull(hook); + } + + [Fact] + public void AddHook_WithSpecifiedNameAndImplementationFactory_AsKeyedService() + { + // Arrange + _systemUnderTest.AddHook("my-custom-name", (serviceProvider) => new NoOpHook()); + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var hook = serviceProvider.GetKeyedService("my-custom-name"); + + // Assert + Assert.NotNull(hook); + } } From ef6dff8faf3871c09b108aa689ef57570b3b478f Mon Sep 17 00:00:00 2001 From: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> Date: Fri, 2 May 2025 20:30:53 +0100 Subject: [PATCH 6/6] Add integration test for AddHook dependency injection method Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com> --- .../FeatureFlagIntegrationTest.cs | 38 +++++++++++++++++-- .../OpenFeature.IntegrationTests.csproj | 1 + 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs index 2f9746eb7..9e1f4bca9 100644 --- a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -5,7 +5,9 @@ using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging.Testing; using OpenFeature.DependencyInjection.Providers.Memory; +using OpenFeature.Hooks; using OpenFeature.IntegrationTests.Services; using OpenFeature.Providers.Memory; @@ -27,7 +29,8 @@ public class FeatureFlagIntegrationTest public async Task VerifyFeatureFlagBehaviorAcrossServiceLifetimesAsync(string userId, bool expectedResult, ServiceLifetime serviceLifetime) { // Arrange - using var server = await CreateServerAsync(serviceLifetime, services => + var logger = new FakeLogger(); + using var server = await CreateServerAsync(serviceLifetime, logger, services => { switch (serviceLifetime) { @@ -50,7 +53,7 @@ public async Task VerifyFeatureFlagBehaviorAcrossServiceLifetimesAsync(string us // Act var response = await client.GetAsync(requestUri).ConfigureAwait(true); - var responseContent = await response.Content.ReadFromJsonAsync>().ConfigureAwait(true); ; + var responseContent = await response.Content.ReadFromJsonAsync>().ConfigureAwait(true); // Assert Assert.True(response.IsSuccessStatusCode, "Expected HTTP status code 200 OK."); @@ -59,7 +62,35 @@ public async Task VerifyFeatureFlagBehaviorAcrossServiceLifetimesAsync(string us Assert.Equal(expectedResult, responseContent.FeatureValue); } - private static async Task CreateServerAsync(ServiceLifetime serviceLifetime, Action? configureServices = null) + [Fact] + public async Task VerifyLoggingHookIsRegisteredAsync() + { + // Arrange + var logger = new FakeLogger(); + using var server = await CreateServerAsync(ServiceLifetime.Transient, logger, services => + { + services.AddTransient(); + }).ConfigureAwait(true); + + var client = server.CreateClient(); + var requestUri = $"/features/{TestUserId}/flags/{FeatureA}"; + + // Act + var response = await client.GetAsync(requestUri).ConfigureAwait(true); + var logs = logger.Collector.GetSnapshot(); + + // Assert + Assert.True(response.IsSuccessStatusCode, "Expected HTTP status code 200 OK."); + Assert.Equal(4, logs.Count); + Assert.Multiple(() => + { + Assert.Contains("Before Flag Evaluation", logs[0].Message); + Assert.Contains("After Flag Evaluation", logs[1].Message); + }); + } + + private static async Task CreateServerAsync(ServiceLifetime serviceLifetime, FakeLogger logger, + Action? configureServices = null) { var builder = WebApplication.CreateBuilder(); builder.WebHost.UseTestServer(); @@ -94,6 +125,7 @@ private static async Task CreateServerAsync(ServiceLifetime serviceL return flagService.GetFlags(); } }); + cfg.AddHook(serviceProvider => new LoggingHook(logger)); }); var app = builder.Build(); diff --git a/test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj b/test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj index baf5fdfb7..151c61b9d 100644 --- a/test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj +++ b/test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj @@ -13,6 +13,7 @@ + runtime; build; native; contentfiles; analyzers; buildtransitive