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

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

CHANGES.txt

@@ -479,6 +479,9 @@ Release 1.2.0 - unreleased
     MAPREDUCE-4970. Child tasks (try to) create security audit log files.
     (sandyr via tucu)
 
+    HADOOP-9124. SortedMapWritable violates contract of Map interface for
+    equals() and hashCode(). (Surenkumar Nihalani via tomwhite)
+
 Release 1.1.2 - Unreleased
 
   INCOMPATIBLE CHANGES

src/core/org/apache/hadoop/io/SortedMapWritable.java

@@ -203,4 +203,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();
+  }
 }

src/test/org/apache/hadoop/io/TestSortedMapWritable.java

@@ -19,15 +19,21 @@
  */
 package org.apache.hadoop.io;
 
+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 java.util.Map;
 
-import junit.framework.TestCase;
+import org.junit.Test;
 
 /**
  * Tests SortedMapWritable
  */
-public class TestSortedMapWritable extends TestCase {
+public class TestSortedMapWritable {
   /** the test */
+  @Test
   @SuppressWarnings("unchecked")
   public void testSortedMapWritable() {
     Text[] keys = {
@@ -92,6 +98,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();
@@ -101,4 +108,60 @@ 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));
+  }
 }