Repository: qpid-jms Updated Branches: refs/heads/master 6217cb29a -> b8d40c2c1
require an explicit clientID be set on the Connection in order to create a DurableSubscriber, preventing their use with auto-generated clientID Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/6d2968c6 Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/6d2968c6 Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/6d2968c6 Branch: refs/heads/master Commit: 6d2968c63dc4e98263b56048a5e8fefbf961208e Parents: 6217cb2 Author: Robert Gemmell <rob...@apache.org> Authored: Wed Jan 14 11:06:52 2015 +0000 Committer: Robert Gemmell <rob...@apache.org> Committed: Wed Jan 14 11:06:52 2015 +0000 ---------------------------------------------------------------------- .../java/org/apache/qpid/jms/JmsConnection.java | 11 +++----- .../java/org/apache/qpid/jms/JmsSession.java | 7 +++++ .../jms/integration/IntegrationTestFixture.java | 10 ++++++-- .../jms/integration/SessionIntegrationTest.java | 27 ++++++++++++++++++++ 4 files changed, 46 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/6d2968c6/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java ---------------------------------------------------------------------- diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java index c71e3bc..bc98a85 100644 --- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java +++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsConnection.java @@ -231,13 +231,6 @@ public class JmsConnection implements Connection, TopicConnection, QueueConnecti destroyResource(connectionInfo); } - synchronized (this) { - if (clientIdSet) { - connectionInfo.setClientId(null); - clientIdSet = false; - } - } - tempDestinations.clear(); started.set(false); connected.set(false); @@ -614,6 +607,10 @@ public class JmsConnection implements Connection, TopicConnection, QueueConnecti return new JmsTransactionId(connectionInfo.getConnectionId(), transactionIdGenerator.incrementAndGet()); } + protected synchronized boolean isExplicitClientID() { + return clientIdSet; + } + //////////////////////////////////////////////////////////////////////////// // Provider interface methods //////////////////////////////////////////////////////////////////////////// http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/6d2968c6/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java ---------------------------------------------------------------------- diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java index e98f59b..92410dc 100644 --- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java +++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/JmsSession.java @@ -417,6 +417,7 @@ public class JmsSession implements Session, QueueSession, TopicSession, JmsMessa public TopicSubscriber createDurableSubscriber(Topic topic, String name, String messageSelector, boolean noLocal) throws JMSException { checkClosed(); checkDestination(topic); + checkClientIDWasSetExplicitly(); messageSelector = checkSelector(messageSelector); JmsDestination dest = JmsMessageTransformation.transformDestination(connection, topic); JmsTopicSubscriber result = new JmsDurableTopicSubscriber(getNextConsumerId(), this, dest, name, false, messageSelector); @@ -424,6 +425,12 @@ public class JmsSession implements Session, QueueSession, TopicSession, JmsMessa return result; } + protected void checkClientIDWasSetExplicitly() throws IllegalStateException { + if (!connection.isExplicitClientID()) { + throw new IllegalStateException("You must specify a unique clientID for the Connection to use a DurableSubscriber"); + } + } + /** * @param name * @throws JMSException http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/6d2968c6/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/IntegrationTestFixture.java ---------------------------------------------------------------------- diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/IntegrationTestFixture.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/IntegrationTestFixture.java index 804111c..34fd3cc 100644 --- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/IntegrationTestFixture.java +++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/IntegrationTestFixture.java @@ -49,6 +49,10 @@ public class IntegrationTestFixture { } Connection establishConnecton(TestAmqpPeer testPeer, String optionsString, Symbol[] serverCapabilities, Map<Symbol, Object> serverProperties) throws JMSException { + return establishConnecton(testPeer, null, serverCapabilities, serverProperties, true); + } + + Connection establishConnecton(TestAmqpPeer testPeer, String optionsString, Symbol[] serverCapabilities, Map<Symbol, Object> serverProperties, boolean setClientId) throws JMSException { testPeer.expectPlainConnect("guest", "guest", serverCapabilities, serverProperties); // Each connection creates a session for managing temporary destinations etc @@ -63,8 +67,10 @@ public class IntegrationTestFixture { ConnectionFactory factory = new JmsConnectionFactory(brokerURI); Connection connection = factory.createConnection("guest", "guest"); - // Set a clientId to provoke the actual AMQP connection process to occur. - connection.setClientID("clientName"); + if(setClientId) { + // Set a clientId to provoke the actual AMQP connection process to occur. + connection.setClientID("clientName"); + } assertNull(testPeer.getThrowable()); return connection; http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/6d2968c6/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java ---------------------------------------------------------------------- diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java index 057465f..2610b97 100644 --- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java +++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SessionIntegrationTest.java @@ -34,6 +34,7 @@ import java.io.IOException; import javax.jms.Connection; import javax.jms.Destination; +import javax.jms.IllegalStateException; import javax.jms.JMSException; import javax.jms.Message; import javax.jms.MessageConsumer; @@ -455,6 +456,32 @@ public class SessionIntegrationTest extends QpidJmsTestCase { } @Test(timeout = 5000) + public void testCreateDurableTopicSubscriberFailsIfConnectionDoesntHaveExplicitClientID() throws Exception { + try (TestAmqpPeer testPeer = new TestAmqpPeer();) { + // Create a connection without an explicit clientId + Connection connection = testFixture.establishConnecton(testPeer, null, null, null, false); + connection.start(); + + testPeer.expectBegin(true); + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + + String topicName = "myTopic"; + Topic dest = session.createTopic(topicName); + String subscriptionName = "mySubscription"; + + try { + // Verify this fails, a clientID is required and only one chosen by the application makes sense + session.createDurableSubscriber(dest, subscriptionName); + fail("expected exception to be thrown due to lack of explicit clientID"); + } catch(IllegalStateException ise) { + // Expected + } + + testPeer.waitForAllHandlersToComplete(1000); + } + } + + @Test(timeout = 5000) public void testCloseDurableTopicSubscriberDetachesWithCloseFalse() throws Exception { try (TestAmqpPeer testPeer = new TestAmqpPeer();) { Connection connection = testFixture.establishConnecton(testPeer); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org
