HADOOP-9124. SortedMapWritable violates contract of Map interface for equals() and hashCode(). (Surenkumar Nihalani via tgraves)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.23@1441575 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -62,6 +62,9 @@ Release 0.23.7 - UNRELEASED
     HADOOP-9231. Parametrize staging URL for the uniformity of
     distributionManagement. (Konstantin Boudnik via tgraves)
 
+    HADOOP-9124. SortedMapWritable violates contract of Map interface for
+    equals() and hashCode(). (Surenkumar Nihalani via tgraves)
+
 Release 0.23.6 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SortedMapWritable.java

@@ -205,4 +205,27 @@ public class SortedMapWritable extends AbstractMapWritable
       e.getValue().write(out);
     }
   }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+
+    if (obj instanceof SortedMapWritable) {
+      Map map = (Map) obj;
+      if (size() != map.size()) {
+        return false;
+      }
+
+      return entrySet().equals(map.entrySet());
+    }
+
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    return instance.hashCode();
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSortedMapWritable.java

@@ -17,15 +17,20 @@
  */
 package org.apache.hadoop.io;
 
-import java.util.Map;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
-import junit.framework.TestCase;
+import java.util.Map;
+import org.junit.Test;
 
 /**
  * Tests SortedMapWritable
  */
-public class TestSortedMapWritable extends TestCase {
+public class TestSortedMapWritable {
   /** the test */
+  @Test
   @SuppressWarnings("unchecked")
   public void testSortedMapWritable() {
     Text[] keys = {
@@ -90,6 +95,7 @@ public class TestSortedMapWritable extends TestCase {
   /**
    * Test that number of "unknown" classes is propagated across multiple copies.
    */
+  @Test
   @SuppressWarnings("deprecation")
   public void testForeignClass() {
     SortedMapWritable inMap = new SortedMapWritable();
@@ -99,4 +105,63 @@ public class TestSortedMapWritable extends TestCase {
     SortedMapWritable copyOfCopy = new SortedMapWritable(outMap);
     assertEquals(1, copyOfCopy.getNewClasses());
   }
+  
+  /**
+   * Tests if equal and hashCode method still hold the contract.
+   */
+  @Test
+  public void testEqualsAndHashCode() {
+    String failureReason;
+    SortedMapWritable mapA = new SortedMapWritable();
+    SortedMapWritable mapB = new SortedMapWritable();
+    
+    // Sanity checks
+    failureReason = "SortedMapWritable couldn't be initialized. Got null reference";
+    assertNotNull(failureReason, mapA);
+    assertNotNull(failureReason, mapB);
+    
+    // Basic null check
+    assertFalse("equals method returns true when passed null", mapA.equals(null));
+    
+    // When entry set is empty, they should be equal
+    assertTrue("Two empty SortedMapWritables are no longer equal", mapA.equals(mapB));
+    
+    // Setup
+    Text[] keys = {
+        new Text("key1"),
+        new Text("key2")
+    };
+    
+    BytesWritable[] values = {
+        new BytesWritable("value1".getBytes()),
+        new BytesWritable("value2".getBytes())
+    };
+    
+    mapA.put(keys[0], values[0]);
+    mapB.put(keys[1], values[1]);
+    
+    // entrySets are different
+    failureReason = "Two SortedMapWritables with different data are now equal";
+    assertTrue(failureReason, mapA.hashCode() != mapB.hashCode());
+    assertTrue(failureReason, !mapA.equals(mapB));
+    assertTrue(failureReason, !mapB.equals(mapA));
+    
+    mapA.put(keys[1], values[1]);
+    mapB.put(keys[0], values[0]);
+    
+    // entrySets are now same
+    failureReason = "Two SortedMapWritables with same entry sets formed in different order are now different";
+    assertEquals(failureReason, mapA.hashCode(), mapB.hashCode());
+    assertTrue(failureReason, mapA.equals(mapB));
+    assertTrue(failureReason, mapB.equals(mapA));
+    
+    // Let's check if entry sets of same keys but different values
+    mapA.put(keys[0], values[1]);
+    mapA.put(keys[1], values[0]);
+    
+    failureReason = "Two SortedMapWritables with different content are now equal";
+    assertTrue(failureReason, mapA.hashCode() != mapB.hashCode());
+    assertTrue(failureReason, !mapA.equals(mapB));
+    assertTrue(failureReason, !mapB.equals(mapA));
+  }
 }