QPID-163 - attempting to change the client id results in an exception being thrown.

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

2 files changed, 49 insertions, 11 deletions

diff --git a/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java b/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
index 98db26d0c4..0bb8736227 100644
--- a/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
+++ b/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
@@ -44,6 +44,7 @@ import org.apache.qpid.jms.FailoverPolicy;
 import org.apache.qpid.url.URLSyntaxException;
 
 import javax.jms.*;
+import javax.jms.IllegalStateException;
 import javax.naming.NamingException;
 import javax.naming.Reference;
 import javax.naming.Referenceable;
@@ -92,7 +93,7 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     /**
      * Maps from session id (Integer) to AMQSession instance
      */
-    private final Map _sessions = new LinkedHashMap(); //fixme this is map is replicated in amqprotocolsession as _channelId2SessionMap
+    private final Map _sessions = new LinkedHashMap(); //fixme this is map is replicated in amqprotocolsession as _channelId2SessionMap    
 
     private String _clientName;
 
@@ -142,7 +143,8 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
                          String clientName, String virtualHost) throws AMQException, URLSyntaxException
     {
         this(new AMQConnectionURL(ConnectionURL.AMQ_PROTOCOL + "://" +
-                                  username + ":" + password + "@" + clientName +
+                                  username + ":" + password + "@" +
+                                  (clientName==null?"":clientName) +
                                   virtualHost + "?brokerlist='" + broker + "'"));
     }
 
@@ -157,11 +159,13 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     {
         this(new AMQConnectionURL(useSSL ?
                                   ConnectionURL.AMQ_PROTOCOL + "://" +
-                                  username + ":" + password + "@" + clientName +
+                                  username + ":" + password + "@" +
+                                  (clientName==null?"":clientName) +
                                   virtualHost + "?brokerlist='tcp://" + host + ":" + port + "'"
                                   + "," + ConnectionURL.OPTIONS_SSL + "='true'" :
                                                                                 ConnectionURL.AMQ_PROTOCOL + "://" +
-                                                                                username + ":" + password + "@" + clientName +
+                                                                                username + ":" + password + "@" +
+                                                                                (clientName==null?"":clientName) +
                                                                                 virtualHost + "?brokerlist='tcp://" + host + ":" + port + "'"
                                                                                 + "," + ConnectionURL.OPTIONS_SSL + "='false'"
         ));
@@ -537,7 +541,10 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     public void setClientID(String clientID) throws JMSException
     {
         checkNotClosed();
-        _clientName = clientID;
+        // in AMQP it is not possible to change the client ID. If one is not specified
+        // upon connection construction, an id is generated automatically. Therefore
+        // we can always throw an exception.
+        throw new IllegalStateException("Client name cannot be changed after being set");
     }
 
     public ConnectionMetaData getMetaData() throws JMSException
@@ -583,7 +590,6 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     public void stop() throws JMSException
     {
         checkNotClosed();
-
         if (_started)
         {
             for (Iterator i = _sessions.values().iterator(); i.hasNext();)
@@ -920,8 +926,8 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     void deregisterSession(int channelId)
     {
         _sessions.remove(channelId);
-    }
-
+    }    
+    
     /**
      * For all sessions, and for all consumers in those sessions, resubscribe. This is called during failover handling.
      * The caller must hold the failover mutex before calling this method.
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java b/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
index d7862d047f..0da4147351 100644
--- a/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
@@ -22,12 +22,10 @@ package org.apache.qpid.test.unit.client.connection;
 
 import org.apache.qpid.client.AMQConnection;
 import org.apache.qpid.client.AMQAuthenticationException;
-import org.apache.qpid.client.vmbroker.AMQVMBrokerCreationException;
 import org.apache.qpid.client.transport.TransportConnection;
 import org.apache.qpid.AMQException;
 import org.apache.qpid.AMQConnectionException;
 import org.apache.qpid.AMQUnresolvedAddressException;
-import org.apache.qpid.test.VMBrokerSetup;
 
 import javax.jms.Connection;
 
@@ -40,6 +38,18 @@ public class ConnectionTest extends TestCase
     String _broker_NotRunning = "vm://:2";
     String _broker_BadDNS = "tcp://hg3sgaaw4lgihjs";
 
+
+    protected void setUp() throws Exception
+    {
+        super.setUp();
+        TransportConnection.createVMBroker(1);
+    }
+
+    protected void tearDown() throws Exception
+    {
+        TransportConnection.killAllVMBrokers();
+    }
+
     public void testSimpleConnection()
     {
         try
@@ -102,8 +112,30 @@ public class ConnectionTest extends TestCase
         }
     }
 
+    public void testClientIdCannotBeChanged() throws Exception
+    {
+        Connection connection = new AMQConnection(_broker, "guest", "guest",
+                                                  "fred", "/test");
+        try
+        {
+            connection.setClientID("someClientId");
+            fail("No IllegalStateException thrown when resetting clientid");
+        }
+        catch (javax.jms.IllegalStateException e)
+        {
+            // PASS
+        }
+    }
+
+    public void testClientIdIsPopulatedAutomatically() throws Exception
+    {
+        Connection connection = new AMQConnection(_broker, "guest", "guest",
+                                                  null, "/test");
+        assertNotNull(connection.getClientID());
+    }
+
     public static junit.framework.Test suite()
     {
-        return new VMBrokerSetup(new junit.framework.TestSuite(ConnectionTest.class));
+        return new junit.framework.TestSuite(ConnectionTest.class);
     }
 }