QPID-2581 : Update ConfigurationPlugin to provide a validateConfiguration() method simplifying the plugins

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

11 files changed, 55 insertions, 50 deletions

diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java
index 90bd3206e1..b1b7a3c8c9 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java
@@ -65,39 +65,11 @@ public class SlowConsumerDetectionConfiguration extends ConfigurationPlugin
     }
 
     @Override
-    public void setConfiguration(String path, Configuration configuration) throws ConfigurationException
+    public void validateConfiguration() throws ConfigurationException
     {
-        super.setConfiguration(path, configuration);
+        validatePositiveLong("delay");
 
-        //Validate Configuration
-
-        try
-        {
-            long delay = _configuration.getLong("delay");
-            if (delay <= 0)
-            {
-                throw new ConfigurationException("Slow Consumer Detection Delay must be a Positive Long value.");
-            }
-        }
-        catch (Exception e)
-        {
-            Throwable last = e;
-
-            // Find the first cause
-            if (e instanceof ConversionException)
-            {
-                Throwable t = e.getCause();
-                while (t != null)
-                {
-                    last = t;
-                    t = last.getCause();
-                }
-            }
-
-            throw new ConfigurationException("Unable to configure Slow Consumer Detection invalid delay:"+ _configuration.getString("delay"), last);
-        }
-
-        String timeUnit = _configuration.getString("timeunit");
+        String timeUnit = getStringValue("timeunit");
 
 
         if (timeUnit != null)
@@ -108,7 +80,7 @@ public class SlowConsumerDetectionConfiguration extends ConfigurationPlugin
             }
             catch (IllegalArgumentException iae)
             {
-                throw new ConfigurationException("Unable to configure Slow Consumer Detection invalid TimeUnit:" + timeUnit);            
+                throw new ConfigurationException("Unable to configure Slow Consumer Detection invalid TimeUnit:" + timeUnit);
             }
         }
 
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionPolicyConfiguration.java b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionPolicyConfiguration.java
index b6a4ab3cbf..134395097d 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionPolicyConfiguration.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionPolicyConfiguration.java
@@ -59,10 +59,9 @@ public class SlowConsumerDetectionPolicyConfiguration extends ConfigurationPlugi
         return getStringValue("name");
     }
 
-    public void setConfiguration(String path, Configuration configuration) throws ConfigurationException
+    @Override
+    public void validateConfiguration() throws ConfigurationException
     {
-        super.setConfiguration(path,configuration);
-
         if (getPolicyName() == null)
         {
             throw new ConfigurationException("No Slow consumer policy defined.");
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java
index 5f1e0c00b9..75b836bfa8 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java
@@ -81,10 +81,8 @@ public class SlowConsumerDetectionQueueConfiguration extends ConfigurationPlugin
     }
 
     @Override
-    public void setConfiguration(String path, Configuration configuration) throws ConfigurationException
+     public void validateConfiguration() throws ConfigurationException
     {
-        super.setConfiguration(path, configuration);
-
         if (!containsPositiveLong("messageAge") &&
             !containsPositiveLong("depth") &&
             !containsPositiveLong("messageCount"))
@@ -102,8 +100,7 @@ public class SlowConsumerDetectionQueueConfiguration extends ConfigurationPlugin
 
         if (policyConfig == null)
         {
-            throw new ConfigurationException("No Slow Consumer Policy specified at:'" +
-                                             path + "'. Known Policies:" + factories.keySet());
+            throw new ConfigurationException("No Slow Consumer Policy specified. Known Policies:" + factories.keySet());
         }
 
         if (_logger.isDebugEnabled())
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyConfiguration.java b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyConfiguration.java
index 27c214097a..ecd83fb27c 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyConfiguration.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/policies/TopicDeletePolicyConfiguration.java
@@ -54,6 +54,12 @@ public class TopicDeletePolicyConfiguration extends ConfigurationPlugin
         return new String[]{"delete-persistent"};
     }
 
+    @Override
+    public void validateConfiguration() throws ConfigurationException
+    {
+        // No validation requried.
+    }
+
     public boolean deletePersistent()
     {
         // If we don't have configuration then we don't deletePersistent Queues 
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java b/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java
index 049402d83e..00411842f0 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java
@@ -138,7 +138,7 @@ public class SlowConsumerDetectionConfigurationTest extends TestCase
             assertEquals("Cause not correct",
                          ConfigurationException.class, cause.getClass());
             assertEquals("Incorrect message.",
-                         "Slow Consumer Detection Delay must be a Positive Long value.",
+                         "SlowConsumerDetectionConfiguration: 'delay' must be a Positive Long value.",
                          cause.getMessage());
         }
     }
