You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ro...@apache.org on 2014/10/16 17:56:17 UTC

[1/4] git commit: add support for sending+recieving byte encoded destination type info, disabled on outbound pending a final toggle

Repository: qpid-jms
Updated Branches:
  refs/heads/master 7525ab93e -> 437a74fb9


add support for sending+recieving byte encoded destination type info, disabled on outbound pending a final toggle


Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/437a74fb
Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/437a74fb
Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/437a74fb

Branch: refs/heads/master
Commit: 437a74fb98ce172e1b8515a84416469e02d403b4
Parents: 8b96bd5
Author: Robert Gemmell <ro...@apache.org>
Authored: Thu Oct 16 16:51:41 2014 +0100
Committer: Robert Gemmell <ro...@apache.org>
Committed: Thu Oct 16 16:55:32 2014 +0100

----------------------------------------------------------------------
 .../amqp/message/AmqpDestinationHelper.java     | 164 +++++++++++--------
 .../amqp/message/AmqpJmsMessageFacade.java      |   4 +-
 .../jms/integration/MessageIntegrationTest.java |  86 +++++-----
 .../amqp/message/AmqpDestinationHelperTest.java |  46 +++---
 4 files changed, 169 insertions(+), 131 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/437a74fb/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelper.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelper.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelper.java
index d492108..b3654a5 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelper.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelper.java
@@ -37,6 +37,13 @@ public class AmqpDestinationHelper {
     public static final String TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME = "x-opt-to-type";
     public static final String REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME = "x-opt-reply-type";
 
+    // For support of current byte type values
+    public static final byte QUEUE_TYPE = 0x00;
+    public static final byte TOPIC_TYPE = 0x01;
+    public static final byte TEMP_QUEUE_TYPE = 0x02;
+    public static final byte TEMP_TOPIC_TYPE = 0x03;
+
+    // For support of old string type values
     static final String QUEUE_ATTRIBUTE = "queue";
     static final String TOPIC_ATTRIBUTE = "topic";
     static final String TEMPORARY_ATTRIBUTE = "temporary";
@@ -46,18 +53,6 @@ public class AmqpDestinationHelper {
     public static final String TEMP_QUEUE_ATTRIBUTES_STRING = QUEUE_ATTRIBUTE + "," + TEMPORARY_ATTRIBUTE;
     public static final String TEMP_TOPIC_ATTRIBUTES_STRING = TOPIC_ATTRIBUTE + "," + TEMPORARY_ATTRIBUTE;
 
-    // TODO - The Type Annotation seems like it could just be a byte value
-
-    /*
-     *  One possible way to encode destination types that isn't a string.
-     *
-     *  public static final byte QUEUE_TYPE = 0x01;
-     *  public static final byte TOPIC_TYPE = 0x02;
-     *  public static final byte TEMP_MASK = 0x04;
-     *  public static final byte TEMP_TOPIC_TYPE = TOPIC_TYPE | TEMP_MASK;
-     *  public static final byte TEMP_QUEUE_TYPE = QUEUE_TYPE | TEMP_MASK;
-     */
-
     /**
      * Given a destination name string, create a JmsDestination object based on the
      * configured destination prefix values.  If no prefix values are configured or the
@@ -95,56 +90,42 @@ public class AmqpDestinationHelper {
      *
      * If an address and type description is provided then this will be used to
      * create the Destination. If the type information is missing, it will be
-     * derived from the consumer destination if present, or default to a generic
+     * derived from the consumer destination if present, or default to a queue
      * destination if not.
      *
      * If the address is null then the consumer destination is returned, unless
      * the useConsumerDestForTypeOnly flag is true, in which case null will be
      * returned.
      */
-
     public JmsDestination getJmsDestination(AmqpJmsMessageFacade message, JmsDestination consumerDestination) {
         String to = message.getToAddress();
-        String toTypeString = (String) message.getMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
-        Set<String> typeSet = null;
+        Byte typeByte = getTypeByte(message, TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
 
-        if (toTypeString != null) {
-            typeSet = splitAttributes(toTypeString);
-        }
-
-        return createDestination(to, typeSet, consumerDestination, false);
+        return createDestination(to, typeByte, consumerDestination, false);
     }
 
     public JmsDestination getJmsReplyTo(AmqpJmsMessageFacade message, JmsDestination consumerDestination) {
         String replyTo = message.getReplyToAddress();
-        String replyToTypeString = (String) message.getMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
-        Set<String> typeSet = null;
+        Byte typeByte = getTypeByte(message, REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
 
-        if (replyToTypeString != null) {
-            typeSet = splitAttributes(replyToTypeString);
-        }
-
-        return createDestination(replyTo, typeSet, consumerDestination, true);
+        return createDestination(replyTo, typeByte, consumerDestination, true);
     }
 
-    private JmsDestination createDestination(String address, Set<String> typeSet, JmsDestination consumerDestination, boolean useConsumerDestForTypeOnly) {
+    private JmsDestination createDestination(String address, Byte typeByte, JmsDestination consumerDestination, boolean useConsumerDestForTypeOnly) {
         if (address == null) {
             return useConsumerDestForTypeOnly ? null : consumerDestination;
         }
 
-        if (typeSet != null && !typeSet.isEmpty()) {
-            if (typeSet.contains(QUEUE_ATTRIBUTE)) {
-                if (typeSet.contains(TEMPORARY_ATTRIBUTE)) {
-                    return new JmsTemporaryQueue(address);
-                } else {
-                    return new JmsQueue(address);
-                }
-            } else if (typeSet.contains(TOPIC_ATTRIBUTE)) {
-                if (typeSet.contains(TEMPORARY_ATTRIBUTE)) {
-                    return new JmsTemporaryTopic(address);
-                } else {
-                    return new JmsTopic(address);
-                }
+        if (typeByte != null) {
+            switch (typeByte) {
+            case QUEUE_TYPE:
+                return new JmsQueue(address);
+            case TOPIC_TYPE:
+                return new JmsTopic(address);
+            case TEMP_QUEUE_TYPE:
+                return new JmsTemporaryQueue(address);
+            case TEMP_TOPIC_TYPE:
+                return new JmsTemporaryTopic(address);
             }
         }
 
@@ -166,59 +147,75 @@ public class AmqpDestinationHelper {
         return new JmsQueue(address);
     }
 
-    public void setToAddressFromDestination(AmqpJmsMessageFacade message, JmsDestination destination) {
+    public void setToAddressFromDestination(AmqpJmsMessageFacade message, JmsDestination destination, boolean useByteValue) {
         String address = destination != null ? destination.getName() : null;
-        String typeString = toTypeAnnotation(destination);
+        Object typeValue = toTypeAnnotation(destination, useByteValue);
 
         message.setToAddress(address);
 
-        if (address == null || typeString == null) {
+        if (address == null || typeValue == null) {
             message.removeMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
         } else {
-            message.setMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, typeString);
+            message.setMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, typeValue);
         }
     }
 
-    public void setReplyToAddressFromDestination(AmqpJmsMessageFacade message, JmsDestination destination) {
+    public void setReplyToAddressFromDestination(AmqpJmsMessageFacade message, JmsDestination destination, boolean useByteValue) {
         String replyToAddress = destination != null ? destination.getName() : null;
-        String typeString = toTypeAnnotation(destination);
+        Object typeValue = toTypeAnnotation(destination, useByteValue);
 
         message.setReplyToAddress(replyToAddress);
 
-        if (replyToAddress == null || typeString == null) {
+        if (replyToAddress == null || typeValue == null) {
             message.removeMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
         } else {
-            message.setMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, typeString);
+            message.setMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, typeValue);
         }
     }
 
     /**
-     * @return the annotation type string, or null if the supplied destination
+     * @return the annotation type value, or null if the supplied destination
      *         is null or can't be classified
      */
-    private String toTypeAnnotation(JmsDestination destination) {
+    private Object toTypeAnnotation(JmsDestination destination, boolean useByteValue) {
         if (destination == null) {
             return null;
         }
 
-        if (destination.isQueue()) {
-            if (destination.isTemporary()) {
-                return TEMP_QUEUE_ATTRIBUTES_STRING;
-            } else {
-                return QUEUE_ATTRIBUTES_STRING;
+        if(useByteValue)
+        {
+            if (destination.isQueue()) {
+                if (destination.isTemporary()) {
+                    return TEMP_QUEUE_TYPE;
+                } else {
+                    return QUEUE_TYPE;
+                }
+            } else if (destination.isTopic()) {
+                if (destination.isTemporary()) {
+                    return TEMP_TOPIC_TYPE;
+                } else {
+                    return TOPIC_TYPE;
+                }
             }
-        } else if (destination.isTopic()) {
-            if (destination.isTemporary()) {
-                return TEMP_TOPIC_ATTRIBUTES_STRING;
-            } else {
-                return TOPIC_ATTRIBUTES_STRING;
+        } else {
+            if (destination.isQueue()) {
+                if (destination.isTemporary()) {
+                    return TEMP_QUEUE_ATTRIBUTES_STRING;
+                } else {
+                    return QUEUE_ATTRIBUTES_STRING;
+                }
+            } else if (destination.isTopic()) {
+                if (destination.isTemporary()) {
+                    return TEMP_TOPIC_ATTRIBUTES_STRING;
+                } else {
+                    return TOPIC_ATTRIBUTES_STRING;
+                }
             }
         }
-
         return null;
     }
 
-    public Set<String> splitAttributes(String typeString) {
+    Set<String> splitAttributesString(String typeString) {
         if (typeString == null) {
             return null;
         }
@@ -235,4 +232,43 @@ public class AmqpDestinationHelper {
 
         return typeSet;
     }
+
+    private Byte getTypeByte(AmqpJmsMessageFacade message, String annotationName) {
+        Object typeAnnotation = message.getMessageAnnotation(annotationName);
+
+        if (typeAnnotation == null) {
+            // Doesn't exist, or null.
+            return null;
+        } else if (typeAnnotation instanceof Byte) {
+            // Return the value found.
+            return (Byte) typeAnnotation;
+        } else {
+            // Handle legacy strings.
+            String typeString = String.valueOf(typeAnnotation);
+            Set<String> typeSet = null;
+
+            if (typeString != null) {
+                typeSet = splitAttributesString(typeString);
+            }
+
+            if (typeSet != null && !typeSet.isEmpty()) {
+                if (typeSet.contains(QUEUE_ATTRIBUTE)) {
+                    if (typeSet.contains(TEMPORARY_ATTRIBUTE)) {
+                        return TEMP_QUEUE_TYPE;
+                    } else {
+                        return QUEUE_TYPE;
+                    }
+                } else if (typeSet.contains(TOPIC_ATTRIBUTE)) {
+                    if (typeSet.contains(TEMPORARY_ATTRIBUTE)) {
+                        return TEMP_TOPIC_TYPE;
+                    } else {
+                        return TOPIC_TYPE;
+                    }
+                }
+            }
+
+            return null;
+        }
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/437a74fb/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
index c34ab87..8a6a20b 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacade.java
@@ -657,7 +657,7 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
     public void setDestination(JmsDestination destination) {
         this.destination = destination;
         lazyCreateMessageAnnotations();
-        AmqpDestinationHelper.INSTANCE.setToAddressFromDestination(this, destination);
+        AmqpDestinationHelper.INSTANCE.setToAddressFromDestination(this, destination, false);
     }
 
     @Override
@@ -678,7 +678,7 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
     public void setReplyTo(JmsDestination replyTo) {
         this.replyTo = replyTo;
         lazyCreateMessageAnnotations();
-        AmqpDestinationHelper.INSTANCE.setReplyToAddressFromDestination(this, replyTo);
+        AmqpDestinationHelper.INSTANCE.setReplyToAddressFromDestination(this, replyTo, false);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/437a74fb/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
index 24ecc64..98c79a7 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
@@ -203,48 +203,6 @@ public class MessageIntegrationTest extends QpidJmsTestCase
     //==============================
 
     /**
-     * Tests that the {@link AmqpMessageSupport#AMQP_TO_ANNOTATION} set on a message to
-     * indicate its 'to' address represents a Topic results in the JMSDestination object being a
-     * Topic. Ensure the consumers destination is not used by consuming from a Queue.
-     */
-    @Test(timeout = 2000)
-    public void testReceivedMessageFromQueueWithToTypeAnnotationForTopic() throws Exception {
-        try (TestAmqpPeer testPeer = new TestAmqpPeer(IntegrationTestFixture.PORT);) {
-            Connection connection = testFixture.establishConnecton(testPeer);
-            connection.start();
-
-            testPeer.expectBegin(true);
-
-            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
-            Queue queue = session.createQueue("myQueue");
-
-            MessageAnnotationsDescribedType msgAnnotations = new MessageAnnotationsDescribedType();
-            msgAnnotations.setSymbolKeyedAnnotation(AmqpMessageSupport.AMQP_TO_ANNOTATION, AmqpMessageSupport.TOPIC_ATTRIBUTES);
-
-            PropertiesDescribedType props = new PropertiesDescribedType();
-            String myTopicAddress = "myTopicAddress";
-            props.setTo(myTopicAddress );
-            props.setMessageId("myMessageIDString");
-            DescribedType amqpValueNullContent = new AmqpValueDescribedType(null);
-
-            testPeer.expectReceiverAttach();
-            testPeer.expectLinkFlowRespondWithTransfer(null, msgAnnotations, props, null, amqpValueNullContent);
-            testPeer.expectDispositionThatIsAcceptedAndSettled();
-
-            MessageConsumer messageConsumer = session.createConsumer(queue);
-            Message receivedMessage = messageConsumer.receive(1000);
-            testPeer.waitForAllHandlersToComplete(3000);
-
-            assertNotNull(receivedMessage);
-
-            Destination dest = receivedMessage.getJMSDestination();
-            assertNotNull("Expected Topic destination but got null", dest);
-            assertTrue("Expected Topic instance but did not get one. Actual type was: " + dest.getClass().getName(), dest instanceof Topic);
-            assertEquals(myTopicAddress, ((Topic)dest).getTopicName());
-        }
-    }
-
-    /**
      * Tests that the lack of a 'to' in the Properties section of the incoming message (e.g
      * one sent by a non-JMS client) is handled by making the JMSDestination method simply
      * return the Queue Destination used to create the consumer that received the message.
@@ -310,6 +268,50 @@ public class MessageIntegrationTest extends QpidJmsTestCase
         }
     }
 
+    // --- old string values --- //
+
+    /**
+     * Tests that the {@link AmqpMessageSupport#AMQP_TO_ANNOTATION} set on a message to
+     * indicate its 'to' address represents a Topic results in the JMSDestination object being a
+     * Topic. Ensure the consumers destination is not used by consuming from a Queue.
+     */
+    @Test(timeout = 2000)
+    public void testReceivedMessageFromQueueWithToTypeAnnotationForTopic() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer(IntegrationTestFixture.PORT);) {
+            Connection connection = testFixture.establishConnecton(testPeer);
+            connection.start();
+
+            testPeer.expectBegin(true);
+
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("myQueue");
+
+            MessageAnnotationsDescribedType msgAnnotations = new MessageAnnotationsDescribedType();
+            msgAnnotations.setSymbolKeyedAnnotation(AmqpMessageSupport.AMQP_TO_ANNOTATION, AmqpMessageSupport.TOPIC_ATTRIBUTES);
+
+            PropertiesDescribedType props = new PropertiesDescribedType();
+            String myTopicAddress = "myTopicAddress";
+            props.setTo(myTopicAddress );
+            props.setMessageId("myMessageIDString");
+            DescribedType amqpValueNullContent = new AmqpValueDescribedType(null);
+
+            testPeer.expectReceiverAttach();
+            testPeer.expectLinkFlowRespondWithTransfer(null, msgAnnotations, props, null, amqpValueNullContent);
+            testPeer.expectDispositionThatIsAcceptedAndSettled();
+
+            MessageConsumer messageConsumer = session.createConsumer(queue);
+            Message receivedMessage = messageConsumer.receive(1000);
+            testPeer.waitForAllHandlersToComplete(3000);
+
+            assertNotNull(receivedMessage);
+
+            Destination dest = receivedMessage.getJMSDestination();
+            assertNotNull("Expected Topic destination but got null", dest);
+            assertTrue("Expected Topic instance but did not get one. Actual type was: " + dest.getClass().getName(), dest instanceof Topic);
+            assertEquals(myTopicAddress, ((Topic)dest).getTopicName());
+        }
+    }
+
     /**
      * Tests that the {@link AmqpMessageSupport#AMQP_REPLY_TO_ANNOTATION} set on a message to
      * indicate its 'reply-to' address represents a Topic results in the JMSReplyTo object being a

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/437a74fb/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelperTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelperTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelperTest.java
index dbf9279..4d851a0 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelperTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpDestinationHelperTest.java
@@ -523,20 +523,20 @@ public class AmqpDestinationHelperTest {
     @Test
     public void testSetToAddressFromDestinationWithNullDestination() {
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
-        helper.setToAddressFromDestination(message, null);
+        helper.setToAddressFromDestination(message, null, false);
         Mockito.verify(message).setToAddress(null);
         Mockito.verify(message).removeMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
     }
 
     @Test(expected=NullPointerException.class)
     public void testSetToAddressFromDestinationWithNullDestinationAndNullMessage() {
-        helper.setToAddressFromDestination(null, null);
+        helper.setToAddressFromDestination(null, null, false);
     }
 
     @Test(expected=NullPointerException.class)
     public void testSetToAddressFromDestinationWithNullMessage() {
         JmsDestination destination = new JmsQueue("testAddress");
-        helper.setToAddressFromDestination(null, destination);
+        helper.setToAddressFromDestination(null, destination, false);
     }
 
     @Test
@@ -545,7 +545,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsQueue("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setToAddressFromDestination(message, destination);
+        helper.setToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, QUEUE_ATTRIBUTES_STRING);
@@ -557,7 +557,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsTopic("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setToAddressFromDestination(message, destination);
+        helper.setToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, TOPIC_ATTRIBUTES_STRING);
@@ -569,7 +569,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsTemporaryQueue("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setToAddressFromDestination(message, destination);
+        helper.setToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, TEMP_QUEUE_ATTRIBUTES_STRING);
@@ -581,7 +581,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsTemporaryTopic("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setToAddressFromDestination(message, destination);
+        helper.setToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, TEMP_TOPIC_ATTRIBUTES_STRING);
@@ -594,7 +594,7 @@ public class AmqpDestinationHelperTest {
         Mockito.when(destination.getName()).thenReturn(testAddress);
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setToAddressFromDestination(message, destination);
+        helper.setToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setToAddress(testAddress);
         Mockito.verify(message).removeMessageAnnotation(TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
@@ -605,20 +605,20 @@ public class AmqpDestinationHelperTest {
     @Test
     public void testSetReplyToAddressFromDestinationWithNullDestination() {
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
-        helper.setReplyToAddressFromDestination(message, null);
+        helper.setReplyToAddressFromDestination(message, null, false);
         Mockito.verify(message).setReplyToAddress(null);
         Mockito.verify(message).removeMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
     }
 
     @Test(expected=NullPointerException.class)
     public void testSetReplyToAddressFromDestinationWithNullDestinationAndNullMessage() {
-        helper.setReplyToAddressFromDestination(null, null);
+        helper.setReplyToAddressFromDestination(null, null, false);
     }
 
     @Test(expected=NullPointerException.class)
     public void testSetReplyToAddressFromDestinationWithNullMessage() {
         JmsDestination destination = new JmsQueue("testAddress");
-        helper.setReplyToAddressFromDestination(null, destination);
+        helper.setReplyToAddressFromDestination(null, destination, false);
     }
 
     @Test
@@ -627,7 +627,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsQueue("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setReplyToAddressFromDestination(message, destination);
+        helper.setReplyToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setReplyToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, QUEUE_ATTRIBUTES_STRING);
@@ -639,7 +639,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsTopic("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setReplyToAddressFromDestination(message, destination);
+        helper.setReplyToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setReplyToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, TOPIC_ATTRIBUTES_STRING);
@@ -651,7 +651,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsTemporaryQueue("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setReplyToAddressFromDestination(message, destination);
+        helper.setReplyToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setReplyToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, TEMP_QUEUE_ATTRIBUTES_STRING);
@@ -663,7 +663,7 @@ public class AmqpDestinationHelperTest {
         JmsDestination destination = new JmsTemporaryTopic("testAddress");
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setReplyToAddressFromDestination(message, destination);
+        helper.setReplyToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setReplyToAddress(testAddress);
         Mockito.verify(message).setMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME, TEMP_TOPIC_ATTRIBUTES_STRING);
@@ -676,7 +676,7 @@ public class AmqpDestinationHelperTest {
         Mockito.when(destination.getName()).thenReturn(testAddress);
         AmqpJmsMessageFacade message = Mockito.mock(AmqpJmsMessageFacade.class);
 
-        helper.setReplyToAddressFromDestination(message, destination);
+        helper.setReplyToAddressFromDestination(message, destination, false);
 
         Mockito.verify(message).setReplyToAddress(testAddress);
         Mockito.verify(message).removeMessageAnnotation(REPLY_TO_TYPE_MSG_ANNOTATION_SYMBOL_NAME);
@@ -692,30 +692,30 @@ public class AmqpDestinationHelperTest {
         set.add(AmqpDestinationHelper.TEMPORARY_ATTRIBUTE);
 
         // test for no NPE errors.
-        assertNull(helper.splitAttributes(null));
+        assertNull(helper.splitAttributesString(null));
 
         // test a single comma separator produces expected set
-        assertEquals(set, helper.splitAttributes(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + "," +
+        assertEquals(set, helper.splitAttributesString(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + "," +
                                                  AmqpDestinationHelper.TEMPORARY_ATTRIBUTE));
 
         // test trailing comma doesn't alter produced set
-        assertEquals(set, helper.splitAttributes(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + "," +
+        assertEquals(set, helper.splitAttributesString(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + "," +
                                                  AmqpDestinationHelper.TEMPORARY_ATTRIBUTE + ","));
 
         // test leading comma doesn't alter produced set
-        assertEquals(set, helper.splitAttributes("," + AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + ","
+        assertEquals(set, helper.splitAttributesString("," + AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + ","
                                                      + AmqpDestinationHelper.TEMPORARY_ATTRIBUTE));
 
         // test consecutive central commas don't alter produced set
-        assertEquals(set, helper.splitAttributes(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + ",," +
+        assertEquals(set, helper.splitAttributesString(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + ",," +
                                                  AmqpDestinationHelper.TEMPORARY_ATTRIBUTE));
 
         // test consecutive trailing commas don't alter produced set
-        assertEquals(set, helper.splitAttributes(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + "," +
+        assertEquals(set, helper.splitAttributesString(AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + "," +
                                                  AmqpDestinationHelper.TEMPORARY_ATTRIBUTE + ",,"));
 
         // test consecutive leading commas don't alter produced set
-        assertEquals(set, helper.splitAttributes("," + AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + ","
+        assertEquals(set, helper.splitAttributesString("," + AmqpDestinationHelper.QUEUE_ATTRIBUTES_STRING + ","
                                                      + AmqpDestinationHelper.TEMPORARY_ATTRIBUTE));
     }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[2/4] git commit: add message to assertion to aid during test failure

Posted by ro...@apache.org.
add message to assertion to aid during test failure


Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/47cf6e4c
Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/47cf6e4c
Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/47cf6e4c

Branch: refs/heads/master
Commit: 47cf6e4c0431a4b0779a2576648281dbcf18690b
Parents: 6bcb554
Author: Robert Gemmell <ro...@apache.org>
Authored: Thu Oct 16 11:55:06 2014 +0100
Committer: Robert Gemmell <ro...@apache.org>
Committed: Thu Oct 16 16:55:32 2014 +0100

----------------------------------------------------------------------
 .../java/org/apache/qpid/jms/failover/FailoverProviderTest.java    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/47cf6e4c/qpid-jms-interop-tests/qpid-jms-activemq-tests/src/test/java/org/apache/qpid/jms/failover/FailoverProviderTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-interop-tests/qpid-jms-activemq-tests/src/test/java/org/apache/qpid/jms/failover/FailoverProviderTest.java b/qpid-jms-interop-tests/qpid-jms-activemq-tests/src/test/java/org/apache/qpid/jms/failover/FailoverProviderTest.java
index a458ab7..7e4e5b3 100644
--- a/qpid-jms-interop-tests/qpid-jms-activemq-tests/src/test/java/org/apache/qpid/jms/failover/FailoverProviderTest.java
+++ b/qpid-jms-interop-tests/qpid-jms-activemq-tests/src/test/java/org/apache/qpid/jms/failover/FailoverProviderTest.java
@@ -87,6 +87,6 @@ public class FailoverProviderTest extends AmqpTestSupport {
 
         provider.connect();
 
-        assertTrue(failed.await(2, TimeUnit.SECONDS));
+        assertTrue("Did not trip latch within expected time", failed.await(2, TimeUnit.SECONDS));
     }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[3/4] git commit: add note about need to check limit to prevent excess provider creation attempts

Posted by ro...@apache.org.
add note about need to check limit to prevent excess provider creation attempts


Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/8b96bd5c
Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/8b96bd5c
Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/8b96bd5c

Branch: refs/heads/master
Commit: 8b96bd5c890d5a01d526507f2e478fe8bbffc543
Parents: 47cf6e4
Author: Robert Gemmell <ro...@apache.org>
Authored: Thu Oct 16 12:20:08 2014 +0100
Committer: Robert Gemmell <ro...@apache.org>
Committed: Thu Oct 16 16:55:32 2014 +0100

----------------------------------------------------------------------
 .../org/apache/qpid/jms/provider/failover/FailoverProvider.java    | 2 ++
 1 file changed, 2 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/8b96bd5c/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/failover/FailoverProvider.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/failover/FailoverProvider.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/failover/FailoverProvider.java
index ca8a5e6..77820a6 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/failover/FailoverProvider.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/failover/FailoverProvider.java
@@ -518,6 +518,8 @@ public class FailoverProvider extends DefaultProviderListener implements Provide
                     return;
                 }
 
+                // TODO: if this is already at/past the limit when we arrive, we should
+                // stop here rather than initialise the provider and only fail (again) after.
                 reconnectAttempts++;
                 Throwable failure = null;
                 URI target = uris.getNext();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[4/4] git commit: title and organise into sections of related tests

Posted by ro...@apache.org.
title and organise into sections of related tests


Project: http://git-wip-us.apache.org/repos/asf/qpid-jms/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-jms/commit/6bcb5546
Tree: http://git-wip-us.apache.org/repos/asf/qpid-jms/tree/6bcb5546
Diff: http://git-wip-us.apache.org/repos/asf/qpid-jms/diff/6bcb5546

Branch: refs/heads/master
Commit: 6bcb55460220f74d762d288d67dbb646f82b564f
Parents: 7525ab9
Author: Robert Gemmell <ro...@apache.org>
Authored: Wed Oct 15 12:45:42 2014 +0100
Committer: Robert Gemmell <ro...@apache.org>
Committed: Thu Oct 16 16:55:32 2014 +0100

----------------------------------------------------------------------
 .../jms/integration/MessageIntegrationTest.java | 64 ++++++++++++--------
 1 file changed, 39 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/6bcb5546/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
index f46a6d0..24ecc64 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/MessageIntegrationTest.java
@@ -59,7 +59,6 @@ import org.apache.qpid.proton.amqp.DescribedType;
 import org.apache.qpid.proton.amqp.Symbol;
 import org.apache.qpid.proton.amqp.UnsignedInteger;
 import org.apache.qpid.proton.amqp.UnsignedLong;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class MessageIntegrationTest extends QpidJmsTestCase
@@ -85,6 +84,9 @@ public class MessageIntegrationTest extends QpidJmsTestCase
 
     private final IntegrationTestFixture testFixture = new IntegrationTestFixture();
 
+    //==== Application Properties Section ====
+    //========================================
+
     @Test(timeout = 2000)
     public void testSendMessageWithApplicationProperties() throws Exception {
         try (TestAmqpPeer testPeer = new TestAmqpPeer(IntegrationTestFixture.PORT);) {
@@ -197,30 +199,8 @@ public class MessageIntegrationTest extends QpidJmsTestCase
         }
     }
 
-    @Test(timeout = 2000)
-    public void testReceiveMessageWithoutMessageId() throws Exception {
-        try (TestAmqpPeer testPeer = new TestAmqpPeer(IntegrationTestFixture.PORT);) {
-            Connection connection = testFixture.establishConnecton(testPeer);
-            connection.start();
-
-            testPeer.expectBegin(true);
-
-            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
-            Queue queue = session.createQueue("myQueue");
-
-            DescribedType amqpValueNullContent = new AmqpValueDescribedType(null);
-
-            testPeer.expectReceiverAttach();
-            testPeer.expectLinkFlowRespondWithTransfer(null, null, null, null, amqpValueNullContent);
-            testPeer.expectDispositionThatIsAcceptedAndSettled();
-
-            MessageConsumer messageConsumer = session.createConsumer(queue);
-            Message receivedMessage = messageConsumer.receive(1000);
-            testPeer.waitForAllHandlersToComplete(2000);
-
-            assertNull(receivedMessage.getJMSMessageID());
-        }
-    }
+    //==== Destination Handling ====
+    //==============================
 
     /**
      * Tests that the {@link AmqpMessageSupport#AMQP_TO_ANNOTATION} set on a message to
@@ -443,6 +423,9 @@ public class MessageIntegrationTest extends QpidJmsTestCase
         }
     }
 
+    //==== TTL / Expiration Handling ====
+    //===================================
+
     /**
      * Tests that lack of the absolute-expiry-time and ttl fields on a message results
      * in it returning 0 for for JMSExpiration
@@ -512,6 +495,34 @@ public class MessageIntegrationTest extends QpidJmsTestCase
         }
     }
 
+    //==== MessageID and CorrelationID Handling ====
+    //==============================================
+
+    @Test(timeout = 2000)
+    public void testReceiveMessageWithoutMessageId() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer(IntegrationTestFixture.PORT);) {
+            Connection connection = testFixture.establishConnecton(testPeer);
+            connection.start();
+
+            testPeer.expectBegin(true);
+
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("myQueue");
+
+            DescribedType amqpValueNullContent = new AmqpValueDescribedType(null);
+
+            testPeer.expectReceiverAttach();
+            testPeer.expectLinkFlowRespondWithTransfer(null, null, null, null, amqpValueNullContent);
+            testPeer.expectDispositionThatIsAcceptedAndSettled();
+
+            MessageConsumer messageConsumer = session.createConsumer(queue);
+            Message receivedMessage = messageConsumer.receive(1000);
+            testPeer.waitForAllHandlersToComplete(2000);
+
+            assertNull(receivedMessage.getJMSMessageID());
+        }
+    }
+
     /**
      * Tests that receiving a message with a string typed message-id results in returning the
      * expected value for JMSMessageId where the JMS "ID:" prefix has been added.
@@ -860,6 +871,9 @@ public class MessageIntegrationTest extends QpidJmsTestCase
         }
     }
 
+    //==== Group Property Handling ====
+    //=================================
+
     /**
      * Tests that when receiving a message with the group-id, reply-to-group-id, and group-sequence
      * fields of the AMQP properties section set, that the expected JMSX or JMS_AMQP properties


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org