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/07 14:38:06 UTC

[2/4] git commit: update group sequence handling to match mapping doc, add separate method to clear group-sequence field without setting it to 0

update group sequence handling to match mapping doc, add separate method to clear group-sequence field without setting it to 0


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

Branch: refs/heads/master
Commit: 9a5bee310fd0f411ead4a4fc5090c542d855d5d0
Parents: cbf2464
Author: Robert Gemmell <ro...@apache.org>
Authored: Tue Oct 7 12:54:52 2014 +0100
Committer: Robert Gemmell <ro...@apache.org>
Committed: Tue Oct 7 12:54:52 2014 +0100

----------------------------------------------------------------------
 .../jms/message/facade/JmsMessageFacade.java    |   5 +
 .../amqp/message/AmqpJmsMessageFacade.java      |  17 ++-
 .../defaults/JmsDefaultMessageFacade.java       |   5 +
 .../amqp/message/AmqpJmsMessageFacadeTest.java  | 115 +++++++++++++++----
 4 files changed, 114 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/9a5bee31/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
index 01be4d6..e442c62 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/facade/JmsMessageFacade.java
@@ -416,4 +416,9 @@ public interface JmsMessageFacade {
      *        the group sequence value to assign this message.
      */
     void setGroupSequence(int groupSequence);
+
+    /**
+     * Clears the group sequence value for this message.
+     */
+    void clearGroupSequence();
 }

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/9a5bee31/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 c05f159..be350c0 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
@@ -713,9 +713,10 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
     @Override
     public int getGroupSequence() {
         if (message.getProperties() != null) {
-            UnsignedInteger sequence = message.getProperties().getGroupSequence();
-            if (sequence != null) {
-                return sequence.intValue();
+            UnsignedInteger groupSeqUint = message.getProperties().getGroupSequence();
+            if (groupSeqUint != null) {
+                // This wraps it into the negative int range if uint is over 2^31-1
+                return groupSeqUint.intValue();
             }
         }
 
@@ -724,10 +725,14 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
 
     @Override
     public void setGroupSequence(int groupSequence) {
-        if (groupSequence < 0 && message.getProperties() != null) {
+        // This wraps it into the upper uint range if a negative was provided
+        message.setGroupSequence(groupSequence);
+    }
+
+    @Override
+    public void clearGroupSequence() {
+        if (message.getProperties() != null) {
             message.getProperties().setGroupSequence(null);
-        } else if (groupSequence > 0) {
-            message.setGroupSequence(groupSequence);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/9a5bee31/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/defaults/JmsDefaultMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/defaults/JmsDefaultMessageFacade.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/defaults/JmsDefaultMessageFacade.java
index 2b03987..547a6bc 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/defaults/JmsDefaultMessageFacade.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/defaults/JmsDefaultMessageFacade.java
@@ -337,6 +337,11 @@ public class JmsDefaultMessageFacade implements JmsMessageFacade {
         this.groupSequence = groupSequence;
     }
 
+    @Override
+    public void clearGroupSequence() {
+        this.groupSequence = 0;
+    }
+
     private void lazyCreateProperties() {
         if (properties == null) {
             properties = new HashMap<String, Object>();

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/9a5bee31/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
index fe3ab20..0dfbc25 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsMessageFacadeTest.java
@@ -60,6 +60,7 @@ import org.mockito.Mockito;
 
 public class AmqpJmsMessageFacadeTest {
 
+    private static final long MAX_UINT = 0xFFFFFFFFL;
     private final JmsDestination consumerDestination = new JmsTopic("TestTopic");
 
     private AmqpJmsMessageFacade createNewMessageFacade() {
@@ -407,8 +408,7 @@ public class AmqpJmsMessageFacadeTest {
     // --- group-id field ---
 
     @Test
-    public void testGetGroupIdIsNullForNewMessage()
-    {
+    public void testGetGroupIdIsNullForNewMessage() {
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
 
         assertNull("expected GroupId to be null on new message", amqpMessageFacade.getGroupId());
@@ -420,8 +420,7 @@ public class AmqpJmsMessageFacadeTest {
      * as tested by {@link #testNewMessageHasNoUnderlyingPropertiesSection()}.
      */
     @Test
-    public void testSetGroupIdNullOnNewMessageDoesNotCreatePropertiesSection() throws Exception
-    {
+    public void testSetGroupIdNullOnNewMessageDoesNotCreatePropertiesSection() throws Exception {
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
 
         amqpMessageFacade.setGroupId(null);
@@ -435,8 +434,7 @@ public class AmqpJmsMessageFacadeTest {
      * as tested by {@link #testNewMessageHasNoUnderlyingPropertiesSection()}.
      */
     @Test
-    public void testSetGroupIdOnNewMessage() throws Exception
-    {
+    public void testSetGroupIdOnNewMessage() throws Exception {
         String groupId = "testValue";
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
 
@@ -452,8 +450,7 @@ public class AmqpJmsMessageFacadeTest {
      * Check that setting UserId null on the message causes any existing value to be cleared
      */
     @Test
-    public void testSetGroupIdNullOnMessageWithExistingGroupId() throws Exception
-    {
+    public void testSetGroupIdNullOnMessageWithExistingGroupId() throws Exception {
         String groupId = "testValue";
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
 
@@ -472,8 +469,7 @@ public class AmqpJmsMessageFacadeTest {
      * as tested by {@link #testNewMessageHasNoUnderlyingPropertiesSection()}.
      */
     @Test
-    public void testGetReplyToGroupIdIsNullForNewMessage()
-    {
+    public void testGetReplyToGroupIdIsNullForNewMessage() {
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
 
         assertNull("expected ReplyToGroupId to be null on new message", amqpMessageFacade.getReplyToGroupId());
@@ -483,8 +479,7 @@ public class AmqpJmsMessageFacadeTest {
      * Check that getting the ReplyToGroupId works on received messages without a properties section
      */
     @Test
-    public void testGetReplyToGroupIdWithReceivedMessageWithNoProperties()
-    {
+    public void testGetReplyToGroupIdWithReceivedMessageWithNoProperties() {
         Message message = Proton.message();
         AmqpJmsMessageFacade amqpMessageFacade = createReceivedMessageFacade(createMockAmqpConsumer(), message);
 
@@ -498,8 +493,7 @@ public class AmqpJmsMessageFacadeTest {
      * as tested by {@link #testNewMessageHasNoUnderlyingPropertiesSection()}.
      */
     @Test
-    public void testSetReplyToGroupIdNullOnNewMessageDoesNotCreatePropertiesSection() throws Exception
-    {
+    public void testSetReplyToGroupIdNullOnNewMessageDoesNotCreatePropertiesSection() throws Exception {
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
 
         amqpMessageFacade.setReplyToGroupId(null);
@@ -512,8 +506,7 @@ public class AmqpJmsMessageFacadeTest {
      * properties section, but no reply-to-group-id
      */
     @Test
-    public void testGetReplyToGroupIdWithReceivedMessageWithPropertiesButNoReplyToGroupId()
-    {
+    public void testGetReplyToGroupIdWithReceivedMessageWithPropertiesButNoReplyToGroupId() {
         Message message = Proton.message();
 
         Properties props = new Properties();
@@ -531,8 +524,7 @@ public class AmqpJmsMessageFacadeTest {
      * received messages with a reply-to-group-id.
      */
     @Test
-    public void testGetReplyToGroupIdWithReceivedMessage()
-    {
+    public void testGetReplyToGroupIdWithReceivedMessage() {
         String replyToGroupId = "myReplyGroup";
 
         Message message = Proton.message();
@@ -553,8 +545,7 @@ public class AmqpJmsMessageFacadeTest {
      * reply-to-group-id of the underlying proton message.
      */
     @Test
-    public void testSetReplyToGroupId()
-    {
+    public void testSetReplyToGroupId() {
         String replyToGroupId = "myReplyGroup";
 
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
@@ -571,8 +562,7 @@ public class AmqpJmsMessageFacadeTest {
      * Test that setting and getting the ReplyToGroupId yields the expected result
      */
     @Test
-    public void testSetGetReplyToGroupId()
-    {
+    public void testSetGetReplyToGroupId() {
         String replyToGroupId = "myReplyGroup";
 
         AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
@@ -585,6 +575,87 @@ public class AmqpJmsMessageFacadeTest {
         assertEquals("expected ReplyToGroupId on message was not found", replyToGroupId, amqpMessageFacade.getReplyToGroupId());
     }
 
+    //TODO: marker
+    // --- group-sequence field ---
+
+    @Test
+    public void testSetGetGroupSequenceOnNewMessage() {
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        int groupSequence = 5;
+        amqpMessageFacade.setGroupSequence(groupSequence);
+
+        assertEquals("underlying message should have groupSequence field value", groupSequence, amqpMessageFacade.getAmqpMessage().getProperties().getGroupSequence().longValue());
+        assertEquals("GroupSequence not as expected", groupSequence, amqpMessageFacade.getGroupSequence());
+    }
+
+    /**
+     * Tests handling of negative values set for group sequence. Negative values are used to map
+     * a value into the upper half of the unsigned int range supported by AMQP group-sequence
+     * field by utilising all of the bits of the signed int value. That is, Integer.MIN_VALUE maps
+     * to the uint value 2^31 and -1 maps to the maximum uint value 2^32-1.
+     */
+    @Test
+    public void testSetGroupSequenceNegativeMapsToUnsignedIntValueOnUnderlyingMessage() {
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        int delta = 9;
+        UnsignedInteger mapped = UnsignedInteger.valueOf(MAX_UINT - delta);
+        amqpMessageFacade.setGroupSequence(-1 - delta);
+
+        assertEquals("underlying message should have no groupSequence field value",mapped, amqpMessageFacade.getAmqpMessage().getProperties().getGroupSequence());
+        assertEquals("GroupSequence not as expected", -1 - delta, amqpMessageFacade.getGroupSequence());
+    }
+
+    @Test
+    public void testGetGroupSequenceOnReceivedMessageWithGroupSequenceJustAboveSignedIntRange() {
+        Message message = Proton.message();
+
+        Properties props = new Properties();
+        props.setGroupSequence(UnsignedInteger.valueOf(1L + Integer.MAX_VALUE));
+        message.setProperties(props);
+
+        AmqpJmsMessageFacade amqpMessageFacade = createReceivedMessageFacade(createMockAmqpConsumer(), message);
+
+        // The unsigned int value >= 2^31 will be represented as a negative, and so should begin from minimum signed int value
+        assertEquals("GroupSequence not as expected", Integer.MIN_VALUE, amqpMessageFacade.getGroupSequence());
+    }
+
+    @Test
+    public void testGetGroupSequenceOnReceivedMessageWithGroupSequenceMaxUnsignedIntValue() {
+        Message message = Proton.message();
+
+        Properties props = new Properties();
+        props.setGroupSequence(UnsignedInteger.valueOf(MAX_UINT));
+        message.setProperties(props);
+
+        AmqpJmsMessageFacade amqpMessageFacade = createReceivedMessageFacade(createMockAmqpConsumer(), message);
+
+        // The unsigned int value 2^32-1 will be represented as a negative, and should be the largest such value, -1
+        assertEquals("GroupSequence not as expected", -1, amqpMessageFacade.getGroupSequence());
+    }
+
+    @Test
+    public void testClearGroupSequenceOnMessageWithExistingGroupSequence() {
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+
+        amqpMessageFacade.setGroupSequence(5);
+
+        amqpMessageFacade.clearGroupSequence();
+
+        assertNull("underlying message should have no groupSequence field value", amqpMessageFacade.getAmqpMessage().getProperties().getGroupSequence());
+        assertEquals("GroupSequence should be 0", 0, amqpMessageFacade.getGroupSequence());
+    }
+
+    @Test
+    public void testClearGroupSequenceOnMessageWithoutExistingGroupSequence() {
+        AmqpJmsMessageFacade amqpMessageFacade = createNewMessageFacade();
+        amqpMessageFacade.clearGroupSequence();
+
+        assertNull("underlying message should still have no properties setion", amqpMessageFacade.getAmqpMessage().getProperties());
+        assertEquals("GroupSequence should be 0", 0, amqpMessageFacade.getGroupSequence());
+    }
+
     // --- message-id and correlation-id ---
 
     @Test


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