From 168bf7b71d2e8fc54e316c0b7d739be9de9157a3 Mon Sep 17 00:00:00 2001 From: Eli Collins Date: Tue, 4 Jan 2011 05:28:54 +0000 Subject: [PATCH] HADOOP-6578. Configuration should trim whitespace around a lot of value types. Contributed by Michele Catasta git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1054903 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 3 + .../org/apache/hadoop/conf/Configuration.java | 37 +++++++-- .../apache/hadoop/conf/TestConfiguration.java | 76 ++++++++++++++++++- 3 files changed, 108 insertions(+), 8 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index aad098a21fe..a62ac5100d9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -249,6 +249,9 @@ Release 0.22.0 - Unreleased HADOOP-6864. Provide a JNI-based implementation of ShellBasedUnixGroupsNetgroupMapping (implementation of GroupMappingServiceProvider) (Erik Seffl via boryas) + HADOOP-6578. Configuration should trim whitespace around a lot of value + types. (Michele Catasta via eli) + OPTIMIZATIONS HADOOP-6884. Add LOG.isDebugEnabled() guard for each LOG.debug(..). diff --git a/src/java/org/apache/hadoop/conf/Configuration.java b/src/java/org/apache/hadoop/conf/Configuration.java index 7f265f3e6b4..6607aa6d0ee 100644 --- a/src/java/org/apache/hadoop/conf/Configuration.java +++ b/src/java/org/apache/hadoop/conf/Configuration.java @@ -544,6 +544,29 @@ public class Configuration implements Iterable>, name = handleDeprecation(name); return substituteVars(getProps().getProperty(name)); } + + /** + * Get the value of the name property as a trimmed String, + * null if no such property exists. + * If the key is deprecated, it returns the value of + * the first key which replaces the deprecated key and is not null + * + * Values are processed for variable expansion + * before being returned. + * + * @param name the property name. + * @return the value of the name or its replacing property, + * or null if no such property exists. + */ + public String getTrimmed(String name) { + String value = get(name); + + if (null == value) { + return null; + } else { + return value.trim(); + } + } /** * Get the value of the name property, without doing @@ -644,7 +667,7 @@ public class Configuration implements Iterable>, * or defaultValue. */ public int getInt(String name, int defaultValue) { - String valueString = get(name); + String valueString = getTrimmed(name); if (valueString == null) return defaultValue; try { @@ -680,7 +703,7 @@ public class Configuration implements Iterable>, * or defaultValue. */ public long getLong(String name, long defaultValue) { - String valueString = get(name); + String valueString = getTrimmed(name); if (valueString == null) return defaultValue; try { @@ -733,7 +756,7 @@ public class Configuration implements Iterable>, * or defaultValue. */ public float getFloat(String name, float defaultValue) { - String valueString = get(name); + String valueString = getTrimmed(name); if (valueString == null) return defaultValue; try { @@ -763,7 +786,7 @@ public class Configuration implements Iterable>, * or defaultValue. */ public boolean getBoolean(String name, boolean defaultValue) { - String valueString = get(name); + String valueString = getTrimmed(name); if ("true".equals(valueString)) return true; else if ("false".equals(valueString)) @@ -1079,7 +1102,7 @@ public class Configuration implements Iterable>, } } - Class clazz = map.get(name); + Class clazz = map.get(name); if (clazz == null) { clazz = Class.forName(name, true, classLoader); if (clazz != null) { @@ -1104,7 +1127,7 @@ public class Configuration implements Iterable>, * or defaultValue. */ public Class[] getClasses(String name, Class ... defaultValue) { - String[] classnames = getStrings(name); + String[] classnames = getTrimmedStrings(name); if (classnames == null) return defaultValue; try { @@ -1129,7 +1152,7 @@ public class Configuration implements Iterable>, * or defaultValue. */ public Class getClass(String name, Class defaultValue) { - String valueString = get(name); + String valueString = getTrimmed(name); if (valueString == null) return defaultValue; try { diff --git a/src/test/core/org/apache/hadoop/conf/TestConfiguration.java b/src/test/core/org/apache/hadoop/conf/TestConfiguration.java index 73a08bb94cd..52c887f308a 100644 --- a/src/test/core/org/apache/hadoop/conf/TestConfiguration.java +++ b/src/test/core/org/apache/hadoop/conf/TestConfiguration.java @@ -29,6 +29,7 @@ import java.util.Random; import java.util.regex.Pattern; import junit.framework.TestCase; +import static org.junit.Assert.assertArrayEquals; import org.apache.hadoop.fs.Path; import org.codehaus.jackson.map.ObjectMapper; @@ -337,6 +338,7 @@ public class TestConfiguration extends TestCase { appendProperty("test.int1", "20"); appendProperty("test.int2", "020"); appendProperty("test.int3", "-20"); + appendProperty("test.int4", " -20 "); endConfig(); Path fileResource = new Path(CONFIG); conf.addResource(fileResource); @@ -346,8 +348,80 @@ public class TestConfiguration extends TestCase { assertEquals(20, conf.getLong("test.int2", 0)); assertEquals(-20, conf.getInt("test.int3", 0)); assertEquals(-20, conf.getLong("test.int3", 0)); + assertEquals(-20, conf.getInt("test.int4", 0)); + assertEquals(-20, conf.getLong("test.int4", 0)); } - + + public void testBooleanValues() throws IOException { + out=new BufferedWriter(new FileWriter(CONFIG)); + startConfig(); + appendProperty("test.bool1", "true"); + appendProperty("test.bool2", "false"); + appendProperty("test.bool3", " true "); + appendProperty("test.bool4", " false "); + appendProperty("test.bool5", "foo"); + endConfig(); + Path fileResource = new Path(CONFIG); + conf.addResource(fileResource); + assertEquals(true, conf.getBoolean("test.bool1", false)); + assertEquals(false, conf.getBoolean("test.bool2", true)); + assertEquals(true, conf.getBoolean("test.bool3", false)); + assertEquals(false, conf.getBoolean("test.bool4", true)); + assertEquals(true, conf.getBoolean("test.bool5", true)); + } + + public void testFloatValues() throws IOException { + out=new BufferedWriter(new FileWriter(CONFIG)); + startConfig(); + appendProperty("test.float1", "3.1415"); + appendProperty("test.float2", "003.1415"); + appendProperty("test.float3", "-3.1415"); + appendProperty("test.float4", " -3.1415 "); + endConfig(); + Path fileResource = new Path(CONFIG); + conf.addResource(fileResource); + assertEquals(3.1415f, conf.getFloat("test.float1", 0.0f)); + assertEquals(3.1415f, conf.getFloat("test.float2", 0.0f)); + assertEquals(-3.1415f, conf.getFloat("test.float3", 0.0f)); + assertEquals(-3.1415f, conf.getFloat("test.float4", 0.0f)); + } + + public void testGetClass() throws IOException { + out=new BufferedWriter(new FileWriter(CONFIG)); + startConfig(); + appendProperty("test.class1", "java.lang.Integer"); + appendProperty("test.class2", " java.lang.Integer "); + endConfig(); + Path fileResource = new Path(CONFIG); + conf.addResource(fileResource); + assertEquals("java.lang.Integer", conf.getClass("test.class1", null).getCanonicalName()); + assertEquals("java.lang.Integer", conf.getClass("test.class2", null).getCanonicalName()); + } + + public void testGetClasses() throws IOException { + out=new BufferedWriter(new FileWriter(CONFIG)); + startConfig(); + appendProperty("test.classes1", "java.lang.Integer,java.lang.String"); + appendProperty("test.classes2", " java.lang.Integer , java.lang.String "); + endConfig(); + Path fileResource = new Path(CONFIG); + conf.addResource(fileResource); + String[] expectedNames = {"java.lang.Integer", "java.lang.String"}; + Class[] defaultClasses = {}; + Class[] classes1 = conf.getClasses("test.classes1", defaultClasses); + Class[] classes2 = conf.getClasses("test.classes2", defaultClasses); + assertArrayEquals(expectedNames, extractClassNames(classes1)); + assertArrayEquals(expectedNames, extractClassNames(classes2)); + } + + private static String[] extractClassNames(Class[] classes) { + String[] classNames = new String[classes.length]; + for (int i = 0; i < classNames.length; i++) { + classNames[i] = classes[i].getCanonicalName(); + } + return classNames; + } + enum Dingo { FOO, BAR }; enum Yak { RAB, FOO }; public void testEnum() throws IOException {