You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ro...@apache.org on 2011/08/13 20:50:30 UTC
svn commit: r1157410 - in /qpid/trunk/qpid/java/broker/src:
main/java/org/apache/qpid/server/registry/
main/java/org/apache/qpid/server/virtualhost/
test/java/org/apache/qpid/server/queue/
test/java/org/apache/qpid/server/virtualhost/
Author: robbie
Date: Sat Aug 13 18:50:30 2011
New Revision: 1157410
URL: http://svn.apache.org/viewvc?rev=1157410&view=rev
Log:
QPID-2873: ensure that queues in the configuration file are always bound to the default exchange with their name, unknown exchanges properly cause exception to be thrown, and you cant use custom bindings against the default exchange
Added:
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java
Modified:
qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/virtualhost/VirtualHostImpl.java
qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java
Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java?rev=1157410&r1=1157409&r2=1157410&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java Sat Aug 13 18:50:30 2011
@@ -568,7 +568,7 @@ public abstract class ApplicationRegistr
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;
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=1157410&r1=1157409&r2=1157410&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 Sat Aug 13 18:50:30 2011
@@ -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
}
}
- 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
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()
Modified: qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java?rev=1157410&r1=1157409&r2=1157410&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java (original)
+++ qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/queue/SimpleAMQQueueTest.java Sat Aug 13 18:50:30 2011
@@ -107,7 +107,7 @@ public class SimpleAMQQueueTest extends
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);
Added: qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java?rev=1157410&view=auto
==============================================================================
--- qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java (added)
+++ qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/virtualhost/VirtualHostImplTest.java Sat Aug 13 18:50:30 2011
@@ -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;
+ }
+}
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org