Repository: accumulo Updated Branches: refs/heads/1.7 b29bd921b -> 61b76b507
ACCUMULO-3742 add ClientConfiguration constructor that reads from a file and handles multi-valued properties correctly Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/f5c7f05a Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/f5c7f05a Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/f5c7f05a Branch: refs/heads/1.7 Commit: f5c7f05af7bdb95a2485bc0566bd2c4968f1427f Parents: f0b3487 Author: Billie Rinaldi <[email protected]> Authored: Tue Apr 21 10:12:23 2015 -0700 Committer: Billie Rinaldi <[email protected]> Committed: Tue Apr 21 10:12:23 2015 -0700 ---------------------------------------------------------------------- .../apache/accumulo/core/cli/ClientOpts.java | 3 +- .../core/client/ClientConfiguration.java | 32 +++++++++++++++++++- .../core/util/shell/ShellOptionsJC.java | 4 +-- .../core/conf/ClientConfigurationTest.java | 21 +++++++++++++ .../src/test/resources/multi-valued.client.conf | 16 ++++++++++ .../minicluster/MiniAccumuloInstance.java | 5 ++- 6 files changed, 74 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java index 8bb8b3f..4d00d97 100644 --- a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java +++ b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java @@ -46,7 +46,6 @@ import org.apache.accumulo.core.security.ColumnVisibility; import org.apache.accumulo.core.volume.VolumeConfiguration; import org.apache.accumulo.core.zookeeper.ZooUtil; import org.apache.accumulo.trace.instrument.Trace; -import org.apache.commons.configuration.PropertiesConfiguration; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.mapreduce.Job; @@ -230,7 +229,7 @@ public class ClientOpts extends Help { if (clientConfigFile == null) clientConfig = ClientConfiguration.loadDefault(); else - clientConfig = new ClientConfiguration(new PropertiesConfiguration(clientConfigFile)); + clientConfig = new ClientConfiguration(clientConfigFile); } catch (Exception e) { throw new IllegalArgumentException(e); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java b/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java index 17ad10b..91cda6a 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java +++ b/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java @@ -27,10 +27,13 @@ import java.util.UUID; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.conf.PropertyType; import org.apache.accumulo.core.util.ArgumentChecker; +import org.apache.commons.configuration.AbstractConfiguration; import org.apache.commons.configuration.CompositeConfiguration; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.PropertiesConfiguration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Contains a list of property keys recognized by the Accumulo client and convenience methods for setting them. @@ -38,6 +41,8 @@ import org.apache.commons.configuration.PropertiesConfiguration; * @since 1.6.0 */ public class ClientConfiguration extends CompositeConfiguration { + private static final Logger log = LoggerFactory.getLogger(ClientConfiguration.class); + public static final String USER_ACCUMULO_DIR_NAME = ".accumulo"; public static final String USER_CONF_FILENAME = "config"; public static final String GLOBAL_CONF_FILENAME = "client.conf"; @@ -105,10 +110,31 @@ public class ClientConfiguration extends CompositeConfiguration { } }; + public ClientConfiguration(String configFile) throws ConfigurationException { + this(new PropertiesConfiguration(), configFile); + } + + private ClientConfiguration(PropertiesConfiguration propertiesConfiguration, String configFile) throws ConfigurationException { + super(propertiesConfiguration); + // Don't do list interpolation + propertiesConfiguration.setListDelimiter('\0'); + propertiesConfiguration.load(configFile); + } + public ClientConfiguration(List<? extends Configuration> configs) { super(configs); // Don't do list interpolation this.setListDelimiter('\0'); + for (Configuration c : configs) { + if (c instanceof AbstractConfiguration) { + AbstractConfiguration abstractConfiguration = (AbstractConfiguration) c; + if (abstractConfiguration.getListDelimiter() != '\0') { + log.warn("Client configuration constructed with a Configuration that did not have list delimiter overridden, multi-valued config properties may " + + "be unavailable"); + abstractConfiguration.setListDelimiter('\0'); + } + } + } } /** @@ -143,7 +169,10 @@ public class ClientConfiguration extends CompositeConfiguration { for (String path : paths) { File conf = new File(path); if (conf.canRead()) { - configs.add(new PropertiesConfiguration(conf)); + PropertiesConfiguration propertiesConfiguration = new PropertiesConfiguration(); + propertiesConfiguration.setListDelimiter('\0'); + propertiesConfiguration.load(conf); + configs.add(propertiesConfiguration); } } return new ClientConfiguration(configs); @@ -154,6 +183,7 @@ public class ClientConfiguration extends CompositeConfiguration { public static ClientConfiguration deserialize(String serializedConfig) { PropertiesConfiguration propConfig = new PropertiesConfiguration(); + propConfig.setListDelimiter('\0'); try { propConfig.load(new StringReader(serializedConfig)); } catch (ConfigurationException e) { http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java index dfa24c5..9206c8d 100644 --- a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java +++ b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java @@ -28,7 +28,6 @@ import org.apache.accumulo.core.client.ClientConfiguration; import org.apache.accumulo.core.client.ClientConfiguration.ClientProperty; import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; import org.apache.commons.configuration.ConfigurationException; -import org.apache.commons.configuration.PropertiesConfiguration; import org.apache.log4j.Logger; import com.beust.jcommander.DynamicParameter; @@ -270,8 +269,7 @@ public class ShellOptionsJC { } public ClientConfiguration getClientConfiguration() throws ConfigurationException, FileNotFoundException { - ClientConfiguration clientConfig = clientConfigFile == null ? ClientConfiguration.loadDefault() : new ClientConfiguration(new PropertiesConfiguration( - getClientConfigFile())); + ClientConfiguration clientConfig = clientConfigFile == null ? ClientConfiguration.loadDefault() : new ClientConfiguration(getClientConfigFile()); if (useSsl()) { clientConfig.setProperty(ClientProperty.INSTANCE_RPC_SSL_ENABLED, "true"); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java b/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java index 40be70f..0ecc519 100644 --- a/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java +++ b/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java @@ -17,12 +17,14 @@ package org.apache.accumulo.core.conf; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import java.util.Arrays; import org.apache.accumulo.core.client.ClientConfiguration; import org.apache.accumulo.core.client.ClientConfiguration.ClientProperty; import org.apache.commons.configuration.Configuration; +import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.PropertiesConfiguration; import org.junit.Test; @@ -63,4 +65,23 @@ public class ClientConfigurationTest { third.addProperty(ClientProperty.INSTANCE_ZK_TIMEOUT.getKey(), "123s"); return new ClientConfiguration(Arrays.asList(first, second, third)); } + + @Test + public void testMultipleValues() throws ConfigurationException { + String val = "comma,separated,list"; + PropertiesConfiguration propertiesConfiguration = new PropertiesConfiguration(); + propertiesConfiguration.setListDelimiter('\0'); + propertiesConfiguration.addProperty(ClientProperty.INSTANCE_ZK_HOST.getKey(), val); + ClientConfiguration conf = new ClientConfiguration(propertiesConfiguration); + assertEquals(val, conf.get(ClientProperty.INSTANCE_ZK_HOST)); + assertEquals(1, conf.getList(ClientProperty.INSTANCE_ZK_HOST.getKey()).size()); + + conf = new ClientConfiguration(new PropertiesConfiguration("multi-valued.client.conf")); + assertNotEquals(val, conf.get(ClientProperty.INSTANCE_ZK_HOST)); + assertEquals(3, conf.getList(ClientProperty.INSTANCE_ZK_HOST.getKey()).size()); + + conf = new ClientConfiguration("multi-valued.client.conf"); + assertEquals(val, conf.get(ClientProperty.INSTANCE_ZK_HOST)); + assertEquals(1, conf.getList(ClientProperty.INSTANCE_ZK_HOST.getKey()).size()); + } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/test/resources/multi-valued.client.conf ---------------------------------------------------------------------- diff --git a/core/src/test/resources/multi-valued.client.conf b/core/src/test/resources/multi-valued.client.conf new file mode 100644 index 0000000..457370b --- /dev/null +++ b/core/src/test/resources/multi-valued.client.conf @@ -0,0 +1,16 @@ +# 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. + +instance.zookeeper.host=comma,separated,list http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java ---------------------------------------------------------------------- diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java b/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java index fb6fb0a..cc6c2ed 100644 --- a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java +++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java @@ -43,7 +43,10 @@ public class MiniAccumuloInstance extends ZooKeeperInstance { public static PropertiesConfiguration getConfigProperties(File directory) { try { - return new PropertiesConfiguration(new File(new File(directory, "conf"), "client.conf")); + PropertiesConfiguration conf = new PropertiesConfiguration(); + conf.setListDelimiter('\0'); + conf.load(new File(new File(directory, "conf"), "client.conf")); + return conf; } catch (ConfigurationException e) { // this should never happen since we wrote the config file ourselves throw new IllegalArgumentException(e);
