diff --git a/pkg/util/cmputil/reporter.go b/pkg/util/cmputil/reporter.go index c9532d3cc0f..cae35bd8e20 100644 --- a/pkg/util/cmputil/reporter.go +++ b/pkg/util/cmputil/reporter.go @@ -78,6 +78,15 @@ func (r DiffReport) String() string { return b.String() } +// Paths returns the slice of paths of the current DiffReport +func (r DiffReport) Paths() []string { + var result = make([]string, len(r)) + for _, diff := range r { + result = append(result, diff.Path) + } + return result +} + type Diff struct { // Path to the field that has difference separated by period. Array index and key are designated by square brackets. // For example, Annotations[12345].Data.Fields[0].ID @@ -87,15 +96,34 @@ type Diff struct { } func (d *Diff) String() string { - left := d.Left.String() + return fmt.Sprintf("%v:\n\t-: %+v\n\t+: %+v\n", d.Path, describeReflectValue(d.Left), describeReflectValue(d.Right)) +} + +func describeReflectValue(v reflect.Value) interface{} { // invalid reflect.Value is produced when two collections (slices\maps) are compared and one misses value. // This way go-cmp indicates that an element was added\removed from a list. - if !d.Left.IsValid() { - left = "" + if !v.IsValid() { + return "" } - right := d.Right.String() - if !d.Right.IsValid() { - right = "" - } - return fmt.Sprintf("%v:\n\t-: %+v\n\t+: %+v", d.Path, left, right) + return v +} + +// IsAddOperation returns true when +// - Left does not have value and Right has +// - the kind of Left and Right is either reflect.Slice or reflect.Map and the length of Left is less than length of Right +// In all other cases it returns false. +// NOTE: this is applicable to diff of Maps and Slices only +func (d *Diff) IsAddOperation() bool { + return !d.Left.IsValid() && d.Right.IsValid() || (d.Left.Kind() == d.Right.Kind() && // cmp reports adding first element to a nil slice as creation of one and therefore Left is valid and it is nil and right is a new slice + (d.Left.Kind() == reflect.Slice || d.Left.Kind() == reflect.Map) && d.Left.Len() < d.Right.Len()) +} + +// IsDeleteOperation returns true when +// - Right does not have value and Left has +// - the kind of Left and Right is either reflect.Slice or reflect.Map and the length of Right is less than length of Left +// In all other cases it returns false. +// NOTE: this is applicable to diff of Maps and Slices only +func (d *Diff) IsDeleteOperation() bool { + return d.Left.IsValid() && !d.Right.IsValid() || (d.Left.Kind() == d.Right.Kind() && + (d.Left.Kind() == reflect.Slice || d.Left.Kind() == reflect.Map) && d.Left.Len() > d.Right.Len()) } diff --git a/pkg/util/cmputil/reporter_test.go b/pkg/util/cmputil/reporter_test.go new file mode 100644 index 00000000000..1ac3a096c61 --- /dev/null +++ b/pkg/util/cmputil/reporter_test.go @@ -0,0 +1,132 @@ +package cmputil + +import ( + "math/rand" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + ptr "github.com/xorcare/pointer" + + "github.com/grafana/grafana/pkg/util" +) + +type subStruct struct { + Data interface{} +} + +type testStruct struct { + Number float64 + NumberPtr *float64 + Text string + TextPtr *string + Flag bool + FlagPtr *bool + Dict map[float64]float64 + Slice []float64 + SubStruct subStruct + SubStructPtr *subStruct +} + +func testStructDiff(left, right testStruct) DiffReport { + var reporter DiffReporter + ops := make([]cmp.Option, 0, 4) + ops = append(ops, cmp.Reporter(&reporter)) + cmp.Equal(left, right, ops...) + return reporter.Diffs +} + +func TestIsAddedDeleted_Collections(t *testing.T) { + testCases := []struct { + name string + left testStruct + right testStruct + field string + }{ + { + name: "nil vs non-empty slice", + left: testStruct{ + Slice: nil, + }, + right: testStruct{ + Slice: []float64{rand.Float64()}, + }, + field: "Slice", + }, + { + name: "empty vs non-empty slice", + left: testStruct{ + Slice: []float64{}, + }, + right: testStruct{ + Slice: []float64{rand.Float64()}, + }, + field: "Slice", + }, + { + name: "nil vs non-empty map", + left: testStruct{ + Dict: nil, + }, + right: testStruct{ + Dict: map[float64]float64{rand.Float64(): rand.Float64()}, + }, + field: "Slice", + }, + { + name: "empty vs non-empty map", + left: testStruct{ + Dict: map[float64]float64{}, + }, + right: testStruct{ + Dict: map[float64]float64{rand.Float64(): rand.Float64()}, + }, + field: "Slice", + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + left := testCase.left + right := testCase.right + field := testCase.field + t.Run("IsAddOperation=true, IsDeleted=false", func(t *testing.T) { + diff := testStructDiff(left, right) + require.Lenf(t, diff, 1, "diff was expected to have only one field %s but got %v", field, diff.String()) + d := diff[0] + require.Truef(t, d.IsAddOperation(), "diff %v should be treated as Add operation but it wasn't", d) + require.Falsef(t, d.IsDeleteOperation(), "diff %v should not be treated as Delete operation but it was", d) + }) + t.Run("IsDeleted=true, IsAddOperation=false", func(t *testing.T) { + diff := testStructDiff(right, left) + require.Lenf(t, diff, 1, "diff was expected to have only one field %s but got %v", field, diff.String()) + d := diff[0] + require.Truef(t, d.IsDeleteOperation(), "diff %v should be treated as Delete operation but it wasn't", d) + require.Falsef(t, d.IsAddOperation(), "diff %v should not be treated as Delete operation but it was", d) + }) + }) + } + + t.Run("IsAddOperation=false, IsDeleted=false if changes in struct fields", func(t *testing.T) { + left := testStruct{} + right := testStruct{ + Number: rand.Float64(), + NumberPtr: ptr.Float64(rand.Float64()), + Text: util.GenerateShortUID(), + TextPtr: ptr.String(util.GenerateShortUID()), + Flag: true, + FlagPtr: ptr.Bool(true), + SubStruct: subStruct{ + Data: rand.Float64(), + }, + SubStructPtr: &subStruct{Data: rand.Float64()}, + } + + diff := testStructDiff(left, right) + require.Len(t, diff, 8) + for _, d := range diff { + assert.Falsef(t, d.IsAddOperation(), "diff %v was not supposed to be Add operation", d.String()) + assert.Falsef(t, d.IsDeleteOperation(), "diff %v was not supposed to be Delete operation", d.String()) + } + }) +}