FFM-11653 Sort Flag/Group rules when saving to cache (#162)

* Bump github.com/hashicorp/go-retryablehttp from 0.7.4 to 0.7.7 Bumps [github.com/hashicorp/go-retryablehttp](https://github.com/hashicorp/go-retryablehttp) from 0.7.4 to 0.7.7. - [Changelog](https://github.com/hashicorp/go-retryablehttp/blob/main/CHANGELOG.md) - [Commits](hashicorp/go-retryablehttp@v0.7.4...v0.7.7) --- updated-dependencies: - dependency-name: github.com/hashicorp/go-retryablehttp dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> * FFM-11653 Sort v2 serving rules when storing segment * FFM-11653 Sort flag rules when storing flag * FFM-11653 Fix typo * FFM-11194 Fix test * FFM-11194 Bump sdk version --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
harness · Jun 27, 2024 · e16f427 · e16f427
1 parent 4e63499
commit e16f427
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 18 deletions.
diff --git a/analyticsservice/analytics.go b/analyticsservice/analytics.go
@@ -26,7 +26,7 @@ const (
 	variationValueAttribute      string = "featureValue"
 	targetAttribute              string = "target"
 	sdkVersionAttribute          string = "SDK_VERSION"
-	SdkVersion                   string = "0.1.23"
+	SdkVersion                   string = "0.1.24"
 	sdkTypeAttribute             string = "SDK_TYPE"
 	sdkType                      string = "server"
 	sdkLanguageAttribute         string = "SDK_LANGUAGE"

diff --git a/evaluation/evaluator.go b/evaluation/evaluator.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -145,9 +144,6 @@ func (e Evaluator) evaluateRules(servingRules []rest.ServingRule, target *Target
 		return ""
 	}
 
-	sort.SliceStable(servingRules, func(i, j int) bool {
-		return servingRules[i].Priority < servingRules[j].Priority
-	})
 	for i := range servingRules {
 		rule := servingRules[i]
 		if len(rule.Clauses) == 0 {
@@ -279,9 +275,6 @@ func (e Evaluator) isTargetIncludedOrExcludedInSegment(segmentList []string, tar
 		// `ServingRules` replaces `Rules, so if sent by the backend then we evaluate them instead
 		if segment.ServingRules != nil && len(*segment.ServingRules) > 0 {
 			v2Rules := *segment.ServingRules
-			sort.SliceStable(v2Rules, func(i, j int) bool {
-				return v2Rules[i].Priority < v2Rules[j].Priority
-			})
 			for _, v2rule := range v2Rules {
 				if e.evaluateGroupRulesV2(v2rule.Clauses, target) {
 					e.logger.Debugf(

diff --git a/evaluation/evaluator_test.go b/evaluation/evaluator_test.go
@@ -832,7 +832,7 @@ func TestEvaluator_evaluateRules(t *testing.T) {
 				// priority is on second one and should return true
 				servingRules: []rest.ServingRule{
 					{
-						Priority: 2,
+						Priority: 1,
 						Clauses: []rest.Clause{
 							{
 								Attribute: identifier,
@@ -841,11 +841,11 @@ func TestEvaluator_evaluateRules(t *testing.T) {
 							},
 						},
 						Serve: rest.Serve{
-							Variation: &identifierFalse,
+							Variation: &identifierTrue,
 						},
 					},
 					{
-						Priority: 1,
+						Priority: 2,
 						Clauses: []rest.Clause{
 							{
 								Attribute: identifier,
@@ -854,7 +854,7 @@ func TestEvaluator_evaluateRules(t *testing.T) {
 							},
 						},
 						Serve: rest.Serve{
-							Variation: &identifierTrue,
+							Variation: &identifierFalse,
 						},
 					},
 				},

diff --git a/go.mod b/go.mod
@@ -9,7 +9,7 @@ require (
 	github.com/golang-jwt/jwt v3.2.2+incompatible
 	github.com/google/uuid v1.5.0
 	github.com/harness-community/sse/v3 v3.1.0
-	github.com/hashicorp/go-retryablehttp v0.7.4
+	github.com/hashicorp/go-retryablehttp v0.7.7
 	github.com/hashicorp/golang-lru v0.5.4
 	github.com/jarcoal/httpmock v1.0.8
 	github.com/json-iterator/go v1.1.12

diff --git a/go.sum b/go.sum
@@ -11,6 +11,7 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/deepmap/oapi-codegen/v2 v2.1.0 h1:I/NMVhJCtuvL9x+S2QzZKpSjGi33oDZwPRdemvOZWyQ=
 github.com/deepmap/oapi-codegen/v2 v2.1.0/go.mod h1:R1wL226vc5VmCNJUvMyYr3hJMm5reyv25j952zAVXZ8=
+github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
 github.com/getkin/kin-openapi v0.124.0 h1:VSFNMB9C9rTKBnQ/fpyDU8ytMTr4dWI9QovSKj9kz/M=
 github.com/getkin/kin-openapi v0.124.0/go.mod h1:wb1aSZA/iWmorQP9KTAS/phLj/t17B5jT7+fS8ed9NM=
 github.com/go-openapi/jsonpointer v0.20.2 h1:mQc3nmndL8ZBzStEo3JYF8wzmeWffDH4VbXz58sAx6Q=
@@ -28,10 +29,9 @@ github.com/harness-community/sse/v3 v3.1.0 h1:uaLxXzC9DjpWEV/qTYU3uJV3eLMTRhMY2P
 github.com/harness-community/sse/v3 v3.1.0/go.mod h1:v4ft76Eaj+kAsUcc29zIspInWgpzsMLlHLb4x/PYVX0=
 github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
 github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
-github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI=
-github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
-github.com/hashicorp/go-retryablehttp v0.7.4 h1:ZQgVdpTdAL7WpMIwLzCfbalOcSUdkDZnpUv3/+BxzFA=
-github.com/hashicorp/go-retryablehttp v0.7.4/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8=
+github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k=
+github.com/hashicorp/go-retryablehttp v0.7.7 h1:C8hUCYzor8PIfXHa4UrZkU4VvK8o9ISHxT2Q8+VepXU=
+github.com/hashicorp/go-retryablehttp v0.7.7/go.mod h1:pkQpWZeYWskR+D1tR2O5OcBFOxfA7DoAO6xtkuQnHTk=
 github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
 github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
 github.com/invopop/yaml v0.2.0 h1:7zky/qH+O0DwAyoobXUqvVBwgBFRxKoQ/3FjcVpjTMY=
@@ -51,6 +51,8 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
+github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
+github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/mitchellh/mapstructure v1.3.3 h1:SzB1nHZ2Xi+17FP0zVQBHIZqvwRN9408fJO8h+eeNA8=
@@ -76,7 +78,6 @@ github.com/spaolacci/murmur3 v1.1.0 h1:7c1g84S4BPRrfL5Xrdp6fOJ206sU9y293DDHaoy0b
 github.com/spaolacci/murmur3 v1.1.0/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
 github.com/spkg/bom v0.0.0-20160624110644-59b7046e48ad/go.mod h1:qLr4V1qq6nMqFKkMo8ZTx3f+BZEkzsRUY10Xsm2mwU0=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=

diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go
@@ -2,6 +2,7 @@ package repository
 
 import (
 	"fmt"
+	"sort"
 
 	"golang.org/x/exp/slices"
 
@@ -156,6 +157,9 @@ func (r FFRepository) SetFlag(featureConfig rest.FeatureConfig, initialLoad bool
 			return
 		}
 	}
+
+	SortFeatureConfigServingRules(&featureConfig)
+
 	flagKey := formatFlagKey(featureConfig.Feature)
 	if r.storage != nil {
 		if err := r.storage.Set(flagKey, featureConfig); err != nil {
@@ -180,6 +184,10 @@ func (r FFRepository) SetFlags(initialLoad bool, envID string, featureConfigs ..
 		}
 	}
 
+	for i := range featureConfigs {
+		SortFeatureConfigServingRules(&featureConfigs[i])
+	}
+
 	key := formatFlagsKey(envID)
 
 	if r.storage != nil {
@@ -204,6 +212,7 @@ func (r FFRepository) SetSegment(segment rest.Segment, initialLoad bool) {
 			return
 		}
 	}
+	SortSegmentServingGroups(&segment)
 	segmentKey := formatSegmentKey(segment.Identifier)
 	if r.storage != nil {
 		if err := r.storage.Set(segmentKey, segment); err != nil {
@@ -228,6 +237,11 @@ func (r FFRepository) SetSegments(initialLoad bool, envID string, segments ...re
 		}
 	}
 
+	// Sort the serving rules of each segment before storing them
+	for i := range segments {
+		SortSegmentServingGroups(&segments[i])
+	}
+
 	key := formatSegmentsKey(envID)
 
 	if r.storage != nil {
@@ -447,6 +461,32 @@ func (r FFRepository) areSegmentsOutdated(envID string, segments ...rest.Segment
 	return false
 }
 
+// SortFeatureConfigServingRules sorts the serving rules of a FeatureConfig by priority
+func SortFeatureConfigServingRules(featureConfig *rest.FeatureConfig) {
+	if featureConfig == nil || featureConfig.Rules == nil || len(*featureConfig.Rules) <= 1 {
+		return
+	}
+
+	rules := *featureConfig.Rules
+
+	sort.SliceStable(rules, func(i, j int) bool {
+		return rules[i].Priority < rules[j].Priority
+	})
+}
+
+// SortSegmentServingGroups sorts the serving rules of a segment by priority
+func SortSegmentServingGroups(segment *rest.Segment) {
+	if segment == nil || segment.ServingRules == nil || len(*segment.ServingRules) <= 1 {
+		return
+	}
+
+	v2Rules := *segment.ServingRules
+
+	sort.SliceStable(v2Rules, func(i, j int) bool {
+		return v2Rules[i].Priority < v2Rules[j].Priority
+	})
+}
+
 // Close all resources
 func (r FFRepository) Close() {