QPID-4390: Move virtual host configuration creation into virtual host adapter

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/java-broker-config-qpid-4390@1420874 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 53 insertions, 106 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
index 35ce66b0cb..9a33a57a54 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/VirtualHostConfiguration.java
@@ -23,11 +23,13 @@ package org.apache.qpid.server.configuration;
 import org.apache.commons.configuration.CompositeConfiguration;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.XMLConfiguration;
 
 import org.apache.qpid.server.configuration.plugins.AbstractConfiguration;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.store.MemoryMessageStore;
 
+import java.io.File;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -47,6 +49,35 @@ public class VirtualHostConfiguration extends AbstractConfiguration
         setConfiguration(config);
     }
 
+    public VirtualHostConfiguration(String name, File configurationFile, Broker broker) throws ConfigurationException
+    {
+        _name = name;
+        _broker = broker;
+        Configuration configuration = null;
+        if (configurationFile == null)
+        {
+            configuration = new XMLConfiguration();
+        }
+        else
+        {
+            Configuration virtualHostConfig = XmlConfigurationUtilities.parseConfig(configurationFile, null);
+
+            Configuration config = virtualHostConfig.subset("virtualhost." + XmlConfigurationUtilities.escapeTagName(name));
+            if (config.isEmpty())
+            {
+                // try to load virtual host configuration from 'name' element
+                config = virtualHostConfig.subset(XmlConfigurationUtilities.escapeTagName(name));
+                if (config.isEmpty())
+                {
+                    // fallback to the original configuration
+                    config = virtualHostConfig;
+                }
+            }
+            configuration = config;
+        }
+        setConfiguration(configuration);
+    }
+
     /**
      * Apply the given configuration to this VirtualHostConfiguration
      *
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/VirtualHostRecoverer.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/VirtualHostRecoverer.java
index ba5be4ab0b..1f4b44e94c 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/VirtualHostRecoverer.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/VirtualHostRecoverer.java
@@ -20,20 +20,9 @@
  */
 package org.apache.qpid.server.configuration.startup;
 
