QPID-3490: creating queue via JMX now binds the queue to the default exchange; prevented unregistration of the default exchange.

Applied patch from Oleksandr Rudyy<orudyy@gmail.com> and myself.

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

12 files changed, 209 insertions, 31 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/AMQBrokerManagerMBean.java b/java/broker/src/main/java/org/apache/qpid/server/AMQBrokerManagerMBean.java
index d1ea5dba69..76b6dad996 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/AMQBrokerManagerMBean.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/AMQBrokerManagerMBean.java
@@ -60,6 +60,7 @@ public class AMQBrokerManagerMBean extends AMQManagedObject implements ManagedBr
     private final QueueRegistry _queueRegistry;
     private final ExchangeRegistry _exchangeRegistry;
     private final ExchangeFactory _exchangeFactory;
+    private final Exchange _defaultExchange;
     private final DurableConfigurationStore _durableConfig;
 
     private final VirtualHostImpl.VirtualHostMBean _virtualHostMBean;
@@ -74,6 +75,7 @@ public class AMQBrokerManagerMBean extends AMQManagedObject implements ManagedBr
 
         _queueRegistry = virtualHost.getQueueRegistry();
         _exchangeRegistry = virtualHost.getExchangeRegistry();
+        _defaultExchange = _exchangeRegistry.getDefaultExchange();
         _durableConfig = virtualHost.getDurableConfigurationStore();
         _exchangeFactory = virtualHost.getExchangeFactory();
     }
@@ -256,13 +258,14 @@ public class AMQBrokerManagerMBean extends AMQManagedObject implements ManagedBr
                 ownerShortString = new AMQShortString(owner);
             }
 
-            queue = AMQQueueFactory.createAMQQueueImpl(new AMQShortString(queueName), durable, ownerShortString, false, false, getVirtualHost(), null);
+            final VirtualHost virtualHost = getVirtualHost();
+            queue = AMQQueueFactory.createAMQQueueImpl(new AMQShortString(queueName), durable, ownerShortString, false, false, virtualHost, null);
             if (queue.isDurable() && !queue.isAutoDelete())
             {
                 _durableConfig.createQueue(queue);
             }
 
