Added batch_remove function

The batch remove function takes multiple array indexes(all indexes are indexes into the original array) and deletes them "atomically"(atomically to JSON patch at any rate)
I also added functionality to the factorizer that condenses any array index removes that may have been emitted by it.  Result is hopefully even more compact difference files and
ones that are easier to create by hand.

Author: joeltucci

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

@@ -0,0 +1,77 @@
+package com.github.fge.jsonpatch;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+import com.fasterxml.jackson.databind.JsonNode;
+
+import com.fasterxml.jackson.databind.node.ArrayNode;
+
+import com.fasterxml.jackson.databind.node.MissingNode;
+import com.github.fge.jackson.jsonpointer.JsonPointer;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Created by joel.tucci on 3/20/14.
+ */
+public class BatchRemoveOperation extends PathValueOperation {
+
+    public static final String OPERATION_NAME="batch_remove";
+
+    @JsonCreator
+    public BatchRemoveOperation(@JsonProperty("path") final JsonPointer path,
+                            @JsonProperty("value") final JsonNode value)
+    {
+        super(OPERATION_NAME, path, value);
+    }
+
+    private void validateDataTypes(JsonNode node) throws JsonPatchException{
+
+        JsonNode parentNode=path.parent().path(node);
+        if (parentNode.isMissingNode())
+            throw new JsonPatchException(BUNDLE.getMessage(
+                    "jsonPatch.noSuchPath"));
+
+        if(!value.isArray())
+            throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.valueNotArray"));
+        if(!parentNode.isArray())
+            throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.nodeNotArray"));
+    }
+
+    @Override
+    public JsonNode apply(JsonNode node) throws JsonPatchException {
+        if (path.isEmpty())
+            return MissingNode.getInstance();
+
+        validateDataTypes(node);
+
+        //The simplest, if perhaps naive implementation is to do a reverse sort on the indices to be deleted and then
+        //delete them in that order.  This is O(n*m)+O(m lg m)(where n is the length of the list and m is the # of nodes to be deleted
+        //but it can be done without copying the entire original array
+        ArrayNode valArray=(ArrayNode) value;
+        final JsonNode ret=node.deepCopy();
+        final ArrayNode targetArray = (ArrayNode) path.parent().get(ret);
+
+
+        List<Integer> victimIndices=new ArrayList<Integer>();
+        for(int i=0;i<valArray.size();i++) {
+            Integer victimIndex=valArray.get(i).asInt();
+            if(victimIndex >= targetArray.size())
+                throw new JsonPatchException(BUNDLE.getMessage(
+                        "jsonPatch.indexDoesNotExist"));
+            victimIndices.add(victimIndex);
+        }
+        Collections.sort(victimIndices);
+
+        for(int i=victimIndices.size()-1;i>=0;i--) {
+            targetArray.remove(victimIndices.get(i));
+        }
+
+
+        return ret;
+    }
+
+}

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

@@ -38,6 +38,7 @@
     @Type(name = "move", value = MoveOperation.class),
     @Type(name = "remove", value = RemoveOperation.class),
     @Type(name = "replace", value = ReplaceOperation.class),
+    @Type(name=BatchRemoveOperation.OPERATION_NAME, value=BatchRemoveOperation.class),
     @Type(name = "test", value = TestOperation.class)
 })
 

src/main/java/com/github/fge/jsonpatch/diff/Diff.java

@@ -19,11 +19,15 @@
 package com.github.fge.jsonpatch.diff;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.github.fge.jackson.JacksonUtils;
 import com.github.fge.jackson.JsonNumEquals;
 import com.github.fge.jackson.jsonpointer.JsonPointer;
 import com.google.common.base.Objects;
 