@@ -170,7 +170,7 @@ public class SlowConsumerDetectionConfigurationTest extends TestCase
             assertEquals("Cause not correct",
                          ConfigurationException.class, cause.getClass());
             assertEquals("Incorrect message.",
-                         "Slow Consumer Detection Delay must be a Positive Long value.",
+                         "SlowConsumerDetectionConfiguration: 'delay' must be a Positive Long value.",
                          cause.getMessage());
         }
     }
@@ -194,7 +194,7 @@ public class SlowConsumerDetectionConfigurationTest extends TestCase
         }
         catch (ConfigurationException e)
         {
-            assertEquals("Incorrect message.", "Unable to configure Slow Consumer Detection invalid delay:null", e.getMessage());
+            assertEquals("Incorrect message.", "SlowConsumerDetectionConfiguration: unable to configure invalid delay:null", e.getMessage());
         }
     }
 
@@ -205,7 +205,7 @@ public class SlowConsumerDetectionConfigurationTest extends TestCase
 
         String TIMEUNIT = "foo";
         XMLConfiguration xmlconfig = new XMLConfiguration();
-
+                                                                                
         xmlconfig.addProperty("delay", "10");
         xmlconfig.addProperty("timeunit", TIMEUNIT);
 
diff --git a/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionQueueConfigurationTest.java b/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionQueueConfigurationTest.java
index 8abd6c1408..b149313dfd 100644
--- a/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionQueueConfigurationTest.java
+++ b/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionQueueConfigurationTest.java
@@ -69,7 +69,7 @@ public class SlowConsumerDetectionQueueConfigurationTest extends TestCase
         }
         catch (ConfigurationException e)
         {
-            assertEquals("No Slow Consumer Policy specified at:''. Known Policies:[]",
+            assertEquals("No Slow Consumer Policy specified. Known Policies:[]",
                          e.getMessage());
         }
     }
@@ -125,7 +125,7 @@ public class SlowConsumerDetectionQueueConfigurationTest extends TestCase
         }
         catch (ConfigurationException e)
         {
-            assertEquals("No Slow Consumer Policy specified at:''. Known Policies:[]",
+            assertEquals("No Slow Consumer Policy specified. Known Policies:[]",
                          e.getMessage());
         }
     }
@@ -154,7 +154,7 @@ public class SlowConsumerDetectionQueueConfigurationTest extends TestCase
         }
         catch (ConfigurationException e)
         {
-            assertEquals("No Slow Consumer Policy specified at:''. Known Policies:[]",
+            assertEquals("No Slow Consumer Policy specified. Known Policies:[]",
                          e.getMessage());
         }
     }
@@ -183,7 +183,7 @@ public class SlowConsumerDetectionQueueConfigurationTest extends TestCase
         }
         catch (ConfigurationException e)
         {
-            assertEquals("No Slow Consumer Policy specified at:''. Known Policies:[]",
+            assertEquals("No Slow Consumer Policy specified. Known Policies:[]",
                          e.getMessage());
         }
     }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/QueueConfiguration.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/QueueConfiguration.java
