Skip to content

Commit f4417d2

Browse files
committed
fix(compareArray): fix adding duplicate items to arrays not generating a add operation
Previously it was only checked if an object was in an array. We now keep track of indexes of objects that have been found and only allow an object to be found once. This fixes adding duplicate arrays because the objects are essentially seen as being different now.
1 parent d220c21 commit f4417d2

File tree

2 files changed

+56
-5
lines changed

2 files changed

+56
-5
lines changed

jsonpatch.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,10 @@ func handleValues(av, bv interface{}, p string, patch []JsonPatchOperation) ([]J
214214

215215
func compareArray(av, bv []interface{}, p string) []JsonPatchOperation {
216216
retval := []JsonPatchOperation{}
217-
// var err error
217+
218218
for i, v := range av {
219219
found := false
220+
// NOTE it is fine to range over all of bv since we are just removing elements.
220221
for _, v2 := range bv {
221222
if reflect.DeepEqual(v, v2) {
222223
found = true
@@ -227,14 +228,27 @@ func compareArray(av, bv []interface{}, p string) []JsonPatchOperation {
227228
}
228229
}
229230

231+
// Find elements that need to be added.
232+
// NOTE we keep track of which indexes we've already found so that duplicate objects work correctly.
233+
foundIndexes := make(map[int]int, len(bv))
230234
for i, v := range bv {
231-
found := false
232-
for _, v2 := range av {
235+
for i2, v2 := range av {
236+
alreadyFoundThisIndex := true
237+
for _, foundI2 := range foundIndexes {
238+
if foundI2 == i2 {
239+
alreadyFoundThisIndex = false
240+
break
241+
}
242+
}
243+
if !alreadyFoundThisIndex {
244+
continue
245+
}
233246
if reflect.DeepEqual(v, v2) {
234-
found = true
247+
foundIndexes[i] = i2
248+
break
235249
}
236250
}
237-
if !found {
251+
if _, ok := foundIndexes[i]; !ok {
238252
retval = append(retval, NewPatch("add", makePath(p, i), v))
239253
}
240254
}

jsonpatch_array_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package jsonpatch
2+
3+
import (
4+
"sort"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
var arrayBase = `{
11+
"persons": [{"name":"Ed"},{}]
12+
}`
13+
14+
var arrayUpdated = `{
15+
"persons": [{"name":"Ed"},{},{}]
16+
}`
17+
18+
func TestArrayAddMultipleEmptyObjects(t *testing.T) {
19+
patch, e := CreatePatch([]byte(arrayBase), []byte(arrayUpdated))
20+
assert.NoError(t, e)
21+
t.Log("Patch:", patch)
22+
assert.Equal(t, 1, len(patch), "they should be equal")
23+
sort.Sort(ByPath(patch))
24+
25+
change := patch[0]
26+
assert.Equal(t, "add", change.Operation, "they should be equal")
27+
assert.Equal(t, "/persons/2", change.Path, "they should be equal")
28+
assert.Equal(t, map[string]interface{}{}, change.Value, "they should be equal")
29+
// change = patch[1]
30+
// assert.Equal(t, "add", change.Operation, "they should be equal")
31+
// assert.Equal(t, "/goods/2/batters/batter/2", change.Path, "they should be equal")
32+
// assert.Equal(t, map[string]interface{}{"id": "1003", "type": "Vanilla"}, change.Value, "they should be equal")
33+
// change = patch[2]
34+
// assert.Equal(t, change.Operation, "remove", "they should be equal")
35+
// assert.Equal(t, change.Path, "/goods/2/topping/2", "they should be equal")
36+
// assert.Equal(t, nil, change.Value, "they should be equal")
37+
}

0 commit comments

Comments
 (0)