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 <sylv...@datastax.com> Authored: Wed Oct 24 20:35:32 2012 +0200 Committer: Sylvain Lebresne <sylv...@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);