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 bf0d236c8b..357b7f6cfd 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 @@ -16,6 +16,14 @@ */ package org.apache.activemq.artemis.jms.client; +import java.lang.ref.WeakReference; +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + import javax.jms.ConnectionConsumer; import javax.jms.ConnectionMetaData; import javax.jms.Destination; @@ -32,13 +40,6 @@ import javax.jms.Session; import javax.jms.Topic; import javax.jms.TopicConnection; import javax.jms.TopicSession; -import java.lang.ref.WeakReference; -import java.security.AccessController; -import java.security.PrivilegedAction; -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import org.apache.activemq.artemis.api.core.ActiveMQException; import org.apache.activemq.artemis.api.core.ActiveMQExceptionType; @@ -242,10 +243,9 @@ public class ActiveMQConnection extends ActiveMQConnectionForContextImpl impleme throw new IllegalStateException("setClientID can only be called directly after the connection is created"); } - validateClientID(initialSession, clientID); - - this.clientID = clientID; try { + validateClientID(initialSession, clientID); + this.clientID = clientID; this.addSessionMetaData(initialSession); } catch (ActiveMQException e) { JMSException ex = new JMSException("Internal error setting metadata jms-client-id"); @@ -257,12 +257,15 @@ public class ActiveMQConnection extends ActiveMQConnectionForContextImpl impleme justCreated = false; } - private void validateClientID(ClientSession validateSession, String clientID) throws InvalidClientIDException { + private void validateClientID(ClientSession validateSession, String clientID) + throws InvalidClientIDException, ActiveMQException { try { validateSession.addUniqueMetaData(ClientSession.JMS_SESSION_CLIENT_ID_PROPERTY, clientID); } catch (ActiveMQException e) { if (e.getType() == ActiveMQExceptionType.DUPLICATE_METADATA) { throw new InvalidClientIDException("clientID=" + clientID + " was already set into another connection"); + } else { + throw e; } } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java index 3057041827..f00fe1916a 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java @@ -1523,6 +1523,9 @@ public class ServerSessionImpl implements ServerSession, FailureListener { ServerSession sessionWithMetaData = server.lookupSession(key, data); if (sessionWithMetaData != null && sessionWithMetaData != this) { // There is a duplication of this property + if (server.hasBrokerPlugins()) { + server.callBrokerPlugins(plugin -> plugin.duplicateSessionMetadataFailure(this, key, data)); + } return false; } else { addMetaData(key, data); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerPlugin.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerPlugin.java index db66bfe772..97e23b48fe 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerPlugin.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerPlugin.java @@ -157,6 +157,18 @@ public interface ActiveMQServerPlugin { } + /** + * Called when adding session metadata fails because the metadata is a duplicate + * + * @param session + * @param key + * @param data + * @throws ActiveMQException + */ + default void duplicateSessionMetadataFailure(ServerSession session, String key, String data) throws ActiveMQException { + + } + /** * After session metadata is added to the session * diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/SessionMetadataAddExceptionTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/SessionMetadataAddExceptionTest.java index 4bc052295e..39a7cc013c 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/SessionMetadataAddExceptionTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/SessionMetadataAddExceptionTest.java @@ -16,7 +16,10 @@ */ package org.apache.activemq.artemis.tests.integration.jms.client; +import java.util.concurrent.atomic.AtomicInteger; + import javax.jms.Connection; +import javax.jms.InvalidClientIDException; import javax.jms.JMSException; import org.apache.activemq.artemis.api.core.ActiveMQException; @@ -33,9 +36,12 @@ import org.junit.Test; * is added */ public class SessionMetadataAddExceptionTest extends JMSTestBase { + private AtomicInteger duplicateCount = new AtomicInteger(); @Override protected Configuration createDefaultConfig(boolean netty) throws Exception { + duplicateCount.set(0); + Configuration config = super.createDefaultConfig(netty); config.registerBrokerPlugin(new ActiveMQServerPlugin() { @@ -50,6 +56,18 @@ public class SessionMetadataAddExceptionTest extends JMSTestBase { } } + @Override + public void duplicateSessionMetadataFailure(ServerSession session, String key, String data) + throws ActiveMQException { + + //count number of times method called + duplicateCount.incrementAndGet(); + + if (data.equals("valid2")) { + throw new ActiveMQException("failure"); + } + } + }); return config; @@ -75,6 +93,36 @@ public class SessionMetadataAddExceptionTest extends JMSTestBase { cf.createConnection(); } + @Test(timeout = 5000) + public void testDuplicateClientIdSet() throws Exception { + ActiveMQConnectionFactory activeMQConnectionFactory = (ActiveMQConnectionFactory) cf; + Connection con = cf.createConnection(); + Connection con2 = cf.createConnection(); + try { + con.setClientID("valid"); + con2.setClientID("valid"); + fail("Should have failed for duplicate clientId"); + } catch (InvalidClientIDException e) { + assertEquals(1, duplicateCount.get()); + } finally { + activeMQConnectionFactory.close(); + } + } + + @Test(timeout = 5000, expected = JMSException.class) + public void testDuplicateClientIdSetActiveMQException() throws Exception { + ActiveMQConnectionFactory activeMQConnectionFactory = (ActiveMQConnectionFactory) cf; + Connection con = cf.createConnection(); + Connection con2 = cf.createConnection(); + try { + con.setClientID("valid2"); + con2.setClientID("valid2"); + fail("Should have failed"); + } finally { + activeMQConnectionFactory.close(); + } + } + @Test(timeout = 5000) public void testValidIdSetConnection() throws Exception { Connection con = cf.createConnection();