Fix array comparison (#2)

Also uses edit distance to generate optimal seq for simple arrays.

Fixes #1

Author: tamalsaha

.gitignore

@@ -22,3 +22,5 @@ _testmain.go
 *.exe
 *.test
 *.prof
+
+.idea/

jsonpatch.go

@@ -8,20 +8,20 @@ import (
 	"strings"
 )
 
-var errBadJSONDoc = fmt.Errorf("Invalid JSON Document")
+var errBadJSONDoc = fmt.Errorf("invalid JSON Document")
 
-type JsonPatchOperation struct {
+type Operation struct {
 	Operation string      `json:"op"`
 	Path      string      `json:"path"`
 	Value     interface{} `json:"value,omitempty"`
 }
 
-func (j *JsonPatchOperation) Json() string {
+func (j *Operation) Json() string {
 	b, _ := json.Marshal(j)
 	return string(b)
 }
 
-func (j *JsonPatchOperation) MarshalJSON() ([]byte, error) {
+func (j *Operation) MarshalJSON() ([]byte, error) {
 	var b bytes.Buffer
 	b.WriteString("{")
 	b.WriteString(fmt.Sprintf(`"op":"%s"`, j.Operation))
@@ -39,14 +39,14 @@ func (j *JsonPatchOperation) MarshalJSON() ([]byte, error) {
 	return b.Bytes(), nil
 }
 
-type ByPath []JsonPatchOperation
+type ByPath []Operation
 
 func (a ByPath) Len() int           { return len(a) }
 func (a ByPath) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
 func (a ByPath) Less(i, j int) bool { return a[i].Path < a[j].Path }
 
-func NewPatch(operation, path string, value interface{}) JsonPatchOperation {
-	return JsonPatchOperation{Operation: operation, Path: path, Value: value}
+func NewPatch(operation, path string, value interface{}) Operation {
+	return Operation{Operation: operation, Path: path, Value: value}
 }
 
 // CreatePatch creates a patch as specified in http://jsonpatch.com/
@@ -55,7 +55,7 @@ func NewPatch(operation, path string, value interface{}) JsonPatchOperation {
 // The function will return an array of JsonPatchOperations
 //
 // An error will be returned if any of the two documents are invalid.
-func CreatePatch(a, b []byte) ([]JsonPatchOperation, error) {
+func CreatePatch(a, b []byte) ([]Operation, error) {
 	aI := map[string]interface{}{}
 	bI := map[string]interface{}{}
 	err := json.Unmarshal(a, &aI)
@@ -66,7 +66,7 @@ func CreatePatch(a, b []byte) ([]JsonPatchOperation, error) {
 	if err != nil {
 		return nil, errBadJSONDoc
 	}
-	return diff(aI, bI, "", []JsonPatchOperation{})
+	return diff(aI, bI, "", []Operation{})
 }
 
 // Returns true if the values matches (must be json types)
@@ -78,22 +78,25 @@ func matchesValue(av, bv interface{}) bool {
 	}
 	switch at := av.(type) {
 	case string:
-		bt := bv.(string)
-		if bt == at {
+		bt, ok := bv.(string)
+		if ok && bt == at {
 			return true
 		}
 	case float64:
-		bt := bv.(float64)
-		if bt == at {
+		bt, ok  := bv.(float64)
+		if ok && bt == at {
 			return true
 		}
 	case bool:
-		bt := bv.(bool)
-		if bt == at {
+		bt, ok  := bv.(bool)
+		if ok && bt == at {
 			return true
 		}
 	case map[string]interface{}:
-		bt := bv.(map[string]interface{})
+		bt, ok := bv.(map[string]interface{})
+		if !ok {
+			return false
+		}
 		for key := range at {
 			if !matchesValue(at[key], bt[key]) {
 				return false
@@ -106,7 +109,10 @@ func matchesValue(av, bv interface{}) bool {
 		}
 		return true
 	case []interface{}:
-		bt := bv.([]interface{})
+		bt, ok := bv.([]interface{})
+		if !ok {
+			return false
+		}
 		if len(bt) != len(at) {
 			return false
 		}
@@ -148,7 +154,7 @@ func makePath(path string, newPart interface{}) string {
 }
 
 // diff returns the (recursive) difference between a and b as an array of JsonPatchOperations.
-func diff(a, b map[string]interface{}, path string, patch []JsonPatchOperation) ([]JsonPatchOperation, error) {
+func diff(a, b map[string]interface{}, path string, patch []Operation) ([]Operation, error) {
 	for key, bv := range b {
 		p := makePath(path, key)
 		av, ok := a[key]
@@ -157,11 +163,6 @@ func diff(a, b map[string]interface{}, path string, patch []JsonPatchOperation)
 			patch = append(patch, NewPatch("add", p, bv))
 			continue
 		}
-		// If types have changed, replace completely
-		if reflect.TypeOf(av) != reflect.TypeOf(bv) {
-			patch = append(patch, NewPatch("replace", p, bv))
-			continue
-		}
 		// Types are the same, compare values
 		var err error
 		patch, err = handleValues(av, bv, p, patch)
@@ -181,7 +182,12 @@ func diff(a, b map[string]interface{}, path string, patch []JsonPatchOperation)
 	return patch, nil
 }
 
-func handleValues(av, bv interface{}, p string, patch []JsonPatchOperation) ([]JsonPatchOperation, error) {
+func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation, error) {
+	// If types have changed, replace completely
+	if reflect.TypeOf(av) != reflect.TypeOf(bv) {
+		return append(patch, NewPatch("replace", p, bv)), nil
+	}
+
 	var err error
 	switch at := av.(type) {
 	case map[string]interface{}:
@@ -195,16 +201,19 @@ func handleValues(av, bv interface{}, p string, patch []JsonPatchOperation) ([]J
 			patch = append(patch, NewPatch("replace", p, bv))
 		}
 	case []interface{}:
-		bt, ok := bv.([]interface{})
-		if !ok {
-			// array replaced by non-array
-			patch = append(patch, NewPatch("replace", p, bv))
-		} else if len(at) != len(bt) {
-			// arrays are not the same length
-			patch = append(patch, compareArray(at, bt, p)...)
-
+		bt := bv.([]interface{})
+		if isSimpleArray(at) && isSimpleArray(bt) {
+			patch = append(patch, compareEditDistance(at, bt, p)...)
 		} else {
-			for i := range bt {
+			n := min(len(at), len(bt))
+			for i := len(at) - 1; i >= n; i-- {
+				patch = append(patch, NewPatch("remove", makePath(p, i), nil))
+			}
+			for i := n; i < len(bt); i++ {
+				patch = append(patch, NewPatch("add", makePath(p, i), bt[i]))
+			}
+			for i := 0; i < n; i++ {
+				var err error
 				patch, err = handleValues(at[i], bt[i], makePath(p, i), patch)
 				if err != nil {
 					return nil, err
@@ -224,34 +233,97 @@ func handleValues(av, bv interface{}, p string, patch []JsonPatchOperation) ([]J
 	return patch, nil
 }
 
-func compareArray(av, bv []interface{}, p string) []JsonPatchOperation {
-	retval := []JsonPatchOperation{}
-	//	var err error
-	for i, v := range av {
-		found := false
-		for _, v2 := range bv {
-			if reflect.DeepEqual(v, v2) {
-				found = true
-				break
+func isBasicType(a interface{}) bool {
+	switch a.(type) {
+	case string, float64, bool:
+	default:
+		return false
+	}
+	return true
+}
+
+func isSimpleArray(a []interface{}) bool {
+	for i := range a {
+		switch a[i].(type) {
+		case string, float64, bool:
+		default:
+			val := reflect.ValueOf(a[i])
+			if val.Kind() == reflect.Map {
+				for _, k := range val.MapKeys() {
+					av := val.MapIndex(k)
+					if av.Kind() == reflect.Ptr || av.Kind() == reflect.Interface {
+						if av.IsNil() {
+							continue
+						}
+						av = av.Elem()
+					}
+					if av.Kind() != reflect.String && av.Kind() != reflect.Float64 && av.Kind() != reflect.Bool {
+						return false
+					}
+				}
+				return true
 			}
-		}
-		if !found {
-			retval = append(retval, NewPatch("remove", makePath(p, i), nil))
+			return false
 		}
 	}
+	return true
+}
+
+// https://en.wikipedia.org/wiki/Wagner%E2%80%93Fischer_algorithm
+// Adapted from https://github.com/texttheater/golang-levenshtein
+func compareEditDistance(s, t []interface{}, p string) []Operation {
+	m := len(s)
+	n := len(t)
+
+	d := make([][]int, m+1)
+	for i := 0; i <= m; i++ {
+		d[i] = make([]int, n+1)
+		d[i][0] = i
+	}
+	for j := 0; j <= n; j++ {
+		d[0][j] = j
+	}
 
-	for i, v := range bv {
-		found := false
-		for _, v2 := range av {
-			if reflect.DeepEqual(v, v2) {
-				found = true
-				break
+	for j := 1; j <= n; j++ {
+		for i := 1; i <= m; i++ {
+			if reflect.DeepEqual(s[i-1], t[j-1]) {
+				d[i][j] = d[i-1][j-1] // no op required
+			} else {
+				del := d[i-1][j] + 1
+				add := d[i][j-1] + 1
+				rep := d[i-1][j-1] + 1
+				d[i][j] = min(rep, min(add, del))
 			}
 		}
-		if !found {
-			retval = append(retval, NewPatch("add", makePath(p, i), v))
-		}
 	}
 
-	return retval
+	return backtrace(s, t, p, m, n, d)
+}
+
+func min(x int, y int) int {
+	if y < x {
+		return y
+	}
+	return x
+}
+
+func backtrace(s, t []interface{}, p string, i int, j int, matrix [][]int) []Operation {
+	if i > 0 && matrix[i-1][j]+1 == matrix[i][j] {
+		return append(backtrace(s, t, p, i-1, j, matrix), NewPatch("remove", makePath(p, i-1), nil))
+	}
+	if j > 0 && matrix[i][j-1]+1 == matrix[i][j] {
+		return append(backtrace(s, t, p, i, j-1, matrix), NewPatch("add", makePath(p, i), t[j-1]))
+	}
+	if i > 0 && j > 0 && matrix[i-1][j-1]+1 == matrix[i][j] {
+		if isBasicType(s[0]) {
+			return append(backtrace(s, t, p, i-1, j-1, matrix), NewPatch("replace", makePath(p, i-1), t[j-1]))
+		}
+
+		p2, _ := handleValues(s[j-1], t[j-1], makePath(p, i-1), []Operation{})
+		return append(backtrace(s, t, p, i-1, j-1, matrix), p2...)
+	}
+	if i > 0 && j > 0 && matrix[i-1][j-1] == matrix[i][j] {
+		return backtrace(s, t, p, i-1, j-1, matrix)
+	}
+	return []Operation{}
 }

jsonpatch_array_test.go

@@ -0,0 +1,68 @@
+package jsonpatch
+
+import (
+	"github.com/stretchr/testify/assert"
+	"sort"
+	"testing"
+)
+
+var arraySrc = `
+{
+  "spec": {
+    "loadBalancerSourceRanges": [
+      "192.101.0.0/16",
+      "192.0.0.0/24"
+    ]
+  }
+}
+`
+
+var arrayDst = `
+{
+  "spec": {
+    "loadBalancerSourceRanges": [
+      "192.101.0.0/24"
+    ]
+  }
+}
+`
+
+func TestArraySame(t *testing.T) {
+	patch, e := CreatePatch([]byte(arraySrc), []byte(arraySrc))
+	assert.NoError(t, e)
+	assert.Equal(t, len(patch), 0, "they should be equal")
+}
+
+func TestArrayBoolReplace(t *testing.T) {
+	patch, e := CreatePatch([]byte(arraySrc), []byte(arrayDst))
+	assert.NoError(t, e)
+	assert.Equal(t, 2, len(patch), "they should be equal")
+	sort.Sort(ByPath(patch))
+
+	change := patch[0]
+	assert.Equal(t, "replace", change.Operation, "they should be equal")
+	assert.Equal(t, "/spec/loadBalancerSourceRanges/0", change.Path, "they should be equal")
+	assert.Equal(t, "192.101.0.0/24", change.Value, "they should be equal")
+	change = patch[1]
+	assert.Equal(t, change.Operation, "remove", "they should be equal")
+	assert.Equal(t, change.Path, "/spec/loadBalancerSourceRanges/1", "they should be equal")
+	assert.Equal(t, nil, change.Value, "they should be equal")
+}
+
+func TestArrayAlmostSame(t *testing.T) {
+	src := `{"Lines":[1,2,3,4,5,6,7,8,9,10]}`
+	to := `{"Lines":[2,3,4,5,6,7,8,9,10,11]}`
+	patch, e := CreatePatch([]byte(src), []byte(to))
+	assert.NoError(t, e)
+	assert.Equal(t, 2, len(patch), "they should be equal")
+	sort.Sort(ByPath(patch))
+
+	change := patch[0]
+	assert.Equal(t, "remove", change.Operation, "they should be equal")
+	assert.Equal(t, "/Lines/0", change.Path, "they should be equal")
+	assert.Equal(t, nil, change.Value, "they should be equal")
+	change = patch[1]
+	assert.Equal(t, change.Operation, "add", "they should be equal")
+	assert.Equal(t, change.Path, "/Lines/10", "they should be equal")
+	assert.Equal(t, float64(11), change.Value, "they should be equal")
+}

jsonpatch_complex_test.go

@@ -1,9 +1,10 @@
 package jsonpatch
 
 import (
-	"github.com/stretchr/testify/assert"
 	"sort"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var complexBase = `{"a":100, "b":[{"c1":"hello", "d1":"foo"},{"c2":"hello2", "d2":"foo2"} ], "e":{"f":200, "g":"h", "i":"j"}}`

jsonpatch_geojson_test.go

@@ -1,9 +1,10 @@
 package jsonpatch
 
 import (
-	"github.com/stretchr/testify/assert"
 	"sort"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var point = `{"type":"Point", "coordinates":[0.0, 1.0]}`