QPID-4250: ensure producer creation on 0-8/0-9/0-9-1 connections respects the qpid.declare_exchanges system property. Add systest to highlight the issue and verify the fix. Add constants for the system properties.

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

5 files changed, 118 insertions, 39 deletions

diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQSession.java b/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
index 1468e90c4e..d4e6ec16e4 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
@@ -136,10 +136,10 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
                                                                   DEFAULT_FLOW_CONTROL_WAIT_FAILURE);
 
     private final boolean _delareQueues =
-        Boolean.parseBoolean(System.getProperty("qpid.declare_queues", "true"));
+        Boolean.parseBoolean(System.getProperty(ClientProperties.QPID_DECLARE_QUEUES_PROP_NAME, "true"));
 
     private final boolean _declareExchanges =
-        Boolean.parseBoolean(System.getProperty("qpid.declare_exchanges", "true"));
+        Boolean.parseBoolean(System.getProperty(ClientProperties.QPID_DECLARE_EXCHANGES_PROP_NAME, "true"));
 
     private final boolean _useAMQPEncodedMapMessage;
 
diff --git a/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer_0_8.java b/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer_0_8.java
index 21ff6c877a..04cc876b76 100644
--- a/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer_0_8.java
+++ b/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer_0_8.java
@@ -55,9 +55,10 @@ public class BasicMessageProducer_0_8 extends BasicMessageProducer
 
     void declareDestination(AMQDestination destination)
     {
-
-        final MethodRegistry methodRegistry = getSession().getMethodRegistry();
-        ExchangeDeclareBody body =
+        if(getSession().isDeclareExchanges())
+        {
+            final MethodRegistry methodRegistry = getSession().getMethodRegistry();
+            ExchangeDeclareBody body =
                 methodRegistry.createExchangeDeclareBody(getSession().getTicket(),
                                                          destination.getExchangeName(),
                                                          destination.getExchangeClass(),
@@ -67,12 +68,10 @@ public class BasicMessageProducer_0_8 extends BasicMessageProducer
                                                          false,
                                                          true,
                                                          null);
-        // Declare the exchange
-        // Note that the durable and internal arguments are ignored since passive is set to false
+            AMQFrame declare = body.generateFrame(getChannelId());
 
-        AMQFrame declare = body.generateFrame(getChannelId());
-
-        getProtocolHandler().writeFrame(declare);
+            getProtocolHandler().writeFrame(declare);
+        }
     }
 
     void sendMessage(AMQDestination destination, Message origMessage, AbstractJMSMessage message,
diff --git a/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java b/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
index 5268ce9bc2..20e523ca97 100644
--- a/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
+++ b/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
@@ -190,6 +190,18 @@ public class ClientProperties
      */
     public static final long DEFAULT_FLOW_CONTROL_WAIT_NOTIFY_PERIOD = 5000L;
 
+    /**
+     * System property to control whether the client will declare queues during
+     * consumer creation when using BindingURLs.
+     */
+    public static final String QPID_DECLARE_QUEUES_PROP_NAME = "qpid.declare_queues";
+
+    /**
+     * System property to control whether the client will declare exchanges during
+     * producer/consumer creation when using BindingURLs.
+     */
+    public static final String QPID_DECLARE_EXCHANGES_PROP_NAME = "qpid.declare_exchanges";
+
 
     private ClientProperties()
     {
diff --git a/java/systests/src/main/java/org/apache/qpid/test/unit/client/DynamicQueueExchangeCreateTest.java b/java/systests/src/main/java/org/apache/qpid/test/unit/client/DynamicQueueExchangeCreateTest.java
index 8577fb5b6a..cdb5e095b1 100644
--- a/java/systests/src/main/java/org/apache/qpid/test/unit/client/DynamicQueueExchangeCreateTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/unit/client/DynamicQueueExchangeCreateTest.java
@@ -21,8 +21,11 @@
 package org.apache.qpid.test.unit.client;
 
 import org.apache.qpid.AMQException;
+import org.apache.qpid.configuration.ClientProperties;
 import org.apache.qpid.protocol.AMQConstant;
+import org.apache.qpid.test.utils.JMXTestUtils;
 import org.apache.qpid.test.utils.QpidBrokerTestCase;
+import org.apache.qpid.url.BindingURL;
 
 import javax.jms.Connection;
 import javax.jms.JMSException;
@@ -37,9 +40,37 @@ import javax.jms.Session;
  */
 public class DynamicQueueExchangeCreateTest extends QpidBrokerTestCase
 {
-    public void testQueueDeclare() throws Exception
+    private JMXTestUtils _jmxUtils;
+
+    @Override
+    public void setUp() throws Exception
+    {
+        _jmxUtils = new JMXTestUtils(this);
+        _jmxUtils.setUp();
+
+        super.setUp();
+        _jmxUtils.open();
+    }
+
+    @Override
+    public void tearDown() throws Exception
+    {
+        try
+        {
+            if (_jmxUtils != null)
+            {
+                _jmxUtils.close();
+            }
+        }
+        finally
+        {
+            super.tearDown();
+        }
+    }
+
+    public void testQueueNotDeclaredDuringConsumerCreation() throws Exception
     {
-        setSystemProperty("qpid.declare_queues", "false");
+        setSystemProperty(ClientProperties.QPID_DECLARE_QUEUES_PROP_NAME, "false");
 
         Connection connection = getConnection();
 
@@ -58,16 +89,16 @@ public class DynamicQueueExchangeCreateTest extends QpidBrokerTestCase
         }
     }
 
-    public void testExchangeDeclare() throws Exception
+    public void testExchangeNotDeclaredDuringConsumerCreation() throws Exception
     {
-        setSystemProperty("qpid.declare_exchanges", "false");
+        setSystemProperty(ClientProperties.QPID_DECLARE_EXCHANGES_PROP_NAME, "false");
 
         Connection connection = getConnection();
 
         Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
 
-        String EXCHANGE_TYPE = "test.direct";
-        Queue queue = session.createQueue("direct://" + EXCHANGE_TYPE + "/queue/queue");
+        String exchangeName = getTestQueueName();
+        Queue queue = session.createQueue("direct://" + exchangeName + "/queue/queue");
 
         try
         {
@@ -78,6 +109,50 @@ public class DynamicQueueExchangeCreateTest extends QpidBrokerTestCase
         {
             checkExceptionErrorCode(e, AMQConstant.NOT_FOUND);
         }
+
+        //verify the exchange was not declared
+        String exchangeObjectName = _jmxUtils.getExchangeObjectName("test", exchangeName);
+        assertFalse("exchange should not exist", _jmxUtils.doesManagedObjectExist(exchangeObjectName));
+    }
+
+    /**
+     * Checks that setting {@value ClientProperties#QPID_DECLARE_EXCHANGES_PROP_NAME} false results in
+     * disabling implicit ExchangeDeclares during producer creation when using a {@link BindingURL}
+     */
+    public void testExchangeNotDeclaredDuringProducerCreation() throws Exception
+    {
+        Connection connection = getConnection();
+        Session session1 = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        String exchangeName1 = getTestQueueName() + "1";
+
+
+        Queue queue = session1.createQueue("direct://" + exchangeName1 + "/queue/queue");
+        session1.createProducer(queue);
+
+        //close the session to ensure any previous commands were fully processed by
+        //the broker before observing their effect
+        session1.close();
+
+        //verify the exchange was declared
+        String exchangeObjectName = _jmxUtils.getExchangeObjectName("test", exchangeName1);
+        assertTrue("exchange should exist", _jmxUtils.doesManagedObjectExist(exchangeObjectName));
+
+        //Now disable the implicit exchange declares and try again
+        setSystemProperty(ClientProperties.QPID_DECLARE_EXCHANGES_PROP_NAME, "false");
+
+        Session session2 = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+        String exchangeName2 = getTestQueueName() + "2";
+
+        Queue queue2 = session2.createQueue("direct://" + exchangeName2 + "/queue/queue");
+        session2.createProducer(queue2);
+
+        //close the session to ensure any previous commands were fully processed by
+        //the broker before observing their effect
+        session2.close();
+
+        //verify the exchange was not declared
+        String exchangeObjectName2 = _jmxUtils.getExchangeObjectName("test", exchangeName2);
+        assertFalse("exchange should not exist", _jmxUtils.doesManagedObjectExist(exchangeObjectName2));
     }
 
     private void checkExceptionErrorCode(JMSException original, AMQConstant code)
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 43b80b45fb..673fdde97d 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
@@ -303,31 +303,13 @@ public class JMXTestUtils
     }
 
     /**
-     * Retrive the ObjectName for the given Exchange on a VirtualHost.
-     *
-     * This is then used to create a proxy to the ManagedExchange MBean.
-     *
-     * @param virtualHostName the VirtualHost the Exchange is on
-     * @param exchange the Exchange to retireve e.g. 'direct'
-     * @return the ObjectName for the given Exchange on the VirtualHost
+     * Generate the ObjectName for the given Exchange on a VirtualHost.
      */
-    @SuppressWarnings("static-access")
-    public ObjectName getExchangeObjectName(String virtualHostName, String exchange)
+    public String getExchangeObjectName(String virtualHostName, String exchange)
     {
-        // Get the name of the test manager
-        String query = "org.apache.qpid:type=VirtualHost.Exchange,VirtualHost="
+        return "org.apache.qpid:type=VirtualHost.Exchange,VirtualHost="
                        + ObjectName.quote(virtualHostName) + ",name="
                        + ObjectName.quote(exchange) + ",*";
-
-        Set<ObjectName> objectNames = queryObjects(query);
-
-        _test.assertNotNull("Null ObjectName Set returned", objectNames);
-        _test.assertEquals("Incorrect number of exchange with name '" + exchange + "' returned", 1, objectNames.size());
-
-        // We have verified we have only one value in objectNames so return it
-        ObjectName objectName = objectNames.iterator().next();
-        _test.getLogger().info("Loading: " + objectName);
-        return objectName;
     }
 
     @SuppressWarnings("static-access")
@@ -343,7 +325,7 @@ public class JMXTestUtils
         return getManagedObject(managedClass, objectName);
     }
 
-    public boolean isManagedObjectExist(String query)
+    public boolean doesManagedObjectExist(String query)
     {
         return !queryObjects(query).isEmpty();
     }
@@ -373,9 +355,20 @@ public class JMXTestUtils
         return getManagedObject(ManagedBroker.class, getVirtualHostManagerObjectName(virtualHost));
     }
 
+    @SuppressWarnings("static-access")
     public ManagedExchange getManagedExchange(String exchangeName)
     {
-        ObjectName objectName = getExchangeObjectName("test", exchangeName);
+        String query = getExchangeObjectName("test", exchangeName);
+
+        Set<ObjectName> objectNames = queryObjects(query);
+
+        _test.assertNotNull("Null ObjectName Set returned", objectNames);
+        _test.assertEquals("Incorrect number of exchange with name '" + exchangeName + "' returned", 1, objectNames.size());
+
+        // We have verified we have only one value in objectNames so return an mbean proxy for it
+        ObjectName objectName = objectNames.iterator().next();
+        _test.getLogger().info("Loading: " + objectName);
+
         return MBeanServerInvocationHandler.newProxyInstance(_mbsc, objectName, ManagedExchange.class, false);
     }