# HG changeset patch # User Elliott Baron # Date 1397763946 14400 # Node ID e6649b85a05598cd3866514311a73967934acd17 # Parent c3eead964d9e6996b007bfebac94be6f7ad86ae1 Certain properties in agent.properties are ignored We are not properly retrieving the properties defined in agent.properties for CONFIG_LISTEN_ADDRESS, and SAVE_ON_EXIT. The problem comes from java.util.Properties implementation of containsKey, which does not override the Hashtable implementation. As a result, containsKey does not consider default properties provided in the java.util.Properties(Properties defaults) constructor. This patch changes containsKey uses to instead use getProperty, which does consider defaults. Reviewed-by: omajid, jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-April/009562.html PR1728 diff -r c3eead964d9e -r e6649b85a055 agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Tue Feb 18 15:42:38 2014 +0100 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Thu Apr 17 15:45:46 2014 -0400 @@ -85,18 +85,20 @@ configuration.setDatabaseURL(db); } - configuration.setPurge(true); - if (properties.containsKey(AgentProperties.SAVE_ON_EXIT.name())) { - String purge = (String) properties.get(AgentProperties.SAVE_ON_EXIT.name()); - configuration.setPurge(!Boolean.parseBoolean(purge)); + String saveOnExit = properties.getProperty(AgentProperties.SAVE_ON_EXIT.name()); + if (saveOnExit != null) { + configuration.setPurge(!Boolean.parseBoolean(saveOnExit)); + } else { + configuration.setPurge(true); } - // TODO: we could avoid this, which means the agent doesn't want to - // accept any connection - configuration.setConfigListenAddress("127.0.0.1:12000"); - if (properties.containsKey(AgentProperties.CONFIG_LISTEN_ADDRESS.name())) { - String address = properties.getProperty(AgentProperties.CONFIG_LISTEN_ADDRESS.name()); - configuration.setConfigListenAddress(address); + String configListenAddress = properties.getProperty(AgentProperties.CONFIG_LISTEN_ADDRESS.name()); + if (configListenAddress != null) { + configuration.setConfigListenAddress(configListenAddress); + } else { + // TODO: we could avoid this, which means the agent doesn't want to + // accept any connection + configuration.setConfigListenAddress("127.0.0.1:12000"); } } } diff -r c3eead964d9e -r e6649b85a055 agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java --- a/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Tue Feb 18 15:42:38 2014 +0100 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Thu Apr 17 15:45:46 2014 -0400 @@ -40,38 +40,109 @@ import java.io.IOException; import java.util.Properties; +import org.junit.After; import org.junit.Assert; -import org.junit.BeforeClass; import org.junit.Test; import com.redhat.thermostat.shared.config.InvalidConfigurationException; import com.redhat.thermostat.testutils.TestUtils; public class AgentConfigsUtilsTest { + + private File tmpDir; - @BeforeClass - public static void setUpOnce() { + @After + public void tearDown() throws IOException { + if (tmpDir != null) { + TestUtils.deleteRecursively(tmpDir); + } + AgentConfigsUtils.setConfigFiles(null, null); + tmpDir = null; + } + + @Test + public void testSystemDbUrl() throws InvalidConfigurationException, IOException { + Properties sysProps = createSystemProperties(); + setConfigs(sysProps, new Properties()); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + + Assert.assertEquals("http://1.2.3.4:9001/hello", config.getDBConnectionString()); + } + + @Test + public void testSystemPurgeProp() throws InvalidConfigurationException, IOException { + Properties sysProps = createSystemProperties(); + setConfigs(sysProps, new Properties()); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + + Assert.assertFalse(config.purge()); + } + + @Test + public void testSystemAddressProp() { + Properties sysProps = createSystemProperties(); + setConfigs(sysProps, new Properties()); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + Assert.assertEquals("42.42.42.42:42", config.getConfigListenAddress()); + } + + @Test + public void testUserDbUrl() throws InvalidConfigurationException, IOException { + Properties sysProps = createSystemProperties(); + Properties userProps = createUserProperties(); + setConfigs(sysProps, userProps); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + + Assert.assertEquals("http://5.6.7.8:9002/world", config.getDBConnectionString()); + } + + @Test + public void testUserPurgeProp() throws InvalidConfigurationException, IOException { + Properties sysProps = createSystemProperties(); + Properties userProps = createUserProperties(); + setConfigs(sysProps, userProps); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + + Assert.assertTrue(config.purge()); + } + + @Test + public void testUserAddressProp() { + Properties sysProps = createSystemProperties(); + Properties userProps = createUserProperties(); + setConfigs(sysProps, userProps); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + + Assert.assertEquals("24.24.24.24:24", config.getConfigListenAddress()); + } + + private Properties createSystemProperties() { Properties agentProperties = new Properties(); + agentProperties.setProperty("DB_URL", "http://1.2.3.4:9001/hello"); agentProperties.setProperty("SAVE_ON_EXIT", "true"); agentProperties.setProperty("CONFIG_LISTEN_ADDRESS", "42.42.42.42:42"); - + return agentProperties; + } + + private Properties createUserProperties() { + Properties agentProperties = new Properties(); + agentProperties.setProperty("DB_URL", "http://5.6.7.8:9002/world"); + agentProperties.setProperty("SAVE_ON_EXIT", "false"); + agentProperties.setProperty("CONFIG_LISTEN_ADDRESS", "24.24.24.24:24"); + return agentProperties; + } + + private void setConfigs(Properties sysProps, Properties userProps) { try { - TestUtils.setupAgentConfigs(agentProperties); - File agentConf = TestUtils.getAgentConfFile(); - // By default system config == user config - AgentConfigsUtils.setConfigFiles(agentConf, agentConf); + String tmpDirPath = TestUtils.setupAgentConfigs(sysProps, userProps); + tmpDir = new File(tmpDirPath); + File sysAgentConf = TestUtils.getAgentConfFile(); + File userAgentConf = TestUtils.getUserAgentConfFile(); + AgentConfigsUtils.setConfigFiles(sysAgentConf, userAgentConf); } catch (IOException e) { throw new AssertionError("Unable to create agent configuration", e); } } - - @Test - public void testCreateAgentConfigs() throws InvalidConfigurationException, IOException { - AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); - - Assert.assertFalse(config.purge()); - Assert.assertEquals("42.42.42.42:42", config.getConfigListenAddress()); - } } diff -r c3eead964d9e -r e6649b85a055 common/test/src/main/java/com/redhat/thermostat/testutils/TestUtils.java --- a/common/test/src/main/java/com/redhat/thermostat/testutils/TestUtils.java Tue Feb 18 15:42:38 2014 +0100 +++ b/common/test/src/main/java/com/redhat/thermostat/testutils/TestUtils.java Thu Apr 17 15:45:46 2014 -0400 @@ -49,7 +49,7 @@ // FIXME the methods in this class can probably be split more sanely public class TestUtils { - private static File agentConf, agentAuth; + private static File sysAgentConf, userAgentConf, agentAuth; /** * @return the process id of the current process @@ -107,8 +107,19 @@ /** * Creates and initializes a directory suitable for use as the agent's * configuration directory + * + * @deprecated use {@link #setupAgentConfigs(Properties, Properties)} instead */ + @Deprecated public static String setupAgentConfigs(Properties agentProperties) throws IOException { + return setupAgentConfigs(agentProperties, agentProperties); + } + + /** + * Creates and initializes a directory suitable for use as the agent's + * configuration directory + */ + public static String setupAgentConfigs(Properties sysProps, Properties userProps) throws IOException { // need to create dummy config files for the tests Random random = new Random(); @@ -116,22 +127,39 @@ Math.abs(random.nextInt()) + File.separatorChar; System.setProperty("THERMOSTAT_HOME", tmpDir); - File etc = makeEtc(tmpDir); - setupSystemAgentConfig(etc, agentProperties); - setupUserAgentConfig(tmpDir); + + File root = new File(tmpDir); + mkdirOrThrow(root); + + // Create system-wide configuration + File sysRoot = new File(root, "system"); + mkdirOrThrow(sysRoot); + File etc = makeEtc(sysRoot); + setupSystemAgentConfig(etc, sysProps); setupAgentAuth(etc); + + // Create user-specific configuration + File userRoot = new File(root, "user"); + mkdirOrThrow(userRoot); + System.setProperty("USER_THERMOSTAT_HOME", userRoot.getAbsolutePath()); + File userEtc = makeEtc(userRoot); + setupUserAgentConfig(userEtc, userProps); return tmpDir; } public static File getAgentConfFile() { - return agentConf; + return sysAgentConf; + } + + public static File getUserAgentConfFile() { + return userAgentConf; } public static File getAgentAuthFile() { return agentAuth; } - + public static void deleteRecursively(File root) throws IOException { if (root.isFile()) { root.delete(); @@ -148,16 +176,16 @@ } } - private static File makeEtc(String root) { - File etc = new File(root, "etc"); - etc.mkdirs(); + private static File makeEtc(File sysRoot) throws IOException { + File etc = new File(sysRoot, "etc"); + mkdirOrThrow(etc); return etc; } private static void setupSystemAgentConfig(File etc, Properties agentProperties) throws FileNotFoundException, IOException { - agentConf = new File(etc, "agent.properties"); + sysAgentConf = new File(etc, "agent.properties"); - try (OutputStream propsOutputStream = new FileOutputStream(agentConf)) { + try (OutputStream propsOutputStream = new FileOutputStream(sysAgentConf)) { agentProperties.store(propsOutputStream, "thermostat agent test properties"); } } @@ -170,16 +198,18 @@ authWriter.close(); } - private static File setupUserAgentConfig(String root) throws IOException { - System.setProperty("USER_THERMOSTAT_HOME", root); - - File agent = new File(root, "agent"); - agent.mkdirs(); - - new File(agent, "run").mkdirs(); - new File(agent, "logs").mkdirs(); - return agent; - + private static void setupUserAgentConfig(File etc, Properties agentProperties) throws IOException { + userAgentConf = new File(etc, "agent.properties"); + + try (OutputStream propsOutputStream = new FileOutputStream(userAgentConf)) { + agentProperties.store(propsOutputStream, "thermostat agent test properties"); + } + } + + private static void mkdirOrThrow(File dir) throws IOException { + if (!dir.mkdirs()) { + throw new IOException("Failed to create user configuration directory"); + } } }