Mercurial > hg > release > thermostat-1.0

changeset 1394:e6649b85a055

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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
author Elliott Baron <ebaron@redhat.com>
date Thu, 17 Apr 2014 15:45:46 -0400
parents c3eead964d9e
children 2f3b449bab76
files agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java common/test/src/main/java/com/redhat/thermostat/testutils/TestUtils.java
diffstat 3 files changed, 150 insertions(+), 47 deletions(-) [+]
line wrap: on
line diff
--- 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");
         }
     }
 }
--- 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());
-    }
 }
 
--- 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");
+        }
     }
 }