diff options
4 files changed, 248 insertions, 35 deletions
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 7c804fc1fd..bfdb30764a 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 @@ -568,7 +568,7 @@ public abstract class ApplicationRegistry implements IApplicationRegistry public VirtualHost createVirtualHost(final VirtualHostConfiguration vhostConfig) throws Exception { - VirtualHostImpl virtualHost = new VirtualHostImpl(this, vhostConfig); + VirtualHostImpl virtualHost = new VirtualHostImpl(this, vhostConfig, null); _virtualHostRegistry.registerVirtualHost(virtualHost); getBroker().addVirtualHost(virtualHost); return virtualHost; diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java index 52acd9085b..5f3446236c 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java @@ -20,7 +20,6 @@ */ package org.apache.qpid.server.virtualhost; -import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -178,22 +177,11 @@ public class VirtualHostImpl implements VirtualHost } } - public VirtualHostImpl(IApplicationRegistry appRegistry, VirtualHostConfiguration hostConfig) throws Exception - { - this(appRegistry, hostConfig, null); - } - - - public VirtualHostImpl(VirtualHostConfiguration hostConfig, MessageStore store) throws Exception - { - this(ApplicationRegistry.getInstance(),hostConfig,store); - } - - private VirtualHostImpl(IApplicationRegistry appRegistry, VirtualHostConfiguration hostConfig, MessageStore store) throws Exception + public VirtualHostImpl(IApplicationRegistry appRegistry, VirtualHostConfiguration hostConfig, MessageStore store) throws Exception { if (hostConfig == null) { - throw new IllegalAccessException("HostConfig and MessageStore cannot be null"); + throw new IllegalArgumentException("HostConfig cannot be null"); } _appRegistry = appRegistry; @@ -462,46 +450,57 @@ public class VirtualHostImpl implements VirtualHost private void configureQueue(QueueConfiguration queueConfiguration) throws AMQException, ConfigurationException { AMQQueue queue = AMQQueueFactory.createAMQQueueImpl(queueConfiguration, this); + String queueName = queue.getName(); if (queue.isDurable()) { getDurableConfigurationStore().createQueue(queue); } + //get the exchange name (returns default exchange name if none was specified) String exchangeName = queueConfiguration.getExchange(); - Exchange exchange = _exchangeRegistry.getExchange(exchangeName == null ? null : new AMQShortString(exchangeName)); - - if (exchange == null) - { - exchange = _exchangeRegistry.getDefaultExchange(); - } - + Exchange exchange = _exchangeRegistry.getExchange(exchangeName); if (exchange == null) { - throw new ConfigurationException("Attempt to bind queue to unknown exchange:" + exchangeName); + throw new ConfigurationException("Attempt to bind queue '" + queueName + "' to unknown exchange:" + exchangeName); } - List routingKeys = queueConfiguration.getRoutingKeys(); - if (routingKeys == null || routingKeys.isEmpty()) - { - routingKeys = Collections.singletonList(queue.getNameShortString()); - } + Exchange defaultExchange = _exchangeRegistry.getDefaultExchange(); + + //get routing keys in configuration (returns empty list if none are defined) + List<?> routingKeys = queueConfiguration.getRoutingKeys(); for (Object routingKeyNameObj : routingKeys) { - AMQShortString routingKey = new AMQShortString(String.valueOf(routingKeyNameObj)); - if (_logger.isInfoEnabled()) + String routingKey = String.valueOf(routingKeyNameObj); + + if (exchange.equals(defaultExchange) && !queueName.equals(routingKey)) { - _logger.info("Binding queue:" + queue + " with routing key '" + routingKey + "' to exchange:" + this); + throw new ConfigurationException("Illegal attempt to bind queue '" + queueName + + "' to the default exchange with a key other than the queue name: " + routingKey); } - _bindingFactory.addBinding(routingKey.toString(), queue, exchange, null); + + configureBinding(queue, exchange, routingKey); } - if (exchange != _exchangeRegistry.getDefaultExchange()) + if (!exchange.equals(defaultExchange)) + { + //bind the queue to the named exchange using its name + configureBinding(queue, exchange, queueName); + } + + //ensure the queue is bound to the default exchange using its name + configureBinding(queue, defaultExchange, queueName); + } + + private void configureBinding(AMQQueue queue, Exchange exchange, String routingKey) throws AMQException + { + if (_logger.isInfoEnabled()) { - _bindingFactory.addBinding(queue.getNameShortString().toString(), queue, exchange, null); + _logger.info("Binding queue:" + queue + " with routing key '" + routingKey + "' to exchange:" + exchange.getName()); } + _bindingFactory.addBinding(routingKey, queue, exchange, null); } public String getName() diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java index 0e136c523f..f4cdbbe02c 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java @@ -107,7 +107,7 @@ public class SimpleAMQQueueTest extends InternalBrokerBaseCase ApplicationRegistry applicationRegistry = (ApplicationRegistry)ApplicationRegistry.getInstance(); PropertiesConfiguration env = new PropertiesConfiguration(); - _virtualHost = new VirtualHostImpl(new VirtualHostConfiguration(getClass().getName(), env), _store); + _virtualHost = new VirtualHostImpl(ApplicationRegistry.getInstance(), new VirtualHostConfiguration(getClass().getName(), env), _store); applicationRegistry.getVirtualHostRegistry().registerVirtualHost(_virtualHost); _queue = (SimpleAMQQueue) AMQQueueFactory.createAMQQueueImpl(_qname, false, _owner, false, false, _virtualHost, _arguments); 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 new file mode 100644 index 0000000000..c87e5a1648 --- /dev/null +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java @@ -0,0 +1,214 @@ +/* + * + * 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; + +import java.io.BufferedWriter; +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; + +import org.apache.commons.configuration.ConfigurationException; +import org.apache.commons.configuration.XMLConfiguration; +import org.apache.qpid.server.configuration.ServerConfiguration; +import org.apache.qpid.server.exchange.Exchange; +import org.apache.qpid.server.queue.AMQQueue; +import org.apache.qpid.server.registry.ApplicationRegistry; +import org.apache.qpid.server.store.TestableMemoryMessageStore; +import org.apache.qpid.server.util.TestApplicationRegistry; +import org.apache.qpid.test.utils.QpidTestCase; + +public class VirtualHostImplTest extends QpidTestCase +{ + private ServerConfiguration _configuration; + private ApplicationRegistry _registry; + + @Override + public void tearDown() throws Exception + { + super.tearDown(); + + ApplicationRegistry.remove(); + } + + /** + * Tests that custom routing keys for the queue specified in the configuration + * file are correctly bound to the exchange (in addition to the queue name) + */ + public void testSpecifyingCustomBindings() throws Exception + { + customBindingTestImpl(new String[]{"custom1","custom2"}); + } + + /** + * Tests that a queue specified in the configuration file to be bound to a + * specified(non-default) direct exchange is a correctly bound to the exchange + * and the default exchange using the queue name. + */ + public void testQueueSpecifiedInConfigurationIsBoundToDefaultExchange() throws Exception + { + customBindingTestImpl(new String[0]); + } + + private void customBindingTestImpl(final String[] routingKeys) throws Exception + { + String exchangeName = getName() +".direct"; + String vhostName = getName(); + String queueName = getName(); + + File config = writeConfigFile(vhostName, queueName, exchangeName, false, routingKeys); + VirtualHost vhost = createVirtualHost(vhostName, config); + assertNotNull("virtualhost should exist", vhost); + + AMQQueue queue = vhost.getQueueRegistry().getQueue(queueName); + assertNotNull("queue should exist", queue); + + Exchange defaultExch = vhost.getExchangeRegistry().getDefaultExchange(); + assertTrue("queue should have been bound to default exchange with its name", defaultExch.isBound(queueName, queue)); + + Exchange exch = vhost.getExchangeRegistry().getExchange(exchangeName); + assertTrue("queue should have been bound to " + exchangeName + " with its name", exch.isBound(queueName, queue)); + + for(String key: routingKeys) + { + assertTrue("queue should have been bound to " + exchangeName + " with key " + key, exch.isBound(key, queue)); + } + } + + /** + * Tests that specifying custom routing keys for a queue in the configuration file results in failure + * to create the vhost (since this is illegal, only queue names are used with the default exchange) + */ + public void testSpecifyingCustomBindingForDefaultExchangeThrowsException() throws Exception + { + File config = writeConfigFile(getName(), getName(), null, false, new String[]{"custom-binding"}); + + try + { + createVirtualHost(getName(), config); + fail("virtualhost creation should have failed due to illegal configuration"); + } + catch (ConfigurationException e) + { + //expected + } + } + + /** + * Tests that specifying an unknown exchange to bind the queue to results in failure to create the vhost + */ + public void testSpecifyingUnknownExchangeThrowsException() throws Exception + { + File config = writeConfigFile(getName(), getName(), "made-up-exchange", true, new String[0]); + + try + { + createVirtualHost(getName(), config); + fail("virtualhost creation should have failed due to illegal configuration"); + } + catch (ConfigurationException e) + { + //expected + } + } + + private VirtualHost createVirtualHost(String vhostName, File config) throws Exception + { + _configuration = new ServerConfiguration(new XMLConfiguration(config)); + + _registry = new TestApplicationRegistry(_configuration); + ApplicationRegistry.initialise(_registry); + + return _registry.getVirtualHostRegistry().getVirtualHost(vhostName); + } + + /** + * Create a configuration file for testing virtualhost creation + * + * @param vhostName name of the virtualhost + * @param queueName name of the queue + * @param exchangeName name of a direct exchange to declare (unless dontDeclare = true) and bind the queue to (null = none) + * @param dontDeclare if true then dont declare the exchange, even if its name is non-null + * @param routingKeys routingKeys to bind the queue with (empty array = none) + * @return + */ + private File writeConfigFile(String vhostName, String queueName, String exchangeName, boolean dontDeclare, String[] routingKeys) + { + File tmpFile = null; + try + { + tmpFile = File.createTempFile(getName(), ".tmp"); + tmpFile.deleteOnExit(); + + FileWriter fstream = new FileWriter(tmpFile); + BufferedWriter writer = new BufferedWriter(fstream); + + //extra outer tag to please Commons Configuration + writer.write("<configuration>"); + + writer.write("<virtualhosts>"); + writer.write(" <default>" + vhostName + "</default>"); + writer.write(" <virtualhost>"); + writer.write(" <store>"); + writer.write(" <class>" + TestableMemoryMessageStore.class.getName() + "</class>"); + writer.write(" </store>"); + writer.write(" <name>" + vhostName + "</name>"); + writer.write(" <" + vhostName + ">"); + if(exchangeName != null && !dontDeclare) + { + writer.write(" <exchanges>"); + writer.write(" <exchange>"); + writer.write(" <type>direct</type>"); + writer.write(" <name>" + exchangeName + "</name>"); + writer.write(" </exchange>"); + writer.write(" </exchanges>"); + } + writer.write(" <queues>"); + writer.write(" <queue>"); + writer.write(" <name>" + queueName + "</name>"); + writer.write(" <" + queueName + ">"); + if(exchangeName != null) + { + writer.write(" <exchange>" + exchangeName + "</exchange>"); + } + for(String routingKey: routingKeys) + { + writer.write(" <routingKey>" + routingKey + "</routingKey>"); + } + writer.write(" </" + queueName + ">"); + writer.write(" </queue>"); + writer.write(" </queues>"); + writer.write(" </" + vhostName + ">"); + writer.write(" </virtualhost>"); + writer.write("</virtualhosts>"); + + writer.write("</configuration>"); + + writer.flush(); + writer.close(); + } + catch (IOException e) + { + fail("Unable to create virtualhost configuration"); + } + + return tmpFile; + } +} |