Skip to content

Add factorizing support for converting duplicate add to copy operations #3

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

Merged
merged 1 commit into from
Apr 12, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 45 additions & 8 deletions src/main/java/com/github/fge/jsonpatch/JsonFactorizingDiff.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
}

/**
Expand Down Expand Up @@ -512,6 +547,7 @@ private enum DiffOperation
REMOVE("remove"),
REPLACE("replace"),
MOVE("move"),
COPY("copy"),
;

private final String opName;
Expand Down Expand Up @@ -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);
Expand Down
53 changes: 53 additions & 0 deletions src/test/resources/jsonpatch/factorizing-diff.json
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
]
}
]