ACCUMULO-2487 match Value implementation to javadocs. check for null args.
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/f2920c26 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/f2920c26 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/f2920c26 Branch: refs/heads/master Commit: f2920c266ff956c190eda86deb07b4ef590d1965 Parents: 828c3b3 Author: Sean Busbey <bus...@cloudera.com> Authored: Thu Mar 20 15:43:02 2014 -0500 Committer: Sean Busbey <bus...@cloudera.com> Committed: Fri Mar 21 19:45:49 2014 -0500 ---------------------------------------------------------------------- .../org/apache/accumulo/core/data/Value.java | 54 +++++++++++--------- .../apache/accumulo/core/data/ValueTest.java | 33 +++++++++--- 2 files changed, 58 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/f2920c26/core/src/main/java/org/apache/accumulo/core/data/Value.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/data/Value.java b/core/src/main/java/org/apache/accumulo/core/data/Value.java index 39ebbd0..11e60e1 100644 --- a/core/src/main/java/org/apache/accumulo/core/data/Value.java +++ b/core/src/main/java/org/apache/accumulo/core/data/Value.java @@ -25,6 +25,8 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.List; +import com.google.common.base.Preconditions; + import org.apache.accumulo.core.Constants; import org.apache.hadoop.io.BytesWritable; import org.apache.hadoop.io.WritableComparable; @@ -36,39 +38,52 @@ import org.apache.hadoop.io.WritableComparator; * 'immutable'. */ public class Value implements WritableComparable<Object> { + private static final byte[] EMPTY = new byte[0]; protected byte[] value; - + /** * Create a zero-size sequence. */ public Value() { - super(); + this(EMPTY, false); } /** * Create a Value using the byte array as the initial value. * - * @param bytes - * This array becomes the backing storage for the object. + * @param bytes May not be null */ - public Value(byte[] bytes) { this(bytes, false); } + /** + * Create a Value using a copy of the ByteBuffer's content. + * + * @param bytes May not be null + */ public Value(ByteBuffer bytes) { + /* TODO ACCUMULO-2509 right now this uses the entire backing array, which must be accessible. */ this(toBytes(bytes), false); } /** + * @param bytes may not be null * @deprecated A copy of the bytes in the buffer is always made. Use {@link #Value(ByteBuffer)} instead. */ @Deprecated public Value(ByteBuffer bytes, boolean copy) { + /* TODO ACCUMULO-2509 right now this uses the entire backing array, which must be accessible. */ this(toBytes(bytes), false); } + /** + * Create a Value based on the given bytes. + * @param bytes may not be null + * @param copy signal if Value must make its own copy of bytes, or if it can use the array directly. + */ public Value(byte[] bytes, boolean copy) { + Preconditions.checkNotNull(bytes); if (!copy) { this.value = bytes; } else { @@ -81,8 +96,7 @@ public class Value implements WritableComparable<Object> { /** * Set the new Value to a copy of the contents of the passed <code>ibw</code>. * - * @param ibw - * the value to set this Value to. + * @param ibw may not be null. */ public Value(final Value ibw) { this(ibw.get(), 0, ibw.getSize()); @@ -91,55 +105,49 @@ public class Value implements WritableComparable<Object> { /** * Set the value to a copy of the given byte range * - * @param newData - * the new values to copy in + * @param newData source of copy, may not be null * @param offset * the offset in newData to start at * @param length * the number of bytes to copy */ public Value(final byte[] newData, final int offset, final int length) { + Preconditions.checkNotNull(newData); this.value = new byte[length]; System.arraycopy(newData, offset, this.value, 0, length); } /** - * Get the data from the BytesWritable. - * - * @return The data is only valid between 0 and getSize() - 1. + * @return the underlying byte array directly. */ public byte[] get() { - if (this.value == null) { - throw new IllegalStateException("Uninitialized. Null constructor " + "called w/o accompanying readFields invocation"); - } + assert(null != value); return this.value; } /** - * @param b - * Use passed bytes as backing array for this instance. + * @param b Use passed bytes as backing array for this instance, may not be null. */ public void set(final byte[] b) { + Preconditions.checkNotNull(b); this.value = b; } /** * - * @param b - * copy bytes + * @param b copy the given byte array, may not be null. */ public void copy(byte[] b) { + Preconditions.checkNotNull(b); this.value = new byte[b.length]; System.arraycopy(b, 0, this.value, 0, b.length); } /** - * @return the current size of the buffer. + * @return the current size of the underlying buffer. */ public int getSize() { - if (this.value == null) { - throw new IllegalStateException("Uninitialized. Null constructor " + "called w/o accompanying readFields invocation"); - } + assert(null != value); return this.value.length; } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f2920c26/core/src/test/java/org/apache/accumulo/core/data/ValueTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/data/ValueTest.java b/core/src/test/java/org/apache/accumulo/core/data/ValueTest.java index 9b68222..c7b9db5 100644 --- a/core/src/test/java/org/apache/accumulo/core/data/ValueTest.java +++ b/core/src/test/java/org/apache/accumulo/core/data/ValueTest.java @@ -47,10 +47,31 @@ public class ValueTest { DATABUFF.rewind(); } - @Test(expected = IllegalStateException.class) - public void testNull() { + @Test + public void testDefault() { Value v = new Value(); - v.get(); + assertEquals(0, v.get().length); + } + + @Test(expected=NullPointerException.class) + public void testNullBytesConstructor() { + Value v = new Value((byte[]) null); + } + + @Test(expected=NullPointerException.class) + public void testNullCopyConstructor() { + Value v = new Value((Value) null); + } + + @Test(expected=NullPointerException.class) + public void testNullByteBufferConstructor() { + Value v = new Value((ByteBuffer)null); + } + + @Test(expected=NullPointerException.class) + public void testNullSet() { + Value v = new Value(); + v.set(null); } @Test @@ -118,10 +139,10 @@ public class ValueTest { assertEquals(DATA.length, v.getSize()); } - @Test(expected = IllegalStateException.class) - public void testGetSize_Null() { + @Test + public void testGetSizeDefault() { Value v = new Value(); - v.getSize(); + assertEquals(0, v.getSize()); } @Test
