QPID-1980 : Update to ServerConfiguration provided by Keith Chow, Updated ServerconfigurationTest

Added ServerConfigurationFileTest that verifies the string value is loadable from the systest config file.

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

4 files changed, 121 insertions, 40 deletions

diff --git a/java/broker/etc/config-systests.xml b/java/broker/etc/config-systests.xml
index 9715963873..5193072746 100644
--- a/java/broker/etc/config-systests.xml
+++ b/java/broker/etc/config-systests.xml
@@ -35,6 +35,8 @@
         <qpidnio>false</qpidnio>
         <protectio>
             <enabled>false</enabled>
+            <readBufferLimitSize>262144</readBufferLimitSize>
+            <writeBufferLimitSize>262144</writeBufferLimitSize>
         </protectio>
         <transport>nio</transport>
         <port>5672</port>
diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
index fc16b75e1a..0bb4c050a3 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
@@ -25,6 +25,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Locale;
 import java.util.Map.Entry;
 
 import org.apache.commons.configuration.CompositeConfiguration;
@@ -37,7 +38,6 @@ import org.apache.qpid.server.configuration.management.ConfigurationManagementMB
 import org.apache.qpid.server.registry.ApplicationRegistry;
 import org.apache.qpid.server.virtualhost.VirtualHost;
 import org.apache.qpid.server.virtualhost.VirtualHostRegistry;
-import org.apache.qpid.tools.messagestore.MessageStoreTool;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -49,9 +49,13 @@ public class ServerConfiguration implements SignalHandler
 
     private Configuration _config;
 
+    // Default Configuration values
+    //todo make these all public, to make validation of configuration easier.
+    public static final int DEFAULT_BUFFER_READ_LIMIT_SIZE = 262144;
+    public static final int DEFAULT_BUFFER_WRITE_LIMIT_SIZE = 262144;
+    public static final boolean DEFAULT_BROKER_CONNECTOR_PROTECTIO_ENABLED = false;
+
     private static final int DEFAULT_FRAME_SIZE = 65536;
-    private static final int DEFAULT_BUFFER_READ_LIMIT_SIZE = 262144;
-    private static final int DEFAULT_BUFFER_WRITE_LIMIT_SIZE = 262144;
     private static final int DEFAULT_PORT = 5672;
     private static final int DEFAUL_SSL_PORT = 8672;
     private static final long DEFAULT_HOUSEKEEPING_PERIOD = 30000L;
@@ -63,15 +67,21 @@ public class ServerConfiguration implements SignalHandler
     private SecurityConfiguration _securityConfiguration = null;
 
     private File _configFile;
-    
+
     private Logger _log = LoggerFactory.getLogger(this.getClass());
 
     private ConfigurationManagementMBean _mbean;
-    
+
 
     // Map of environment variables to config items
     private static final Map<String, String> envVarMap = new HashMap<String, String>();
-    
+
+    // Configuration values to be read from the configuration file
+    //todo Move all properties to static values to ensure system testing can be performed.
+    public static final String CONNECTOR_PROTECTIO_ENABLED = "connector.protectio.enabled";
+    public static final String CONNECTOR_PROTECTIO_READ_BUFFER_LIMIT_SIZE = "connector.protectio.readBufferLimitSize";
+    public static final String CONNECTOR_PROTECTIO_WRITE_BUFFER_LIMIT_SIZE = "connector.protectio.writeBufferLimitSize";
+
     {
         envVarMap.put("QPID_PORT", "connector.port");
         envVarMap.put("QPID_ENABLEDIRECTBUFFERS", "advanced.enableDirectBuffers");
@@ -96,16 +106,16 @@ public class ServerConfiguration implements SignalHandler
         envVarMap.put("QPID_ENABLEPOOLEDALLOCATOR", "advanced.enablePooledAllocator");
         envVarMap.put("QPID_STATUS-UPDATES", "status-updates");
     }
