Merge pull request #3 from nikhilvavs/add_in_place

tananaev · web-flow · commit 0aadeb6500e3 · 2020-11-06T18:25:22.000-08:00
Bump gson version and remove duplicating in each operation
diff --git a/build.gradle b/build.gradle
@@ -12,7 +12,7 @@ repositories {
 }
 
 dependencies {
-    compile 'com.google.code.gson:gson:2.8.0'
+    compile 'com.google.code.gson:gson:2.8.5'
     testCompile 'junit:junit:4.12'
 }
 
diff --git a/src/main/java/com/tananaev/jsonpatch/JsonPatch.java b/src/main/java/com/tananaev/jsonpatch/JsonPatch.java
@@ -20,17 +20,19 @@
 import com.google.gson.JsonElement;
 import com.tananaev.jsonpatch.gson.JsonPathSerializer;
 import com.tananaev.jsonpatch.operation.AbsOperation;
+import com.tananaev.jsonpatch.operation.InPlaceElementWrapper;
 
 import java.util.LinkedList;
 
 public class JsonPatch extends LinkedList<AbsOperation> {
 
     public JsonElement apply(JsonElement original) {
-        JsonElement result = original;
+        JsonElement result = original.deepCopy();
+        InPlaceElementWrapper inPlaceElement = new InPlaceElementWrapper(result);
         for ( AbsOperation operation: this){
-            result = operation.apply(result);
+            operation.applyInPlace(inPlaceElement);
         }
-        return result;
+        return inPlaceElement.getJsonElement();
     }
 
     @Override
diff --git a/src/main/java/com/tananaev/jsonpatch/operation/AbsOperation.java b/src/main/java/com/tananaev/jsonpatch/operation/AbsOperation.java
@@ -1,7 +1,6 @@
 package com.tananaev.jsonpatch.operation;
 
 import com.google.gson.JsonElement;
-import com.google.gson.JsonParser;
 import com.google.gson.annotations.SerializedName;
 import com.tananaev.jsonpatch.JsonPath;
 
@@ -18,10 +17,24 @@ public AbsOperation(){
 
     public abstract String getOperationName();
 
-    public abstract JsonElement apply( JsonElement original );
-
-    protected JsonElement duplicate(JsonElement original) {
-        return new JsonParser().parse(original.toString());
-    }
+    /**
+     * Applies the operation on the source and returns the patched object. The source is unmodified.
+     * @param sourceElement to which the patch is to be applied
+     * @return final json after patch is applied
+     */
+    public JsonElement apply( JsonElement sourceElement ){
+        JsonElement copiedSource = sourceElement.deepCopy();
+        InPlaceElementWrapper inPlaceElement = new InPlaceElementWrapper(copiedSource);
+        applyInPlace(inPlaceElement);
+        return inPlaceElement.getJsonElement();
+    };
+
+    /**
+     * An optimised version of apply where the source is not copied and is directly modified if possible.
+     * Updates the inPlaceElement to contain the patched json element.
+     * Refer to {@link InPlaceElementWrapper} for details on why a wrapper is needed
+     * @param inPlaceElement input to which the patch is to be applied
+     */
+    public abstract void applyInPlace(InPlaceElementWrapper inPlaceElement );
 
 }
diff --git a/src/main/java/com/tananaev/jsonpatch/operation/AddOperation.java b/src/main/java/com/tananaev/jsonpatch/operation/AddOperation.java
@@ -25,10 +25,9 @@ public String getOperationName() {
     }
 
     @Override
-    public JsonElement apply(JsonElement original) {
-        JsonElement result = duplicate( original );
-
-        JsonElement item = path.head().navigate(result);
+    public void applyInPlace(InPlaceElementWrapper inPlaceElement){
+        JsonElement sourceElement = inPlaceElement.getJsonElement();
+        JsonElement item = path.head().navigate(sourceElement);
 
         if ( item.isJsonObject() ){
             item.getAsJsonObject().add(path.tail(),data);
@@ -54,8 +53,6 @@ public JsonElement apply(JsonElement original) {
             }
 
         }
-
-        return result;
     }
 
 }
diff --git a/src/main/java/com/tananaev/jsonpatch/operation/InPlaceElementWrapper.java b/src/main/java/com/tananaev/jsonpatch/operation/InPlaceElementWrapper.java
@@ -0,0 +1,26 @@
+package com.tananaev.jsonpatch.operation;
+
+import com.google.gson.JsonElement;
+
+/**
+ * Wrapper class over json element to pass as an input for in place operations.
+ * This is needed because we cannot convert actual JsonElement types into another in place
+ * i.e, we cannot convert a JsonPrimitive (or other) into a JsonObject (or other) in-place
+ * if needed by the patch operation.
+ */
+public class InPlaceElementWrapper {
+
+    private JsonElement jsonElement;
+
+    public JsonElement getJsonElement() {
+        return jsonElement;
+    }
+
+    public InPlaceElementWrapper(JsonElement element) {
+        this.jsonElement = element;
+    }
+
+    public void setJsonElement(JsonElement jsonElement) {
+        this.jsonElement = jsonElement;
+    }
+}
diff --git a/src/main/java/com/tananaev/jsonpatch/operation/MoveOperation.java b/src/main/java/com/tananaev/jsonpatch/operation/MoveOperation.java
@@ -23,18 +23,17 @@ public String getOperationName() {
     }
 
     @Override
-    public JsonElement apply(JsonElement original) {
-        JsonElement result = duplicate( original );
+    public void applyInPlace(InPlaceElementWrapper inPlaceElement) {
+        JsonElement sourceElement = inPlaceElement.getJsonElement();
+        JsonElement value = from.navigate(sourceElement);
 
-        JsonElement value = from.navigate(result);
+        JsonElement existingElement = from.head().navigate(sourceElement);
+        JsonElement destination = path.head().navigate(sourceElement);
 
-        JsonElement source = from.head().navigate(result);
-        JsonElement destination = path.head().navigate(result);
-
-        if ( source.isJsonObject() ){
-            source.getAsJsonObject().remove(from.tail());
-        } else if ( source.isJsonArray() ){
-            JsonArray array = source.getAsJsonArray();
+        if ( existingElement.isJsonObject() ){
+            existingElement.getAsJsonObject().remove(from.tail());
+        } else if ( existingElement.isJsonArray() ){
+            JsonArray array = existingElement.getAsJsonArray();
 
             int index = (from.tail().equals("-")) ? array.size() : Integer.valueOf(from.tail());
 
@@ -64,8 +63,6 @@ public JsonElement apply(JsonElement original) {
                 array.add(stuff);
             }
         }
-
-        return result;
     }
 
 }
diff --git a/src/main/java/com/tananaev/jsonpatch/operation/RemoveOperation.java b/src/main/java/com/tananaev/jsonpatch/operation/RemoveOperation.java
@@ -16,10 +16,9 @@ public String getOperationName() {
     }
 
     @Override
-    public JsonElement apply(JsonElement original) {
-        JsonElement result = duplicate( original );
+    public void applyInPlace(InPlaceElementWrapper inPlaceElement) {
 
-        JsonElement item = path.head().navigate(result);
+        JsonElement item = path.head().navigate(inPlaceElement.getJsonElement());
 
         if ( item.isJsonObject() ){
             item.getAsJsonObject().remove(path.tail());
@@ -31,7 +30,6 @@ public JsonElement apply(JsonElement original) {
             array.remove(index);
         }
 
-        return result;
     }
 
 }
diff --git a/src/main/java/com/tananaev/jsonpatch/operation/ReplaceOperation.java b/src/main/java/com/tananaev/jsonpatch/operation/ReplaceOperation.java
@@ -22,10 +22,9 @@ public String getOperationName() {
     }
 
     @Override
-    public JsonElement apply(JsonElement original) {
-        JsonElement result = duplicate( original );
+    public void applyInPlace(InPlaceElementWrapper inPlaceElement) {
 
-        JsonElement item = path.head().navigate(result);
+        JsonElement item = path.head().navigate(inPlaceElement.getJsonElement());
 
         if ( item.isJsonObject() ){
             JsonObject object = item.getAsJsonObject();
@@ -45,10 +44,9 @@ public JsonElement apply(JsonElement original) {
             }
 
         } else {
-            return data;
+            inPlaceElement.setJsonElement(data);
         }
 
-        return result;
     }
 
 }
diff --git a/src/test/java/com/tananaev/jsonpatch/test/OperationTest.java b/src/test/java/com/tananaev/jsonpatch/test/OperationTest.java
@@ -22,20 +22,13 @@
 import com.google.gson.JsonPrimitive;
 import com.tananaev.jsonpatch.JsonPath;
 import com.tananaev.jsonpatch.operation.AddOperation;
+import com.tananaev.jsonpatch.operation.InPlaceElementWrapper;
 import com.tananaev.jsonpatch.operation.ReplaceOperation;
 import org.junit.Assert;
 import org.junit.Test;
 
 public class OperationTest {
 
-    @Test
-    public void basicAddWorks(){
-        JsonElement element = new JsonObject();
-
-        JsonElement result = new AddOperation(new JsonPath("/a"), new JsonObject() ).apply(element);
-
-        Assert.assertEquals("{\"a\":{}}", result.toString());
-    }
 
     @Test
     public void replaceRootNullWithPrimitive(){
diff --git a/src/test/java/com/tananaev/jsonpatch/test/operation/AddOperationTest.java b/src/test/java/com/tananaev/jsonpatch/test/operation/AddOperationTest.java
@@ -0,0 +1,35 @@
+package com.tananaev.jsonpatch.test.operation;
+
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.tananaev.jsonpatch.JsonPath;
+import com.tananaev.jsonpatch.operation.AddOperation;
+import com.tananaev.jsonpatch.operation.InPlaceElementWrapper;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class AddOperationTest {
+
+    @Test
+    public void basicAddWorks(){
+        JsonElement element = new JsonObject();
+        String originalData = element.toString();
+
+        JsonElement result = new AddOperation(new JsonPath("/a"), new JsonObject() ).apply(element);
+
+        Assert.assertEquals(originalData, element.toString());
+        Assert.assertEquals("{\"a\":{}}", result.toString());
+    }
+
+    @Test
+    public void basicInPlaceAddWorks(){
+        JsonElement element = new JsonObject();
+        InPlaceElementWrapper inPlaceElement = new InPlaceElementWrapper(element);
+        AddOperation addOperation = new AddOperation(new JsonPath("/a"), new JsonObject());
+
+        addOperation.applyInPlace(inPlaceElement);
+        JsonElement result = inPlaceElement.getJsonElement();
+
+        Assert.assertEquals("{\"a\":{}}", result.toString());
+    }
+}
diff --git a/src/test/java/com/tananaev/jsonpatch/test/operation/MoveOperationTest.java b/src/test/java/com/tananaev/jsonpatch/test/operation/MoveOperationTest.java
@@ -0,0 +1,40 @@
+package com.tananaev.jsonpatch.test.operation;
+
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonPrimitive;
+import com.tananaev.jsonpatch.JsonPath;
+import com.tananaev.jsonpatch.operation.AddOperation;
+import com.tananaev.jsonpatch.operation.InPlaceElementWrapper;
+import com.tananaev.jsonpatch.operation.MoveOperation;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class MoveOperationTest {
+
+    @Test
+    public void basicMoveWorks(){
+        JsonElement element = new JsonObject();
+        element.getAsJsonObject().add("a", new JsonPrimitive("123"));
+        String originalData = element.toString();
+
+        JsonElement result = new MoveOperation(new JsonPath("/a"), new JsonPath("/b") ).apply(element);
+
+        Assert.assertEquals(originalData, element.toString());
+        Assert.assertEquals("{\"b\":\"123\"}", result.toString());
+    }
+
+    @Test
+    public void basicInPlaceMoveWorks(){
+
+        JsonElement element = new JsonObject();
+        element.getAsJsonObject().add("a", new JsonPrimitive("123"));
+        InPlaceElementWrapper inPlaceElement = new InPlaceElementWrapper(element);
+
+        MoveOperation moveOperation = new MoveOperation(new JsonPath("/a"), new JsonPath("/b") );
+        moveOperation.applyInPlace(inPlaceElement);
+
+        Assert.assertEquals("{\"b\":\"123\"}", inPlaceElement.getJsonElement().toString());
+
+    }
+}
diff --git a/src/test/java/com/tananaev/jsonpatch/test/operation/RemoveOperationTest.java b/src/test/java/com/tananaev/jsonpatch/test/operation/RemoveOperationTest.java
@@ -0,0 +1,40 @@
+package com.tananaev.jsonpatch.test.operation;
+
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonPrimitive;
+import com.tananaev.jsonpatch.JsonPath;
+import com.tananaev.jsonpatch.operation.InPlaceElementWrapper;
+import com.tananaev.jsonpatch.operation.MoveOperation;
+import com.tananaev.jsonpatch.operation.RemoveOperation;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class RemoveOperationTest {
+
+    @Test
+    public void basicRemoveWorks(){
+        JsonElement element = new JsonObject();
+        element.getAsJsonObject().add("a", new JsonPrimitive("123"));
+        String originalData = element.toString();
+
+        JsonElement result = new RemoveOperation(new JsonPath("/a") ).apply(element);
+
+        Assert.assertEquals(originalData, element.toString());
+        Assert.assertEquals("{}", result.toString());
+    }
+
+    @Test
+    public void basicInPlaceRemoveWorks(){
+
+        JsonElement element = new JsonObject();
+        element.getAsJsonObject().add("a", new JsonPrimitive("123"));
+        InPlaceElementWrapper inPlaceElement = new InPlaceElementWrapper(element);
+
+        RemoveOperation removeOperation = new RemoveOperation(new JsonPath("/a"));
+        removeOperation.applyInPlace(inPlaceElement);
+
+        Assert.assertEquals("{}", inPlaceElement.getJsonElement().toString());
+
+    }
+}
diff --git a/src/test/java/com/tananaev/jsonpatch/test/operation/ReplaceOperationTest.java b/src/test/java/com/tananaev/jsonpatch/test/operation/ReplaceOperationTest.java
@@ -0,0 +1,43 @@
+package com.tananaev.jsonpatch.test.operation;
+
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonPrimitive;
+import com.tananaev.jsonpatch.JsonPath;
+import com.tananaev.jsonpatch.operation.InPlaceElementWrapper;
+import com.tananaev.jsonpatch.operation.RemoveOperation;
+import com.tananaev.jsonpatch.operation.ReplaceOperation;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ReplaceOperationTest {
+    @Test
+    public void basicReplaceWorks(){
+        JsonElement element = new JsonObject();
+        element.getAsJsonObject().add("a", new JsonPrimitive("123"));
+        String originalData = element.toString();
+
+        JsonElement newElement = new JsonPrimitive("1234");
+
+        JsonElement result = new ReplaceOperation(new JsonPath("/a"), newElement).apply(element);
+
+        Assert.assertEquals(originalData, element.toString());
+        Assert.assertEquals("{\"a\":\"1234\"}", result.toString());
+    }
+
+    @Test
+    public void basicInPlaceReplaceWorks(){
+
+        JsonElement element = new JsonObject();
+        element.getAsJsonObject().add("a", new JsonPrimitive("123"));
+
+        JsonElement newElement = new JsonPrimitive("1234");
+        InPlaceElementWrapper inPlaceElement = new InPlaceElementWrapper(element);
+
+        ReplaceOperation replaceOperation = new ReplaceOperation(new JsonPath("/a"), newElement);
+        replaceOperation.applyInPlace(inPlaceElement);
+
+        Assert.assertEquals("{\"a\":\"1234\"}", inPlaceElement.getJsonElement().toString());
+
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ repositories {`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`dependencies {`
`15`		`- compile 'com.google.code.gson:gson:2.8.0'`
	`15`	`+ compile 'com.google.code.gson:gson:2.8.5'`
`16`	`16`	`testCompile 'junit:junit:4.12'`
`17`	`17`	`}`
`18`	`18`
Original file line number	Diff line number	Diff line change
`@@ -25,10 +25,9 @@ public String getOperationName() {`
`25`	`25`	`}`
`26`	`26`
`27`	`27`	`@Override`
`28`		`- public JsonElement apply(JsonElement original) {`
`29`		`- JsonElement result = duplicate( original );`
`30`		`-`
`31`		`- JsonElement item = path.head().navigate(result);`
	`28`	`+ public void applyInPlace(InPlaceElementWrapper inPlaceElement){`
	`29`	`+ JsonElement sourceElement = inPlaceElement.getJsonElement();`
	`30`	`+ JsonElement item = path.head().navigate(sourceElement);`
`32`	`31`
`33`	`32`	`if ( item.isJsonObject() ){`
`34`	`33`	`item.getAsJsonObject().add(path.tail(),data);`
`@@ -54,8 +53,6 @@ public JsonElement apply(JsonElement original) {`
`54`	`53`	`}`
`55`	`54`
`56`	`55`	`}`
`57`		`-`
`58`		`- return result;`
`59`	`56`	`}`
`60`	`57`
`61`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,10 +16,9 @@ public String getOperationName() {`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`@Override`
`19`		`- public JsonElement apply(JsonElement original) {`
`20`		`- JsonElement result = duplicate( original );`
	`19`	`+ public void applyInPlace(InPlaceElementWrapper inPlaceElement) {`
`21`	`20`
`22`		`- JsonElement item = path.head().navigate(result);`
	`21`	`+ JsonElement item = path.head().navigate(inPlaceElement.getJsonElement());`
`23`	`22`
`24`	`23`	`if ( item.isJsonObject() ){`
`25`	`24`	`item.getAsJsonObject().remove(path.tail());`
`@@ -31,7 +30,6 @@ public JsonElement apply(JsonElement original) {`
`31`	`30`	`array.remove(index);`
`32`	`31`	`}`
`33`	`32`
`34`		`- return result;`
`35`	`33`	`}`
`36`	`34`
`37`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -22,10 +22,9 @@ public String getOperationName() {`
`22`	`22`	`}`
`23`	`23`
`24`	`24`	`@Override`
`25`		`- public JsonElement apply(JsonElement original) {`
`26`		`- JsonElement result = duplicate( original );`
	`25`	`+ public void applyInPlace(InPlaceElementWrapper inPlaceElement) {`
`27`	`26`
`28`		`- JsonElement item = path.head().navigate(result);`
	`27`	`+ JsonElement item = path.head().navigate(inPlaceElement.getJsonElement());`
`29`	`28`
`30`	`29`	`if ( item.isJsonObject() ){`
`31`	`30`	`JsonObject object = item.getAsJsonObject();`
`@@ -45,10 +44,9 @@ public JsonElement apply(JsonElement original) {`
`45`	`44`	`}`
`46`	`45`
`47`	`46`	`} else {`
`48`		`- return data;`
	`47`	`+ inPlaceElement.setJsonElement(data);`
`49`	`48`	`}`
`50`	`49`
`51`		`- return result;`
`52`	`50`	`}`
`53`	`51`
`54`	`52`	`}`