-            _queueRegistry.registerQueue(queue);
+            virtualHost.getBindingFactory().addBinding(queueName, queue, _defaultExchange, null);
         }
         catch (AMQException ex)
         {
diff --git a/java/broker/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java b/java/broker/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java
index 0e7459498a..8d2dee5aaa 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/exchange/DefaultExchangeRegistry.java
@@ -27,7 +27,9 @@ import java.util.concurrent.ConcurrentMap;
 import org.apache.log4j.Logger;
 import org.apache.qpid.AMQException;
 import org.apache.qpid.AMQSecurityException;
+import org.apache.qpid.exchange.ExchangeDefaults;
 import org.apache.qpid.framing.AMQShortString;
+import org.apache.qpid.protocol.AMQConstant;
 import org.apache.qpid.server.queue.IncomingMessage;
 import org.apache.qpid.server.store.DurableConfigurationStore;
 import org.apache.qpid.server.virtualhost.VirtualHost;
@@ -87,12 +89,22 @@ public class DefaultExchangeRegistry implements ExchangeRegistry
 
     public void unregisterExchange(AMQShortString name, boolean inUse) throws AMQException
     {
-        // Check access
-        if (!_host.getSecurityManager().authoriseDelete(_exchangeMap.get(name)))
+        final Exchange exchange = _exchangeMap.get(name);
+        if (exchange == null)
+        {
+            throw new AMQException(AMQConstant.NOT_FOUND, "Unknown exchange " + name, null);
+        }
+
+        if (ExchangeDefaults.DEFAULT_EXCHANGE_NAME.equals(name))
+        {
+            throw new AMQException(AMQConstant.NOT_ALLOWED, "Cannot unregister the default exchange", null);
+        }
+
+        if (!_host.getSecurityManager().authoriseDelete(exchange))
         {
             throw new AMQSecurityException();
         }
-        
+
         // TODO: check inUse argument
 
         Exchange e = _exchangeMap.remove(name);
diff --git a/java/broker/src/test/java/org/apache/qpid/server/AMQBrokerManagerMBeanTest.java b/java/broker/src/test/java/org/apache/qpid/server/AMQBrokerManagerMBeanTest.java
index 6c135e8ba7..fdd533b704 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/AMQBrokerManagerMBeanTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/AMQBrokerManagerMBeanTest.java
@@ -20,9 +20,9 @@
  */
 package org.apache.qpid.server;
 
-import junit.framework.TestCase;
 import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.management.common.mbeans.ManagedBroker;
+import org.apache.qpid.server.exchange.Exchange;
 import org.apache.qpid.server.exchange.ExchangeRegistry;
 import org.apache.qpid.server.queue.QueueRegistry;
 import org.apache.qpid.server.registry.ApplicationRegistry;
@@ -81,6 +81,20 @@ public class AMQBrokerManagerMBeanTest extends InternalBrokerBaseCase
         assertTrue(_queueRegistry.getQueue(new AMQShortString(queueName)) == null);
     }
 
+    public void testCreateNewQueueBindsToDefaultExchange() throws Exception
+    {
+        String queueName = "testQueue_" + System.currentTimeMillis();
+
+        ManagedBroker mbean = new AMQBrokerManagerMBean((VirtualHostImpl.VirtualHostMBean) _vHost.getManagedObject());
+        ExchangeRegistry exReg = _vHost.getExchangeRegistry();
+        Exchange defaultExchange =  exReg.getDefaultExchange();
+
+        mbean.createNewQueue(queueName, "test", false);
+        assertTrue(_queueRegistry.getQueue(new AMQShortString(queueName)) != null);
+
+        assertTrue("New queue should be bound to default exchange", defaultExchange.isBound(new AMQShortString(queueName)));
+    }
+
     @Override
     public void setUp() throws Exception
     {
diff --git a/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagedBrokerMBeanTest.java b/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagedBrokerMBeanTest.java
new file mode 100644
index 0000000000..390a7b55e4
--- /dev/null
+++ b/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagedBrokerMBeanTest.java
@@ -0,0 +1,139 @@
+/*
+ * 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.management.jmx;
+
+import javax.management.JMException;
+import javax.management.MBeanException;
+import javax.management.ObjectName;
+
+import org.apache.qpid.exchange.ExchangeDefaults;
+import org.apache.qpid.management.common.mbeans.ManagedBroker;
+import org.apache.qpid.management.common.mbeans.ManagedExchange;
+import org.apache.qpid.test.utils.JMXTestUtils;
+import org.apache.qpid.test.utils.QpidBrokerTestCase;
+
+/**
+ * Tests the JMX API for the Managed Broker.
+ *
+ */
+public class ManagedBrokerMBeanTest extends QpidBrokerTestCase
+{
+    /**
+     * Test virtual host
+     */
+    private static final String VIRTUAL_HOST = "test";
+
+    /**
+     * Test exchange type
+     */
+    private static final String EXCHANGE_TYPE = "topic";
+
+    /**
+     * JMX helper.
+     */
+    private JMXTestUtils _jmxUtils;
+    private ManagedBroker _managedBroker;
+
+    public void setUp() throws Exception
+    {
+        _jmxUtils = new JMXTestUtils(this);
+        _jmxUtils.setUp();
+        super.setUp();
+        _jmxUtils.open();
+        _managedBroker = _jmxUtils.getManagedBroker(VIRTUAL_HOST);
+    }
+
+    public void tearDown() throws Exception
+    {
+        if (_jmxUtils != null)
+        {
+            _jmxUtils.close();
+        }
+        super.tearDown();
+    }
+
+    /**
+     * Tests queue creation/deletion also verifying the automatic binding to the default exchange.
+     */
+    public void testCreateQueueAndDeletion() throws Exception
+    {
+        final String queueName = getTestQueueName();
+        final ManagedExchange defaultExchange = _jmxUtils.getManagedExchange(ExchangeDefaults.DEFAULT_EXCHANGE_NAME.asString());
+
+        // Check that bind does not exist before queue creation
+        assertFalse("Binding to " + queueName + " should not exist in default exchange before queue creation",
+                     defaultExchange.bindings().containsKey(new String[] {queueName}));
+
+        _managedBroker.createNewQueue(queueName, "testowner", true);
+
+        // Ensure the queue exists
+        assertNotNull("Queue object name expected to exist", _jmxUtils.getQueueObjectName("test", queueName));
+        assertNotNull("Manager queue expected to be available", _jmxUtils.getManagedQueue(queueName));
+
+        // Now verify that the default exchange has been bound.
+        assertTrue("Binding to " + queueName + " should exist in default exchange after queue creation",
+                     defaultExchange.bindings().containsKey(new String[] {queueName}));
+
+        // Now delete the queue
+        _managedBroker.deleteQueue(queueName);
+
+        // Finally ensure that the binding has been removed.
+        assertFalse("Binding to " + queueName + " should not exist in default exchange after queue deletion",
+                defaultExchange.bindings().containsKey(new String[] {queueName}));
+    }
+
+    /**
+     * Tests exchange creation/deletion via JMX API.
+     */
+    public void testCreateExchangeAndUnregister() throws Exception
+    {
+        String exchangeName = getTestName();
+        _managedBroker.createNewExchange(exchangeName, "topic", true);
+        String queryString = "org.apache.qpid:type=VirtualHost.Exchange,VirtualHost="
+                + ObjectName.quote(VIRTUAL_HOST) + ",name=" + ObjectName.quote(exchangeName) + ",ExchangeType="
+                + EXCHANGE_TYPE;
+        ManagedExchange exchange = _jmxUtils.getManagedObject(ManagedExchange.class, queryString);
+        assertNotNull("Exchange should exist", exchange);
+
+        _managedBroker.unregisterExchange(exchangeName);
+        assertFalse("Exchange should have been removed", _jmxUtils.isManagedObjectExist(queryString));
+    }
+
+    /**
+     * Tests that it is disallowed to unregister the default exchange.
+     */
+    public void testUnregisterOfDefaultExchangeDisallowed() throws Exception
+    {
+        String defaultExchangeName = ExchangeDefaults.DEFAULT_EXCHANGE_NAME.asString();
+
+        try
+        {
+            _managedBroker.unregisterExchange(defaultExchangeName);
+            fail("Exception not thrown");
+        }
+        catch (MBeanException mbe)
+        {
+            // PASS
+            assertEquals("Error in unregistering exchange " + defaultExchangeName, mbe.getMessage());
+            assertTrue(mbe.getCause().getMessage().contains("Cannot unregister the default exchange"));
+        }
+        final ManagedExchange defaultExchange = _jmxUtils.getManagedExchange(defaultExchangeName);
+        assertNotNull("Exchange should exist", defaultExchange);
+    }
+}
diff --git a/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagementActorLoggingTest.java b/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagementActorLoggingTest.java
index 12a1682212..0e2875235f 100644
--- a/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagementActorLoggingTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagementActorLoggingTest.java
@@ -45,12 +45,11 @@ public class ManagementActorLoggingTest extends AbstractTestLogging
 {
     private JMXTestUtils _jmxUtils;
     private boolean _closed = false;
-    private static final String USER = "admin";
 
     @Override
     public void setUp() throws Exception
     {
-        _jmxUtils = new JMXTestUtils(this, USER, USER);
+        _jmxUtils = new JMXTestUtils(this);
         _jmxUtils.setUp();
         super.setUp();
         _jmxUtils.open();
@@ -364,7 +363,7 @@ public class ManagementActorLoggingTest extends AbstractTestLogging
 
         List<String> results = waitAndFindMatches("BND-1001");
 
-        assertEquals("More than one bind creation found", 1, results.size());
+        assertEquals("Unexpected number of bindings logged", 2, results.size());
 
         String log = getLogMessage(results, 0);
 
@@ -391,7 +390,7 @@ public class ManagementActorLoggingTest extends AbstractTestLogging
 
         List<String> results = waitAndFindMatches("BND-1001");
 
-        assertEquals("More than one bind creation found", 1, results.size());
+        assertEquals("Unexpected number of bindings logged", 2, results.size());
 
         String log = getLogMessage(results, 0);
 
@@ -418,7 +417,7 @@ public class ManagementActorLoggingTest extends AbstractTestLogging
 
         List<String> results = waitAndFindMatches("BND-1001");
 
-        assertEquals("More than one bind creation found", 1, results.size());
+        assertEquals("Unexpected number of bindings logged", 2, results.size());
 
         String log = getLogMessage(results, 0);
 
diff --git a/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java b/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java
index e3fd042560..ddc51f69bd 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/queue/ModelTest.java
@@ -60,7 +60,6 @@ import java.lang.reflect.UndeclaredThrowableException;
 public class ModelTest extends QpidBrokerTestCase
 {
 
-    private static final String USER = "admin";
     private JMXTestUtils _jmxUtils;
     private static final String VIRTUALHOST_NAME = "test";
 
@@ -68,7 +67,7 @@ public class ModelTest extends QpidBrokerTestCase
     public void setUp() throws Exception
     {
         // Create a JMX Helper
-        _jmxUtils = new JMXTestUtils(this, USER, USER);
+        _jmxUtils = new JMXTestUtils(this);
         _jmxUtils.setUp();
         super.setUp();
 
diff --git a/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java b/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java
index a724e6c66e..775d2c3eb0 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/queue/ProducerFlowControlTest.java
@@ -55,11 +55,10 @@ public class ProducerFlowControlTest extends AbstractTestLogging
 
     private JMXTestUtils _jmxUtils;
     private boolean _jmxUtilConnected;
-    private static final String USER = "admin";
 
     public void setUp() throws Exception
     {
-        _jmxUtils = new JMXTestUtils(this, USER , USER);
+        _jmxUtils = new JMXTestUtils(this);
         _jmxUtils.setUp();
         _jmxUtilConnected=false;
         super.setUp();
diff --git a/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java b/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java
index b823690002..4552cf7004 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalACLJMXTest.java
@@ -52,7 +52,7 @@ public class ExternalACLJMXTest extends AbstractACLTestCase
     @Override
     public void setUp() throws Exception
     {
-        _jmx = new JMXTestUtils(this, "admin", "admin");
+        _jmx = new JMXTestUtils(this);
         _jmx.setUp();
         super.setUp();
         _jmx.open();
diff --git a/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalAdminACLTest.java b/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalAdminACLTest.java
index 290cbfdc14..b0bc4fd946 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalAdminACLTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/security/acl/ExternalAdminACLTest.java
@@ -63,7 +63,7 @@ public class ExternalAdminACLTest extends AbstractACLTestCase
     {
 		_testConfigFile = createTempTestLog4JConfig();
 		
-        _jmx = new JMXTestUtils(this, "admin", "admin");
+        _jmx = new JMXTestUtils(this);
         _jmx.setUp();
         super.setUp();
         _jmx.open();
diff --git a/java/systests/src/main/java/org/apache/qpid/test/client/message/JMSDestinationTest.java b/java/systests/src/main/java/org/apache/qpid/test/client/message/JMSDestinationTest.java
index a7efe4922b..2d326d73b8 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/client/message/JMSDestinationTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/client/message/JMSDestinationTest.java
@@ -58,7 +58,6 @@ public class JMSDestinationTest extends QpidBrokerTestCase
     private Connection _connection;
     private Session _session;
 
-    private static final String USER = "admin";
     private CountDownLatch _receiveMessage;
     private Message _message;
 
@@ -143,7 +142,7 @@ public class JMSDestinationTest extends QpidBrokerTestCase
     public void testMovedToQueue() throws Exception
     {
         // Setup JMXUtils
-        JMXTestUtils jmxUtils = new JMXTestUtils(this, USER, USER);
+        JMXTestUtils jmxUtils = new JMXTestUtils(this);
         jmxUtils.setUp();
         // Open the JMX Connection
         jmxUtils.open();
diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/JMXTestUtils.java b/java/systests/src/main/java/org/apache/qpid/test/utils/JMXTestUtils.java
index 1fde6c7c73..3a1710671c 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/utils/JMXTestUtils.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/JMXTestUtils.java
@@ -51,18 +51,26 @@ import org.apache.qpid.management.common.mbeans.UserManagement;
  */
 public class JMXTestUtils
 {
-    QpidBrokerTestCase _test;
-    MBeanServerConnection _mbsc;
-    JMXConnector _jmxc;
+    private static final String DEFAULT_PASSWORD = "admin";
+    private static final String DEFAULT_USERID = "admin";
 
-    private String USER;
-    private String PASSWORD;
+    private MBeanServerConnection _mbsc;
+    private JMXConnector _jmxc;
+
+    private final String _user;
+    private final String _password;
+    private final QpidBrokerTestCase _test;
 
     public JMXTestUtils(QpidBrokerTestCase test, String user, String password)
     {
         _test = test;
-        USER = user;
-        PASSWORD = password;
+        _user = user;
+        _password = password;
+    }
+
+    public JMXTestUtils(QpidBrokerTestCase test)
+    {
+        this(test, DEFAULT_USERID, DEFAULT_PASSWORD);
     }
 
     public void setUp() throws IOException, ConfigurationException, Exception
@@ -73,7 +81,7 @@ public class JMXTestUtils
     public void open() throws Exception
     {
         _jmxc = JMXConnnectionFactory.getJMXConnection(5000, "127.0.0.1",
-                    _test.getManagementPort(_test.getPort()), USER, PASSWORD);
+                    _test.getManagementPort(_test.getPort()), _user, _password);
 
         _mbsc = _jmxc.getMBeanServerConnection();
     }
@@ -319,6 +327,12 @@ public class JMXTestUtils
         return getManagedObject(managedClass, objectName);
     }
 
+    public boolean isManagedObjectExist(String query)
+    {
+        return !queryObjects(query).isEmpty();
+
+    }
+
     public <T> T getManagedObject(Class<T> managedClass, ObjectName objectName)
     {
         return MBeanServerInvocationHandler.newProxyInstance(_mbsc, objectName, managedClass, false);
@@ -370,7 +384,7 @@ public class JMXTestUtils
     }
 
     /**
-     * Retrive {@link ServerInformation} JMX MBean.
+     * Retrieve {@link ServerInformation} JMX MBean.
      */
     public ServerInformation getServerInformation()
     {
@@ -387,7 +401,7 @@ public class JMXTestUtils
     }
 
     /**
-     * Retrive all {@link ManagedConnection} objects.
+     * Retrieve all {@link ManagedConnection} objects.
      */
     public List<ManagedConnection> getAllManagedConnections()
     {
@@ -402,7 +416,7 @@ public class JMXTestUtils
     }
 
     /**
-     * Retrive all {@link ManagedConnection} objects for a particular virtual host.
+     * Retrieve all {@link ManagedConnection} objects for a particular virtual host.
      */
     public List<ManagedConnection> getManagedConnections(String vhost)
     {
diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java b/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
index bb44aea659..71fbf4cc81 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
@@ -1326,7 +1326,7 @@ public class QpidBrokerTestCase extends QpidTestCase
      */
     public void reloadBrokerSecurityConfig() throws Exception
     {
-        JMXTestUtils jmxu = new JMXTestUtils(this, "admin" , "admin");
+        JMXTestUtils jmxu = new JMXTestUtils(this);
         jmxu.open();
         
         try