QPID-2720: make the PluginManager able to work with a pre-existing BundleContext

Manually applied patch from Danushka Menikkumbura (no longer applied after intervening commits), then updated PluginManager close() to always close the ServiceTrackers it creates, removed the static field/methods causing test failures which lead to previously reverting the patch, enabled passing a pre existing BundleContext into the Broker instance at startup instead, and finally removed the unused MockPluginManager.


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

8 files changed, 125 insertions, 137 deletions

diff --git a/qpid/java/broker-plugins/extras/src/test/java/org/apache/qpid/server/plugins/ExtrasTest.java b/qpid/java/broker-plugins/extras/src/test/java/org/apache/qpid/server/plugins/ExtrasTest.java
index 57b6e19b5d..db3ebfd4e1 100644
--- a/qpid/java/broker-plugins/extras/src/test/java/org/apache/qpid/server/plugins/ExtrasTest.java
+++ b/qpid/java/broker-plugins/extras/src/test/java/org/apache/qpid/server/plugins/ExtrasTest.java
@@ -67,7 +67,7 @@ public class ExtrasTest extends TestCase
     
     public void testNoExchanges() throws Exception
     {
-        PluginManager manager = new PluginManager("/path/to/nowhere", "/tmp");
+        PluginManager manager = new PluginManager("/path/to/nowhere", "/tmp", null);
         Map<String, ExchangeType<?>> exchanges = manager.getExchanges();
         assertTrue("Exchanges found", exchanges.isEmpty());
     }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java
index 3729c9a1ed..a4e365005e 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java
@@ -109,7 +109,7 @@ public class Broker
 
         configureLogging(logConfigFile, options.getLogWatchFrequency());
 
-        ConfigurationFileApplicationRegistry config = new ConfigurationFileApplicationRegistry(configFile);
+        ConfigurationFileApplicationRegistry config = new ConfigurationFileApplicationRegistry(configFile, options.getBundleContext());
         ServerConfiguration serverConfig = config.getConfiguration();
         updateManagementPort(serverConfig, options.getJmxPort());
 
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java
index b83da92660..b2d029eb95 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/BrokerOptions.java
@@ -26,6 +26,8 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import org.osgi.framework.BundleContext;
+
 public class BrokerOptions
 {
     /** serialVersionUID */
@@ -51,9 +53,11 @@ public class BrokerOptions
     private String _logConfigFile;
     private String _bind;
     private Integer _jmxPort;
+    private BundleContext _bundleContext;
 
     private Integer _logWatchFrequency = 0;
 
+
     public void addPort(final int port)
     {
         _ports.add(port);
@@ -149,4 +153,14 @@ public class BrokerOptions
     {
         _logWatchFrequency = logWatchFrequency;
     }
+
+    public BundleContext getBundleContext()
+    {
+        return _bundleContext ;
+    }
+
+    public void setBundleContext(final BundleContext bundleContext)
+    {
+        _bundleContext = bundleContext;
+    }
 }
\ No newline at end of file
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java
index bb3e4a61d1..43c4fa26b7 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/plugins/PluginManager.java
@@ -65,6 +65,7 @@ import org.apache.qpid.server.virtualhost.plugins.policies.TopicDeletePolicy;
 import org.apache.qpid.slowconsumerdetection.policies.SlowConsumerPolicyPluginFactory;
 import org.apache.qpid.util.FileUtils;
 import org.osgi.framework.BundleActivator;
+import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Version;
 import org.osgi.framework.launch.Framework;
@@ -140,7 +141,7 @@ public class PluginManager implements Closeable
     }
     
     
-    public PluginManager(String pluginPath, String cachePath) throws Exception
+    public PluginManager(String pluginPath, String cachePath, BundleContext bundleContext) throws Exception
     {
         // Store all non-OSGi plugins
         // A little gross that we have to add them here, but not all the plugins are OSGIfied
@@ -179,88 +180,97 @@ public class PluginManager implements Closeable
             _authenticationManagerPlugins.put(pluginFactory.getPluginName(), pluginFactory);
         }
 
