Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: consider default client when loading states #309

Merged
merged 5 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions openfeature/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,16 +713,18 @@ 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
}
if _, ok := provider.(NoopProvider); !ok {
warber marked this conversation as resolved.
Show resolved Hide resolved
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
// 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)
Expand Down
26 changes: 23 additions & 3 deletions openfeature/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package openfeature
import (
"context"
"errors"
"math"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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")
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
}

ctrl := gomock.NewController(t)
mockHook := NewMockHook(ctrl)
Expand Down
23 changes: 15 additions & 8 deletions openfeature/event_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 8 additions & 10 deletions openfeature/event_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
Expand All @@ -695,7 +695,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
rsp <- e
}

client := NewClient(clientAssociation)
client := api.GetNamedClient(clientAssociation)
client.AddHandler(ProviderReady, &callback)

select {
Expand All @@ -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
Expand All @@ -722,7 +721,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
eventingImpl,
}

err := SetProvider(readyEventingProvider)
err := SetProviderAndWait(readyEventingProvider)
if err != nil {
t.Fatal(err)
}
Expand All @@ -732,7 +731,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
rsp <- e
}

client := NewClient("someClient")
client := api.GetNamedClient("someClient")
client.AddHandler(ProviderReady, &callback)

select {
Expand Down Expand Up @@ -770,7 +769,7 @@ func TestEventHandler_ProviderReadiness(t *testing.T) {
rsp <- e
}

client := NewClient("someClient")
client := api.GetNamedClient("someClient")
client.AddHandler(ProviderReady, &callback)

select {
Expand Down Expand Up @@ -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()
Expand Down
43 changes: 43 additions & 0 deletions openfeature/openfeature_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package openfeature

import (
"context"
"errors"
"fmt"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -718,6 +720,47 @@ func TestDefaultClientUsage(t *testing.T) {
}
}

func TestLateBindingOfDefaultProvider(t *testing.T) {
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
// 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)
Expand Down
Loading