Merge pull request #36 from howardjohn/v2/improvements

Various improvements to performance and stability

.github/workflows/ci.yml

@@ -12,7 +12,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v2
         with:
-          go-version: 1.17
+          go-version: '1.20'
 
       - name: Build
         run: |

v2/bench_test.go

@@ -0,0 +1,56 @@
+package jsonpatch_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"gomodules.xyz/jsonpatch/v2"
+)
+
+func BenchmarkCreatePatch(b *testing.B) {
+	cases := []struct {
+		name string
+		a, b string
+	}{
+		{
+			"complex",
+			superComplexBase,
+			superComplexA,
+		},
+		{
+			"large array",
+			largeArray(1000),
+			largeArray(1000),
+		},
+		{
+			"simple",
+			simpleA,
+			simpleB,
+		},
+	}
+
+	for _, tt := range cases {
+		b.Run(tt.name, func(b *testing.B) {
+			at := []byte(tt.a)
+			bt := []byte(tt.b)
+			for n := 0; n < b.N; n++ {
+				_, _ = jsonpatch.CreatePatch(at, bt)
+			}
+		})
+	}
+}
+
+func largeArray(size int) string {
+	type nested struct {
+		A, B string
+	}
+	type example struct {
+		Objects []nested
+	}
+	a := example{}
+	for i := 0; i < size; i++ {
+		a.Objects = append(a.Objects, nested{A: "a", B: "b"})
+	}
+	res, _ := json.Marshal(a)
+	return string(res)
+}

v2/fuzz_test.go

@@ -0,0 +1,63 @@
+package jsonpatch_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	jp "github.com/evanphx/json-patch"
+	"github.com/stretchr/testify/assert"
+	"gomodules.xyz/jsonpatch/v2"
+)
+
+func FuzzCreatePatch(f *testing.F) {
+	add := func(a, b string) {
+		f.Add([]byte(a), []byte(b))
+	}
+	add(simpleA, simpleB)
+	add(superComplexBase, superComplexA)
+	add(hyperComplexBase, hyperComplexA)
+	add(arraySrc, arrayDst)
+	add(empty, simpleA)
+	add(point, lineString)
+	f.Fuzz(func(t *testing.T, a, b []byte) {
+		checkFuzz(t, a, b)
+	})
+}
+
+func checkFuzz(t *testing.T, src, dst []byte) {
+	t.Logf("Test: %v -> %v", string(src), string(dst))
+	patch, err := jsonpatch.CreatePatch(src, dst)
+	if err != nil {
+		// Ok to error, src or dst may be invalid
+		t.Skip()
+	}
+
+	// Applying library only works with arrays and structs, no primitives
+	// We still do CreatePatch to make sure it doesn't panic
+	if isPrimitive(src) || isPrimitive(dst) {
+		return
+	}
+
+	for _, p := range patch {
+		if p.Path == "" {
+			// json-patch doesn't handle this properly, but it is valid
+			return
+		}
+	}
+
+	data, err := json.Marshal(patch)
+	assert.Nil(t, err)
+
+	t.Logf("Applying patch %v", string(data))
+	p2, err := jp.DecodePatch(data)
+	assert.Nil(t, err)
+
+	d2, err := p2.Apply(src)
+	assert.Nil(t, err)
+
+	assert.JSONEq(t, string(dst), string(d2))
+}
+
+func isPrimitive(data []byte) bool {
+	return data[0] != '{' && data[0] != '['
+}

v2/go.mod

@@ -1,8 +1,14 @@
 module gomodules.xyz/jsonpatch/v2
 
-go 1.12
+go 1.20
 
 require (
 	github.com/evanphx/json-patch v0.5.2
 	github.com/stretchr/testify v1.3.0
 )
+
+require (
+	github.com/davecgh/go-spew v1.1.0 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+)

v2/jsonpatch.go

