From 754e9db5fdee3807758a4c2da14bc61e715d616f Mon Sep 17 00:00:00 2001
From: Michael Andre Pearce <Michael.Andre.Pearce@me.com>
Date: Tue, 6 Jun 2017 09:54:02 +0100
Subject: [PATCH] ARTEMIS-1207: [Core JMS Client] Align order of when
 setClientId can be called with AcitveMQ5 and QPID

Update ActiveMQConnection to change behaviour

Add test to avoid regression.
---
 .../jms/client/ActiveMQConnection.java        |  3 --
 .../jms/connection/ExceptionListenerTest.java | 27 +++++++++++++
 .../artemis/jms/tests/ConnectionTest.java     | 39 ++++++++++---------
 3 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java
index 90ab952599..51a89e331f 100644
--- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java
+++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java
@@ -280,8 +280,6 @@ public class ActiveMQConnection extends ActiveMQConnectionForContextImpl impleme
    public ExceptionListener getExceptionListener() throws JMSException {
       checkClosed();
 
-      justCreated = false;
-
       return exceptionListener;
    }
 
@@ -290,7 +288,6 @@ public class ActiveMQConnection extends ActiveMQConnectionForContextImpl impleme
       checkClosed();
 
       exceptionListener = listener;
-      justCreated = false;
    }
 
    @Override
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/ExceptionListenerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/ExceptionListenerTest.java
index feaa7a3421..9b913089bf 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/ExceptionListenerTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/ExceptionListenerTest.java
@@ -143,4 +143,31 @@ public class ExceptionListenerTest extends ActiveMQTestBase {
       conn.close();
    }
 
+
+   /**
+    * The JMS Spec isn't specific about if ClientId can be set after Exception Listener or not,
+    * simply it states that clientId must be set before any operation (read as remote)
+    *
+    * QpidJMS and ActiveMQ5 both interpret that therefor you can set the exception lister first.
+    * As such we align with those, allowing the exception listener to be set prior to the clientId,
+    * This to avoid causing implementation nuance's, when switching code from one client to another.
+    *
+    * This test is to test this and to ensure it doesn't get accidentally regressed.
+    */
+   @Test
+   public void testSetClientIdAfterSetExceptionListener() throws Exception {
+      Connection conn = cf.createConnection();
+      conn.setExceptionListener((e) -> { });
+      conn.setClientID("clientId");
+      conn.close();
+   }
+
+   @Test
+   public void testSetClientIdAfterGetExceptionListener() throws Exception {
+      Connection conn = cf.createConnection();
+      conn.getExceptionListener();
+      conn.setClientID("clientId");
+      conn.close();
+   }
+
 }
diff --git a/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/ConnectionTest.java b/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/ConnectionTest.java
index eab72f0544..2d89713c25 100644
--- a/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/ConnectionTest.java
+++ b/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/ConnectionTest.java
@@ -153,24 +153,6 @@ public class ConnectionTest extends JMSTestCase {
       //      {
       //      }
       //      connection.close();
-
-      connection = createConnection();
-      ExceptionListener listener = connection.getExceptionListener();
-      try {
-         connection.setClientID(clientID);
-         ProxyAssertSupport.fail();
-      } catch (javax.jms.IllegalStateException e) {
-      }
-      connection.close();
-
-      connection = createConnection();
-      connection.setExceptionListener(listener);
-      try {
-         connection.setClientID(clientID);
-         ProxyAssertSupport.fail();
-      } catch (javax.jms.IllegalStateException e) {
-      }
-      connection.close();
    }
 
    @Test
@@ -233,6 +215,27 @@ public class ConnectionTest extends JMSTestCase {
 
       conn.close();
 
+      // Ensure setting / getting exception listener can occur before setting clientid.
+      final String clientID = "my-test-client-id";
+
+      Connection connection = createConnection();
+      ExceptionListener listener = connection.getExceptionListener();
+      try {
+         connection.setClientID(clientID);
+      } catch (javax.jms.IllegalStateException e) {
+         ProxyAssertSupport.fail();
+      }
+      connection.close();
+
+      connection = createConnection();
+      connection.setExceptionListener(listener);
+      try {
+         connection.setClientID(clientID);
+      } catch (javax.jms.IllegalStateException e) {
+         ProxyAssertSupport.fail();
+      }
+      connection.close();
+
    }
 
    // This test is to check netty issue in https://jira.jboss.org/jira/browse/JBMESSAGING-1618