Author: tomwhite Date: Fri Feb 1 15:03:35 2013 New Revision: 1441475 URL: http://svn.apache.org/viewvc?rev=1441475&view=rev Log: HADOOP-9124. SortedMapWritable violates contract of Map interface for equals() and hashCode(). Contributed by Surenkumar Nihalani
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SortedMapWritable.java hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSortedMapWritable.java Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1441475&r1=1441474&r2=1441475&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Fri Feb 1 15:03:35 2013 @@ -592,6 +592,9 @@ Release 2.0.3-alpha - Unreleased HADOOP-9221. Convert remaining xdocs to APT. (Andy Isaacson via atm) HADOOP-8981. TestMetricsSystemImpl fails on Windows. (Xuan Gong via suresh) + + HADOOP-9124. SortedMapWritable violates contract of Map interface for + equals() and hashCode(). (Surenkumar Nihalani via tomwhite) Release 2.0.2-alpha - 2012-09-07 Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SortedMapWritable.java URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SortedMapWritable.java?rev=1441475&r1=1441474&r2=1441475&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SortedMapWritable.java (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SortedMapWritable.java Fri Feb 1 15:03:35 2013 @@ -203,4 +203,27 @@ public class SortedMapWritable extends A 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(); + } } Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSortedMapWritable.java URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSortedMapWritable.java?rev=1441475&r1=1441474&r2=1441475&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSortedMapWritable.java (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestSortedMapWritable.java Fri Feb 1 15:03:35 2013 @@ -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 exten /** * 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 exten 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)); + } }