test to ensure locking of a provider to a transaction

Signed-off-by: Skye Gill <gill.skye95@gmail.com>

Makefile

@@ -2,6 +2,7 @@
 mockgen:
 	mockgen -source=pkg/openfeature/provider.go -destination=pkg/openfeature/provider_mock_test.go -package=openfeature
 	mockgen -source=pkg/openfeature/hooks.go -destination=pkg/openfeature/hooks_mock_test.go -package=openfeature
+	mockgen -source=pkg/openfeature/mutex.go -destination=pkg/openfeature/mutex_mock_test.go -package=openfeature
 test:
 	go test -cover ./...
 lint:

pkg/openfeature/client.go

@@ -554,13 +554,15 @@ func (c *Client) evaluate(
 		"evaluating flag", "flag", flag, "type", flagType.String(), "defaultValue", defaultValue,
 		"evaluationContext", evalCtx, "evaluationOptions", options,
 	)
-	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
+	globalCtx := api.evalCtx
 	api.RUnlock()
+
+	evalCtx = mergeContexts(evalCtx, c.evaluationContext, globalCtx)                                                           // API (global) -> client -> invocation
 	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
 

pkg/openfeature/client_test.go

@@ -837,3 +837,32 @@ func TestSwitchingProvidersMidEvaluationCausesNoImpactToEvaluation(t *testing.T)
 		t.Error(err)
 	}
 }
+
+func TestClientProviderLock(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer t.Cleanup(initSingleton)
+
+	mockMutex := NewMockmutex(ctrl)
+	mockProvider1 := NewMockFeatureProvider(ctrl)
+	mockProvider2 := NewMockFeatureProvider(ctrl)
+
+	mockProvider1.EXPECT().BooleanEvaluation(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
+	mockProvider1.EXPECT().Hooks().AnyTimes()
+	mockProvider1.EXPECT().Metadata().AnyTimes()
+
+	mockMutex.EXPECT().RLock().DoAndReturn(func() {
+		api.prvder = mockProvider1
+	})
+
+	// test that any provider change after RUnlock has no impact on this transaction
+	mockMutex.EXPECT().RUnlock().DoAndReturn(func() {
+		api.prvder = mockProvider2
+	})
+	api.mutex = mockMutex
+
+	client := NewClient("test").WithLogger(logr.New(logger{}))
+	_, err := client.BooleanValue(context.Background(), "foo", false, EvaluationContext{})
+	if err != nil {
+		t.Error(err)
+	}
+}

pkg/openfeature/mutex.go

@@ -0,0 +1,9 @@
+package openfeature
+
+// for the sake of testing
+type mutex interface {
+	Lock()
+	Unlock()
+	RLock()
+	RUnlock()
+}

pkg/openfeature/openfeature.go

@@ -11,7 +11,7 @@ type evaluationAPI struct {
 	hks     []Hook
 	evalCtx EvaluationContext
 	logger  logr.Logger
-	sync.RWMutex
+	mutex
 }
 
 // api is the global evaluationAPI.  This is a singleton and there can only be one instance.
@@ -28,6 +28,7 @@ func initSingleton() {
 		hks:     []Hook{},
 		evalCtx: EvaluationContext{},
 		logger:  logr.New(logger{}),
+		mutex:   &sync.RWMutex{},
 	}
 }