Skip to content

Commit aa62214

Browse files
petrukipetruki
authored
Improved Strategy comparison logic - removed STRATEGY_VALUE (#44)
1 parent 4cfd8f4 commit aa62214

File tree

6 files changed

+177
-43
lines changed

6 files changed

+177
-43
lines changed

resources/fixtures/comparator/deleted_strategy_value.json renamed to resources/fixtures/comparator/changed_strategy_value.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
"strategy": "VALUE_VALIDATION",
1616
"activated": false,
1717
"operation": "EXIST",
18-
"values": []
18+
"values": [
19+
"user_2"
20+
]
1921
}
2022
],
2123
"components": [

resources/fixtures/comparator/new_strategy_value.json renamed to resources/fixtures/comparator/changed_strategy_value_new.json

File renamed without changes.

‎resources/fixtures/comparator/changed_strategy_value_new_deleted.json‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
{
2+
"domain": {
3+
"group": [
4+
{
5+
"name": "Release 1",
6+
"description": "Showcase configuration",
7+
"activated": true,
8+
"config": [
9+
{
10+
"key": "MY_SWITCHER_1",
11+
"description": "My first switcher",
12+
"activated": true,
13+
"strategies": [
14+
{
15+
"strategy": "VALUE_VALIDATION",
16+
"activated": false,
17+
"operation": "EXIST",
18+
"values": [
19+
"user_1",
20+
"user_3"
21+
]
22+
}
23+
],
24+
"components": [
25+
"switcher-playground"
26+
]
27+
},
28+
{
29+
"key": "MY_SWITCHER_2",
30+
"description": "",
31+
"activated": false,
32+
"strategies": [],
33+
"components": [
34+
"switcher-playground"
35+
]
36+
},
37+
{
38+
"key": "MY_SWITCHER_3",
39+
"description": "",
40+
"activated": true,
41+
"strategies": [],
42+
"components": [
43+
"benchmark"
44+
]
45+
}
46+
]
47+
}
48+
]
49+
}
50+
}

‎resources/fixtures/comparator/staging.json‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
{
2+
"domain": {
3+
"group": [
4+
{
5+
"name": "Release 1",
6+
"description": "Showcase configuration",
7+
"activated": true,
8+
"config": [
9+
{
10+
"key": "MY_SWITCHER_1",
11+
"description": "My first switcher",
12+
"activated": true,
13+
"strategies": [
14+
{
15+
"strategy": "VALUE_VALIDATION",
16+
"activated": false,
17+
"operation": "EXIST",
18+
"values": [
19+
"user_1",
20+
"user_2"
21+
]
22+
}
23+
],
24+
"components": [
25+
"switcher-playground"
26+
]
27+
},
28+
{
29+
"key": "MY_SWITCHER_2",
30+
"description": "",
31+
"activated": false,
32+
"strategies": [],
33+
"components": [
34+
"switcher-playground"
35+
]
36+
},
37+
{
38+
"key": "MY_SWITCHER_3",
39+
"description": "",
40+
"activated": true,
41+
"strategies": [],
42+
"components": [
43+
"benchmark"
44+
]
45+
}
46+
]
47+
}
48+
]
49+
}
50+
}

‎src/core/comparator.go‎

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,10 @@ const (
2424
CHANGED DiffType = "CHANGED"
2525
DELETED DiffType = "DELETED"
2626

27-
GROUP DiffResult = "GROUP"
28-
CONFIG DiffResult = "CONFIG"
29-
STRATEGY DiffResult = "STRATEGY"
30-
STRATEGY_VALUE DiffResult = "STRATEGY_VALUE"
31-
COMPONENT DiffResult = "COMPONENT"
27+
GROUP DiffResult = "GROUP"
28+
CONFIG DiffResult = "CONFIG"
29+
STRATEGY DiffResult = "STRATEGY"
30+
COMPONENT DiffResult = "COMPONENT"
3231
)
3332

3433
func NewComparatorService() *ComparatorService {
@@ -148,10 +147,9 @@ func checkStrategyDiff(leftConfig model.Config, rightConfig model.Config, leftGr
148147
if diffType == CHANGED {
149148
diffFound = compareAndUpdateBool(leftStrategy.Activated, rightStrategy.Activated, diffFound, &modelDiffFound.Activated)
150149
diffFound = compareAndUpdateString(leftStrategy.Operation, rightStrategy.Operation, diffFound, &modelDiffFound.Operation)
150+
diffFound = compareAndUpdateArray(leftStrategy.Values, rightStrategy.Values, diffFound, &modelDiffFound.Values)
151151
}
152152

153-
checkValuesDiff(leftStrategy, rightStrategy, leftGroup, leftConfig, diffResult, diffType)
154-
155153
if diffFound {
156154
appendDiffResults(string(diffType), string(STRATEGY),
157155
[]string{leftGroup.Name, leftConfig.Key, leftStrategy.Strategy}, modelDiffFound, diffResult)
@@ -160,26 +158,6 @@ func checkStrategyDiff(leftConfig model.Config, rightConfig model.Config, leftGr
160158
}
161159
}
162160

163-
func checkValuesDiff(leftStrategy model.Strategy, rightStrategy model.Strategy, leftGroup model.Group, leftConfig model.Config,
164-
diffResult *model.DiffResult, diffType DiffType) {
165-
166-
if len(leftStrategy.Values) == 0 {
167-
return
168-
}
169-
170-
var diff []string
171-
for _, leftValue := range leftStrategy.Values {
172-
if (diffType == NEW || diffType == DELETED) && !slices.Contains(rightStrategy.Values, leftValue) {
173-
diff = append(diff, leftValue)
174-
}
175-
}
176-
177-
if len(diff) > 0 {
178-
appendDiffResults(string(diffType), string(STRATEGY_VALUE),
179-
[]string{leftGroup.Name, leftConfig.Key, leftStrategy.Strategy}, diff, diffResult)
180-
}
181-
}
182-
183161
func checkComponentsDiff(leftConfig model.Config, rightConfig model.Config, leftGroup model.Group,
184162
diffResult *model.DiffResult, diffType DiffType) {
185163

@@ -195,7 +173,8 @@ func checkComponentsDiff(leftConfig model.Config, rightConfig model.Config, left
195173
}
196174

197175
if len(diff) > 0 {
198-
appendDiffResults(string(diffType), string(COMPONENT), []string{leftGroup.Name, leftConfig.Key}, diff, diffResult)
176+
appendDiffResults(string(diffType), string(COMPONENT),
177+
[]string{leftGroup.Name, leftConfig.Key}, diff, diffResult)
199178
}
200179
}
201180

@@ -224,6 +203,18 @@ func compareAndUpdateString(left string, right string, diffFound bool, modelDiff
224203
return diffFound
225204
}
226205

206+
func compareAndUpdateArray(left []string, right []string, diffFound bool, modelDiffFound *[]string) bool {
207+
// Arrays are optional and will only evaluate if right is not empty
208+
slices.Sort(left)
209+
slices.Sort(right)
210+
211+
if slices.Compare(left, right) != 0 {
212+
diffFound = true
213+
*modelDiffFound = right
214+
}
215+
return diffFound
216+
}
217+
227218
func appendDiffResults(action string, diff string, path []string, content any, diffResult *model.DiffResult) {
228219
diffResult.Changes = append(diffResult.Changes, model.DiffDetails{
229220
Action: action,

‎src/core/comparator_test.go‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
)
1010

1111
const DEFAULT_JSON = "../../resources/fixtures/comparator/default.json"
12+
const STAGING_JSON = "../../resources/fixtures/comparator/staging.json"
1213

1314
func TestCheckGroupSnapshot(t *testing.T) {
1415
c := NewComparatorService()
@@ -492,10 +493,10 @@ func TestCheckStrategySnapshot(t *testing.T) {
492493
]}`, utils.ToJsonFromObject(actual))
493494
})
494495

495-
t.Run("Should return new strategy value", func(t *testing.T) {
496+
t.Run("Should return changes in strategy value (new)", func(t *testing.T) {
496497
// Given
497498
jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)
498-
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_strategy_value.json")
499+
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy_value_new.json")
499500
fromApi := c.NewSnapshotFromJson([]byte(jsonApi))
500501
fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo))
501502

@@ -509,24 +510,27 @@ func TestCheckStrategySnapshot(t *testing.T) {
509510
assert.JSONEq(t, `{
510511
"changes": [
511512
{
512-
"action": "NEW",
513-
"diff": "STRATEGY_VALUE",
513+
"action": "CHANGED",
514+
"diff": "STRATEGY",
514515
"path": [
515516
"Release 1",
516517
"MY_SWITCHER_1",
517518
"VALUE_VALIDATION"
518519
],
519-
"content": [
520-
"user_2"
521-
]
520+
"content": {
521+
"values": [
522+
"user_1",
523+
"user_2"
524+
]
525+
}
522526
}
523527
]}`, utils.ToJsonFromObject(actual))
524528
})
525529

526-
t.Run("Should return deleted strategy value", func(t *testing.T) {
530+
t.Run("Should return changes in strategy value (replaced)", func(t *testing.T) {
527531
// Given
528532
jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)
529-
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_strategy_value.json")
533+
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy_value.json")
530534
fromApi := c.NewSnapshotFromJson([]byte(jsonApi))
531535
fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo))
532536

@@ -540,19 +544,56 @@ func TestCheckStrategySnapshot(t *testing.T) {
540544
assert.JSONEq(t, `{
541545
"changes": [
542546
{
543-
"action": "DELETED",
544-
"diff": "STRATEGY_VALUE",
547+
"action": "CHANGED",
548+
"diff": "STRATEGY",
545549
"path": [
546550
"Release 1",
547551
"MY_SWITCHER_1",
548552
"VALUE_VALIDATION"
549553
],
550-
"content": [
551-
"user_1"
552-
]
554+
"content": {
555+
"values": [
556+
"user_2"
557+
]
558+
}
559+
}
560+
]}`, utils.ToJsonFromObject(actual))
561+
})
562+
563+
t.Run("Should return changes in strategy value (new, deleted)", func(t *testing.T) {
564+
// Given
565+
jsonApi := utils.ReadJsonFromFile(STAGING_JSON)
566+
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy_value_new_deleted.json")
567+
fromApi := c.NewSnapshotFromJson([]byte(jsonApi))
568+
fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo))
569+
570+
// Test Check/Merge changes
571+
diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED)
572+
diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW)
573+
diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED)
574+
actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted})
575+
576+
assert.NotNil(t, actual)
577+
assert.JSONEq(t, `{
578+
"changes": [
579+
{
580+
"action": "CHANGED",
581+
"diff": "STRATEGY",
582+
"path": [
583+
"Release 1",
584+
"MY_SWITCHER_1",
585+
"VALUE_VALIDATION"
586+
],
587+
"content": {
588+
"values": [
589+
"user_1",
590+
"user_3"
591+
]
592+
}
553593
}
554594
]}`, utils.ToJsonFromObject(actual))
555595
})
596+
556597
}
557598

558599
func TestCheckComponentSnapshot(t *testing.T) {

0 commit comments

Comments
 (0)