Fixes evaluating empty value from repository when syncing API (#42)

Author: petruki

resources/fixtures/comparator/changed_config_missing_description.json

@@ -0,0 +1,48 @@
+{
+  "domain": {
+    "group": [
+      {
+        "name": "Release 1",
+        "description": "Showcase configuration",
+        "activated": true,
+        "config": [
+          {
+            "key": "MY_SWITCHER_1",
+            "activated": false,
+            "strategies": [
+              {
+                "strategy": "VALUE_VALIDATION",
+                "activated": false,
+                "operation": "EXIST",
+                "values": [
+                  "user_1"
+                ]
+              }
+            ],
+            "components": [
+              "switcher-playground"
+            ]
+          },
+          {
+            "key": "MY_SWITCHER_2",
+            "description": "",
+            "activated": false,
+            "strategies": [],
+            "components": [
+              "switcher-playground"
+            ]
+          },
+          {
+            "key": "MY_SWITCHER_3",
+            "description": "",
+            "activated": true,
+            "strategies": [],
+            "components": [
+              "benchmark"
+            ]
+          }
+        ]
+      }
+    ]
+  }
+}

resources/fixtures/comparator/changed_strategy_missing_activated.json

@@ -0,0 +1,48 @@
+{
+  "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",
+                "operation": "EXIST",
+                "values": [
+                  "user_1"
+                ]
+              }
+            ],
+            "components": [
+              "switcher-playground"
+            ]
+          },
+          {
+            "key": "MY_SWITCHER_2",
+            "description": "",
+            "activated": false,
+            "strategies": [],
+            "components": [
+              "switcher-playground"
+            ]
+          },
+          {
+            "key": "MY_SWITCHER_3",
+            "description": "",
+            "activated": true,
+            "strategies": [],
+            "components": [
+              "benchmark"
+            ]
+          }
+        ]
+      }
+    ]
+  }
+}

src/core/comparator.go

@@ -200,15 +200,24 @@ func checkComponentsDiff(leftConfig model.Config, rightConfig model.Config, left
 }
 
 func compareAndUpdateBool(left *bool, right *bool, diffFound bool, modelDiffFound **bool) bool {
-	if *left != *right {
+	// Bool are required and will assume right is equal to left if right is nil
+	// E.g. when Respository (right) has not been set, it will assume the value from the left (API)
+	if right == nil {
+		right = new(bool)
+		*right = *left
+		diffFound = true
+		*modelDiffFound = right
+	} else if *left != *right {
 		diffFound = true
 		*modelDiffFound = right
 	}
+
 	return diffFound
 }
 
 func compareAndUpdateString(left string, right string, diffFound bool, modelDiffFound *string) bool {
-	if left != right {
+	// Strings are optional and will only evaluate if right is not empty
+	if right != "" && left != right {
 		diffFound = true
 		*modelDiffFound = right
 	}

src/core/comparator_test.go

@@ -198,6 +198,36 @@ func TestCheckConfigSnapshot(t *testing.T) {
 		]}`, utils.ToJsonFromObject(actual))
 	})
 
+	t.Run("Should return changes in config - missing description assume not changed", func(t *testing.T) {
+		// Given
+		jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)
+		jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_config_missing_description.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": "CONFIG",
+				"path": [
+					"Release 1",
+					"MY_SWITCHER_1"
+				],
+				"content": {
+					"activated": false
+				}
+			}
+		]}`, utils.ToJsonFromObject(actual))
+	})
+
 	t.Run("Should return changes in config after calling RemoveDeleted", func(t *testing.T) {
 		// Given
 		jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)
@@ -367,6 +397,37 @@ func TestCheckStrategySnapshot(t *testing.T) {
 		]}`, utils.ToJsonFromObject(actual))
 	})
 
+	t.Run("Should return changes in strategy - missing activated assume is changed", func(t *testing.T) {
+		// Given
+		jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)
+		jsonRepo := utils.ReadJsonFromFile("../../resources/fixtures/comparator/changed_strategy_missing_activated.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": {
+					"activated": false
+				}
+			}
+		]}`, utils.ToJsonFromObject(actual))
+	})
+
 	t.Run("Should return new strategy", func(t *testing.T) {
 		// Given
 		jsonApi := utils.ReadJsonFromFile(DEFAULT_JSON)