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 2015/10/08 16:40:57 UTC

qpid-jms git commit: QPIDJMS-105: consolidate the handling a little more, tidy up to make things clearer

Repository: qpid-jms
Updated Branches:
  refs/heads/master f6e5460e3 -> 7bdd5361a


QPIDJMS-105: consolidate the handling a little more, tidy up to make things clearer


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

Branch: refs/heads/master
Commit: 7bdd5361acad873fa99dbbf228bf1863db98442a
Parents: f6e5460
Author: Robert Gemmell <ro...@apache.org>
Authored: Thu Oct 8 15:35:50 2015 +0100
Committer: Robert Gemmell <ro...@apache.org>
Committed: Thu Oct 8 15:35:50 2015 +0100

----------------------------------------------------------------------
 .../java/org/apache/qpid/jms/JmsSession.java    | 12 +++----
 .../org/apache/qpid/jms/message/JmsMessage.java |  6 ++--
 .../qpid/jms/message/JmsStreamMessage.java      |  4 +--
 .../jms/message/facade/JmsMessageFacade.java    |  5 +--
 .../amqp/message/AmqpJmsBytesMessageFacade.java |  4 +--
 .../amqp/message/AmqpJmsMessageFacade.java      |  8 +----
 .../message/AmqpJmsObjectMessageFacade.java     |  4 +--
 .../ForeignMessageIntegrationTest.java          | 35 ++++++++++++++++++++
 .../apache/qpid/jms/message/JmsMessageTest.java |  2 +-
 .../facade/test/JmsTestMessageFacade.java       |  2 +-
 .../amqp/message/AmqpJmsMessageFacadeTest.java  | 14 ++------
 .../message/AmqpJmsObjectMessageFacadeTest.java |  2 +-
 12 files changed, 56 insertions(+), 42 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/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 8a6c51c..a6765a0 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
@@ -726,6 +726,10 @@ public class JmsSession implements Session, QueueSession, TopicSession, JmsMessa
                 copy = jmsMessage.copy();
             } else {
                 copy = JmsMessageTransformation.transformMessage(connection, original);
+                copy.getFacade().setProviderMessageIdObject(messageId);
+                // If the original was a foreign message, we still need to update it
+                // with the properly encoded Message ID String, get it from the copy.
+                original.setJMSMessageID(copy.getJMSMessageID());
             }
 
             // Update the JmsMessage based copy with the required values.
@@ -735,13 +739,7 @@ public class JmsSession implements Session, QueueSession, TopicSession, JmsMessa
             boolean sync = connection.isAlwaysSyncSend() ||
                            (!connection.isForceAsyncSend() && deliveryMode == DeliveryMode.PERSISTENT && !getTransacted());
 
