[2/6] accumulo git commit: ACCUMULO-3718 make Mutation#hashCode and Mutation#equals not change the state of the mutation

[uj...@apache.org]

ACCUMULO-3718 make Mutation#hashCode and Mutation#equals not change the state of the mutation


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/73ce9cfb
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/73ce9cfb
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/73ce9cfb

Branch: refs/heads/1.6
Commit: 73ce9cfb925b9b5606aee941467fc3c045bc2fac
Parents: 83ee1c1
Author: Bill Slacum <uj...@apache.org>
Authored: Thu Apr 9 19:17:58 2015 -0400
Committer: Bill Slacum <uj...@apache.org>
Committed: Thu Apr 9 19:17:58 2015 -0400

----------------------------------------------------------------------
 .../org/apache/accumulo/core/data/Mutation.java | 34 +++++++++++++++++---
 .../apache/accumulo/core/data/MutationTest.java | 28 ++++++++++++++++
 2 files changed, 57 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/73ce9cfb/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
index 0861cc4..81ad531 100644
--- a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
@@ -191,6 +191,20 @@ public class Mutation implements Writable {
     }
   }
 
+  /* This is so hashCode & equals can be called without changing this object.
+   *
+   * It will return a copy of the current data buffer if serialized has not been
+   * called previously. Otherwise, this.data will be returned since the buffer is
+   * null and will not change.
+   */
+  private byte[] serializedSnapshot() {
+    if (buffer != null) {
+      return buffer.toArray();
+    } else {
+      return this.data;
+    }
+  }
+
   /**
    * @since 1.5.0
    */
@@ -691,13 +705,13 @@ public class Mutation implements Writable {
 
   @Override
   public int hashCode() {
-    return toThrift().hashCode();
+    return toThrift(false).hashCode();
   }
 
   public boolean equals(Mutation m) {
-    serialize();
-    m.serialize();
-    if (Arrays.equals(row, m.row) && entries == m.entries && Arrays.equals(data, m.data)) {
+    byte[] myData = serializedSnapshot();
+    byte[] otherData = m.serializedSnapshot();
+    if (Arrays.equals(row, m.row) && entries == m.entries && Arrays.equals(myData, otherData)) {
       if (values == null && m.values == null)
         return true;
 
@@ -716,7 +730,17 @@ public class Mutation implements Writable {
   }
 
   public TMutation toThrift() {
-    serialize();
+    return toThrift(true);
+  }
+
+  private TMutation toThrift(boolean serialize) {
+    byte[] data;
+    if (serialize) {
+      this.serialize();
+      data = this.data;
+    } else {
+      data = serializedSnapshot();
+    }
     return new TMutation(java.nio.ByteBuffer.wrap(row), java.nio.ByteBuffer.wrap(data), ByteBufferUtil.toByteBuffers(values), entries);
   }
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/73ce9cfb/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
index 8b50788..740baa7 100644
--- a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
@@ -19,6 +19,7 @@ package org.apache.accumulo.core.data;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -608,4 +609,31 @@ public class MutationTest {
     new Mutation(tm1);
   }
 
+  /* The following two tests assert that no exception is thrown after calling
+   * hashCode or equals on a Mutation. These guard against the condition noted
+   * in ACCUMULO-3718.
+   */
+  @Test
+  public void testPutAfterHashCode() {
+    Mutation m = new Mutation("r");
+    m.hashCode();
+    try {
+        m.put("cf", "cq", "v");
+    } catch(IllegalStateException e) {
+      fail("Calling Mutation#hashCode then Mutation#put should not result in an IllegalStateException.");
+    }
+  }
+
+  @Test
+  public void testPutAfterEquals() {
+    Mutation m = new Mutation("r");
+    Mutation m2 = new Mutation("r2");
+    m.equals(m2);
+    try {
+        m.put("cf", "cq", "v");
+        m2.put("cf", "cq", "v");
+    } catch(IllegalStateException e) {
+      fail("Calling Mutation#equals then Mutation#put should not result in an IllegalStateException.");
+    }
+  }
 }