You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ri...@apache.org on 2010/05/20 17:20:05 UTC

svn commit: r946669 - in /qpid/trunk/qpid/java: broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/ broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualh...

Author: ritchiem
Date: Thu May 20 15:20:04 2010
New Revision: 946669

URL: http://svn.apache.org/viewvc?rev=946669&view=rev
Log:
QPID-1447 : Modified VirtualHostHouseKeepingPlugin to return a TimeUnit and force plugin to perform validation. Potentially could be refactored to allow all VHHKPlugins to process TimeUnits in a consistent way.
Add Config validation and UnitTest for SlowConsumerDetctionConfiguration.

Added:
    qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java
Modified:
    qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java
    qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java
    qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/plugins/VirtualHostHouseKeepingPlugin.java

Modified: qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java?rev=946669&r1=946668&r2=946669&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java (original)
+++ qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionConfiguration.java Thu May 20 15:20:04 2010
@@ -22,6 +22,7 @@ package org.apache.qpid.server.configura
 
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.ConversionException;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory;
 
@@ -44,6 +45,9 @@ public class SlowConsumerDetectionConfig
         }
     }
 
+    //Set Default time unit to seconds
+    TimeUnit _timeUnit = TimeUnit.SECONDS;
+
     public String[] getElementsProcessed()
     {
         return new String[]{"delay",
@@ -55,9 +59,9 @@ public class SlowConsumerDetectionConfig
         return _configuration.getLong("delay", 10);
     }
 
-    public String getTimeUnit()
+    public TimeUnit getTimeUnit()
     {
-        return _configuration.getString("timeunit", TimeUnit.SECONDS.toString());
+        return  _timeUnit;
     }
 
     @Override
@@ -65,6 +69,50 @@ public class SlowConsumerDetectionConfig
     {
         super.setConfiguration(path, configuration);
 
+        //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");
+
+
+        if (timeUnit != null)
+        {
+            try
+            {
+                _timeUnit = TimeUnit.valueOf(timeUnit.toUpperCase());
+            }
+            catch (IllegalArgumentException iae)
+            {
+                throw new ConfigurationException("Unable to configure Slow Consumer Detection invalid TimeUnit:" + timeUnit);            
+            }
+        }
+
+
         System.out.println("Configured SCDC");
         System.out.println("Delay:" + getDelay());
         System.out.println("TimeUnit:" + getTimeUnit());

Modified: qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java?rev=946669&r1=946668&r2=946669&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java (original)
+++ qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/configuration/plugin/SlowConsumerDetectionQueueConfiguration.java Thu May 20 15:20:04 2010
@@ -29,10 +29,8 @@ import org.apache.qpid.server.registry.A
 import org.apache.qpid.slowconsumerdetection.policies.SlowConsumerPolicyPlugin;
 import org.apache.qpid.slowconsumerdetection.policies.SlowConsumerPolicyPluginFactory;
 
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.Set;
 
 public class SlowConsumerDetectionQueueConfiguration extends ConfigurationPlugin
 {
@@ -93,23 +91,22 @@ public class SlowConsumerDetectionQueueC
         Map<String, SlowConsumerPolicyPluginFactory> factories =
                 pluginManager.getPlugins(SlowConsumerPolicyPluginFactory.class);
 
-        Iterator<?> keys = policyConfig.getConfig().getKeys();
-
-        while (keys.hasNext())
-        {
-            String key = (String) keys.next();
-
-            _logger.debug("Policy Keys:" + key);
-
-        }
-
         if (policyConfig == null)
         {
             throw new ConfigurationException("No Slow Consumer Policy specified at:" + path + ". Known Policies:" + factories.keySet());
-        }        
+        }
 
         if (_logger.isDebugEnabled())
         {
+            Iterator<?> keys = policyConfig.getConfig().getKeys();
+
+            while (keys.hasNext())
+            {
+                String key = (String) keys.next();
+
+                _logger.debug("Policy Keys:" + key);
+            }
+
             _logger.debug("Configured SCDQC");
             _logger.debug("Age:" + getMessageAge());
             _logger.debug("Depth:" + getDepth());

Modified: qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java?rev=946669&r1=946668&r2=946669&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java (original)
+++ qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/main/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetection.java Thu May 20 15:20:04 2010
@@ -29,11 +29,12 @@ import org.apache.qpid.server.virtualhos
 import org.apache.qpid.server.virtualhost.plugins.VirtualHostPluginFactory;
 import org.apache.qpid.slowconsumerdetection.policies.SlowConsumerPolicyPlugin;
 
+import java.util.concurrent.TimeUnit;
+
 class SlowConsumerDetection extends VirtualHostHouseKeepingPlugin
 {
     Logger _logger = Logger.getLogger(SlowConsumerDetection.class);
     private SlowConsumerDetectionConfiguration _config;
-    private SlowConsumerPolicyPlugin _policy;
 
     public static class SlowConsumerFactory implements VirtualHostPluginFactory
     {
@@ -88,7 +89,7 @@ class SlowConsumerDetection extends Virt
         return _config.getDelay();
     }
 
-    public String getTimeUnit()
+    public TimeUnit getTimeUnit()
     {
         return _config.getTimeUnit();
     }

Added: qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java?rev=946669&view=auto
==============================================================================
--- qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java (added)
+++ qpid/trunk/qpid/java/broker-plugins/experimental/slowconsumerdetection/src/test/java/org/apache/qpid/server/virtualhost/plugin/SlowConsumerDetectionConfigurationTest.java Thu May 20 15:20:04 2010
@@ -0,0 +1,251 @@
+/*
+ *
+ * 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.virtualhost.plugin;
+
+import junit.framework.TestCase;
+import org.apache.commons.configuration.CompositeConfiguration;
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.XMLConfiguration;
+import org.apache.qpid.server.configuration.plugin.SlowConsumerDetectionConfiguration;
+
+import java.util.concurrent.TimeUnit;
+
+/** Provide Unit Test coverage of the SlowConsumerConfiguration */
+public class SlowConsumerDetectionConfigurationTest extends TestCase
+{
+
+    public void testConfigLoadingValidConfig()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("delay", "10");
+        xmlconfig.addProperty("timeunit", TimeUnit.MICROSECONDS.toString());
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+
+        try
+        {
+            config.setConfiguration("", composite);
+        }
+        catch (ConfigurationException e)
+        {
+            e.printStackTrace();
+            fail(e.getMessage());
+        }
+    }
+
+    /**
+     * TimeUnit parsing requires the String value be in UpperCase.
+     * Ensure we can handle when the user doesn't know this.
+     */
+    public void testConfigLoadingValidConfigStrangeTimeUnit()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("delay", "10");
+        xmlconfig.addProperty("timeunit", "MiCrOsEcOnDs");
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+
+        try
+        {
+            config.setConfiguration("", composite);
+        }
+        catch (ConfigurationException e)
+        {
+            e.printStackTrace();
+            fail(e.getMessage());
+        }
+    }
+
+    /** Test that delay must be long not a string value. */
+    public void testConfigLoadingInValidDelayString()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("delay", "ten");
+        xmlconfig.addProperty("timeunit", TimeUnit.MICROSECONDS.toString());
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+
+        try
+        {
+            config.setConfiguration("", composite);
+            fail("Configuration should fail to validate");
+        }
+        catch (ConfigurationException e)
+        {
+            Throwable cause = e.getCause();
+
+            assertEquals("Cause not correct", NumberFormatException.class, cause.getClass());
+        }
+    }
+
+    /** Test that negative delays are invalid */
+    public void testConfigLoadingInValidDelayNegative()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("delay", "-10");
+        xmlconfig.addProperty("timeunit", TimeUnit.MICROSECONDS.toString());
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+
+        try
+        {
+            config.setConfiguration("", composite);
+            fail("Configuration should fail to validate");
+        }
+        catch (ConfigurationException e)
+        {
+            Throwable cause = e.getCause();
+
+            assertNotNull("Configuration Exception must not be null.", cause);
+            assertEquals("Cause not correct",
+                         ConfigurationException.class, cause.getClass());
+            assertEquals("Incorrect message.",
+                         "Slow Consumer Detection Delay must be a Positive Long value.",
+                         cause.getMessage());
+        }
+    }
+
+    /** Tet that delay cannot be 0 */
+    public void testConfigLoadingInValidDelayZero()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("delay", "0");
+        xmlconfig.addProperty("timeunit", TimeUnit.MICROSECONDS.toString());
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+
+        try
+        {
+            config.setConfiguration("", composite);
+            fail("Configuration should fail to validate");
+        }
+        catch (ConfigurationException e)
+        {
+            Throwable cause = e.getCause();
+
+            assertNotNull("Configuration Exception must not be null.", cause);
+            assertEquals("Cause not correct",
+                         ConfigurationException.class, cause.getClass());
+            assertEquals("Incorrect message.",
+                         "Slow Consumer Detection Delay must be a Positive Long value.",
+                         cause.getMessage());
+        }
+    }
+
+    /** Test that missing delay fails */
+    public void testConfigLoadingInValidMissingDelay()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("timeunit", TimeUnit.SECONDS.toString());
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+        try
+        {
+            config.setConfiguration("", composite);
+            fail("Configuration should fail to validate");
+        }
+        catch (ConfigurationException e)
+        {
+            assertEquals("Incorrect message.", "Unable to configure Slow Consumer Detection invalid delay:null", e.getMessage());
+        }
+    }
+
+    /** Test that erroneous TimeUnit fails */
+    public void testConfigLoadingInValidTimeUnit()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        String TIMEUNIT = "foo";
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("delay", "10");
+        xmlconfig.addProperty("timeunit", TIMEUNIT);
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+        try
+        {
+            config.setConfiguration("", composite);
+            fail("Configuration should fail to validate");
+        }
+        catch (ConfigurationException e)
+        {
+            assertEquals("Incorrect message.", "Unable to configure Slow Consumer Detection invalid TimeUnit:" + TIMEUNIT, e.getMessage());
+        }
+    }
+
+    /** Test Missing TimeUnit value gets default */
+    public void testConfigLoadingMissingTimeUnitDefaults()
+    {
+        SlowConsumerDetectionConfiguration config = new SlowConsumerDetectionConfiguration();
+
+        XMLConfiguration xmlconfig = new XMLConfiguration();
+
+        xmlconfig.addProperty("delay", "10");
+
+        // Create a CompositeConfiguration as this is what the broker uses
+        CompositeConfiguration composite = new CompositeConfiguration();
+        composite.addConfiguration(xmlconfig);
+        try
+        {
+            config.setConfiguration("", composite);
+        }
+        catch (ConfigurationException e)
+        {
+            e.printStackTrace();
+            fail(e.getMessage());
+        }
+
+        assertEquals("Default TimeUnit incorrect", TimeUnit.SECONDS, config.getTimeUnit());
+    }
+
+}

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java?rev=946669&r1=946668&r2=946669&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java Thu May 20 15:20:04 2010
@@ -343,26 +343,9 @@ public class VirtualHostImpl implements 
                         VirtualHostHouseKeepingPlugin plugin =
                                 plugins.get(pluginName).newInstance(this);
 
