You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2012/07/13 19:31:35 UTC

svn commit: r1361306 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/kernel/ test/java/org/apache/jackrabbit/oak/kernel/

Author: jukka
Date: Fri Jul 13 17:31:34 2012
New Revision: 1361306

URL: http://svn.apache.org/viewvc?rev=1361306&view=rev
Log:
OAK-188: Invalid JSOP encoding in CommitBuilder

Fix a similar problem also in the KernelNodeStoreBranch class

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/CoreValueMapper.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/CoreValueFactoryTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/CoreValueMapper.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/CoreValueMapper.java?rev=1361306&r1=1361305&r2=1361306&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/CoreValueMapper.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/CoreValueMapper.java Fri Jul 13 17:31:34 2012
@@ -34,7 +34,7 @@ import org.apache.jackrabbit.oak.api.Cor
  */
 public class CoreValueMapper {
 
-    private static final Map<Integer, String> TYPE2HINT = new HashMap<Integer, String>();
+    public static final Map<Integer, String> TYPE2HINT = new HashMap<Integer, String>();
     private static final Map<String, Integer> HINT2TYPE = new HashMap<String, Integer>();
 
     static {
@@ -51,61 +51,6 @@ public class CoreValueMapper {
     private CoreValueMapper() {
     }
 
-    /**
-     * Returns the internal JSON representation of the specified {@code value}
-     * that is stored in the MicroKernel. All property types that are not
-     * reflected as JSON types are converted to strings and get a type prefix.
-     *
-     * @param value The core value to be converted.
-     * @return The encoded JSON string.
-     */
-    public static String toJsonValue(CoreValue value) {
-        String jsonString;
-        switch (value.getType()) {
-            case PropertyType.BOOLEAN:
-                jsonString = Boolean.toString(value.getBoolean());
-                break;
-            case PropertyType.LONG:
-                jsonString = Long.toString(value.getLong());
-                break;
-            case PropertyType.STRING:
-                String str = value.getString();
-                if (startsWithHint(str)) {
-                    jsonString = buildJsonStringWithHint(value);
-                } else {
-                    jsonString = jsonEncode(value.getString());
-                }
-                break;
-            default:
-                // any other type
-                jsonString = buildJsonStringWithHint(value);
-        }
-        return jsonString;
-    }
-
-    /**
-     * Returns an JSON array containing the JSON representation of the
-     * specified values.
-     *
-     * @param values The values to be converted to a JSON array.
-     * @return JSON array containing the JSON representation of the specified
-     * values.
-     * @see #toJsonValue(org.apache.jackrabbit.oak.api.CoreValue)
-     */
-    public static String toJsonArray(Iterable<CoreValue> values) {
-        StringBuilder sb = new StringBuilder();
-        sb.append('[');
-        for (CoreValue cv : values) {
-            sb.append(toJsonValue(cv));
-            sb.append(',');
-        }
-        if (sb.length() > 1) {
-            sb.deleteCharAt(sb.length() - 1);
-        }
-        sb.append(']');
-        return sb.toString();
-    }
-
     public static CoreValue fromJsopReader(JsopReader reader, MicroKernel kernel) {
         return fromJsopReader(reader, new CoreValueFactoryImpl(kernel));
     }
@@ -158,24 +103,6 @@ public class CoreValueMapper {
         return values;
     }
 
-    //--------------------------------------------------------------------------
-    /**
-     * Build the JSON representation of the specified value consisting of
-     * a leading type hint, followed by ':" and the String conversion of this
-     * value.
-     *
-     * @param value The value to be serialized.
-     * @return The string representation of the specified value including a
-     * leading type hint.
-     */
-    private static String buildJsonStringWithHint(CoreValue value) {
-        StringBuilder sb = new StringBuilder();
-        sb.append(TYPE2HINT.get(value.getType()));
-        sb.append(':');
-        sb.append(value.getString());
-        return jsonEncode(sb.toString());
-    }
-
     /**
      * Returns {@code true} if the specified JSON String represents a value
      * serialization that includes a leading type hint.
@@ -185,63 +112,8 @@ public class CoreValueMapper {
      * hint; {@code false} otherwise.
      * @see #buildJsonStringWithHint(org.apache.jackrabbit.oak.api.CoreValue)
      */
-    private static boolean startsWithHint(String jsonString) {
+    public static boolean startsWithHint(String jsonString) {
         return jsonString.length() >= 4 && jsonString.charAt(3) == ':';
     }
 
-    /**
-     * Escape quotes, \, /, \r, \n, \b, \f, \t and other control characters
-     * (U+0000 through U+001F) and surround with double quotes.
-     */
-    private static String jsonEncode(String value) {
-        if (value == null) {
-            return null;
-        }
-
-        StringBuilder sb = new StringBuilder("\"");
-        for (int i = 0; i < value.length(); i++) {
-            char ch = value.charAt(i);
-            switch (ch) {
-                case '"':
-                    sb.append("\\\"");
-                    break;
-                case '\\':
-                    sb.append("\\\\");
-                    break;
-                case '\b':
-                    sb.append("\\b");
-                    break;
-                case '\f':
-                    sb.append("\\f");
-                    break;
-                case '\n':
-                    sb.append("\\n");
-                    break;
-                case '\r':
-                    sb.append("\\r");
-                    break;
-                case '\t':
-                    sb.append("\\t");
-                    break;
-                default:
-                    //Reference: http://www.unicode.org/versions/Unicode5.1.0/
-                    if (ch >= '\u0000' && ch <= '\u001F' ||
-                            ch >= '\u007F' && ch <= '\u009F' ||
-                            ch >= '\u2000' && ch <= '\u20FF') {
-
-                        String ss = Integer.toHexString(ch);
-                        sb.append("\\u");
-                        for (int k = 0; k < 4 - ss.length(); k++) {
-                            sb.append('0');
-                        }
-                        sb.append(ss.toUpperCase());
-                    } else {
-                        sb.append(ch);
-                    }
-            }
-        }
-
-        return sb.append('"').toString();
-    }
-
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java?rev=1361306&r1=1361305&r2=1361306&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeStoreBranch.java Fri Jul 13 17:31:34 2012
@@ -16,9 +16,13 @@
  */
 package org.apache.jackrabbit.oak.kernel;
 
+import javax.jcr.PropertyType;
+
 import org.apache.jackrabbit.mk.api.MicroKernel;
 import org.apache.jackrabbit.mk.api.MicroKernelException;
+import org.apache.jackrabbit.mk.json.JsopBuilder;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.CoreValue;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.spi.commit.CommitEditor;
@@ -30,6 +34,7 @@ import org.apache.jackrabbit.oak.spi.sta
 import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
 import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
 import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
+import static org.apache.jackrabbit.oak.kernel.CoreValueMapper.TYPE2HINT;
 
 /**
  * {@code NodeStoreBranch} based on {@link MicroKernel} branching and merging.
@@ -163,40 +168,40 @@ class KernelNodeStoreBranch implements N
     }
 
     private String buildJsop() {
-        StringBuilder jsop = new StringBuilder();
+        JsopBuilder jsop = new JsopBuilder();
         diffToJsop(committed, currentRoot, "", jsop);
         return jsop.toString();
     }
 
     private static void diffToJsop(NodeState before, NodeState after, final String path,
-            final StringBuilder jsop) {
+            final JsopBuilder jsop) {
         after.compareAgainstBaseState(before, new NodeStateDiff() {
             @Override
             public void propertyAdded(PropertyState after) {
-                jsop.append('^').append(buildPath(after.getName()))
-                        .append(':').append(toJson(after));
+                jsop.tag('^').key(buildPath(after.getName()));
+                toJson(after, jsop);
             }
 
             @Override
             public void propertyChanged(PropertyState before, PropertyState after) {
-                jsop.append('^').append(buildPath(after.getName()))
-                        .append(':').append(toJson(after));
+                jsop.tag('^').key(buildPath(after.getName()));
+                toJson(after, jsop);
             }
 
             @Override
             public void propertyDeleted(PropertyState before) {
-                jsop.append('^').append(buildPath(before.getName())).append(":null");
+                jsop.tag('^').key(buildPath(before.getName())).value(null);
             }
 
             @Override
             public void childNodeAdded(String name, NodeState after) {
-                jsop.append('+').append(buildPath(name)).append(':');
-                toJson(after);
+                jsop.tag('+').key(buildPath(name));
+                toJson(after, jsop);
             }
 
             @Override
             public void childNodeDeleted(String name, NodeState before) {
-                jsop.append('-').append(buildPath(name));
+                jsop.tag('-').value(buildPath(name));
             }
 
             @Override
@@ -205,33 +210,50 @@ class KernelNodeStoreBranch implements N
             }
 
             private String buildPath(String name) {
-                return '"' + PathUtils.concat(path, name) + '"';
+                return PathUtils.concat(path, name);
             }
 
-            private String toJson(PropertyState propertyState) {
-                return propertyState.isArray()
-                    ? CoreValueMapper.toJsonArray(propertyState.getValues())
-                    : CoreValueMapper.toJsonValue(propertyState.getValue());
+            private void toJson(NodeState nodeState, JsopBuilder jsop) {
+                jsop.object();
+                for (PropertyState property : nodeState.getProperties()) {
+                    jsop.key(property.getName());
+                    toJson(property, jsop);
+                }
+                for (ChildNodeEntry child : nodeState.getChildNodeEntries()) {
+                    jsop.key(child.getName());
+                    toJson(child.getNodeState(), jsop);
+                }
+                jsop.endObject();
             }
 
-            private void toJson(NodeState nodeState) {
-                jsop.append('{');
-                String comma = "";
-                for (PropertyState property : nodeState.getProperties()) {
-                    String value = toJson(property);
-                    jsop.append(comma);
-                    comma = ",";
-                    jsop.append('"').append(property.getName()).append("\":").append(value);
+            private void toJson(PropertyState propertyState, JsopBuilder jsop) {
+                if (propertyState.isArray()) {
+                    jsop.array();
+                    for (CoreValue value : propertyState.getValues()) {
+                        toJson(value, jsop);
+                    }
+                    jsop.endArray();
+                } else {
+                    toJson(propertyState.getValue(), jsop);
                 }
+            }
 
-                for (ChildNodeEntry child : nodeState.getChildNodeEntries()) {
-                    jsop.append(comma);
-                    comma = ",";
-                    jsop.append('"').append(child.getName()).append("\":");
-                    toJson(child.getNodeState());
+            private void toJson(CoreValue value, JsopBuilder jsop) {
+                int type = value.getType();
+                if (type == PropertyType.BOOLEAN) {
+                    jsop.value(value.getBoolean());
+                } else if (type == PropertyType.LONG) {
+                    jsop.value(value.getLong());
+                } else {
+                    String string = value.getString();
+                    if (type != PropertyType.STRING
+                            || CoreValueMapper.startsWithHint(string)) {
+                        string = TYPE2HINT.get(type) + ':' + string;
+                    }
+                    jsop.value(string);
                 }
-                jsop.append('}');
             }
+
         });
     }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/CoreValueFactoryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/CoreValueFactoryTest.java?rev=1361306&r1=1361305&r2=1361306&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/CoreValueFactoryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/CoreValueFactoryTest.java Fri Jul 13 17:31:34 2012
@@ -142,14 +142,6 @@ public class CoreValueFactoryTest {
     }
 
     @Test
-    public void testToJsonValue() throws IOException {
-        for (CoreValue v : singleValueMap.keySet()) {
-            String json = singleValueMap.get(v);
-            assertEquals(json, CoreValueMapper.toJsonValue(v));
-        }
-    }
-
-    @Test
     public void testFromJsonValue() throws IOException {
         for (CoreValue v : singleValueMap.keySet()) {
             String json = singleValueMap.get(v);
@@ -159,14 +151,6 @@ public class CoreValueFactoryTest {
     }
 
     @Test
-    public void testToJsonArray() throws IOException {
-        for (String json : mvValueMap.keySet()) {
-            List<CoreValue> values = mvValueMap.get(json);
-            assertEquals(json, CoreValueMapper.toJsonArray(values));
-        }
-    }
-
-    @Test
     public void testListFromJsopReader() throws IOException {
         for (String json : mvValueMap.keySet()) {
             List<CoreValue> values = mvValueMap.get(json);