fix: consider default client when loading states (#309)

fix: consider default client when loading states

Signed-off-by: warber <[email protected]>

Author: warber

openfeature/client.go

@@ -713,16 +713,19 @@ func (c *Client) evaluate(
 		c.finallyHooks(ctx, hookCtx, providerInvocationClientApiHooks, options)
 	}()
 
-	// short circuit if provider is in NOT READY state
-	if c.State() == NotReadyState {
-		c.errorHooks(ctx, hookCtx, providerInvocationClientApiHooks, ProviderNotReadyError, options)
-		return evalDetails, ProviderNotReadyError
-	}
+        // bypass short-circuit logic for the Noop provider; it is essentially stateless and a "special case"
+	if _, ok := provider.(NoopProvider); !ok {
+		// short circuit if provider is in NOT READY state
+		if c.State() == NotReadyState {
+			c.errorHooks(ctx, hookCtx, providerInvocationClientApiHooks, ProviderNotReadyError, options)
+			return evalDetails, ProviderNotReadyError
+		}
 
-	// short circuit if provider is in FATAL state
-	if c.State() == FatalState {
-		c.errorHooks(ctx, hookCtx, providerInvocationClientApiHooks, ProviderFatalError, options)
-		return evalDetails, ProviderFatalError
+		// short circuit if provider is in FATAL state
+		if c.State() == FatalState {
+			c.errorHooks(ctx, hookCtx, providerInvocationClientApiHooks, ProviderFatalError, options)
+			return evalDetails, ProviderFatalError
+		}
 	}
 
 	evalCtx, err = c.beforeHooks(ctx, hookCtx, apiClientInvocationProviderHooks, evalCtx, options)

openfeature/client_test.go

@@ -3,6 +3,7 @@ package openfeature
 import (
 	"context"
 	"errors"
+	"math"
 	"reflect"
 	"testing"
 	"time"
@@ -1385,7 +1386,24 @@ func TestRequirement_1_7_6(t *testing.T) {
 	mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), ProviderNotReadyError, gomock.Any())
 	mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
 
-	client := GetApiInstance().GetNamedClient(t.Name())
+	notReadyEventingProvider := struct {
+		FeatureProvider
+		StateHandler
+		EventHandler
+	}{
+		NoopProvider{},
+		&stateHandlerForTests{
+			initF: func(e EvaluationContext) error {
+				<-time.After(math.MaxInt)
+				return nil
+			},
+		},
+		&ProviderEventing{},
+	}
+
+	_ = GetApiInstance().SetProvider(notReadyEventingProvider)
+
+	client := GetApiInstance().GetNamedClient("somOtherClient")
 	client.AddHooks(mockHook)
 
 	if client.State() != NotReadyState {
@@ -1401,7 +1419,6 @@ func TestRequirement_1_7_6(t *testing.T) {
 	if res != defaultVal {
 		t.Fatalf("expected resolved boolean value to default to %t, got %t", defaultVal, res)
 	}
-
 }
 
 // The client MUST default, run error hooks, and indicate an error if flag resolution is attempted while the provider
@@ -1422,7 +1439,10 @@ func TestRequirement_1_7_7(t *testing.T) {
 		&ProviderEventing{},
 	}
 
-	_ = SetNamedProviderAndWait(t.Name(), provider)
+	err := SetNamedProviderAndWait(t.Name(), provider)
+	if err == nil {
+		t.Errorf("provider registration was expected to fail but succeeded unexpectedly")
+	}
 
 	ctrl := gomock.NewController(t)
 	mockHook := NewMockHook(ctrl)

openfeature/event_executor.go

@@ -27,7 +27,7 @@ type clientEvent interface {
 	State(domain string) State
 }
 
-const defaultClient = ""
+const defaultDomain = ""
 
 // event executor is a registry to connect API and Client event handlers to Providers
 
@@ -102,7 +102,7 @@ func (e *eventExecutor) AddHandler(t EventType, c EventCallback) {
 		e.apiRegistry[t] = append(e.apiRegistry[t], c)
 	}
 
-	e.emitOnRegistration(defaultClient, e.defaultProviderReference, t, c)
+	e.emitOnRegistration(defaultDomain, e.defaultProviderReference, t, c)
 }
 
 // RemoveHandler removes an API(global) level handler
@@ -189,7 +189,7 @@ func (e *eventExecutor) GetClientRegistry(client string) scopedCallback {
 // emitOnRegistration fulfils the spec requirement to fire events if the
 // event type and the state of the associated provider are compatible.
 func (e *eventExecutor) emitOnRegistration(domain string, providerReference providerReference, eventType EventType, callback EventCallback) {
-	state, ok := e.states.Load(domain)
+	state, ok := e.loadState(domain)
 	if !ok {
 		return
 	}
@@ -213,12 +213,19 @@ func (e *eventExecutor) emitOnRegistration(domain string, providerReference prov
 	}
 }
 
-func (e *eventExecutor) State(domain string) State {
-	s, ok := e.states.Load(domain)
+func (e *eventExecutor) loadState(domain string) (State, bool) {
+	state, ok := e.states.Load(domain)
 	if !ok {
-		return NotReadyState
+		if state, ok = e.states.Load(defaultDomain); !ok {
+			return NotReadyState, false
+		}
 	}
-	return s.(State)
+	return state.(State), true
+}
+
+func (e *eventExecutor) State(domain string) State {
+	state, _ := e.loadState(domain)
+	return state
 }
 
 // registerDefaultProvider registers the default FeatureProvider and remove the old default provider if available
@@ -357,7 +364,7 @@ func (e *eventExecutor) triggerEvent(event Event, handler FeatureProvider) {
 	}
 
 	// handling the default provider
-	e.states.Store(defaultClient, stateFromEvent(event))
+	e.states.Store(defaultDomain, stateFromEvent(event))
 	// invoke default provider bound (no provider associated) handlers by filtering
 	for domain, registry := range e.scopedRegistry {
 		if _, ok := e.namedProviderReference[domain]; ok {

openfeature/event_executor_test.go

@@ -638,7 +638,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 		defer t.Cleanup(initSingleton)
 
 		eventingImpl := &ProviderEventing{
-			c: make(chan Event, 1),
+			c: make(chan Event),
 		}
 
 		readyEventingProvider := struct {
@@ -673,7 +673,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 		defer t.Cleanup(initSingleton)
 
 		eventingImpl := &ProviderEventing{
-			c: make(chan Event, 1),
+			c: make(chan Event),
 		}
 
 		readyEventingProvider := struct {
@@ -685,7 +685,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 		}
 
 		clientAssociation := "clientA"
-		err := SetNamedProvider(clientAssociation, readyEventingProvider)
+		err := SetNamedProviderAndWait(clientAssociation, readyEventingProvider)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -695,7 +695,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 			rsp <- e
 		}
 
-		client := NewClient(clientAssociation)
+		client := api.GetNamedClient(clientAssociation)
 		client.AddHandler(ProviderReady, &callback)
 
 		select {
@@ -710,9 +710,8 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 		defer t.Cleanup(initSingleton)
 
 		eventingImpl := &ProviderEventing{
-			c: make(chan Event, 1),
+			c: make(chan Event),
 		}
-		eventingImpl.Invoke(Event{EventType: ProviderConfigChange})
 
 		readyEventingProvider := struct {
 			FeatureProvider
@@ -722,7 +721,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 			eventingImpl,
 		}
 
-		err := SetProvider(readyEventingProvider)
+		err := SetProviderAndWait(readyEventingProvider)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -732,7 +731,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 			rsp <- e
 		}
 
-		client := NewClient("someClient")
+		client := api.GetNamedClient("someClient")
 		client.AddHandler(ProviderReady, &callback)
 
 		select {
@@ -770,7 +769,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
 			rsp <- e
 		}
 
-		client := NewClient("someClient")
+		client := api.GetNamedClient("someClient")
 		client.AddHandler(ProviderReady, &callback)
 
 		select {
@@ -1357,7 +1356,6 @@ func TestEventHandler_1ToNMapping(t *testing.T) {
 }
 
 // Contract tests - registration & removal
-
 func TestEventHandler_Registration(t *testing.T) {
 	t.Run("API handlers", func(t *testing.T) {
 		executor := newEventExecutor()

openfeature/openfeature_test.go

@@ -1,7 +1,9 @@
 package openfeature
 
 import (
+	"context"
 	"errors"
+	"fmt"
 	"reflect"
 	"testing"
 	"time"
@@ -718,6 +720,47 @@ func TestDefaultClientUsage(t *testing.T) {
 	}
 }
 
+func TestLateBindingOfDefaultProvider(t *testing.T) {
+	// we are expecting
+	expectedResultUnboundProvider := "default-value-from-unbound-provider"
+	expectedResultFromLateDefaultProvider := "value-from-late-default-provider"
+
+	ctrl := gomock.NewController(t)
+	defaultProvider := NewMockFeatureProvider(ctrl)
+	defaultProvider.EXPECT().Metadata().Return(Metadata{Name: "defaultClientReplacement"}).AnyTimes()
+	defaultProvider.EXPECT().Hooks().AnyTimes().Return([]Hook{})
+	defaultProvider.EXPECT().StringEvaluation(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(StringResolutionDetail{Value: expectedResultFromLateDefaultProvider})
+
+	client := NewClient("app")
+	strResult, err := client.StringValue(context.TODO(), "flag", expectedResultUnboundProvider, EvaluationContext{})
+	if err != nil {
+		if err != nil {
+			t.Errorf("flag evaluation failed %v", err)
+		}
+	}
+
+	if strResult != expectedResultUnboundProvider {
+		t.Errorf("expected %s, but got %s", expectedResultUnboundProvider, strResult)
+	}
+
+	err = SetProviderAndWait(defaultProvider)
+	if err != nil {
+		t.Errorf("provider registration failed %v", err)
+	}
+
+	strResult, err = client.StringValue(context.TODO(), "flag", "default", EvaluationContext{})
+	if err != nil {
+		if err != nil {
+			t.Errorf("flag evaluation failed %v", err)
+		}
+	}
+
+	if strResult != expectedResultFromLateDefaultProvider {
+		t.Errorf("expected %s, but got %s", expectedResultFromLateDefaultProvider, strResult)
+	}
+	fmt.Println(strResult)
+}
+
 // Nil providers are not accepted for default and named providers
 func TestForNilProviders(t *testing.T) {
 	defer t.Cleanup(initSingleton)