QPID-4815: make BrokerOptions the primary source of the location for the logging configuration file

merged from trunk r1479309


git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/0.22@1479311 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 55 insertions, 53 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java
index 25322c7a71..703162a609 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java
@@ -109,7 +109,6 @@ public class Broker
 
     private void startupImpl(final BrokerOptions options) throws Exception
     {
-        final String qpidHome = System.getProperty(BrokerProperties.PROPERTY_QPID_HOME);
         String storeLocation = options.getConfigurationStoreLocation();
         String storeType = options.getConfigurationStoreType();
 
@@ -119,8 +118,7 @@ public class Broker
         //embedding the broker and want to configure it themselves.
         if(!options.isSkipLoggingConfiguration())
         {
-            File logConfigFile = getConfigFile(options.getLogConfigFile(), BrokerOptions.DEFAULT_LOG_CONFIG_FILE, qpidHome, false);
-            configureLogging(logConfigFile, options.getLogWatchFrequency());
+            configureLogging(new File(options.getLogConfigFileLocation()), options.getLogWatchFrequency());
         }
 
         BrokerConfigurationStoreCreator storeCreator = new BrokerConfigurationStoreCreator();
@@ -152,36 +150,6 @@ public class Broker
 
     }
 
-
-    private File getConfigFile(final String fileName,
-                               final String defaultFileName,
-                               final String qpidHome, boolean throwOnFileNotFound) throws InitException
-    {
-        File configFile = null;
-        if (fileName != null)
-        {
-            configFile = new File(fileName);
-        }
-        else
-        {
-            configFile = new File(qpidHome, defaultFileName);
-        }
-
-        if (!configFile.exists() && throwOnFileNotFound)
-        {
-            String error = "File " + configFile + " could not be found. Check the file exists and is readable.";
-
-            if (qpidHome == null)
-            {
-                error = error + "\nNote: " + BrokerProperties.PROPERTY_QPID_HOME + " is not set.";
-            }
-
-            throw new InitException(error, null);
-        }
-
-        return configFile;
-    }
-
     public static void parsePortList(Set<Integer> output, List<?> ports) throws InitException
     {
         if(ports != null)
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java
index ac6ee55c86..239726ba63 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java
@@ -87,11 +87,6 @@ public class BrokerOptions
     private boolean _overwriteConfigurationStore;
     private Map<String, String> _configProperties = new HashMap<String,String>();
 
-    public String getLogConfigFile()
-    {
-        return _logConfigFile;
-    }
-
     public String getManagementModePassword()
     {
         if(_managementModePassword == null)
@@ -107,11 +102,6 @@ public class BrokerOptions
         _managementModePassword = managementModePassword;
     }
 
-    public void setLogConfigFile(final String logConfigFile)
-    {
-        _logConfigFile = logConfigFile;
-    }
-
     public int getLogWatchFrequency()
     {
         return _logWatchFrequency;
@@ -332,6 +322,32 @@ public class BrokerOptions
         return Collections.unmodifiableMap(properties);
     }
 
+    /**
+     * Get the broker logging configuration file location.
+     *
+     * If not previously explicitly set, defaults to {@value #DEFAULT_LOG_CONFIG_FILE} within the broker
+     * home directory if configured (gathered via config property {@link #QPID_HOME_DIR}) or the current
+     * JVM working directory if not.
+     *
+     * @return the previously set logging configuration file location, or the default location if none was set.
+     */
+    public String getLogConfigFileLocation()
+    {
+        if(_logConfigFile == null)
+        {
+            String homeDir = getHomeDir();
+
+            return new File(homeDir, DEFAULT_LOG_CONFIG_FILE).getAbsolutePath();
+        }
+
+        return _logConfigFile;
+    }
+
+    public void setLogConfigFileLocation(final String logConfigFile)
+    {
+        _logConfigFile = logConfigFile;
+    }
+
     private String getWorkDir()
     {
         if(!_configProperties.containsKey(QPID_WORK_DIR))
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java
index 03514939ec..66ef296a4b 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java
@@ -258,7 +258,7 @@ public class Main
             String logConfig = _commandLine.getOptionValue(OPTION_LOG_CONFIG_FILE.getOpt());
             if(logConfig != null)
             {
-                options.setLogConfigFile(logConfig);
+                options.setLogConfigFileLocation(logConfig);
             }
 
             boolean overwriteConfigurationStore = _commandLine.hasOption(OPTION_OVERWRITE_CONFIGURATION_STORE.getOpt());
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/BrokerOptionsTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/BrokerOptionsTest.java
index 47710ed485..7d956881f5 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/BrokerOptionsTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/BrokerOptionsTest.java
@@ -23,6 +23,7 @@ package org.apache.qpid.server;
 import java.io.File;
 import java.util.Map;
 
+import org.apache.qpid.server.configuration.BrokerProperties;
 import org.apache.qpid.test.utils.QpidTestCase;
 
 public class BrokerOptionsTest extends QpidTestCase
@@ -83,16 +84,30 @@ public class BrokerOptionsTest extends QpidTestCase
         assertEquals(testConfigFile, _options.getConfigurationStoreLocation());
     }
 
-    public void testDefaultLogConfigFile()
+    public void testDefaultLogConfigFileWithQpidHome()
     {
-        assertNull(_options.getLogConfigFile());
+        String qpidHome = "/test/value";
+        setTestSystemProperty(BrokerProperties.PROPERTY_QPID_HOME, qpidHome);
+
+        String expectedPath = new File(qpidHome, BrokerOptions.DEFAULT_LOG_CONFIG_FILE).getAbsolutePath();
+
+        assertEquals(expectedPath, _options.getLogConfigFileLocation());
+    }
+
+    public void testDefaultLogConfigFileWithoutQpiddHome()
+    {
+        setTestSystemProperty(BrokerProperties.PROPERTY_QPID_HOME, null);
+
+        String expectedPath = new File(BrokerOptions.DEFAULT_LOG_CONFIG_FILE).getAbsolutePath();
+
+        assertEquals(expectedPath, _options.getLogConfigFileLocation());
     }
 
     public void testOverriddenLogConfigFile()
     {
         final String testLogConfigFile = "etc/mytestlog4j.xml";
-        _options.setLogConfigFile(testLogConfigFile);
-        assertEquals(testLogConfigFile, _options.getLogConfigFile());
+        _options.setLogConfigFileLocation(testLogConfigFile);
+        assertEquals(testLogConfigFile, _options.getLogConfigFileLocation());
     }
 
     public void testDefaultLogWatchFrequency()
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/MainTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/MainTest.java
index 7e451fca86..27ad5a0313 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/MainTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/MainTest.java
@@ -38,16 +38,19 @@ public class MainTest extends QpidTestCase
 
     public void testNoOptionsSpecified()
     {
-        BrokerOptions options = startDummyMain("");
-
         String qpidWork = "/qpid/work";
         setTestSystemProperty(BrokerProperties.PROPERTY_QPID_WORK, qpidWork);
+        String qpidHome = "/qpid/home";
+        setTestSystemProperty(BrokerProperties.PROPERTY_QPID_HOME, qpidHome);
 
         String expectedStorePath = new File(qpidWork, BrokerOptions.DEFAULT_CONFIG_NAME_PREFIX + ".json").getAbsolutePath();
+        String expectedLogConfigPath = new File(qpidHome, BrokerOptions.DEFAULT_LOG_CONFIG_FILE).getAbsolutePath();
+
+        BrokerOptions options = startDummyMain("");
 
         assertEquals("json", options.getConfigurationStoreType());
         assertEquals(expectedStorePath, options.getConfigurationStoreLocation());
-        assertEquals(null, options.getLogConfigFile());
+        assertEquals(expectedLogConfigPath, options.getLogConfigFileLocation());
         assertEquals(0, options.getLogWatchFrequency());
         assertEquals(BrokerOptions.DEFAULT_INITIAL_CONFIG_LOCATION, options.getInitialConfigurationLocation());
         assertFalse(options.isOverwriteConfigurationStore());
@@ -88,7 +91,7 @@ public class MainTest extends QpidTestCase
     {
         BrokerOptions options = startDummyMain("-l wxyz/log4j.xml");
 
-        assertEquals("wxyz/log4j.xml", options.getLogConfigFile());
+        assertEquals("wxyz/log4j.xml", options.getLogConfigFileLocation());
     }
 
     public void testLogWatch()
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
index c14c724419..e61efd3e32 100755
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
@@ -475,7 +475,7 @@ public class QpidBrokerTestCase extends QpidTestCase
 
             //Set the log config file, relying on the log4j.configuration system property
             //set on the JVM by the JUnit runner task in module.xml.
-            options.setLogConfigFile(_logConfigFile.getAbsolutePath());
+            options.setLogConfigFileLocation(_logConfigFile.getAbsolutePath());
 
             Broker broker = new Broker();
             _logger.info("Starting internal broker (same JVM)");