@@ -1,7 +1,6 @@
 package jsonpatch
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"reflect"
@@ -24,21 +23,28 @@ func (j *Operation) Json() string {
 }
 
 func (j *Operation) MarshalJSON() ([]byte, error) {
-	var b bytes.Buffer
-	b.WriteString("{")
-	b.WriteString(fmt.Sprintf(`"op":"%s"`, j.Operation))
-	b.WriteString(fmt.Sprintf(`,"path":"%s"`, j.Path))
-	// Consider omitting Value for non-nullable operations.
-	if j.Value != nil || j.Operation == "replace" || j.Operation == "add" {
-		v, err := json.Marshal(j.Value)
-		if err != nil {
-			return nil, err
-		}
-		b.WriteString(`,"value":`)
-		b.Write(v)
-	}
-	b.WriteString("}")
-	return b.Bytes(), nil
+	// Ensure for add and replace we emit `value: null`
+	if j.Value == nil && (j.Operation == "replace" || j.Operation == "add") {
+		return json.Marshal(struct {
+			Operation string      `json:"op"`
+			Path      string      `json:"path"`
+			Value     interface{} `json:"value"`
+		}{
+			Operation: j.Operation,
+			Path:      j.Path,
+		})
+	}
+	// otherwise just marshal normally. We cannot literally do json.Marshal(j) as it would be recursively
+	// calling this function.
+	return json.Marshal(struct {
+		Operation string      `json:"op"`
+		Path      string      `json:"path"`
+		Value     interface{} `json:"value,omitempty"`
+	}{
+		Operation: j.Operation,
+		Path:      j.Path,
+		Value:     j.Value,
+	})
 }
 
 type ByPath []Operation
@@ -149,9 +155,6 @@ func makePath(path string, newPart interface{}) string {
 	if path == "" {
 		return "/" + key
 	}
-	if strings.HasSuffix(path, "/") {
-		return path + key
-	}
 	return path + "/" + key
 }
 
@@ -211,22 +214,18 @@ func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation,
 		}
 	case []interface{}:
 		bt := bv.([]interface{})
-		if isSimpleArray(at) && isSimpleArray(bt) {
-			patch = append(patch, compareEditDistance(at, bt, p)...)
-		} else {
-			n := min(len(at), len(bt))
-			for i := len(at) - 1; i >= n; i-- {
-				patch = append(patch, NewOperation("remove", makePath(p, i), nil))
-			}
-			for i := n; i < len(bt); i++ {
-				patch = append(patch, NewOperation("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
-				}
+		n := min(len(at), len(bt))
+		for i := len(at) - 1; i >= n; i-- {
+			patch = append(patch, NewOperation("remove", makePath(p, i), nil))
+		}
+		for i := n; i < len(bt); i++ {
+			patch = append(patch, NewOperation("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
 			}
 		}
 	default:
@@ -235,100 +234,9 @@ func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation,
 	return patch, nil
 }
 
-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
-			}
-			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 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))
-			}
-		}
-	}
-
-	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] {
-		op := NewOperation("remove", makePath(p, i-1), nil)
-		return append([]Operation{op}, backtrace(s, t, p, i-1, j, matrix)...)
-	}
-	if j > 0 && matrix[i][j-1]+1 == matrix[i][j] {
-		op := NewOperation("add", makePath(p, i), t[j-1])
-		return append([]Operation{op}, backtrace(s, t, p, i, j-1, matrix)...)
-	}
-	if i > 0 && j > 0 && matrix[i-1][j-1]+1 == matrix[i][j] {
-		if isBasicType(s[0]) {
-			op := NewOperation("replace", makePath(p, i-1), t[j-1])
-			return append([]Operation{op}, backtrace(s, t, p, i-1, j-1, matrix)...)
-		}
-
-		p2, _ := handleValues(s[i-1], t[j-1], makePath(p, i-1), []Operation{})
-		return append(p2, backtrace(s, t, p, i-1, j-1, matrix)...)
-	}
-	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{}
-}

v2/jsonpatch_test.go

@@ -836,6 +836,15 @@ var (
 }`
 )
 
+var (
+	emptyKeyA = `{"":[0]}`
+	emptyKeyB = `{"":[]}`
+)
+
+var (
+	specialChars = string([]byte{123, 34, 92, 98, 34, 58, 91, 93, 125})
+)
+
 func TestCreatePatch(t *testing.T) {
 	cases := []struct {
 		name string
@@ -881,6 +890,10 @@ func TestCreatePatch(t *testing.T) {
 		{"Array at root", `[{"asdf":"qwerty"}]`, `[{"asdf":"bla"},{"asdf":"zzz"}]`},
 		{"Empty array at root", `[]`, `[{"asdf":"bla"},{"asdf":"zzz"}]`},
 		{"Null Key uses replace operation", nullKeyA, nullKeyB},
+		// empty key
+		{"Empty key", emptyKeyA, emptyKeyB},
+		// special chars
+		{"Special chars", empty, specialChars},
 	}
 
 	for _, c := range cases {