accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [1/5] accumulo git commit: ACCUMULO-4113 Fix incorrect usage of ByteBuffer
Date Wed, 20 Jan 2016 19:44:16 GMT
Repository: accumulo
Updated Branches:
  refs/heads/master a12b3741a -> 739f906cf


ACCUMULO-4113 Fix incorrect usage of ByteBuffer


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/27300d81
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/27300d81
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/27300d81

Branch: refs/heads/master
Commit: 27300d81eaee14516e705d0032bc0bdc46880d4c
Parents: a2c2d38
Author: Keith Turner <kturner@apache.org>
Authored: Wed Jan 20 12:36:20 2016 -0500
Committer: Keith Turner <kturner@apache.org>
Committed: Wed Jan 20 12:36:20 2016 -0500

----------------------------------------------------------------------
 .../accumulo/core/data/ArrayByteSequence.java   | 12 ++-
 .../accumulo/core/util/ByteBufferUtil.java      | 34 +++++--
 .../core/util/UnsynchronizedBuffer.java         |  4 +-
 .../accumulo/core/util/ByteBufferUtilTest.java  | 98 ++++++++++++++++++++
 4 files changed, 132 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/27300d81/core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java b/core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java
index d9ddc67..d040139 100644
--- a/core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java
@@ -21,6 +21,8 @@ import static com.google.common.base.Charsets.UTF_8;
 import java.io.Serializable;
 import java.nio.ByteBuffer;
 
