QPID-2581 : Update Plugins to have a consistent configure(ConfigurationPlugin config) method.

Further work is required to ensure that all ConfigurationPlugins perform config validation rather than leaving that to the plugin.
The plugin should just use the config.

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

13 files changed, 61 insertions, 95 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 75846b26ef..98fc198cc8 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
@@ -60,17 +60,12 @@ public class AccessControl extends AbstractPlugin
 
         public AccessControl newInstance(ConfigurationPlugin config) throws ConfigurationException
         {
-            AccessControl plugin = new AccessControl(config);
-            plugin.configure();
+            AccessControl plugin = new AccessControl();
+            plugin.configure(config);
             return plugin;
         }
     };
-    
-    public AccessControl(ConfigurationPlugin config)
-    {
-        _config = config.getConfiguration(AccessControlConfiguration.class);
-    }
-    
+
     public Result getDefault()
     {
         return _ruleSet.getDefault();
@@ -112,9 +107,9 @@ public class AccessControl extends AbstractPlugin
         return _ruleSet.check(principal.getName(), operation, objectType, properties);
     }
 
-    @Override
-    public void configure() throws ConfigurationException
+    public void configure(ConfigurationPlugin config) throws ConfigurationException
     {
+        _config = config.getConfiguration(AccessControlConfiguration.class);        
         AccessControlConfiguration accessConfig = (AccessControlConfiguration) _config;
             
         if (isConfigured())
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 5aba110462..9dd0ec47ec 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
@@ -144,11 +144,6 @@ class SlowConsumerDetection extends VirtualHostHouseKeepingPlugin
         return false;
     }
 
-    public String getPluginName()
-    {
-        return SlowConsumerDetection.class.getName();
-    }
-
     public boolean isConfigured()
     {
         return _config != null && _virtualhost != null;
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 3ddccb3497..05ff75f138 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
@@ -47,7 +47,9 @@ public class TopicDeletePolicy implements SlowConsumerPolicyPlugin
             TopicDeletePolicyConfiguration config =
                     configuration.getConfiguration(TopicDeletePolicyConfiguration.class);
             
-            return new TopicDeletePolicy(config);
+            TopicDeletePolicy policy = new TopicDeletePolicy();
+            policy.configure(config);
+            return policy;
         }
 
         public String getPluginName()
@@ -61,11 +63,6 @@ public class TopicDeletePolicy implements SlowConsumerPolicyPlugin
         }
     }
 
-    public TopicDeletePolicy(TopicDeletePolicyConfiguration config)
-    {
-        _configuration = config;
-    }
-
     public void performPolicy(AMQQueue q)
     {
         if (q == null)
@@ -131,21 +128,13 @@ public class TopicDeletePolicy implements SlowConsumerPolicyPlugin
         return false;
     }
 
-    @Override
-    public String getPluginName()
-    {
-        return "topicdelete";
-    }
-
-    @Override
     public boolean isConfigured()
     {
         return _configuration != null;
     }
 
-    @Override
-    public void configure() throws ConfigurationException
+    public void configure(ConfigurationPlugin config) throws ConfigurationException
     {
-        // Empty
+        _configuration = (TopicDeletePolicyConfiguration) config;
     }
 }
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyTest.java b/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyTest.java
index 19954cb652..253745eec4 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyTest.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyTest.java
@@ -100,9 +100,10 @@ public class TopicDeletePolicyTest extends InternalBrokerBaseCase
 
 
     /** Check that a null queue passed in does not upset the policy. */
