diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/CompoundConfiguration.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/CompoundConfiguration.java index e8d90968cd9..d784a0b8019 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/CompoundConfiguration.java +++ b/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. *

+ * The iterator on CompoundConfiguration is unmodifiable. Obtaining iterator is an expensive + * operation. + *

* 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> { 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> 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 m = map; + @Override + public Iterator> iterator() { + Map ret = new HashMap(); + for (Map.Entry 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 m = map; + @Override + public Iterator> 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> iterator() { - throw new UnsupportedOperationException("Immutable Configuration"); + Map ret = new HashMap(); + if (!configs.isEmpty()) { + for (int i = configs.size() - 1; i >= 0; i--) { + ImmutableConfigMap map = configs.get(i); + Iterator> iter = map.iterator(); + while (iter.hasNext()) { + Map.Entry entry = iter.next(); + ret.put(entry.getKey(), entry.getValue()); + } + } + } + return UnmodifiableIterator.decorate(ret.entrySet().iterator()); } @Override diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestCompoundConfiguration.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestCompoundConfiguration.java index a81f29e174d..2ba7ee13b35 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestCompoundConfiguration.java +++ b/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 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 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 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 map2 = new HashMap(); - map2.put("A", "3"); - map2.put("B", "4"); + String newValueForA = "3", newValueForB = "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("4", compoundConf.get("B")); + assertEquals(newValueForA, compoundConf.get("A")); + assertEquals(newValueForB, compoundConf.get("B")); assertEquals("5", compoundConf.get("D")); + + int cnt = 0; + for (Map.Entry 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); } }