Browse Source

ZOOKEEPER-1338. class cast exceptions may be thrown by multi ErrorResult class (invalid equals) (phunt via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1240959 13f79535-47bb-0310-9956-ffa450edef68
Mahadev Konar 13 years ago
parent
commit
e657907115

+ 3 - 0
CHANGES.txt

@@ -130,6 +130,9 @@ BUGFIXES:
 
   ZOOKEEPER-1337. multi's "Transaction" class is missing tests. (camille
   and phunt via mahadev)
+
+  ZOOKEEPER-1338. class cast exceptions may be thrown by multi ErrorResult 
+  class (invalid equals) (phunt via mahadev)
  
 IMPROVEMENTS:
 

+ 8 - 8
src/java/main/org/apache/zookeeper/OpResult.java

@@ -23,7 +23,7 @@ import org.apache.zookeeper.data.Stat;
 /**
  * Encodes the result of a single part of a multiple operation commit.
  */
-public class OpResult {
+public abstract class OpResult {
     private int type;
 
     private OpResult(int type) {
@@ -61,10 +61,10 @@ public class OpResult {
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
-            if (!(o instanceof OpResult)) return false;
+            if (!(o instanceof CreateResult)) return false;
 
             CreateResult other = (CreateResult) o;
-            return getType() == other.getType() && path.equals(other.path);
+            return getType() == other.getType() && path.equals(other.getPath());
         }
 
         @Override
@@ -84,9 +84,9 @@ public class OpResult {
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
-            if (!(o instanceof OpResult)) return false;
+            if (!(o instanceof DeleteResult)) return false;
 
-            OpResult opResult = (OpResult) o;
+            DeleteResult opResult = (DeleteResult) o;
             return getType() == opResult.getType();
         }
 
@@ -115,7 +115,7 @@ public class OpResult {
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
-            if (!(o instanceof OpResult)) return false;
+            if (!(o instanceof SetDataResult)) return false;
 
             SetDataResult other = (SetDataResult) o;
             return getType() == other.getType() && stat.getMzxid() == other.stat.getMzxid();
@@ -138,7 +138,7 @@ public class OpResult {
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
-            if (!(o instanceof OpResult)) return false;
+            if (!(o instanceof CheckResult)) return false;
 
             CheckResult other = (CheckResult) o;
             return getType() == other.getType();
@@ -171,7 +171,7 @@ public class OpResult {
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
-            if (!(o instanceof OpResult)) return false;
+            if (!(o instanceof ErrorResult)) return false;
 
             ErrorResult other = (ErrorResult) o;
             return getType() == other.getType() && err == other.getErr();

+ 41 - 0
src/java/test/org/apache/zookeeper/test/MultiTransactionTest.java

@@ -38,7 +38,9 @@ import org.apache.zookeeper.OpResult.CheckResult;
 import org.apache.zookeeper.OpResult.CreateResult;
 import org.apache.zookeeper.OpResult.DeleteResult;
 import org.apache.zookeeper.OpResult.ErrorResult;
+import org.apache.zookeeper.OpResult.SetDataResult;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.Stat;
 import org.apache.zookeeper.server.SyncRequestProcessor;
 import org.junit.Assert;
 import org.junit.Before;
@@ -206,6 +208,45 @@ public class MultiTransactionTest extends ClientBase {
         }
     }
 
+    /**
+     * Exercise the equals methods of OpResult classes.
+     */
+    @Test
+    public void testOpResultEquals() {
+        opEquals(new CreateResult("/foo"),
+                new CreateResult("/foo"),
+                new CreateResult("nope"));
+
+        opEquals(new CheckResult(),
+                new CheckResult(),
+                null);
+        
+        opEquals(new SetDataResult(new Stat(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11)),
+                new SetDataResult(new Stat(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11)),
+                new SetDataResult(new Stat(11, 12, 13, 14, 15, 16, 17, 18, 19, 110, 111)));
+        
+        opEquals(new ErrorResult(1),
+                new ErrorResult(1),
+                new ErrorResult(2));
+        
+        opEquals(new DeleteResult(),
+                new DeleteResult(),
+                null);
+
+        opEquals(new ErrorResult(1),
+                new ErrorResult(1),
+                new ErrorResult(2));
+    }
+
+    private void opEquals(OpResult expected, OpResult value, OpResult near) {
+        assertEquals(value, value);
+        assertFalse(value.equals(new Object()));
+        assertFalse(value.equals(near));
+        assertFalse(value.equals(value instanceof CreateResult ?
+                new ErrorResult(1) : new CreateResult("nope2")));
+        assertTrue(value.equals(expected));
+    }
+
     @Test
     public void testWatchesTriggered() throws KeeperException, InterruptedException {
         HasTriggeredWatcher watcher = new HasTriggeredWatcher();