hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mberto...@apache.org
Subject [01/13] hbase git commit: HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified
Date Wed, 25 Mar 2015 16:59:50 GMT
Repository: hbase
Updated Branches:
  refs/heads/hbase-12439 e3db688a3 -> 37df6fe61 (forced update)


HBASE-13273 Make Result.EMPTY_RESULT read-only; currently it can be modified

Signed-off-by: Sean Busbey <busbey@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/8cb4f89c
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8cb4f89c
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8cb4f89c

Branch: refs/heads/hbase-12439
Commit: 8cb4f89c013b254c6d3e9bee7b63137a391bcaba
Parents: fcc09dc
Author: Mikhail Antonov <olorinbant@gmail.com>
Authored: Fri Mar 20 16:24:54 2015 -0700
Committer: Sean Busbey <busbey@cloudera.com>
Committed: Tue Mar 24 12:33:02 2015 -0700

----------------------------------------------------------------------
 .../org/apache/hadoop/hbase/client/Result.java  | 33 ++++++++++++++++++--
 .../apache/hadoop/hbase/client/TestResult.java  | 27 ++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/8cb4f89c/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 2cc38bf..b6600a9 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
@@ -102,7 +102,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;
 
@@ -112,6 +112,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} won't do or in old 'mapred' as opposed
@@ -119,7 +121,16 @@ public class Result implements CellScannable, CellScanner {
    * {@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;
   }
 
   /**
@@ -172,6 +183,7 @@ public class Result implements CellScannable, CellScanner {
     this.exists = exists;
     this.stale = stale;
     this.partial = partial;
+    this.readonly = false;
   }
 
   /**
@@ -834,9 +846,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;
@@ -866,6 +881,7 @@ public class Result implements CellScannable, CellScanner {
   }
 
   public void setExists(Boolean exists) {
+    checkReadonly();
     this.exists = exists;
   }
 
@@ -891,8 +907,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;
   }
 
@@ -903,4 +922,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!");
+    }
+  }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/hbase/blob/8cb4f89c/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 fd4b01a..aa25dd3 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.ClientTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -229,6 +230,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.


Mime
View raw message