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

8 files changed, 97 insertions, 60 deletions

diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
index 69e3218bf5..59fbaa4a34 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
@@ -79,13 +79,6 @@ public class AccessControl extends AbstractPlugin
         return _ruleSet.getDefault();
     }
 
-    /** Parse a version two access control file. */
-    private void parseFile(File aclFile) throws ConfigurationException
-    {
-        ConfigurationFile configFile = new PlainConfiguration(aclFile);
-        _ruleSet = configFile.load();
-    }
-    
     /**
      * Object instance access authorisation.
      *
@@ -115,14 +108,12 @@ public class AccessControl extends AbstractPlugin
         return _ruleSet.check(principal.getName(), operation, objectType, properties);
     }
 
-    public void configure(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config)
     {
         super.configure(config);
 
         AccessControlConfiguration accessConfig = (AccessControlConfiguration) _config;
 
-        String fileName = accessConfig.getFileName();
-        File aclFile = new File(fileName);
-        parseFile(aclFile);
+        _ruleSet = accessConfig.getRuleSet();
     }
 }
diff --git a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java
index 2d6ac99a98..f7db740ebc 100644
--- a/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java
+++ b/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
+import java.io.File;
 import java.util.Arrays;
 import java.util.List;
 
@@ -27,6 +28,9 @@ import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory;
+import org.apache.qpid.server.security.access.config.ConfigurationFile;
+import org.apache.qpid.server.security.access.config.PlainConfiguration;
+import org.apache.qpid.server.security.access.config.RuleSet;
 
 public class AccessControlConfiguration extends ConfigurationPlugin
 {
@@ -41,17 +45,39 @@ public class AccessControlConfiguration extends ConfigurationPlugin
 
         public List<String> getParentPaths()
         {
-            return Arrays.asList("security", "virtualhosts.virtualhost.security");
+            return Arrays.asList("security.aclv2", "virtualhosts.virtualhost.security.aclv2");
         }
     };
 
+    private RuleSet _ruleSet;
+
     public String[] getElementsProcessed()
     {
-        return new String[] { "aclv2" };
+        return new String[] { "" };
     }
 
     public String getFileName()
     {
-        return _configuration.getString("aclv2");
+        return _configuration.getString("");
     }
+
+    public void validateConfiguration() throws ConfigurationException
+    {
+        String filename = getFileName();
+        if (filename == null)
+        {
+            throw new ConfigurationException("No ACL file name specified");
+        }
+
+        File aclFile = new File(filename);
+        
+        ConfigurationFile configFile = new PlainConfiguration(aclFile);
+        _ruleSet = configFile.load();
+    }
+
+    public RuleSet getRuleSet()
+    {
+        return _ruleSet;
+    }
+
 }
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java
index 6b5be4a9e2..73ba91f1e8 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java
@@ -38,32 +38,24 @@ class SlowConsumerDetection extends VirtualHostHouseKeepingPlugin
 
     public static class SlowConsumerFactory implements VirtualHostPluginFactory
     {
-        public Class<SlowConsumerDetection> getPluginClass()
+        public SlowConsumerDetection newInstance(VirtualHost vhost)
         {
-            return SlowConsumerDetection.class;
-        }
+            SlowConsumerDetectionConfiguration config = vhost.getConfiguration().getConfiguration(SlowConsumerDetectionConfiguration.class);
 
-        public String getPluginName()
-        {
-            return SlowConsumerDetection.class.getName();
-        }
+            if (config == null)
+            {
+                return null;
+            }
 
-        public SlowConsumerDetection newInstance(VirtualHost vhost) throws ConfigurationException
-        {
             SlowConsumerDetection plugin = new SlowConsumerDetection(vhost);
-            plugin.configure(vhost.getConfiguration());
+            plugin.configure(config);
             return plugin;
         }
     }
 
-    public void configure(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config)
     {        
-        _config = config.getConfiguration(SlowConsumerDetectionConfiguration.class);
-        
-        if (_config == null)
-        {
-            throw new IllegalArgumentException("Plugin has not been configured");
-        }
+        _config = (SlowConsumerDetectionConfiguration) config;
     }
     
     public SlowConsumerDetection(VirtualHost vhost)
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicy.java b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicy.java
index 5262042b17..9bd1dd304f 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicy.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicy.java
@@ -128,7 +128,7 @@ public class TopicDeletePolicy implements SlowConsumerPolicyPlugin
         return false;
     }
 
-    public void configure(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config)
     {
         _configuration = (TopicDeletePolicyConfiguration) config;
     }
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.");
+        }
+    }
+
 }
diff --git a/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java b/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
index 8c38c6e76c..1bf8761978 100644
--- a/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
+++ b/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXML.java
@@ -76,7 +76,7 @@ public class SimpleXML extends AbstractPlugin
         }
     };
 
-    public void configure(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config)
     {
         super.configure(config);
 
diff --git a/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXMLConfiguration.java b/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXMLConfiguration.java
index b73ab97080..e95c21b590 100644
--- a/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXMLConfiguration.java
+++ b/java/broker-plugins/simple-xml/src/main/java/org/apache/qpid/server/security/access/plugins/SimpleXMLConfiguration.java
@@ -41,17 +41,17 @@ public class SimpleXMLConfiguration extends ConfigurationPlugin
 
         public List<String> getParentPaths()
         {
-            return Arrays.asList("security", "virtualhosts.virtualhost.security");
+            return Arrays.asList("security.access_control_list", "virtualhosts.virtualhost.security.access_control_list");
         }
     };
 
     public String[] getElementsProcessed()
     {
-        return new String[] { "access_control_list" };
+        return new String[] { "" };
     }
 
     public Configuration getConfiguration()
     {
-        return _configuration.subset("access_control_list");
+        return _configuration;
     }
 }