Repository: hbase
Updated Branches:
refs/heads/0.98 2e3b1ae26 -> 1c5000171
HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified
Signed-off-by: Sean Busbey <bus...@apache.org>
Conflicts:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/1c500017
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/1c500017
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/1c500017
Branch: refs/heads/0.98
Commit: 1c500017123a6f74b4ae53c960f2c433ac309919
Parents: 2e3b1ae
Author: Mikhail Antonov <olorinb...@gmail.com>
Authored: Fri Mar 20 16:24:54 2015 -0700
Committer: Sean Busbey <bus...@cloudera.com>
Committed: Tue Mar 24 13:38:46 2015 -0700
----------------------------------------------------------------------
.../org/apache/hadoop/hbase/client/Result.java | 35 ++++++++++++++++++--
.../apache/hadoop/hbase/client/TestResult.java | 27 +++++++++++++++
2 files changed, 59 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/1c500017/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
----------------------------------------------------------------------
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
index c220718..9a04708 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
@@ -86,7 +86,7 @@ public class Result implements CellScannable, CellScanner {
private static ThreadLocal<byte[]> localBuffer = new ThreadLocal<byte[]>();
private static final int PAD_WIDTH = 128;
- public static final Result EMPTY_RESULT = new Result();
+ public static final Result EMPTY_RESULT = new Result(true);
private final static int INITIAL_CELLSCANNER_INDEX = -1;
@@ -96,6 +96,8 @@ public class Result implements CellScannable, CellScanner {
private int cellScannerIndex = INITIAL_CELLSCANNER_INDEX;
private ClientProtos.RegionLoadStats loadStats;
+ private final boolean readonly;
+
/**
* Creates an empty Result w/ no KeyValue payload; returns null if you call
{@link #rawCells()}.
* Use this to represent no results if <code>null</code> won't do or in old
'mapred' as oppposed to 'mapreduce' package
@@ -103,7 +105,16 @@ public class Result implements CellScannable, CellScanner {
* instance with a {@link #copyFrom(Result)} call.
*/
public Result() {
- super();
+ this(false);
+ }
+
+ /**
+ * Allows to construct special purpose immutable Result objects,
+ * such as EMPTY_RESULT.
+ * @param readonly whether this Result instance is readonly
+ */
+ private Result(boolean readonly) {
+ this.readonly = readonly;
}
/**
@@ -111,7 +122,7 @@ public class Result implements CellScannable, CellScanner {
*/
@Deprecated
public Result(KeyValue [] cells) {
- this.cells = cells;
+ this(cells, null);
}
/**
@@ -152,6 +163,7 @@ public class Result implements CellScannable, CellScanner {
private Result(Cell[] cells, Boolean exists) {
this.cells = cells;
this.exists = exists;
+ this.readonly = false;
}
/**
@@ -806,9 +818,12 @@ public class Result implements CellScannable, CellScanner {
/**
* Copy another Result into this one. Needed for the old Mapred framework
+ * @throws UnsupportedOperationException if invoked on instance of
EMPTY_RESULT
+ * (which is supposed to be immutable).
* @param other
*/
public void copyFrom(Result other) {
+ checkReadonly();
this.row = null;
this.familyMap = null;
this.cells = other.cells;
@@ -838,14 +853,18 @@ public class Result implements CellScannable, CellScanner
{
}
public void setExists(Boolean exists) {
+ checkReadonly();
this.exists = exists;
}
/**
* Add load information about the region to the information about the result
* @param loadStats statistics about the current region from which this was
returned
+ * @throws UnsupportedOperationException if invoked on instance of
EMPTY_RESULT
+ * (which is supposed to be immutable).
*/
public void addResults(ClientProtos.RegionLoadStats loadStats) {
+ checkReadonly();
this.loadStats = loadStats;
}
@@ -856,4 +875,14 @@ public class Result implements CellScannable, CellScanner {
public ClientProtos.RegionLoadStats getStats() {
return loadStats;
}
+
+ /**
+ * All methods modifying state of Result object must call this method
+ * to ensure that special purpose immutable Results can't be accidentally
modified.
+ */
+ private void checkReadonly() {
+ if (readonly == true) {
+ throw new UnsupportedOperationException("Attempting to modify readonly
EMPTY_RESULT!");
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/1c500017/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
----------------------------------------------------------------------
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
index f631a20..d9496b3 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
@@ -34,6 +34,7 @@ import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.CellScanner;
import org.apache.hadoop.hbase.CellUtil;
import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.protobuf.generated.ClientProtos;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.junit.experimental.categories.Category;
@@ -228,6 +229,32 @@ public class TestResult extends TestCase {
}
}
+ /**
+ * Verifies that one can't modify instance of EMPTY_RESULT.
+ */
+ public void testEmptyResultIsReadonly() {
+ Result emptyResult = Result.EMPTY_RESULT;
+ Result otherResult = new Result();
+
+ try {
+ emptyResult.copyFrom(otherResult);
+ fail("UnsupportedOperationException should have been thrown!");
+ } catch (UnsupportedOperationException ex) {
+ LOG.debug("As expected: " + ex.getMessage());
+ }
+ try {
+
emptyResult.addResults(ClientProtos.RegionLoadStats.getDefaultInstance());
+ fail("UnsupportedOperationException should have been thrown!");
+ } catch (UnsupportedOperationException ex) {
+ LOG.debug("As expected: " + ex.getMessage());
+ }
+ try {
+ emptyResult.setExists(true);
+ fail("UnsupportedOperationException should have been thrown!");
+ } catch (UnsupportedOperationException ex) {
+ LOG.debug("As expected: " + ex.getMessage());
+ }
+ }
/**
* Microbenchmark that compares {@link Result#getValue} and {@link
Result#loadValue} performance.
