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/02/21 14:10:37 UTC

svn commit: r1570564 - in /qpid/jms/trunk/src: main/java/org/apache/qpid/jms/engine/ main/java/org/apache/qpid/jms/impl/ test/java/org/apache/qpid/jms/engine/ test/java/org/apache/qpid/jms/impl/

Author: robbie
Date: Fri Feb 21 13:10:37 2014
New Revision: 1570564

URL: http://svn.apache.org/r1570564
Log:
QPIDJMS-14: round out TextMessage support, add verification of message body state and implement clearBody()

Modified:
    qpid/jms/trunk/src/main/java/org/apache/qpid/jms/engine/AmqpTextMessage.java
    qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/MessageImpl.java
    qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/TextMessageImpl.java
    qpid/jms/trunk/src/test/java/org/apache/qpid/jms/engine/AmqpTextMessageTest.java
    qpid/jms/trunk/src/test/java/org/apache/qpid/jms/impl/TextMessageImplTest.java

Modified: qpid/jms/trunk/src/main/java/org/apache/qpid/jms/engine/AmqpTextMessage.java
URL: http://svn.apache.org/viewvc/qpid/jms/trunk/src/main/java/org/apache/qpid/jms/engine/AmqpTextMessage.java?rev=1570564&r1=1570563&r2=1570564&view=diff
==============================================================================
--- qpid/jms/trunk/src/main/java/org/apache/qpid/jms/engine/AmqpTextMessage.java (original)
+++ qpid/jms/trunk/src/main/java/org/apache/qpid/jms/engine/AmqpTextMessage.java Fri Feb 21 13:10:37 2014
@@ -44,12 +44,6 @@ public class AmqpTextMessage extends Amq
      */
     public static final String CONTENT_TYPE = "text/plain";
 
-    /**
-     * Message type annotation value, only to be used when message uses
-     * an amqp-value body section containing a null value, not otherwise.
-     */
-    public static final String MSG_TYPE_ANNOTATION_VALUE = "TextMessage";
-
     private CharsetDecoder _decoder =  Charset.forName(UTF_8).newDecoder();
 
     public AmqpTextMessage()
