You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by bu...@apache.org on 2014/03/22 02:02:39 UTC
[1/3] git commit: ACCUMULO-2487 match Value implementation to
javadocs. check for null args.
Repository: accumulo
Updated Branches:
refs/heads/1.6.0-SNAPSHOT 828c3b3ee -> f2920c266
refs/heads/master a03071cd3 -> c5b8335d3
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/1.6.0-SNAPSHOT
Commit: f2920c266ff956c190eda86deb07b4ef590d1965
Parents: 828c3b3
Author: Sean Busbey <bu...@cloudera.com>
Authored: Thu Mar 20 15:43:02 2014 -0500
Committer: Sean Busbey <bu...@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
[3/3] git commit: Merge branch '1.6.0-SNAPSHOT'
Posted by bu...@apache.org.
Merge branch '1.6.0-SNAPSHOT'
Conflicts:
core/src/main/java/org/apache/accumulo/core/data/Value.java
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/c5b8335d
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/c5b8335d
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/c5b8335d
Branch: refs/heads/master
Commit: c5b8335d31c7f318a903c52dc6d4195f9642137e
Parents: a03071c f2920c2
Author: Sean Busbey <bu...@cloudera.com>
Authored: Fri Mar 21 19:50:05 2014 -0500
Committer: Sean Busbey <bu...@cloudera.com>
Committed: Fri Mar 21 19:50:05 2014 -0500
----------------------------------------------------------------------
.../org/apache/accumulo/core/data/Value.java | 42 ++++++++++----------
.../apache/accumulo/core/data/ValueTest.java | 33 ++++++++++++---
2 files changed, 49 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/accumulo/blob/c5b8335d/core/src/main/java/org/apache/accumulo/core/data/Value.java
----------------------------------------------------------------------
diff --cc core/src/main/java/org/apache/accumulo/core/data/Value.java
index ec07411,11e60e1..b3ae5b8
--- a/core/src/main/java/org/apache/accumulo/core/data/Value.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/Value.java
@@@ -36,42 -38,38 +38,45 @@@ import org.apache.hadoop.io.WritableCom
* 'immutable'.
*/
public class Value implements WritableComparable<Object> {
+ private static final byte[] EMPTY = new byte[0];
protected byte[] value;
-
+
/**
- * Create a zero-size sequence.
+ * Creates a zero-size sequence.
*/
public Value() {
- super();
+ this(EMPTY, false);
}
/**
- * Create a Value using the byte array as the initial value.
+ * Creates a Value using a byte array as the initial value. The given byte
+ * array is used directly as the backing array, so later changes made to the
+ * array reflect into the new Value.
*
- * @param bytes bytes of value
+ * @param bytes May not be null
*/
public Value(byte[] bytes) {
this(bytes, false);
}
/**
- * Create a Value using a copy of the ByteBuffer's content.
+ * Creates a Value using the bytes in a buffer as the initial value. Makes
+ * a defensive copy.
*
- * @param bytes bytes of value
+ * @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.
+ *
+ * @param bytes bytes of value
+ * @param copy false to use the backing array of the buffer directly as the
+ * backing array, true to force a copy
*/
@Deprecated
public Value(ByteBuffer bytes, boolean copy) {
@@@ -79,13 -78,12 +85,14 @@@
}
/**
- * Create a Value based on the given bytes.
+ * Creates a Value using a byte array as the initial value.
+ *
- * @param bytes bytes of value
+ * @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.
+ * @param copy false to use the given byte array directly as the backing
+ * array, true to force a copy
*/
public Value(byte[] bytes, boolean copy) {
+ Preconditions.checkNotNull(bytes);
if (!copy) {
this.value = bytes;
} else {
@@@ -96,74 -94,60 +103,69 @@@
}
/**
- * Set the new Value to a copy of the contents of the passed <code>ibw</code>.
+ * Creates a new Value based on another.
*
- * @param ibw original Value
+ * @param ibw may not be null.
*/
public Value(final Value ibw) {
this(ibw.get(), 0, ibw.getSize());
}
/**
- * Set the value to a copy of the given byte range
+ * Creates a Value based on a range in a byte array. A copy of the bytes is
+ * always made.
*
- * @param newData byte array containing value bytes
+ * @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
+ * @param offset the offset in newData to start with for valye bytes
+ * @param length the number of bytes in the value
+ * @throws IndexOutOfBoundsException if offset or length are invalid
*/
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);
}
/**
+ * Gets the byte data of this value.
+ *
- * @return value bytes
- * @throws IllegalStateException if this object is uninitialized because it
- * was not read correctly from a data stream
+ * @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, may not be null.
+ * Sets the byte data of this value. The given byte array is used directly as
+ * the backing array, so later changes made to the array reflect into this
+ * Value.
+ *
- * @param b bytes of value
++ * @param b may not be null
*/
public void set(final byte[] b) {
+ Preconditions.checkNotNull(b);
this.value = b;
}
/**
- *
- * @param b copy the given byte array, may not be null.
+ * Sets the byte data of this value. The given byte array is copied.
+ *
- * @param b bytes of value
++ * @param b 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 underlying buffer.
+ * Gets the size of this value.
+ *
+ * @return size in bytes
- * @throws IllegalStateException if this object is uninitialized because it
- * was not read correctly from a data stream
*/
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;
}
[2/3] git commit: ACCUMULO-2487 match Value implementation to
javadocs. check for null args.
Posted by bu...@apache.org.
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 <bu...@cloudera.com>
Authored: Thu Mar 20 15:43:02 2014 -0500
Committer: Sean Busbey <bu...@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