You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@johnzon.apache.org by rm...@apache.org on 2020/05/07 07:34:59 UTC

[johnzon] branch master updated: JOHNZON-312 JsonPointer patch process shouldnt check subobjects/arrays not matching the pointer

This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/johnzon.git


The following commit(s) were added to refs/heads/master by this push:
     new 826ca4f  JOHNZON-312 JsonPointer patch process shouldnt check subobjects/arrays not matching the pointer
826ca4f is described below

commit 826ca4f210990cea1dcfb295f34ccb5ada3906d9
Author: Romain Manni-Bucau <rm...@gmail.com>
AuthorDate: Thu May 7 09:34:52 2020 +0200

    JOHNZON-312 JsonPointer patch process shouldnt check subobjects/arrays not matching the pointer
---
 .../org/apache/johnzon/core/JsonPointerImpl.java   | 30 ++++++++---
 .../org/apache/johnzon/core/JsonPointerUtil.java   | 24 ++++++---
 .../org/apache/johnzon/core/JsonPatchDiffTest.java | 60 ++++++++++++++++++++++
 .../apache/johnzon/core/JsonPointerUtilTest.java   | 10 ++++
 4 files changed, 108 insertions(+), 16 deletions(-)

diff --git a/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerImpl.java b/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerImpl.java
index e9cd32c..5ad5958 100644
--- a/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerImpl.java
+++ b/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerImpl.java
@@ -31,6 +31,7 @@ import javax.json.spi.JsonProvider;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.regex.Pattern;
 
 
@@ -356,12 +357,17 @@ public class JsonPointerImpl implements JsonPointer {
         List<String> currentPath = new ArrayList<>();
         currentPath.add("");
 
-        return (T) addInternal(jsonValue, newValue, currentPath);
+        return (T) addInternal(jsonValue, newValue, currentPath, true);
     }
 
