From f9684b858dfef40576e0031654b421a37e8bb1d6 Mon Sep 17 00:00:00 2001 From: Skye Gill Date: Fri, 6 Jan 2023 15:07:15 +0000 Subject: [PATCH] feat!: consolidated configuration change events into one event (#241) ## This PR Consolidates configuration change events into a singular event (avoid superfluous bulk emissions). Existing `configuration_change` event listeners will need to update their handling to consume the singular events. ### Related Issues Fixes #238 - [x] [Update go-sdk-contrib](https://github.com/open-feature/go-sdk-contrib/pull/66) - [ ] Update java-sdk-contrib ### Notes ### Follow-up Tasks ### How to test Signed-off-by: Skye Gill --- pkg/eval/ievaluator.go | 16 +-------- pkg/eval/json_evaluator.go | 2 +- pkg/eval/json_evaluator_model.go | 31 ++++++++--------- pkg/eval/json_evaluator_test.go | 58 ++++++++++++++++++++------------ pkg/eval/mock/ievaluator.go | 5 ++- pkg/runtime/runtime.go | 14 ++++---- 6 files changed, 61 insertions(+), 65 deletions(-) diff --git a/pkg/eval/ievaluator.go b/pkg/eval/ievaluator.go index fed40f687..af37e5102 100644 --- a/pkg/eval/ievaluator.go +++ b/pkg/eval/ievaluator.go @@ -12,19 +12,13 @@ const ( NotificationUpdate StateChangeNotificationType = "update" ) -type StateChangeNotification struct { - Type StateChangeNotificationType `json:"type"` - Source string `json:"source"` - FlagKey string `json:"flagKey"` -} - /* IEvaluator implementations store the state of the flags, do parsing and validation of the flag state and evaluate flags in response to handlers. */ type IEvaluator interface { GetState() (string, error) - SetState(source string, state string) ([]StateChangeNotification, error) + SetState(source string, state string) (map[string]interface{}, error) ResolveBooleanValue( reqID string, @@ -47,11 +41,3 @@ type IEvaluator interface { flagKey string, context *structpb.Struct) (value map[string]any, variant string, reasons string, err error) } - -func (s *StateChangeNotification) ToMap() map[string]interface{} { - return map[string]interface{}{ - "type": string(s.Type), - "source": s.Source, - "flagKey": s.FlagKey, - } -} diff --git a/pkg/eval/json_evaluator.go b/pkg/eval/json_evaluator.go index 8aeba0ea5..e6a1277d3 100644 --- a/pkg/eval/json_evaluator.go +++ b/pkg/eval/json_evaluator.go @@ -49,7 +49,7 @@ func (je *JSONEvaluator) GetState() (string, error) { return string(data), nil } -func (je *JSONEvaluator) SetState(source string, state string) ([]StateChangeNotification, error) { +func (je *JSONEvaluator) SetState(source string, state string) (map[string]interface{}, error) { schemaLoader := gojsonschema.NewStringLoader(schema.FlagdDefinitions) flagStringLoader := gojsonschema.NewStringLoader(state) result, err := gojsonschema.Validate(schemaLoader, flagStringLoader) diff --git a/pkg/eval/json_evaluator_model.go b/pkg/eval/json_evaluator_model.go index d85559674..97076ceba 100644 --- a/pkg/eval/json_evaluator_model.go +++ b/pkg/eval/json_evaluator_model.go @@ -16,18 +16,17 @@ type Evaluators struct { Evaluators map[string]json.RawMessage `json:"$evaluators"` } -func (f Flags) Merge(logger *logger.Logger, source string, ff Flags) (Flags, []StateChangeNotification) { - notifications := []StateChangeNotification{} +func (f Flags) Merge(logger *logger.Logger, source string, ff Flags) (Flags, map[string]interface{}) { + notifications := map[string]interface{}{} result := Flags{Flags: make(map[string]Flag)} for k, v := range f.Flags { if v.Source == source { if _, ok := ff.Flags[k]; !ok { // flag has been deleted - notifications = append(notifications, StateChangeNotification{ - Type: NotificationDelete, - Source: source, - FlagKey: k, - }) + notifications[k] = map[string]interface{}{ + "type": string(NotificationDelete), + "source": source, + } continue } } @@ -37,11 +36,10 @@ func (f Flags) Merge(logger *logger.Logger, source string, ff Flags) (Flags, []S v.Source = source val, ok := result.Flags[k] if !ok { - notifications = append(notifications, StateChangeNotification{ - Type: NotificationCreate, - Source: source, - FlagKey: k, - }) + notifications[k] = map[string]interface{}{ + "type": string(NotificationCreate), + "source": source, + } } else if !reflect.DeepEqual(val, v) { if val.Source != source { logger.Warn( @@ -53,11 +51,10 @@ func (f Flags) Merge(logger *logger.Logger, source string, ff Flags) (Flags, []S ), ) } - notifications = append(notifications, StateChangeNotification{ - Type: NotificationUpdate, - Source: source, - FlagKey: k, - }) + notifications[k] = map[string]interface{}{ + "type": string(NotificationUpdate), + "source": source, + } } result.Flags[k] = v } diff --git a/pkg/eval/json_evaluator_test.go b/pkg/eval/json_evaluator_test.go index 67263ee03..96a986338 100644 --- a/pkg/eval/json_evaluator_test.go +++ b/pkg/eval/json_evaluator_test.go @@ -739,35 +739,40 @@ func BenchmarkResolveObjectValue(b *testing.B) { func TestMergeFlags(t *testing.T) { t.Parallel() tests := []struct { - name string - current eval.Flags - new eval.Flags - newSource string - want eval.Flags + name string + current eval.Flags + new eval.Flags + newSource string + want eval.Flags + wantNotifs map[string]interface{} }{ { - name: "both nil", - current: eval.Flags{Flags: nil}, - new: eval.Flags{Flags: nil}, - want: eval.Flags{Flags: map[string]eval.Flag{}}, + name: "both nil", + current: eval.Flags{Flags: nil}, + new: eval.Flags{Flags: nil}, + want: eval.Flags{Flags: map[string]eval.Flag{}}, + wantNotifs: map[string]interface{}{}, }, { - name: "both empty flags", - current: eval.Flags{Flags: map[string]eval.Flag{}}, - new: eval.Flags{Flags: map[string]eval.Flag{}}, - want: eval.Flags{Flags: map[string]eval.Flag{}}, + name: "both empty flags", + current: eval.Flags{Flags: map[string]eval.Flag{}}, + new: eval.Flags{Flags: map[string]eval.Flag{}}, + want: eval.Flags{Flags: map[string]eval.Flag{}}, + wantNotifs: map[string]interface{}{}, }, { - name: "empty current", - current: eval.Flags{Flags: nil}, - new: eval.Flags{Flags: map[string]eval.Flag{}}, - want: eval.Flags{Flags: map[string]eval.Flag{}}, + name: "empty current", + current: eval.Flags{Flags: nil}, + new: eval.Flags{Flags: map[string]eval.Flag{}}, + want: eval.Flags{Flags: map[string]eval.Flag{}}, + wantNotifs: map[string]interface{}{}, }, { - name: "empty new", - current: eval.Flags{Flags: map[string]eval.Flag{}}, - new: eval.Flags{Flags: nil}, - want: eval.Flags{Flags: map[string]eval.Flag{}}, + name: "empty new", + current: eval.Flags{Flags: map[string]eval.Flag{}}, + new: eval.Flags{Flags: nil}, + want: eval.Flags{Flags: map[string]eval.Flag{}}, + wantNotifs: map[string]interface{}{}, }, { name: "extra fields on each", @@ -793,6 +798,9 @@ func TestMergeFlags(t *testing.T) { Source: "2", }, }}, + wantNotifs: map[string]interface{}{ + "paka": map[string]interface{}{"type": "write", "source": "2"}, + }, }, { name: "override", @@ -807,6 +815,10 @@ func TestMergeFlags(t *testing.T) { "waka": {DefaultVariant: "on"}, "paka": {DefaultVariant: "on"}, }}, + wantNotifs: map[string]interface{}{ + "waka": map[string]interface{}{"type": "update", "source": ""}, + "paka": map[string]interface{}{"type": "write", "source": ""}, + }, }, { name: "identical", @@ -819,6 +831,7 @@ func TestMergeFlags(t *testing.T) { want: eval.Flags{Flags: map[string]eval.Flag{ "hello": {DefaultVariant: "off"}, }}, + wantNotifs: map[string]interface{}{}, }, } @@ -826,8 +839,9 @@ func TestMergeFlags(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, _ := tt.current.Merge(logger.NewLogger(nil, false), tt.newSource, tt.new) + got, gotNotifs := tt.current.Merge(logger.NewLogger(nil, false), tt.newSource, tt.new) require.Equal(t, tt.want, got) + require.Equal(t, tt.wantNotifs, gotNotifs) }) } } diff --git a/pkg/eval/mock/ievaluator.go b/pkg/eval/mock/ievaluator.go index 6cf92db5c..5a31d97a3 100644 --- a/pkg/eval/mock/ievaluator.go +++ b/pkg/eval/mock/ievaluator.go @@ -8,7 +8,6 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" - eval "github.com/open-feature/flagd/pkg/eval" structpb "google.golang.org/protobuf/types/known/structpb" ) @@ -136,10 +135,10 @@ func (mr *MockIEvaluatorMockRecorder) ResolveStringValue(reqID, flagKey, context } // SetState mocks base method. -func (m *MockIEvaluator) SetState(source, state string) ([]eval.StateChangeNotification, error) { +func (m *MockIEvaluator) SetState(source, state string) (map[string]interface{}, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SetState", source, state) - ret0, _ := ret[0].([]eval.StateChangeNotification) + ret0, _ := ret[0].(map[string]interface{}) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index dae9d5c1f..81def2c68 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -80,12 +80,12 @@ func (r *Runtime) updateState(ctx context.Context, syncr sync.ISync) error { if err != nil { return fmt.Errorf("set state: %w", err) } - for _, n := range notifications { - r.Logger.Info(fmt.Sprintf("configuration change (%s) for flagKey %s (%s)", n.Type, n.FlagKey, n.Source)) - r.Service.Notify(service.Notification{ - Type: service.ConfigurationChange, - Data: n.ToMap(), - }) - } + + r.Service.Notify(service.Notification{ + Type: service.ConfigurationChange, + Data: map[string]interface{}{ + "flags": notifications, + }, + }) return nil }