You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2012/10/24 20:41:24 UTC

[2/2] git commit: Fix CompositeType.{get/from}String methods

Fix CompositeType.{get/from}String methods

patch by slebresne; reviewed by jbellis for CASSANDRA-4842


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5d5207b9
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5d5207b9
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5d5207b9

Branch: refs/heads/trunk
Commit: 5d5207b9111ddbc576d67153175e5a6e27994b73
Parents: 95fb613
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Oct 24 20:35:32 2012 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Oct 24 20:35:32 2012 +0200

----------------------------------------------------------------------
 CHANGES.txt                                        |    1 +
 .../db/marshal/AbstractCompositeType.java          |  115 ++++++++++-----
 .../cassandra/db/marshal/CompositeTypeTest.java    |   35 +++++-
 3 files changed, 115 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/5d5207b9/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index f309ef1..95feb9b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -6,6 +6,7 @@
  * fix wrong leveled compaction progress calculation (CASSANDRA-4807)
  * add a close() method to CRAR to prevent leaking file descriptors (CASSANDRA-4820)
  * fix potential infinite loop in get_count (CASSANDRA-4833)
+ * fix compositeType.{get/from}String methods (CASSANDRA-4842)
 
 1.1.6
  * Wait for writes on synchronous read digest mismatch (CASSANDRA-4792)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/5d5207b9/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java b/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java
index dac94e2..5c58c8e 100644
--- a/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java
+++ b/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java
@@ -20,6 +20,7 @@ package org.apache.cassandra.db.marshal;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -125,33 +126,6 @@ public abstract class AbstractCompositeType extends AbstractType<ByteBuffer>
         return l.toArray(new ByteBuffer[l.size()]);
     }
 
-    public String getString(ByteBuffer bytes)
-    {
-        StringBuilder sb = new StringBuilder();
-        ByteBuffer bb = bytes.duplicate();
-        int i = 0;
-
-        while (bb.remaining() > 0)
-        {
-            if (bb.remaining() != bytes.remaining())
-                sb.append(":");
-
-            AbstractType<?> comparator = getAndAppendNextComparator(i, bb, sb);
-            ByteBuffer value = getWithShortLength(bb);
-
-            sb.append(comparator.getString(value));
-
-            byte b = bb.get();
-            if (b != 0)
-            {
-                sb.append(":!");
-                break;
-            }
-            ++i;
-        }
-        return sb.toString();
-    }
-
     public static class CompositeComponent
     {
         public AbstractType comparator;
@@ -185,16 +159,87 @@ public abstract class AbstractCompositeType extends AbstractType<ByteBuffer>
     }
 
     /*
-     * FIXME: this would break if some of the component string representation
-     * contains ':'. None of our current comparator do so, so this is probably
-     * not an urgent matter, but this could break for custom comparator.
-     * (DynamicCompositeType would break on '@' too)
+     * Escapes all occurences of the ':' character from the input, replacing them by "\:".
+     * Furthermore, if the last character is '\' or '!', a '!' is appended.
+     */
+    static String escape(String input)
+    {
+        if (input.isEmpty())
+            return input;
+
+        String res = input.replaceAll(":", "\\\\:");
+        char last = res.charAt(res.length() - 1);
+        return last == '\\' || last == '!' ? res + '!' : res;
+    }
+
+    /*
+     * Reverses the effect of espace().
+     * Replaces all occurences of "\:" by ":" and remove last character if it is '!'.
      */
