ACCUMULO-2657 Properly interpret default empty string ABSOLUTEPATH config
properties
This commit alters how the default value for an ABSOLUTEPATH configuration
property is
determined.
* The property no longer needs to be annotated @Interpolated for it to be fully
processed, i.e., for File.getAbsolutePath() to be run on it.
* Processing does not occur when the property default value is an empty string,
so the
default value remains an empty string instead of being processed into the
value of
System.getProperty("user.dir").
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/d77cd3fa
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/d77cd3fa
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/d77cd3fa
Branch: refs/heads/master
Commit: d77cd3fa14d0d9690ae5b17ad851ba915fffd27f
Parents: ef12e59
Author: Bill Havanki <bhava...@cloudera.com>
Authored: Tue Apr 15 10:32:45 2014 -0400
Committer: Christopher Tubbs <ctubb...@apache.org>
Committed: Tue Apr 15 14:46:33 2014 -0400
----------------------------------------------------------------------
.../org/apache/accumulo/core/conf/Property.java | 12 ++--
.../apache/accumulo/core/conf/PropertyType.java | 2 +
.../apache/accumulo/core/conf/PropertyTest.java | 36 ++--------
.../accumulo/core/conf/PropertyTypeTest.java | 70 ++++++++++++++++++++
4 files changed, 84 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/accumulo/blob/d77cd3fa/core/src/main/java/org/apache/accumulo/core/conf/Property.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
index 9b803e2..60969be 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@ -462,6 +462,7 @@ public enum Property {
}
public String getDefaultValue() {
+ String v;
if (isInterpolated()) {
PropertiesConfiguration pconf = new PropertiesConfiguration();
Properties systemProperties = System.getProperties();
@@ -469,14 +470,13 @@ public enum Property {
pconf.append(new MapConfiguration(systemProperties));
}
pconf.addProperty("hack_default_value", this.defaultValue);
- String v = pconf.getString("hack_default_value");
- if (this.type == PropertyType.ABSOLUTEPATH)
- return new File(v).getAbsolutePath();
- else
- return v;
+ v = pconf.getString("hack_default_value");
} else {
- return getRawDefaultValue();
+ v = getRawDefaultValue();
}
+ if (this.type == PropertyType.ABSOLUTEPATH && !(v.trim().equals("")))
+ v = new File(v).getAbsolutePath();
+ return v;
}
public PropertyType getType() {
http://git-wip-us.apache.org/repos/asf/accumulo/blob/d77cd3fa/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
index 7b6a926..60812d6 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java
@@ -56,6 +56,8 @@ public enum PropertyType {
"An absolute filesystem path. The filesystem depends on the property.
This is the same as path, but enforces that its root is explicitly specified.")
{
@Override
public boolean isValidFormat(String value) {
+ if (value.trim().equals(""))
+ return true;
return new Path(value).isAbsolute();
}
},
http://git-wip-us.apache.org/repos/asf/accumulo/blob/d77cd3fa/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
index d9a747b..bca2e22 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
@@ -78,36 +78,6 @@ public class PropertyTest {
}
}
- private void typeCheckValidFormat(PropertyType type, String... args) {
- for (String s : args)
- assertTrue(s + " should be valid", type.isValidFormat(s));
- }
-
- private void typeCheckInvalidFormat(PropertyType type, String... args) {
- for (String s : args)
- assertFalse(s + " should be invalid", type.isValidFormat(s));
- }
-
- @Test
- public void testTypes() {
- typeCheckValidFormat(PropertyType.TIMEDURATION, "600", "30s", "45m",
"30000ms", "3d", "1h");
- typeCheckInvalidFormat(PropertyType.TIMEDURATION, "1w", "1h30m", "1s
200ms", "ms", "", "a");
-
- typeCheckValidFormat(PropertyType.MEMORY, "1024", "20B", "100K", "1500M",
"2G");
- typeCheckInvalidFormat(PropertyType.MEMORY, "1M500K", "1M 2K", "1MB",
"1.5G", "1,024K", "", "a");
-
- typeCheckValidFormat(PropertyType.HOSTLIST, "localhost",
"server1,server2,server3", "server1:1111,server2:3333", "localhost:1111",
"server2:1111",
- "www.server", "www.server:1111", "www.server.com",
"www.server.com:111");
- typeCheckInvalidFormat(PropertyType.HOSTLIST, ":111", "local host");
-
- typeCheckValidFormat(PropertyType.ABSOLUTEPATH, "/foo", "/foo/c", "/");
- // in hadoop 2.0 Path only normalizes Windows paths properly when run on a
Windows system
- // this makes the following checks fail
- if (System.getProperty("os.name").toLowerCase().contains("windows"))
- typeCheckValidFormat(PropertyType.ABSOLUTEPATH, "d:\\foo12",
"c:\\foo\\g", "c:\\foo\\c", "c:\\");
- typeCheckInvalidFormat(PropertyType.ABSOLUTEPATH, "foo12", "foo/g",
"foo\\c");
- }
-
@Test
public void testRawDefaultValues() {
AccumuloConfiguration conf =
AccumuloConfiguration.getDefaultConfiguration();
@@ -117,6 +87,12 @@ public class PropertyTest {
}
@Test
+ public void testGetDefaultValue_AbsolutePath() {
+ // should not expand because default is ""
+ assertEquals("", Property.GENERAL_MAVEN_PROJECT_BASEDIR.getDefaultValue());
+ }
+
+ @Test
public void testSensitiveKeys() {
final TreeMap<String,String> extras = new TreeMap<String,String>();
extras.put("trace.token.property.blah", "something");
http://git-wip-us.apache.org/repos/asf/accumulo/blob/d77cd3fa/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java
----------------------------------------------------------------------
diff --git
a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java
new file mode 100644
index 0000000..e1accb5
--- /dev/null
+++ b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTypeTest.java
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.accumulo.core.conf;
+
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+public class PropertyTypeTest {
+ @Test
+ public void testToString() {
+ assertEquals("string", PropertyType.STRING.toString());
+ }
+
+ @Test
+ public void testGetFormatDescription() {
+ assertEquals("An arbitrary string of characters whose format is
unspecified and interpreted based on the context of the property to which it
applies.",
+ PropertyType.STRING.getFormatDescription());
+ }
+
+ private void typeCheckValidFormat(PropertyType type, String... args) {
+ for (String s : args)
+ assertTrue(s + " should be valid", type.isValidFormat(s));
+ }
+
+ private void typeCheckInvalidFormat(PropertyType type, String... args) {
+ for (String s : args)
+ assertFalse(s + " should be invalid", type.isValidFormat(s));
+ }
+
+ @Test
+ public void testTypeFormats() {
+ typeCheckValidFormat(PropertyType.TIMEDURATION, "600", "30s", "45m",
"30000ms", "3d", "1h");
+ typeCheckInvalidFormat(PropertyType.TIMEDURATION, "1w", "1h30m", "1s
200ms", "ms", "", "a");
+
+ typeCheckValidFormat(PropertyType.MEMORY, "1024", "20B", "100K", "1500M",
"2G");
+ typeCheckInvalidFormat(PropertyType.MEMORY, "1M500K", "1M 2K", "1MB",
"1.5G", "1,024K", "", "a");
+
+ typeCheckValidFormat(PropertyType.HOSTLIST, "localhost",
"server1,server2,server3", "server1:1111,server2:3333", "localhost:1111",
"server2:1111",
+ "www.server", "www.server:1111", "www.server.com",
"www.server.com:111");
+ typeCheckInvalidFormat(PropertyType.HOSTLIST, ":111", "local host");
+
+ typeCheckValidFormat(PropertyType.ABSOLUTEPATH, "/foo", "/foo/c", "/");
+ // in hadoop 2.0 Path only normalizes Windows paths properly when run on a
Windows system
+ // this makes the following checks fail
+ if (System.getProperty("os.name").toLowerCase().contains("windows"))
+ typeCheckValidFormat(PropertyType.ABSOLUTEPATH, "d:\\foo12",
"c:\\foo\\g", "c:\\foo\\c", "c:\\");
+ typeCheckValidFormat(PropertyType.ABSOLUTEPATH,
System.getProperty("user.dir"));
+ typeCheckInvalidFormat(PropertyType.ABSOLUTEPATH, "foo12", "foo/g",
"foo\\c");
+ }
+
+ @Test
+ public void testIsValidFormat_RegexAbsent() {
+ // assertTrue(PropertyType.PREFIX.isValidFormat("whatever")); currently
forbidden
+ assertTrue(PropertyType.PREFIX.isValidFormat(null));
+ }
+}
