From 2f0edc05b5cd7c09bfcbb8663cc2a5a7cc9151ee Mon Sep 17 00:00:00 2001 From: bbland1 <104288486+bbland1@users.noreply.github.com> Date: Wed, 26 Feb 2025 01:54:59 -0500 Subject: [PATCH 1/2] feat: add evaluation details to finally hook Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com> --- openfeature/client.go | 6 ++-- openfeature/client_test.go | 4 +-- openfeature/hooks.go | 4 +-- openfeature/hooks/logging_hook.go | 2 +- openfeature/hooks_mock_test.go | 4 +-- openfeature/hooks_test.go | 52 +++++++++++++++---------------- 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/openfeature/client.go b/openfeature/client.go index 095d398d..6b20a33c 100644 --- a/openfeature/client.go +++ b/openfeature/client.go @@ -710,7 +710,7 @@ func (c *Client) evaluate( } defer func() { - c.finallyHooks(ctx, hookCtx, providerInvocationClientApiHooks, options) + c.finallyHooks(ctx, hookCtx, providerInvocationClientApiHooks, evalDetails, options) }() // bypass short-circuit logic for the Noop provider; it is essentially stateless and a "special case" @@ -828,9 +828,9 @@ func (c *Client) errorHooks(ctx context.Context, hookCtx HookContext, hooks []Ho } } -func (c *Client) finallyHooks(ctx context.Context, hookCtx HookContext, hooks []Hook, options EvaluationOptions) { +func (c *Client) finallyHooks(ctx context.Context, hookCtx HookContext, hooks []Hook, evalDetails InterfaceEvaluationDetails, options EvaluationOptions) { for _, hook := range hooks { - hook.Finally(ctx, hookCtx, options.hookHints) + hook.Finally(ctx, hookCtx,evalDetails, options.hookHints) } } diff --git a/openfeature/client_test.go b/openfeature/client_test.go index 137b57ae..e54809c7 100644 --- a/openfeature/client_test.go +++ b/openfeature/client_test.go @@ -1314,7 +1314,7 @@ func TestRequirement_1_7_6(t *testing.T) { ctrl := gomock.NewController(t) mockHook := NewMockHook(ctrl) mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), ProviderNotReadyError, gomock.Any()) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) notReadyEventingProvider := struct { FeatureProvider @@ -1377,7 +1377,7 @@ func TestRequirement_1_7_7(t *testing.T) { ctrl := gomock.NewController(t) mockHook := NewMockHook(ctrl) mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), ProviderFatalError, gomock.Any()) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) client := GetApiInstance().GetNamedClient(t.Name()) client.AddHooks(mockHook) diff --git a/openfeature/hooks.go b/openfeature/hooks.go index a0a475f7..70d3b563 100644 --- a/openfeature/hooks.go +++ b/openfeature/hooks.go @@ -9,7 +9,7 @@ type Hook interface { Before(ctx context.Context, hookContext HookContext, hookHints HookHints) (*EvaluationContext, error) After(ctx context.Context, hookContext HookContext, flagEvaluationDetails InterfaceEvaluationDetails, hookHints HookHints) error Error(ctx context.Context, hookContext HookContext, err error, hookHints HookHints) - Finally(ctx context.Context, hookContext HookContext, hookHints HookHints) + Finally(ctx context.Context, hookContext HookContext, flagEvaluationDetails InterfaceEvaluationDetails, hookHints HookHints) } // HookHints contains a map of hints for hooks @@ -108,4 +108,4 @@ func (UnimplementedHook) After(context.Context, HookContext, InterfaceEvaluation return nil } func (UnimplementedHook) Error(context.Context, HookContext, error, HookHints) {} -func (UnimplementedHook) Finally(context.Context, HookContext, HookHints) {} +func (UnimplementedHook) Finally(context.Context, HookContext, InterfaceEvaluationDetails, HookHints) {} diff --git a/openfeature/hooks/logging_hook.go b/openfeature/hooks/logging_hook.go index 0d08d66a..7e01d64c 100644 --- a/openfeature/hooks/logging_hook.go +++ b/openfeature/hooks/logging_hook.go @@ -91,6 +91,6 @@ func (h *LoggingHook) Error(ctx context.Context, hookContext of.HookContext, err h.logger.Error("Error stage", args...) } -func (h *LoggingHook) Finally(ctx context.Context, hCtx of.HookContext, hint of.HookHints) { +func (h *LoggingHook) Finally(ctx context.Context, hCtx of.HookContext,flagEvaluationDetails of.InterfaceEvaluationDetails, hint of.HookHints) { } diff --git a/openfeature/hooks_mock_test.go b/openfeature/hooks_mock_test.go index 456ab535..197d7995 100644 --- a/openfeature/hooks_mock_test.go +++ b/openfeature/hooks_mock_test.go @@ -76,13 +76,13 @@ func (mr *MockHookMockRecorder) Error(ctx, hookContext, err, hookHints interface } // Finally mocks base method. -func (m *MockHook) Finally(ctx context.Context, hookContext HookContext, hookHints HookHints) { +func (m *MockHook) Finally(ctx context.Context, hookContext HookContext, flagEvaluationDetails InterfaceEvaluationDetails, hookHints HookHints) { m.ctrl.T.Helper() m.ctrl.Call(m, "Finally", ctx, hookContext, hookHints) } // Finally indicates an expected call of Finally. -func (mr *MockHookMockRecorder) Finally(ctx, hookContext, hookHints interface{}) *gomock.Call { +func (mr *MockHookMockRecorder) Finally(ctx, hookContext, flagEvaluationDetails, hookHints interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Finally", reflect.TypeOf((*MockHook)(nil).Finally), ctx, hookContext, hookHints) } diff --git a/openfeature/hooks_test.go b/openfeature/hooks_test.go index 2ffe9338..21bc81c5 100644 --- a/openfeature/hooks_test.go +++ b/openfeature/hooks_test.go @@ -174,7 +174,7 @@ func TestRequirement_4_3_2(t *testing.T) { mockProvider.EXPECT().StringEvaluation(gomock.Any(), flagKey, defaultValue, flatCtx). After(mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any())) mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook)) if err != nil { @@ -244,9 +244,9 @@ func TestRequirement_4_3_3(t *testing.T) { mockHook2.EXPECT().Before(gomock.Any(), hook2Ctx, gomock.Any()) mockHook1.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) mockHook2.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook1, mockHook2)) if err != nil { @@ -321,7 +321,7 @@ func TestRequirement_4_3_4(t *testing.T) { } mockProvider.EXPECT().StringEvaluation(gomock.Any(), flagKey, defaultValue, flattenContext(expectedMergedContext)) mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, invEvalCtx, WithHooks(mockHook)) if err != nil { @@ -357,7 +357,7 @@ func TestRequirement_4_3_5(t *testing.T) { // assert that the After hooks are executed after the flag evaluation mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). After(mockProvider.EXPECT().StringEvaluation(gomock.Any(), flagKey, defaultValue, flatCtx)) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook)) if err != nil { @@ -407,7 +407,7 @@ func TestRequirement_4_3_6(t *testing.T) { // assert that the Error hooks are executed after the failed Before hooks mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). After(mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced"))) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook)) if err == nil { @@ -440,7 +440,7 @@ func TestRequirement_4_3_6(t *testing.T) { }, }), ) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook)) if err == nil { @@ -467,7 +467,7 @@ func TestRequirement_4_3_6(t *testing.T) { // assert that the Error hooks are executed after the failed After hooks mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). After(mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("forced"))) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook)) if err == nil { @@ -514,7 +514,7 @@ func TestRequirement_4_3_7(t *testing.T) { mockProvider.EXPECT().Hooks().AnyTimes() // assert that the Finally hook runs after the Before & After stages - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()). + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). After(mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())). After(mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any())) mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx) @@ -541,7 +541,7 @@ func TestRequirement_4_3_7(t *testing.T) { mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced")) // assert that the Finally hook runs after the Error stage - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()). + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). After(mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())) _, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook)) @@ -554,7 +554,7 @@ func TestRequirement_4_3_7(t *testing.T) { mockHook := NewMockHook(ctrl) type requirement interface { - Finally(ctx context.Context, hookContext HookContext, hookHints HookHints) + Finally(ctx context.Context, hookContext HookContext, flagEvaluationDetails InterfaceEvaluationDetails, hookHints HookHints) } var hookI interface{} = mockHook @@ -673,10 +673,10 @@ func TestRequirement_4_4_2(t *testing.T) { After(mockProviderHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())) // finally: Invocation, Client, API - mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()). - After(mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())). - After(mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())). - After(mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())) + mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + After(mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())). + After(mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())). + After(mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())) mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx) @@ -717,10 +717,10 @@ func TestRequirement_4_4_2(t *testing.T) { After(mockInvocationHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())). After(mockProviderHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())) - mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) - mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) - mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) - mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) + mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) + mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) + mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockInvocationHook)) if err == nil { @@ -779,9 +779,9 @@ func TestRequirement_4_4_6(t *testing.T) { mockHook1.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced")) // the lack of mockHook2.EXPECT().Before() asserts that remaining hooks aren't invoked after an error mockHook1.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) mockHook2.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook1, mockHook2)) if err == nil { @@ -814,9 +814,9 @@ func TestRequirement_4_4_6(t *testing.T) { mockHook1.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("forced")) // the lack of mockHook2.EXPECT().After() asserts that remaining hooks aren't invoked after an error mockHook1.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) mockHook2.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx) @@ -850,7 +850,7 @@ func TestRequirement_4_4_7(t *testing.T) { mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced")) mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) res, err := GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook)) if err == nil { @@ -900,7 +900,7 @@ func TestRequirement_4_5_2(t *testing.T) { mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), hookHints) mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), hookHints) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), hookHints) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), hookHints) mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx) @@ -933,7 +933,7 @@ func TestRequirement_4_5_2(t *testing.T) { mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced")) mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), hookHints) - mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()) + mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) _, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook), WithHookHints(hookHints)) if err == nil { From 87d35413c418ccf5cdcdeee3e394397d79761b2a Mon Sep 17 00:00:00 2001 From: Brianna Bland <104288486+bbland1@users.noreply.github.com> Date: Thu, 27 Feb 2025 13:31:32 -0500 Subject: [PATCH 2/2] fixed spacing issue openfeature/hooks/logging_hook.go Co-authored-by: Todd Baert Signed-off-by: Brianna Bland <104288486+bbland1@users.noreply.github.com> --- openfeature/hooks/logging_hook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfeature/hooks/logging_hook.go b/openfeature/hooks/logging_hook.go index 7e01d64c..16109d04 100644 --- a/openfeature/hooks/logging_hook.go +++ b/openfeature/hooks/logging_hook.go @@ -91,6 +91,6 @@ func (h *LoggingHook) Error(ctx context.Context, hookContext of.HookContext, err h.logger.Error("Error stage", args...) } -func (h *LoggingHook) Finally(ctx context.Context, hCtx of.HookContext,flagEvaluationDetails of.InterfaceEvaluationDetails, hint of.HookHints) { +func (h *LoggingHook) Finally(ctx context.Context, hCtx of.HookContext, flagEvaluationDetails of.InterfaceEvaluationDetails, hint of.HookHints) { }