From 8cb5f6b9b683f004c34dea384453324947ee8460 Mon Sep 17 00:00:00 2001 From: Benjamin Evenson <2031163+benjiro@users.noreply.github.com> Date: Sat, 6 Aug 2022 12:36:49 +1000 Subject: [PATCH] Feat: Add support for provider hooks - [Breaking] Remove IFeatureProvider interface infavor of FeatureProvider abstract class so default implementations can exist - Make sure Provider hooks are called in the correct order - Use strick mocking mode so sequence is validated correctly - Update test cases for provider hooks support Signed-off-by: Benjamin Evenson <2031163+benjiro@users.noreply.github.com> --- ...IFeatureProvider.cs => FeatureProvider.cs} | 29 ++- .../Model/ResolutionDetails.cs | 2 +- src/OpenFeature.SDK/NoOpProvider.cs | 14 +- src/OpenFeature.SDK/OpenFeature.cs | 12 +- src/OpenFeature.SDK/OpenFeatureClient.cs | 7 +- .../ClearOpenFeatureInstanceFixture.cs | 13 ++ .../FeatureProviderTests.cs | 4 +- .../OpenFeatureClientTests.cs | 62 +++++- .../OpenFeatureHookTests.cs | 207 ++++++++++++------ .../OpenFeature.SDK.Tests/OpenFeatureTests.cs | 14 +- .../TestImplementations.cs | 30 ++- 11 files changed, 268 insertions(+), 126 deletions(-) rename src/OpenFeature.SDK/{IFeatureProvider.cs => FeatureProvider.cs} (70%) create mode 100644 test/OpenFeature.SDK.Tests/ClearOpenFeatureInstanceFixture.cs diff --git a/src/OpenFeature.SDK/IFeatureProvider.cs b/src/OpenFeature.SDK/FeatureProvider.cs similarity index 70% rename from src/OpenFeature.SDK/IFeatureProvider.cs rename to src/OpenFeature.SDK/FeatureProvider.cs index 5a463eb5..ef767b4a 100644 --- a/src/OpenFeature.SDK/IFeatureProvider.cs +++ b/src/OpenFeature.SDK/FeatureProvider.cs @@ -1,3 +1,5 @@ +using System; +using System.Collections.Generic; using System.Threading.Tasks; using OpenFeature.SDK.Model; @@ -8,13 +10,26 @@ namespace OpenFeature.SDK /// A provider acts as the translates layer between the generic feature flag structure to a target feature flag system. /// /// Provider specification - public interface IFeatureProvider + public abstract class FeatureProvider { + /// + /// Gets a immutable list of hooks that belong to the provider. + /// By default return a empty list + /// + /// Executed in the order of hooks + /// before: API, Client, Invocation, Provider + /// after: Provider, Invocation, Client, API + /// error (if applicable): Provider, Invocation, Client, API + /// finally: Provider, Invocation, Client, API + /// + /// + public virtual IReadOnlyList GetProviderHooks() => Array.Empty(); + /// /// Metadata describing the provider. /// /// - Metadata GetMetadata(); + public abstract Metadata GetMetadata(); /// /// Resolves a boolean feature flag @@ -24,7 +39,7 @@ public interface IFeatureProvider /// /// /// - Task> ResolveBooleanValue(string flagKey, bool defaultValue, + public abstract Task> ResolveBooleanValue(string flagKey, bool defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null); /// @@ -35,7 +50,7 @@ Task> ResolveBooleanValue(string flagKey, bool defaultVa /// /// /// - Task> ResolveStringValue(string flagKey, string defaultValue, + public abstract Task> ResolveStringValue(string flagKey, string defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null); /// @@ -46,7 +61,7 @@ Task> ResolveStringValue(string flagKey, string defaul /// /// /// - Task> ResolveIntegerValue(string flagKey, int defaultValue, + public abstract Task> ResolveIntegerValue(string flagKey, int defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null); /// @@ -57,7 +72,7 @@ Task> ResolveIntegerValue(string flagKey, int defaultValu /// /// /// - Task> ResolveDoubleValue(string flagKey, double defaultValue, + public abstract Task> ResolveDoubleValue(string flagKey, double defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null); /// @@ -69,7 +84,7 @@ Task> ResolveDoubleValue(string flagKey, double defaul /// /// Type of object /// - Task> ResolveStructureValue(string flagKey, T defaultValue, + public abstract Task> ResolveStructureValue(string flagKey, T defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null); } } diff --git a/src/OpenFeature.SDK/Model/ResolutionDetails.cs b/src/OpenFeature.SDK/Model/ResolutionDetails.cs index 98313f9e..4672327f 100644 --- a/src/OpenFeature.SDK/Model/ResolutionDetails.cs +++ b/src/OpenFeature.SDK/Model/ResolutionDetails.cs @@ -3,7 +3,7 @@ namespace OpenFeature.SDK.Model { /// - /// Defines the contract that the is required to return + /// Defines the contract that the is required to return /// Describes the details of the feature flag being evaluated /// /// Flag value type diff --git a/src/OpenFeature.SDK/NoOpProvider.cs b/src/OpenFeature.SDK/NoOpProvider.cs index 9ad298c2..0b5d621b 100644 --- a/src/OpenFeature.SDK/NoOpProvider.cs +++ b/src/OpenFeature.SDK/NoOpProvider.cs @@ -4,37 +4,37 @@ namespace OpenFeature.SDK { - internal class NoOpFeatureProvider : IFeatureProvider + internal class NoOpFeatureProvider : FeatureProvider { private readonly Metadata _metadata = new Metadata(NoOpProvider.NoOpProviderName); - public Metadata GetMetadata() + public override Metadata GetMetadata() { return this._metadata; } - public Task> ResolveBooleanValue(string flagKey, bool defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) + public override Task> ResolveBooleanValue(string flagKey, bool defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { return Task.FromResult(NoOpResponse(flagKey, defaultValue)); } - public Task> ResolveStringValue(string flagKey, string defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) + public override Task> ResolveStringValue(string flagKey, string defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { return Task.FromResult(NoOpResponse(flagKey, defaultValue)); } - public Task> ResolveIntegerValue(string flagKey, int defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) + public override Task> ResolveIntegerValue(string flagKey, int defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { return Task.FromResult(NoOpResponse(flagKey, defaultValue)); } - public Task> ResolveDoubleValue(string flagKey, double defaultValue, EvaluationContext context = null, + public override Task> ResolveDoubleValue(string flagKey, double defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { return Task.FromResult(NoOpResponse(flagKey, defaultValue)); } - public Task> ResolveStructureValue(string flagKey, T defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) + public override Task> ResolveStructureValue(string flagKey, T defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { return Task.FromResult(NoOpResponse(flagKey, defaultValue)); } diff --git a/src/OpenFeature.SDK/OpenFeature.cs b/src/OpenFeature.SDK/OpenFeature.cs index 66f04612..8099299b 100644 --- a/src/OpenFeature.SDK/OpenFeature.cs +++ b/src/OpenFeature.SDK/OpenFeature.cs @@ -12,7 +12,7 @@ namespace OpenFeature.SDK public sealed class OpenFeature { private EvaluationContext _evaluationContext = new EvaluationContext(); - private IFeatureProvider _featureProvider = new NoOpFeatureProvider(); + private FeatureProvider _featureProvider = new NoOpFeatureProvider(); private readonly List _hooks = new List(); /// @@ -29,14 +29,14 @@ private OpenFeature() { } /// /// Sets the feature provider /// - /// Implementation of - public void SetProvider(IFeatureProvider featureProvider) => this._featureProvider = featureProvider; + /// Implementation of + public void SetProvider(FeatureProvider featureProvider) => this._featureProvider = featureProvider; /// /// Gets the feature provider /// - /// - public IFeatureProvider GetProvider() => this._featureProvider; + /// + public FeatureProvider GetProvider() => this._featureProvider; /// /// Gets providers metadata @@ -81,7 +81,7 @@ public FeatureClient GetClient(string name = null, string version = null, ILogge /// Sets the global /// /// - public void SetContext(EvaluationContext context) => this._evaluationContext = context; + public void SetContext(EvaluationContext context) => this._evaluationContext = context ?? new EvaluationContext(); /// /// Gets the global diff --git a/src/OpenFeature.SDK/OpenFeatureClient.cs b/src/OpenFeature.SDK/OpenFeatureClient.cs index 0a54df3d..35904823 100644 --- a/src/OpenFeature.SDK/OpenFeatureClient.cs +++ b/src/OpenFeature.SDK/OpenFeatureClient.cs @@ -17,19 +17,19 @@ namespace OpenFeature.SDK public sealed class FeatureClient : IFeatureClient { private readonly ClientMetadata _metadata; - private readonly IFeatureProvider _featureProvider; + private readonly FeatureProvider _featureProvider; private readonly List _hooks = new List(); private readonly ILogger _logger; /// /// Initializes a new instance of the class. /// - /// Feature provider used by client + /// Feature provider used by client /// Name of client /// Version of client /// Logger used by client /// Throws if any of the required parameters are null - public FeatureClient(IFeatureProvider featureProvider, string name, string version, ILogger logger = null) + public FeatureClient(FeatureProvider featureProvider, string name, string version, ILogger logger = null) { this._featureProvider = featureProvider ?? throw new ArgumentNullException(nameof(featureProvider)); this._metadata = new ClientMetadata(name, version); @@ -209,6 +209,7 @@ private async Task> EvaluateFlag( .Concat(OpenFeature.Instance.GetHooks()) .Concat(this._hooks) .Concat(options?.Hooks ?? Enumerable.Empty()) + .Concat(this._featureProvider.GetProviderHooks()) .ToList() .AsReadOnly(); diff --git a/test/OpenFeature.SDK.Tests/ClearOpenFeatureInstanceFixture.cs b/test/OpenFeature.SDK.Tests/ClearOpenFeatureInstanceFixture.cs new file mode 100644 index 00000000..3cf4bb5d --- /dev/null +++ b/test/OpenFeature.SDK.Tests/ClearOpenFeatureInstanceFixture.cs @@ -0,0 +1,13 @@ +namespace OpenFeature.SDK.Tests +{ + public class ClearOpenFeatureInstanceFixture + { + // Make sure the singleton is cleared between tests + public ClearOpenFeatureInstanceFixture() + { + OpenFeature.Instance.SetContext(null); + OpenFeature.Instance.ClearHooks(); + OpenFeature.Instance.SetProvider(new NoOpFeatureProvider()); + } + } +} diff --git a/test/OpenFeature.SDK.Tests/FeatureProviderTests.cs b/test/OpenFeature.SDK.Tests/FeatureProviderTests.cs index 0a48205d..b92ccb6a 100644 --- a/test/OpenFeature.SDK.Tests/FeatureProviderTests.cs +++ b/test/OpenFeature.SDK.Tests/FeatureProviderTests.cs @@ -9,7 +9,7 @@ namespace OpenFeature.SDK.Tests { - public class FeatureProviderTests + public class FeatureProviderTests : ClearOpenFeatureInstanceFixture { [Fact] [Specification("2.1", "The provider interface MUST define a `metadata` member or accessor, containing a `name` field or accessor of type string, which identifies the provider implementation.")] @@ -67,7 +67,7 @@ public async Task Provider_Must_ErrorType() var defaultIntegerValue = fixture.Create(); var defaultDoubleValue = fixture.Create(); var defaultStructureValue = fixture.Create(); - var providerMock = new Mock(); + var providerMock = new Mock(MockBehavior.Strict); providerMock.Setup(x => x.ResolveBooleanValue(flagName, defaultBoolValue, It.IsAny(), It.IsAny())) .ReturnsAsync(new ResolutionDetails(flagName, defaultBoolValue, ErrorType.General, NoOpProvider.ReasonNoOp, NoOpProvider.Variant)); diff --git a/test/OpenFeature.SDK.Tests/OpenFeatureClientTests.cs b/test/OpenFeature.SDK.Tests/OpenFeatureClientTests.cs index 6319aa10..f10e2d2c 100644 --- a/test/OpenFeature.SDK.Tests/OpenFeatureClientTests.cs +++ b/test/OpenFeature.SDK.Tests/OpenFeatureClientTests.cs @@ -14,7 +14,7 @@ namespace OpenFeature.SDK.Tests { - public class OpenFeatureClientTests + public class OpenFeatureClientTests : ClearOpenFeatureInstanceFixture { [Fact] [Specification("1.2.1", "The client MUST provide a method to add `hooks` which accepts one or more API-conformant `hooks`, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.")] @@ -22,9 +22,9 @@ public void OpenFeatureClient_Should_Allow_Hooks() { var fixture = new Fixture(); var clientName = fixture.Create(); - var hook1 = new Mock().Object; - var hook2 = new Mock().Object; - var hook3 = new Mock().Object; + var hook1 = new Mock(MockBehavior.Strict).Object; + var hook2 = new Mock(MockBehavior.Strict).Object; + var hook3 = new Mock(MockBehavior.Strict).Object; var client = OpenFeature.Instance.GetClient(clientName); @@ -160,8 +160,8 @@ public async Task OpenFeatureClient_Should_Return_DefaultValue_When_Type_Mismatc var clientVersion = fixture.Create(); var flagName = fixture.Create(); var defaultValue = fixture.Create(); - var mockedFeatureProvider = new Mock(); - var mockedLogger = new Mock>(); + var mockedFeatureProvider = new Mock(MockBehavior.Strict); + var mockedLogger = new Mock>(MockBehavior.Default); // This will fail to case a String to TestStructure mockedFeatureProvider @@ -169,6 +169,8 @@ public async Task OpenFeatureClient_Should_Return_DefaultValue_When_Type_Mismatc .ReturnsAsync(new ResolutionDetails(flagName, "Mismatch")); mockedFeatureProvider.Setup(x => x.GetMetadata()) .Returns(new Metadata(fixture.Create())); + mockedFeatureProvider.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); OpenFeature.Instance.SetProvider(mockedFeatureProvider.Object); var client = OpenFeature.Instance.GetClient(clientName, clientVersion, mockedLogger.Object); @@ -198,12 +200,14 @@ public async Task Should_Resolve_BooleanValue() var flagName = fixture.Create(); var defaultValue = fixture.Create(); - var featureProviderMock = new Mock(); + var featureProviderMock = new Mock(MockBehavior.Strict); featureProviderMock .Setup(x => x.ResolveBooleanValue(flagName, defaultValue, It.IsAny(), null)) .ReturnsAsync(new ResolutionDetails(flagName, defaultValue)); featureProviderMock.Setup(x => x.GetMetadata()) .Returns(new Metadata(fixture.Create())); + featureProviderMock.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); OpenFeature.Instance.SetProvider(featureProviderMock.Object); var client = OpenFeature.Instance.GetClient(clientName, clientVersion); @@ -222,12 +226,14 @@ public async Task Should_Resolve_StringValue() var flagName = fixture.Create(); var defaultValue = fixture.Create(); - var featureProviderMock = new Mock(); + var featureProviderMock = new Mock(MockBehavior.Strict); featureProviderMock .Setup(x => x.ResolveStringValue(flagName, defaultValue, It.IsAny(), null)) .ReturnsAsync(new ResolutionDetails(flagName, defaultValue)); featureProviderMock.Setup(x => x.GetMetadata()) .Returns(new Metadata(fixture.Create())); + featureProviderMock.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); OpenFeature.Instance.SetProvider(featureProviderMock.Object); var client = OpenFeature.Instance.GetClient(clientName, clientVersion); @@ -238,7 +244,7 @@ public async Task Should_Resolve_StringValue() } [Fact] - public async Task Should_Resolve_NumberValue() + public async Task Should_Resolve_IntegerValue() { var fixture = new Fixture(); var clientName = fixture.Create(); @@ -246,12 +252,14 @@ public async Task Should_Resolve_NumberValue() var flagName = fixture.Create(); var defaultValue = fixture.Create(); - var featureProviderMock = new Mock(); + var featureProviderMock = new Mock(MockBehavior.Strict); featureProviderMock .Setup(x => x.ResolveIntegerValue(flagName, defaultValue, It.IsAny(), null)) .ReturnsAsync(new ResolutionDetails(flagName, defaultValue)); featureProviderMock.Setup(x => x.GetMetadata()) .Returns(new Metadata(fixture.Create())); + featureProviderMock.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); OpenFeature.Instance.SetProvider(featureProviderMock.Object); var client = OpenFeature.Instance.GetClient(clientName, clientVersion); @@ -261,6 +269,32 @@ public async Task Should_Resolve_NumberValue() featureProviderMock.Verify(x => x.ResolveIntegerValue(flagName, defaultValue, It.IsAny(), null), Times.Once); } + [Fact] + public async Task Should_Resolve_DoubleValue() + { + var fixture = new Fixture(); + var clientName = fixture.Create(); + var clientVersion = fixture.Create(); + var flagName = fixture.Create(); + var defaultValue = fixture.Create(); + + var featureProviderMock = new Mock(MockBehavior.Strict); + featureProviderMock + .Setup(x => x.ResolveDoubleValue(flagName, defaultValue, It.IsAny(), null)) + .ReturnsAsync(new ResolutionDetails(flagName, defaultValue)); + featureProviderMock.Setup(x => x.GetMetadata()) + .Returns(new Metadata(fixture.Create())); + featureProviderMock.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); + + OpenFeature.Instance.SetProvider(featureProviderMock.Object); + var client = OpenFeature.Instance.GetClient(clientName, clientVersion); + + (await client.GetDoubleValue(flagName, defaultValue)).Should().Be(defaultValue); + + featureProviderMock.Verify(x => x.ResolveDoubleValue(flagName, defaultValue, It.IsAny(), null), Times.Once); + } + [Fact] public async Task Should_Resolve_StructureValue() { @@ -270,12 +304,14 @@ public async Task Should_Resolve_StructureValue() var flagName = fixture.Create(); var defaultValue = fixture.Create(); - var featureProviderMock = new Mock(); + var featureProviderMock = new Mock(MockBehavior.Strict); featureProviderMock .Setup(x => x.ResolveStructureValue(flagName, defaultValue, It.IsAny(), null)) .ReturnsAsync(new ResolutionDetails(flagName, defaultValue)); featureProviderMock.Setup(x => x.GetMetadata()) .Returns(new Metadata(fixture.Create())); + featureProviderMock.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); OpenFeature.Instance.SetProvider(featureProviderMock.Object); var client = OpenFeature.Instance.GetClient(clientName, clientVersion); @@ -294,12 +330,14 @@ public async Task When_Exception_Occurs_During_Evaluation_Should_Return_Error() var flagName = fixture.Create(); var defaultValue = fixture.Create(); - var featureProviderMock = new Mock(); + var featureProviderMock = new Mock(MockBehavior.Strict); featureProviderMock .Setup(x => x.ResolveStructureValue(flagName, defaultValue, It.IsAny(), null)) .Throws(new FeatureProviderException(ErrorType.ParseError)); featureProviderMock.Setup(x => x.GetMetadata()) .Returns(new Metadata(fixture.Create())); + featureProviderMock.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); OpenFeature.Instance.SetProvider(featureProviderMock.Object); var client = OpenFeature.Instance.GetClient(clientName, clientVersion); diff --git a/test/OpenFeature.SDK.Tests/OpenFeatureHookTests.cs b/test/OpenFeature.SDK.Tests/OpenFeatureHookTests.cs index da75ee61..baec1384 100644 --- a/test/OpenFeature.SDK.Tests/OpenFeatureHookTests.cs +++ b/test/OpenFeature.SDK.Tests/OpenFeatureHookTests.cs @@ -11,11 +11,12 @@ namespace OpenFeature.SDK.Tests { - public class OpenFeatureHookTests + public class OpenFeatureHookTests : ClearOpenFeatureInstanceFixture { [Fact] [Specification("1.5.1", "The `evaluation options` structure's `hooks` field denotes an ordered collection of hooks that the client MUST execute for the respective flag evaluation, in addition to those already configured.")] - [Specification("4.4.2", "Hooks MUST be evaluated in the following order: - before: API, Client, Invocation - after: Invocation, Client, API - error (if applicable): Invocation, Client, API - finally: Invocation, Client, API")] + [Specification("2.10", "The provider interface MUST define a provider hook mechanism which can be optionally implemented in order to add hook instances to the evaluation life-cycle.")] + [Specification("4.4.2", "Hooks MUST be evaluated in the following order: - before: API, Client, Invocation, Provider - after: Provider, Invocation, Client, API - error (if applicable): Provider, Invocation, Client, API - finally: Provider, Invocation, Client, API")] public async Task Hooks_Should_Be_Called_In_Order() { var fixture = new Fixture(); @@ -23,57 +24,102 @@ public async Task Hooks_Should_Be_Called_In_Order() var clientVersion = fixture.Create(); var flagName = fixture.Create(); var defaultValue = fixture.Create(); - var clientHook = new Mock(); - var invocationHook = new Mock(); + var apiHook = new Mock(MockBehavior.Strict); + var clientHook = new Mock(MockBehavior.Strict); + var invocationHook = new Mock(MockBehavior.Strict); + var providerHook = new Mock(MockBehavior.Strict); var sequence = new MockSequence(); - invocationHook.InSequence(sequence).Setup(x => x.Before(It.IsAny>(), - It.IsAny>())) + apiHook.InSequence(sequence).Setup(x => x.Before(It.IsAny>(), + It.IsAny>())) .ReturnsAsync(new EvaluationContext()); clientHook.InSequence(sequence).Setup(x => x.Before(It.IsAny>(), + It.IsAny>())) + .ReturnsAsync(new EvaluationContext()); + + invocationHook.InSequence(sequence).Setup(x => x.Before(It.IsAny>(), It.IsAny>())) .ReturnsAsync(new EvaluationContext()); + providerHook.InSequence(sequence).Setup(x => x.Before(It.IsAny>(), + It.IsAny>())) + .ReturnsAsync(new EvaluationContext()); + + providerHook.InSequence(sequence).Setup(x => x.After(It.IsAny>(), + It.IsAny>(), + It.IsAny>())).Returns(Task.CompletedTask); + invocationHook.InSequence(sequence).Setup(x => x.After(It.IsAny>(), It.IsAny>(), - It.IsAny>())); + It.IsAny>())).Returns(Task.CompletedTask); clientHook.InSequence(sequence).Setup(x => x.After(It.IsAny>(), It.IsAny>(), - It.IsAny>())); + It.IsAny>())).Returns(Task.CompletedTask); + + apiHook.InSequence(sequence).Setup(x => x.After(It.IsAny>(), + It.IsAny>(), + It.IsAny>())).Returns(Task.CompletedTask); + + providerHook.InSequence(sequence).Setup(x => x.Finally(It.IsAny>(), + It.IsAny>())).Returns(Task.CompletedTask); invocationHook.InSequence(sequence).Setup(x => x.Finally(It.IsAny>(), - It.IsAny>())); + It.IsAny>())).Returns(Task.CompletedTask); clientHook.InSequence(sequence).Setup(x => x.Finally(It.IsAny>(), - It.IsAny>())); + It.IsAny>())).Returns(Task.CompletedTask); - OpenFeature.Instance.SetProvider(new NoOpFeatureProvider()); + apiHook.InSequence(sequence).Setup(x => x.Finally(It.IsAny>(), + It.IsAny>())).Returns(Task.CompletedTask); + + var testProvider = new TestProvider(); + testProvider.AddHook(providerHook.Object); + OpenFeature.Instance.AddHooks(apiHook.Object); + OpenFeature.Instance.SetProvider(testProvider); var client = OpenFeature.Instance.GetClient(clientName, clientVersion); client.AddHooks(clientHook.Object); await client.GetBooleanValue(flagName, defaultValue, new EvaluationContext(), new FlagEvaluationOptions(invocationHook.Object, new Dictionary())); - invocationHook.Verify(x => x.Before( + apiHook.Verify(x => x.Before( It.IsAny>(), It.IsAny>()), Times.Once); clientHook.Verify(x => x.Before( It.IsAny>(), It.IsAny>()), Times.Once); + invocationHook.Verify(x => x.Before( + It.IsAny>(), It.IsAny>()), Times.Once); + + providerHook.Verify(x => x.Before( + It.IsAny>(), It.IsAny>()), Times.Once); + + providerHook.Verify(x => x.After( + It.IsAny>(), It.IsAny>(), It.IsAny>()), Times.Once); + invocationHook.Verify(x => x.After( It.IsAny>(), It.IsAny>(), It.IsAny>()), Times.Once); clientHook.Verify(x => x.After( It.IsAny>(), It.IsAny>(), It.IsAny>()), Times.Once); + apiHook.Verify(x => x.After( + It.IsAny>(), It.IsAny>(), It.IsAny>()), Times.Once); + + providerHook.Verify(x => x.Finally( + It.IsAny>(), It.IsAny>()), Times.Once); + invocationHook.Verify(x => x.Finally( It.IsAny>(), It.IsAny>()), Times.Once); clientHook.Verify(x => x.Finally( It.IsAny>(), It.IsAny>()), Times.Once); + + apiHook.Verify(x => x.Finally( + It.IsAny>(), It.IsAny>()), Times.Once); } [Fact] @@ -122,8 +168,8 @@ public void Hook_Context_Should_Have_Properties_And_Be_Immutable() public async Task Evaluation_Context_Must_Be_Mutable_Before_Hook() { var evaluationContext = new EvaluationContext { ["test"] = "test" }; - var hook1 = new Mock(); - var hook2 = new Mock(); + var hook1 = new Mock(MockBehavior.Strict); + var hook2 = new Mock(MockBehavior.Strict); var hookContext = new HookContext("test", false, FlagValueType.Boolean, new ClientMetadata("test", "1.0.0"), new Metadata(NoOpProvider.NoOpProviderName), evaluationContext); @@ -182,14 +228,17 @@ public async Task Hook_Should_Return_No_Errors() [Specification("4.5.3", "The hook MUST NOT alter the `hook hints` structure.")] public async Task Hook_Should_Execute_In_Correct_Order() { - var featureProvider = new Mock(); - var hook = new Mock(); + var featureProvider = new Mock(MockBehavior.Strict); + var hook = new Mock(MockBehavior.Strict); var sequence = new MockSequence(); featureProvider.Setup(x => x.GetMetadata()) .Returns(new Metadata(null)); + featureProvider.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); + hook.InSequence(sequence).Setup(x => x.Before(It.IsAny>(), It.IsAny>())) .ReturnsAsync(new EvaluationContext()); @@ -217,41 +266,50 @@ public async Task Hook_Should_Execute_In_Correct_Order() } [Fact] - [Specification("4.4.1", "The API, Client and invocation MUST have a method for registering hooks which accepts `flag evaluation options`")] + [Specification("4.4.1", "The API, Client, Provider, and invocation MUST have a method for registering hooks.")] public async Task Register_Hooks_Should_Be_Available_At_All_Levels() { - var hook1 = new Mock(); - var hook2 = new Mock(); - var hook3 = new Mock(); + var hook1 = new Mock(MockBehavior.Strict); + var hook2 = new Mock(MockBehavior.Strict); + var hook3 = new Mock(MockBehavior.Strict); + var hook4 = new Mock(MockBehavior.Strict); + var testProvider = new TestProvider(); + testProvider.AddHook(hook4.Object); OpenFeature.Instance.AddHooks(hook1.Object); + OpenFeature.Instance.SetProvider(testProvider); var client = OpenFeature.Instance.GetClient(); client.AddHooks(hook2.Object); await client.GetBooleanValue("test", false, null, new FlagEvaluationOptions(hook3.Object, new Dictionary())); - client.ClearHooks(); + OpenFeature.Instance.GetHooks().Count.Should().Be(1); + client.GetHooks().Count.Should().Be(1); + testProvider.GetProviderHooks().Count.Should().Be(1); } [Fact] [Specification("4.4.3", "If a `finally` hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining `finally` hooks.")] public async Task Finally_Hook_Should_Be_Executed_Even_If_Abnormal_Termination() { - var featureProvider = new Mock(); - var hook1 = new Mock(); - var hook2 = new Mock(); + var featureProvider = new Mock(MockBehavior.Strict); + var hook1 = new Mock(MockBehavior.Strict); + var hook2 = new Mock(MockBehavior.Strict); var sequence = new MockSequence(); featureProvider.Setup(x => x.GetMetadata()) .Returns(new Metadata(null)); + featureProvider.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); + hook1.InSequence(sequence).Setup(x => - x.Before(It.IsAny>(), It.IsAny>())) + x.Before(It.IsAny>(), null)) .ReturnsAsync(new EvaluationContext()); hook2.InSequence(sequence).Setup(x => - x.Before(It.IsAny>(), It.IsAny>())) + x.Before(It.IsAny>(), null)) .ReturnsAsync(new EvaluationContext()); featureProvider.InSequence(sequence) @@ -259,76 +317,83 @@ public async Task Finally_Hook_Should_Be_Executed_Even_If_Abnormal_Termination() null)) .ReturnsAsync(new ResolutionDetails("test", false)); - hook1.InSequence(sequence).Setup(x => x.After(It.IsAny>(), - It.IsAny>(), It.IsAny>())); - hook2.InSequence(sequence).Setup(x => x.After(It.IsAny>(), - It.IsAny>(), It.IsAny>())); + It.IsAny>(), null)) + .Returns(Task.CompletedTask); - hook1.Setup(x => - x.Finally(It.IsAny>(), It.IsAny>())) - .Throws(new Exception()); + hook1.InSequence(sequence).Setup(x => x.After(It.IsAny>(), + It.IsAny>(), null)) + .Returns(Task.CompletedTask); hook2.InSequence(sequence).Setup(x => - x.Finally(It.IsAny>(), It.IsAny>())); + x.Finally(It.IsAny>(), null)) + .Returns(Task.CompletedTask); + + hook1.InSequence(sequence).Setup(x => + x.Finally(It.IsAny>(), null)) + .Throws(new Exception()); OpenFeature.Instance.SetProvider(featureProvider.Object); var client = OpenFeature.Instance.GetClient(); client.AddHooks(new[] { hook1.Object, hook2.Object }); + client.GetHooks().Count.Should().Be(2); await client.GetBooleanValue("test", false); - hook1.Verify(x => x.Before(It.IsAny>(), It.IsAny>()), Times.Once); - hook1.Verify(x => x.After(It.IsAny>(), It.IsAny>(), It.IsAny>()), Times.Once); - hook1.Verify(x => x.Finally(It.IsAny>(), It.IsAny>()), Times.Once); - hook2.Verify(x => x.Before(It.IsAny>(), It.IsAny>()), Times.Once); - hook2.Verify(x => x.After(It.IsAny>(), It.IsAny>(), It.IsAny>()), Times.Once); - hook2.Verify(x => x.Finally(It.IsAny>(), It.IsAny>()), Times.Once); + hook1.Verify(x => x.Before(It.IsAny>(), null), Times.Once); + hook2.Verify(x => x.Before(It.IsAny>(), null), Times.Once); featureProvider.Verify(x => x.ResolveBooleanValue(It.IsAny(), It.IsAny(), It.IsAny(), null), Times.Once); + hook2.Verify(x => x.After(It.IsAny>(), It.IsAny>(), null), Times.Once); + hook1.Verify(x => x.After(It.IsAny>(), It.IsAny>(), null), Times.Once); + hook2.Verify(x => x.Finally(It.IsAny>(), null), Times.Once); + hook1.Verify(x => x.Finally(It.IsAny>(), null), Times.Once); } [Fact] - [Specification("4.4.4", "If a `finally` hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining `finally` hooks.")] + [Specification("4.4.4", "If an `error` hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining `error` hooks.")] public async Task Error_Hook_Should_Be_Executed_Even_If_Abnormal_Termination() { - var featureProvider = new Mock(); - var hook1 = new Mock(); - var hook2 = new Mock(); + var featureProvider1 = new Mock(MockBehavior.Strict); + var hook1 = new Mock(MockBehavior.Strict); + var hook2 = new Mock(MockBehavior.Strict); var sequence = new MockSequence(); - featureProvider.Setup(x => x.GetMetadata()) + featureProvider1.Setup(x => x.GetMetadata()) .Returns(new Metadata(null)); + featureProvider1.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); hook1.InSequence(sequence).Setup(x => - x.Before(It.IsAny>(), It.IsAny>())) + x.Before(It.IsAny>(), null)) .ReturnsAsync(new EvaluationContext()); hook2.InSequence(sequence).Setup(x => - x.Before(It.IsAny>(), It.IsAny>())) + x.Before(It.IsAny>(), null)) .ReturnsAsync(new EvaluationContext()); - featureProvider.InSequence(sequence) + featureProvider1.InSequence(sequence) .Setup(x => x.ResolveBooleanValue(It.IsAny(), It.IsAny(), It.IsAny(), null)) .Throws(new Exception()); - hook1.InSequence(sequence).Setup(x => + hook2.InSequence(sequence).Setup(x => x.Error(It.IsAny>(), It.IsAny(), null)) - .ThrowsAsync(new Exception()); + .Returns(Task.CompletedTask); - hook2.InSequence(sequence).Setup(x => - x.Error(It.IsAny>(), It.IsAny(), null)); + hook1.InSequence(sequence).Setup(x => + x.Error(It.IsAny>(), It.IsAny(), null)) + .Returns(Task.CompletedTask); - OpenFeature.Instance.SetProvider(featureProvider.Object); + OpenFeature.Instance.SetProvider(featureProvider1.Object); var client = OpenFeature.Instance.GetClient(); client.AddHooks(new[] { hook1.Object, hook2.Object }); await client.GetBooleanValue("test", false); - hook1.Verify(x => x.Before(It.IsAny>(), It.IsAny>()), Times.Once); + hook1.Verify(x => x.Before(It.IsAny>(), null), Times.Once); + hook2.Verify(x => x.Before(It.IsAny>(), null), Times.Once); hook1.Verify(x => x.Error(It.IsAny>(), It.IsAny(), null), Times.Once); - hook2.Verify(x => x.Before(It.IsAny>(), It.IsAny>()), Times.Once); hook2.Verify(x => x.Error(It.IsAny>(), It.IsAny(), null), Times.Once); } @@ -336,14 +401,16 @@ public async Task Error_Hook_Should_Be_Executed_Even_If_Abnormal_Termination() [Specification("4.4.6", "If an error occurs during the evaluation of `before` or `after` hooks, any remaining hooks in the `before` or `after` stages MUST NOT be invoked.")] public async Task Error_Occurs_During_Before_After_Evaluation_Should_Not_Invoke_Any_Remaining_Hooks() { - var featureProvider = new Mock(); - var hook1 = new Mock(); - var hook2 = new Mock(); + var featureProvider = new Mock(MockBehavior.Strict); + var hook1 = new Mock(MockBehavior.Strict); + var hook2 = new Mock(MockBehavior.Strict); var sequence = new MockSequence(); featureProvider.Setup(x => x.GetMetadata()) .Returns(new Metadata(null)); + featureProvider.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); hook1.InSequence(sequence).Setup(x => x.Before(It.IsAny>(), It.IsAny>())) @@ -371,29 +438,35 @@ public async Task Error_Occurs_During_Before_After_Evaluation_Should_Not_Invoke_ [Specification("4.5.1", "`Flag evaluation options` MAY contain `hook hints`, a map of data to be provided to hook invocations.")] public async Task Hook_Hints_May_Be_Optional() { - var featureProvider = new Mock(); - var hook = new Mock(); + var featureProvider = new Mock(MockBehavior.Strict); + var hook = new Mock(MockBehavior.Strict); var defaultEmptyHookHints = new Dictionary(); var flagOptions = new FlagEvaluationOptions(hook.Object); + EvaluationContext evaluationContext = null; var sequence = new MockSequence(); featureProvider.Setup(x => x.GetMetadata()) .Returns(new Metadata(null)); + featureProvider.Setup(x => x.GetProviderHooks()) + .Returns(Array.Empty()); + hook.InSequence(sequence) .Setup(x => x.Before(It.IsAny>(), defaultEmptyHookHints)) - .ReturnsAsync(new EvaluationContext()); + .ReturnsAsync(evaluationContext); featureProvider.InSequence(sequence) .Setup(x => x.ResolveBooleanValue(It.IsAny(), It.IsAny(), It.IsAny(), flagOptions)) .ReturnsAsync(new ResolutionDetails("test", false)); hook.InSequence(sequence) - .Setup(x => x.After(It.IsAny>(), It.IsAny>(), defaultEmptyHookHints)); + .Setup(x => x.After(It.IsAny>(), It.IsAny>(), defaultEmptyHookHints)) + .Returns(Task.CompletedTask); hook.InSequence(sequence) - .Setup(x => x.Finally(It.IsAny>(), defaultEmptyHookHints)); + .Setup(x => x.Finally(It.IsAny>(), defaultEmptyHookHints)) + .Returns(Task.CompletedTask); OpenFeature.Instance.SetProvider(featureProvider.Object); var client = OpenFeature.Instance.GetClient(); @@ -410,8 +483,8 @@ public async Task Hook_Hints_May_Be_Optional() [Specification("4.4.7", "If an error occurs in the `before` hooks, the default value MUST be returned.")] public async Task When_Error_Occurs_In_Before_Hook_Should_Return_Default_Value() { - var featureProvider = new Mock(); - var hook = new Mock(); + var featureProvider = new Mock(MockBehavior.Strict); + var hook = new Mock(MockBehavior.Strict); var exceptionToThrow = new Exception("Fails during default"); var sequence = new MockSequence(); @@ -424,10 +497,10 @@ public async Task When_Error_Occurs_In_Before_Hook_Should_Return_Default_Value() .ThrowsAsync(exceptionToThrow); hook.InSequence(sequence) - .Setup(x => x.Error(It.IsAny>(), It.IsAny(), null)); + .Setup(x => x.Error(It.IsAny>(), It.IsAny(), null)).Returns(Task.CompletedTask); hook.InSequence(sequence) - .Setup(x => x.Finally(It.IsAny>(), null)); + .Setup(x => x.Finally(It.IsAny>(), null)).Returns(Task.CompletedTask); var client = OpenFeature.Instance.GetClient(); client.AddHooks(hook.Object); diff --git a/test/OpenFeature.SDK.Tests/OpenFeatureTests.cs b/test/OpenFeature.SDK.Tests/OpenFeatureTests.cs index 5c7cb768..03144afa 100644 --- a/test/OpenFeature.SDK.Tests/OpenFeatureTests.cs +++ b/test/OpenFeature.SDK.Tests/OpenFeatureTests.cs @@ -1,9 +1,5 @@ -using System; -using System.Collections.Generic; -using System.Linq; using AutoFixture; using FluentAssertions; -using Microsoft.Extensions.Logging; using Moq; using OpenFeature.SDK.Constant; using OpenFeature.SDK.Model; @@ -12,7 +8,7 @@ namespace OpenFeature.SDK.Tests { - public class OpenFeatureTests + public class OpenFeatureTests : ClearOpenFeatureInstanceFixture { [Fact] [Specification("1.1.1", "The API, and any state it maintains SHOULD exist as a global singleton, even in cases wherein multiple versions of the API are present at runtime.")] @@ -29,10 +25,10 @@ public void OpenFeature_Should_Be_Singleton() public void OpenFeature_Should_Add_Hooks() { var openFeature = OpenFeature.Instance; - var hook1 = new Mock().Object; - var hook2 = new Mock().Object; - var hook3 = new Mock().Object; - var hook4 = new Mock().Object; + var hook1 = new Mock(MockBehavior.Strict).Object; + var hook2 = new Mock(MockBehavior.Strict).Object; + var hook3 = new Mock(MockBehavior.Strict).Object; + var hook4 = new Mock(MockBehavior.Strict).Object; openFeature.ClearHooks(); diff --git a/test/OpenFeature.SDK.Tests/TestImplementations.cs b/test/OpenFeature.SDK.Tests/TestImplementations.cs index 0898100f..0b2d29cd 100644 --- a/test/OpenFeature.SDK.Tests/TestImplementations.cs +++ b/test/OpenFeature.SDK.Tests/TestImplementations.cs @@ -37,48 +37,54 @@ public override Task Finally(HookContext context, IReadOnlyDictionary _hooks = new List(); + public static string Name => "test-provider"; - public Metadata GetMetadata() + public void AddHook(Hook hook) => this._hooks.Add(hook); + + public override IReadOnlyList GetProviderHooks() => this._hooks.AsReadOnly(); + + public override Metadata GetMetadata() { return new Metadata(Name); } - public Task> ResolveBooleanValue(string flagKey, bool defaultValue, + public override Task> ResolveBooleanValue(string flagKey, bool defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { - throw new NotImplementedException(); + return Task.FromResult(new ResolutionDetails(flagKey, defaultValue)); } - public Task> ResolveStringValue(string flagKey, string defaultValue, + public override Task> ResolveStringValue(string flagKey, string defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { - throw new NotImplementedException(); + return Task.FromResult(new ResolutionDetails(flagKey, defaultValue)); } - public Task> ResolveIntegerValue(string flagKey, int defaultValue, + public override Task> ResolveIntegerValue(string flagKey, int defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { - throw new NotImplementedException(); + return Task.FromResult(new ResolutionDetails(flagKey, defaultValue)); } - public Task> ResolveDoubleValue(string flagKey, double defaultValue, + public override Task> ResolveDoubleValue(string flagKey, double defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { - throw new NotImplementedException(); + return Task.FromResult(new ResolutionDetails(flagKey, defaultValue)); } - public Task> ResolveStructureValue(string flagKey, T defaultValue, + public override Task> ResolveStructureValue(string flagKey, T defaultValue, EvaluationContext context = null, FlagEvaluationOptions config = null) { - throw new NotImplementedException(); + return Task.FromResult(new ResolutionDetails(flagKey, defaultValue)); } } }