-    public void testNullQueueParameter()
+    public void testNullQueueParameter() throws ConfigurationException
     {
-        TopicDeletePolicy policy = new TopicDeletePolicy(_config);
+        TopicDeletePolicy policy = new TopicDeletePolicy();
+        policy.configure(_config);
 
         try
         {
@@ -119,9 +120,10 @@ public class TopicDeletePolicyTest extends InternalBrokerBaseCase
      * Set a owning Session to null which means this is not an exclusive queue
      * so the queue should not be deleted
      */
-    public void testNonExclusiveQueue()
+    public void testNonExclusiveQueue() throws ConfigurationException
     {
-        TopicDeletePolicy policy = new TopicDeletePolicy(_config);
+        TopicDeletePolicy policy = new TopicDeletePolicy();
+        policy.configure(_config);
 
         MockAMQQueue queue = createOwnedQueue();
 
@@ -139,9 +141,10 @@ public class TopicDeletePolicyTest extends InternalBrokerBaseCase
      *
      * JMS Queues are not to be processed so this should not delete the queue.
      */
-    public void testQueuesAreNotProcessed()
+    public void testQueuesAreNotProcessed() throws ConfigurationException
     {
-        TopicDeletePolicy policy = new TopicDeletePolicy(_config);
+        TopicDeletePolicy policy = new TopicDeletePolicy();
+        policy.configure(_config);
 
         MockAMQQueue queue = createOwnedQueue();
 
@@ -158,9 +161,10 @@ public class TopicDeletePolicyTest extends InternalBrokerBaseCase
      * Give a non auto-delete queue is bound to the topic exchange the
      * TopicDeletePolicy will close the connection and delete the queue,
      */
-    public void testNonAutoDeleteTopicIsNotClosed()
+    public void testNonAutoDeleteTopicIsNotClosed() throws ConfigurationException
     {
-        TopicDeletePolicy policy = new TopicDeletePolicy(_config);
+        TopicDeletePolicy policy = new TopicDeletePolicy();
+        policy.configure(_config);
 
         MockAMQQueue queue = createOwnedQueue();
 
@@ -178,9 +182,10 @@ public class TopicDeletePolicyTest extends InternalBrokerBaseCase
      * Give a auto-delete queue bound to the topic exchange the TopicDeletePolicy will
      * close the connection and delete the queue
      */
-    public void testTopicIsClosed()
+    public void testTopicIsClosed() throws ConfigurationException
     {
-        TopicDeletePolicy policy = new TopicDeletePolicy(_config);
+        TopicDeletePolicy policy = new TopicDeletePolicy();
+        policy.configure(_config);
 
         final MockAMQQueue queue = createOwnedQueue();
 
@@ -198,9 +203,10 @@ public class TopicDeletePolicyTest extends InternalBrokerBaseCase
      * Give a queue bound to the topic exchange the TopicDeletePolicy will
      * close the connection and NOT delete the queue
      */
-    public void testNonAutoDeleteTopicIsClosedNotDeleted() throws AMQException
+    public void testNonAutoDeleteTopicIsClosedNotDeleted() throws AMQException, ConfigurationException
     {
-        TopicDeletePolicy policy = new TopicDeletePolicy(_config);
+        TopicDeletePolicy policy = new TopicDeletePolicy();
+        policy.configure(_config);
 
         MockAMQQueue queue = createOwnedQueue();
 
@@ -217,12 +223,13 @@ public class TopicDeletePolicyTest extends InternalBrokerBaseCase
      * configured with the delete-persistent tag will close the connection
      * and delete the queue
      */
-    public void testPersistentTopicIsClosedAndDeleted()
+    public void testPersistentTopicIsClosedAndDeleted() throws ConfigurationException
     {
         //Set the config to delete persistent queues
         _config.getConfig().addProperty("delete-persistent", "");
 
-        TopicDeletePolicy policy = new TopicDeletePolicy(_config);
+        TopicDeletePolicy policy = new TopicDeletePolicy();
+        policy.configure(_config);
 
         assertTrue("Config was not updated to delete Persistent topics",
                    _config.deletePersistent());
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 acd74d49f5..6c5ecf1d92 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
@@ -43,8 +43,8 @@ public class Firewall extends AbstractPlugin
     {
         public Firewall newInstance(ConfigurationPlugin config) throws ConfigurationException
         {
-            Firewall plugin = new Firewall(config);
-            plugin.configure();
+            Firewall plugin = new Firewall();
+            plugin.configure(config);
             return plugin;
         }
         
@@ -118,19 +118,16 @@ public class Firewall extends AbstractPlugin
         }
     }
     
-    public Firewall(ConfigurationPlugin config)
-    {
-        _config = config.getConfiguration(FirewallConfiguration.class);
-    }
 
-    public void configure() throws ConfigurationException
+    public void configure(ConfigurationPlugin config) throws ConfigurationException
     {
-        FirewallConfiguration config = (FirewallConfiguration) _config;
+        _config = config.getConfiguration(FirewallConfiguration.class);
+        FirewallConfiguration firewallConfiguration = (FirewallConfiguration) _config;
         
         if (isConfigured())
         {
             // Get default action
-            String defaultAction = config.getConfiguration().getString("[@default-action]");
+            String defaultAction = firewallConfiguration.getConfiguration().getString("[@default-action]");
             if (defaultAction == null)
             {
                 _default = Result.ABSTAIN;
@@ -144,8 +141,8 @@ public class Firewall extends AbstractPlugin
                 _default = Result.DENIED;
             }
             
-            CompositeConfiguration finalConfig = new CompositeConfiguration(config.getConfiguration());
-            List subFiles = config.getConfiguration().getList("xml[@fileName]");
+            CompositeConfiguration finalConfig = new CompositeConfiguration(firewallConfiguration.getConfiguration());
+            List subFiles = firewallConfiguration.getConfiguration().getList("xml[@fileName]");
             for (Object subFile : subFiles)
             {
                 finalConfig.addConfiguration(new XMLConfiguration((String) subFile));
diff --git a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
index 431da470e0..ab340ab849 100644
--- a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
+++ b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
@@ -119,9 +119,8 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         // Configure plugin
         FirewallConfiguration config = new FirewallConfiguration();
         config.setConfiguration("", new XMLConfiguration(confFile));
-        Firewall plugin = new Firewall(config);
-        plugin._config = config;
-        plugin.configure();
+        Firewall plugin = new Firewall();
+        plugin.configure(config);
         return plugin;
     }
 
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 4b3d8f02de..26a5c132f9 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
@@ -52,8 +52,8 @@ public class SimpleXML extends AbstractPlugin
     {
         public SimpleXML newInstance(ConfigurationPlugin config) throws ConfigurationException
         {
-            SimpleXML plugin = new SimpleXML(config);
-            plugin.configure();
+            SimpleXML plugin = new SimpleXML();
+            plugin.configure(config);
             return plugin;
         }
 
@@ -68,20 +68,17 @@ public class SimpleXML extends AbstractPlugin
         }
     };
 
-    public SimpleXML(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config) throws ConfigurationException
     {
         _config = config.getConfiguration(SimpleXMLConfiguration.class);
-    }
 
-    public void configure() throws ConfigurationException
-    {
-        SimpleXMLConfiguration config = (SimpleXMLConfiguration) _config;
+        SimpleXMLConfiguration configuration = (SimpleXMLConfiguration) _config;
         
         if (isConfigured())
         {
             _users = new ConcurrentHashMap<String, PrincipalPermissions>();
 
-            processConfig(config.getConfiguration());
+            processConfig(configuration.getConfiguration());
         }
     }
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/plugins/Plugin.java b/java/broker/src/main/java/org/apache/qpid/server/plugins/Plugin.java
index 252bd711e8..27bb050db4 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/plugins/Plugin.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/plugins/Plugin.java
@@ -19,14 +19,11 @@
 package org.apache.qpid.server.plugins;
 
 import org.apache.commons.configuration.ConfigurationException;
+import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
 
 public interface Plugin
 {
-    /**
-     * The name of this plugin.
-     */
-    String getPluginName();
-    
+
     /**
      * Is this plugin configured?.
      */
@@ -35,5 +32,5 @@ public interface Plugin
     /**
      * Configure this plugin
      */
-    void configure() throws ConfigurationException;
+    public void configure(ConfigurationPlugin config) throws ConfigurationException;
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/AbstractPlugin.java b/java/broker/src/main/java/org/apache/qpid/server/security/AbstractPlugin.java
index 8c9c2050e8..e0813b1fa7 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/AbstractPlugin.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/AbstractPlugin.java
@@ -35,11 +35,6 @@ public abstract class AbstractPlugin implements SecurityPlugin
     
     public ConfigurationPlugin _config;
 	
-    public String getPluginName()
-    {
-        return getClass().getSimpleName();
-    }
-	
 	public Result getDefault()
 	{
 		return Result.ABSTAIN;
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
index 82963bbadc..7525362bfc 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/AllowAll.java
@@ -57,8 +57,8 @@ public class AllowAll extends BasicPlugin
     {
         public AllowAll newInstance(ConfigurationPlugin config) throws ConfigurationException
         {
-            AllowAll plugin = new AllowAll(config);
-            plugin.configure();
+            AllowAll plugin = new AllowAll();
+            plugin.configure(config);
             return plugin;
         }
 
@@ -79,8 +79,8 @@ public class AllowAll extends BasicPlugin
 		return Result.ALLOWED;
     }
 
-    public AllowAll(ConfigurationPlugin config)
+    public void configure(ConfigurationPlugin config) throws ConfigurationException
     {
         _config = config.getConfiguration(AllowAllConfiguration.class);
-    }
+    }    
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicPlugin.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicPlugin.java
index 5fc1ef7795..f3161551dc 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicPlugin.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/BasicPlugin.java
@@ -31,7 +31,7 @@ import org.apache.qpid.server.security.access.Operation;
 /**
  * This {@link SecurityPlugin} simply abstains from all authorisation requests and ignores configuration.
  */
-public class BasicPlugin extends AbstractPlugin
+public abstract class BasicPlugin extends AbstractPlugin
 {
     public Result access(ObjectType objectType, Object instance)
     {
@@ -42,10 +42,4 @@ public class BasicPlugin extends AbstractPlugin
     {
         return getDefault();
     }
-
-    @Override
-    public void configure() throws ConfigurationException
-    {
-        // Not used
-    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
index 24af215a0c..7647072abc 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/DenyAll.java
@@ -57,8 +57,8 @@ public class DenyAll extends BasicPlugin
     {
         public DenyAll newInstance(ConfigurationPlugin config) throws ConfigurationException
         {
-            DenyAll plugin = new DenyAll(config);
-            plugin.configure();
+            DenyAll plugin = new DenyAll();
+            plugin.configure(config);
             return plugin;
         }
 
@@ -79,8 +79,9 @@ public class DenyAll extends BasicPlugin
 		return Result.DENIED;
     }
 
-    public DenyAll(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config) throws ConfigurationException
     {
         _config = config.getConfiguration(DenyAllConfiguration.class);
     }
+
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/LegacyAccess.java b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/LegacyAccess.java
index 2c0994b52a..d2b72503a4 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/LegacyAccess.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/access/plugins/LegacyAccess.java
@@ -58,8 +58,8 @@ public class LegacyAccess extends BasicPlugin
     {
         public LegacyAccess newInstance(ConfigurationPlugin config) throws ConfigurationException
         {
-            LegacyAccess plugin = new LegacyAccess(config);
-            plugin.configure();
+            LegacyAccess plugin = new LegacyAccess();
+            plugin.configure(config);
             return plugin;
         }
 
@@ -74,7 +74,7 @@ public class LegacyAccess extends BasicPlugin
         }
     };
     
-    public LegacyAccess(ConfigurationPlugin config) throws ConfigurationException
+    public void configure(ConfigurationPlugin config) throws ConfigurationException
     {
         _config = config.getConfiguration(LegacyAccessConfiguration.class);
     }