Merge pull request #3 from rwatler/master

Add factorizing support for converting duplicate add to copy operations

Author: fge

src/main/java/com/github/fge/jsonpatch/JsonFactorizingDiff.java

@@ -49,8 +49,9 @@
  * </ul>
  *
  * Array values generate operations in the order of elements. Factorizing is
- * done to merge add and remove into move operations if values are equivalent.
- * Copy and test operations are not generated.</p>
+ * done to merge add and remove into move operations and convert duplicate
+ * add to copy operations if values are equivalent. Test operations are not
+ * generated.</p>
  *
  * <p>Note that due to the way {@link JsonNode} is implemented, this class is
  * inherently <b>not</b> thread safe (since {@code JsonNode} is mutable). It is
@@ -327,9 +328,11 @@ else if (lengths[x][y] == lengths[x][y - 1])
 
     /**
      * Factorize list of ordered differences. Where removed values are
-     * equivalent to added values, merge add and remove to move operation
-     * differences. Because remove operation differences are relocated in
-     * the process of merging, other differences can be side effected.
+     * equivalent to added values, merge add and remove to move
+     * differences. Because remove differences are relocated in the
+     * process of merging, other differences can be side effected.
+     * Add differences with equivalent values to previous add
+     * differences are converted to copy differences.
      *
      * @param diffs list of ordered differences.
      */
@@ -364,7 +367,8 @@ private static void factorizeDiffs(final List<Diff> diffs)
         // are performed out of the original diff ordering just before the
         // paired add when converted to a move. consequently, moves that are
         // deferred or advanced must be tracked to allow proper diff array
-        // index adjustments for diffs operating on the same arrays.
+        // index adjustments for diffs operating on the same arrays. diff
+        // order is assumed to be acyclic and linearly processing arrays.
         final List<Diff> deferredArrayRemoves = Lists.newArrayList();
         final List<Diff> advancedArrayRemoves = Lists.newArrayList();
         for (final Iterator<Diff> diffIter = diffs.iterator();
@@ -445,6 +449,37 @@ private static void factorizeDiffs(final List<Diff> diffs)
                 diff.secondArrayIndex = adjustSecondArrayIndex(deferredArrayRemoves,
                     diff.arrayPath, diff.secondArrayIndex);
         }
+
+        // Factorize add diffs with equivalent non-empty object or array
+        // values into copy diffs; from paths for copy diffs can be set using
+        // previous add diff paths and/or array paths because diff order is
+        // acyclic and immutable for this factorization. The only exception
+        // to this rule are adds that append to arrays: these have no concrete
+        // path that can serve as a copy diff from path.
+        final List<Diff> addDiffs = Lists.newArrayList();
+        for (final Diff diff: diffs)
+            if (diff.operation == DiffOperation.ADD)
+                if (diff.value.size() > 0) {
+                    // check add diff value against list of previous add diffs
+                    Diff addDiff = null;
+                    for (final Diff testAddDiff: addDiffs)
+                        if (EQUIVALENCE.equivalent(diff.value, testAddDiff.value)) {
+                            addDiff = testAddDiff;
+                            break;
+                        }
+                    // if not found previously, save add diff, (if not appending
+                    // to an array which can have no concrete from path), and continue
+                    if (addDiff == null) {
+                        if (diff.arrayPath == null || diff.secondArrayIndex != -1)
+                            addDiffs.add(diff);
+                        continue;
+                    }
+                    // previous add diff found by value: convert add diff to copy
+                    // diff with from path set to concrete add diff path
+                    diff.operation = DiffOperation.COPY;
+                    diff.fromPath = addDiff.arrayPath != null ? addDiff.getSecondArrayPath()
+                        : addDiff.path;
+                }
     }
 
     /**
@@ -512,6 +547,7 @@ private enum DiffOperation
         REMOVE("remove"),
         REPLACE("replace"),
         MOVE("move"),
+        COPY("copy"),
         ;
 
         private final String opName;
@@ -577,15 +613,16 @@ private JsonNode asJsonPatch()
                 : path;
             final ObjectNode patch = operation.newOp(ptr);
             /*
-             * A remomve only has a path
+             * A remove only has a path
              */
             if (operation == DiffOperation.REMOVE)
                 return patch;
             /*
              * A move has a "source path" (the "from" member), other defined
              * operations (add and replace) have a value instead.
              */
-            if (operation == DiffOperation.MOVE)
+            if (operation == DiffOperation.MOVE
+                || operation == DiffOperation.COPY)
                 patch.put("from", fromPath.toString());
             else
                 patch.put("value", value);

src/test/resources/jsonpatch/factorizing-diff.json

@@ -230,5 +230,58 @@
             { "op": "move", "from": "/b/5", "path": "/b/3" },
             { "op": "move", "from": "/b/0", "path": "/b/6" }
         ]
+    },
+    {
+        "first": { "b": [0, 1, 2, 3, 4, 5, 6, 7, 8] },
+        "second": { "b": [1, 3, 6, 4, 5, 7, 0, 8], "c": 2 },
+        "patch": [
+            { "op": "move", "from": "/b/2", "path": "/c" },
+            { "op": "move", "from": "/b/5", "path": "/b/3" },
+            { "op": "move", "from": "/b/0", "path": "/b/6" }
+        ]
+    },
+    {
+        "first": {},
+        "second": { "a": 1, "b": 1},
+        "patch": [
+            { "op": "add", "path": "/a", "value": 1 },
+            { "op": "add", "path": "/b", "value": 1 }
+        ]
+    },
+    {
+        "first": {},
+        "second": { "a": {}, "b": {}},
+        "patch": [
+            { "op": "add", "path": "/a", "value": {} },
+            { "op": "add", "path": "/b", "value": {} }
+        ]
+    },
+    {
+        "first": {},
+        "second": { "a": { "a": 1 }, "b": { "a": 1.0 }},
+        "patch": [
+            { "op": "add", "path": "/a", "value": { "a": 1 } },
+            { "op": "copy", "from": "/a", "path": "/b" }
+        ]
+    },
+    {
+        "first": [],
+        "second": [ [ 0 ], [ 0 ] ],
+        "patch": [
+            { "op": "add", "path": "/-", "value": [ 0 ] },
+            { "op": "add", "path": "/-", "value": [ 0 ] }
+        ]
+    },
+    {
+        "first": [ "eol" ],
+        "second": [ { "a": 1 }, { "a": 1.0 }, [], [], [ 0 ], [ 0 ], "eol" ],
+        "patch": [
+            { "op": "add", "path": "/0", "value": { "a": 1 } },
+            { "op": "copy", "from": "/0", "path": "/1" },
+            { "op": "add", "path": "/2", "value": [] },
+            { "op": "add", "path": "/3", "value": [] },
+            { "op": "add", "path": "/4", "value": [ 0 ] },
+            { "op": "copy", "from": "/4", "path": "/5" }
+        ]
     }
 ]