HADOOP-10465. Fix use of generics within SortedMapWritable. Contributed by Bertrand Dechoux.

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

@@ -978,6 +978,9 @@ Release 2.8.0 - UNRELEASED
     HADOOP-10555. Add offset support to MurmurHash.
     (Sergey Shelukhin via wheat9)
 
+    HADOOP-10465. Fix use of generics within SortedMapWritable.
+    (Bertrand Dechoux via wheat9)
+
   OPTIMIZATIONS
 
     HADOOP-11785. Reduce the number of listStatus operation in distcp

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

@@ -36,15 +36,15 @@ import org.apache.hadoop.util.ReflectionUtils;
  */
 @InterfaceAudience.Public
 @InterfaceStability.Stable
-public class SortedMapWritable extends AbstractMapWritable
+public class SortedMapWritable<K extends WritableComparable<? super K>> extends AbstractMapWritable
-  implements SortedMap<WritableComparable, Writable> {
+  implements SortedMap<K, Writable> {
   
-  private SortedMap<WritableComparable, Writable> instance;
+  private SortedMap<K, Writable> instance;
   
   /** default constructor. */
   public SortedMapWritable() {
     super();
-    this.instance = new TreeMap<WritableComparable, Writable>();
+    this.instance = new TreeMap<K, Writable>();
   }
   
   /**
@@ -52,45 +52,39 @@ public class SortedMapWritable extends AbstractMapWritable
    * 
    * @param other the map to copy from
    */
-  public SortedMapWritable(SortedMapWritable other) {
+  public SortedMapWritable(SortedMapWritable<K> other) {
     this();
     copy(other);
   }
 
   @Override
-  public Comparator<? super WritableComparable> comparator() {
+  public Comparator<? super K> comparator() {
     // Returning null means we use the natural ordering of the keys
     return null;
   }
 
   @Override
-  public WritableComparable firstKey() {
+  public K firstKey() {
     return instance.firstKey();
   }
 
   @Override
-  public SortedMap<WritableComparable, Writable>
+  public SortedMap<K, Writable> headMap(K toKey) {
-  headMap(WritableComparable toKey) {
-    
     return instance.headMap(toKey);
   }
 
   @Override
-  public WritableComparable lastKey() {
+  public K lastKey() {
     return instance.lastKey();
   }
 
   @Override
-  public SortedMap<WritableComparable, Writable>
+  public SortedMap<K, Writable> subMap(K fromKey, K toKey) {
-  subMap(WritableComparable fromKey, WritableComparable toKey) {
-    
     return instance.subMap(fromKey, toKey);
   }
 
   @Override
-  public SortedMap<WritableComparable, Writable>
+  public SortedMap<K, Writable> tailMap(K fromKey) {
-  tailMap(WritableComparable fromKey) {
-    
     return instance.tailMap(fromKey);
   }
 
@@ -110,7 +104,7 @@ public class SortedMapWritable extends AbstractMapWritable
   }
 
   @Override
-  public Set<java.util.Map.Entry<WritableComparable, Writable>> entrySet() {
+  public Set<Map.Entry<K, Writable>> entrySet() {
     return instance.entrySet();
   }
 
@@ -125,22 +119,21 @@ public class SortedMapWritable extends AbstractMapWritable
   }
 
   @Override
-  public Set<WritableComparable> keySet() {
+  public Set<K> keySet() {
     return instance.keySet();
   }
 
   @Override
-  public Writable put(WritableComparable key, Writable value) {
+  public Writable put(K key, Writable value) {
     addToMap(key.getClass());
     addToMap(value.getClass());
     return instance.put(key, value);
   }
 
   @Override
-  public void putAll(Map<? extends WritableComparable, ? extends Writable> t) {
+  public void putAll(Map<? extends K, ? extends Writable> t) {
-    for (Map.Entry<? extends WritableComparable, ? extends Writable> e:
+    for (Map.Entry<? extends K, ? extends Writable> e:
       t.entrySet()) {
-      
       put(e.getKey(), e.getValue());
     }
   }
@@ -172,8 +165,8 @@ public class SortedMapWritable extends AbstractMapWritable
     // Then read each key/value pair
     
     for (int i = 0; i < entries; i++) {
-      WritableComparable key =
+      K key =
-        (WritableComparable) ReflectionUtils.newInstance(getClass(
+        (K) ReflectionUtils.newInstance(getClass(
             in.readByte()), getConf());
       
       key.readFields(in);
@@ -196,7 +189,7 @@ public class SortedMapWritable extends AbstractMapWritable
     
     // Then write out each key/value pair
     
-    for (Map.Entry<WritableComparable, Writable> e: instance.entrySet()) {
+    for (Map.Entry<K, Writable> e: instance.entrySet()) {
       out.writeByte(getId(e.getKey().getClass()));
       e.getKey().write(out);
       out.writeByte(getId(e.getValue().getClass()));
@@ -211,7 +204,7 @@ public class SortedMapWritable extends AbstractMapWritable
     }
 
     if (obj instanceof SortedMapWritable) {
-      Map map = (Map) obj;
+      Map<?,?> map = (Map<?,?>) obj;
       if (size() != map.size()) {
         return false;
       }

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

@@ -45,7 +45,7 @@ public class TestSortedMapWritable {
         new BytesWritable("value3".getBytes())
     };
 
-    SortedMapWritable inMap = new SortedMapWritable();
+    SortedMapWritable<Text> inMap = new SortedMapWritable<Text>();
     for (int i = 0; i < keys.length; i++) {
       inMap.put(keys[i], values[i]);
     }
@@ -53,13 +53,14 @@ public class TestSortedMapWritable {
     assertEquals(0, inMap.firstKey().compareTo(keys[0]));
     assertEquals(0, inMap.lastKey().compareTo(keys[2]));
 
-    SortedMapWritable outMap = new SortedMapWritable(inMap);
+    SortedMapWritable<Text> outMap = new SortedMapWritable<Text>(inMap);
     assertEquals(inMap.size(), outMap.size());
     
-    for (Map.Entry<WritableComparable, Writable> e: inMap.entrySet()) {
+    for (Map.Entry<Text, Writable> e: inMap.entrySet()) {
       assertTrue(outMap.containsKey(e.getKey()));
-      assertEquals(0, ((WritableComparable) outMap.get(e.getKey())).compareTo(
+      WritableComparable<WritableComparable<?>> aValue = (WritableComparable<WritableComparable<?>>) outMap.get(e.getKey());
-          e.getValue()));
+      WritableComparable<WritableComparable<?>> bValue = (WritableComparable<WritableComparable<?>>) e.getValue();
+      assertEquals(0, aValue.compareTo(bValue));
     }
     
     // Now for something a little harder...
@@ -69,24 +70,24 @@ public class TestSortedMapWritable {
         new Text("map2")
     };
     
-    SortedMapWritable mapOfMaps = new SortedMapWritable();
+    SortedMapWritable<Text> mapOfMaps = new SortedMapWritable<Text>();
     mapOfMaps.put(maps[0], inMap);
     mapOfMaps.put(maps[1], outMap);
     
-    SortedMapWritable copyOfMapOfMaps = new SortedMapWritable(mapOfMaps);
+    SortedMapWritable<Text> copyOfMapOfMaps = new SortedMapWritable<Text>(mapOfMaps);
     for (int i = 0; i < maps.length; i++) {
       assertTrue(copyOfMapOfMaps.containsKey(maps[i]));
 
-      SortedMapWritable a = (SortedMapWritable) mapOfMaps.get(maps[i]);
+      SortedMapWritable<Text> a = (SortedMapWritable<Text>) mapOfMaps.get(maps[i]);
-      SortedMapWritable b = (SortedMapWritable) copyOfMapOfMaps.get(maps[i]);
+      SortedMapWritable<Text> b = (SortedMapWritable<Text>) copyOfMapOfMaps.get(maps[i]);
       assertEquals(a.size(), b.size());
       for (Writable key: a.keySet()) {
         assertTrue(b.containsKey(key));
         
         // This will work because we know what we put into each set
         
-        WritableComparable aValue = (WritableComparable) a.get(key);
+        WritableComparable<WritableComparable<?>> aValue = (WritableComparable<WritableComparable<?>>) a.get(key);
-        WritableComparable bValue = (WritableComparable) b.get(key);
+        WritableComparable<WritableComparable<?>> bValue = (WritableComparable<WritableComparable<?>>) b.get(key);
         assertEquals(0, aValue.compareTo(bValue));
       }
     }
@@ -98,11 +99,11 @@ public class TestSortedMapWritable {
   @Test
   @SuppressWarnings("deprecation")
   public void testForeignClass() {
-    SortedMapWritable inMap = new SortedMapWritable();
+    SortedMapWritable<Text> inMap = new SortedMapWritable<Text>();
     inMap.put(new Text("key"), new UTF8("value"));
     inMap.put(new Text("key2"), new UTF8("value2"));
-    SortedMapWritable outMap = new SortedMapWritable(inMap);
+    SortedMapWritable<Text> outMap = new SortedMapWritable<Text>(inMap);
-    SortedMapWritable copyOfCopy = new SortedMapWritable(outMap);
+    SortedMapWritable<Text> copyOfCopy = new SortedMapWritable<Text>(outMap);
     assertEquals(1, copyOfCopy.getNewClasses());
   }
   
@@ -112,8 +113,8 @@ public class TestSortedMapWritable {
   @Test
   public void testEqualsAndHashCode() {
     String failureReason;
-    SortedMapWritable mapA = new SortedMapWritable();
+    SortedMapWritable<Text> mapA = new SortedMapWritable<Text>();
-    SortedMapWritable mapB = new SortedMapWritable();
+    SortedMapWritable<Text> mapB = new SortedMapWritable<Text>();
     
     // Sanity checks
     failureReason = "SortedMapWritable couldn't be initialized. Got null reference";
@@ -167,8 +168,8 @@ public class TestSortedMapWritable {
 
   @Test(timeout = 1000)
   public void testPutAll() {
-    SortedMapWritable map1 = new SortedMapWritable();
+    SortedMapWritable<Text> map1 = new SortedMapWritable<Text>();
-    SortedMapWritable map2 = new SortedMapWritable();
+    SortedMapWritable<Text> map2 = new SortedMapWritable<Text>();
     map1.put(new Text("key"), new Text("value"));
     map2.putAll(map1);
 

hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/typedbytes/TypedBytesWritableInput.java

@@ -330,22 +330,25 @@ public class TypedBytesWritableInput implements Configurable {
     return readMap(null);
   }
 
-  public SortedMapWritable readSortedMap(SortedMapWritable mw)
+  public <K extends WritableComparable<? super K>>
+    SortedMapWritable<K> readSortedMap(SortedMapWritable<K> mw)
     throws IOException {
     if (mw == null) {
-      mw = new SortedMapWritable();
+      mw = new SortedMapWritable<K>();
     }
     int length = in.readMapHeader();
     for (int i = 0; i < length; i++) {
-      WritableComparable key = (WritableComparable) read();
+      @SuppressWarnings("unchecked")
+      K key = (K) read();
       Writable value = read();
       mw.put(key, value);
     }
     return mw;
   }
 
-  public SortedMapWritable readSortedMap() throws IOException {
+  public <K extends WritableComparable<? super K>> SortedMapWritable<K>
-    return readSortedMap(null);
+    readSortedMap() throws IOException {
+    return readSortedMap((SortedMapWritable<K>)null);
   }
   
   public Writable readWritable(Writable writable) throws IOException {

hadoop-tools/hadoop-streaming/src/main/java/org/apache/hadoop/typedbytes/TypedBytesWritableOutput.java

@@ -131,7 +131,7 @@ public class TypedBytesWritableOutput {
     } else if (w instanceof MapWritable) {
       writeMap((MapWritable) w);
     } else if (w instanceof SortedMapWritable) {
-      writeSortedMap((SortedMapWritable) w);
+      writeSortedMap((SortedMapWritable<?>) w);
     } else if (w instanceof Record) {
       writeRecord((Record) w);
     } else {
@@ -200,9 +200,9 @@ public class TypedBytesWritableOutput {
     }
   }
 
-  public void writeSortedMap(SortedMapWritable smw) throws IOException {
+  public void writeSortedMap(SortedMapWritable<?> smw) throws IOException {
     out.writeMapHeader(smw.size());
-    for (Map.Entry<WritableComparable, Writable> entry : smw.entrySet()) {
+    for (Map.Entry<? extends WritableComparable<?>, Writable> entry : smw.entrySet()) {
       write(entry.getKey());
       write(entry.getValue());
     }