@@ -67,9 +61,6 @@ public class AmqpTextMessage extends Amq
     {
         AmqpValue body = new AmqpValue(text);
         getMessage().setBody(body);
-
-        //TODO: clear the content-type in the case where we had received
-        //a message containing Data+ContentType
     }
 
     /**

Modified: qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/MessageImpl.java
URL: http://svn.apache.org/viewvc/qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/MessageImpl.java?rev=1570564&r1=1570563&r2=1570564&view=diff
==============================================================================
--- qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/MessageImpl.java (original)
+++ qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/MessageImpl.java Fri Feb 21 13:10:37 2014
@@ -53,6 +53,7 @@ public abstract class MessageImpl<T exte
     private Destination _destination;
     private Destination _replyTo;
     private boolean _propertiesWritable;
+    private boolean _bodyWritable;
 
     /**
      * Used to record the value of JMS_AMQP_TTL property
@@ -66,6 +67,7 @@ public abstract class MessageImpl<T exte
         _amqpMessage = amqpMessage;
         _sessionImpl = sessionImpl;
         _propertiesWritable = true;
+        _bodyWritable = true;
     }
 
     //message just received
@@ -91,6 +93,7 @@ public abstract class MessageImpl<T exte
         }
 
         _propertiesWritable = false;
+        _bodyWritable = false;
     }
 
     T getUnderlyingAmqpMessage(boolean prepareForSending)
@@ -171,12 +174,25 @@ public abstract class MessageImpl<T exte
         }
     }
 
-    private void setPropertiesWritable(boolean writable)
+    void setBodyWritable(boolean writable)
+    {
+        _bodyWritable = writable;
+    }
+
+    void checkBodyWritable() throws MessageNotWriteableException
+    {
+        if(!_bodyWritable)
+        {
+            throw new MessageNotWriteableException("Message body is currently in read-only mode");
+        }
+    }
+
+    void setPropertiesWritable(boolean writable)
     {
         _propertiesWritable = writable;
     }
 
-    private void checkPropertiesWritable() throws MessageNotWriteableException
+    void checkPropertiesWritable() throws MessageNotWriteableException
     {
         if(!_propertiesWritable)
         {

Modified: qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/TextMessageImpl.java
URL: http://svn.apache.org/viewvc/qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/TextMessageImpl.java?rev=1570564&r1=1570563&r2=1570564&view=diff
==============================================================================
--- qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/TextMessageImpl.java (original)
+++ qpid/jms/trunk/src/main/java/org/apache/qpid/jms/impl/TextMessageImpl.java Fri Feb 21 13:10:37 2014
@@ -42,9 +42,7 @@ public class TextMessageImpl extends Mes
     protected AmqpTextMessage prepareUnderlyingAmqpMessageForSending(AmqpTextMessage amqpMessage)
     {
         //Nothing to do here currently, the message operations are all
-        //already operating on the AmqpMessage directly
-
-        //TODO: do we need to do anything later with properties/headers etc?
+        //already operating on the AmqpTextMessage directly
         return amqpMessage;
     }
 
@@ -59,14 +57,19 @@ public class TextMessageImpl extends Mes
     @Override
     public void setText(String text) throws JMSException
     {
-        //TODO: checkWritable();
+        checkBodyWritable();
+
         getUnderlyingAmqpMessage(false).setText(text);
     }
 
     @Override
     public void clearBody() throws JMSException
     {
-        // TODO Auto-generated method stub
-        throw new UnsupportedOperationException("Not Implemented");
+        AmqpTextMessage underlyingAmqpMessage = getUnderlyingAmqpMessage(false);
+        underlyingAmqpMessage.setText(null);
+
+        underlyingAmqpMessage.setContentType(null);
+
+        setBodyWritable(true);
     }
 }

Modified: qpid/jms/trunk/src/test/java/org/apache/qpid/jms/engine/AmqpTextMessageTest.java
URL: http://svn.apache.org/viewvc/qpid/jms/trunk/src/test/java/org/apache/qpid/jms/engine/AmqpTextMessageTest.java?rev=1570564&r1=1570563&r2=1570564&view=diff
==============================================================================
--- qpid/jms/trunk/src/test/java/org/apache/qpid/jms/engine/AmqpTextMessageTest.java (original)
+++ qpid/jms/trunk/src/test/java/org/apache/qpid/jms/engine/AmqpTextMessageTest.java Fri Feb 21 13:10:37 2014
@@ -127,6 +127,9 @@ public class AmqpTextMessageTest extends
         Message message = Proton.message();
         message.setBody(new Data(null));
 
+        //This shouldn't happen with actual received messages, since Data sections can't really
+        //have a null value in them, they would have an empty byte array, but just in case...
+
         AmqpTextMessage amqpTextMessage = new AmqpTextMessage(_mockDelivery, message, _mockAmqpConnection);
 
         assertEquals("expected zero-length string", "", amqpTextMessage.getText());

Modified: qpid/jms/trunk/src/test/java/org/apache/qpid/jms/impl/TextMessageImplTest.java
URL: http://svn.apache.org/viewvc/qpid/jms/trunk/src/test/java/org/apache/qpid/jms/impl/TextMessageImplTest.java?rev=1570564&r1=1570563&r2=1570564&view=diff
==============================================================================
--- qpid/jms/trunk/src/test/java/org/apache/qpid/jms/impl/TextMessageImplTest.java (original)
+++ qpid/jms/trunk/src/test/java/org/apache/qpid/jms/impl/TextMessageImplTest.java Fri Feb 21 13:10:37 2014
@@ -22,12 +22,20 @@ package org.apache.qpid.jms.impl;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
+import java.nio.charset.Charset;
+
+import javax.jms.JMSException;
+import javax.jms.MessageNotWriteableException;
 
 import org.apache.qpid.jms.QpidJmsTestCase;
 import org.apache.qpid.jms.engine.AmqpConnection;
 import org.apache.qpid.jms.engine.AmqpTextMessage;
 import org.apache.qpid.proton.Proton;
+import org.apache.qpid.proton.amqp.Binary;
 import org.apache.qpid.proton.amqp.messaging.AmqpValue;
+import org.apache.qpid.proton.amqp.messaging.Data;
 import org.apache.qpid.proton.engine.Delivery;
 import org.apache.qpid.proton.message.Message;
 import org.junit.Before;
@@ -94,4 +102,96 @@ public class TextMessageImplTest extends
 
         assertEquals(value, textMessageImpl.getText());
     }
+
+    /**
+     * Test that when attempting to set the body of a received message, if clearBody() has
+     * not been called first then a MessageNotWritableException is thrown.
+     */
+    @Test
+    public void testAttemptingToSetTextOnReceivedMessageWithoutClearBodyResultsInMNWE() throws Exception
+    {
+        messageClearBodyAndWritableTestImpl(false, true);
+    }
+
+    /**
+     * Test that when attempting to set the body of a received message, the new content
+     * is successfully set if clearBody() has been called first.
+     */
+    @Test
+    public void testAttemptingToSetTextOnReceivedMessageAfterClearBodySucceeds() throws Exception
+    {
+        messageClearBodyAndWritableTestImpl(true, true);
+    }
+
+    /**
+     * Test that once clearBody() has been called, null is returned instead of previous content.
+     */
+    @Test
+    public void testClearBodyResultsInNullContent() throws Exception
+    {
+        messageClearBodyAndWritableTestImpl(true, false);
+    }
+
+    private void messageClearBodyAndWritableTestImpl(boolean clearBody, boolean setNewText) throws JMSException
+    {
+        Message message = Proton.message();
+        message.setBody(new AmqpValue("originalContent"));
+
+        AmqpTextMessage amqpTextMessage = new AmqpTextMessage(_mockDelivery, message, _mockAmqpConnection);
+        TextMessageImpl textMessageImpl = new TextMessageImpl(amqpTextMessage, _mockSessionImpl,_mockConnectionImpl, null);
+
+        if(clearBody)
+        {
+            textMessageImpl.clearBody();
+
+            if(setNewText)
+            {
+                textMessageImpl.setText("myNewText");
+                assertEquals("new message content not as expected", "myNewText", textMessageImpl.getText());
+            }
+            else
+            {
+                assertNull("message content was not cleared", textMessageImpl.getText());
+            }
+        }
+        else
+        {
+            try
+            {
+                textMessageImpl.setText("myNewText");
+                fail("expected exception was not thrown");
+            }
+            catch(MessageNotWriteableException mnwe)
+            {
+                //expected
+            }
+        }
+
+    }
+
+    /**
+     * Test that when clearing the body of a TextMessage, which sets the body value to null and
+     * defaults to using an AmqpValue body, that any content-type value on the original message
+     * is also cleared (since content-type SHOULD NOT be set except when sending Data sections)
+     */
+    @Test
+    public void testClearBodyWithReceivedMessageUsingDataSectionAndContentTypeResultsInClearingContentType() throws Exception
+    {
+        String messageCotnent = "myContentString";
+        byte[] encodedBytes = messageCotnent.getBytes(Charset.forName("UTF-8"));
+
+        Message message = Proton.message();
+        message.setBody(new Data(new Binary(encodedBytes)));
+        message.setContentType(AmqpTextMessage.CONTENT_TYPE);
+
+        AmqpTextMessage amqpTextMessage = new AmqpTextMessage(_mockDelivery, message, _mockAmqpConnection);
+        TextMessageImpl textMessageImpl = new TextMessageImpl(amqpTextMessage, _mockSessionImpl,_mockConnectionImpl, null);
+
+        assertEquals("Expected content not returned", messageCotnent, textMessageImpl.getText());
+        assertEquals("Expected content type to be set", AmqpTextMessage.CONTENT_TYPE, amqpTextMessage.getContentType());
+
+        textMessageImpl.clearBody();
+
+        assertNull("Expected content type to be cleared", amqpTextMessage.getContentType());
+    }
 }



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