QPID-2581 : Update Plugins that use configuration to only throw ConfigurationExceptions during the configuration phase of loading configuration.

Creating a plugin and providing it with configuration should not throw a configuration exception.
Added configuration validation to newer plugins SimpleXML still needs validation.
todo Docuement Configuration mechanism.

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

2 files changed, 52 insertions, 24 deletions

diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
index d91ee69ce2..6fe0d03741 100644
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
+++ b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
@@ -25,6 +25,7 @@ import java.net.UnknownHostException;
 import java.util.List;
 
 import org.apache.commons.configuration.CompositeConfiguration;
+import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.XMLConfiguration;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
@@ -127,32 +128,15 @@ public class Firewall extends AbstractPlugin
     }
     
 
-    public void configure(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config)
     {
         super.configure(config);
         FirewallConfiguration firewallConfiguration = (FirewallConfiguration) _config;
 
         // Get default action
-        String defaultAction = firewallConfiguration.getConfiguration().getString("[@default-action]");
-        if (defaultAction == null)
-        {
-            _default = Result.ABSTAIN;
-        }
-        else if (defaultAction.equalsIgnoreCase(FirewallRule.ALLOW))
-        {
-            _default = Result.ALLOWED;
-        }
-        else
-        {
-            _default = Result.DENIED;
-        }
+        _default = firewallConfiguration.getDefaultAction();
 
-        CompositeConfiguration finalConfig = new CompositeConfiguration(firewallConfiguration.getConfiguration());
-        List subFiles = firewallConfiguration.getConfiguration().getList("xml[@fileName]");
-        for (Object subFile : subFiles)
-        {
-            finalConfig.addConfiguration(new XMLConfiguration((String) subFile));
-        }
+        Configuration finalConfig = firewallConfiguration.getConfiguration();
 
         // all rules must have an access attribute
         int numRules = finalConfig.getList("rule[@access]").size();
diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallConfiguration.java b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallConfiguration.java
index fe9f48f950..1a2bf62d73 100644
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallConfiguration.java
+++ b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/FirewallConfiguration.java
@@ -23,14 +23,20 @@ package org.apache.qpid.server.security.access.plugins;
 import java.util.Arrays;
 import java.util.List;
 
+import org.apache.commons.configuration.CompositeConfiguration;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.XMLConfiguration;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory;
+import org.apache.qpid.server.security.Result;
+import org.apache.qpid.server.security.access.config.FirewallRule;
 
 public class FirewallConfiguration extends ConfigurationPlugin
 {
-    public static final ConfigurationPluginFactory FACTORY = new ConfigurationPluginFactory() 
+    CompositeConfiguration _finalConfig;
+
+    public static final ConfigurationPluginFactory FACTORY = new ConfigurationPluginFactory()
     {
         public ConfigurationPlugin newInstance(String path, Configuration config) throws ConfigurationException
         {
@@ -41,17 +47,55 @@ public class FirewallConfiguration extends ConfigurationPlugin
 
         public List<String> getParentPaths()
         {
-            return Arrays.asList("security", "virtualhosts.virtualhost.security");
+            return Arrays.asList("security.firewall", "virtualhosts.virtualhost.security.firewall");
         }
     };
 
     public String[] getElementsProcessed()
     {
-        return new String[] { "firewall" };
+        return new String[] { "" };
     }
 
     public Configuration getConfiguration()
     {
-        return _configuration.subset("firewall");
+        return _finalConfig;
     }
+
+    public Result getDefaultAction()
+    {
+        String defaultAction = _configuration.getString("[@default-action]");
+        if (defaultAction == null)
+        {
+            return Result.ABSTAIN;
+        }
+        else if (defaultAction.equalsIgnoreCase(FirewallRule.ALLOW))
+        {
+            return Result.ALLOWED;
+        }
+        else
+        {
+            return Result.DENIED;
+        }
+    }
+
+
+
+    @Override
+    public void validateConfiguration() throws ConfigurationException
+    {
+        // Valid Configuration either has xml links to new files
+        _finalConfig = new CompositeConfiguration(_configuration);
+        List subFiles = _configuration.getList("xml[@fileName]");
+        for (Object subFile : subFiles)
+        {
+            _finalConfig.addConfiguration(new XMLConfiguration((String) subFile));
+        }
+
+        // all rules must have an access attribute
+        if (_finalConfig.getList("rule[@access]").size() == 0)
+        {
+            throw new ConfigurationException("No rules found in firewall configuration.");
+        }
+    }
+
 }