Repository: hbase Updated Branches: refs/heads/branch-1.0 ccbb51d74 -> 04f712208
HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified Signed-off-by: Sean Busbey <[email protected]> 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/04f71220 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/04f71220 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/04f71220 Branch: refs/heads/branch-1.0 Commit: 04f7122087d8814bcfb26098cd7e2016b8e3f0bf Parents: ccbb51d Author: Mikhail Antonov <[email protected]> Authored: Fri Mar 20 16:24:54 2015 -0700 Committer: Sean Busbey <[email protected]> Committed: Tue Mar 24 13:32:52 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/04f71220/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 c7a0871..ae0d871 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 @@ -89,7 +89,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; @@ -99,6 +99,8 @@ public class Result implements CellScannable, CellScanner { private int cellScannerIndex = INITIAL_CELLSCANNER_INDEX; private ClientProtos.RegionLoadStats stats; + 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 @@ -106,7 +108,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; } /** @@ -114,7 +125,7 @@ public class Result implements CellScannable, CellScanner { */ @Deprecated public Result(KeyValue [] cells) { - this.cells = cells; + this(cells, null, false); } /** @@ -167,6 +178,7 @@ public class Result implements CellScannable, CellScanner { this.cells = cells; this.exists = exists; this.stale = stale; + this.readonly = false; } /** @@ -837,9 +849,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; @@ -869,6 +884,7 @@ public class Result implements CellScannable, CellScanner { } public void setExists(Boolean exists) { + checkReadonly(); this.exists = exists; } @@ -884,8 +900,11 @@ public class Result implements CellScannable, CellScanner { /** * 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.stats = loadStats; } @@ -896,4 +915,14 @@ public class Result implements CellScannable, CellScanner { public ClientProtos.RegionLoadStats getStats() { return stats; } + + /** + * 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/04f71220/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.