-        // Check the plugin directory path is set and exist
-        if (pluginPath == null)
+        if(bundleContext == null)
         {
-            _logger.info("No plugin path specified, no plugins will be loaded.");
-            return;
-        }
-        File pluginDir = new File(pluginPath);
-        if (!pluginDir.exists())
-        {
-            _logger.warn("Plugin dir : "  + pluginDir + " does not exist.");
-            return;
-        } 
+            // Check the plugin directory path is set and exist
+            if (pluginPath == null)
+            {
+                _logger.info("No plugin path specified, no plugins will be loaded.");
+                return;
+            }
+            File pluginDir = new File(pluginPath);
+            if (!pluginDir.exists())
+            {
+                _logger.warn("Plugin dir : "  + pluginDir + " does not exist.");
+                return;
+            }
 
-        // Add the bundle provided service interface package and the core OSGi
-        // packages to be exported from the class path via the system bundle.
-        
-        // Setup OSGi configuration property map
-        final StringMap configMap = new StringMap(false);
-        configMap.put(FRAMEWORK_SYSTEMPACKAGES, OSGI_SYSTEM_PACKAGES);
+            // Add the bundle provided service interface package and the core OSGi
+            // packages to be exported from the class path via the system bundle.
 
-        // No automatic shutdown hook
-        configMap.put("felix.shutdown.hook", "false");
-        
-        // Add system activator
-        List<BundleActivator> activators = new ArrayList<BundleActivator>();
-        _activator = new Activator();
-        activators.add(_activator);
-        configMap.put(SYSTEMBUNDLE_ACTIVATORS_PROP, activators);
+            // Setup OSGi configuration property map
+            final StringMap configMap = new StringMap(false);
+            configMap.put(FRAMEWORK_SYSTEMPACKAGES, OSGI_SYSTEM_PACKAGES);
 
-        if (cachePath != null)
-        {
-            File cacheDir = new File(cachePath);
-            if (!cacheDir.exists() && cacheDir.canWrite())
+            // No automatic shutdown hook
+            configMap.put("felix.shutdown.hook", "false");
+
+            // Add system activator
+            List<BundleActivator> activators = new ArrayList<BundleActivator>();
+            _activator = new Activator();
+            activators.add(_activator);
+            configMap.put(SYSTEMBUNDLE_ACTIVATORS_PROP, activators);
+
+            if (cachePath != null)
             {
-                _logger.info("Creating plugin cache directory: " + cachePath);
-                cacheDir.mkdir();
+                File cacheDir = new File(cachePath);
+                if (!cacheDir.exists() && cacheDir.canWrite())
+                {
+                    _logger.info("Creating plugin cache directory: " + cachePath);
+                    cacheDir.mkdir();
+                }
+
+                // Set plugin cache directory and empty it
+                _logger.info("Cache bundles in directory " + cachePath);
+                configMap.put(FRAMEWORK_STORAGE, cachePath);
             }
-            
-            // Set plugin cache directory and empty it
-            _logger.info("Cache bundles in directory " + cachePath);
-            configMap.put(FRAMEWORK_STORAGE, cachePath);
-        }
-        configMap.put(FRAMEWORK_STORAGE_CLEAN, FRAMEWORK_STORAGE_CLEAN_ONFIRSTINIT);
-        
-        // Set directory with plugins to auto-deploy
-        _logger.info("Auto deploying bundles from directory " + pluginPath);
-        configMap.put(AUTO_DEPLOY_DIR_PROPERY, pluginPath);
-        configMap.put(AUTO_DEPLOY_ACTION_PROPERY, AUTO_DEPLOY_INSTALL_VALUE + "," + AUTO_DEPLOY_START_VALUE);        
-        
-        // Start plugin manager and trackers
-        _felix = new Felix(configMap);
-        try
-        {
-            _logger.info("Starting plugin manager...");
-            _felix.init();
-	        process(configMap, _felix.getBundleContext());
-            _felix.start();
-            _logger.info("Started plugin manager");
+            configMap.put(FRAMEWORK_STORAGE_CLEAN, FRAMEWORK_STORAGE_CLEAN_ONFIRSTINIT);
+
+            // Set directory with plugins to auto-deploy
+            _logger.info("Auto deploying bundles from directory " + pluginPath);
+            configMap.put(AUTO_DEPLOY_DIR_PROPERY, pluginPath);
+            configMap.put(AUTO_DEPLOY_ACTION_PROPERY, AUTO_DEPLOY_INSTALL_VALUE + "," + AUTO_DEPLOY_START_VALUE);
+
+            // Start plugin manager
+            _felix = new Felix(configMap);
+            try
+            {
+                _logger.info("Starting plugin manager framework");
+                _felix.init();
+                process(configMap, _felix.getBundleContext());
+                _felix.start();
+                _logger.info("Started plugin manager framework");
+            }
+            catch (BundleException e)
+            {
+                throw new ConfigurationException("Could not start plugin manager: " + e.getMessage(), e);
+            }
+
+            bundleContext = _activator.getContext();
         }
