Skip to content
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

merge_patch: inconsistent behaviour merging empty sub-object #1289

Closed
adam-p opened this issue Oct 10, 2018 · 6 comments
Closed

merge_patch: inconsistent behaviour merging empty sub-object #1289

adam-p opened this issue Oct 10, 2018 · 6 comments
Labels
kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@adam-p
Copy link

adam-p commented Oct 10, 2018

When using merge_patch with an object like {{"k", {}}}, different results are obtained than with other methods for providing an empty object.

Example: https://wandbox.org/permlink/7SDUyC98tSZX7sPL

Like...

start with: {"k":{"a":"b"}}

merge_patch({{"k", json({})}}) --> {"k":{"a":"b"}}

merge_patch({{"k", {}}}) --> {}

It may be that I'm confused about what {} means here. If it's equivalent to null, then I think this makes sense. But I don't see anything in the README about that being the case.

@gregmarr
Copy link
Contributor

JSON Merge Patch is defined here: https://tools.ietf.org/html/rfc7386

 define MergePatch(Target, Patch):
   if Patch is an Object:
     if Target is not an Object:
   Target = {} # Ignore the contents and set it to an empty Object
     for each Name/Value pair in Patch:
   if Value is null:
     if Name exists in Target:
       remove the Name/Value pair from Target
   else:
     Target[Name] = MergePatch(Target[Name], Value)
     return Target
   else:
     return Patch

There are a few things to note about the function. If the patch is
anything other than an object, the result will always be to replace
the entire target with the entire patch. Also, it is not possible to
patch part of a target that is not an object, such as to replace just
some of the values in an array.

The MergePatch operation is defined to operate at the level of data
items, not at the level of textual representation. There is no
expectation that the MergePatch operation will preserve features at
the textual-representation level such as white space, member
ordering, number precision beyond what is available in the target's
implementation, and so forth. In addition, even if the target
implementation allows multiple name/value pairs with the same name,
the result of the MergePatch operation on such objects is not
defined.

@gregmarr
Copy link
Contributor

So I agree that this looks like {} is being treated as null.

@gregmarr
Copy link
Contributor

// create a JSON object
json j = {};

j.merge_patch({{"k", {}}});
std::cout << "{}: " << j.dump() << std::endl << std::endl;

since j is null, I would expect {} here to be null as well.

@nlohmann
Copy link
Owner

Indeed, the code json j = {} is equivalent to json j or json j = nullptr and in all cases j is null.

@nlohmann nlohmann added kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Oct 11, 2018
@nlohmann
Copy link
Owner

@adam-p Do you need further assistance or can we close this issue?

@adam-p
Copy link
Author

adam-p commented Oct 16, 2018

I'm good. Thanks. (Sorry for the delayed response.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

3 participants