-    
+
     public ServerConfiguration(File configurationURL) throws ConfigurationException
     {
         this(parseConfig(configurationURL));
         _configFile = configurationURL;
-        try 
+        try
         {
             Signal sig = new sun.misc.Signal("HUP");
             sun.misc.Signal.handle(sig, this);
-        } 
+        }
         catch (IllegalArgumentException e)
         {
             // We're on something that doesn't handle SIGHUP, how sad, Windows. 
@@ -115,16 +125,16 @@ public class ServerConfiguration implements SignalHandler
     public ServerConfiguration(Configuration conf) throws ConfigurationException
     {
         setConfig(conf);
-        
+
         substituteEnvironmentVariables();
-        
+
         _jmxPort = getConfig().getInt("management.jmxport", 8999);
         _securityConfiguration = new SecurityConfiguration(conf.subset("security"));
 
         setupVirtualHosts(conf);
-        
+
     }
-    
+
     private void setupVirtualHosts(Configuration conf) throws ConfigurationException
     {
         List vhosts = conf.getList("virtualhosts");
@@ -140,7 +150,7 @@ public class ServerConfiguration implements SignalHandler
                 {
                     String name = (String) hosts.get(j);
                     // Add the keys of the virtual host to the main config then bail out
-                    
+
                     Configuration myConf = vhostConfiguration.subset("virtualhost." + name);
                     Iterator k = myConf.getKeys();
                     while (k.hasNext())
@@ -169,13 +179,13 @@ public class ServerConfiguration implements SignalHandler
             String val = System.getenv(var.getKey());
             if (val != null)
             {
-                getConfig().setProperty(var.getValue(), val); 
+                getConfig().setProperty(var.getValue(), val);
             }
         }
     }
 
     private final static Configuration parseConfig(File file) throws ConfigurationException
-    {      
+    {
         ConfigurationFactory factory = new ConfigurationFactory();
         factory.setConfigurationFileName(file.getAbsolutePath());
         Configuration conf = factory.getConfiguration();
@@ -202,7 +212,7 @@ public class ServerConfiguration implements SignalHandler
             return super.interpolate(obj);
         }
     }
-    
+
     private final static Configuration flatConfig(File file) throws ConfigurationException
     {
         // We have to override the interpolate methods so that
@@ -238,7 +248,7 @@ public class ServerConfiguration implements SignalHandler
         catch (ConfigurationException e)
         {
              _log.error("Could not reload configuration file", e);
-        }        
+        }
     }
 
     public void reparseConfigFile() throws ConfigurationException
@@ -248,7 +258,7 @@ public class ServerConfiguration implements SignalHandler
             Configuration newConfig = parseConfig(_configFile);
             _securityConfiguration = new SecurityConfiguration(newConfig.subset("security"));
             ApplicationRegistry.getInstance().getAccessManager().configurePlugins(_securityConfiguration);
