Skip to content

Commit

Permalink
fix: operation marshaling with typed nil value
Browse files Browse the repository at this point in the history
  • Loading branch information
wI2L committed Oct 20, 2022
1 parent 5fbf127 commit db0a75e
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 9 deletions.
9 changes: 7 additions & 2 deletions operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package jsondiff
import (
"encoding/json"
"strings"
"unsafe"

"github.com/tidwall/gjson"
)
Expand Down Expand Up @@ -56,10 +57,14 @@ func (jn jsonNull) MarshalJSON() ([]byte, error) {
// MarshalJSON implements the json.Marshaler interface.
func (o Operation) MarshalJSON() ([]byte, error) {
type op Operation

if !o.marshalWithValue() {
o.Value = nil
} else if o.Value == nil {
o.Value = jsonNull{}
} else {
// Generic check that passes for nil and type nil interface values.
if (*[2]uintptr)(unsafe.Pointer(&o.Value))[1] == 0 {
o.Value = jsonNull{}
}
}
if !o.hasFrom() {
o.From = emptyPtr
Expand Down
46 changes: 44 additions & 2 deletions operation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ func TestOperationMarshalJSON(t *testing.T) {
Op Operation
Out string // marshaled output
}{
// Replace operations should ALWAYS be marshaled
// Replace and add operations should ALWAYS be marshaled
// with a value, even if it is null (override omitempty).
{
Operation{
Expand All @@ -17,6 +17,14 @@ func TestOperationMarshalJSON(t *testing.T) {
},
`{"op":"replace","path":"/foo/bar","value":null}`,
},
{
Operation{
Type: OperationReplace,
Path: "/foo/bar",
Value: typeNilIface(),
},
`{"op":"replace","path":"/foo/bar","value":null}`,
},
{
Operation{
Type: OperationReplace,
Expand All @@ -25,6 +33,31 @@ func TestOperationMarshalJSON(t *testing.T) {
},
`{"op":"replace","path":"/foo/bar","value":"foo"}`,
},
{
// assigned interface
Operation{
Type: OperationAdd,
Path: "",
Value: nil,
},
`{"op":"add","path":"","value":null}`,
},
{
// unassigned interface Value
Operation{
Type: OperationAdd,
Path: "",
},
`{"op":"add","path":"","value":null}`,
},
{
Operation{
Type: OperationAdd,
Path: "",
Value: typeNilIface(),
},
`{"op":"add","path":"","value":null}`,
},
{
// Remove operation should NEVER be marshaled with
// a value.
Expand Down Expand Up @@ -101,6 +134,15 @@ func TestNilPatchString(t *testing.T) {
s := p.String()

if s != "" {
t.Errorf("stringified operation mismatch, got %q, wantempty string", s)
t.Errorf("stringified operation mismatch, got %q, want empty string", s)
}
}

func typeNilIface() interface{} {
var i *int
var p interface{}

p = i

return p
}
10 changes: 5 additions & 5 deletions testdata/tests/root.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,21 +130,21 @@
"before": "foobar",
"after": null,
"patch": [
{ "op": "add", "path": "" }
{ "op": "add", "path": "", "value": null }
]
}, {
"name": "remove document (number)",
"before": 1337,
"after": null,
"patch": [
{ "op": "add", "path": "" }
{ "op": "add", "path": "", "value": null }
]
}, {
"name": "remove document (boolean)",
"before": true,
"after": null,
"patch": [
{ "op": "add", "path": "" }
{ "op": "add", "path": "", "value": null }
]
}, {
"name": "remove document (object)",
Expand All @@ -154,7 +154,7 @@
},
"after": null,
"patch": [
{ "op": "add", "path": "" }
{ "op": "add", "path": "", "value": null }
]
}, {
"name": "remove document (array)",
Expand All @@ -163,7 +163,7 @@
],
"after": null,
"patch": [
{ "op": "add", "path": "" }
{ "op": "add", "path": "", "value": null }
]
}, {
"name": "replace array with object",
Expand Down

0 comments on commit db0a75e

Please sign in to comment.