QPID-4335, QPID-4353: Refactored broker plugins to use simplified ServiceLoader-based model rather than embedding Felix to use OSGi.

Removed the ability to reload security configuration because this feature is not very useful in
its current form and was making our code hard to refactor.

Modified all tests to use jars rather than classes. This makes them closer to real-world deployments, e.g. the META-INF/services
file is read from within the jar.
Also moved various system tests from their respective modules into "systests". This removes the need for most modules to depend
on systests, thus simplifying our dependency graph.

Applied patch from myself, Keith Wall and Phil Harvey <phil@philharveyonline.com>

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

5 files changed, 48 insertions, 182 deletions

diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java
deleted file mode 100644
index 63f7e254ae..0000000000
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlConfiguration.java
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- *
- * 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.security.access.plugins;
-
-import java.io.File;
-import java.util.Arrays;
-import java.util.List;
-
-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
-{
-    public static final ConfigurationPluginFactory FACTORY = new ConfigurationPluginFactory()
-    {
-        public ConfigurationPlugin newInstance(String path, Configuration config) throws ConfigurationException
-        {
-            ConfigurationPlugin instance = new AccessControlConfiguration();
-            instance.setConfiguration(path, config);
-            return instance;
-        }
-
-        public List<String> getParentPaths()
-        {
-            return Arrays.asList("security.acl", "virtualhosts.virtualhost.security.acl");
-        }
-    };
-
-    private RuleSet _ruleSet;
-
-    public String[] getElementsProcessed()
-    {
-        return new String[] { "" };
-    }
-
-    public String getFileName()
-    {
-        return getConfig().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/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
index d94948237f..19b9135ea6 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControl.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControl.java
@@ -22,58 +22,45 @@ package org.apache.qpid.server.security.access.plugins;
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.io.File;
 
 import javax.security.auth.Subject;
 
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.lang.ObjectUtils;
 import org.apache.log4j.Logger;
-import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
-import org.apache.qpid.server.security.AbstractPlugin;
 import org.apache.qpid.server.security.Result;
 import org.apache.qpid.server.security.SecurityManager;
-import org.apache.qpid.server.security.SecurityPluginFactory;
+import org.apache.qpid.server.security.AccessControl;
 import org.apache.qpid.server.security.access.ObjectProperties;
 import org.apache.qpid.server.security.access.ObjectType;
 import org.apache.qpid.server.security.access.Operation;
+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;
 
-/**
- * This access control plugin implements version two plain text access control.
- */
-public class AccessControl extends AbstractPlugin
+public class DefaultAccessControl implements AccessControl
 {
-    private static final Logger _logger = Logger.getLogger(AccessControl.class);
+    private static final Logger _logger = Logger.getLogger(DefaultAccessControl.class);
 
     private RuleSet _ruleSet;
 
-    public static final SecurityPluginFactory<AccessControl> FACTORY = new SecurityPluginFactory<AccessControl>()
+    public DefaultAccessControl(String fileName) throws ConfigurationException
     {
-        public Class<AccessControl> getPluginClass()
-        {
-            return AccessControl.class;
-        }
-
-        public String getPluginName()
+        if (_logger.isDebugEnabled())
         {
-            return AccessControl.class.getName();
+            _logger.debug("Creating AccessControl instance using file: " + fileName);
         }
+        File aclFile = new File(fileName);
 
-        public AccessControl newInstance(ConfigurationPlugin config) throws ConfigurationException
-        {
-            AccessControlConfiguration configuration = config.getConfiguration(AccessControlConfiguration.class.getName());
-
-            // If there is no configuration for this plugin then don't load it.
-            if (configuration == null)
-            {
-                return null;
-            }
+        ConfigurationFile configFile = new PlainConfiguration(aclFile);
+        _ruleSet = configFile.load();
+    }
 
-            AccessControl plugin = new AccessControl();
-            plugin.configure(configuration);
-            return plugin;
-        }
-    };
+    DefaultAccessControl(RuleSet rs) throws ConfigurationException
+    {
+        _ruleSet = rs;
+    }
 
     public Result getDefault()
     {
@@ -132,13 +119,4 @@ public class AccessControl extends AbstractPlugin
             return Result.DENIED;
         }
     }
-
-    public void configure(ConfigurationPlugin config)
-    {
-        super.configure(config);
-
-        AccessControlConfiguration accessConfig = (AccessControlConfiguration) getConfig();
-
-        _ruleSet = accessConfig.getRuleSet();
-    }
 }
diff --git a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlActivator.java b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactory.java
index 7c83446cf1..38ea61357e 100644
--- a/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/AccessControlActivator.java
+++ b/qpid/java/broker-plugins/access-control/src/main/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlFactory.java
@@ -1,5 +1,5 @@
 /*
- * 
+ *
  * 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
@@ -7,35 +7,41 @@
  * 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.security.access.plugins;
 
-import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory;
-import org.apache.qpid.server.security.SecurityPluginActivator;
-import org.apache.qpid.server.security.SecurityPluginFactory;
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.qpid.server.plugin.AccessControlFactory;
+import org.apache.qpid.server.security.AccessControl;
 
-/**
- * The OSGi {@link org.osgi.framework.BundleActivator} for {@link AccessControl}.
- */
-public class AccessControlActivator extends SecurityPluginActivator
+public class DefaultAccessControlFactory implements AccessControlFactory
 {
-	public SecurityPluginFactory getFactory()
-	{
-	    return AccessControl.FACTORY;
-	}
-
-    public ConfigurationPluginFactory getConfigurationFactory()
+    public AccessControl createInstance(Configuration securityConfiguration)
     {
-        return AccessControlConfiguration.FACTORY;
+        String aclConfiguration = securityConfiguration.getString("acl");
+        if(aclConfiguration == null)
+        {
+            return null;
+        }
+
+        try
+        {
+            return new DefaultAccessControl(aclConfiguration);
+        }
+        catch (ConfigurationException e)
+        {
+            throw new RuntimeException("caught exception during instance creation", e);
+        }
     }
 }