-            
+
             VirtualHostRegistry vhostRegistry = ApplicationRegistry.getInstance().getVirtualHostRegistry();
             for (String hostname : _virtualHosts.keySet())
             {
@@ -343,17 +353,17 @@ public class ServerConfiguration implements SignalHandler
 
     public boolean getProtectIOEnabled()
     {
-        return getConfig().getBoolean("broker.connector.protectio.enabled", false);
+        return getConfig().getBoolean(CONNECTOR_PROTECTIO_ENABLED, DEFAULT_BROKER_CONNECTOR_PROTECTIO_ENABLED);
     }
 
     public int getBufferReadLimit()
     {
-        return getConfig().getInt("broker.connector.protectio.readBufferLimitSize", DEFAULT_BUFFER_READ_LIMIT_SIZE);
+        return getConfig().getInt(CONNECTOR_PROTECTIO_READ_BUFFER_LIMIT_SIZE, DEFAULT_BUFFER_READ_LIMIT_SIZE);
     }
 
     public int getBufferWriteLimit()
     {
-        return getConfig().getInt("broker.connector.protectio.writeBufferLimitSize", DEFAULT_BUFFER_WRITE_LIMIT_SIZE);
+        return getConfig().getInt(CONNECTOR_PROTECTIO_WRITE_BUFFER_LIMIT_SIZE, DEFAULT_BUFFER_WRITE_LIMIT_SIZE);
     }
 
     public boolean getSynchedClocks()
@@ -543,8 +553,8 @@ public class ServerConfiguration implements SignalHandler
 
     public long getHousekeepingCheckPeriod()
     {
-        return getConfig().getLong("housekeeping.checkPeriod", 
-                   getConfig().getLong("housekeeping.expiredMessageCheckPeriod", 
+        return getConfig().getLong("housekeeping.checkPeriod",
+                   getConfig().getLong("housekeeping.expiredMessageCheckPeriod",
                            DEFAULT_HOUSEKEEPING_PERIOD));
     }
 
diff --git a/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java b/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
index fabd4ce923..a673a75fda 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
@@ -20,31 +20,25 @@
  */
 package org.apache.qpid.server.configuration;
 
-import java.io.File;
-import java.io.FileWriter;
-import java.io.IOException;
-import java.io.RandomAccessFile;
-import java.util.Iterator;
-import java.util.List;
-
+import junit.framework.TestCase;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.PropertiesConfiguration;
-import org.apache.commons.configuration.SystemConfiguration;
 import org.apache.commons.configuration.XMLConfiguration;
-import org.apache.qpid.AMQException;
 import org.apache.qpid.codec.AMQCodecFactory;
 import org.apache.qpid.server.protocol.AMQMinaProtocolSession;
 import org.apache.qpid.server.protocol.AMQProtocolSession;
 import org.apache.qpid.server.protocol.TestIoSession;
-import org.apache.qpid.server.queue.MockProtocolSession;
 import org.apache.qpid.server.registry.ApplicationRegistry;
 import org.apache.qpid.server.registry.ConfigurationFileApplicationRegistry;
-import org.apache.qpid.server.security.access.ACLManager;
 import org.apache.qpid.server.virtualhost.VirtualHost;
 import org.apache.qpid.server.virtualhost.VirtualHostRegistry;
 
-import junit.framework.TestCase;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.util.Iterator;
+import java.util.List;
 
 public class ServerConfigurationTest extends TestCase
 {
@@ -200,7 +194,7 @@ public class ServerConfigurationTest extends TestCase
         assertEquals(false, serverConfig.getProtectIOEnabled());
 
         // Check value we set
-        _config.setProperty("broker.connector.protectio.enabled", true);
+        _config.setProperty(ServerConfiguration.CONNECTOR_PROTECTIO_ENABLED, true);
         serverConfig = new ServerConfiguration(_config);
         assertEquals(true, serverConfig.getProtectIOEnabled());
     }
@@ -212,7 +206,7 @@ public class ServerConfigurationTest extends TestCase
         assertEquals(262144, serverConfig.getBufferReadLimit());
 
         // Check value we set
-        _config.setProperty("broker.connector.protectio.readBufferLimitSize", 23);
+        _config.setProperty(ServerConfiguration.CONNECTOR_PROTECTIO_READ_BUFFER_LIMIT_SIZE, 23);
         serverConfig = new ServerConfiguration(_config);
         assertEquals(23, serverConfig.getBufferReadLimit());
     }
@@ -224,7 +218,7 @@ public class ServerConfigurationTest extends TestCase
         assertEquals(262144, serverConfig.getBufferWriteLimit());
 
         // Check value we set
-        _config.setProperty("broker.connector.protectio.writeBufferLimitSize", 23);
+        _config.setProperty(ServerConfiguration.CONNECTOR_PROTECTIO_WRITE_BUFFER_LIMIT_SIZE, 23);
         serverConfig = new ServerConfiguration(_config);
         assertEquals(23, serverConfig.getBufferWriteLimit());
     }
diff --git a/java/systests/src/main/java/org/apache/qpid/server/configuration/ServerConfigurationFileTest.java b/java/systests/src/main/java/org/apache/qpid/server/configuration/ServerConfigurationFileTest.java
new file mode 100644
index 0000000000..c25a53c4ea
--- /dev/null
+++ b/java/systests/src/main/java/org/apache/qpid/server/configuration/ServerConfigurationFileTest.java
@@ -0,0 +1,75 @@
+/*
+ *
+ * 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.qpid.server.configuration;
+
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.qpid.test.utils.QpidTestCase;
+
+/**
+ * This system test ensures that when loading our default system-test
+ * configuration file the configuration is correctly loaded.
+ *
+ * All configuration values should be set in the systest config file so that
+ * the ability to load them can be validated.
+ *
+ */
+public class ServerConfigurationFileTest extends QpidTestCase
+{
+    ServerConfiguration _serverConfig;
+
+    public void setUp() throws ConfigurationException
+    {
+        if (!_configFile.exists())
+        {
+            fail("Unable to test without config file:" + _configFile);
+        }
+        _serverConfig = new ServerConfiguration(_configFile);
+    }
+
+    /**
+     * This helper method ensures that when we attempt to read a value that is
+     * set in the configuration file we do actualy read a value and not
+     * simply get a defaulted value from the ServerConfiguration.get*() methods.
+     *
+     * @param property the propert to test
+     */
+    private void validatePropertyDefinedInFile(String property)
+    {
+        //Verify that we are not just picking up the the default value from the getBoolean
+        assertNotNull("The value set in the configuration file is not being read for property:" + property,
+                      _serverConfig.getConfig().getProperty(property));
+    }
+
+    public void testProtectIOEnabled() throws ConfigurationException
+    {
+        validatePropertyDefinedInFile(ServerConfiguration.CONNECTOR_PROTECTIO_ENABLED);
+    }
+
+    public void testProtectIOReadBufferLimitSize() throws ConfigurationException
+    {
+        validatePropertyDefinedInFile(ServerConfiguration.CONNECTOR_PROTECTIO_READ_BUFFER_LIMIT_SIZE);
+    }
+
+    public void testProtectIOWriteBufferLimitSize() throws ConfigurationException
+    {
+        validatePropertyDefinedInFile(ServerConfiguration.CONNECTOR_PROTECTIO_WRITE_BUFFER_LIMIT_SIZE);
+    }
+}