From ab7150139f1d8734b4e34ea611d666821c4a6a5f Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Mon, 8 Jul 2024 12:10:35 -0400 Subject: [PATCH 01/13] Add event, emit event, rename a field --- internal/dev_server/api/server.go | 8 ++++---- internal/dev_server/db/sqlite.go | 6 +++--- internal/dev_server/model/events.go | 14 ++++++++++++++ internal/dev_server/model/override.go | 13 +++---------- internal/dev_server/model/override_test.go | 8 ++++---- internal/dev_server/model/project.go | 19 +++++++++++++------ internal/dev_server/model/project_test.go | 19 +++++++++++++++---- 7 files changed, 56 insertions(+), 31 deletions(-) create mode 100644 internal/dev_server/model/events.go diff --git a/internal/dev_server/api/server.go b/internal/dev_server/api/server.go index cdaf80d9..0047b05e 100644 --- a/internal/dev_server/api/server.go +++ b/internal/dev_server/api/server.go @@ -56,7 +56,7 @@ func (s Server) GetDevProjectsProjectKey(ctx context.Context, request GetDevProj LastSyncedFromSource: project.LastSyncTime.Unix(), Context: project.Context, SourceEnvironmentKey: project.SourceEnvironmentKey, - FlagsState: &project.FlagState, + FlagsState: &project.AllFlagsState, } if request.Params.Expand != nil { @@ -98,7 +98,7 @@ func (s Server) PostDevProjectsProjectKey(ctx context.Context, request PostDevPr LastSyncedFromSource: project.LastSyncTime.Unix(), Context: project.Context, SourceEnvironmentKey: project.SourceEnvironmentKey, - FlagsState: &project.FlagState, + FlagsState: &project.AllFlagsState, } if request.Params.Expand != nil { @@ -143,7 +143,7 @@ func (s Server) PatchDevProjectsProjectKey(ctx context.Context, request PatchDev LastSyncedFromSource: project.LastSyncTime.Unix(), Context: project.Context, SourceEnvironmentKey: project.SourceEnvironmentKey, - FlagsState: &project.FlagState, + FlagsState: &project.AllFlagsState, } if request.Params.Expand != nil { @@ -188,7 +188,7 @@ func (s Server) PatchDevProjectsProjectKeySync(ctx context.Context, request Patc LastSyncedFromSource: project.LastSyncTime.Unix(), Context: project.Context, SourceEnvironmentKey: project.SourceEnvironmentKey, - FlagsState: &project.FlagState, + FlagsState: &project.AllFlagsState, } if request.Params.Expand != nil { diff --git a/internal/dev_server/db/sqlite.go b/internal/dev_server/db/sqlite.go index 22063ac6..e833a125 100644 --- a/internal/dev_server/db/sqlite.go +++ b/internal/dev_server/db/sqlite.go @@ -57,7 +57,7 @@ func (s Sqlite) GetDevProject(ctx context.Context, key string) (*model.Project, } // Parse the flag state JSON string - if err := json.Unmarshal([]byte(flagStateData), &project.FlagState); err != nil { + if err := json.Unmarshal([]byte(flagStateData), &project.AllFlagsState); err != nil { return nil, errors.Wrap(err, "unable to unmarshal flag state data") } @@ -65,7 +65,7 @@ func (s Sqlite) GetDevProject(ctx context.Context, key string) (*model.Project, } func (s Sqlite) UpdateProject(ctx context.Context, project model.Project) (bool, error) { - flagsStateJson, err := json.Marshal(project.FlagState) + flagsStateJson, err := json.Marshal(project.AllFlagsState) if err != nil { return false, errors.Wrap(err, "unable to marshal flags state when updating project") } @@ -107,7 +107,7 @@ func (s Sqlite) DeleteDevProject(ctx context.Context, key string) (bool, error) } func (s Sqlite) InsertProject(ctx context.Context, project model.Project) error { - flagsStateJson, err := json.Marshal(project.FlagState) + flagsStateJson, err := json.Marshal(project.AllFlagsState) if err != nil { return errors.Wrap(err, "unable to marshal flags state when writing project") } diff --git a/internal/dev_server/model/events.go b/internal/dev_server/model/events.go new file mode 100644 index 00000000..1701322a --- /dev/null +++ b/internal/dev_server/model/events.go @@ -0,0 +1,14 @@ +package model + +// Event for individual flag overrides +type UpsertOverrideEvent struct { + FlagKey string + ProjectKey string + FlagState FlagState +} + +// Event for full project sync +type SyncEvent struct { + ProjectKey string + AllFlagsState FlagsState +} diff --git a/internal/dev_server/model/override.go b/internal/dev_server/model/override.go index a86f6513..f2eb4918 100644 --- a/internal/dev_server/model/override.go +++ b/internal/dev_server/model/override.go @@ -14,12 +14,6 @@ type Override struct { Version int } -type UpsertOverrideEvent struct { - FlagKey string - ProjectKey string - FlagState FlagState -} - func UpsertOverride(ctx context.Context, projectKey, flagKey string, value ldvalue.Value) (Override, error) { // TODO: validate if the flag type matches @@ -31,7 +25,7 @@ func UpsertOverride(ctx context.Context, projectKey, flagKey string, value ldval } var flagExists bool - for flag := range project.FlagState { + for flag := range project.AllFlagsState { if flagKey == flag { flagExists = true break @@ -54,9 +48,8 @@ func UpsertOverride(ctx context.Context, projectKey, flagKey string, value ldval return Override{}, err } - observers := GetObserversFromContext(ctx) - flagState := override.Apply(project.FlagState[flagKey]) - observers.Notify(UpsertOverrideEvent{ + flagState := override.Apply(project.AllFlagsState[flagKey]) + GetObserversFromContext(ctx).Notify(UpsertOverrideEvent{ FlagKey: flagKey, ProjectKey: projectKey, FlagState: flagState, diff --git a/internal/dev_server/model/override_test.go b/internal/dev_server/model/override_test.go index 0ef4c79b..6b47b563 100644 --- a/internal/dev_server/model/override_test.go +++ b/internal/dev_server/model/override_test.go @@ -28,8 +28,8 @@ func TestUpsertOverride(t *testing.T) { } project := &model.Project{ - Key: projKey, - FlagState: model.FlagsState{flagKey: model.FlagState{Value: ldvalue.Bool(false), Version: 1}}, + Key: projKey, + AllFlagsState: model.FlagsState{flagKey: model.FlagState{Value: ldvalue.Bool(false), Version: 1}}, } ctx = model.ContextWithStore(ctx, store) @@ -50,8 +50,8 @@ func TestUpsertOverride(t *testing.T) { t.Run("Returns error if flag does not exist in project", func(t *testing.T) { badProj := model.Project{ - Key: projKey, - FlagState: model.FlagsState{}, + Key: projKey, + AllFlagsState: model.FlagsState{}, } store.EXPECT().GetDevProject(gomock.Any(), projKey).Return(&badProj, nil) diff --git a/internal/dev_server/model/project.go b/internal/dev_server/model/project.go index 5cc0051c..012ffb87 100644 --- a/internal/dev_server/model/project.go +++ b/internal/dev_server/model/project.go @@ -14,7 +14,7 @@ type Project struct { SourceEnvironmentKey string Context ldcontext.Context LastSyncTime time.Time - FlagState FlagsState + AllFlagsState FlagsState } // CreateProject creates a project and adds it to the database. @@ -35,7 +35,7 @@ func CreateProject(ctx context.Context, projectKey, sourceEnvironmentKey string, return Project{}, err } - project.FlagState = flagsState + project.AllFlagsState = flagsState project.LastSyncTime = time.Now() store := StoreFromContext(ctx) @@ -65,7 +65,7 @@ func UpdateProject(ctx context.Context, projectKey string, context *ldcontext.Co if err != nil { return Project{}, err } - project.FlagState = flagsState + project.AllFlagsState = flagsState project.LastSyncTime = time.Now() } @@ -76,6 +76,7 @@ func UpdateProject(ctx context.Context, projectKey string, context *ldcontext.Co if !updated { return Project{}, errors.New("Project not updated") } + return *project, nil } @@ -90,8 +91,9 @@ func SyncProject(ctx context.Context, projectKey string) (Project, error) { return Project{}, err } - project.FlagState = flagsState + project.AllFlagsState = flagsState project.LastSyncTime = time.Now() + updated, err := store.UpdateProject(ctx, *project) if err != nil { return Project{}, err @@ -99,6 +101,11 @@ func SyncProject(ctx context.Context, projectKey string) (Project, error) { if !updated { return Project{}, errors.New("Project not updated") } + + GetObserversFromContext(ctx).Notify(SyncEvent{ + ProjectKey: project.Key, + AllFlagsState: project.AllFlagsState, + }) return *project, nil } @@ -108,8 +115,8 @@ func (p Project) GetFlagStateWithOverridesForProject(ctx context.Context) (Flags if err != nil { return FlagsState{}, errors.Wrapf(err, "unable to fetch overrides for project %s", p.Key) } - withOverrides := make(FlagsState, len(p.FlagState)) - for flagKey, flagState := range p.FlagState { + withOverrides := make(FlagsState, len(p.AllFlagsState)) + for flagKey, flagState := range p.AllFlagsState { if override, ok := overrides.GetFlag(flagKey); ok { flagState = override.Apply(flagState) } diff --git a/internal/dev_server/model/project_test.go b/internal/dev_server/model/project_test.go index 48d33b27..2afff534 100644 --- a/internal/dev_server/model/project_test.go +++ b/internal/dev_server/model/project_test.go @@ -58,13 +58,13 @@ func TestCreateProject(t *testing.T) { Key: projKey, SourceEnvironmentKey: sourceEnvKey, Context: ldcontext.NewBuilder("user").Key("dev-environment").Build(), - FlagState: model.FromAllFlags(allFlags), + AllFlagsState: model.FromAllFlags(allFlags), } assert.Equal(t, expectedProj.Key, p.Key) assert.Equal(t, expectedProj.SourceEnvironmentKey, p.SourceEnvironmentKey) assert.Equal(t, expectedProj.Context, p.Context) - assert.Equal(t, expectedProj.FlagState, p.FlagState) + assert.Equal(t, expectedProj.AllFlagsState, p.AllFlagsState) }) } @@ -134,6 +134,11 @@ func TestSyncProject(t *testing.T) { ctx := model.ContextWithStore(context.Background(), store) ctx, api, sdk := adapters_mocks.WithMockApiAndSdk(ctx, mockController) + observer := mocks.NewMockObserver(mockController) + observers := model.NewObservers() + observers.RegisterObserver(observer) + ctx = model.SetObserversOnContext(ctx, observers) + proj := model.Project{ Key: "projKey", SourceEnvironmentKey: "srcEnvKey", @@ -183,6 +188,12 @@ func TestSyncProject(t *testing.T) { api.EXPECT().GetSdkKey(gomock.Any(), proj.Key, proj.SourceEnvironmentKey).Return("sdkKey", nil) sdk.EXPECT().GetAllFlagsState(gomock.Any(), gomock.Any(), "sdkKey").Return(flagstate.AllFlags{}, nil) store.EXPECT().UpdateProject(gomock.Any(), gomock.Any()).Return(true, nil) + observer. + EXPECT(). + Handle(model.SyncEvent{ + ProjectKey: proj.Key, + AllFlagsState: model.FlagsState{}, + }) project, err := model.SyncProject(ctx, proj.Key) assert.Nil(t, err) @@ -196,8 +207,8 @@ func TestGetFlagStateWithOverridesForProject(t *testing.T) { ctx := model.ContextWithStore(context.Background(), store) flagKey := "flg" proj := model.Project{ - Key: "projKey", - FlagState: model.FlagsState{flagKey: model.FlagState{Value: ldvalue.Bool(false), Version: 1}}, + Key: "projKey", + AllFlagsState: model.FlagsState{flagKey: model.FlagState{Value: ldvalue.Bool(false), Version: 1}}, } t.Run("Returns error if store fetch fails", func(t *testing.T) { From de60967dd1b3edb6d74e7c6912b7b63d975f5188 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Mon, 8 Jul 2024 12:28:13 -0400 Subject: [PATCH 02/13] server side update --- internal/dev_server/sdk/stream_server_flags.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/dev_server/sdk/stream_server_flags.go b/internal/dev_server/sdk/stream_server_flags.go index 45b41613..d2a1fa28 100644 --- a/internal/dev_server/sdk/stream_server_flags.go +++ b/internal/dev_server/sdk/stream_server_flags.go @@ -54,6 +54,7 @@ func (c serverFlagsObserver) Handle(event interface{}) { if event.ProjectKey != c.projectKey { return } + data, err := json.Marshal(serverSidePatchData{ Path: fmt.Sprintf("/flags/%s", event.FlagKey), Data: serverFlagFromFlagState(event.FlagKey, event.FlagState), @@ -61,10 +62,25 @@ func (c serverFlagsObserver) Handle(event interface{}) { if err != nil { panic(errors.Wrap(err, "failed to marshal flag state in observer")) } + c.updateChan <- Message{ Event: "patch", Data: data, } + case model.SyncEvent: + if event.ProjectKey != c.projectKey { + return + } + + data, err := json.Marshal(ServerAllPayloadFromFlagsState(event.AllFlagsState)) + if err != nil { + panic(errors.Wrap(err, "failed to marshal flag state in observer")) + } + + c.updateChan <- Message{ + Event: "put", + Data: data, + } } } From e1724a03dec6ad5f95504e685cbdc541a3a6ced1 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Mon, 8 Jul 2024 14:03:11 -0400 Subject: [PATCH 03/13] client side event --- .../dev_server/sdk/stream_client_flags.go | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/internal/dev_server/sdk/stream_client_flags.go b/internal/dev_server/sdk/stream_client_flags.go index e5fd953c..64097707 100644 --- a/internal/dev_server/sdk/stream_client_flags.go +++ b/internal/dev_server/sdk/stream_client_flags.go @@ -50,7 +50,7 @@ func (c clientFlagsObserver) Handle(event interface{}) { log.Printf("clientFlagsObserver: handling flag state event: %v", event) switch event := event.(type) { case model.UpsertOverrideEvent: - data, err := json.Marshal(clientSidePatchData{ + data, err := json.Marshal(clientFlag{ Key: event.FlagKey, Version: event.FlagState.Version, Value: event.FlagState.Value, @@ -58,15 +58,36 @@ func (c clientFlagsObserver) Handle(event interface{}) { if err != nil { panic(errors.Wrap(err, "failed to marshal flag state in observer")) } + c.updateChan <- Message{ Event: "patch", Data: data, } + case model.SyncEvent: + clientFlags := clientFlags{} + for flagKey, flagState := range event.AllFlagsState { + clientFlags[flagKey] = clientFlag{ + Version: flagState.Version, + Value: flagState.Value, + } + } + + data, err := json.Marshal(clientFlags) + if err != nil { + panic(errors.Wrap(err, "failed to marshal flag state in observer")) + } + + c.updateChan <- Message{ + Event: "put", + Data: data, + } } } -type clientSidePatchData struct { - Key string `json:"key"` +type clientFlag struct { + Key string `json:"key,omitempty"` Version int `json:"version"` Value ldvalue.Value `json:"value"` } + +type clientFlags map[string]clientFlag From a343185972282af1f382a23885bb852b74ae6412 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Mon, 8 Jul 2024 14:14:55 -0400 Subject: [PATCH 04/13] Code consolidation and cleanup --- .../dev_server/sdk/stream_client_flags.go | 14 ++-------- .../dev_server/sdk/stream_server_flags.go | 14 ++-------- internal/dev_server/sdk/streaming.go | 28 ++++++++++++++++++- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/internal/dev_server/sdk/stream_client_flags.go b/internal/dev_server/sdk/stream_client_flags.go index 64097707..1ad7049e 100644 --- a/internal/dev_server/sdk/stream_client_flags.go +++ b/internal/dev_server/sdk/stream_client_flags.go @@ -50,7 +50,7 @@ func (c clientFlagsObserver) Handle(event interface{}) { log.Printf("clientFlagsObserver: handling flag state event: %v", event) switch event := event.(type) { case model.UpsertOverrideEvent: - data, err := json.Marshal(clientFlag{ + err := SendMessage(c.updateChan, TYPE_PATCH, clientFlag{ Key: event.FlagKey, Version: event.FlagState.Version, Value: event.FlagState.Value, @@ -58,11 +58,6 @@ func (c clientFlagsObserver) Handle(event interface{}) { if err != nil { panic(errors.Wrap(err, "failed to marshal flag state in observer")) } - - c.updateChan <- Message{ - Event: "patch", - Data: data, - } case model.SyncEvent: clientFlags := clientFlags{} for flagKey, flagState := range event.AllFlagsState { @@ -72,15 +67,10 @@ func (c clientFlagsObserver) Handle(event interface{}) { } } - data, err := json.Marshal(clientFlags) + err := SendMessage(c.updateChan, TYPE_PUT, clientFlags) if err != nil { panic(errors.Wrap(err, "failed to marshal flag state in observer")) } - - c.updateChan <- Message{ - Event: "put", - Data: data, - } } } diff --git a/internal/dev_server/sdk/stream_server_flags.go b/internal/dev_server/sdk/stream_server_flags.go index d2a1fa28..8863a2a1 100644 --- a/internal/dev_server/sdk/stream_server_flags.go +++ b/internal/dev_server/sdk/stream_server_flags.go @@ -55,32 +55,22 @@ func (c serverFlagsObserver) Handle(event interface{}) { return } - data, err := json.Marshal(serverSidePatchData{ + err := SendMessage(c.updateChan, TYPE_PATCH, serverSidePatchData{ Path: fmt.Sprintf("/flags/%s", event.FlagKey), Data: serverFlagFromFlagState(event.FlagKey, event.FlagState), }) if err != nil { panic(errors.Wrap(err, "failed to marshal flag state in observer")) } - - c.updateChan <- Message{ - Event: "patch", - Data: data, - } case model.SyncEvent: if event.ProjectKey != c.projectKey { return } - data, err := json.Marshal(ServerAllPayloadFromFlagsState(event.AllFlagsState)) + err := SendMessage(c.updateChan, TYPE_PUT, ServerAllPayloadFromFlagsState(event.AllFlagsState)) if err != nil { panic(errors.Wrap(err, "failed to marshal flag state in observer")) } - - c.updateChan <- Message{ - Event: "put", - Data: data, - } } } diff --git a/internal/dev_server/sdk/streaming.go b/internal/dev_server/sdk/streaming.go index 90dba3e5..04f650c8 100644 --- a/internal/dev_server/sdk/streaming.go +++ b/internal/dev_server/sdk/streaming.go @@ -1,6 +1,7 @@ package sdk import ( + "encoding/json" "fmt" "net/http" "time" @@ -8,8 +9,15 @@ import ( "github.com/pkg/errors" ) +type MessageType string + +const ( + TYPE_PUT MessageType = "put" + TYPE_PATCH MessageType = "patch" +) + type Message struct { - Event string + Event MessageType Data []byte } @@ -67,3 +75,21 @@ func OpenStream(w http.ResponseWriter, done <-chan struct{}, initialMessage Mess }() return updateChan, errChan } + +func SendMessage( + updateChan chan<- Message, + msgType MessageType, + data interface{}, +) error { + payload, err := json.Marshal(data) + if err != nil { + return err + } + + updateChan <- Message{ + Event: msgType, + Data: payload, + } + + return nil +} From e31492c2a9dc9f6c077d0478ad5b4923051f5f72 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Mon, 8 Jul 2024 17:53:32 -0400 Subject: [PATCH 05/13] little more cleanup --- internal/dev_server/sdk/stream_client_flags.go | 6 +++++- internal/dev_server/sdk/stream_server_flags.go | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/dev_server/sdk/stream_client_flags.go b/internal/dev_server/sdk/stream_client_flags.go index 1ad7049e..dd3cd410 100644 --- a/internal/dev_server/sdk/stream_client_flags.go +++ b/internal/dev_server/sdk/stream_client_flags.go @@ -22,7 +22,11 @@ func StreamClientFlags(w http.ResponseWriter, r *http.Request) { WriteError(ctx, w, errors.Wrap(err, "failed to marshal flag state")) return } - updateChan, doneChan := OpenStream(w, r.Context().Done(), Message{"put", jsonBody}) // TODO Wireup updateChan + updateChan, doneChan := OpenStream( + w, + r.Context().Done(), + Message{Event: TYPE_PUT, Data: jsonBody}, + ) // TODO Wireup updateChan defer close(updateChan) projectKey := GetProjectKeyFromContext(ctx) observer := clientFlagsObserver{updateChan, projectKey} diff --git a/internal/dev_server/sdk/stream_server_flags.go b/internal/dev_server/sdk/stream_server_flags.go index 8863a2a1..ebb2926e 100644 --- a/internal/dev_server/sdk/stream_server_flags.go +++ b/internal/dev_server/sdk/stream_server_flags.go @@ -24,7 +24,11 @@ func StreamServerAllPayload(w http.ResponseWriter, r *http.Request) { WriteError(ctx, w, errors.Wrap(err, "failed to marshal flag state")) return } - updateChan, doneChan := OpenStream(w, r.Context().Done(), Message{"put", jsonBody}) + updateChan, doneChan := OpenStream( + w, + r.Context().Done(), + Message{Event: TYPE_PUT, Data: jsonBody}, + ) defer close(updateChan) observer := serverFlagsObserver{updateChan, projectKey} observers := model.GetObserversFromContext(ctx) From 8138abc8fe27d1806576901aaf8a0f4439e04001 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Mon, 8 Jul 2024 17:53:45 -0400 Subject: [PATCH 06/13] half an attempt at an integration test --- internal/dev_server/sdk/go_sdk_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/dev_server/sdk/go_sdk_test.go b/internal/dev_server/sdk/go_sdk_test.go index ac4e5332..dd162d83 100644 --- a/internal/dev_server/sdk/go_sdk_test.go +++ b/internal/dev_server/sdk/go_sdk_test.go @@ -12,6 +12,7 @@ import ( "github.com/launchdarkly/go-sdk-common/v3/ldcontext" "github.com/launchdarkly/go-sdk-common/v3/ldvalue" ldclient "github.com/launchdarkly/go-server-sdk/v7" + "github.com/launchdarkly/go-server-sdk/v7/interfaces" "github.com/launchdarkly/go-server-sdk/v7/interfaces/flagstate" "github.com/launchdarkly/ldcli/internal/dev_server/adapters/mocks" "github.com/launchdarkly/ldcli/internal/dev_server/db" @@ -123,4 +124,25 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { assert.Equal(t, value.AsArbitraryValue(), flagUpdate.NewValue.AsArbitraryValue()) }) } + + t.Run("Sync sends full flag payload for project", func(t *testing.T) { + trackers := make(map[string]<-chan interfaces.FlagValueChangeEvent, len(updates)) + + for flagKey := range updates { + flagUpdateChan := ld.GetFlagTracker().AddFlagValueChangeListener(flagKey, ldContext, ldvalue.String("uh-oh")) + defer ld.GetFlagTracker().RemoveFlagValueChangeListener(flagUpdateChan) + trackers[flagKey] = flagUpdateChan + } + + _, err := model.SyncProject(ctx, projectKey) + require.NoError(t, err) + + for flagKey, value := range updates { + updateTracker, ok := trackers[flagKey] + require.True(t, ok) + + update := <-updateTracker + assert.Equal(t, value.AsArbitraryValue(), update.NewValue.AsArbitraryValue()) + } + }) } From fa1f288acc0fb0948e364b13ad6406c5bdaedb44 Mon Sep 17 00:00:00 2001 From: Mike Zorn Date: Mon, 8 Jul 2024 16:57:55 -0700 Subject: [PATCH 07/13] correct log text --- internal/dev_server/sdk/stream_server_flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/dev_server/sdk/stream_server_flags.go b/internal/dev_server/sdk/stream_server_flags.go index ebb2926e..1d3c0a4f 100644 --- a/internal/dev_server/sdk/stream_server_flags.go +++ b/internal/dev_server/sdk/stream_server_flags.go @@ -52,7 +52,7 @@ type serverFlagsObserver struct { } func (c serverFlagsObserver) Handle(event interface{}) { - log.Printf("clientFlagsObserver: handling flag state event: %v", event) + log.Printf("serverFlagsObserver: handling flag state event: %v", event) switch event := event.(type) { case model.UpsertOverrideEvent: if event.ProjectKey != c.projectKey { From 87b2fdeaa21b8e84b8b27a09f880737eb1a07c49 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Tue, 9 Jul 2024 10:04:27 -0400 Subject: [PATCH 08/13] add overrides --- internal/dev_server/model/project.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/dev_server/model/project.go b/internal/dev_server/model/project.go index 012ffb87..6d2677cb 100644 --- a/internal/dev_server/model/project.go +++ b/internal/dev_server/model/project.go @@ -102,9 +102,14 @@ func SyncProject(ctx context.Context, projectKey string) (Project, error) { return Project{}, errors.New("Project not updated") } + allFlagsWithOverrides, err := project.GetFlagStateWithOverridesForProject(ctx) + if err != nil { + return Project{}, errors.Wrapf(err, "unable to get overrides for project, %s", projectKey) + } + GetObserversFromContext(ctx).Notify(SyncEvent{ ProjectKey: project.Key, - AllFlagsState: project.AllFlagsState, + AllFlagsState: allFlagsWithOverrides, }) return *project, nil } From 2ad9e37741dea3ea32f6d9149a798c24ccdefe20 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Tue, 9 Jul 2024 10:06:01 -0400 Subject: [PATCH 09/13] add expected times (causes timeout) --- internal/dev_server/sdk/go_sdk_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/dev_server/sdk/go_sdk_test.go b/internal/dev_server/sdk/go_sdk_test.go index dd162d83..49a93e9e 100644 --- a/internal/dev_server/sdk/go_sdk_test.go +++ b/internal/dev_server/sdk/go_sdk_test.go @@ -63,7 +63,7 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { AddFlag("jsonFlag", flagstate.FlagState{Value: ldvalue.CopyArbitraryValue(map[string]any{"cat": "hat"})}). Build() - sdk.EXPECT().GetAllFlagsState(gomock.Any(), gomock.Any(), testSdkKey).Return(allFlags, nil) + sdk.EXPECT().GetAllFlagsState(gomock.Any(), gomock.Any(), testSdkKey).Return(allFlags, nil).Times(2) _, err = model.CreateProject(ctx, projectKey, environmentKey, nil) require.NoError(t, err) From 56daafb50180ef51522964e676b6ce40870b2ad3 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Tue, 9 Jul 2024 10:24:53 -0400 Subject: [PATCH 10/13] make test go --- internal/dev_server/sdk/go_sdk_test.go | 28 +++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/internal/dev_server/sdk/go_sdk_test.go b/internal/dev_server/sdk/go_sdk_test.go index 49a93e9e..a446bc4d 100644 --- a/internal/dev_server/sdk/go_sdk_test.go +++ b/internal/dev_server/sdk/go_sdk_test.go @@ -125,23 +125,41 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { }) } + newUpdates := map[string]ldvalue.Value{ + "boolFlag": ldvalue.Bool(true), + "stringFlag": ldvalue.String("fool"), + "intFlag": ldvalue.Int(567), + "doubleFlag": ldvalue.Float64(99.99), + "jsonFlag": ldvalue.CopyArbitraryValue(map[string]any{"tortoise": "hare"}), + } + + // This test is testing the "put" payload in a roundabout way by verifying each of the flags are in there. t.Run("Sync sends full flag payload for project", func(t *testing.T) { - trackers := make(map[string]<-chan interfaces.FlagValueChangeEvent, len(updates)) + trackers := make(map[string]*<-chan interfaces.FlagValueChangeEvent, len(updates)) - for flagKey := range updates { + for flagKey, newValue := range newUpdates { flagUpdateChan := ld.GetFlagTracker().AddFlagValueChangeListener(flagKey, ldContext, ldvalue.String("uh-oh")) defer ld.GetFlagTracker().RemoveFlagValueChangeListener(flagUpdateChan) - trackers[flagKey] = flagUpdateChan + trackers[flagKey] = &flagUpdateChan + + _, err := store.UpsertOverride(ctx, model.Override{ + ProjectKey: projectKey, + FlagKey: flagKey, + Value: newValue, + Active: true, + Version: 1, + }) + require.NoError(t, err) } _, err := model.SyncProject(ctx, projectKey) require.NoError(t, err) - for flagKey, value := range updates { + for flagKey, value := range newUpdates { updateTracker, ok := trackers[flagKey] require.True(t, ok) - update := <-updateTracker + update := <-*updateTracker assert.Equal(t, value.AsArbitraryValue(), update.NewValue.AsArbitraryValue()) } }) From 797990a09936bcfe27a09a5df7716906f2a6d7f3 Mon Sep 17 00:00:00 2001 From: Thomas Varney <70972175+tvarney13@users.noreply.github.com> Date: Tue, 9 Jul 2024 12:59:51 -0400 Subject: [PATCH 11/13] Make comment more accurate Co-authored-by: Mike Zorn --- internal/dev_server/sdk/go_sdk_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/dev_server/sdk/go_sdk_test.go b/internal/dev_server/sdk/go_sdk_test.go index a446bc4d..f89c8e44 100644 --- a/internal/dev_server/sdk/go_sdk_test.go +++ b/internal/dev_server/sdk/go_sdk_test.go @@ -134,6 +134,8 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { } // This test is testing the "put" payload in a roundabout way by verifying each of the flags are in there. + // Note that the store.UpsertOverride is called directly so that updates for the overrides are not sent via + // patches. t.Run("Sync sends full flag payload for project", func(t *testing.T) { trackers := make(map[string]*<-chan interfaces.FlagValueChangeEvent, len(updates)) From 19cdfda9ec1036572d5cfd44347bf54b8f453b29 Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Tue, 9 Jul 2024 13:01:50 -0400 Subject: [PATCH 12/13] chans are pointer types already --- internal/dev_server/sdk/go_sdk_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/dev_server/sdk/go_sdk_test.go b/internal/dev_server/sdk/go_sdk_test.go index f89c8e44..fe1f9914 100644 --- a/internal/dev_server/sdk/go_sdk_test.go +++ b/internal/dev_server/sdk/go_sdk_test.go @@ -134,15 +134,15 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { } // This test is testing the "put" payload in a roundabout way by verifying each of the flags are in there. - // Note that the store.UpsertOverride is called directly so that updates for the overrides are not sent via + // Note that the store.UpsertOverride is called directly so that updates for the overrides are not sent via // patches. t.Run("Sync sends full flag payload for project", func(t *testing.T) { - trackers := make(map[string]*<-chan interfaces.FlagValueChangeEvent, len(updates)) + trackers := make(map[string]<-chan interfaces.FlagValueChangeEvent, len(updates)) for flagKey, newValue := range newUpdates { flagUpdateChan := ld.GetFlagTracker().AddFlagValueChangeListener(flagKey, ldContext, ldvalue.String("uh-oh")) defer ld.GetFlagTracker().RemoveFlagValueChangeListener(flagUpdateChan) - trackers[flagKey] = &flagUpdateChan + trackers[flagKey] = flagUpdateChan _, err := store.UpsertOverride(ctx, model.Override{ ProjectKey: projectKey, @@ -161,7 +161,7 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { updateTracker, ok := trackers[flagKey] require.True(t, ok) - update := <-*updateTracker + update := <-updateTracker assert.Equal(t, value.AsArbitraryValue(), update.NewValue.AsArbitraryValue()) } }) From d341962ca8f5d1b62de17320f2b15c7e5166ab2c Mon Sep 17 00:00:00 2001 From: Thomas Varney Date: Tue, 9 Jul 2024 13:13:04 -0400 Subject: [PATCH 13/13] modify test --- internal/dev_server/sdk/go_sdk_test.go | 71 ++++++++++++-------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/internal/dev_server/sdk/go_sdk_test.go b/internal/dev_server/sdk/go_sdk_test.go index fe1f9914..b6a32370 100644 --- a/internal/dev_server/sdk/go_sdk_test.go +++ b/internal/dev_server/sdk/go_sdk_test.go @@ -63,7 +63,7 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { AddFlag("jsonFlag", flagstate.FlagState{Value: ldvalue.CopyArbitraryValue(map[string]any{"cat": "hat"})}). Build() - sdk.EXPECT().GetAllFlagsState(gomock.Any(), gomock.Any(), testSdkKey).Return(allFlags, nil).Times(2) + sdk.EXPECT().GetAllFlagsState(gomock.Any(), gomock.Any(), testSdkKey).Return(allFlags, nil) _, err = model.CreateProject(ctx, projectKey, environmentKey, nil) require.NoError(t, err) @@ -107,57 +107,32 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { assert.Equal(t, map[string]any{"cat": "hat"}, val.AsArbitraryValue()) }) - updates := map[string]ldvalue.Value{ - "boolFlag": ldvalue.Bool(false), - "stringFlag": ldvalue.String("drool"), - "intFlag": ldvalue.Int(456), - "doubleFlag": ldvalue.Float64(88.88), - "jsonFlag": ldvalue.CopyArbitraryValue(map[string]any{"tortoise": "shell"}), - } - for flagKey, value := range updates { - t.Run(fmt.Sprintf("%s is %v after override", flagKey, value), func(t *testing.T) { - flagUpdateChan := ld.GetFlagTracker().AddFlagValueChangeListener(flagKey, ldContext, ldvalue.String("uh-oh")) - defer ld.GetFlagTracker().RemoveFlagValueChangeListener(flagUpdateChan) - _, err := model.UpsertOverride(ctx, projectKey, flagKey, value) - require.NoError(t, err) - flagUpdate := <-flagUpdateChan - assert.Equal(t, value.AsArbitraryValue(), flagUpdate.NewValue.AsArbitraryValue()) - }) - } + // Mock scenario: we re-sync and the SDK returns new values and higher version numbers + updatedFlags := flagstate.NewAllFlagsBuilder(). + AddFlag("boolFlag", flagstate.FlagState{Value: ldvalue.Bool(false), Version: 2}). + AddFlag("stringFlag", flagstate.FlagState{Value: ldvalue.String("pool"), Version: 2}). + AddFlag("intFlag", flagstate.FlagState{Value: ldvalue.Int(789), Version: 2}). + AddFlag("doubleFlag", flagstate.FlagState{Value: ldvalue.Float64(101.01), Version: 2}). + AddFlag("jsonFlag", flagstate.FlagState{Value: ldvalue.CopyArbitraryValue(map[string]any{"cat": "bababooey"}), Version: 2}). + Build() + valuesMap := updatedFlags.ToValuesMap() - newUpdates := map[string]ldvalue.Value{ - "boolFlag": ldvalue.Bool(true), - "stringFlag": ldvalue.String("fool"), - "intFlag": ldvalue.Int(567), - "doubleFlag": ldvalue.Float64(99.99), - "jsonFlag": ldvalue.CopyArbitraryValue(map[string]any{"tortoise": "hare"}), - } + sdk.EXPECT().GetAllFlagsState(gomock.Any(), gomock.Any(), testSdkKey).Return(updatedFlags, nil) // This test is testing the "put" payload in a roundabout way by verifying each of the flags are in there. - // Note that the store.UpsertOverride is called directly so that updates for the overrides are not sent via - // patches. t.Run("Sync sends full flag payload for project", func(t *testing.T) { - trackers := make(map[string]<-chan interfaces.FlagValueChangeEvent, len(updates)) + trackers := make(map[string]<-chan interfaces.FlagValueChangeEvent, len(valuesMap)) - for flagKey, newValue := range newUpdates { + for flagKey := range valuesMap { flagUpdateChan := ld.GetFlagTracker().AddFlagValueChangeListener(flagKey, ldContext, ldvalue.String("uh-oh")) defer ld.GetFlagTracker().RemoveFlagValueChangeListener(flagUpdateChan) trackers[flagKey] = flagUpdateChan - - _, err := store.UpsertOverride(ctx, model.Override{ - ProjectKey: projectKey, - FlagKey: flagKey, - Value: newValue, - Active: true, - Version: 1, - }) - require.NoError(t, err) } _, err := model.SyncProject(ctx, projectKey) require.NoError(t, err) - for flagKey, value := range newUpdates { + for flagKey, value := range valuesMap { updateTracker, ok := trackers[flagKey] require.True(t, ok) @@ -165,4 +140,22 @@ func TestSDKRoutesViaGoSDK(t *testing.T) { assert.Equal(t, value.AsArbitraryValue(), update.NewValue.AsArbitraryValue()) } }) + + updates := map[string]ldvalue.Value{ + "boolFlag": ldvalue.Bool(true), + "stringFlag": ldvalue.String("drool"), + "intFlag": ldvalue.Int(456), + "doubleFlag": ldvalue.Float64(88.88), + "jsonFlag": ldvalue.CopyArbitraryValue(map[string]any{"tortoise": "shell"}), + } + for flagKey, value := range updates { + t.Run(fmt.Sprintf("%s is %v after override", flagKey, value), func(t *testing.T) { + flagUpdateChan := ld.GetFlagTracker().AddFlagValueChangeListener(flagKey, ldContext, ldvalue.String("uh-oh")) + defer ld.GetFlagTracker().RemoveFlagValueChangeListener(flagUpdateChan) + _, err := model.UpsertOverride(ctx, projectKey, flagKey, value) + require.NoError(t, err) + flagUpdate := <-flagUpdateChan + assert.Equal(t, value.AsArbitraryValue(), flagUpdate.NewValue.AsArbitraryValue()) + }) + } }