+import java.util.Collection;
+
 /**
  * Difference representation. Captures diff information required to
  * generate JSON patch operations and factorize differences.
@@ -76,6 +80,15 @@ static Diff arrayRemove(final JsonPointer basePath,
             array2.getIndex(), array1.getElement().deepCopy());
     }
 
+    static Diff batchRemove(final JsonPointer basePath, final Collection<Integer> victimIndices) {
+        ArrayNode victimNodes=new ArrayNode(JacksonUtils.nodeFactory());
+        for(final Integer victimIndex : victimIndices) {
+            victimNodes.add(victimIndex);
+        }
+        return new Diff(DiffOperation.BATCH_REMOVE, basePath,-1,-1,victimNodes);
+
+    }
+
     static Diff arrayAdd(final JsonPointer basePath, final JsonNode node)
     {
         return new Diff(DiffOperation.ADD, basePath, -1, -1, node.deepCopy());

src/main/java/com/github/fge/jsonpatch/diff/DiffOperation.java

@@ -34,6 +34,7 @@ enum DiffOperation
     REPLACE("replace"),
     MOVE("move"),
     COPY("copy"),
+    BATCH_REMOVE("batch_remove")
     ;
 
     private final String opName;

src/main/java/com/github/fge/jsonpatch/diff/JsonDiff.java

@@ -90,6 +90,7 @@ public static JsonNode asJson(final JsonNode source, final JsonNode target)
 
         // factorize diffs to optimize patch operations
         DiffFactorizer.factorizeDiffs(diffs);
+        RemoveCoalescer.coalesceRemoves(diffs);
 
         // generate patch operations from node diffs
         final ArrayNode patch = FACTORY.arrayNode();

src/main/java/com/github/fge/jsonpatch/diff/RemoveCoalescer.java

@@ -0,0 +1,89 @@
+package com.github.fge.jsonpatch.diff;
+
+import com.github.fge.jackson.jsonpointer.JsonPointer;
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+/**
+ * Created by joel.tucci on 3/20/14.
+ */
+final class RemoveCoalescer {
+
+    private RemoveCoalescer() {
+    }
+
+    private static int calculateBlockRemoveLength(int originalDiffListIndex,List<Diff> diffs) {
+        //seek ahead to find out where next instruction begins
+        Diff curDiff=diffs.get(originalDiffListIndex);
+        int seekIndex=originalDiffListIndex+1;
+        while(seekIndex<diffs.size() && diffs.get(seekIndex).arrayPath.equals(curDiff.arrayPath) &&
+                diffs.get(seekIndex).operation == DiffOperation.REMOVE) {
+            seekIndex++;
+        }
+        return seekIndex-originalDiffListIndex;
+    }
+
+    private static Diff condenseRemovesToBatchCommand(List<Diff> diffs,int removeBlockLength,int startIndex) {
+        List<Integer> removedIndices=Lists.newArrayList();
+        int curOffset=0;  //Needed to map between the current remove indices and the original array
+        Diff curDiff=diffs.get(startIndex);
+        removedIndices.add(curDiff.firstArrayIndex);
+        Integer previousIndex;
+        for(int blockRemoveIndex=1;blockRemoveIndex<removeBlockLength;blockRemoveIndex++) {
+            previousIndex=curDiff.firstArrayIndex;
+            curDiff=diffs.get(blockRemoveIndex+startIndex);
+            if(curDiff.firstArrayIndex == previousIndex)
+                curOffset++;
+            removedIndices.add(curDiff.firstArrayIndex+curOffset);
+        }
+        return Diff.batchRemove(diffs.get(startIndex).arrayPath, removedIndices);
+
+    }
+
+    /**
+     * Combine multiple remove commands on an array into a single batch_remove.
+     * This not only results in more compact JSON, it also makes diffs easier to understand and create
+     * The value in the batch remove correspond to the values in the original array, and do not
+     * have to either be specified in reverse order or require difficult calculations to figure out which nodes
+     * are actually getting removed
+     * @param diffs
+     */
+    public static void coalesceRemoves(final List<Diff> diffs) {
+
+        //I am assuming that the previous step emits the removes in order, i.e. all removes
+        //for a given array are consecutive.  This is how the current factorizediffs implementation works
+        //obviously if this assumption no longer holds then this part will no longer optimize the removes
+
+        final List<Diff> coalescedDiffs = Lists.newArrayList();
+        for(int i=0;i<diffs.size();i++) {
+
+            Diff curDiff=diffs.get(i);
+            if(curDiff.operation != DiffOperation.REMOVE) {
+                coalescedDiffs.add(curDiff);
+                continue; //Only coalesce remove operations
+            }
+
+            if(curDiff.arrayPath == null) {
+                coalescedDiffs.add(curDiff);
+                continue; //not an array
+            }
+
+            int removeBlockLength=calculateBlockRemoveLength(i,diffs);
+            if(removeBlockLength == 1) {
+                //Single remove, so just add it directly
+                coalescedDiffs.add(diffs.get(i));
+                continue;
+            }
+
+            coalescedDiffs.add(condenseRemovesToBatchCommand(diffs,removeBlockLength,i));
+            //Now that we have condensed all these remove commands, we need to skip ahead to the next operation
+            i=i+removeBlockLength-1;
+
+        }
+
+        diffs.clear();
+        diffs.addAll(coalescedDiffs);
+    }
+
+}