-        catch (BundleException e)
+        else
         {
-            throw new ConfigurationException("Could not start plugin manager: " + e.getMessage(), e);
+            _logger.info("Using the specified external BundleContext");
         }
-        
+
         // TODO save trackers in a map, keyed by class name
         
-        _exchangeTracker = new ServiceTracker(_activator.getContext(), ExchangeType.class.getName(), null);
+        _exchangeTracker = new ServiceTracker(bundleContext, ExchangeType.class.getName(), null);
         _exchangeTracker.open();
 
-        _securityTracker = new ServiceTracker(_activator.getContext(), SecurityPluginFactory.class.getName(), null);
+        _securityTracker = new ServiceTracker(bundleContext, SecurityPluginFactory.class.getName(), null);
         _securityTracker.open();
 
-        _configTracker = new ServiceTracker(_activator.getContext(), ConfigurationPluginFactory.class.getName(), null);
+        _configTracker = new ServiceTracker(bundleContext, ConfigurationPluginFactory.class.getName(), null);
         _configTracker.open();
 
-        _virtualHostTracker = new ServiceTracker(_activator.getContext(), VirtualHostPluginFactory.class.getName(), null);
+        _virtualHostTracker = new ServiceTracker(bundleContext, VirtualHostPluginFactory.class.getName(), null);
         _virtualHostTracker.open();
  
-        _policyTracker = new ServiceTracker(_activator.getContext(), SlowConsumerPolicyPluginFactory.class.getName(), null);
+        _policyTracker = new ServiceTracker(bundleContext, SlowConsumerPolicyPluginFactory.class.getName(), null);
         _policyTracker.open();
         
-        _authenticationManagerTracker = new ServiceTracker(_activator.getContext(), AuthenticationManagerPluginFactory.class.getName(), null);
+        _authenticationManagerTracker = new ServiceTracker(bundleContext, AuthenticationManagerPluginFactory.class.getName(), null);
         _authenticationManagerTracker.open();
 
         _logger.info("Opened service trackers");