-    private JsonValue addInternal(JsonValue jsonValue, JsonValue newValue, List<String> currentPath) {
+    private JsonValue addInternal(JsonValue jsonValue, JsonValue newValue, List<String> currentPath,
+                                  boolean check) {
         if (jsonValue.getValueType() == JsonValue.ValueType.OBJECT) {
-            JsonObject jsonObject = (JsonObject) jsonValue;
+            JsonObject jsonObject = jsonValue.asJsonObject();
+            if (!check) {
+                return provider.createObjectBuilder(jsonObject).build();
+            }
+
             JsonObjectBuilder objectBuilder = provider.createObjectBuilder();
 
             if (jsonObject.isEmpty() && isPositionToAdd(currentPath)) {
@@ -370,22 +376,25 @@ public class JsonPointerImpl implements JsonPointer {
                 for (Map.Entry<String, JsonValue> entry : jsonObject.entrySet()) {
 
                     currentPath.add(entry.getKey());
-                    objectBuilder.add(entry.getKey(), addInternal(entry.getValue(), newValue, currentPath));
+                    objectBuilder.add(entry.getKey(), addInternal(entry.getValue(), newValue, currentPath, canMatch(currentPath)));
                     currentPath.remove(entry.getKey());
 
-                    if (isPositionToAdd(currentPath)) {
+                    if (check && isPositionToAdd(currentPath)) {
                         objectBuilder.add(referenceTokens.get(referenceTokens.size() - 1), newValue);
                     }
                 }
             }
             return objectBuilder.build();
         } else if (jsonValue.getValueType() == JsonValue.ValueType.ARRAY) {
-            JsonArray jsonArray = (JsonArray) jsonValue;
+            JsonArray jsonArray = jsonValue.asJsonArray();
+            if (!check) {
+                return provider.createArrayBuilder(jsonArray).build();
+            }
             JsonArrayBuilder arrayBuilder = provider.createArrayBuilder();
 
             int arrayIndex = -1;
             if (isPositionToAdd(currentPath)) {
-                arrayIndex = getArrayIndex(referenceTokens.get(referenceTokens.size() - 1), jsonArray, true);
+                arrayIndex = getArrayIndex(referenceTokens.get(referenceTokens.size() - 1), jsonArray, canMatch(currentPath));
             }
 
             int jsonArraySize = jsonArray.size();
@@ -399,7 +408,7 @@ public class JsonPointerImpl implements JsonPointer {
 
                 String path = String.valueOf(i);
                 currentPath.add(path);
-                arrayBuilder.add(addInternal(jsonArray.get(i), newValue, currentPath));
+                arrayBuilder.add(addInternal(jsonArray.get(i), newValue, currentPath, canMatch(currentPath)));
                 currentPath.remove(path);
             }
             return arrayBuilder.build();
@@ -412,6 +421,11 @@ public class JsonPointerImpl implements JsonPointer {
                 currentPath.get(currentPath.size() - 1).equals(referenceTokens.get(referenceTokens.size() - 2));
     }
 
+    private boolean canMatch(final List<String> currentPath) {
+        return currentPath.size() <= referenceTokens.size() &&
+                Objects.equals(currentPath.get(currentPath.size() - 1), referenceTokens.get(currentPath.size() - 1));
+    }
+
     private JsonValue remove(final JsonValue jsonValue, final int currentPosition) {
         if (referenceTokens.size() <= currentPosition) { // unlikely
             return jsonValue;
diff --git a/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerUtil.java b/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerUtil.java
index 91bc9c2..a62d4d1 100644
--- a/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerUtil.java
+++ b/johnzon-core/src/main/java/org/apache/johnzon/core/JsonPointerUtil.java
@@ -18,21 +18,19 @@
  */
 package org.apache.johnzon.core;
 
-public class JsonPointerUtil {
-
+public final class JsonPointerUtil {
     private JsonPointerUtil() {
-
+        // no-op
     }
 
     /**
      * Transforms "~" to "~0" and then "/" to "~1"
      */
     public static String encode(String s) {
-        if (s == null || s.length() == 0) {
+        if (s == null || s.isEmpty()) {
             return s;
         }
-
-        return s.replace("~", "~0").replace("/", "~1");
+        return replace(replace(s, "~", "~0"), "/", "~1");
     }
 
     /**
@@ -42,8 +40,18 @@ public class JsonPointerUtil {
         if (s == null || s.length() == 0) {
             return s;
         }
-
-        return s.replace("~1", "/").replace("~0", "~");
+        return replace(replace(s, "~1", "/"), "~0", "~");
     }
 
+    // regex have too much overhead (Matcher + Pattern) at runtime even when precompiled
+    private static String replace(final String src, final String from, final String to) {
+        if (src.isEmpty()) {
+            return src;
+        }
+        final int start = src.indexOf(from);
+        if (start >= 0) {
+            return src.substring(0, start) + to + replace(src.substring(start + from.length()), from, to);
+        }
+        return src;
+    }
 }
diff --git a/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPatchDiffTest.java b/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPatchDiffTest.java
index d591b98..fe53f37 100644
--- a/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPatchDiffTest.java
+++ b/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPatchDiffTest.java
@@ -16,16 +16,21 @@
  */
 package org.apache.johnzon.core;
 
+import static java.util.Collections.singletonMap;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
 import java.io.StringReader;
+import java.io.StringWriter;
 
 import javax.json.Json;
 import javax.json.JsonArray;
 import javax.json.JsonObject;
 import javax.json.JsonPatch;
+import javax.json.JsonReader;
 import javax.json.JsonValue;
+import javax.json.JsonWriter;
+import javax.json.stream.JsonGenerator;
 
 import org.junit.Assert;
 import org.junit.Ignore;
@@ -33,6 +38,61 @@ import org.junit.Test;
 
 public class JsonPatchDiffTest {
     @Test
+    public void nestedObjects() {
+        final String source = "{\n" +
+                "  \"caseDiscussionDailySchedule\":{\n" +
+                "    \"schedule\":{\n" +
+                "      \"TUESDAY\":[\n" +
+                "        {\n" +
+                "          \"start\":\"07:00+03:00\",\n" +
+                "          \"end\":\"08:00+03:00\"\n" +
+                "        }\n" +
+                "      ],\n" +
+                "      \"MONDAY\":[\n" +
+                "        {\n" +
+                "          \"start\":\"07:00+03:00\",\n" +
+                "          \"end\":\"08:00+03:00\"\n" +
+                "        }\n" +
+                "      ]\n" +
+                "    }\n" +
+                "  }\n" +
+                "}";
+        final String operation = "[" +
+                "{\"op\":\"replace\",\"path\":\"/caseDiscussionDailySchedule/schedule/MONDAY/0/start\",\"value\":null}" +
+                "]";
+        final String expectedResult = "{\n" +
+                "  \"caseDiscussionDailySchedule\":{\n" +
+                "    \"schedule\":{\n" +
+                "      \"TUESDAY\":[\n" +
+                "        {\n" +
+                "          \"start\":\"07:00+03:00\",\n" +
+                "          \"end\":\"08:00+03:00\"\n" +
+                "        }\n" +
+                "      ],\n" +
+                "      \"MONDAY\":[\n" +
+                "        {\n" +
+                "          \"end\":\"08:00+03:00\",\n" +
+                "          \"start\":null\n" +
+                "        }\n" +
+                "      ]\n" +
+                "    }\n" +
+                "  }\n" +
+                "}";
+        final JsonReader reader1 = Json.createReader(new StringReader(source));
+        final JsonObject src = reader1.readObject();
+        reader1.close();
+        final JsonReader reader2 = Json.createReader(new StringReader(operation));
+        final JsonArray op = reader2.readArray();
+        reader2.close();
+        final JsonObject result = Json.createPatch(op).apply(src);
+        final StringWriter formattedResult = new StringWriter();
+        final JsonWriter writer = Json.createWriterFactory(singletonMap(JsonGenerator.PRETTY_PRINTING, true))
+                .createWriter(formattedResult);
+        writer.write(result);
+        writer.close();
+        assertEquals(expectedResult, formattedResult.toString());
+    }
+    @Test
     public void fromEmptyArray() {
         final JsonObject from = Json.createObjectBuilder().add("testEmpty", JsonValue.EMPTY_JSON_ARRAY).build();
         final JsonObject to = Json.createObjectBuilder()
diff --git a/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPointerUtilTest.java b/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPointerUtilTest.java
index 73b0e01..435b709 100644
--- a/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPointerUtilTest.java
+++ b/johnzon-core/src/test/java/org/apache/johnzon/core/JsonPointerUtilTest.java
@@ -95,4 +95,14 @@ public class JsonPointerUtilTest {
         assertEquals("~1", decodedString);
     }
 
+    @Test
+    public void someComplexRoundTrip() {
+        assertRoundTrip("/a/b/e/~foo", "~1a~1b~1e~1~0foo");
+        assertRoundTrip("/first/0/second~almost/third~another/last", "~1first~10~1second~0almost~1third~0another~1last");
+    }
+
+    private void assertRoundTrip(final String clear, final String encoded) {
+        assertEquals(encoded, JsonPointerUtil.encode(clear));
+        assertEquals(clear, JsonPointerUtil.decode(encoded));
+    }
 }