+    static String unescape(String input)
+    {
+        if (input.isEmpty())
+            return input;
+
+        String res = input.replaceAll("\\\\:", ":");
+        char last = res.charAt(res.length() - 1);
+        return last == '!' ? res.substring(0, res.length() - 1) : res;
+    }
+
+    /*
+     * Split the input on character ':', unless the previous character is '\'.
+     */
+    static List<String> split(String input)
+    {
+        if (input.isEmpty())
+            return Collections.<String>emptyList();
+
+        List<String> res = new ArrayList<String>();
+        int prev = 0;
+        for (int i = 0; i < input.length(); i++)
+        {
+            if (input.charAt(i) != ':' || (i > 0 && input.charAt(i-1) == '\\'))
+                continue;
+
+            res.add(input.substring(prev, i));
+            prev = i + 1;
+        }
+        res.add(input.substring(prev, input.length()));
+        return res;
+    }
+
+    public String getString(ByteBuffer bytes)
+    {
+        StringBuilder sb = new StringBuilder();
+        ByteBuffer bb = bytes.duplicate();
+        int i = 0;
+
+        while (bb.remaining() > 0)
+        {
+            if (bb.remaining() != bytes.remaining())
+                sb.append(":");
+
+            AbstractType<?> comparator = getAndAppendNextComparator(i, bb, sb);
+            ByteBuffer value = getWithShortLength(bb);
+
+            sb.append(escape(comparator.getString(value)));
+
+            byte b = bb.get();
+            if (b != 0)
+            {
+                sb.append(":!");
+                break;
+            }
+            ++i;
+        }
+        return sb.toString();
+    }
+
     public ByteBuffer fromString(String source)
     {
-        String[] parts = source.split(":");
-        List<ByteBuffer> components = new ArrayList<ByteBuffer>(parts.length);
-        List<ParsedComparator> comparators = new ArrayList<ParsedComparator>(parts.length);
+        List<String> parts = split(source);
+        List<ByteBuffer> components = new ArrayList<ByteBuffer>(parts.size());
+        List<ParsedComparator> comparators = new ArrayList<ParsedComparator>(parts.size());
         int totalLength = 0, i = 0;
         boolean lastByteIsOne = false;
 
@@ -210,7 +255,7 @@ public abstract class AbstractCompositeType extends AbstractType<ByteBuffer>
             AbstractType<?> type = p.getAbstractType();
             part = p.getRemainingPart();
 
-            ByteBuffer component = type.fromString(part);
+            ByteBuffer component = type.fromString(unescape(part));
             totalLength += p.getComparatorSerializedSize() + 2 + component.remaining() + 1;
             components.add(component);
             comparators.add(p);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/5d5207b9/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java b/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
index 1bc3d70..8ce349b 100644
--- a/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
+++ b/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
@@ -20,12 +20,14 @@ package org.apache.cassandra.db.marshal;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.UUID;
 
 import org.junit.Test;
 import static org.junit.Assert.fail;
+import static org.junit.Assert.assertEquals;
 
 import org.apache.cassandra.SchemaLoader;
 import org.apache.cassandra.Util;
@@ -46,7 +48,6 @@ public class CompositeTypeTest extends SchemaLoader
         subComparators.add(TimeUUIDType.instance);
         subComparators.add(IntegerType.instance);
         comparator = CompositeType.getInstance(subComparators);
-
     }
 
     private static final int UUID_COUNT = 3;
@@ -220,6 +221,38 @@ public class CompositeTypeTest extends SchemaLoader
         assert !TypeParser.parse("CompositeType(IntegerType)").isCompatibleWith(TypeParser.parse("CompositeType(BytesType)"));
     }
 
+    @Test
+    public void testEscapeUnescape()
+    {
+        List<AbstractType<?>> subComparators = new ArrayList<AbstractType<?>>(){{;
+            add(UTF8Type.instance);
+            add(UTF8Type.instance);
+        }};
+        CompositeType comp = CompositeType.getInstance(subComparators);
+
+        String[][] inputs = new String[][]{
+            new String[]{ "foo", "bar" },
+            new String[]{ "", "" },
+            new String[]{ "foo\\", "bar" },
+            new String[]{ "foo\\:", "bar" },
+            new String[]{ "foo:", "bar" },
+            new String[]{ "foo", "b:ar" },
+            new String[]{ "foo!", "b:ar" },
+        };
+
+        for (String[] input : inputs)
+        {
+            CompositeType.Builder builder = new CompositeType.Builder(comp);
+            for (String part : input)
+                builder.add(UTF8Type.instance.fromString(part));
+
+            ByteBuffer value = comp.fromString(comp.getString(builder.build()));
+            ByteBuffer[] splitted = comp.split(value);
+            for (int i = 0; i < splitted.length; i++)
+                assertEquals(input[i], UTF8Type.instance.getString(splitted[i]));
+        }
+    }
+
     private void addColumn(RowMutation rm, ByteBuffer cname)
     {
         rm.add(new QueryPath(cfName, null , cname), ByteBufferUtil.EMPTY_BYTE_BUFFER, 0);