diff --git a/src/main/java/com/github/fge/jsonpatch/JsonFactorizingDiff.java b/src/main/java/com/github/fge/jsonpatch/JsonFactorizingDiff.java index 5715461a..f5a07c3c 100644 --- a/src/main/java/com/github/fge/jsonpatch/JsonFactorizingDiff.java +++ b/src/main/java/com/github/fge/jsonpatch/JsonFactorizingDiff.java @@ -49,8 +49,9 @@ * * * 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.

+ * 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.

* *

Note that due to the way {@link JsonNode} is implemented, this class is * inherently not 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 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 deferredArrayRemoves = Lists.newArrayList(); final List advancedArrayRemoves = Lists.newArrayList(); for (final Iterator diffIter = diffs.iterator(); @@ -445,6 +449,37 @@ private static void factorizeDiffs(final List 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 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,7 +613,7 @@ 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; @@ -585,7 +621,8 @@ private JsonNode asJsonPatch() * 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); diff --git a/src/test/resources/jsonpatch/factorizing-diff.json b/src/test/resources/jsonpatch/factorizing-diff.json index 40914c7a..324200b4 100644 --- a/src/test/resources/jsonpatch/factorizing-diff.json +++ b/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" } + ] } ] \ No newline at end of file