-import static org.apache.qpid.server.util.MapValueConverter.getStringAttribute;
-
-import java.io.File;
-import java.util.Map;
-
-import org.apache.commons.configuration.Configuration;
-import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.XMLConfiguration;
 import org.apache.qpid.server.configuration.ConfigurationEntry;
 import org.apache.qpid.server.configuration.ConfiguredObjectRecoverer;
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.configuration.RecovererProvider;
-import org.apache.qpid.server.configuration.VirtualHostConfiguration;
-import org.apache.qpid.server.configuration.XmlConfigurationUtilities;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.VirtualHost;
@@ -54,45 +43,7 @@ public class VirtualHostRecoverer implements ConfiguredObjectRecoverer<VirtualHo
     public VirtualHost create(RecovererProvider recovererProvider, ConfigurationEntry entry, ConfiguredObject... parents)
     {
         Broker broker = RecovererHelper.verifyOnlyBrokerIsParent(parents);
-        Map<String, Object> attributes = entry.getAttributes();
-        String name = getStringAttribute(VirtualHost.NAME, attributes);
-        String configuration = getStringAttribute(VirtualHost.CONFIGURATION, attributes, null);
-        Configuration conf = null;
-        if (configuration == null)
-        {
-            // TODO throw an exception
-            conf = new XMLConfiguration();
-        }
-        else
-        {
-            File configurationFile = new File(configuration);
-            if (!configurationFile.exists())
-            {
-                throw new IllegalConfigurationException("Configuration file '" + configurationFile + "' for virtual host '" + name + "' does not exist.");
-            }
-
-            try
-            {
-                Configuration virtualHostConfig = XmlConfigurationUtilities.parseConfig(configurationFile, null);
-                conf = virtualHostConfig.subset("virtualhost." + XmlConfigurationUtilities.escapeTagName(name));
-            }
-            catch (ConfigurationException e)
-            {
-                throw new IllegalConfigurationException("Cannot load configuration for virtual host '" + name + "' from file " + configurationFile);
-            }
-        }
-        // TODO: remove virtual host configuration
-        VirtualHostConfiguration virtualHostConfiguration = null;
-        try
-        {
-            virtualHostConfiguration = new VirtualHostConfiguration(name, conf, broker);
-        }
-        catch (ConfigurationException e)
-        {
-            throw new IllegalConfigurationException("Cannot create configuration for virtual host '" + name + "'");
-        }
-        return new VirtualHostAdapter(entry.getId(), broker, attributes, _brokerStatisticsGatherer, broker.getSecurityManager(),
-                virtualHostConfiguration);
+        return new VirtualHostAdapter(entry.getId(), entry.getAttributes(),broker, _brokerStatisticsGatherer);
     }
 
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
index 57cee88cb3..feb802d68b 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
@@ -213,8 +213,8 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
     private VirtualHost createVirtualHost(final Map<String, Object> attributes)
             throws AccessControlException, IllegalArgumentException
     {
-        final VirtualHostAdapter virtualHostAdapter = new VirtualHostAdapter(UUID.randomUUID(), this, attributes,
-                _statisticsGatherer, _securityManager, null);
+        final VirtualHostAdapter virtualHostAdapter = new VirtualHostAdapter(UUID.randomUUID(), attributes, this,
+                _statisticsGatherer);
 
         synchronized (_vhostAdapters)
         {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java
index 0d5bbc85f8..7d436d1d97 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/VirtualHostAdapter.java
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.model.adapter;
 
+import java.io.File;
 import java.security.AccessControlException;
 import java.security.Principal;
 import java.util.ArrayList;
@@ -84,30 +85,20 @@ public final class VirtualHostAdapter extends AbstractAdapter implements Virtual
 
     private final Map<org.apache.qpid.server.exchange.Exchange, ExchangeAdapter> _exchangeAdapters =
             new HashMap<org.apache.qpid.server.exchange.Exchange, ExchangeAdapter>();
-
     private StatisticsAdapter _statistics;
-
     private final Broker _broker;
-
     private final List<VirtualHostAlias> _aliases = new ArrayList<VirtualHostAlias>();
-
     private final String _name;
-
-    private VirtualHostConfiguration _configuration;
-
+    private final String _configurationFile;
     private StatisticsGatherer _brokerStatisticsGatherer;
 
-    private SecurityManager _securityManager;
-
-    public VirtualHostAdapter(UUID id, Broker broker, Map<String, Object> attributes,
-            StatisticsGatherer brokerStatisticsGatherer, SecurityManager securityManager, VirtualHostConfiguration configuration)
+    public VirtualHostAdapter(UUID id, Map<String, Object> attributes, Broker broker, StatisticsGatherer brokerStatisticsGatherer)
     {
         super(id);
         _broker = broker;
         _name = (String)attributes.get(NAME);
-        _configuration = configuration;
+        _configurationFile = (String)attributes.get(CONFIGURATION);
         _brokerStatisticsGatherer = brokerStatisticsGatherer;
-        _securityManager = securityManager;
         addParent(Broker.class, broker);
     }
 
@@ -902,11 +893,12 @@ public final class VirtualHostAdapter extends AbstractAdapter implements Virtual
             VirtualHostRegistry virtualHostRegistry = _broker.getVirtualHostRegistry();
             try
             {
-                _virtualHost = new VirtualHostImpl(virtualHostRegistry, _brokerStatisticsGatherer, _securityManager, _configuration);
+                VirtualHostConfiguration configuration = new VirtualHostConfiguration(_name, new File(_configurationFile) , _broker);
+                _virtualHost = new VirtualHostImpl(_broker.getVirtualHostRegistry(), _brokerStatisticsGatherer, _broker.getSecurityManager(), configuration);
             }
             catch (Exception e)
             {
-               throw new RuntimeException("Failed to create virtual host", e);
+               throw new RuntimeException("Failed to create virtual host " + _name, e);
             }
 
             virtualHostRegistry.registerVirtualHost(_virtualHost);
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java
index 2d069afe95..0346d76426 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/VirtualHostConfigurationTest.java
@@ -79,7 +79,7 @@ public class VirtualHostConfigurationTest extends QpidTestCase
 
     private VirtualHost createVirtualHost(String hostName) throws Exception
     {
-        Configuration config = getConfigXml().subset("virtualhosts.virtualhost." + VirtualHostConfiguration.escapeTagName(hostName));
+        Configuration config = getConfigXml().subset("virtualhosts.virtualhost." + XmlConfigurationUtilities.escapeTagName(hostName));
         VirtualHostConfiguration virtualHostConfiguration = new VirtualHostConfiguration(hostName, config, _broker);
         return BrokerTestHelper.createVirtualHost(virtualHostConfiguration, _virtualHostRegistry);
     }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java
index a43c539c8d..77dd91a34c 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java
@@ -21,15 +21,15 @@
 package org.apache.qpid.server.virtualhost;
 
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 import org.apache.commons.configuration.ConfigurationException;
 
-import org.apache.qpid.server.configuration.ConfigurationEntry;
-import org.apache.qpid.server.configuration.startup.VirtualHostRecoverer;
+import org.apache.qpid.server.configuration.VirtualHostConfiguration;
+
 import org.apache.qpid.server.exchange.Exchange;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.queue.AMQQueue;
+import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.stats.StatisticsGatherer;
 import org.apache.qpid.server.store.MemoryMessageStore;
 import org.apache.qpid.server.util.BrokerTestHelper;
@@ -39,9 +39,6 @@ import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.UUID;
 
 public class VirtualHostImplTest extends QpidTestCase
 {
@@ -100,15 +97,11 @@ public class VirtualHostImplTest extends QpidTestCase
         }
         catch (RuntimeException e)
         {
-            // PASS
-            assertEquals("Failed to create virtual host", e.getMessage());
-
             assertNotNull(e.getCause());
-            assertNotNull(e.getCause().getCause());
 
-            assertEquals(ConfigurationException.class, e.getCause().getCause().getClass());
+            assertEquals(ConfigurationException.class, e.getCause().getClass());
 
-            Throwable configException = e.getCause().getCause();
+            Throwable configException = e.getCause();
             assertEquals("Illegal attempt to bind queue '" + queueName + "' to the default exchange with a key other than the queue name: " + customBinding, configException.getMessage());
         }
     }
@@ -148,15 +141,11 @@ public class VirtualHostImplTest extends QpidTestCase
         }
         catch (RuntimeException e)
         {
-            // PASS
-            assertEquals("Failed to create virtual host", e.getMessage());
-
             assertNotNull(e.getCause());
-            assertNotNull(e.getCause().getCause());
 
-            assertEquals(ConfigurationException.class, e.getCause().getCause().getClass());
+            assertEquals(ConfigurationException.class, e.getCause().getClass());
 
-            Throwable configException = e.getCause().getCause();
+            Throwable configException = e.getCause();
             assertEquals("Attempt to bind queue '" + queueName + "' to unknown exchange:" + exchangeName, configException.getMessage());
         }
     }
