Add EMIT_TEST_OPERATIONS diff flag (supports #91) (#92)

* Security upgrade (see FasterXML/jackson-databind#2186)

* Minor cleanup

* Further cleanup

* Added DiffFlags.EMIT_TEST_OPERATIONS, along with associated tests and functionality

* Further (minor) cleanup

* Corrected @SInCE version on EMIT_COPY_OPERATIONS

Author: holograph

src/main/java/com/flipkart/zjsonpatch/DiffFlags.java

@@ -28,13 +28,30 @@ public enum DiffFlags {
     OMIT_COPY_OPERATION,
 
     /**
-     * This flag adds a <i>fromValue</i> field to all {@link Operation#REPLACE}operations.
+     * This flag adds a <i>fromValue</i> field to all {@link Operation#REPLACE} operations.
      * <i>fromValue</i> represents the the value replaced by a {@link Operation#REPLACE}
-     * operation, in other words, the original value.
+     * operation, in other words, the original value. This can be useful for debugging
+     * output or custom processing of the diffs by downstream systems.
+     *
+     * Please note that this is a non-standard extension to RFC 6902 and will not affect
+     * how patches produced by this library are processed by this or other libraries.
      *
      * @since 0.4.1
      */
-    ADD_ORIGINAL_VALUE_ON_REPLACE;
+    ADD_ORIGINAL_VALUE_ON_REPLACE,
+
+    /**
+     * This flag instructs the diff generator to emit {@link Operation#TEST} operations
+     * that validate the state of the source document before each mutation. This can be
+     * useful if you want to ensure data integrity prior to applying the patch.
+     *
+     * The resulting patches are standard per RFC 6902 and should be processed correctly
+     * by any compliant library; due to the associated space and performance costs,
+     * however, this isn't default behavior.
+     *
+     * @since 0.4.8
+     */
+    EMIT_TEST_OPERATIONS;
 
 
     public static EnumSet<DiffFlags> defaults() {

src/main/java/com/flipkart/zjsonpatch/JsonDiff.java

@@ -31,52 +31,55 @@
 
 public final class JsonDiff {
 
-    private JsonDiff() {
+    private final List<Diff> diffs = new ArrayList<Diff>();
+    private final EnumSet<DiffFlags> flags;
 
+    private JsonDiff(EnumSet<DiffFlags> flags) {
+        this.flags = flags.clone();
     }
 
     public static JsonNode asJson(final JsonNode source, final JsonNode target) {
         return asJson(source, target, DiffFlags.defaults());
     }
 
     public static JsonNode asJson(final JsonNode source, final JsonNode target, EnumSet<DiffFlags> flags) {
-        final List<Diff> diffs = new ArrayList<Diff>();
-        List<Object> path = new ArrayList<Object>(0);
+        JsonDiff diff = new JsonDiff(flags);
 
         // generating diffs in the order of their occurrence
+        List<Object> path = new ArrayList<Object>(0);
+        diff.generateDiffs(path, source, target);
 
-        generateDiffs(diffs, path, source, target);
-
-        if (!flags.contains(DiffFlags.OMIT_MOVE_OPERATION)) {
-
+        if (!flags.contains(DiffFlags.OMIT_MOVE_OPERATION))
             // Merging remove & add to move operation
+            diff.introduceMoveOperation();
 
-            compactDiffs(diffs);
-        }
-
-        if (!flags.contains(DiffFlags.OMIT_COPY_OPERATION)) {
-
+        if (!flags.contains(DiffFlags.OMIT_COPY_OPERATION))
             // Introduce copy operation
+            diff.introduceCopyOperation(source, target);
 
-            introduceCopyOperation(source, target, diffs);
-        }
-
-        return getJsonNodes(diffs, flags);
+        return diff.getJsonNodes();
     }
 
     private static List<Object> getMatchingValuePath(Map<JsonNode, List<Object>> unchangedValues, JsonNode value) {
         return unchangedValues.get(value);
     }
 
-    private static void introduceCopyOperation(JsonNode source, JsonNode target, List<Diff> diffs) {
+    private void introduceCopyOperation(JsonNode source, JsonNode target) {
         Map<JsonNode, List<Object>> unchangedValues = getUnchangedPart(source, target);
+
         for (int i = 0; i < diffs.size(); i++) {
             Diff diff = diffs.get(i);
-            if (Operation.ADD == diff.getOperation()) {
-                List<Object> matchingValuePath = getMatchingValuePath(unchangedValues, diff.getValue());
-                if (matchingValuePath != null && isAllowed(matchingValuePath, diff.getPath())) {
-                    diffs.set(i, new Diff(Operation.COPY, matchingValuePath, diff.getPath()));
+            if (Operation.ADD != diff.getOperation()) continue;
+
+            List<Object> matchingValuePath = getMatchingValuePath(unchangedValues, diff.getValue());
+            if (matchingValuePath != null && isAllowed(matchingValuePath, diff.getPath())) {
+                // Matching value found; replace add with copy
+                if (flags.contains(DiffFlags.EMIT_TEST_OPERATIONS)) {
+                    // Prepend test node
+                    diffs.add(i, new Diff(Operation.TEST, matchingValuePath, diff.getValue()));
+                    i++;
                 }
+                diffs.set(i, new Diff(Operation.COPY, matchingValuePath, diff.getPath()));
             }
         }
     }
@@ -171,7 +174,7 @@ private static void computeObject(Map<JsonNode, List<Object>> unchangedValues, L
      * This method merge 2 diffs ( remove then add, or vice versa ) with same value into one Move operation,
      * all the core logic resides here only
      */
-    private static void compactDiffs(List<Diff> diffs) {
+    private void introduceMoveOperation() {
         for (int i = 0; i < diffs.size(); i++) {
             Diff diff1 = diffs.get(i);
 
@@ -234,7 +237,7 @@ private static void updatePathWithCounters(List<Integer> counters, List<Object>
         for (int i = 0; i < counters.size(); i++) {
             int value = counters.get(i);
             if (value != 0) {
-                Integer currValue = Integer.parseInt(path.get(i).toString());
+                int currValue = Integer.parseInt(path.get(i).toString());
                 path.set(i, String.valueOf(currValue + value));
             }
         }
@@ -270,7 +273,7 @@ private static void updateCounters(Diff pseudo, int idx, List<Integer> counters)
         }
     }
 
-    private static ArrayNode getJsonNodes(List<Diff> diffs, EnumSet<DiffFlags> flags) {
+    private ArrayNode getJsonNodes() {
         JsonNodeFactory FACTORY = JsonNodeFactory.instance;
         final ArrayNode patch = FACTORY.arrayNode();
         for (Diff diff : diffs) {
@@ -315,26 +318,27 @@ private static ObjectNode getJsonNode(JsonNodeFactory FACTORY, Diff diff, EnumSe
         return jsonNode;
     }
 
-    private static void generateDiffs(List<Diff> diffs, List<Object> path, JsonNode source, JsonNode target) {
+    private void generateDiffs(List<Object> path, JsonNode source, JsonNode target) {
         if (!source.equals(target)) {
             final NodeType sourceType = NodeType.getNodeType(source);
             final NodeType targetType = NodeType.getNodeType(target);
 
             if (sourceType == NodeType.ARRAY && targetType == NodeType.ARRAY) {
                 //both are arrays
-                compareArray(diffs, path, source, target);
+                compareArray(path, source, target);
             } else if (sourceType == NodeType.OBJECT && targetType == NodeType.OBJECT) {
                 //both are json
-                compareObjects(diffs, path, source, target);
+                compareObjects(path, source, target);
             } else {
                 //can be replaced
-
+                if (flags.contains(DiffFlags.EMIT_TEST_OPERATIONS))
+                    diffs.add(new Diff(Operation.TEST, path, source));
                 diffs.add(Diff.generateDiff(Operation.REPLACE, path, source, target));
             }
         }
     }
 
-    private static void compareArray(List<Diff> diffs, List<Object> path, JsonNode source, JsonNode target) {
+    private void compareArray(List<Object> path, JsonNode source, JsonNode target) {
         List<JsonNode> lcs = getLCS(source, target);
         int srcIdx = 0;
         int targetIdx = 0;
@@ -365,12 +369,14 @@ private static void compareArray(List<Diff> diffs, List<Object> path, JsonNode s
                 } else if (lcsNode.equals(targetNode)) { //targetNode node is same as lcs, but not src
                     //removal,
                     List<Object> currPath = getPath(path, pos);
+                    if (flags.contains(DiffFlags.EMIT_TEST_OPERATIONS))
+                        diffs.add(new Diff(Operation.TEST, currPath, srcNode));
                     diffs.add(Diff.generateDiff(Operation.REMOVE, currPath, srcNode));
                     srcIdx++;
                 } else {
                     List<Object> currPath = getPath(path, pos);
                     //both are unequal to lcs node
-                    generateDiffs(diffs, currPath, srcNode, targetNode);
+                    generateDiffs(currPath, srcNode, targetNode);
                     srcIdx++;
                     targetIdx++;
                     pos++;
@@ -382,26 +388,26 @@ private static void compareArray(List<Diff> diffs, List<Object> path, JsonNode s
             JsonNode srcNode = source.get(srcIdx);
             JsonNode targetNode = target.get(targetIdx);
             List<Object> currPath = getPath(path, pos);
-            generateDiffs(diffs, currPath, srcNode, targetNode);
+            generateDiffs(currPath, srcNode, targetNode);
             srcIdx++;
             targetIdx++;
             pos++;
         }
-        pos = addRemaining(diffs, path, target, pos, targetIdx, targetSize);
-        removeRemaining(diffs, path, pos, srcIdx, srcSize, source);
+        pos = addRemaining(path, target, pos, targetIdx, targetSize);
+        removeRemaining(path, pos, srcIdx, srcSize, source);
     }
 
-    private static Integer removeRemaining(List<Diff> diffs, List<Object> path, int pos, int srcIdx, int srcSize, JsonNode source) {
-
+    private void removeRemaining(List<Object> path, int pos, int srcIdx, int srcSize, JsonNode source) {
         while (srcIdx < srcSize) {
             List<Object> currPath = getPath(path, pos);
+            if (flags.contains(DiffFlags.EMIT_TEST_OPERATIONS))
+                diffs.add(new Diff(Operation.TEST, currPath, source.get(srcIdx)));
             diffs.add(Diff.generateDiff(Operation.REMOVE, currPath, source.get(srcIdx)));
             srcIdx++;
         }
-        return pos;
     }
 
-    private static Integer addRemaining(List<Diff> diffs, List<Object> path, JsonNode target, int pos, int targetIdx, int targetSize) {
+    private int addRemaining(List<Object> path, JsonNode target, int pos, int targetIdx, int targetSize) {
         while (targetIdx < targetSize) {
             JsonNode jsonNode = target.get(targetIdx);
             List<Object> currPath = getPath(path, pos);
@@ -412,18 +418,20 @@ private static Integer addRemaining(List<Diff> diffs, List<Object> path, JsonNod
         return pos;
     }
 
-    private static void compareObjects(List<Diff> diffs, List<Object> path, JsonNode source, JsonNode target) {
+    private void compareObjects(List<Object> path, JsonNode source, JsonNode target) {
         Iterator<String> keysFromSrc = source.fieldNames();
         while (keysFromSrc.hasNext()) {
             String key = keysFromSrc.next();
             if (!target.has(key)) {
                 //remove case
                 List<Object> currPath = getPath(path, key);
+                if (flags.contains(DiffFlags.EMIT_TEST_OPERATIONS))
+                    diffs.add(new Diff(Operation.TEST, currPath, source.get(key)));
                 diffs.add(Diff.generateDiff(Operation.REMOVE, currPath, source.get(key)));
                 continue;
             }
             List<Object> currPath = getPath(path, key);
-            generateDiffs(diffs, currPath, source.get(key), target.get(key));
+            generateDiffs(currPath, source.get(key), target.get(key));
         }
         Iterator<String> keysFromTarget = target.fieldNames();
         while (keysFromTarget.hasNext()) {

src/test/java/com/flipkart/zjsonpatch/JsonDiffTest.java

@@ -34,8 +34,8 @@
  * Unit test
  */
 public class JsonDiffTest {
-    static ObjectMapper objectMapper = new ObjectMapper();
-    static ArrayNode jsonNode;
+    private static ObjectMapper objectMapper = new ObjectMapper();
+    private static ArrayNode jsonNode;
 
     @BeforeClass
     public static void beforeClass() throws IOException {
@@ -46,48 +46,30 @@ public static void beforeClass() throws IOException {
     }
 
     @Test
-    public void testSampleJsonDiff() throws Exception {
+    public void testSampleJsonDiff() {
         for (int i = 0; i < jsonNode.size(); i++) {
             JsonNode first = jsonNode.get(i).get("first");
             JsonNode second = jsonNode.get(i).get("second");
-
-//            System.out.println("Test # " + i);
-//            System.out.println(first);
-//            System.out.println(second);
-
             JsonNode actualPatch = JsonDiff.asJson(first, second);
-
-
-//            System.out.println(actualPatch);
-
             JsonNode secondPrime = JsonPatch.apply(actualPatch, first);
-//            System.out.println(secondPrime);
-            Assert.assertTrue(second.equals(secondPrime));
+            Assert.assertEquals(second, secondPrime);
         }
     }
 
     @Test
-    public void testGeneratedJsonDiff() throws Exception {
+    public void testGeneratedJsonDiff() {
         Random random = new Random();
         for (int i = 0; i < 1000; i++) {
             JsonNode first = TestDataGenerator.generate(random.nextInt(10));
             JsonNode second = TestDataGenerator.generate(random.nextInt(10));
-
             JsonNode actualPatch = JsonDiff.asJson(first, second);
-//            System.out.println("Test # " + i);
-//
-//            System.out.println(first);
-//            System.out.println(second);
-//            System.out.println(actualPatch);
-
             JsonNode secondPrime = JsonPatch.apply(actualPatch, first);
-//            System.out.println(secondPrime);
-            Assert.assertTrue(second.equals(secondPrime));
+            Assert.assertEquals(second, secondPrime);
         }
     }
 
     @Test
-    public void testRenderedRemoveOperationOmitsValueByDefault() throws Exception {
+    public void testRenderedRemoveOperationOmitsValueByDefault() {
         ObjectNode source = objectMapper.createObjectNode();
         ObjectNode target = objectMapper.createObjectNode();
         source.put("field", "value");
@@ -100,7 +82,7 @@ public void testRenderedRemoveOperationOmitsValueByDefault() throws Exception {
     }
 
     @Test
-    public void testRenderedRemoveOperationRetainsValueIfOmitDiffFlagNotSet() throws Exception {
+    public void testRenderedRemoveOperationRetainsValueIfOmitDiffFlagNotSet() {
         ObjectNode source = objectMapper.createObjectNode();
         ObjectNode target = objectMapper.createObjectNode();
         source.put("field", "value");
@@ -123,20 +105,13 @@ public void testRenderedOperationsExceptMoveAndCopy() throws Exception {
 
         JsonNode diff = JsonDiff.asJson(source, target, flags);
 
-//        System.out.println(source);
-//        System.out.println(target);
-//        System.out.println(diff);
-
         for (JsonNode d : diff) {
             Assert.assertNotEquals(Operation.MOVE.rfcName(), d.get("op").textValue());
             Assert.assertNotEquals(Operation.COPY.rfcName(), d.get("op").textValue());
         }
 
         JsonNode targetPrime = JsonPatch.apply(diff, source);
-//        System.out.println(targetPrime);
-        Assert.assertTrue(target.equals(targetPrime));
-
-
+        Assert.assertEquals(target, targetPrime);
     }
 
     @Test
@@ -145,8 +120,7 @@ public void testPath() throws Exception {
         JsonNode patch = objectMapper.readTree("[{\"op\":\"copy\",\"from\":\"/profiles/def/0\", \"path\":\"/profiles/def/0\"},{\"op\":\"replace\",\"path\":\"/profiles/def/0/hello\",\"value\":\"world2\"}]");
 
         JsonNode target = JsonPatch.apply(patch, source);
-//        System.out.println(target);
         JsonNode expected = objectMapper.readTree("{\"profiles\":{\"abc\":[],\"def\":[{\"hello\":\"world2\"},{\"hello\":\"world\"}]}}");
-        Assert.assertTrue(target.equals(expected));
+        Assert.assertEquals(target, expected);
     }
 }

src/test/java/com/flipkart/zjsonpatch/JsonDiffTest2.java

@@ -33,8 +33,8 @@
  * @author ctranxuan (streamdata.io).
  */
 public class JsonDiffTest2 {
-    static ObjectMapper objectMapper = new ObjectMapper();
-    static ArrayNode jsonNode;
+    private static ObjectMapper objectMapper = new ObjectMapper();
+    private static ArrayNode jsonNode;
 
     @BeforeClass
     public static void beforeClass() throws IOException {
@@ -45,20 +45,15 @@ public static void beforeClass() throws IOException {
     }
 
     @Test
-    public void testPatchAppliedCleanly() throws Exception {
+    public void testPatchAppliedCleanly() {
         for (int i = 0; i < jsonNode.size(); i++) {
             JsonNode first = jsonNode.get(i).get("first");
             JsonNode second = jsonNode.get(i).get("second");
             JsonNode patch = jsonNode.get(i).get("patch");
             String message = jsonNode.get(i).get("message").toString();
 
-//            System.out.println("Test # " + i);
-//            System.out.println(first);
-//            System.out.println(second);
-//            System.out.println(patch);
-
             JsonNode secondPrime = JsonPatch.apply(patch, first);
-//            System.out.println(secondPrime);
+
             Assert.assertThat(message, secondPrime, equalTo(second));
         }