Skip to content

Improved Strategy comparison logic - removed STRATEGY_VALUE #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
petruki merged 1 commit into from
Sep 29, 2024
Merged

Improved Strategy comparison logic - removed STRATEGY_VALUE #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ...es/comparator/deleted_strategy_value.json → ...es/comparator/changed_strategy_value.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
"strategy": "VALUE_VALIDATION",
"activated": false,
"operation": "EXIST",
"values": []
"values": [
"user_2"
]
}
],
"components": [
Expand Down
0 ...xtures/comparator/new_strategy_value.json → ...omparator/changed_strategy_value_new.json
View file
Open in desktop
File renamed without changes.
50 changes: 50 additions & 0 deletions resources/fixtures/comparator/changed_strategy_value_new_deleted.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
{
"domain": {
"group": [
{
"name": "Release 1",
"description": "Showcase configuration",
"activated": true,
"config": [
{
"key": "MY_SWITCHER_1",
"description": "My first switcher",
"activated": true,
"strategies": [
{
"strategy": "VALUE_VALIDATION",
"activated": false,
"operation": "EXIST",
"values": [
"user_1",
"user_3"
]
}
],
"components": [
"switcher-playground"
]
},
{
"key": "MY_SWITCHER_2",
"description": "",
"activated": false,
"strategies": [],
"components": [
"switcher-playground"
]
},
{
"key": "MY_SWITCHER_3",
"description": "",
"activated": true,
"strategies": [],
"components": [
"benchmark"
]
}
]
}
]
}
}
50 changes: 50 additions & 0 deletions resources/fixtures/comparator/staging.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
{
"domain": {
"group": [
{
"name": "Release 1",
"description": "Showcase configuration",
"activated": true,
"config": [
{
"key": "MY_SWITCHER_1",
"description": "My first switcher",
"activated": true,
"strategies": [
{
"strategy": "VALUE_VALIDATION",
"activated": false,
"operation": "EXIST",
"values": [
"user_1",
"user_2"
]
}
],
"components": [
"switcher-playground"
]
},
{
"key": "MY_SWITCHER_2",
"description": "",
"activated": false,
"strategies": [],
"components": [
"switcher-playground"
]
},
{
"key": "MY_SWITCHER_3",
"description": "",
"activated": true,
"strategies": [],
"components": [
"benchmark"
]
}
]
}
]
}
}
47 changes: 19 additions & 28 deletions src/core/comparator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ const (
CHANGED DiffType = "CHANGED"
DELETED DiffType = "DELETED"

GROUP DiffResult = "GROUP"
CONFIG DiffResult = "CONFIG"
STRATEGY DiffResult = "STRATEGY"
STRATEGY_VALUE DiffResult = "STRATEGY_VALUE"
COMPONENT DiffResult = "COMPONENT"
GROUP DiffResult = "GROUP"
CONFIG DiffResult = "CONFIG"
STRATEGY DiffResult = "STRATEGY"
COMPONENT DiffResult = "COMPONENT"
)

func NewComparatorService() *ComparatorService {
Expand Down Expand Up @@ -148,10 +147,9 @@ func checkStrategyDiff(leftConfig model.Config, rightConfig model.Config, leftGr
if diffType == CHANGED {
diffFound = compareAndUpdateBool(leftStrategy.Activated, rightStrategy.Activated, diffFound, &modelDiffFound.Activated)
diffFound = compareAndUpdateString(leftStrategy.Operation, rightStrategy.Operation, diffFound, &modelDiffFound.Operation)
diffFound = compareAndUpdateArray(leftStrategy.Values, rightStrategy.Values, diffFound, &modelDiffFound.Values)
}

checkValuesDiff(leftStrategy, rightStrategy, leftGroup, leftConfig, diffResult, diffType)

if diffFound {
appendDiffResults(string(diffType), string(STRATEGY),
[]string{leftGroup.Name, leftConfig.Key, leftStrategy.Strategy}, modelDiffFound, diffResult)
Expand All @@ -160,26 +158,6 @@ func checkStrategyDiff(leftConfig model.Config, rightConfig model.Config, leftGr
}
}

func checkValuesDiff(leftStrategy model.Strategy, rightStrategy model.Strategy, leftGroup model.Group, leftConfig model.Config,
diffResult *model.DiffResult, diffType DiffType) {

if len(leftStrategy.Values) == 0 {
return
}

var diff []string
for _, leftValue := range leftStrategy.Values {
if (diffType == NEW || diffType == DELETED) && !slices.Contains(rightStrategy.Values, leftValue) {
diff = append(diff, leftValue)
}
}

if len(diff) > 0 {
appendDiffResults(string(diffType), string(STRATEGY_VALUE),
[]string{leftGroup.Name, leftConfig.Key, leftStrategy.Strategy}, diff, diffResult)
}
}

func checkComponentsDiff(leftConfig model.Config, rightConfig model.Config, leftGroup model.Group,
diffResult *model.DiffResult, diffType DiffType) {

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

if len(diff) > 0 {
appendDiffResults(string(diffType), string(COMPONENT), []string{leftGroup.Name, leftConfig.Key}, diff, diffResult)
appendDiffResults(string(diffType), string(COMPONENT),
[]string{leftGroup.Name, leftConfig.Key}, diff, diffResult)
}
}

Expand Down Expand Up @@ -224,6 +203,18 @@ func compareAndUpdateString(left string, right string, diffFound bool, modelDiff
return diffFound
}

func compareAndUpdateArray(left []string, right []string, diffFound bool, modelDiffFound *[]string) bool {
// Arrays are optional and will only evaluate if right is not empty
slices.Sort(left)
slices.Sort(right)

if slices.Compare(left, right) != 0 {
diffFound = true
*modelDiffFound = right
}
return diffFound
}

func appendDiffResults(action string, diff string, path []string, content any, diffResult *model.DiffResult) {
diffResult.Changes = append(diffResult.Changes, model.DiffDetails{
Action: action,
Expand Down
69 changes: 55 additions & 14 deletions src/core/comparator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
)

const DEFAULT_JSON = "../../resources/fixtures/comparator/default.json"
const STAGING_JSON = "../../resources/fixtures/comparator/staging.json"

func TestCheckGroupSnapshot(t *testing.T) {
c := NewComparatorService()
Expand Down Expand Up @@ -492,10 +493,10 @@ func TestCheckStrategySnapshot(t *testing.T) {
]}`, utils.ToJsonFromObject(actual))
})

t.Run("Should return new strategy value", func(t *testing.T) {
t.Run("Should return changes in strategy value (new)", func(t *testing.T) {
// Given
jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/new_strategy_value.json")
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy_value_new.json")
fromApi := c.NewSnapshotFromJson([]byte(jsonApi))
fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo))

Expand All @@ -509,24 +510,27 @@ func TestCheckStrategySnapshot(t *testing.T) {
assert.JSONEq(t, `{
"changes": [
{
"action": "NEW",
"diff": "STRATEGY_VALUE",
"action": "CHANGED",
"diff": "STRATEGY",
"path": [
"Release 1",
"MY_SWITCHER_1",
"VALUE_VALIDATION"
],
"content": [
"user_2"
]
"content": {
"values": [
"user_1",
"user_2"
]
}
}
]}`, utils.ToJsonFromObject(actual))
})

t.Run("Should return deleted strategy value", func(t *testing.T) {
t.Run("Should return changes in strategy value (replaced)", func(t *testing.T) {
// Given
jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/deleted_strategy_value.json")
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy_value.json")
fromApi := c.NewSnapshotFromJson([]byte(jsonApi))
fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo))

Expand All @@ -540,19 +544,56 @@ func TestCheckStrategySnapshot(t *testing.T) {
assert.JSONEq(t, `{
"changes": [
{
"action": "DELETED",
"diff": "STRATEGY_VALUE",
"action": "CHANGED",
"diff": "STRATEGY",
"path": [
"Release 1",
"MY_SWITCHER_1",
"VALUE_VALIDATION"
],
"content": [
"user_1"
]
"content": {
"values": [
"user_2"
]
}
}
]}`, utils.ToJsonFromObject(actual))
})

t.Run("Should return changes in strategy value (new, deleted)", func(t *testing.T) {
// Given
jsonApi := utils.ReadJsonFromFile(STAGING_JSON)
jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy_value_new_deleted.json")
fromApi := c.NewSnapshotFromJson([]byte(jsonApi))
fromRepo := c.NewSnapshotFromJson([]byte(jsonRepo))

// Test Check/Merge changes
diffChanged := c.CheckSnapshotDiff(fromApi, fromRepo, CHANGED)
diffNew := c.CheckSnapshotDiff(fromRepo, fromApi, NEW)
diffDeleted := c.CheckSnapshotDiff(fromApi, fromRepo, DELETED)
actual := c.MergeResults([]model.DiffResult{diffChanged, diffNew, diffDeleted})

assert.NotNil(t, actual)
assert.JSONEq(t, `{
"changes": [
{
"action": "CHANGED",
"diff": "STRATEGY",
"path": [
"Release 1",
"MY_SWITCHER_1",
"VALUE_VALIDATION"
],
"content": {
"values": [
"user_1",
"user_3"
]
}
}
]}`, utils.ToJsonFromObject(actual))
})

}

func TestCheckComponentSnapshot(t *testing.T) {
Expand Down