index ee68143a2d..adbb0059d8 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/configuration/QueueConfiguration.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/QueueConfiguration.java
@@ -68,6 +68,12 @@ public class QueueConfiguration extends ConfigurationPlugin
         };
     }
 
+    @Override
+    public void validateConfiguration() throws ConfigurationException
+    {
+        //Currently doesn't do validation
+    }
+
     public VirtualHostConfiguration getVirtualHostConfiguration()
     {
         return _vHostConfig;
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 fe399669fd..8f9bded82a 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
@@ -192,6 +192,12 @@ public class ServerConfiguration extends ConfigurationPlugin implements SignalHa
         return new String[]{""};
     }
 
+    @Override
+    public void validateConfiguration() throws ConfigurationException
+    {
+        //Currently doesn't do validation
+    }
+
     /*
      * Modified to enforce virtualhosts configuration in external file or main file, but not
      * both, as a fix for QPID-2360 and QPID-2361.
diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
index ca1f25952e..00b109f33b 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
@@ -206,6 +206,12 @@ public class VirtualHostConfiguration extends ConfigurationPlugin
 
     }
 
+    @Override
+    public void validateConfiguration() throws ConfigurationException
+    {
+        //Currently doesn't do validation
+    }
+
     public int getHouseKeepingThreadCount()
     {
         return getIntValue("housekeeping.poolSize", Runtime.getRuntime().availableProcessors());
diff --git a/java/broker/src/main/java/org/apache/qpid/server/configuration/plugins/ConfigurationPlugin.java b/java/broker/src/main/java/org/apache/qpid/server/configuration/plugins/ConfigurationPlugin.java
index e6306b70d5..cb21f07eaf 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/configuration/plugins/ConfigurationPlugin.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/configuration/plugins/ConfigurationPlugin.java
@@ -156,8 +156,12 @@ public abstract class ConfigurationPlugin
                 _pluginConfiguration.put(elementHandler.getClass(), elementHandler);
             }
         }
+
+        validateConfiguration();
     }
 
+    abstract public void validateConfiguration() throws ConfigurationException;
+
     protected boolean hasConfiguration()
     {
         return _configuration != null;
diff --git a/java/broker/src/test/java/org/apache/qpid/server/configuration/plugins/ConfigurationPluginTest.java b/java/broker/src/test/java/org/apache/qpid/server/configuration/plugins/ConfigurationPluginTest.java
index 2f2a47cd14..090fb36a4b 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/configuration/plugins/ConfigurationPluginTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/configuration/plugins/ConfigurationPluginTest.java
@@ -28,8 +28,8 @@ import org.apache.commons.configuration.XMLConfiguration;
 import java.util.List;
 
 /**
- * Test that verifies that given a configuration the
- * Plugin manager
+ * Test that verifies that given a Configuration a ConfigurationPlugin can
+ * process and validate that data.
  */
 public class ConfigurationPluginTest extends TestCase
 {
@@ -48,6 +48,12 @@ public class ConfigurationPluginTest extends TestCase
                                 "true", "list", "double"};
         }
 
+        @Override
+        public void validateConfiguration() throws ConfigurationException
+        {
+            // no validation requried
+        }
+
         public String getName()
         {
             return getStringValue("name");
@@ -58,6 +64,7 @@ public class ConfigurationPluginTest extends TestCase
             return getStringValue("[@property]");
         }
 
+
     }
 
     ConfigPlugin _plugin;
@@ -170,6 +177,8 @@ public class ConfigurationPluginTest extends TestCase
 
     public void testBoolean()
     {
+        assertTrue("Boolean value not returned", _plugin.containsBoolean("boolean"));
+        assertFalse("Boolean value not returned", _plugin.containsBoolean("NonExistent"));
         assertTrue("Boolean value not returned", _plugin.getBooleanValue("boolean"));
         assertFalse("default String value not returned", _plugin.getBooleanValue("NonExistent"));
         assertTrue("set default String value not returned", _plugin.getBooleanValue("NonExistent", true));