Skip to content

Emit the null merge patch when the target is JSON null - #54

Open
youdie006 wants to merge 1 commit into
wI2L:masterfrom
youdie006:mergepatch-null-target
Open

youdie006 wants to merge 1 commit into
wI2L:masterfrom
youdie006:mergepatch-null-target

Conversation

@youdie006

Copy link
Copy Markdown

MergePatch and MergePatchJSON return no patch at all when the target is JSON null, where
RFC 7386 says the patch is null.

patch := mergePatch(si, ti)
if patch == nil {
    return nil, nil
}
return json.Marshal(patch)

The guard reads as "there were no differences", but mergePatch never returns nil for that.
Two equal objects give {}, equal scalars give the scalar. merge.go:49 is its only nil
source:

if src == nil || tgt == nil {
    return tgt
}

So patch == nil holds in exactly one case - the target is null - which is the one case the
guard must not swallow. On master:

MergePatchJSON({"a":"foo"} , null   ) = ""        <- want `null`
MergePatchJSON(null        , null   ) = ""        <- want `null`
MergePatchJSON({"a":"foo"} , "bar"  ) = "bar"
MergePatchJSON({"a":1}     , {"a":1}) = "{}"      <- "no differences" is {}, not nil

An empty patch means "apply nothing", so a caller round-tripping {"a":"foo"} through the patch
keeps {"a":"foo"} where the RFC requires null.

This is already asserted by the repo's own test data - testdata/tests/mergepatch/rfc.json case
#11, {"a":"foo"} -> null => null, straight from RFC 7386 Appendix A. It passes because
merge_test.go:36 calls the unexported mergePatch directly and never goes through the exported
wrappers. The public surface of merge-patch generation is covered only by the two happy-path
examples in example_test.go.

The change

Drop the guard from both wrappers - json.Marshal(nil) already emits null.

Observable change, stated plainly: for a null target these now return []byte("null") and
nil error, where before they returned nil, nil. Anything treating an empty result as "no
change" will see 4 bytes instead. That is the point of the fix, but it is a behaviour change on
that one input. Every other input is byte-identical.

Test

Test_mergePatchNullTarget in merge_test.go, driving MergePatchJSON rather than the
unexported function, with RFC #12 alongside as a control. Reverting only merge.go:

--- FAIL: Test_mergePatchNullTarget/rfc_#11
    merge_test.go:65: got "", want "null"
--- FAIL: Test_mergePatchNullTarget/null_to_null
    merge_test.go:65: got "", want "null"

rfc #12 passes either way, confirming the non-null path is untouched.

CI's own command passes with the change:

go test -race -coverprofile=coverage.txt -covermode=atomic ./...
ok  github.com/wI2L/jsondiff  1.184s  coverage: 97.3% of statements

go vet and gofmt -l clean.


Disclosure: found and prepared with AI assistance (Claude). Every figure above is from a run on
this branch.

mergePatch returns nil only when the target is null, and null is itself
the merge patch (RFC 7386 Appendix A, wI2L#11). The guard read it as an
absence of differences and returned no patch, so a caller applying the
result kept the source document.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant