diff --git a/cache/key_value_cache.go b/cache/key_value_cache.go index 29acfaea..06c1bb83 100644 --- a/cache/key_value_cache.go +++ b/cache/key_value_cache.go @@ -98,7 +98,6 @@ func (k *KeyValCache) Get(ctx context.Context, key string, value interface{}) er } return fmt.Errorf("%w: KeyValCache.Get failed for key: %q", err, key) } - return k.unmarshalFn(b, value) } diff --git a/cache/refresher.go b/cache/refresher.go index 70e0fb47..065483ca 100644 --- a/cache/refresher.go +++ b/cache/refresher.go @@ -334,31 +334,66 @@ func (s Refresher) handleRemoveAPIKeyEvent(ctx context.Context, env, apiKey stri }) } -func (s Refresher) handleFetchFeatureEvent(ctx context.Context, env, id string) error { - s.log.Debug("updating featureConfig entry", "environment", env, "identifier", id) - - featureConfigs, err := s.clientService.FetchFeatureConfigForEnvironment(ctx, s.config.Token(), s.config.ClusterIdentifier(), env) +// handleFetchFeatureEvent handles any PATCH/CREATE Feature Config events that we receive from Harness Saas. +// +// When we get a PATCH/CREATE feature event there are three keys in the cache that we need to update. +// 1. env--feature-config- entry which is the individual feature config record for that environment +// 2. env--feature-configs entry which is the collection of featureConfigs for that environment. Since a change has been made to a flag in this environment, we need to update that specific flag in this collection. +// 3. inventory entry for the featureConfig. This Inventory entry tracks all the resources in the cache associated with the Proxy key. It's used for cache cleanup and diffing assets during stream disconnects. +// +// In order to ensure we update these records correctly, when we get a PATCH/CREATE feature event we need to do the following: +// 1. Fetch the featureConfig from HarnessSaas. +// 2. Update the individual featureConfig record in the cache. +// 3. Update the featureConfigs record in the cache. +// 4. Update the inventory record for the featureConfig. +func (s Refresher) handleFetchFeatureEvent(ctx context.Context, env, identifier string) error { + s.log.Debug("updating featureConfig entry", "environment", env, "identifier", identifier) + + // Make a request to Harness Saas to fetch the updated featureConfig + fc, err := s.clientService.GetFeatureConfigByIdentifier(ctx, domain.GetFeatureConfigsByIdentifierInput{ + AuthToken: s.config.Token(), + Cluster: s.config.ClusterIdentifier(), + EnvID: env, + Identifier: identifier, + }) if err != nil { return err } - features := make([]domain.FeatureFlag, 0, len(featureConfigs)) - for _, v := range featureConfigs { - features = append(features, domain.FeatureFlag(v)) + updatedFlagConfig := domain.FeatureFlag(fc) + + // Retrieve the currently cached flag config values. + featureConfigs, ok := s.flagRepo.GetFeatureConfigForEnvironment(ctx, env) + if !ok { + return fmt.Errorf("failed to get featureConfig for environment %s", env) } - // set the config - if err := s.flagRepo.Add(ctx, domain.FlagConfig{ - EnvironmentID: env, - FeatureConfigs: features, - }); err != nil { + // Iterate over the cached flag config values and update the record that has changed + replaceFeatureConfig(updatedFlagConfig, &featureConfigs) + + // Update the cached flagConfig. This will take care of updating the individual cached record stored at env--feature-config- and the collection of featureConfigs stored at env--feature-configs. + if err := s.flagRepo.Add(ctx, domain.FlagConfig{EnvironmentID: env, FeatureConfigs: featureConfigs}); err != nil { return err } - // patch the inventory + + // Update the inventory entry for the featureConfig. return s.inventory.Patch(ctx, s.config.Key(), func(assets map[string]int64) (map[string]int64, error) { - return addFeatureItems(assets, env, features) + return addFeatureItems(assets, env, featureConfigs) }) } +func replaceFeatureConfig(newConfig domain.FeatureFlag, featureConfigs *[]domain.FeatureFlag) { + if featureConfigs == nil { + return + } + + for i := range *featureConfigs { + if (*featureConfigs)[i].Feature == newConfig.Feature { + (*featureConfigs)[i] = newConfig + break + } + } +} + func (s Refresher) handleDeleteFeatureEvent(ctx context.Context, env, identifier string) error { s.log.Debug("removing featureConfig entry", "environment", env, "identifier", identifier) featureConfigEntry := string(domain.NewFeatureConfigKey(env, identifier)) diff --git a/cache/refresher_test.go b/cache/refresher_test.go index 0715feb3..ce96bbce 100644 --- a/cache/refresher_test.go +++ b/cache/refresher_test.go @@ -20,12 +20,17 @@ func TestRefresher_HandleMessage(t *testing.T) { message domain.SSEMessage } + type mocks struct { + clientService mockClientService + } + type expected struct { err error } testCases := map[string]struct { args args + mocks mocks expected expected shouldErr bool }{ @@ -33,6 +38,9 @@ func TestRefresher_HandleMessage(t *testing.T) { args: args{ message: domain.SSEMessage{Domain: "Foo"}, }, + mocks: mocks{ + clientService: mockClientService{}, + }, expected: expected{err: ErrUnexpectedMessageDomain}, shouldErr: true, }, @@ -43,6 +51,9 @@ func TestRefresher_HandleMessage(t *testing.T) { Event: "foo", }, }, + mocks: mocks{ + clientService: mockClientService{}, + }, expected: expected{err: ErrUnexpectedEventType}, shouldErr: true, }, @@ -53,17 +64,26 @@ func TestRefresher_HandleMessage(t *testing.T) { Event: "foo", }, }, + mocks: mocks{ + clientService: mockClientService{}, + }, expected: expected{err: ErrUnexpectedEventType}, shouldErr: true, }, "Given I have an SSEMessage with the domain 'flag' event 'patch'": { args: args{ message: domain.SSEMessage{ - Domain: domain.MsgDomainFeature, - Event: domain.EventPatch, + Domain: domain.MsgDomainFeature, + Event: domain.EventPatch, + Environment: "123", }, }, - expected: expected{err: nil}, + mocks: mocks{clientService: mockClientService{getFeatureConfigByIdentifier: func(ctx context.Context, input domain.GetFeatureConfigsByIdentifierInput) (clientgen.FeatureConfig, error) { + return clientgen.FeatureConfig{Feature: "Foobar", Version: domain.ToPtr(int64(2))}, nil + }}}, + expected: expected{ + err: nil, + }, shouldErr: false, }, "Given I have an SSEMessage with the domain 'flag' event 'create'": { @@ -73,6 +93,9 @@ func TestRefresher_HandleMessage(t *testing.T) { Event: domain.EventCreate, }, }, + mocks: mocks{clientService: mockClientService{getFeatureConfigByIdentifier: func(ctx context.Context, input domain.GetFeatureConfigsByIdentifierInput) (clientgen.FeatureConfig, error) { + return clientgen.FeatureConfig{Feature: "Foobar", Version: domain.ToPtr(int64(2))}, nil + }}}, expected: expected{err: nil}, shouldErr: false, }, @@ -93,6 +116,12 @@ func TestRefresher_HandleMessage(t *testing.T) { Event: domain.EventPatch, }, }, + mocks: mocks{clientService: mockClientService{FetchSegmentConfigForEnvironmentFn: func(ctx context.Context, authToken, envId string) ([]clientgen.Segment, error) { + return []clientgen.Segment{ + {Identifier: "foo"}, + {Identifier: "bar"}, + }, nil + }}}, expected: expected{err: nil}, shouldErr: false, }, @@ -103,6 +132,12 @@ func TestRefresher_HandleMessage(t *testing.T) { Event: domain.EventCreate, }, }, + mocks: mocks{clientService: mockClientService{FetchSegmentConfigForEnvironmentFn: func(ctx context.Context, authToken, envId string) ([]clientgen.Segment, error) { + return []clientgen.Segment{ + {Identifier: "foo"}, + {Identifier: "bar"}, + }, nil + }}}, expected: expected{err: nil}, shouldErr: false, }, @@ -113,6 +148,9 @@ func TestRefresher_HandleMessage(t *testing.T) { Event: domain.EventDelete, }, }, + mocks: mocks{ + clientService: mockClientService{}, + }, expected: expected{err: nil}, shouldErr: false, }, @@ -144,6 +182,9 @@ func TestRefresher_HandleMessage(t *testing.T) { Environments: []string{"123"}, }, }, + mocks: mocks{clientService: mockClientService{PageProxyConfigFn: func(ctx context.Context, input domain.GetProxyConfigInput) ([]domain.ProxyConfig, error) { + return []domain.ProxyConfig{}, nil + }}}, expected: expected{err: nil}, shouldErr: false, }, @@ -194,20 +235,6 @@ func TestRefresher_HandleMessage(t *testing.T) { }, } - mockClient := mockClientService{ - - PageProxyConfigFn: func(ctx context.Context, input domain.GetProxyConfigInput) ([]domain.ProxyConfig, error) { - return []domain.ProxyConfig{}, nil - }, - - FetchFeatureConfigForEnvironmentFn: func(ctx context.Context, authToken, envId string) ([]clientgen.FeatureConfig, error) { - return []clientgen.FeatureConfig{}, nil - }, - FetchSegmentConfigForEnvironmentFn: func(ctx context.Context, authToken, envId string) ([]clientgen.Segment, error) { - return []clientgen.Segment{}, nil - }, - } - authRepo := mockAuthRepo{ addFn: func(ctx context.Context, values ...domain.AuthConfig) error { return nil @@ -289,7 +316,7 @@ func TestRefresher_HandleMessage(t *testing.T) { t.Run(desc, func(t *testing.T) { - r := NewRefresher(log.NewNoOpLogger(), config, mockClient, inventoryRepo, authRepo, flagRepo, segmentRepo) + r := NewRefresher(log.NewNoOpLogger(), config, tc.mocks.clientService, inventoryRepo, authRepo, flagRepo, segmentRepo) err := r.HandleMessage(context.Background(), tc.args.message) if tc.shouldErr { assert.NotNil(t, err) @@ -735,6 +762,16 @@ type mockClientService struct { PageProxyConfigFn func(ctx context.Context, input domain.GetProxyConfigInput) ([]domain.ProxyConfig, error) FetchFeatureConfigForEnvironmentFn func(ctx context.Context, authToken, envId string) ([]clientgen.FeatureConfig, error) FetchSegmentConfigForEnvironmentFn func(ctx context.Context, authToken, envId string) ([]clientgen.Segment, error) + getFeatureConfigByIdentifier func(ctx context.Context, input domain.GetFeatureConfigsByIdentifierInput) (clientgen.FeatureConfig, error) + getSegmentByIdentifier func(ctx context.Context, input domain.GetSegmentByIdentifierInput) (clientgen.Segment, error) +} + +func (c mockClientService) GetFeatureConfigByIdentifier(ctx context.Context, input domain.GetFeatureConfigsByIdentifierInput) (clientgen.FeatureConfig, error) { + return c.getFeatureConfigByIdentifier(ctx, input) +} + +func (c mockClientService) GetSegmentByIdentifier(ctx context.Context, input domain.GetSegmentByIdentifierInput) (clientgen.Segment, error) { + return c.getSegmentByIdentifier(ctx, input) } func (c mockClientService) FetchSegmentConfigForEnvironment(ctx context.Context, authToken, cluster, envId string) ([]clientgen.Segment, error) { @@ -903,3 +940,50 @@ func TestRefresher_addSegmentItems(t *testing.T) { }) } } + +func TestReplaceFeatureConfig(t *testing.T) { + var version1 int64 = 1 + var version2 int64 = 2 + + tests := []struct { + name string + initialConfigs []domain.FeatureFlag + newConfig domain.FeatureFlag + expected []domain.FeatureFlag + }{ + { + name: "replaces matching feature", + initialConfigs: []domain.FeatureFlag{ + {Feature: "featA", Version: &version1}, + {Feature: "featB", Version: &version1}, + }, + newConfig: domain.FeatureFlag{Feature: "featA", Version: &version2}, + expected: []domain.FeatureFlag{ + {Feature: "featA", Version: &version2}, + {Feature: "featB", Version: &version1}, + }, + }, + { + name: "no matching feature - no change", + initialConfigs: []domain.FeatureFlag{ + {Feature: "featA", Version: &version1}, + }, + newConfig: domain.FeatureFlag{Feature: "featB", Version: &version2}, + expected: []domain.FeatureFlag{ + {Feature: "featA", Version: &version1}, + }, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + configs := append([]domain.FeatureFlag{}, tt.initialConfigs...) // copy to avoid mutation + replaceFeatureConfig(tt.newConfig, &configs) + if !reflect.DeepEqual(configs, tt.expected) { + t.Errorf("expected %+v, got %+v", tt.expected, configs) + } + }) + } +} diff --git a/clients/client_service/client.go b/clients/client_service/client.go index d5478e62..89fe25a6 100644 --- a/clients/client_service/client.go +++ b/clients/client_service/client.go @@ -277,14 +277,7 @@ func (c Client) FetchSegmentConfigForEnvironment(ctx context.Context, authToken, return *resp.JSON200, nil } -type GetFeatureConfigsByIdentifierInput struct { - AuthToken string - Cluster string - EnvID string - Identifier string -} - -func (c Client) GetFeatureConfigByIdentifier(ctx context.Context, input GetFeatureConfigsByIdentifierInput) (clientgen.FeatureConfig, error) { +func (c Client) GetFeatureConfigByIdentifier(ctx context.Context, input domain.GetFeatureConfigsByIdentifierInput) (clientgen.FeatureConfig, error) { resp, err := c.client.GetFeatureConfigByIdentifierWithResponse( ctx, input.EnvID, @@ -308,14 +301,7 @@ func (c Client) GetFeatureConfigByIdentifier(ctx context.Context, input GetFeatu return *resp.JSON200, nil } -type GetSegmentByIdentifierInput struct { - AuthToken string - Cluster string - EnvID string - Identifier string -} - -func (c Client) GetSegmentByIdentifier(ctx context.Context, input GetSegmentByIdentifierInput) (clientgen.Segment, error) { +func (c Client) GetSegmentByIdentifier(ctx context.Context, input domain.GetSegmentByIdentifierInput) (clientgen.Segment, error) { resp, err := c.client.GetSegmentByIdentifierWithResponse( ctx, input.EnvID, diff --git a/clients/client_service/client_test.go b/clients/client_service/client_test.go index 76592935..b9d48a55 100644 --- a/clients/client_service/client_test.go +++ b/clients/client_service/client_test.go @@ -473,7 +473,7 @@ func mustMarshal(v interface{}) []byte { } func TestClient_GetFeatureConfigsByIdentifier(t *testing.T) { - validInput := GetFeatureConfigsByIdentifierInput{ + validInput := domain.GetFeatureConfigsByIdentifierInput{ AuthToken: "foo", EnvID: "123", Identifier: "Hello", @@ -496,7 +496,7 @@ func TestClient_GetFeatureConfigsByIdentifier(t *testing.T) { } type args struct { - input GetFeatureConfigsByIdentifierInput + input domain.GetFeatureConfigsByIdentifierInput } type mocks struct { @@ -680,7 +680,7 @@ func TestClient_GetFeatureConfigsByIdentifier(t *testing.T) { } func TestClient_GetSegmentsByIdentifier(t *testing.T) { - validInput := GetSegmentByIdentifierInput{ + validInput := domain.GetSegmentByIdentifierInput{ AuthToken: "foo", EnvID: "123", Identifier: "Hello", @@ -702,7 +702,7 @@ func TestClient_GetSegmentsByIdentifier(t *testing.T) { } type args struct { - input GetSegmentByIdentifierInput + input domain.GetSegmentByIdentifierInput } type mocks struct { diff --git a/config/remote/config_test.go b/config/remote/config_test.go index f644e507..72eef370 100644 --- a/config/remote/config_test.go +++ b/config/remote/config_test.go @@ -221,6 +221,7 @@ func (m *mockFlagRepo) Add(ctx context.Context, config ...domain.FlagConfig) err } type mockClientService struct { + domain.ClientService authProxyKey func() (domain.AuthenticateProxyKeyResponse, error) pageProxyConfig func() ([]domain.ProxyConfig, error) } diff --git a/domain/requests.go b/domain/requests.go index b6e08208..89b4c48c 100644 --- a/domain/requests.go +++ b/domain/requests.go @@ -120,3 +120,17 @@ func AddHarnessXHeaders(envID string) func(ctx context.Context, req *http.Reques return nil } } + +type GetSegmentByIdentifierInput struct { + AuthToken string + Cluster string + EnvID string + Identifier string +} + +type GetFeatureConfigsByIdentifierInput struct { + AuthToken string + Cluster string + EnvID string + Identifier string +} diff --git a/domain/services.go b/domain/services.go index 97557782..f2463d36 100644 --- a/domain/services.go +++ b/domain/services.go @@ -12,4 +12,6 @@ type ClientService interface { PageProxyConfig(ctx context.Context, input GetProxyConfigInput) ([]ProxyConfig, error) FetchFeatureConfigForEnvironment(ctx context.Context, authToken, cluster string, envID string) ([]clientgen.FeatureConfig, error) FetchSegmentConfigForEnvironment(ctx context.Context, authToken, cluster string, envID string) ([]clientgen.Segment, error) + GetFeatureConfigByIdentifier(ctx context.Context, input GetFeatureConfigsByIdentifierInput) (clientgen.FeatureConfig, error) + GetSegmentByIdentifier(ctx context.Context, input GetSegmentByIdentifierInput) (clientgen.Segment, error) }