HBASE-8240 CompoundConfiguration should implement Iterable (Ted Yu)

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1464669 13f79535-47bb-0310-9956-ffa450edef68

hbase-common/src/main/java/org/apache/hadoop/hbase/CompoundConfiguration.java

@@ -25,10 +25,12 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.collections.iterators.UnmodifiableIterator;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
@@ -42,6 +44,9 @@ import org.apache.hadoop.util.StringUtils;
  * functionality, which performs a deep merge and mutates the common data
  * structure.
  * <p>
+ * The iterator on CompoundConfiguration is unmodifiable. Obtaining iterator is an expensive
+ * operation.
+ * <p>
  * For clarity: the shallow merge allows the user to mutate either of the
  * configuration objects and have changes reflected everywhere. In contrast to a
  * deep merge, that requires you to explicitly know all applicable copies to
@@ -57,7 +62,7 @@ public class CompoundConfiguration extends Configuration {
 
   // Devs: these APIs are the same contract as their counterparts in
   // Configuration.java
-  private static interface ImmutableConfigMap {
+  private static interface ImmutableConfigMap extends Iterable<Map.Entry<String,String>> {
     String get(String key);
     String getRaw(String key);
     Class<?> getClassByName(String name) throws ClassNotFoundException;
@@ -82,6 +87,11 @@ public class CompoundConfiguration extends Configuration {
     this.configs.add(0, new ImmutableConfigMap() {
       Configuration c = conf;
 
+      @Override
+      public Iterator<Map.Entry<String,String>> iterator() {
+        return c.iterator();
+      }
+      
       @Override
       public String get(String key) {
         return c.get(key);
@@ -127,6 +137,17 @@ public class CompoundConfiguration extends Configuration {
     this.configs.add(0, new ImmutableConfigMap() {
       Map<ImmutableBytesWritable, ImmutableBytesWritable> m = map;
 
+      @Override
+      public Iterator<Map.Entry<String,String>> iterator() {
+        Map<String, String> ret = new HashMap<String, String>();
+        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : map.entrySet()) {
+          String key = Bytes.toString(entry.getKey().get());
+          String val = entry.getValue() == null ? null : Bytes.toString(entry.getValue().get());
+          ret.put(key, val);
+        }
+        return ret.entrySet().iterator();
+      }
+      
       @Override
       public String get(String key) {
         ImmutableBytesWritable ibw = new ImmutableBytesWritable(Bytes
@@ -175,6 +196,11 @@ public class CompoundConfiguration extends Configuration {
     this.configs.add(0, new ImmutableConfigMap() {
       Map<String, String> m = map;
 
+      @Override
+      public Iterator<Map.Entry<String,String>> iterator() {
+        return map.entrySet().iterator();
+      }
+
       @Override
       public String get(String key) {
         return m.get(key);
@@ -434,7 +460,18 @@ public class CompoundConfiguration extends Configuration {
 
   @Override
   public Iterator<Map.Entry<String, String>> iterator() {
-    throw new UnsupportedOperationException("Immutable Configuration");
+    Map<String, String> ret = new HashMap<String, String>();
+    if (!configs.isEmpty()) {
+      for (int i = configs.size() - 1; i >= 0; i--) {
+        ImmutableConfigMap map = configs.get(i);
+        Iterator<Map.Entry<String, String>> iter = map.iterator();
+        while (iter.hasNext()) {
+          Map.Entry<String, String> entry = iter.next();
+          ret.put(entry.getKey(), entry.getValue());
+        }
+      }
+    }
+    return UnmodifiableIterator.decorate(ret.entrySet().iterator());
   }
 
   @Override

hbase-common/src/test/java/org/apache/hadoop/hbase/TestCompoundConfiguration.java

@@ -20,6 +20,7 @@
 package org.apache.hadoop.hbase;
 
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
 
 import org.apache.hadoop.conf.Configuration;
@@ -35,6 +36,7 @@ import junit.framework.TestCase;
 @Category(SmallTests.class)
 public class TestCompoundConfiguration extends TestCase {
   private Configuration baseConf;
+  private int baseConfSize;
 
   @Override
   protected void setUp() throws Exception {
@@ -42,6 +44,7 @@ public class TestCompoundConfiguration extends TestCase {
     baseConf.set("A", "1");
     baseConf.setInt("B", 2);
     baseConf.set("C", "3");
+    baseConfSize = baseConf.size();
   }
 
   @Test
@@ -80,6 +83,15 @@ public class TestCompoundConfiguration extends TestCase {
     assertEquals(4, compoundConf.getInt("D", 0));
     assertNull(compoundConf.get("E"));
     assertEquals(6, compoundConf.getInt("F", 6));
+    
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("B")) assertEquals("2b", entry.getValue());
+      else if (entry.getKey().equals("G")) assertEquals(null, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(baseConfSize + 1, cnt);
   }
 
   private ImmutableBytesWritable strToIbw(String s) {
@@ -107,6 +119,15 @@ public class TestCompoundConfiguration extends TestCase {
     assertNull(compoundConf.get("E"));
     assertEquals(6, compoundConf.getInt("F", 6));
     assertNull(compoundConf.get("G"));
+    
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("B")) assertEquals("2b", entry.getValue());
+      else if (entry.getKey().equals("G")) assertEquals(null, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(baseConfSize + 2, cnt);
   }
 
   @Test
@@ -126,6 +147,15 @@ public class TestCompoundConfiguration extends TestCase {
     assertNull(compoundConf.get("E"));
     assertEquals(6, compoundConf.getInt("F", 6));
     assertNull(compoundConf.get("G"));
+
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("B")) assertEquals("2b", entry.getValue());
+      else if (entry.getKey().equals("G")) assertEquals(null, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(4, cnt);
   }
 
   @Test
@@ -134,16 +164,26 @@ public class TestCompoundConfiguration extends TestCase {
     map1.put("A", "2");
     map1.put("D", "5");
     Map<String, String> map2 = new HashMap<String, String>();
-    map2.put("A", "3");
+    String newValueForA = "3", newValueForB = "4";
-    map2.put("B", "4");
+    map2.put("A", newValueForA);
+    map2.put("B", newValueForB);
 
     CompoundConfiguration compoundConf = new CompoundConfiguration()
       .addStringMap(map1).add(baseConf);
     assertEquals("1", compoundConf.get("A"));
     assertEquals("5", compoundConf.get("D"));
     compoundConf.addStringMap(map2);
-    assertEquals("3", compoundConf.get("A"));
+    assertEquals(newValueForA, compoundConf.get("A"));
-    assertEquals("4", compoundConf.get("B"));
+    assertEquals(newValueForB, compoundConf.get("B"));
     assertEquals("5", compoundConf.get("D"));
+
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("A")) assertEquals(newValueForA, entry.getValue());
+      else if (entry.getKey().equals("B")) assertEquals(newValueForB, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(baseConfSize + 1, cnt);
   }
 }