diff --git a/qpid/java/broker-plugins/access-control/src/main/resources/META-INF/services/org.apache.qpid.server.plugin.AccessControlFactory b/qpid/java/broker-plugins/access-control/src/main/resources/META-INF/services/org.apache.qpid.server.plugin.AccessControlFactory
new file mode 100644
index 0000000000..85b942383f
--- /dev/null
+++ b/qpid/java/broker-plugins/access-control/src/main/resources/META-INF/services/org.apache.qpid.server.plugin.AccessControlFactory
@@ -0,0 +1 @@
+org.apache.qpid.server.security.access.plugins.DefaultAccessControlFactory
diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
index a65c442bcf..a8406308c0 100644
--- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/AccessControlTest.java
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
@@ -30,7 +30,6 @@ import javax.security.auth.Subject;
 import junit.framework.TestCase;
 
 import org.apache.commons.configuration.ConfigurationException;
-import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
 import org.apache.qpid.server.logging.UnitTestMessageLogger;
 import org.apache.qpid.server.logging.actors.CurrentActor;
 import org.apache.qpid.server.logging.actors.TestLogActor;
@@ -45,20 +44,16 @@ import org.apache.qpid.server.security.access.config.RuleSet;
 import org.apache.qpid.server.security.auth.TestPrincipalUtils;
 
 /**
- * Unit test for ACL V2 plugin.
- *
- * This unit test tests the AccessControl class and it collaboration with {@link RuleSet},
- * {@link SecurityManager} and {@link CurrentActor}.   The ruleset is configured programmatically,
- * rather than from an external file.
+ * In these tests, the ruleset is configured programmatically rather than from an external file.
  *
  * @see RuleSetTest
  */
-public class AccessControlTest extends TestCase
+public class DefaultAccessControlTest extends TestCase
 {
     private static final String ALLOWED_GROUP = "allowed_group";
     private static final String DENIED_GROUP = "denied_group";
 
-    private AccessControl _plugin = null;  // Class under test
+    private DefaultAccessControl _plugin = null;  // Class under test
     private final UnitTestMessageLogger messageLogger = new UnitTestMessageLogger();
 
     private void setUpGroupAccessControl() throws ConfigurationException
@@ -68,7 +63,7 @@ public class AccessControlTest extends TestCase
 
     private void configureAccessControl(final RuleSet rs) throws ConfigurationException
     {
-        _plugin = (AccessControl) AccessControl.FACTORY.newInstance(createConfiguration(rs));
+        _plugin = new DefaultAccessControl(rs);
         SecurityManager.setThreadSubject(null);
         CurrentActor.set(new TestLogActor(messageLogger));
     }
@@ -207,12 +202,11 @@ public class AccessControlTest extends TestCase
         SecurityManager.setThreadSubject(subject);
 
         RuleSet mockRuleSet = mock(RuleSet.class);
-        ConfigurationPlugin accessControlConfiguration = createConfiguration(mockRuleSet);
 
         InetAddress inetAddress = InetAddress.getLocalHost();
         InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 1);
 
-        AccessControl accessControl = AccessControl.FACTORY.newInstance(accessControlConfiguration);
+        DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet);
 
         accessControl.access(ObjectType.VIRTUALHOST, inetSocketAddress);
 
@@ -235,9 +229,7 @@ public class AccessControlTest extends TestCase
                 ObjectProperties.EMPTY,
                 inetAddress)).thenThrow(new RuntimeException());
 
-        ConfigurationPlugin accessControlConfiguration = createConfiguration(mockRuleSet);
-
-        AccessControl accessControl = AccessControl.FACTORY.newInstance(accessControlConfiguration);
+        DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet);
         Result result = accessControl.access(ObjectType.VIRTUALHOST, inetSocketAddress);
 
         assertEquals(Result.DENIED, result);
@@ -373,32 +365,4 @@ public class AccessControlTest extends TestCase
         Result result = _plugin.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
         assertEquals(expectedResult, result);
     }
-
-    /**
-     * Creates a configuration plugin for the {@link AccessControl} plugin.
-     */
-    private ConfigurationPlugin createConfiguration(final RuleSet rs)
-    {
-        final ConfigurationPlugin cp = new ConfigurationPlugin()
-        {
-            @SuppressWarnings("unchecked")
-            public AccessControlConfiguration getConfiguration(final String plugin)
-            {
-                return new AccessControlConfiguration()
-                {
-                    public RuleSet getRuleSet()
-                    {
-                        return rs;
-                    }
-                };
-            }
-
-            public String[] getElementsProcessed()
-            {
-                throw new UnsupportedOperationException();
-            }
-        };
-
-        return cp;
-    }
 }