@@ -189,30 +178,14 @@ public class VirtualHostImplTest extends QpidTestCase
 
     private VirtualHost createVirtualHost(String vhostName, File config) throws Exception
     {
-        recoverAndStartVirtualHost(vhostName, config);
-        return _virtualHostRegistry.getVirtualHost(vhostName);
-    }
-
-    private void recoverAndStartVirtualHost(String vhostName, File config)
-    {
-        // broker mock object with security manager
         Broker broker = BrokerTestHelper.createBrokerMock();
         _virtualHostRegistry = broker.getVirtualHostRegistry();
 
-        // configuration entry
-        Map<String, Object> attributes = new HashMap<String, Object>();
-        attributes.put(org.apache.qpid.server.model.VirtualHost.NAME, vhostName);
-        attributes.put(org.apache.qpid.server.model.VirtualHost.CONFIGURATION, config.getAbsoluteFile());
-        ConfigurationEntry entry = mock(ConfigurationEntry.class);
-        when(entry.getId()).thenReturn(UUID.randomUUID());
-        when(entry.getAttributes()).thenReturn(attributes);
-
-        // recovering
-        VirtualHostRecoverer recovever = new VirtualHostRecoverer(mock(StatisticsGatherer.class));
-        org.apache.qpid.server.model.VirtualHost model= recovever.create(null, entry, broker);
+        VirtualHostConfiguration configuration = new  VirtualHostConfiguration(vhostName, config, broker);
+        VirtualHost host = new VirtualHostImpl(_virtualHostRegistry, mock(StatisticsGatherer.class), new SecurityManager(null), configuration);
+        _virtualHostRegistry.registerVirtualHost(host);
 
-        // starting
-        model.setDesiredState(org.apache.qpid.server.model.State.INITIALISING, org.apache.qpid.server.model.State.ACTIVE);
+        return host;
     }
 
     /**