FFM-12699 - Update refresher to only pull flags that have changed

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)
 }
 

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-<id>-feature-config-<identifier> entry which is the individual feature config record for that environment
+// 2. env-<id>-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-<id>-feature-config-<identifier> and the collection of featureConfigs stored at env-<id>-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))

cache/refresher_test.go

@@ -20,19 +20,27 @@ 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
 	}{
 		"Given I have an SSEMessage with the domain 'Foo'": {
 			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)
+			}
+		})
+	}
+}

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,

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 {

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)
 }