forked from mattbaird/jsonpatch
-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various improvements to performance and stability #36
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
fc9e957
Update to Go 1.20
howardjohn a81e531
Go.mod for go 1.20
howardjohn 87f8876
add benchmarks
howardjohn 4844bbf
Add fuzzing test
howardjohn 471fa30
Fix CI
howardjohn 1f7eb13
Improve fuzz tests
howardjohn 2c05a51
Fix inputs with empty keys
howardjohn c616a48
Fix marshaling invalid chars
howardjohn e0f6c24
Fix known broken case
howardjohn 45186ed
(hackily) remove inefficient optimization
howardjohn 6d5c3df
Fully remove dead code
howardjohn File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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] != '[' | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this part not needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test case for this is:
The patch should be
Before this change, we would have the path
/0
. which was not correct.I am not sure why it was originally added. Removal of this code did not impact any existing test cases, and the fuzz tester should verify we round-trip successfully. So I think the only case this code was hit was for the empty key edge case