src/test/java/com/github/fge/jsonpatch/BatchRemoveOperationTest.java

@@ -0,0 +1,22 @@
+package com.github.fge.jsonpatch;
+
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.github.fge.jackson.JacksonUtils;
+import org.testng.annotations.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Created by joel.tucci on 3/20/14.
+ */
+public class BatchRemoveOperationTest extends JsonPatchOperationTest {
+
+    protected BatchRemoveOperationTest(String prefix) throws IOException {
+        super(BatchRemoveOperation.OPERATION_NAME);
+    }
+
+
+}
+

src/test/java/com/github/fge/jsonpatch/ReplaceOperationTest.java

@@ -18,6 +18,8 @@
 
 package com.github.fge.jsonpatch;
 
+import org.testng.annotations.Test;
+
 import java.io.IOException;
 
 public final class ReplaceOperationTest
@@ -28,4 +30,5 @@ public ReplaceOperationTest()
     {
         super("replace");
     }
+
 }

src/test/resources/jsonpatch/batch_remove.json

@@ -0,0 +1,33 @@
+{
+    "errors": [
+        {
+            "op": { "op": "batch_remove", "path": "/x/y/-" },
+            "node": { "x": {} },
+            "message": "jsonPatch.noSuchPath"
+        },
+        {
+            "op": { "op": "batch_remove", "path": "/x/y", "value":[1,3] },
+            "node": { "x": { "y":7} },
+            "message": "jsonPatch.nodeNotArray"
+        },
+        {
+            "op": { "op": "batch_remove", "path": "/x/y/-", "value":0 },
+            "node": { "x": { "y": [1,2,3]} },
+            "message": "jsonPatch.valueNotArray"
+        },
+        {
+            "op": { "op": "batch_remove", "path": "/x/y/-", "value": [3,5] },
+            "node": { "x": { "y": [1,2,3]} },
+            "message": "jsonPatch.indexDoesNotExist"
+        }
+	
+	
+    ],
+    "ops": [
+        {
+            "op": { "op": "batch_remove", "path": "/x/y/-", "value": [4,0,1] },
+            "node": { "x": { "a": "b", "y": [1,2,3,4,5] } },
+            "expected": { "x": { "a": "b","y":[3,4] } }
+        }
+    ]
+}

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

@@ -63,8 +63,14 @@
         "first": [ 1, 2, 3 ],
         "second": [ 1 ],
         "patch": [
-            { "op": "remove", "path": "/1" },
-            { "op": "remove", "path": "/1" }
+            { "op": "batch_remove", "path": "/-", "value":[1,2] }
+        ]
+    },
+    {
+        "first": {"x":{"a":7, "y":[ 1, 2, 3,4 ]}},
+        "second": {"x":{"a":7, "y":[ 1, 4 ]}},
+        "patch": [
+            { "op": "batch_remove", "path": "/x/y/-", "value":[1,2] }
         ]
     },
     {
@@ -133,9 +139,7 @@
         "first": [0, 1, 2, 3, 4, 5, 6, 7],
         "second": [3, 6, 4, 5, 7],
         "patch": [
-            { "op": "remove", "path": "/0" },
-            { "op": "remove", "path": "/0" },
-            { "op": "remove", "path": "/0" },
+            { "op": "batch_remove", "path": "/-", "value":[0,1,2] },
             { "op": "move", "from": "/3", "path": "/1" }
         ]
     },
@@ -289,4 +293,4 @@
         "second": [ 2, 1 ],
         "patch": [ { "op": "move", "from": "/1", "path": "/0" } ]
     }
-]
+]