-            copy.onSend(messageId, timeToLive);
-
-            if(!isJmsMessage) {
-                // If the original was a foreign message, we still need to update it
-                // with the properly encoded Message ID String, get it from the copy.
-                original.setJMSMessageID(copy.getJMSMessageID());
-            }
+            copy.onSend(timeToLive);
 
             JmsOutboundMessageDispatch envelope = new JmsOutboundMessageDispatch();
             envelope.setMessage(copy);

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
index 01ec217..af58312 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsMessage.java
@@ -482,17 +482,15 @@ public class JmsMessage implements javax.jms.Message {
      * ready to be written to the wire.  This processing can include such tasks as ensuring
      * that the proper message headers are set or compressing message bodies etc.
      *
-     * @param messageId
-     *        a hint that the user requested not to sent the MessageID if possible.
      * @param producerTtl
      *        the time to live value that the producer was configured with at send time.
      *
      * @throws JMSException if an error occurs while preparing the message for send.
      */
-    public void onSend(Object messageId, long producerTtl) throws JMSException {
+    public void onSend(long producerTtl) throws JMSException {
         setReadOnlyBody(true);
         setReadOnlyProperties(true);
-        facade.onSend(messageId, producerTtl);
+        facade.onSend(producerTtl);
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsStreamMessage.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsStreamMessage.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsStreamMessage.java
index 1590caa..7230e75 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsStreamMessage.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/message/JmsStreamMessage.java
@@ -47,8 +47,8 @@ public class JmsStreamMessage extends JmsMessage implements StreamMessage {
     }
 
     @Override
-    public void onSend(Object messageId, long producerTtl) throws JMSException {
-        super.onSend(messageId, producerTtl);
+    public void onSend(long producerTtl) throws JMSException {
+        super.onSend(producerTtl);
         reset();
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/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 4af1b73..aead344 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
@@ -85,15 +85,12 @@ public interface JmsMessageFacade {
      * The method allows for passing through producer configuration details not
      * explicitly mapped into the JMS Message allowing the facade to create the
      * most correct and compact message on the wire.
-     *
-     * @param messageId
-     *        the message ID value to assign to the outgoing message.
      * @param producerTtl
      *        the time to live value configured on the producer when sent.
      *
      * @throws JMSException if an error occurs while preparing the message for send.
      */
-    void onSend(Object messageId, long producerTtl) throws JMSException;
+    void onSend(long producerTtl) throws JMSException;
 
     /**
      * Called before a message is dispatched to its intended consumer to allow for

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsBytesMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsBytesMessageFacade.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsBytesMessageFacade.java
index 30f5365..9ce2d5c 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsBytesMessageFacade.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsBytesMessageFacade.java
@@ -216,8 +216,8 @@ public class AmqpJmsBytesMessageFacade extends AmqpJmsMessageFacade implements J
     }
 
     @Override
-    public void onSend(Object messageId, long producerTtl) throws JMSException {
-        super.onSend(messageId, producerTtl);
+    public void onSend(long producerTtl) throws JMSException {
+        super.onSend(producerTtl);
 
         reset();
     }

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/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 a4fd26a..07eed51 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
@@ -202,7 +202,7 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
     }
 
     @Override
-    public void onSend(Object messageId, long producerTtl) throws JMSException {
+    public void onSend(long producerTtl) throws JMSException {
 
         // Set the ttl field of the Header field if needed, complementing the expiration
         // field of Properties for any peers that only inspect the mutable ttl field.
@@ -222,12 +222,6 @@ public class AmqpJmsMessageFacade implements JmsMessageFacade {
             }
         }
 
-        if (messageId instanceof String) {
-            setMessageId((String) messageId);
-        } else {
-            message.setMessageId(messageId);
-        }
-
         setMessageAnnotation(JMS_MSG_TYPE, getJmsMsgType());
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacade.java b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacade.java
index 5400819..032ad07 100644
--- a/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacade.java
+++ b/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacade.java
@@ -114,8 +114,8 @@ public class AmqpJmsObjectMessageFacade extends AmqpJmsMessageFacade implements
     }
 
     @Override
-    public void onSend(Object messageId, long producerTtl) throws JMSException {
-        super.onSend(messageId, producerTtl);
+    public void onSend(long producerTtl) throws JMSException {
+        super.onSend(producerTtl);
         delegate.onSend();
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ForeignMessageIntegrationTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ForeignMessageIntegrationTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ForeignMessageIntegrationTest.java
index b84c722..3543fc3 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ForeignMessageIntegrationTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/ForeignMessageIntegrationTest.java
@@ -20,7 +20,9 @@ package org.apache.qpid.jms.integration;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
 import javax.jms.Connection;
@@ -29,6 +31,7 @@ import javax.jms.Queue;
 import javax.jms.Session;
 
 import org.apache.qpid.jms.message.foreign.ForeignJmsBytesMessage;
+import org.apache.qpid.jms.message.foreign.ForeignJmsTextMessage;
 import org.apache.qpid.jms.provider.amqp.message.AmqpMessageSupport;
 import org.apache.qpid.jms.test.QpidJmsTestCase;
 import org.apache.qpid.jms.test.testpeer.TestAmqpPeer;
@@ -78,6 +81,38 @@ public class ForeignMessageIntegrationTest extends QpidJmsTestCase {
         }
     }
 
+    @Test(timeout = 20000)
+    public void testSentForeignMessageHasMessageId() throws Exception {
+        try (TestAmqpPeer testPeer = new TestAmqpPeer();) {
+            Connection connection = testFixture.establishConnecton(testPeer);
+            testPeer.expectBegin();
+            testPeer.expectSenderAttach();
+
+            Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+            Queue queue = session.createQueue("myQueue");
+            MessageProducer producer = session.createProducer(queue);
+
+            MessageHeaderSectionMatcher headersMatcher = new MessageHeaderSectionMatcher(true).withDurable(equalTo(true));
+            MessageAnnotationsSectionMatcher msgAnnotationsMatcher = new MessageAnnotationsSectionMatcher(true);
+            MessagePropertiesSectionMatcher propertiesMatcher = new MessagePropertiesSectionMatcher(true);
+            propertiesMatcher.withMessageId(notNullValue(String.class));
+            TransferPayloadCompositeMatcher messageMatcher = new TransferPayloadCompositeMatcher();
+            messageMatcher.setHeadersMatcher(headersMatcher);
+            messageMatcher.setMessageAnnotationsMatcher(msgAnnotationsMatcher);
+            messageMatcher.setPropertiesMatcher(propertiesMatcher);
+
+            testPeer.expectTransfer(messageMatcher);
+
+            ForeignJmsTextMessage foreign = new ForeignJmsTextMessage();
+
+            producer.send(foreign);
+
+            assertNotNull("JMSMessageID should not be null", foreign.getJMSMessageID());
+
+            testPeer.waitForAllHandlersToComplete(2000);
+        }
+    }
+
     /**
      * Test that after sending a message with the disableMessageID hint set, which already had
      * a JMSMessageID value, that the message object then has a null JMSMessageID value, and no

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
index 02f2a36..2726810 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/JmsMessageTest.java
@@ -88,7 +88,7 @@ public class JmsMessageTest {
         JmsMessage msg = factory.createMessage();
         assertFalse(msg.isReadOnlyBody());
         assertFalse(msg.isReadOnlyProperties());
-        msg.onSend(null, 0);
+        msg.onSend(0);
         assertTrue(msg.isReadOnlyBody());
         assertTrue(msg.isReadOnlyProperties());
     }

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
index fb53aae..b08d362 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/message/facade/test/JmsTestMessageFacade.java
@@ -128,7 +128,7 @@ public class JmsTestMessageFacade implements JmsMessageFacade {
     }
 
     @Override
-    public void onSend(Object messageId, long producerTtl) throws JMSException {
+    public void onSend(long producerTtl) throws JMSException {
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/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 6e3348b..690e407 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
@@ -160,7 +160,7 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
 
         assertEquals("TTL has been unset already", origTtl, message.getTtl());
 
-        amqpMessageFacade.onSend(null, 0);
+        amqpMessageFacade.onSend(0);
 
         // check value on underlying TTL field is NOT set
         assertEquals("TTL has not been cleared", 0, message.getTtl());
@@ -178,7 +178,7 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
 
         assertEquals("TTL has been unset already", origTtl, message.getTtl());
 
-        amqpMessageFacade.onSend(null, newTtl);
+        amqpMessageFacade.onSend(newTtl);
 
         // check value on underlying TTL field is NOT set
         assertEquals("TTL has not been overriden", newTtl, message.getTtl());
@@ -194,7 +194,7 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
         AmqpJmsMessageFacade amqpMessageFacade = createReceivedMessageFacade(createMockAmqpConsumer(), message);
         amqpMessageFacade.setAmqpTimeToLiveOverride((long) overrideTtl);
 
-        amqpMessageFacade.onSend(null, producerTtl);
+        amqpMessageFacade.onSend(producerTtl);
 
         // check value on underlying TTL field is set to the override
         assertEquals("TTL has not been overriden", overrideTtl, message.getTtl());
@@ -1848,14 +1848,6 @@ public class AmqpJmsMessageFacadeTest extends AmqpJmsMessageTypesTestCase  {
     // ===============================================
 
     @Test
-    public void testOnSendWithNullMessageIdClearsMessageID() throws JMSException {
-        Message message = Mockito.mock(Message.class);
-        JmsMessageFacade amqpMessageFacade = createReceivedMessageFacade(createMockAmqpConsumer(), message);
-        amqpMessageFacade.onSend(null, 0);
-        Mockito.verify(message).setMessageId(null);
-    }
-
-    @Test
     public void testClearBodyRemoveMessageBody() {
         Message message = Mockito.mock(Message.class);
         JmsMessageFacade amqpMessageFacade = createReceivedMessageFacade(createMockAmqpConsumer(), message);

http://git-wip-us.apache.org/repos/asf/qpid-jms/blob/7bdd5361/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacadeTest.java
----------------------------------------------------------------------
diff --git a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacadeTest.java b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacadeTest.java
index 984c5ff..439957e 100644
--- a/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacadeTest.java
+++ b/qpid-jms-client/src/test/java/org/apache/qpid/jms/provider/amqp/message/AmqpJmsObjectMessageFacadeTest.java
@@ -97,7 +97,7 @@ public class AmqpJmsObjectMessageFacadeTest extends AmqpJmsMessageTypesTestCase
 
     private void doNewMessageToSendHasBodySectionRepresentingNull(boolean amqpTyped) throws Exception {
         AmqpJmsObjectMessageFacade amqpObjectMessageFacade = createNewObjectMessageFacade(amqpTyped);
-        amqpObjectMessageFacade.onSend(null, 0);
+        amqpObjectMessageFacade.onSend(0);
 
         Message protonMessage = amqpObjectMessageFacade.getAmqpMessage();
         assertNotNull("Message body should be presents", protonMessage.getBody());


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