HADOOP-7300. Configuration methods that return collections are inconsistent about mutability. Contributed by Todd Lipcon.

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

CHANGES.txt

@@ -699,6 +699,9 @@ Release 0.22.0 - Unreleased
     HADOOP-7296. The FsPermission(FsPermission) constructor does not use the
     sticky bit. (Siddharth Seth via tomwhite)
 
+    HADOOP-7300. Configuration methods that return collections are inconsistent
+    about mutability. (todd)
+
 Release 0.21.1 - Unreleased
 
   IMPROVEMENTS

src/java/org/apache/hadoop/conf/Configuration.java

@@ -1033,7 +1033,7 @@ public String[] getStrings(String name, String... defaultValue) {
   public Collection<String> getTrimmedStringCollection(String name) {
     String valueString = get(name);
     if (null == valueString) {
-      Collection<String> empty = Collections.emptyList();
+      Collection<String> empty = new ArrayList<String>();
       return empty;
     }
     return StringUtils.getTrimmedStringCollection(valueString);

src/java/org/apache/hadoop/util/StringUtils.java

@@ -329,7 +329,8 @@ public static Collection<String> getStringCollection(String str){
    * @return a <code>Collection</code> of <code>String</code> values
    */
   public static Collection<String> getTrimmedStringCollection(String str){
-    return Arrays.asList(getTrimmedStrings(str));
+    return new ArrayList<String>(
+      Arrays.asList(getTrimmedStrings(str)));
   }
   
   /**

src/test/core/org/apache/hadoop/conf/TestConfiguration.java

@@ -24,6 +24,7 @@
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Random;
@@ -449,6 +450,40 @@ public void testGetClasses() throws IOException {
     assertArrayEquals(expectedNames, extractClassNames(classes2));
   }
   
+  public void testGetStringCollection() throws IOException {
+    Configuration c = new Configuration();
+    c.set("x", " a, b\n,\nc ");
+    Collection<String> strs = c.getTrimmedStringCollection("x");
+    assertEquals(3, strs.size());
+    assertArrayEquals(new String[]{ "a", "b", "c" },
+                      strs.toArray(new String[0]));
+
+    // Check that the result is mutable
+    strs.add("z");
+
+    // Make sure same is true for missing config
+    strs = c.getStringCollection("does-not-exist");
+    assertEquals(0, strs.size());
+    strs.add("z");
+  }
+
+  public void testGetTrimmedStringCollection() throws IOException {
+    Configuration c = new Configuration();
+    c.set("x", "a, b, c");
+    Collection<String> strs = c.getStringCollection("x");
+    assertEquals(3, strs.size());
+    assertArrayEquals(new String[]{ "a", " b", " c" },
+                      strs.toArray(new String[0]));
+
+    // Check that the result is mutable
+    strs.add("z");
+
+    // Make sure same is true for missing config
+    strs = c.getStringCollection("does-not-exist");
+    assertEquals(0, strs.size());
+    strs.add("z");
+  }
+
   private static String[] extractClassNames(Class<?>[] classes) {
     String[] classNames = new String[classes.length];
     for (int i = 0; i < classNames.length; i++) {