diff --git a/restapi/delta_checker.go b/restapi/delta_checker.go index 207a800..f233216 100644 --- a/restapi/delta_checker.go +++ b/restapi/delta_checker.go @@ -53,10 +53,10 @@ func getDelta(recordedResource map[string]interface{}, actualResource map[string modifiedResource[key] = valRecorded } } else if reflect.TypeOf(valRecorded).Kind() == reflect.Slice { - // Since we don't support ignoring differences in lists (besides ignoring the list as a - // whole), it is safe to deep compare the two list values. - if !reflect.DeepEqual(valRecorded, valActual) { - modifiedResource[key] = valActual + // Use reflection to handle slice comparison with ignoreList support + modifiedSlice, sliceHasChanges := compareSlicesWithIgnoreListReflection(valRecorded, valActual, key, ignoreList) + if sliceHasChanges { + modifiedResource[key] = modifiedSlice hasChanges = true } else { modifiedResource[key] = valRecorded @@ -92,12 +92,68 @@ func getDelta(recordedResource map[string]interface{}, actualResource map[string return modifiedResource, hasChanges } +/* + * Compares two slices with ignoreList support for map elements within the slice using reflection. + * Returns the modified slice and a boolean indicating if there were changes. + */ +func compareSlicesWithIgnoreListReflection(recordedResource interface{}, actualResource interface{}, key string, ignoreList []string) (interface{}, bool) { + recordedValue := reflect.ValueOf(recordedResource) + actualValue := reflect.ValueOf(actualResource) + + // Verify both are slices + if recordedValue.Kind() != reflect.Slice || actualValue.Kind() != reflect.Slice { + // Fallback to deep comparison if not both slices + return actualResource, !reflect.DeepEqual(recordedResource, actualResource) + } + + // If slices have different lengths, that's always a change + if recordedValue.Len() != actualValue.Len() { + return actualResource, true + } + + hasChanges := false + deeperIgnoreList := _descendIgnoreList(key, ignoreList) + + // Create new slice with same type as recorded + modifiedSlice := reflect.MakeSlice(recordedValue.Type(), recordedValue.Len(), recordedValue.Len()) + + for i := 0; i < recordedValue.Len(); i++ { + recordedElement := recordedValue.Index(i).Interface() + actualElement := actualValue.Index(i).Interface() + + // Check if both elements are maps + mapRecorded, okRecorded := recordedElement.(map[string]interface{}) + mapActual, okActual := actualElement.(map[string]interface{}) + + if okRecorded && okActual { + // Both elements are maps, use getDelta recursively + modifiedElement, elementHasChanges := getDelta(mapRecorded, mapActual, deeperIgnoreList) + if elementHasChanges { + modifiedSlice.Index(i).Set(reflect.ValueOf(modifiedElement)) + hasChanges = true + } else { + modifiedSlice.Index(i).Set(reflect.ValueOf(recordedElement)) + } + } else { + // At least one element is not a map, use simple comparison + if !reflect.DeepEqual(recordedElement, actualElement) { + modifiedSlice.Index(i).Set(reflect.ValueOf(actualElement)) + hasChanges = true + } else { + modifiedSlice.Index(i).Set(reflect.ValueOf(recordedElement)) + } + } + } + + return modifiedSlice.Interface(), hasChanges +} + /* * Modifies an ignoreList to be relative to a descended path. * E.g. given descendPath = "bar", and the ignoreList [foo, bar.alpha, bar.bravo], this returns [alpha, bravo] */ func _descendIgnoreList(descendPath string, ignoreList []string) []string { - newIgnoreList := make([]string, len(ignoreList)) + var newIgnoreList []string for _, ignorePath := range ignoreList { pathComponents := strings.Split(ignorePath, ".") diff --git a/restapi/delta_checker_test.go b/restapi/delta_checker_test.go index 12cdfeb..ad1f597 100644 --- a/restapi/delta_checker_test.go +++ b/restapi/delta_checker_test.go @@ -248,7 +248,6 @@ var deltaTestCases = []deltaTestCase{ resultHasDelta: false, }, - // We don't currently support ignoring a change like this, but we could in the future with a syntax like `list[].val` similar to jq { testCase: "Server changes a sub-value in a list of objects", o1: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}, {"key": "bar", "val": "x"}}}, @@ -256,6 +255,80 @@ var deltaTestCases = []deltaTestCase{ ignoreList: []string{}, resultHasDelta: true, }, + + // Test slice ignoreList functionality for map elements + { + testCase: "Server changes sub-value in list of objects (ignored)", + o1: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}, {"key": "bar", "val": "x"}}}, + o2: MapAny{"list": []MapAny{{"key": "foo", "val": "Y"}, {"key": "bar", "val": "Z"}}}, + ignoreList: []string{"list.val"}, + resultHasDelta: false, + }, + + { + testCase: "Server changes some sub-values in list of objects (partial ignore)", + o1: MapAny{"list": []MapAny{{"key": "foo", "val": "x", "other": "a"}, {"key": "bar", "val": "x", "other": "b"}}}, + o2: MapAny{"list": []MapAny{{"key": "foo", "val": "Y", "other": "A"}, {"key": "bar", "val": "Z", "other": "B"}}}, + ignoreList: []string{"list.val"}, + resultHasDelta: true, + }, + + { + testCase: "Server adds field to objects in list (ignored)", + o1: MapAny{"list": []MapAny{{"key": "foo"}, {"key": "bar"}}}, + o2: MapAny{"list": []MapAny{{"key": "foo", "new": "field1"}, {"key": "bar", "new": "field2"}}}, + ignoreList: []string{"list.new"}, + resultHasDelta: false, + }, + + { + testCase: "Mixed slice types with maps (some ignored)", + o1: MapAny{"list": []interface{}{MapAny{"key": "foo", "val": "x"}, "string_element", 42}}, + o2: MapAny{"list": []interface{}{MapAny{"key": "foo", "val": "Y"}, "different_string", 42}}, + ignoreList: []string{"list.val"}, + resultHasDelta: true, + }, + + // Test different length slices with ignoreList - length changes are always detected + { + testCase: "Server adds element to list (always a change regardless of ignored fields)", + o1: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}}}, + o2: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}, {"val": "ignored_addition"}}}, + ignoreList: []string{"list.val"}, + resultHasDelta: true, + }, + + { + testCase: "Server adds element to list with non-ignored fields", + o1: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}}}, + o2: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}, {"key": "new", "val": "ignored_addition"}}}, + ignoreList: []string{"list.val"}, + resultHasDelta: true, + }, + + { + testCase: "Server removes element from list (always a change regardless of ignored fields)", + o1: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}, {"val": "ignored_removal"}}}, + o2: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}}}, + ignoreList: []string{"list.val"}, + resultHasDelta: true, + }, + + { + testCase: "Server removes element from list with non-ignored fields", + o1: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}, {"key": "removed", "val": "ignored_removal"}}}, + o2: MapAny{"list": []MapAny{{"key": "foo", "val": "x"}}}, + ignoreList: []string{"list.val"}, + resultHasDelta: true, + }, + + { + testCase: "Server adds non-map element to list", + o1: MapAny{"list": []interface{}{MapAny{"key": "foo", "val": "x"}}}, + o2: MapAny{"list": []interface{}{MapAny{"key": "foo", "val": "x"}, "new_string"}}, + ignoreList: []string{"list.val"}, + resultHasDelta: true, + }, } /* @@ -322,6 +395,10 @@ func TestHasDeltaModifiedResource(t *testing.T) { "hunting": "birds", "eating": "plants", }, + "friends": []map[string]interface{}{ + {"name": "Whiskers", "age": 3, "secret": "loves_tuna"}, + {"name": "Mittens", "age": 2, "secret": "hides_toys"}, + }, } actualInput := map[string]interface{}{ @@ -332,6 +409,10 @@ func TestHasDeltaModifiedResource(t *testing.T) { "eating": "plants", "sleeping": "yep", }, + "friends": []map[string]interface{}{ + {"name": "Whiskers", "age": 4, "secret": "loves_salmon", "new_field": "ignored"}, + {"name": "Mittens", "age": 2, "secret": "steals_socks", "new_field": "ignored"}, + }, } expectedOutput := map[string]interface{}{ @@ -341,9 +422,13 @@ func TestHasDeltaModifiedResource(t *testing.T) { "hunting": "birds", "eating": "plants", }, + "friends": []map[string]interface{}{ + {"name": "Whiskers", "age": 4, "secret": "loves_tuna"}, + {"name": "Mittens", "age": 2, "secret": "hides_toys"}, + }, } - ignoreList := []string{"hairball", "hobbies.sleeping", "name"} + ignoreList := []string{"hairball", "hobbies.sleeping", "name", "friends.secret", "friends.new_field"} modified, _ := getDelta(recordedInput, actualInput, ignoreList) if !reflect.DeepEqual(expectedOutput, modified) {