Fix/add duplicate object to array

jsonpatch.go

@@ -224,34 +224,44 @@ func handleValues(av, bv interface{}, p string, patch []JsonPatchOperation) ([]J
 	return patch, nil
 }
 
+// compareArray generates remove and add operations for `av` and `bv`.
 func compareArray(av, bv []interface{}, p string) []JsonPatchOperation {
 	retval := []JsonPatchOperation{}
-	//	var err error
+
+	// Find elements that need to be removed
+	processArray(av, bv, func(i int, value interface{}) {
+		retval = append(retval, NewPatch("remove", makePath(p, i), nil))
+	})
+
+	// Find elements that need to be added.
+	// NOTE we pass in `bv` then `av` so that processArray can find the missing elements.
+	processArray(bv, av, func(i int, value interface{}) {
+		retval = append(retval, NewPatch("add", makePath(p, i), value))
+	})
+
+	return retval
+}
+
+// processArray processes `av` and `bv` calling `applyOp` whenever a value is absent.
+// It keeps track of which indexes have already had `applyOp` called for and automatically skips them so you can process duplicate objects correctly.
+func processArray(av, bv []interface{}, applyOp func(i int, value interface{})) {
+	foundIndexes := make(map[int]struct{}, len(av))
+	reverseFoundIndexes := make(map[int]struct{}, len(av))
 	for i, v := range av {
-		found := false
-		for _, v2 := range bv {
-			if reflect.DeepEqual(v, v2) {
-				found = true
-				break
+		for i2, v2 := range bv {
+			if _, ok := reverseFoundIndexes[i2]; ok {
+				// We already found this index.
+				continue
 			}
-		}
-		if !found {
-			retval = append(retval, NewPatch("remove", makePath(p, i), nil))
-		}
-	}
-
-	for i, v := range bv {
-		found := false
-		for _, v2 := range av {
 			if reflect.DeepEqual(v, v2) {
-				found = true
+				// Mark this index as found since it matches exactly.
+				foundIndexes[i] = struct{}{}
+				reverseFoundIndexes[i2] = struct{}{}
 				break
 			}
 		}
-		if !found {
-			retval = append(retval, NewPatch("add", makePath(p, i), v))
+		if _, ok := foundIndexes[i]; !ok {
+			applyOp(i, v)
 		}
 	}
-
-	return retval
 }

jsonpatch_array_test.go

@@ -0,0 +1,69 @@
+package jsonpatch
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+var (
+	arrayBase = `{
+  "persons": [{"name":"Ed"},{}]
+}`
+
+	arrayUpdated = `{
+  "persons": [{"name":"Ed"},{},{}]
+}`
+)
+
+func TestArrayAddMultipleEmptyObjects(t *testing.T) {
+	patch, e := CreatePatch([]byte(arrayBase), []byte(arrayUpdated))
+	assert.NoError(t, e)
+	t.Log("Patch:", patch)
+	assert.Equal(t, 1, len(patch), "they should be equal")
+	sort.Sort(ByPath(patch))
+
+	change := patch[0]
+	assert.Equal(t, "add", change.Operation, "they should be equal")
+	assert.Equal(t, "/persons/2", change.Path, "they should be equal")
+	assert.Equal(t, map[string]interface{}{}, change.Value, "they should be equal")
+}
+
+func TestArrayRemoveMultipleEmptyObjects(t *testing.T) {
+	patch, e := CreatePatch([]byte(arrayUpdated), []byte(arrayBase))
+	assert.NoError(t, e)
+	t.Log("Patch:", patch)
+	assert.Equal(t, 1, 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, "/persons/2", change.Path, "they should be equal")
+	assert.Equal(t, nil, change.Value, "they should be equal")
+}
+
+var (
+	arrayWithSpacesBase = `{
+	"persons": [{"name":"Ed"},{},{},{"name":"Sally"},{}]
+}`
+
+	arrayWithSpacesUpdated = `{
+  "persons": [{"name":"Ed"},{},{"name":"Sally"},{}]
+}`
+)
+
+// TestArrayRemoveSpaceInbetween tests removing one blank item from a group blanks which is in between non blank items which also end with a blank item. This tests that the correct index is removed
+func TestArrayRemoveSpaceInbetween(t *testing.T) {
+	t.Skip("This test fails. TODO change compareArray algorithm to match by index instead of by object equality")
+	patch, e := CreatePatch([]byte(arrayWithSpacesBase), []byte(arrayWithSpacesUpdated))
+	assert.NoError(t, e)
+	t.Log("Patch:", patch)
+	assert.Equal(t, 1, 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, "/persons/2", change.Path, "they should be equal")
+	assert.Equal(t, nil, change.Value, "they should be equal")
+}