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

copy/replace in one patch doc #64

Closed
amarchewka opened this issue Jan 30, 2018 · 4 comments
Closed

copy/replace in one patch doc #64

amarchewka opened this issue Jan 30, 2018 · 4 comments
Assignees

Comments

@amarchewka
Copy link

If I start with the following JSON doc:
{"profiles":{"abc":[],"def":[{"hello":"world"}]}}

and apply the following patch:
[{"op":"copy","from":"/profiles/def/0", "path":"/profiles/def/0"},{"op":"replace","path":"/profiles/def/0/hello","value":"world2"}]

I get the following output:
{"profiles":{"abc":[],"def":[{"hello":"world2"},{"hello":"world2"}]}}

That is, the "replace" operation was applied to both array elements.
(Or possibly, the "replace" was applied before the "copy".)

Applying the "copy" and then the "replace" in 2 separate patch docs works as expected.

The following on-line tool [https://json-schema-validator.herokuapp.com/jsonpatch.jsp]
agrees the original output should have been
{"profiles":{"abc":[],"def":[{"hello":"world2"},{"hello":"world"}]}}
with the "replace" applied only to array element 0.

Thanks for a great library, btw!

@luliu
Copy link

luliu commented Jan 30, 2018

I noticed in InPlaceApplyProcessor's copy function, the same valueNode object is assigned to the toPath:

@Override
public void copy(List<String> fromPath, List<String> toPath) {
JsonNode parentNode = getParentNode(fromPath, Operation.COPY);
String field = fromPath.get(fromPath.size() - 1).replaceAll("\"", "");
JsonNode valueNode = parentNode.isArray() ? parentNode.get(Integer.parseInt(field)) : parentNode.get(field);
add(toPath, valueNode);
}

Is this a bug? As any mutation done to the source JsonNode in the same batch of patch operations will affect the target since they're the same object.

Here's an example:

Original document:

{"top": {"object1": {"key1":"val1","key2":"val2"}, "object2":"-----"}

Patch:

[
{"op":"copy", "from":"/top/object1", "path":"/top/object2"},
{"op":"replace","path":"/top/object1/key1","value":"++++"}
]

Result:

{"top": {"object1": {"key1":"++++","key2":"val2"}, "object2": {"key1":"++++","key2":"val2"}}}

Note that both object1 and object2's key1 changed to ++++.

@vishwakarma
Copy link
Member

@luliu @amarchewka
Agreed, this certainly looks like a bug ( Using copy by reference instead of copy by value ), i will go deeper into the code and will fix the issue by this week.
Thank you for raising the issue.

@vishwakarma vishwakarma self-assigned this Jan 31, 2018
@amarchewka
Copy link
Author

That's great. Thanks!

@vishwakarma
Copy link
Member

The bug is fixed, Closing the issue.

Thanks again for raising the issue.

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

No branches or pull requests

3 participants