From af52e8e3aa62986515b4dc30c9c85abc79ffc8a8 Mon Sep 17 00:00:00 2001 From: Tyson Warner Date: Thu, 22 Mar 2018 14:16:39 -0400 Subject: [PATCH 1/5] Add removals from array operations in reverse order of indices to prevent index out of bounds when applying patch --- jsonpatch.go | 31 ++++++++++++++++++-- jsonpatch_array_test.go | 65 +++++++++++++++++++++++++++++++---------- 2 files changed, 77 insertions(+), 19 deletions(-) diff --git a/jsonpatch.go b/jsonpatch.go index a727bc6..11c3b48 100644 --- a/jsonpatch.go +++ b/jsonpatch.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "reflect" + "sort" "strings" ) @@ -83,12 +84,12 @@ func matchesValue(av, bv interface{}) bool { return true } case float64: - bt, ok := bv.(float64) + bt, ok := bv.(float64) if ok && bt == at { return true } case bool: - bt, ok := bv.(bool) + bt, ok := bv.(bool) if ok && bt == at { return true } @@ -170,6 +171,7 @@ func diff(a, b map[string]interface{}, path string, patch []Operation) ([]Operat return nil, err } } + // Now add all deleted values as nil for key := range a { _, found := b[key] @@ -179,6 +181,7 @@ func diff(a, b map[string]interface{}, path string, patch []Operation) ([]Operat patch = append(patch, NewPatch("remove", p, nil)) } } + return patch, nil } @@ -203,7 +206,29 @@ 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)...) + ptc := compareEditDistance(at, bt, p) + + // Gather removals + var removals []Operation + for _, v := range ptc { + if v.Operation == "remove" { + removals = append(removals, v) + } + } + + // Filter out unsorted removals + var filtered []Operation + for _, v := range ptc { + if v.Operation != "remove" { + filtered = append(filtered, v) + } + } + + // Add back removals in sorted order + sort.Sort(sort.Reverse(ByPath(removals))) + + ptc = append(filtered, removals...) + patch = append(patch, ptc...) } else { n := min(len(at), len(bt)) for i := len(at) - 1; i >= n; i-- { diff --git a/jsonpatch_array_test.go b/jsonpatch_array_test.go index 0b862f5..fbff64b 100644 --- a/jsonpatch_array_test.go +++ b/jsonpatch_array_test.go @@ -1,29 +1,30 @@ package jsonpatch import ( - "github.com/stretchr/testify/assert" "sort" "testing" + + "github.com/stretchr/testify/assert" ) var arraySrc = ` { - "spec": { - "loadBalancerSourceRanges": [ - "192.101.0.0/16", - "192.0.0.0/24" - ] - } + "spec": { + "loadBalancerSourceRanges": [ + "192.101.0.0/16", + "192.0.0.0/24" + ] + } } ` var arrayDst = ` { - "spec": { - "loadBalancerSourceRanges": [ - "192.101.0.0/24" - ] - } + "spec": { + "loadBalancerSourceRanges": [ + "192.101.0.0/24" + ] + } } ` @@ -55,14 +56,46 @@ func TestArrayAlmostSame(t *testing.T) { 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, 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") + change = patch[1] 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") +} + +func TestArrayReplacement(t *testing.T) { + src := `{"letters":["A","B","C","D","E"]}` + to := `{"letters":["F","G","H"]}` + patch, e := CreatePatch([]byte(src), []byte(to)) + assert.NoError(t, e) + assert.Equal(t, 5, len(patch), "they should be equal") + + change := patch[0] + assert.Equal(t, change.Operation, "replace", "they should be equal") + assert.Equal(t, change.Path, "/letters/0", "they should be equal") + assert.Equal(t, "F", 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") + assert.Equal(t, change.Operation, "replace", "they should be equal") + assert.Equal(t, change.Path, "/letters/1", "they should be equal") + assert.Equal(t, "G", change.Value, "they should be equal") + + change = patch[2] + assert.Equal(t, change.Operation, "replace", "they should be equal") + assert.Equal(t, change.Path, "/letters/2", "they should be equal") + assert.Equal(t, "H", change.Value, "they should be equal") + + change = patch[3] + assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/4", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") + + change = patch[4] + assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/3", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") } From 2c2eab94434d149f782a695d9d564cfda4eb6d19 Mon Sep 17 00:00:00 2001 From: Tyson Warner Date: Thu, 22 Mar 2018 14:39:14 -0400 Subject: [PATCH 2/5] Update README --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index bbbf729..29a6484 100644 --- a/README.md +++ b/README.md @@ -1,15 +1,15 @@ # jsonpatch -[![Build Status](https://travis-ci.org/appscode/jsonpatch.svg?branch=master)](https://travis-ci.org/appscode/jsonpatch) -[![Go Report Card](https://goreportcard.com/badge/appscode/jsonpatch "Go Report Card")](https://goreportcard.com/report/appscode/jsonpatch) -[![GoDoc](https://godoc.org/github.com/appscode/jsonpatch?status.svg "GoDoc")](https://godoc.org/github.com/appscode/jsonpatch) +[![Build Status](https://travis-ci.org/nylon22/jsonpatch.svg?branch=master)](https://travis-ci.org/nylon22/jsonpatch) +[![Go Report Card](https://goreportcard.com/badge/nylon22/jsonpatch "Go Report Card")](https://goreportcard.com/report/nylon22/jsonpatch) +[![GoDoc](https://godoc.org/github.com/nylon22/jsonpatch?status.svg "GoDoc")](https://godoc.org/github.com/nylon22/jsonpatch) As per http://jsonpatch.com JSON Patch is specified in RFC 6902 from the IETF. JSON Patch allows you to generate JSON that describes changes you want to make to a document, so you don't have to send the whole doc. JSON Patch format is supported by HTTP PATCH method, allowing for standards based partial updates via REST APIs. ```console -go get github.com/appscode/jsonpatch +go get github.com/nylon22/jsonpatch ``` I tried some of the other "jsonpatch" go implementations, but none of them could diff two json documents and @@ -32,7 +32,7 @@ package main import ( "fmt" - "github.com/appscode/jsonpatch" + "github.com/nylon22/jsonpatch" ) var simpleA = `{"a":100, "b":200, "c":"hello"}` From 3569aca19175fd843a12e483e3582a5af9927592 Mon Sep 17 00:00:00 2001 From: Tyson Warner Date: Thu, 22 Mar 2018 15:01:58 -0400 Subject: [PATCH 3/5] Don't iterate twice through ptc --- jsonpatch.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/jsonpatch.go b/jsonpatch.go index 11c3b48..9739523 100644 --- a/jsonpatch.go +++ b/jsonpatch.go @@ -208,18 +208,13 @@ func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation, if isSimpleArray(at) && isSimpleArray(bt) { ptc := compareEditDistance(at, bt, p) - // Gather removals + // Gather removals and non-removals var removals []Operation + var filtered []Operation for _, v := range ptc { if v.Operation == "remove" { removals = append(removals, v) - } - } - - // Filter out unsorted removals - var filtered []Operation - for _, v := range ptc { - if v.Operation != "remove" { + } else { filtered = append(filtered, v) } } From 07d1a9755db72441771dc2b87c8272d33e504a6f Mon Sep 17 00:00:00 2001 From: Tyson Warner Date: Fri, 23 Mar 2018 09:56:18 -0400 Subject: [PATCH 4/5] Fix sort by path index --- jsonpatch.go | 26 +++++++++++++++++++++++- jsonpatch_array_test.go | 44 ++++++++++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/jsonpatch.go b/jsonpatch.go index 9739523..4910b32 100644 --- a/jsonpatch.go +++ b/jsonpatch.go @@ -6,6 +6,7 @@ import ( "fmt" "reflect" "sort" + "strconv" "strings" ) @@ -46,6 +47,25 @@ 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 } +type ByPathIndex []Operation + +func (a ByPathIndex) Len() int { return len(a) } +func (a ByPathIndex) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a ByPathIndex) Less(i, j int) bool { + sp1 := strings.Split(a[i].Path, "/") + sp2 := strings.Split(a[j].Path, "/") + idx1, err := strconv.Atoi(sp1[len(sp1)-1]) + if err != nil { + panic(fmt.Sprintf("expected int. got: %T", sp1[len(sp1)-1])) + } + idx2, err := strconv.Atoi(sp2[len(sp2)-1]) + if err != nil { + panic(fmt.Sprintf("expected int. got: %T", sp2[len(sp2)-1])) + } + + return idx1 < idx2 +} + func NewPatch(operation, path string, value interface{}) Operation { return Operation{Operation: operation, Path: path, Value: value} } @@ -219,9 +239,13 @@ func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation, } } - // Add back removals in sorted order + // Sort alphabetically by path sort.Sort(sort.Reverse(ByPath(removals))) + // Sort numerically by path index + sort.Sort(sort.Reverse(ByPathIndex(removals))) + + // Add back removals in sorted order ptc = append(filtered, removals...) patch = append(patch, ptc...) } else { diff --git a/jsonpatch_array_test.go b/jsonpatch_array_test.go index fbff64b..5c8e004 100644 --- a/jsonpatch_array_test.go +++ b/jsonpatch_array_test.go @@ -68,34 +68,64 @@ func TestArrayAlmostSame(t *testing.T) { } func TestArrayReplacement(t *testing.T) { - src := `{"letters":["A","B","C","D","E"]}` - to := `{"letters":["F","G","H"]}` + src := `{"letters":["A","B","C","D","E","F","G","H","I","J","K"]}` + to := `{"letters":["L","M","N"]}` patch, e := CreatePatch([]byte(src), []byte(to)) assert.NoError(t, e) - assert.Equal(t, 5, len(patch), "they should be equal") + assert.Equal(t, 11, len(patch), "they should be equal") change := patch[0] assert.Equal(t, change.Operation, "replace", "they should be equal") assert.Equal(t, change.Path, "/letters/0", "they should be equal") - assert.Equal(t, "F", change.Value, "they should be equal") + assert.Equal(t, "L", change.Value, "they should be equal") change = patch[1] assert.Equal(t, change.Operation, "replace", "they should be equal") assert.Equal(t, change.Path, "/letters/1", "they should be equal") - assert.Equal(t, "G", change.Value, "they should be equal") + assert.Equal(t, "M", change.Value, "they should be equal") change = patch[2] assert.Equal(t, change.Operation, "replace", "they should be equal") assert.Equal(t, change.Path, "/letters/2", "they should be equal") - assert.Equal(t, "H", change.Value, "they should be equal") + assert.Equal(t, "N", change.Value, "they should be equal") change = patch[3] assert.Equal(t, "remove", change.Operation, "they should be equal") - assert.Equal(t, "/letters/4", change.Path, "they should be equal") + assert.Equal(t, "/letters/10", change.Path, "they should be equal") assert.Equal(t, nil, change.Value, "they should be equal") change = patch[4] assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/9", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") + + change = patch[5] + assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/8", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") + + change = patch[6] + assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/7", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") + + change = patch[7] + assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/6", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") + + change = patch[8] + assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/5", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") + + change = patch[9] + assert.Equal(t, "remove", change.Operation, "they should be equal") + assert.Equal(t, "/letters/4", change.Path, "they should be equal") + assert.Equal(t, nil, change.Value, "they should be equal") + + change = patch[10] + assert.Equal(t, "remove", change.Operation, "they should be equal") assert.Equal(t, "/letters/3", change.Path, "they should be equal") assert.Equal(t, nil, change.Value, "they should be equal") } From 883d2b8639ff76f2804cfa712716d325fd258a2a Mon Sep 17 00:00:00 2001 From: Tyson Warner Date: Fri, 23 Mar 2018 14:19:15 -0400 Subject: [PATCH 5/5] Revert "Update README" This reverts commit 2c2eab94434d149f782a695d9d564cfda4eb6d19. --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 29a6484..bbbf729 100644 --- a/README.md +++ b/README.md @@ -1,15 +1,15 @@ # jsonpatch -[![Build Status](https://travis-ci.org/nylon22/jsonpatch.svg?branch=master)](https://travis-ci.org/nylon22/jsonpatch) -[![Go Report Card](https://goreportcard.com/badge/nylon22/jsonpatch "Go Report Card")](https://goreportcard.com/report/nylon22/jsonpatch) -[![GoDoc](https://godoc.org/github.com/nylon22/jsonpatch?status.svg "GoDoc")](https://godoc.org/github.com/nylon22/jsonpatch) +[![Build Status](https://travis-ci.org/appscode/jsonpatch.svg?branch=master)](https://travis-ci.org/appscode/jsonpatch) +[![Go Report Card](https://goreportcard.com/badge/appscode/jsonpatch "Go Report Card")](https://goreportcard.com/report/appscode/jsonpatch) +[![GoDoc](https://godoc.org/github.com/appscode/jsonpatch?status.svg "GoDoc")](https://godoc.org/github.com/appscode/jsonpatch) As per http://jsonpatch.com JSON Patch is specified in RFC 6902 from the IETF. JSON Patch allows you to generate JSON that describes changes you want to make to a document, so you don't have to send the whole doc. JSON Patch format is supported by HTTP PATCH method, allowing for standards based partial updates via REST APIs. ```console -go get github.com/nylon22/jsonpatch +go get github.com/appscode/jsonpatch ``` I tried some of the other "jsonpatch" go implementations, but none of them could diff two json documents and @@ -32,7 +32,7 @@ package main import ( "fmt" - "github.com/nylon22/jsonpatch" + "github.com/appscode/jsonpatch" ) var simpleA = `{"a":100, "b":200, "c":"hello"}`