From b5ddf62c79ff9fc73d7d847501bcf8bafe89ac00 Mon Sep 17 00:00:00 2001 From: Artyom Tonoyan Date: Thu, 28 Nov 2024 21:22:18 +0400 Subject: [PATCH 1/7] feat(di): Enhance feature provider registration for improved user experience Signed-off-by: Artyom Tonoyan --- .../IFeatureProviderFactory.cs | 23 --- .../OpenFeatureBuilderExtensions.cs | 173 ++++++++---------- .../OpenFeatureOptions.cs | 4 +- .../OpenFeatureServiceCollectionExtensions.cs | 11 +- .../Memory/FeatureBuilderExtensions.cs | 109 +++++++++-- .../Memory/InMemoryProviderFactory.cs | 34 ---- .../Memory/InMemoryProviderOptions.cs | 19 ++ .../NoOpFeatureProviderFactory.cs | 9 - .../OpenFeatureBuilderExtensionsTests.cs | 171 +++++++++++++++-- ...FeatureServiceCollectionExtensionsTests.cs | 1 + 10 files changed, 352 insertions(+), 202 deletions(-) delete mode 100644 src/OpenFeature.DependencyInjection/IFeatureProviderFactory.cs delete mode 100644 src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderFactory.cs create mode 100644 src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderOptions.cs delete mode 100644 test/OpenFeature.DependencyInjection.Tests/NoOpFeatureProviderFactory.cs diff --git a/src/OpenFeature.DependencyInjection/IFeatureProviderFactory.cs b/src/OpenFeature.DependencyInjection/IFeatureProviderFactory.cs deleted file mode 100644 index 8c40cee3..00000000 --- a/src/OpenFeature.DependencyInjection/IFeatureProviderFactory.cs +++ /dev/null @@ -1,23 +0,0 @@ -namespace OpenFeature.DependencyInjection; - -/// -/// Provides a contract for creating instances of . -/// This factory interface enables custom configuration and initialization of feature providers -/// to support domain-specific or application-specific feature flag management. -/// -#if NET8_0_OR_GREATER -[System.Diagnostics.CodeAnalysis.Experimental(Diagnostics.FeatureCodes.NewDi)] -#endif -public interface IFeatureProviderFactory -{ - /// - /// Creates an instance of a configured according to - /// the specific settings implemented by the concrete factory. - /// - /// - /// A new instance of . - /// The configuration and behavior of this provider instance are determined by - /// the implementation of this method. - /// - FeatureProvider Create(); -} diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs index a494b045..a9c3f258 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureBuilderExtensions.cs @@ -21,9 +21,7 @@ public static partial class OpenFeatureBuilderExtensions /// the desired configuration /// The instance. /// Thrown when the or action is null. - public static OpenFeatureBuilder AddContext( - this OpenFeatureBuilder builder, - Action configure) + public static OpenFeatureBuilder AddContext(this OpenFeatureBuilder builder, Action configure) { Guard.ThrowIfNull(builder); Guard.ThrowIfNull(configure); @@ -38,9 +36,7 @@ public static OpenFeatureBuilder AddContext( /// the desired configuration /// The instance. /// Thrown when the or action is null. - public static OpenFeatureBuilder AddContext( - this OpenFeatureBuilder builder, - Action configure) + public static OpenFeatureBuilder AddContext(this OpenFeatureBuilder builder, Action configure) { Guard.ThrowIfNull(builder); Guard.ThrowIfNull(configure); @@ -57,122 +53,106 @@ public static OpenFeatureBuilder AddContext( } /// - /// Adds a new feature provider with specified options and configuration builder. + /// Adds a feature provider using a factory method without additional configuration options. + /// This method adds the feature provider as a transient service and sets it as the default provider within the application. /// - /// The type for configuring the feature provider. - /// The type of the provider factory implementing . - /// The instance. - /// An optional action to configure the provider factory of type . - /// The instance. - /// Thrown when the is null. - public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, Action? configureFactory = null) + /// The used to configure feature flags. + /// + /// A factory method that creates and returns a + /// instance based on the provided service provider. + /// + /// The updated instance with the default feature provider set and configured. + /// Thrown if the is null, as a valid builder is required to add and configure providers. + public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, Func implementationFactory) + => AddProvider(builder, implementationFactory, null); + + /// + /// Adds a feature provider using a factory method to create the provider instance and optionally configures its settings. + /// This method adds the feature provider as a transient service and sets it as the default provider within the application. + /// + /// Type derived from used to configure the feature provider. + /// The used to configure feature flags. + /// + /// A factory method that creates and returns a + /// instance based on the provided service provider. + /// + /// An optional delegate to configure the provider-specific options. + /// The updated instance with the default feature provider set and configured. + /// Thrown if the is null, as a valid builder is required to add and configure providers. + public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, Func implementationFactory, Action? configureOptions) where TOptions : OpenFeatureOptions - where TProviderFactory : class, IFeatureProviderFactory { Guard.ThrowIfNull(builder); builder.HasDefaultProvider = true; - - builder.Services.Configure(options => - { - options.AddDefaultProviderName(); - }); - - if (configureFactory != null) + builder.Services.PostConfigure(options => options.AddDefaultProviderName()); + if (configureOptions != null) { - builder.Services.AddOptions() - .Validate(options => options != null, $"{typeof(TProviderFactory).Name} configuration is invalid.") - .Configure(configureFactory); + builder.Services.Configure(configureOptions); } - else - { - builder.Services.AddOptions() - .Configure(options => { }); - } - - builder.Services.TryAddSingleton(static provider => - { - var providerFactory = provider.GetRequiredService>().Value; - return providerFactory.Create(); - }); + builder.Services.TryAddTransient(implementationFactory); builder.AddClient(); - return builder; } /// - /// Adds a new feature provider with the default type and a specified configuration builder. - /// - /// The type of the provider factory implementing . - /// The instance. - /// An optional action to configure the provider factory of type . - /// The configured instance. - /// Thrown when the is null. - public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, Action? configureFactory = null) - where TProviderFactory : class, IFeatureProviderFactory - => AddProvider(builder, configureFactory); - - /// - /// Adds a feature provider with specified options and configuration builder for the specified domain. + /// Adds a feature provider for a specific domain using provided options and a configuration builder. /// - /// The type for configuring the feature provider. - /// The type of the provider factory implementing . - /// The instance. + /// Type derived from used to configure the feature provider. + /// The used to configure feature flags. /// The unique name of the provider. - /// An optional action to configure the provider factory of type . - /// The instance. - /// Thrown when the or is null or empty. - public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, string domain, Action? configureFactory = null) + /// + /// A factory method that creates a feature provider instance. + /// It adds the provider as a transient service unless it is already added. + /// + /// An optional delegate to configure the provider-specific options. + /// The updated instance with the new feature provider configured. + /// + /// Thrown if either or is null or if the is empty. + /// + public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, string domain, Func implementationFactory, Action? configureOptions) where TOptions : OpenFeatureOptions - where TProviderFactory : class, IFeatureProviderFactory { Guard.ThrowIfNull(builder); - Guard.ThrowIfNullOrWhiteSpace(domain, nameof(domain)); builder.DomainBoundProviderRegistrationCount++; - builder.Services.Configure(options => - { - options.AddProviderName(domain); - }); - - if (configureFactory != null) - { - builder.Services.AddOptions(domain) - .Validate(options => options != null, $"{typeof(TProviderFactory).Name} configuration is invalid.") - .Configure(configureFactory); - } - else + builder.Services.PostConfigure(options => options.AddProviderName(domain)); + if (configureOptions != null) { - builder.Services.AddOptions(domain) - .Configure(options => { }); + builder.Services.Configure(domain, configureOptions); } - builder.Services.TryAddKeyedSingleton(domain, static (provider, key) => + builder.Services.TryAddKeyedTransient(domain, (provider, key) => { - var options = provider.GetRequiredService>(); - var providerFactory = options.Get(key!.ToString()); - return providerFactory.Create(); + if (key == null) + { + throw new ArgumentNullException(nameof(key)); + } + return implementationFactory(provider, key.ToString()!); }); builder.AddClient(domain); - return builder; } /// - /// Adds a feature provider with a specified configuration builder for the specified domain, using default . + /// Adds a feature provider for a specified domain using the default options. + /// This method configures a feature provider without custom options, delegating to the more generic AddProvider method. /// - /// The type of the provider factory implementing . - /// The instance. - /// The unique domain of the provider. - /// An optional action to configure the provider factory of type . - /// The configured instance. - /// Thrown when the or is null or empty. - public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, string domain, Action? configureFactory = null) - where TProviderFactory : class, IFeatureProviderFactory - => AddProvider(builder, domain, configureFactory); + /// The used to configure feature flags. + /// The unique name of the provider. + /// + /// A factory method that creates a feature provider instance. + /// It adds the provider as a transient service unless it is already added. + /// + /// The updated instance with the new feature provider configured. + /// + /// Thrown if either or is null or if the is empty. + /// + public static OpenFeatureBuilder AddProvider(this OpenFeatureBuilder builder, string domain, Func implementationFactory) + => AddProvider(builder, domain, implementationFactory, configureOptions: null); /// /// Adds a feature client to the service collection, configuring it to work with a specific context if provided. @@ -231,19 +211,24 @@ internal static OpenFeatureBuilder AddClient(this OpenFeatureBuilder builder, st } /// - /// Configures a default client for OpenFeature using the provided factory function. + /// Adds a default to the based on the policy name options. + /// This method configures the dependency injection container to resolve the appropriate + /// depending on the policy name selected. + /// If no name is selected (i.e., null), it retrieves the default client. /// /// The instance. - /// - /// A factory function that creates an based on the service provider and . - /// /// The configured instance. - internal static OpenFeatureBuilder AddDefaultClient(this OpenFeatureBuilder builder, Func clientFactory) + internal static OpenFeatureBuilder AddPolicyBasedClient(this OpenFeatureBuilder builder) { builder.Services.AddScoped(provider => { var policy = provider.GetRequiredService>().Value; - return clientFactory(provider, policy); + var name = policy.DefaultNameSelector(provider); + if (name == null) + { + return provider.GetRequiredService(); + } + return provider.GetRequiredKeyedService(name); }); return builder; diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs index 1be312ed..b2f15e44 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureOptions.cs @@ -26,13 +26,13 @@ public class OpenFeatureOptions /// Registers the default provider name if no specific name is provided. /// Sets to true. /// - public void AddDefaultProviderName() => AddProviderName(null); + protected internal void AddDefaultProviderName() => AddProviderName(null); /// /// Registers a new feature provider name. This operation is thread-safe. /// /// The name of the feature provider to register. Registers as default if null. - public void AddProviderName(string? name) + protected internal void AddProviderName(string? name) { if (string.IsNullOrWhiteSpace(name)) { diff --git a/src/OpenFeature.DependencyInjection/OpenFeatureServiceCollectionExtensions.cs b/src/OpenFeature.DependencyInjection/OpenFeatureServiceCollectionExtensions.cs index e7a503bb..74d01ad3 100644 --- a/src/OpenFeature.DependencyInjection/OpenFeatureServiceCollectionExtensions.cs +++ b/src/OpenFeature.DependencyInjection/OpenFeatureServiceCollectionExtensions.cs @@ -53,16 +53,7 @@ public static IServiceCollection AddOpenFeature(this IServiceCollection services }); } - builder.AddDefaultClient((provider, policy) => - { - var name = policy.DefaultNameSelector.Invoke(provider); - if (name == null) - { - return provider.GetRequiredService(); - } - return provider.GetRequiredKeyedService(name); - }); - + builder.AddPolicyBasedClient(); return services; } } diff --git a/src/OpenFeature.DependencyInjection/Providers/Memory/FeatureBuilderExtensions.cs b/src/OpenFeature.DependencyInjection/Providers/Memory/FeatureBuilderExtensions.cs index 199e01b0..d6346ad7 100644 --- a/src/OpenFeature.DependencyInjection/Providers/Memory/FeatureBuilderExtensions.cs +++ b/src/OpenFeature.DependencyInjection/Providers/Memory/FeatureBuilderExtensions.cs @@ -1,3 +1,5 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using OpenFeature.Providers.Memory; namespace OpenFeature.DependencyInjection.Providers.Memory; @@ -10,46 +12,115 @@ namespace OpenFeature.DependencyInjection.Providers.Memory; #endif public static partial class FeatureBuilderExtensions { + /// + /// Adds an in-memory feature provider to the with a factory for flags. + /// + /// The instance to configure. + /// + /// A factory function to provide an of flags. + /// If null, an empty provider will be created. + /// + /// The instance for chaining. + public static OpenFeatureBuilder AddInMemoryProvider(this OpenFeatureBuilder builder, Func?> flagsFactory) + => builder.AddProvider(provider => + { + var flags = flagsFactory(provider); + if (flags == null) + { + return new InMemoryProvider(); + } + + return new InMemoryProvider(flags); + }); + + /// + /// Adds an in-memory feature provider to the with a domain and factory for flags. + /// + /// The instance to configure. + /// The unique domain of the provider. + /// + /// A factory function to provide an of flags. + /// If null, an empty provider will be created. + /// + /// The instance for chaining. + public static OpenFeatureBuilder AddInMemoryProvider(this OpenFeatureBuilder builder, string domain, Func?> flagsFactory) + => AddInMemoryProvider(builder, domain, (provider, _) => flagsFactory(provider)); + + /// + /// Adds an in-memory feature provider to the with a domain and contextual flag factory. + /// If null, an empty provider will be created. + /// + /// The instance to configure. + /// The unique domain of the provider. + /// + /// A factory function to provide an of flags based on service provider and domain. + /// + /// The instance for chaining. + public static OpenFeatureBuilder AddInMemoryProvider(this OpenFeatureBuilder builder, string domain, Func?> flagsFactory) + => builder.AddProvider(domain, (provider, key) => + { + var flags = flagsFactory(provider, key); + if (flags == null) + { + return new InMemoryProvider(); + } + + return new InMemoryProvider(flags); + }); + /// /// Adds an in-memory feature provider to the with optional flag configuration. /// /// The instance to configure. /// /// An optional delegate to configure feature flags in the in-memory provider. - /// If provided, it allows setting up the initial flags. + /// If null, an empty provider will be created. /// /// The instance for chaining. public static OpenFeatureBuilder AddInMemoryProvider(this OpenFeatureBuilder builder, Action>? configure = null) - => builder.AddProvider(factory => ConfigureFlags(factory, configure)); + => builder.AddProvider(CreateProvider, options => ConfigureFlags(options, configure)); /// - /// Adds an in-memory feature provider with a specific domain to the - /// with optional flag configuration. + /// Adds an in-memory feature provider with a specific domain to the with optional flag configuration. /// /// The instance to configure. /// The unique domain of the provider /// /// An optional delegate to configure feature flags in the in-memory provider. - /// If provided, it allows setting up the initial flags. + /// If null, an empty provider will be created. /// /// The instance for chaining. public static OpenFeatureBuilder AddInMemoryProvider(this OpenFeatureBuilder builder, string domain, Action>? configure = null) - => builder.AddProvider(domain, factory => ConfigureFlags(factory, configure)); + => builder.AddProvider(domain, CreateProvider, options => ConfigureFlags(options, configure)); - /// - /// Configures the feature flags for an instance. - /// - /// The to configure. - /// - /// An optional delegate that sets up the initial flags in the provider's flag dictionary. - /// - private static void ConfigureFlags(InMemoryProviderFactory factory, Action>? configure) + private static FeatureProvider CreateProvider(IServiceProvider provider, string domain) { - if (configure == null) - return; + var options = provider.GetRequiredService>().Get(domain); + if (options.Flags == null) + { + return new InMemoryProvider(); + } - var flag = new Dictionary(); - configure.Invoke(flag); - factory.Flags = flag; + return new InMemoryProvider(options.Flags); + } + + private static FeatureProvider CreateProvider(IServiceProvider provider) + { + var options = provider.GetRequiredService>().Value; + if (options.Flags == null) + { + return new InMemoryProvider(); + } + + return new InMemoryProvider(options.Flags); + } + + private static void ConfigureFlags(InMemoryProviderOptions options, Action>? configure) + { + if (configure != null) + { + options.Flags = new Dictionary(); + configure.Invoke(options.Flags); + } } } diff --git a/src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderFactory.cs b/src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderFactory.cs deleted file mode 100644 index 2d155dd9..00000000 --- a/src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderFactory.cs +++ /dev/null @@ -1,34 +0,0 @@ -using OpenFeature.Providers.Memory; - -namespace OpenFeature.DependencyInjection.Providers.Memory; - -/// -/// A factory for creating instances of , -/// an in-memory implementation of . -/// This factory allows for the customization of feature flags to facilitate -/// testing and lightweight feature flag management without external dependencies. -/// -#if NET8_0_OR_GREATER -[System.Diagnostics.CodeAnalysis.Experimental(Diagnostics.FeatureCodes.NewDi)] -#endif -public class InMemoryProviderFactory : IFeatureProviderFactory -{ - /// - /// Gets or sets the collection of feature flags used to configure the - /// instances. This dictionary maps - /// flag names to instances, enabling pre-configuration - /// of features for testing or in-memory evaluation. - /// - internal IDictionary? Flags { get; set; } - - /// - /// Creates a new instance of with the specified - /// flags set in . This instance is configured for in-memory - /// feature flag management, suitable for testing or lightweight feature toggling scenarios. - /// - /// - /// A configured that can be used to manage - /// feature flags in an in-memory context. - /// - public FeatureProvider Create() => new InMemoryProvider(Flags); -} diff --git a/src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderOptions.cs b/src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderOptions.cs new file mode 100644 index 00000000..ea5433f4 --- /dev/null +++ b/src/OpenFeature.DependencyInjection/Providers/Memory/InMemoryProviderOptions.cs @@ -0,0 +1,19 @@ +using OpenFeature.Providers.Memory; + +namespace OpenFeature.DependencyInjection.Providers.Memory; + +/// +/// Options for configuring the in-memory feature flag provider. +/// +public class InMemoryProviderOptions : OpenFeatureOptions +{ + /// + /// Gets or sets the feature flags to be used by the in-memory provider. + /// + /// + /// This property allows you to specify a dictionary of flags where the key is the flag name + /// and the value is the corresponding instance. + /// If no flags are provided, the in-memory provider will start with an empty set of flags. + /// + public IDictionary? Flags { get; set; } +} diff --git a/test/OpenFeature.DependencyInjection.Tests/NoOpFeatureProviderFactory.cs b/test/OpenFeature.DependencyInjection.Tests/NoOpFeatureProviderFactory.cs deleted file mode 100644 index 1ee14bf0..00000000 --- a/test/OpenFeature.DependencyInjection.Tests/NoOpFeatureProviderFactory.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace OpenFeature.DependencyInjection.Tests; - -#if NET8_0_OR_GREATER -[System.Diagnostics.CodeAnalysis.Experimental(Diagnostics.FeatureCodes.NewDi)] -#endif -public class NoOpFeatureProviderFactory : IFeatureProviderFactory -{ - public FeatureProvider Create() => new NoOpFeatureProvider(); -} diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs index 3f6ef227..087336a0 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureBuilderExtensionsTests.cs @@ -1,5 +1,6 @@ using FluentAssertions; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using OpenFeature.Model; using Xunit; @@ -22,12 +23,12 @@ public OpenFeatureBuilderExtensionsTests() public void AddContext_Delegate_ShouldAddServiceToCollection(bool useServiceProviderDelegate) { // Act - var result = useServiceProviderDelegate ? + var featureBuilder = useServiceProviderDelegate ? _systemUnderTest.AddContext(_ => { }) : _systemUnderTest.AddContext((_, _) => { }); // Assert - result.Should().BeSameAs(_systemUnderTest, "The method should return the same builder instance."); + featureBuilder.Should().BeSameAs(_systemUnderTest, "The method should return the same builder instance."); _systemUnderTest.IsContextConfigured.Should().BeTrue("The context should be configured."); _services.Should().ContainSingle(serviceDescriptor => serviceDescriptor.ServiceType == typeof(EvaluationContext) && @@ -61,37 +62,185 @@ public void AddContext_Delegate_ShouldCorrectlyHandles(bool useServiceProviderDe #if NET8_0_OR_GREATER [System.Diagnostics.CodeAnalysis.Experimental(Diagnostics.FeatureCodes.NewDi)] #endif - [Fact] - public void AddProvider_ShouldAddProviderToCollection() + [Theory] + [InlineData(1, true, 0)] + [InlineData(2, false, 1)] + [InlineData(3, true, 0)] + [InlineData(4, false, 1)] + public void AddProvider_ShouldAddProviderToCollection(int providerRegistrationType, bool expectsDefaultProvider, int expectsDomainBoundProvider) { // Act - var result = _systemUnderTest.AddProvider(); + var featureBuilder = providerRegistrationType switch + { + 1 => _systemUnderTest.AddProvider(_ => new NoOpFeatureProvider()), + 2 => _systemUnderTest.AddProvider("test", (_, _) => new NoOpFeatureProvider()), + 3 => _systemUnderTest.AddProvider(_ => new NoOpFeatureProvider(), o => { }), + 4 => _systemUnderTest.AddProvider("test", (_, _) => new NoOpFeatureProvider(), o => { }), + _ => throw new InvalidOperationException("Invalid mode.") + }; // Assert _systemUnderTest.IsContextConfigured.Should().BeFalse("The context should not be configured."); - result.Should().BeSameAs(_systemUnderTest, "The method should return the same builder instance."); + _systemUnderTest.HasDefaultProvider.Should().Be(expectsDefaultProvider, "The default provider flag should be set correctly."); + _systemUnderTest.IsPolicyConfigured.Should().BeFalse("The policy should not be configured."); + _systemUnderTest.DomainBoundProviderRegistrationCount.Should().Be(expectsDomainBoundProvider, "The domain-bound provider count should be correct."); + featureBuilder.Should().BeSameAs(_systemUnderTest, "The method should return the same builder instance."); _services.Should().ContainSingle(serviceDescriptor => serviceDescriptor.ServiceType == typeof(FeatureProvider) && - serviceDescriptor.Lifetime == ServiceLifetime.Singleton, + serviceDescriptor.Lifetime == ServiceLifetime.Transient, "A singleton service of type FeatureProvider should be added."); } + class TestOptions : OpenFeatureOptions { } + #if NET8_0_OR_GREATER [System.Diagnostics.CodeAnalysis.Experimental(Diagnostics.FeatureCodes.NewDi)] #endif - [Fact] - public void AddProvider_ShouldResolveCorrectProvider() + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public void AddProvider_ShouldResolveCorrectProvider(int providerRegistrationType) { // Arrange - _systemUnderTest.AddProvider(); + _ = providerRegistrationType switch + { + 1 => _systemUnderTest.AddProvider(_ => new NoOpFeatureProvider()), + 2 => _systemUnderTest.AddProvider("test", (_, _) => new NoOpFeatureProvider()), + 3 => _systemUnderTest.AddProvider(_ => new NoOpFeatureProvider(), o => { }), + 4 => _systemUnderTest.AddProvider("test", (_, _) => new NoOpFeatureProvider(), o => { }), + _ => throw new InvalidOperationException("Invalid mode.") + }; var serviceProvider = _services.BuildServiceProvider(); // Act - var provider = serviceProvider.GetService(); + var provider = providerRegistrationType switch + { + 1 or 3 => serviceProvider.GetService(), + 2 or 4 => serviceProvider.GetKeyedService("test"), + _ => throw new InvalidOperationException("Invalid mode.") + }; + + // Assert + provider.Should().NotBeNull("The FeatureProvider should be resolvable."); + provider.Should().BeOfType("The resolved provider should be of type DefaultFeatureProvider."); + } + + [Theory] + [InlineData(1, true, 1)] + [InlineData(2, true, 1)] + [InlineData(3, false, 2)] + [InlineData(4, true, 1)] + [InlineData(5, true, 1)] + [InlineData(6, false, 2)] + [InlineData(7, true, 2)] + [InlineData(8, true, 2)] + public void AddProvider_VerifiesDefaultAndDomainBoundProvidersBasedOnConfiguration(int providerRegistrationType, bool expectsDefaultProvider, int expectsDomainBoundProvider) + { + // Act + var featureBuilder = providerRegistrationType switch + { + 1 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider()) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()), + 2 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider()) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()), + 3 => _systemUnderTest + .AddProvider("test1", (_, _) => new NoOpFeatureProvider()) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider()), + 4 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider(), o => { }) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()), + 5 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider(), o => { }) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()), + 6 => _systemUnderTest + .AddProvider("test1", (_, _) => new NoOpFeatureProvider(), o => { }) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider()), + 7 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider()) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider()), + 8 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider(), o => { }) + .AddProvider("test", (_, _) => new NoOpFeatureProvider(), o => { }) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider(), o => { }), + _ => throw new InvalidOperationException("Invalid mode.") + }; // Assert _systemUnderTest.IsContextConfigured.Should().BeFalse("The context should not be configured."); + _systemUnderTest.HasDefaultProvider.Should().Be(expectsDefaultProvider, "The default provider flag should be set correctly."); + _systemUnderTest.IsPolicyConfigured.Should().BeFalse("The policy should not be configured."); + _systemUnderTest.DomainBoundProviderRegistrationCount.Should().Be(expectsDomainBoundProvider, "The domain-bound provider count should be correct."); + featureBuilder.Should().BeSameAs(_systemUnderTest, "The method should return the same builder instance."); + } + + [Theory] + [InlineData(1, null)] + [InlineData(2, "test")] + [InlineData(3, "test2")] + [InlineData(4, "test")] + [InlineData(5, null)] + [InlineData(6, "test1")] + [InlineData(7, "test2")] + [InlineData(8, null)] + public void AddProvider_ConfiguresPolicyNameAcrossMultipleProviderSetups(int providerRegistrationType, string? policyName) + { + // Arrange + var featureBuilder = providerRegistrationType switch + { + 1 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider()) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + 2 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider()) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + 3 => _systemUnderTest + .AddProvider("test1", (_, _) => new NoOpFeatureProvider()) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider()) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + 4 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider(), o => { }) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + 5 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider(), o => { }) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + 6 => _systemUnderTest + .AddProvider("test1", (_, _) => new NoOpFeatureProvider(), o => { }) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider()) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + 7 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider()) + .AddProvider("test", (_, _) => new NoOpFeatureProvider()) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider()) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + 8 => _systemUnderTest + .AddProvider(_ => new NoOpFeatureProvider(), o => { }) + .AddProvider("test", (_, _) => new NoOpFeatureProvider(), o => { }) + .AddProvider("test2", (_, _) => new NoOpFeatureProvider(), o => { }) + .AddPolicyName(policy => policy.DefaultNameSelector = provider => policyName), + _ => throw new InvalidOperationException("Invalid mode.") + }; + + var serviceProvider = _services.BuildServiceProvider(); + + // Act + var policy = serviceProvider.GetRequiredService>().Value; + var name = policy.DefaultNameSelector(serviceProvider); + var provider = name == null ? + serviceProvider.GetService() : + serviceProvider.GetRequiredKeyedService(name); + + // Assert + featureBuilder.IsPolicyConfigured.Should().BeTrue("The policy should be configured."); provider.Should().NotBeNull("The FeatureProvider should be resolvable."); provider.Should().BeOfType("The resolved provider should be of type DefaultFeatureProvider."); } diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs index 40e761d2..dc24a60f 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs @@ -25,6 +25,7 @@ public void AddOpenFeature_ShouldRegisterApiInstanceAndLifecycleManagerAsSinglet _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(Api) && s.Lifetime == ServiceLifetime.Singleton); _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(IFeatureLifecycleManager) && s.Lifetime == ServiceLifetime.Singleton); _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(IFeatureClient) && s.Lifetime == ServiceLifetime.Scoped); + _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(FeatureProvider) && s.Lifetime == ServiceLifetime.Transient); } [Fact] From fe43af66075458d94eba5ecea9a0361644dfee1a Mon Sep 17 00:00:00 2001 From: Artyom Tonoyan Date: Thu, 28 Nov 2024 23:53:16 +0400 Subject: [PATCH 2/7] test: Create integration test project Signed-off-by: Artyom Tonoyan --- Directory.Packages.props | 3 +- OpenFeature.sln | 7 + .../FeatureFlagIntegrationTest.cs | 144 ++++++++++++++++++ .../OpenFeature.IntegrationTests.csproj | 31 ++++ .../Services/FeatureFlagResponse.cs | 3 + .../IFeatureFlagConfigurationService.cs | 8 + .../Services/UserInfo.cs | 3 + .../Services/UserInfoHelper.cs | 36 +++++ 8 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs create mode 100644 test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj create mode 100644 test/OpenFeature.IntegrationTests/Services/FeatureFlagResponse.cs create mode 100644 test/OpenFeature.IntegrationTests/Services/IFeatureFlagConfigurationService.cs create mode 100644 test/OpenFeature.IntegrationTests/Services/UserInfo.cs create mode 100644 test/OpenFeature.IntegrationTests/Services/UserInfoHelper.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 6db91397..47330d45 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -9,6 +9,7 @@ + @@ -29,7 +30,7 @@ - + diff --git a/OpenFeature.sln b/OpenFeature.sln index e8191acd..ff4cb97e 100644 --- a/OpenFeature.sln +++ b/OpenFeature.sln @@ -85,6 +85,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "OpenFeature.DependencyInjec EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "OpenFeature.Hosting", "src\OpenFeature.Hosting\OpenFeature.Hosting.csproj", "{C99DA02A-3981-45A6-B3F8-4A1A48653DEE}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "OpenFeature.IntegrationTests", "test\OpenFeature.IntegrationTests\OpenFeature.IntegrationTests.csproj", "{68463B47-36B4-8DB5-5D02-662C169E85B0}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -119,6 +121,10 @@ Global {C99DA02A-3981-45A6-B3F8-4A1A48653DEE}.Debug|Any CPU.Build.0 = Debug|Any CPU {C99DA02A-3981-45A6-B3F8-4A1A48653DEE}.Release|Any CPU.ActiveCfg = Release|Any CPU {C99DA02A-3981-45A6-B3F8-4A1A48653DEE}.Release|Any CPU.Build.0 = Release|Any CPU + {68463B47-36B4-8DB5-5D02-662C169E85B0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {68463B47-36B4-8DB5-5D02-662C169E85B0}.Debug|Any CPU.Build.0 = Debug|Any CPU + {68463B47-36B4-8DB5-5D02-662C169E85B0}.Release|Any CPU.ActiveCfg = Release|Any CPU + {68463B47-36B4-8DB5-5D02-662C169E85B0}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -137,6 +143,7 @@ Global {C5415057-2700-48B5-940A-7A10969FA639} = {C97E9975-E10A-4817-AE2C-4DD69C3C02D4} {EB35F9F6-8A79-410E-A293-9387BC4AC9A7} = {65FBA159-23E0-4CF9-881B-F78DBFF198E9} {C99DA02A-3981-45A6-B3F8-4A1A48653DEE} = {C97E9975-E10A-4817-AE2C-4DD69C3C02D4} + {68463B47-36B4-8DB5-5D02-662C169E85B0} = {65FBA159-23E0-4CF9-881B-F78DBFF198E9} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {41F01B78-FB06-404F-8AD0-6ED6973F948F} diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs new file mode 100644 index 00000000..6ee249bc --- /dev/null +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -0,0 +1,144 @@ +using System.Net.Http.Json; +using System.Text.Json; +using FluentAssertions; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using OpenFeature.DependencyInjection.Providers.Memory; +using OpenFeature.IntegrationTests.Services; +using OpenFeature.Providers.Memory; + +namespace OpenFeature.IntegrationTests; + +public class FeatureFlagIntegrationTest +{ + // TestUserId is "off", other users are "on" + private const string FeatureA = "feature-a"; + private const string TestUserId = "123"; + + [Theory] + [InlineData(TestUserId, false, ServiceLifetime.Singleton)] + [InlineData(TestUserId, false, ServiceLifetime.Scoped)] + [InlineData(TestUserId, false, ServiceLifetime.Transient)] + [InlineData("SomeOtherId", true, ServiceLifetime.Singleton)] + [InlineData("SomeOtherId", true, ServiceLifetime.Scoped)] + [InlineData("SomeOtherId", true, ServiceLifetime.Transient)] + public async Task VerifyFeatureFlagBehaviorAcrossServiceLifetimesAsync(string userId, bool expectedResult, ServiceLifetime serviceLifetime) + { + // Arrange + using var server = await CreateServerAsync(services => + { + switch (serviceLifetime) + { + case ServiceLifetime.Singleton: + services.AddSingleton(); + break; + case ServiceLifetime.Scoped: + services.AddScoped(); + break; + case ServiceLifetime.Transient: + services.AddTransient(); + break; + default: + throw new ArgumentOutOfRangeException(nameof(serviceLifetime), serviceLifetime, null); + } + }); + + var client = server.CreateClient(); + var requestUri = $"/features/{userId}/flags/{FeatureA}"; + + // Act + var response = await client.GetAsync(requestUri); + var responseContent = await response.Content.ReadFromJsonAsync>(); + + // Assert + response.IsSuccessStatusCode.Should().BeTrue("Expected HTTP status code 200 OK."); + responseContent.Should().NotBeNull("Expected response content to be non-null."); + responseContent!.FeatureName.Should().Be(FeatureA, "Expected feature name to be 'feature-a'."); + responseContent.FeatureValue.Should().Be(expectedResult, "Expected feature value to match the expected result."); + } + + private static async Task CreateServerAsync(Action? configureServices = null) + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + + configureServices?.Invoke(builder.Services); + builder.Services.TryAddSingleton(); + + builder.Services.AddHttpContextAccessor(); + builder.Services.AddOpenFeature(cfg => + { + cfg.AddHostedFeatureLifecycle(); + cfg.AddContext((builder, provider) => + { + // Retrieve the HttpContext from IHttpContextAccessor, ensuring it's not null. + var context = provider.GetRequiredService().HttpContext + ?? throw new InvalidOperationException("HttpContext is not available."); + + var userId = UserInfoHelper.GetUserId(context); + builder.Set("user", userId); + }); + cfg.AddInMemoryProvider(provider => + { + var flagService = provider.GetRequiredService(); + return flagService.GetFlags(); + }); + }); + + var app = builder.Build(); + + app.UseRouting(); + app.Map($"/features/{{userId}}/flags/{{featureName}}", async context => + { + var client = context.RequestServices.GetRequiredService(); + var featureName = UserInfoHelper.GetFeatureName(context); + var res = await client.GetBooleanValueAsync(featureName, false).ConfigureAwait(false); + var result = await client.GetBooleanValueAsync(featureName, false).ConfigureAwait(false); + + var response = new FeatureFlagResponse(featureName, result); + + // Serialize the response object to JSON + var jsonResponse = JsonSerializer.Serialize(response, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + + // Write the JSON response + context.Response.ContentType = "application/json"; + await context.Response.WriteAsync(jsonResponse).ConfigureAwait(false); + }); + + await app.StartAsync().ConfigureAwait(false); + + return app.GetTestServer(); + } + + public class FlagConfigurationService : IFeatureFlagConfigurationService + { + private readonly IDictionary _flags; + public FlagConfigurationService() + { + _flags = new Dictionary + { + { + "feature-a", new Flag( + variants: new Dictionary() + { + { "on", true }, + { "off", false } + }, + defaultVariant: "on", context => { + var id = context.GetValue("user").AsString; + if(id == null) + { + return "on"; // default variant + } + + return id == TestUserId ? "off" : "on"; + }) + } + }; + } + public Dictionary GetFlags() => _flags.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + } +} diff --git a/test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj b/test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj new file mode 100644 index 00000000..8287b2ec --- /dev/null +++ b/test/OpenFeature.IntegrationTests/OpenFeature.IntegrationTests.csproj @@ -0,0 +1,31 @@ + + + + net8.0 + enable + enable + + false + true + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/OpenFeature.IntegrationTests/Services/FeatureFlagResponse.cs b/test/OpenFeature.IntegrationTests/Services/FeatureFlagResponse.cs new file mode 100644 index 00000000..50285cc0 --- /dev/null +++ b/test/OpenFeature.IntegrationTests/Services/FeatureFlagResponse.cs @@ -0,0 +1,3 @@ +namespace OpenFeature.IntegrationTests.Services; + +public record FeatureFlagResponse(string FeatureName, T FeatureValue) where T : notnull; diff --git a/test/OpenFeature.IntegrationTests/Services/IFeatureFlagConfigurationService.cs b/test/OpenFeature.IntegrationTests/Services/IFeatureFlagConfigurationService.cs new file mode 100644 index 00000000..1b51a60a --- /dev/null +++ b/test/OpenFeature.IntegrationTests/Services/IFeatureFlagConfigurationService.cs @@ -0,0 +1,8 @@ +using OpenFeature.Providers.Memory; + +namespace OpenFeature.IntegrationTests.Services; + +internal interface IFeatureFlagConfigurationService +{ + Dictionary GetFlags(); +} diff --git a/test/OpenFeature.IntegrationTests/Services/UserInfo.cs b/test/OpenFeature.IntegrationTests/Services/UserInfo.cs new file mode 100644 index 00000000..c2c5d8c1 --- /dev/null +++ b/test/OpenFeature.IntegrationTests/Services/UserInfo.cs @@ -0,0 +1,3 @@ +namespace OpenFeature.IntegrationTests.Services; + +public record UserInfo(string UserId, string FeatureName); diff --git a/test/OpenFeature.IntegrationTests/Services/UserInfoHelper.cs b/test/OpenFeature.IntegrationTests/Services/UserInfoHelper.cs new file mode 100644 index 00000000..0e057a6b --- /dev/null +++ b/test/OpenFeature.IntegrationTests/Services/UserInfoHelper.cs @@ -0,0 +1,36 @@ +using Microsoft.AspNetCore.Http; + +namespace OpenFeature.IntegrationTests.Services; + +public static class UserInfoHelper +{ + /// + /// Extracts the user ID from the HTTP request context. + /// + /// The HTTP context containing the request. + /// The user ID as a string. + /// Thrown if the user ID is not found in the route values. + public static string GetUserId(HttpContext context) + { + if (context.Request.RouteValues.TryGetValue("userId", out var userId) && userId is string userIdString) + { + return userIdString; + } + throw new ArgumentNullException(nameof(userId), "User ID not found in route values."); + } + + /// + /// Extracts the feature name from the HTTP request context. + /// + /// The HTTP context containing the request. + /// The feature name as a string. + /// Thrown if the feature name is not found in the route values. + public static string GetFeatureName(HttpContext context) + { + if (context.Request.RouteValues.TryGetValue("featureName", out var featureName) && featureName is string featureNameString) + { + return featureNameString; + } + throw new ArgumentNullException(nameof(featureName), "Feature name not found in route values."); + } +} From e063f9daef16993f3b98fc52358c38845289f341 Mon Sep 17 00:00:00 2001 From: Artyom Tonoyan Date: Fri, 29 Nov 2024 00:36:10 +0400 Subject: [PATCH 3/7] fix(provider-registration): Correct formatting issues Signed-off-by: Artyom Tonoyan --- .../FeatureFlagIntegrationTest.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs index 6ee249bc..5bad09b4 100644 --- a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -95,8 +95,8 @@ private static async Task CreateServerAsync(Action(); var featureName = UserInfoHelper.GetFeatureName(context); - var res = await client.GetBooleanValueAsync(featureName, false).ConfigureAwait(false); - var result = await client.GetBooleanValueAsync(featureName, false).ConfigureAwait(false); + var res = await client.GetBooleanValueAsync(featureName, false).ConfigureAwait(true); + var result = await client.GetBooleanValueAsync(featureName, false).ConfigureAwait(true); var response = new FeatureFlagResponse(featureName, result); @@ -105,10 +105,10 @@ private static async Task CreateServerAsync(Action Date: Fri, 29 Nov 2024 01:41:50 +0400 Subject: [PATCH 4/7] fix: Correct formatting issues Signed-off-by: Artyom Tonoyan --- .../FeatureFlagIntegrationTest.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs index 5bad09b4..559bf4bb 100644 --- a/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs +++ b/test/OpenFeature.IntegrationTests/FeatureFlagIntegrationTest.cs @@ -44,14 +44,14 @@ public async Task VerifyFeatureFlagBehaviorAcrossServiceLifetimesAsync(string us default: throw new ArgumentOutOfRangeException(nameof(serviceLifetime), serviceLifetime, null); } - }); + }).ConfigureAwait(true); var client = server.CreateClient(); var requestUri = $"/features/{userId}/flags/{FeatureA}"; // Act - var response = await client.GetAsync(requestUri); - var responseContent = await response.Content.ReadFromJsonAsync>(); + var response = await client.GetAsync(requestUri).ConfigureAwait(true); + var responseContent = await response.Content.ReadFromJsonAsync>().ConfigureAwait(true); ; // Assert response.IsSuccessStatusCode.Should().BeTrue("Expected HTTP status code 200 OK."); From a9c84648c91e4264a47486455e057985910cfa82 Mon Sep 17 00:00:00 2001 From: Artyom Tonoyan Date: Fri, 29 Nov 2024 08:13:27 +0400 Subject: [PATCH 5/7] fix(tests): Resolve failed test cases Signed-off-by: Artyom Tonoyan --- .../OpenFeatureServiceCollectionExtensionsTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs index dc24a60f..40e761d2 100644 --- a/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs +++ b/test/OpenFeature.DependencyInjection.Tests/OpenFeatureServiceCollectionExtensionsTests.cs @@ -25,7 +25,6 @@ public void AddOpenFeature_ShouldRegisterApiInstanceAndLifecycleManagerAsSinglet _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(Api) && s.Lifetime == ServiceLifetime.Singleton); _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(IFeatureLifecycleManager) && s.Lifetime == ServiceLifetime.Singleton); _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(IFeatureClient) && s.Lifetime == ServiceLifetime.Scoped); - _systemUnderTest.Should().ContainSingle(s => s.ServiceType == typeof(FeatureProvider) && s.Lifetime == ServiceLifetime.Transient); } [Fact] From 480c790f0b89e216ca8d96271f15f30956a2c34c Mon Sep 17 00:00:00 2001 From: Artyom Tonoyan Date: Tue, 3 Dec 2024 23:00:36 +0400 Subject: [PATCH 6/7] Update README.md Signed-off-by: Artyom Tonoyan --- README.md | 60 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index acb31e8f..451d68e9 100644 --- a/README.md +++ b/README.md @@ -338,41 +338,43 @@ builder.Services.AddOpenFeature(featureBuilder => { }); }); ``` -#### Creating a New Provider -To integrate a custom provider, such as InMemoryProvider, you’ll need to create a factory that builds and configures the provider. This section demonstrates how to set up InMemoryProvider as a new provider with custom configuration options. -**Configuring InMemoryProvider as a New Provider** -
Begin by creating a custom factory class, `InMemoryProviderFactory`, that implements `IFeatureProviderFactory`. This factory will initialize your provider with any necessary configurations. -```csharp -public class InMemoryProviderFactory : IFeatureProviderFactory -{ - internal IDictionary? Flags { get; set; } - - public FeatureProvider Create() => new InMemoryProvider(Flags); -} -``` -**Adding an Extension Method to OpenFeatureBuilder** -
To streamline the configuration process, add an extension method, `AddInMemoryProvider`, to `OpenFeatureBuilder`. This allows you to set up the provider with either a domain-scoped or a default configuration. +### Registering a Custom Provider +You can register a custom provider, such as `InMemoryProvider`, with OpenFeature using the `AddProvider` method. This approach allows you to dynamically resolve services or configurations during registration. ```csharp -public static partial class FeatureBuilderExtensions -{ - public static OpenFeatureBuilder AddInMemoryProvider(this OpenFeatureBuilder builder, Action>? configure = null) - => builder.AddProvider(factory => ConfigureFlags(factory, configure)); +services.AddOpenFeature() + .AddProvider(provider => + { + // Resolve services or configurations as needed + var configuration = provider.GetRequiredService(); + var flags = new Dictionary + { + { "feature-key", new Flag(configuration.GetValue("FeatureFlags:Key")) } + }; + + // Register a custom provider, such as InMemoryProvider + return new InMemoryProvider(flags); + }); +``` - public static OpenFeatureBuilder AddInMemoryProvider(this OpenFeatureBuilder builder, string domain, Action>? configure = null) - => builder.AddProvider(domain, factory => ConfigureFlags(factory, configure)); +#### Adding a Domain-Scoped Provider - private static void ConfigureFlags(InMemoryProviderFactory factory, Action>? configure) - { - if (configure == null) - return; +You can also register a domain-scoped custom provider, enabling configurations specific to each domain: - var flag = new Dictionary(); - configure.Invoke(flag); - factory.Flags = flag; - } -} +```csharp +services.AddOpenFeature() + .AddProvider("my-domain", (provider, domain) => + { + // Resolve services or configurations as needed for the domain + var flags = new Dictionary + { + { $"{domain}-feature-key", new Flag(true) } + }; + + // Register a domain-scoped custom provider such as InMemoryProvider + return new InMemoryProvider(flags); + }); ``` From e3836971b016a24e7dcfa4bcb645eebbcd0414c7 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Tue, 3 Dec 2024 17:06:25 +0100 Subject: [PATCH 7/7] feat: Support Returning Error Resolutions from Providers (#323) When provider resolutions with error code set other than `None` are returned, the provider acts as if an error was thrown. Signed-off-by: christian.lutnik Signed-off-by: Artyom Tonoyan --- src/OpenFeature/OpenFeatureClient.cs | 18 +++++++++- .../OpenFeatureClientTests.cs | 36 +++++++++++++++++++ test/OpenFeature.Tests/TestImplementations.cs | 29 ++++++++++++--- 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/OpenFeature/OpenFeatureClient.cs b/src/OpenFeature/OpenFeatureClient.cs index 08e29533..c2621785 100644 --- a/src/OpenFeature/OpenFeatureClient.cs +++ b/src/OpenFeature/OpenFeatureClient.cs @@ -263,7 +263,23 @@ private async Task> EvaluateFlagAsync( (await resolveValueDelegate.Invoke(flagKey, defaultValue, contextFromHooks.EvaluationContext, cancellationToken).ConfigureAwait(false)) .ToFlagEvaluationDetails(); - await this.TriggerAfterHooksAsync(allHooksReversed, hookContext, evaluation, options, cancellationToken).ConfigureAwait(false); + if (evaluation.ErrorType == ErrorType.None) + { + await this.TriggerAfterHooksAsync( + allHooksReversed, + hookContext, + evaluation, + options, + cancellationToken + ).ConfigureAwait(false); + } + else + { + var exception = new FeatureProviderException(evaluation.ErrorType, evaluation.ErrorMessage); + this.FlagEvaluationErrorWithDescription(flagKey, evaluation.ErrorType.GetDescription(), exception); + await this.TriggerErrorHooksAsync(allHooksReversed, hookContext, exception, options, cancellationToken) + .ConfigureAwait(false); + } } catch (FeatureProviderException ex) { diff --git a/test/OpenFeature.Tests/OpenFeatureClientTests.cs b/test/OpenFeature.Tests/OpenFeatureClientTests.cs index ce3e9e93..ee9eee0c 100644 --- a/test/OpenFeature.Tests/OpenFeatureClientTests.cs +++ b/test/OpenFeature.Tests/OpenFeatureClientTests.cs @@ -433,6 +433,41 @@ public async Task When_Exception_Occurs_During_Evaluation_Should_Return_Error() _ = featureProviderMock.Received(1).ResolveStructureValueAsync(flagName, defaultValue, Arg.Any()); } + [Fact] + public async Task When_Error_Is_Returned_From_Provider_Should_Not_Run_After_Hook_But_Error_Hook() + { + var fixture = new Fixture(); + var domain = fixture.Create(); + var clientVersion = fixture.Create(); + var flagName = fixture.Create(); + var defaultValue = fixture.Create(); + const string testMessage = "Couldn't parse flag data."; + + var featureProviderMock = Substitute.For(); + featureProviderMock.ResolveStructureValueAsync(flagName, defaultValue, Arg.Any()) + .Returns(Task.FromResult(new ResolutionDetails(flagName, defaultValue, ErrorType.ParseError, + "ERROR", null, testMessage))); + featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create())); + featureProviderMock.GetProviderHooks().Returns(ImmutableList.Empty); + + await Api.Instance.SetProviderAsync(featureProviderMock); + var client = Api.Instance.GetClient(domain, clientVersion); + var testHook = new TestHook(); + client.AddHooks(testHook); + var response = await client.GetObjectDetailsAsync(flagName, defaultValue); + + response.ErrorType.Should().Be(ErrorType.ParseError); + response.Reason.Should().Be(Reason.Error); + response.ErrorMessage.Should().Be(testMessage); + _ = featureProviderMock.Received(1) + .ResolveStructureValueAsync(flagName, defaultValue, Arg.Any()); + + Assert.Equal(1, testHook.BeforeCallCount); + Assert.Equal(0, testHook.AfterCallCount); + Assert.Equal(1, testHook.ErrorCallCount); + Assert.Equal(1, testHook.FinallyCallCount); + } + [Fact] public async Task Cancellation_Token_Added_Is_Passed_To_Provider() { @@ -454,6 +489,7 @@ public async Task Cancellation_Token_Added_Is_Passed_To_Provider() { await Task.Delay(10); // artificially delay until cancelled } + return new ResolutionDetails(flagName, defaultString, ErrorType.None, cancelledReason); }); featureProviderMock.GetMetadata().Returns(new Metadata(fixture.Create())); diff --git a/test/OpenFeature.Tests/TestImplementations.cs b/test/OpenFeature.Tests/TestImplementations.cs index 7a1dff10..ea35b870 100644 --- a/test/OpenFeature.Tests/TestImplementations.cs +++ b/test/OpenFeature.Tests/TestImplementations.cs @@ -8,28 +8,49 @@ namespace OpenFeature.Tests { - public class TestHookNoOverride : Hook { } + public class TestHookNoOverride : Hook + { + } public class TestHook : Hook { - public override ValueTask BeforeAsync(HookContext context, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + private int _beforeCallCount; + public int BeforeCallCount { get => this._beforeCallCount; } + + private int _afterCallCount; + public int AfterCallCount { get => this._afterCallCount; } + + private int _errorCallCount; + public int ErrorCallCount { get => this._errorCallCount; } + + private int _finallyCallCount; + public int FinallyCallCount { get => this._finallyCallCount; } + + public override ValueTask BeforeAsync(HookContext context, + IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._beforeCallCount); return new ValueTask(EvaluationContext.Empty); } public override ValueTask AfterAsync(HookContext context, FlagEvaluationDetails details, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._afterCallCount); return new ValueTask(); } - public override ValueTask ErrorAsync(HookContext context, Exception error, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + public override ValueTask ErrorAsync(HookContext context, Exception error, + IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._errorCallCount); return new ValueTask(); } - public override ValueTask FinallyAsync(HookContext context, IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) + public override ValueTask FinallyAsync(HookContext context, + IReadOnlyDictionary? hints = null, CancellationToken cancellationToken = default) { + Interlocked.Increment(ref this._finallyCallCount); return new ValueTask(); } }