From d1eaaf30e215d46facd0ca61b47d59665780aba3 Mon Sep 17 00:00:00 2001
From: Todd Lipcon <todd@apache.org>
Date: Wed, 18 May 2011 18:30:04 +0000
Subject: [PATCH] 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                                   |  3 ++
 .../org/apache/hadoop/conf/Configuration.java |  2 +-
 .../org/apache/hadoop/util/StringUtils.java   |  3 +-
 .../apache/hadoop/conf/TestConfiguration.java | 35 +++++++++++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index a1d4a9e523..1615e25c0f 100644
--- a/CHANGES.txt
+++ b/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
diff --git a/src/java/org/apache/hadoop/conf/Configuration.java b/src/java/org/apache/hadoop/conf/Configuration.java
index a4c2fd6005..0563600bc4 100644
--- a/src/java/org/apache/hadoop/conf/Configuration.java
+++ b/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);
diff --git a/src/java/org/apache/hadoop/util/StringUtils.java b/src/java/org/apache/hadoop/util/StringUtils.java
index 3da4a723b4..261399c9be 100644
--- a/src/java/org/apache/hadoop/util/StringUtils.java
+++ b/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)));
   }
   
   /**
diff --git a/src/test/core/org/apache/hadoop/conf/TestConfiguration.java b/src/test/core/org/apache/hadoop/conf/TestConfiguration.java
index 278c4705d9..9faef9f5b7 100644
--- a/src/test/core/org/apache/hadoop/conf/TestConfiguration.java
+++ b/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;
@@ -448,6 +449,40 @@ public void testGetClasses() throws IOException {
     assertArrayEquals(expectedNames, extractClassNames(classes1));
     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];