-                        TimeUnit units = TimeUnit.MILLISECONDS;
-
-                        if (plugin.getTimeUnit() != null)
-                        {
-                            try
-                            {
-                                units = TimeUnit.valueOf(plugin.getTimeUnit());
-                            }
-                            catch (IllegalArgumentException iae)
-                            {
-                                _logger.warn("Plugin:" + pluginName +
-                                             " provided an illegal TimeUnit value:"
-                                             + plugin.getTimeUnit());
-                                // Warn and use default of millseconds
-                                // Should not occur in a well behaved plugin
-                            }
-                        }
 
                         _houseKeepingTasks.scheduleAtFixedRate(plugin, plugin.getDelay() / 2,
-                                                       plugin.getDelay(), units);
+                                                       plugin.getDelay(), plugin.getTimeUnit());
 
                         _logger.info("Loaded VirtualHostPlugin:" + plugin);
                     }

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/plugins/VirtualHostHouseKeepingPlugin.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/plugins/VirtualHostHouseKeepingPlugin.java?rev=946669&r1=946668&r2=946669&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/plugins/VirtualHostHouseKeepingPlugin.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/plugins/VirtualHostHouseKeepingPlugin.java Thu May 20 15:20:04 2010
@@ -23,6 +23,8 @@ package org.apache.qpid.server.virtualho
 import org.apache.qpid.server.virtualhost.HouseKeepingTask;
 import org.apache.qpid.server.virtualhost.VirtualHost;
 
+import java.util.concurrent.TimeUnit;
+
 public abstract class VirtualHostHouseKeepingPlugin extends HouseKeepingTask
 {
     public VirtualHostHouseKeepingPlugin(VirtualHost vhost)
@@ -44,6 +46,6 @@ public abstract class VirtualHostHouseKe
      *
      * @see java.util.concurrent.TimeUnit for valid value.
      */
-    public abstract String getTimeUnit();
+    public abstract TimeUnit getTimeUnit();
 
 }



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org