QPID-2903, QPID-3390, QPID-3392: split tests into those with and without exchange creation arguments, add verification of exchange declare arguments, add new test to validate behaviour when supplying nonsense/unsupported arguments, exclude failign tests until functionality is implemented

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

3 files changed, 114 insertions, 17 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
index a2f92af8e3..f316d60c6a 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSessionDelegate.java
@@ -457,6 +457,19 @@ public class ServerSessionDelegate extends SessionDelegate
         VirtualHost virtualHost = getVirtualHost(session);
         Exchange exchange = getExchange(session, exchangeName);
 
+        //we must check for any unsupported arguments present and throw not-implemented
+        if(method.hasArguments())
+        {
+            Map<String,Object> args = method.getArguments();
+
+            //QPID-3392: currently we don't support any!
+            if(!args.isEmpty())
+            {
+                exception(session, method, ExecutionErrorCode.NOT_IMPLEMENTED, "Unsupported exchange argument(s) found " + args.keySet().toString());
+                return;
+            }
+        }
+
         if(method.getPassive())
         {
             if(exchange == null)
@@ -466,7 +479,6 @@ public class ServerSessionDelegate extends SessionDelegate
             }
             else
             {
-                // TODO - check exchange has same properties
                 if(!exchange.getTypeShortString().toString().equals(method.getType()))
                 {
                     exception(session, method, ExecutionErrorCode.NOT_ALLOWED, "Cannot redeclare with a different exchange type");
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/test/client/destination/AddressBasedDestinationTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/test/client/destination/AddressBasedDestinationTest.java
index 9fe6636daa..b5bb74327e 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/test/client/destination/AddressBasedDestinationTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/test/client/destination/AddressBasedDestinationTest.java
@@ -55,6 +55,7 @@ import org.apache.qpid.client.messaging.address.Node.QueueNode;
 import org.apache.qpid.jndi.PropertiesFileInitialContextFactory;
 import org.apache.qpid.messaging.Address;
 import org.apache.qpid.test.utils.QpidBrokerTestCase;
+import org.apache.qpid.transport.ExecutionErrorCode;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -269,8 +270,33 @@ public class AddressBasedDestinationTest extends QpidBrokerTestCase
     
     public void testCreateExchange() throws Exception
     {
+        createExchangeImpl(false, false);
+    }
+
+    /**
+     * Verify creating an exchange via an Address, with supported
+     * exchange-declare arguments.
+     */
+    public void testCreateExchangeWithArgs() throws Exception
+    {
+        createExchangeImpl(true, false);
+    }
+
+    /**
+     * Verify that when creating an exchange via an Address, if a
+     * nonsense argument is specified the broker throws an execution
+     * exception back on the session with NOT_IMPLEMENTED status.
+     */
+    public void testCreateExchangeWithNonsenseArgs() throws Exception
+    {
+        createExchangeImpl(true, true);
+    }
+
+    private void createExchangeImpl(final boolean withExchangeArgs,
+            final boolean useNonsenseArguments) throws Exception
+    {
         Session jmsSession = _connection.createSession(false,Session.AUTO_ACKNOWLEDGE);
-        
+
         String addr = "ADDR:my-exchange/hello; " + 
                       "{ " + 
                         "create: always, " +                        
@@ -280,17 +306,36 @@ public class AddressBasedDestinationTest extends QpidBrokerTestCase
                              "x-declare: " +
                              "{ " + 
                                  "type:direct, " + 
-                                 "auto-delete: true, " +
-                                 "arguments: {" +  
-                                   "'qpid.msg_sequence': 1, " +
-                                   "'qpid.ive': 1" +
-                                 "}" +
+                                 "auto-delete: true" +
+                                 createExchangeArgsString(withExchangeArgs, useNonsenseArguments) +
                              "}" +
                         "}" +
                       "}";
         
         AMQDestination dest = new AMQAnyDestination(addr);
-        MessageConsumer cons = jmsSession.createConsumer(dest); 
+
+        MessageConsumer cons;
+        try
+        {
+            cons = jmsSession.createConsumer(dest);
+            if(useNonsenseArguments)
+            {
+                fail("Expected execution exception during exchange declare did not occur");
+            }
+        }
+        catch(JMSException e)
+        {
+            if(useNonsenseArguments && e.getCause().getMessage().contains(ExecutionErrorCode.NOT_IMPLEMENTED.toString()))
+            {
+                //expected because we used an argument which the broker doesn't have functionality
+                //for. We can't do the rest of the test as a result of the exception, just stop.
+                return;
+            }
+            else
+            {
+                fail("Unexpected exception whilst creating consumer: " + e);
+            }
+        }
         
         assertTrue("Exchange not created as expected",(
                 (AMQSession_0_10)jmsSession).isExchangeExist(dest, (ExchangeNode)dest.getTargetNode() , true));
@@ -305,6 +350,32 @@ public class AddressBasedDestinationTest extends QpidBrokerTestCase
         cons = jmsSession.createConsumer(dest); 
     }
     
+    private String createExchangeArgsString(final boolean withExchangeArgs,
+                                            final boolean useNonsenseArguments)
+    {
+        String argsString;
+
+        if(withExchangeArgs && useNonsenseArguments)
+        {
+            argsString = ", arguments: {" +
+            "'abcd.1234.wxyz': 1, " +
+            "}";
+        }
+        else if(withExchangeArgs)
+        {
+            argsString = ", arguments: {" +
+            "'qpid.msg_sequence': 1, " +
+            "'qpid.ive': 1" +
+            "}";
+        }
+        else
+        {
+            argsString = "";
+        }
+
+        return argsString;
+    }
+
     public void checkQueueForBindings(Session jmsSession, AMQDestination dest,String headersBinding) throws Exception
     {
     	assertTrue("Queue not created as expected",(
@@ -558,11 +629,25 @@ public class AddressBasedDestinationTest extends QpidBrokerTestCase
     }
     
     /**
-     * Test goal: Verifies that session.creatTopic method
-     *            works as expected both with the new and old addressing scheme.
+     * Test goal: Verifies that session.creatTopic method works as expected
+     * both with the new and old addressing scheme.
      */
     public void testSessionCreateTopic() throws Exception
     {
+        sessionCreateTopicImpl(false);
+    }
+
+    /**
+     * Test goal: Verifies that session.creatTopic method works as expected
+     * both with the new and old addressing scheme when adding exchange arguments.
+     */
+    public void testSessionCreateTopicWithExchangeArgs() throws Exception
+    {
+        sessionCreateTopicImpl(true);
+    }
+
+    private void sessionCreateTopicImpl(boolean withExchangeArgs) throws Exception
+    {
         Session ssn = _connection.createSession(false,Session.AUTO_ACKNOWLEDGE);
         
         // Using the BURL method
@@ -582,7 +667,7 @@ public class AddressBasedDestinationTest extends QpidBrokerTestCase
         prod.send(ssn.createTextMessage("test"));
         assertNotNull("consumer should receive a message",cons.receive(1000));
         cons.close();
-        
+
         String addr = "ADDR:vehicles/bus; " + 
         "{ " + 
           "create: always, " +                        
@@ -592,11 +677,8 @@ public class AddressBasedDestinationTest extends QpidBrokerTestCase
                "x-declare: " +
                "{ " + 
                    "type:direct, " + 
-                   "auto-delete: true, " +
-                   "arguments: {" +  
-                       "'qpid.msg_sequence': 1, " +
-                       "'qpid.ive': 1" + 
-                   "}" +
+                   "auto-delete: true" +
+                   createExchangeArgsString(withExchangeArgs, false) +
                "}" +
           "}, " +
           "link: {name : my-topic, " +
diff --git a/qpid/java/test-profiles/Java010Excludes b/qpid/java/test-profiles/Java010Excludes
index 36f156157a..f8210e2b06 100755
--- a/qpid/java/test-profiles/Java010Excludes
+++ b/qpid/java/test-profiles/Java010Excludes
@@ -77,8 +77,11 @@ org.apache.qpid.test.unit.client.channelclose.ChannelCloseTest#*
 
 //Temporarily adding the following until the issues are sorted out.
 //Should probably raise JIRAs for them.
-org.apache.qpid.test.client.destination.AddressBasedDestinationTest#testCreateExchange
 org.apache.qpid.test.client.destination.AddressBasedDestinationTest#testBrowseMode
 
 // QPID-3133: On 0-10, the exception listener is currently not invoked when reconnection fails to occurs. 
 org.apache.qpid.server.failover.FailoverMethodTest#*
+
+// QPID-3092: the Java broker does not yet implement exchange creation arguments
+org.apache.qpid.test.client.destination.AddressBasedDestinationTest#testCreateExchangeWithArgs
+org.apache.qpid.test.client.destination.AddressBasedDestinationTest#testSessionCreateTopicWithExchangeArgs