@@ -340,21 +350,21 @@ public class PluginManager implements Closeable
 
     public void close()
     {
-        if (_felix != null)
+        try
         {
-            try
-            {
-                // Close all bundle trackers
-                _exchangeTracker.close();
-                _securityTracker.close();
-                _configTracker.close();
-                _virtualHostTracker.close();
-                _policyTracker.close();
-                _authenticationManagerTracker.close();
-            }
-            finally
+            // Close all bundle trackers
+            _exchangeTracker.close();
+            _securityTracker.close();
+            _configTracker.close();
+            _virtualHostTracker.close();
+            _policyTracker.close();
+            _authenticationManagerTracker.close();
+        }
+        finally
+        {
+            if (_felix != null)
             {
-                _logger.info("Stopping plugin manager");
+                _logger.info("Stopping plugin manager framework");
                 try
                 {
                     // FIXME should be stopAndWait() but hangs VM, need upgrade in felix
@@ -373,7 +383,12 @@ public class PluginManager implements Closeable
                 {
                     // Ignore
                 }
-                _logger.info("Stopped plugin manager");
+                _logger.info("Stopped plugin manager framework");
+            }
+            else
+            {
+                _logger.info("Plugin manager was started with an external BundleContext, " +
+                             "skipping remaining shutdown tasks");
             }
         }
     }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
index bfdb30764a..c07074f69c 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
@@ -65,6 +65,7 @@ import org.apache.qpid.server.transport.QpidAcceptor;
 import org.apache.qpid.server.virtualhost.VirtualHost;
 import org.apache.qpid.server.virtualhost.VirtualHostImpl;
 import org.apache.qpid.server.virtualhost.VirtualHostRegistry;
+import org.osgi.framework.BundleContext;
 
 
 /**
@@ -111,6 +112,8 @@ public abstract class ApplicationRegistry implements IApplicationRegistry
     private boolean _statisticsEnabled = false;
     private StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
 
+    private BundleContext _bundleContext;
+
     static
     {
         Runtime.getRuntime().addShutdownHook(new Thread(new ShutdownService()));
@@ -209,7 +212,13 @@ public abstract class ApplicationRegistry implements IApplicationRegistry
 
     protected ApplicationRegistry(ServerConfiguration configuration)
     {
+        this(configuration, null);
+    }
+
+    protected ApplicationRegistry(ServerConfiguration configuration, BundleContext bundleContext)
+    {
         _configuration = configuration;
+        _bundleContext = bundleContext;
     }
 
     public void configure() throws ConfigurationException
@@ -218,7 +227,7 @@ public abstract class ApplicationRegistry implements IApplicationRegistry
 
         try
         {
-            _pluginManager = new PluginManager(_configuration.getPluginDirectory(), _configuration.getCacheDirectory());
+            _pluginManager = new PluginManager(_configuration.getPluginDirectory(), _configuration.getCacheDirectory(), _bundleContext);
         }
         catch (Exception e)
         {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java
index ff2a8c959b..9121f8f927 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ConfigurationFileApplicationRegistry.java
@@ -29,12 +29,18 @@ import org.apache.qpid.server.logging.actors.BrokerActor;
 import org.apache.qpid.server.logging.actors.CurrentActor;
 import org.apache.qpid.server.management.JMXManagedObjectRegistry;
 import org.apache.qpid.server.management.NoopManagedObjectRegistry;
+import org.osgi.framework.BundleContext;
 
 public class ConfigurationFileApplicationRegistry extends ApplicationRegistry
 {
     public ConfigurationFileApplicationRegistry(File configurationURL) throws ConfigurationException
     {
-        super(new ServerConfiguration(configurationURL));
+        this(configurationURL, null);
+    }
+
+    public ConfigurationFileApplicationRegistry(File configurationURL, BundleContext bundleContext) throws ConfigurationException
+    {
+        super(new ServerConfiguration(configurationURL), bundleContext);
     }
 
     @Override
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/plugins/MockPluginManager.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/plugins/MockPluginManager.java
deleted file mode 100644
index a64ec5d3b1..0000000000
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/plugins/MockPluginManager.java
+++ /dev/null
@@ -1,56 +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.plugins;
-
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import org.apache.qpid.server.configuration.plugins.ConfigurationPluginFactory;
-import org.apache.qpid.server.exchange.ExchangeType;
-import org.apache.qpid.server.security.SecurityPluginFactory;
-
-public class MockPluginManager extends PluginManager
-{
-    private Map<String, SecurityPluginFactory> _securityPlugins = new HashMap<String, SecurityPluginFactory>();
-    private Map<List<String>, ConfigurationPluginFactory> _configPlugins = new HashMap<List<String>, ConfigurationPluginFactory>();
-
-    public MockPluginManager(String pluginPath, String cachePath) throws Exception
-    {
-        super(pluginPath, cachePath);
-    }
-
-    @Override
-    public Map<String, ExchangeType<?>> getExchanges()
-    {
-       return null;
-    }
-
-    @Override
-    public Map<String, SecurityPluginFactory> getSecurityPlugins()
-    {
-        return _securityPlugins;
-    }
-
-    @Override
-    public Map<List<String>, ConfigurationPluginFactory> getConfigurationPlugins()
-    {
-        return _configPlugins;
-    }
-}
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/plugins/PluginTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/plugins/PluginTest.java
index 8c18ab85b0..8c945aabfb 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/plugins/PluginTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/plugins/PluginTest.java
@@ -49,7 +49,7 @@ public class PluginTest extends InternalBrokerBaseCase
     
     public void testNoExchanges() throws Exception
     {
-        PluginManager manager = new PluginManager("/path/to/nowhere", "/tmp");
+        PluginManager manager = new PluginManager("/path/to/nowhere", "/tmp", null);
         Map<String, ExchangeType<?>> exchanges = manager.getExchanges();
         assertTrue("Exchanges found", exchanges.isEmpty());
     }