flipkart-incubator / zjsonpatch

This is an implementation of RFC 6902 JSON Patch written in Java
Apache License 2.0
523 stars 148 forks source link

Diff then patch fails on certain json #116

Open robseed opened 4 years ago

robseed commented 4 years ago

Expected Behavior

If JsonDiff.asJson succeeds then JsonPatch.apply should succeed

Actual Behavior

JsonPatch.apply fails with [MOVE Operation] Missing field

Steps to Reproduce the Problem

ObjectMapper mapper = new ObjectMapper();
JsonNode source = mapper.readTree("{\"16\":[{\"id\":\"one\",\"type\":\"page\",\"length\":1}],\"63\":[{\"id\":\"two\",\"length\":2}],\"76\":[{\"id\":\"three\",\"length\":3}]}");
JsonNode target = mapper.readTree("{\"16\":[{\"id\":\"four\",\"length\":4}],\"76\":[{\"id\":\"five\",\"type\":\"page\",\"length\":5}]}");
JsonNode diff = JsonDiff.asJson(source, target);
JsonPatch.apply(diff, source);

[MOVE Operation] Missing field "77" at root
    at com.flipkart.zjsonpatch.JsonPatch.process(JsonPatch.java:112)
    at com.flipkart.zjsonpatch.JsonPatch.apply(JsonPatch.java:127)
    at com.flipkart.zjsonpatch.JsonPatch.apply(JsonPatch.java:132)

Specifications

The json looks contrived, but it is a real world example from comparing facebook posts. Java 1.8, Jackson 2.9.9, zjsonpatch 0.4.9

holograph commented 4 years ago

Error reproduced, I'm looking into it. Especially with the error message it is indeed quite strange.

holograph commented 4 years ago

OK, the problem has to do with JSON Pointer semantics. When processing a list of diffs to introduce move ops, the assumption is the a numerical path segment is an array index (which means it has to be taken into account in adjusting the resulting paths).

That is incorrect: JSON Pointer doesn't actually know whether a path segment refers to array or object. For example, the path /0/abc will correctly evaluate against both of these documents: {"0": {"abc": true}} and [{"abc":true}].

Solving this requires modifying JsonDiff.introduceMoveOperation and JsonDiff.updatePath to take the source document into account; I'm working on it, but it'll take a bit of time.