+import org.apache.accumulo.core.util.ByteBufferUtil;
+
 public class ArrayByteSequence extends ByteSequence implements Serializable {
 
   private static final long serialVersionUID = 1L;
@@ -52,15 +54,14 @@ public class ArrayByteSequence extends ByteSequence implements Serializable
{
   }
 
   public ArrayByteSequence(ByteBuffer buffer) {
-    this.length = buffer.remaining();
-
     if (buffer.hasArray()) {
       this.data = buffer.array();
-      this.offset = buffer.position();
+      this.offset = buffer.position() + buffer.arrayOffset();
+      this.length = buffer.remaining();
     } else {
-      this.data = new byte[length];
       this.offset = 0;
-      buffer.get(data);
+      this.data = ByteBufferUtil.toBytes(buffer);
+      this.length = data.length;
     }
   }
 
@@ -118,6 +119,7 @@ public class ArrayByteSequence extends ByteSequence implements Serializable
{
     return copy;
   }
 
+  @Override
   public String toString() {
     return new String(data, offset, length, UTF_8);
   }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/27300d81/core/src/main/java/org/apache/accumulo/core/util/ByteBufferUtil.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/util/ByteBufferUtil.java b/core/src/main/java/org/apache/accumulo/core/util/ByteBufferUtil.java
index be5cddf..cdde05c 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/ByteBufferUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/ByteBufferUtil.java
@@ -31,7 +31,15 @@ public class ByteBufferUtil {
   public static byte[] toBytes(ByteBuffer buffer) {
     if (buffer == null)
       return null;
-    return Arrays.copyOfRange(buffer.array(), buffer.position(), buffer.limit());
+    if (buffer.hasArray()) {
+      // did not use buffer.get() because it changes the position
+      return Arrays.copyOfRange(buffer.array(), buffer.position() + buffer.arrayOffset(),
buffer.limit() + buffer.arrayOffset());
+    } else {
+      byte[] data = new byte[buffer.remaining()];
+      // duplicate inorder to avoid changing position
+      buffer.duplicate().get(data);
+      return data;
+    }
   }
 
   public static List<ByteBuffer> toByteBuffers(Collection<byte[]> bytesList)
{
@@ -47,23 +55,32 @@ public class ByteBufferUtil {
   public static List<byte[]> toBytesList(Collection<ByteBuffer> bytesList) {
     if (bytesList == null)
       return null;
-    ArrayList<byte[]> result = new ArrayList<byte[]>();
+    ArrayList<byte[]> result = new ArrayList<byte[]>(bytesList.size());
     for (ByteBuffer bytes : bytesList) {
       result.add(toBytes(bytes));
     }
     return result;
   }
 
-  public static Text toText(ByteBuffer bytes) {
-    if (bytes == null)
+  public static Text toText(ByteBuffer byteBuffer) {
+    if (byteBuffer == null)
       return null;
-    Text result = new Text();
-    result.set(bytes.array(), bytes.position(), bytes.remaining());
-    return result;
+
+    if (byteBuffer.hasArray()) {
+      Text result = new Text();
+      result.set(byteBuffer.array(), byteBuffer.arrayOffset() + byteBuffer.position(), byteBuffer.remaining());
+      return result;
+    } else {
+      return new Text(toBytes(byteBuffer));
+    }
   }
 
   public static String toString(ByteBuffer bytes) {
-    return new String(bytes.array(), bytes.position(), bytes.remaining(), UTF_8);
+    if (bytes.hasArray()) {
+      return new String(bytes.array(), bytes.arrayOffset() + bytes.position(), bytes.remaining(),
UTF_8);
+    } else {
+      return new String(toBytes(bytes), UTF_8);
+    }
   }
 
   public static ByteBuffer toByteBuffers(ByteSequence bs) {
@@ -73,7 +90,6 @@ public class ByteBufferUtil {
     if (bs.isBackedByArray()) {
       return ByteBuffer.wrap(bs.getBackingArray(), bs.offset(), bs.length());
     } else {
-      // TODO create more efficient impl
       return ByteBuffer.wrap(bs.toArray());
     }
   }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/27300d81/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java b/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
index f353613..b776553 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
@@ -122,8 +122,8 @@ public class UnsynchronizedBuffer {
         offset = buffer.arrayOffset() + buffer.position();
         data = buffer.array();
       } else {
-        data = new byte[buffer.remaining()];
-        buffer.get(data);
+        offset = 0;
+        data = ByteBufferUtil.toBytes(buffer);
       }
     }
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/27300d81/core/src/test/java/org/apache/accumulo/core/util/ByteBufferUtilTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/util/ByteBufferUtilTest.java b/core/src/test/java/org/apache/accumulo/core/util/ByteBufferUtilTest.java
new file mode 100644
index 0000000..f4ca0e9
--- /dev/null
+++ b/core/src/test/java/org/apache/accumulo/core/util/ByteBufferUtilTest.java
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.accumulo.core.util;
+
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.accumulo.core.data.ArrayByteSequence;
+import org.apache.hadoop.io.Text;
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.google.common.base.Charsets;
+
+public class ByteBufferUtilTest {
+
+  private static void assertEquals(String expected, ByteBuffer bb) {
+    Assert.assertEquals(new Text(expected), ByteBufferUtil.toText(bb));
+    Assert.assertEquals(expected, new String(ByteBufferUtil.toBytes(bb), Charsets.UTF_8));
+    Assert.assertEquals(expected, ByteBufferUtil.toString(bb));
+
+    List<byte[]> bal = ByteBufferUtil.toBytesList(Collections.singletonList(bb));
+    Assert.assertEquals(1, bal.size());
+    Assert.assertEquals(expected, new String(bal.get(0), Charsets.UTF_8));
+
+    Assert.assertEquals(new ArrayByteSequence(expected), new ArrayByteSequence(bb));
+  }
+
+  @Test
+  public void testNonZeroArrayOffset() {
+    byte[] data = "0123456789".getBytes(Charsets.UTF_8);
+
+    ByteBuffer bb1 = ByteBuffer.wrap(data, 3, 4);
+
+    // create a ByteBuffer with a non-zero array offset
+    ByteBuffer bb2 = bb1.slice();
+
+    // The purpose of this test is to ensure ByteBufferUtil code works when arrayOffset is
non-zero. The following asserts are not to test ByteBuffer, but
+    // ensure the behavior of slice() is as expected.
+
+    Assert.assertEquals(3, bb2.arrayOffset());
+    Assert.assertEquals(0, bb2.position());
+    Assert.assertEquals(4, bb2.limit());
+
+    // start test with non zero arrayOffset
+    assertEquals("3456", bb2);
+
+    // read one byte from byte buffer... this should cause position to be non-zero in addition
to array offset
+    bb2.get();
+    assertEquals("456", bb2);
+
+  }
+
+  @Test
+  public void testZeroArrayOffsetAndNonZeroPosition() {
+    byte[] data = "0123456789".getBytes(Charsets.UTF_8);
+    ByteBuffer bb1 = ByteBuffer.wrap(data, 3, 4);
+
+    assertEquals("3456", bb1);
+  }
+
+  @Test
+  public void testZeroArrayOffsetAndPosition() {
+    byte[] data = "0123456789".getBytes(Charsets.UTF_8);
+    ByteBuffer bb1 = ByteBuffer.wrap(data, 0, 4);
+    assertEquals("0123", bb1);
+  }
+
+  @Test
+  public void testDirectByteBuffer() {
+    // allocate direct so it does not have a backing array
+    ByteBuffer bb = ByteBuffer.allocateDirect(10);
+    bb.put("0123456789".getBytes(Charsets.UTF_8));
+    bb.rewind();
+
+    assertEquals("0123456789", bb);
+
+    // advance byte buffer position
+    bb.get();
+    assertEquals("123456789", bb);
+  }
+}


Mime
View raw message