lock providers and hooks to an evaluation's transaction

Signed-off-by: Skye Gill <gill.skye95@gmail.com>
open-feature · Oct 7, 2022 · 40af78d · 40af78d
1 parent efd2342
commit 40af78d
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 8 deletions.
diff --git a/pkg/openfeature/client.go b/pkg/openfeature/client.go
@@ -556,13 +556,21 @@ func (c *Client) evaluate(
 	)
 	evalCtx = mergeContexts(evalCtx, c.evaluationContext, api.evaluationContext()) // API (global) -> client -> invocation
 
+	// ensure that the same provider & hooks are used across this transaction to avoid unexpected behaviour
+	api.RLock()
+	provider := api.prvder
+	globalHooks := api.hks
+	api.RUnlock()
+	apiClientInvocationProviderHooks := append(append(append(globalHooks, c.hooks...), options.hooks...), provider.Hooks()...) // API, Client, Invocation, Provider
+	providerInvocationClientApiHooks := append(append(append(provider.Hooks(), options.hooks...), c.hooks...), globalHooks...) // Provider, Invocation, Client, API
+
 	var err error
 	hookCtx := HookContext{
 		flagKey:           flag,
 		flagType:          flagType,
 		defaultValue:      defaultValue,
 		clientMetadata:    c.metadata,
-		providerMetadata:  api.provider().Metadata(),
+		providerMetadata:  provider.Metadata(),
 		evaluationContext: evalCtx,
 	}
 	evalDetails := InterfaceEvaluationDetails{
@@ -573,8 +581,6 @@ func (c *Client) evaluate(
 		},
 	}
 
-	apiClientInvocationProviderHooks := append(append(append(api.hooks(), c.hooks...), options.hooks...), api.provider().Hooks()...) // API, Client, Invocation, Provider
-	providerInvocationClientApiHooks := append(append(append(api.provider().Hooks(), options.hooks...), c.hooks...), api.hooks()...) // Provider, Invocation, Client, API
 	defer func() {
 		c.finallyHooks(hookCtx, providerInvocationClientApiHooks, options)
 	}()
@@ -595,25 +601,25 @@ func (c *Client) evaluate(
 	var resolution InterfaceResolutionDetail
 	switch flagType {
 	case Object:
-		resolution = api.provider().ObjectEvaluation(ctx, flag, defaultValue, flatCtx)
+		resolution = provider.ObjectEvaluation(ctx, flag, defaultValue, flatCtx)
 	case Boolean:
 		defValue := defaultValue.(bool)
-		res := api.provider().BooleanEvaluation(ctx, flag, defValue, flatCtx)
+		res := provider.BooleanEvaluation(ctx, flag, defValue, flatCtx)
 		resolution.ProviderResolutionDetail = res.ProviderResolutionDetail
 		resolution.Value = res.Value
 	case String:
 		defValue := defaultValue.(string)
-		res := api.provider().StringEvaluation(ctx, flag, defValue, flatCtx)
+		res := provider.StringEvaluation(ctx, flag, defValue, flatCtx)
 		resolution.ProviderResolutionDetail = res.ProviderResolutionDetail
 		resolution.Value = res.Value
 	case Float:
 		defValue := defaultValue.(float64)
-		res := api.provider().FloatEvaluation(ctx, flag, defValue, flatCtx)
+		res := provider.FloatEvaluation(ctx, flag, defValue, flatCtx)
 		resolution.ProviderResolutionDetail = res.ProviderResolutionDetail
 		resolution.Value = res.Value
 	case Int:
 		defValue := defaultValue.(int64)
-		res := api.provider().IntEvaluation(ctx, flag, defValue, flatCtx)
+		res := provider.IntEvaluation(ctx, flag, defValue, flatCtx)
 		resolution.ProviderResolutionDetail = res.ProviderResolutionDetail
 		resolution.Value = res.Value
 	}

diff --git a/pkg/openfeature/client_test.go b/pkg/openfeature/client_test.go
@@ -806,3 +806,34 @@ func TestErrorCodeFromProviderReturnedInEvaluationDetails(t *testing.T) {
 		)
 	}
 }
+
+func TestSwitchingProvidersMidEvaluationCausesNoImpactToEvaluation(t *testing.T) {
+	defer t.Cleanup(initSingleton)
+	ctrl := gomock.NewController(t)
+
+	mockProvider1 := NewMockFeatureProvider(ctrl)
+	mockProvider2 := NewMockFeatureProvider(ctrl)
+	mockProvider1Hook := NewMockHook(ctrl)
+	mockProvider1.EXPECT().Metadata().AnyTimes()
+	mockProvider2.EXPECT().Metadata().AnyTimes()
+	mockProvider1.EXPECT().Hooks().Return([]Hook{mockProvider1Hook}).AnyTimes()
+
+	// set new provider during initial provider's Before hook
+	mockProvider1Hook.EXPECT().Before(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(_ HookContext, _ HookHints) (*EvaluationContext, error) {
+			SetProvider(mockProvider2)
+			return nil, nil
+		})
+	SetProvider(mockProvider1)
+
+	mockProvider1.EXPECT().BooleanEvaluation(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
+	// ensure that the first provider's hooks are still fired
+	mockProvider1Hook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any())
+	mockProvider1Hook.EXPECT().Finally(gomock.Any(), gomock.Any())
+
+	client := NewClient("test")
+	_, err := client.BooleanValue(context.Background(), "foo", true, EvaluationContext{})
+	if err != nil {
+		t.Error(err)
+	}
+}