QPID-122

Pulled _readable (_readableMessage) boolean up to AbstractJMSMessage.java and created (_readableProperties).
Checks are now carried out to ensure that the message is readable/writable for the operation so that the correct exception can be thrown. As per the spec.
clearBody() method had been implemented in AbstractJMSMessage.java to reset the _readableMessage as a result implementations must now implement clearBodyImpl() which is called before the _readableMessage is reset.

Test updated to check that properties and body are correctly read/write set.

git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk/qpid@480119 13f79535-47bb-0310-9956-ffa450edef68

9 files changed, 263 insertions, 72 deletions

diff --git a/java/client/src/main/java/org/apache/qpid/client/message/AbstractJMSMessage.java b/java/client/src/main/java/org/apache/qpid/client/message/AbstractJMSMessage.java
index 0c71b3f94c..a5b135da2d 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/AbstractJMSMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/AbstractJMSMessage.java
@@ -34,6 +34,8 @@ import org.apache.qpid.framing.PropertyFieldTable;
 
 import javax.jms.Destination;
 import javax.jms.JMSException;
+import javax.jms.MessageNotReadableException;
+import javax.jms.MessageNotWriteableException;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Iterator;
@@ -54,12 +56,14 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
     public static final char LONG_PROPERTY_PREFIX = PropertyFieldTable.LONG_PROPERTY_PREFIX;
     public static final char FLOAT_PROPERTY_PREFIX = PropertyFieldTable.FLOAT_PROPERTY_PREFIX;
     public static final char DOUBLE_PROPERTY_PREFIX = PropertyFieldTable.DOUBLE_PROPERTY_PREFIX;
-    public static final char STRING_PROPERTY_PREFIX = PropertyFieldTable.STRING_PROPERTY_PREFIX ;
+    public static final char STRING_PROPERTY_PREFIX = PropertyFieldTable.STRING_PROPERTY_PREFIX;
 
 
     protected boolean _redelivered;
 
     protected ByteBuffer _data;
+    private boolean _readableProperties = false;
+    private boolean _readableMessage = false;
 
     protected AbstractJMSMessage(ByteBuffer data)
     {
@@ -69,6 +73,8 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
         {
             _data.acquire();
         }
+        _readableProperties = (_contentHeaderProperties != null);
+        _readableMessage = (data != null);
     }
 
     protected AbstractJMSMessage(long deliveryTag, BasicContentHeaderProperties contentHeader, ByteBuffer data) throws AMQException
@@ -79,6 +85,9 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
         {
             _data.acquire();
         }
+
+        _readableProperties = (_contentHeaderProperties != null);
+        _readableMessage = data != null;
     }
 
     protected AbstractJMSMessage(BasicContentHeaderProperties contentHeader, long deliveryTag)
@@ -170,7 +179,7 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
         if (!(destination instanceof AMQDestination))
         {
             throw new IllegalArgumentException("ReplyTo destination my be an AMQ destination - passed argument was type " +
-                    destination.getClass());
+                                               destination.getClass());
         }
         final AMQDestination amqd = (AMQDestination) destination;
 
@@ -216,9 +225,9 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
     }
 
     public void setJMSType(String string) throws JMSException
-    {
+    {        
         //throw new JMSException("Cannot set JMS Type - it is implicitly defined based on message type");
-    	// this is not spec comliant, should not throw the message
+        // this is not spec comliant, should not throw the message
     }
 
     public long getJMSExpiration() throws JMSException
@@ -247,8 +256,17 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
         {
             getJmsContentHeaderProperties().getHeaders().clear();
         }
+
+        _readableProperties = false;
     }
 
+    public void clearBody() throws JMSException
+    {
+        clearBodyImpl();
+        _readableMessage = false;
+    }
+
+
     public boolean propertyExists(String propertyName) throws JMSException
     {
         checkPropertyName(propertyName);
@@ -460,6 +478,7 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
 
     public void setBooleanProperty(String propertyName, boolean b) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         //getJmsContentHeaderProperties().headers.put(BOOLEAN_PROPERTY_PREFIX + propertyName, Boolean.valueOf(b));
         getJmsContentHeaderProperties().getHeaders().put(BOOLEAN_PROPERTY_PREFIX + propertyName, b ? new Long(1) : new Long(0));
@@ -467,42 +486,49 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
 
     public void setByteProperty(String propertyName, byte b) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         getJmsContentHeaderProperties().getHeaders().put(BYTE_PROPERTY_PREFIX + propertyName, new Byte(b));
     }
 
     public void setShortProperty(String propertyName, short i) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         getJmsContentHeaderProperties().getHeaders().put(SHORT_PROPERTY_PREFIX + propertyName, new Short(i));
     }
 
     public void setIntProperty(String propertyName, int i) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         getJmsContentHeaderProperties().getHeaders().put(INT_PROPERTY_PREFIX + propertyName, new Integer(i));
     }
 
     public void setLongProperty(String propertyName, long l) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         getJmsContentHeaderProperties().getHeaders().put(LONG_PROPERTY_PREFIX + propertyName, new Long(l));
     }
 
     public void setFloatProperty(String propertyName, float f) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         getJmsContentHeaderProperties().getHeaders().put(FLOAT_PROPERTY_PREFIX + propertyName, new Float(f));
     }
 
     public void setDoubleProperty(String propertyName, double v) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         getJmsContentHeaderProperties().getHeaders().put(DOUBLE_PROPERTY_PREFIX + propertyName, new Double(v));
     }
 
     public void setStringProperty(String propertyName, String value) throws JMSException
     {
+        checkWritableProperties();
         checkPropertyName(propertyName);
         getJmsContentHeaderProperties().getHeaders().put(STRING_PROPERTY_PREFIX + propertyName, value);
     }
@@ -533,7 +559,13 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
         }
     }
 
-    public abstract void clearBody() throws JMSException;
+
+    /**
+     * This forces concrete classes to implement clearBody()
+     *
+     * @throws JMSException
+     */
+    public abstract void clearBodyImpl() throws JMSException;
 
     /**
      * Get a String representation of the body of the message. Used in the
@@ -604,7 +636,7 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
                                 break;
                             default:
                                 buf.append("<unknown type (identifier " +
-                                        typeIdentifier + ") ");
+                                           typeIdentifier + ") ");
                         }
                         buf.append(String.valueOf(entry.getValue()));
                     }
@@ -630,6 +662,7 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
 
     private void checkPropertyName(String propertyName)
     {
+
         if (propertyName == null)
         {
             throw new IllegalArgumentException("Property name must not be null");
@@ -688,4 +721,44 @@ public abstract class AbstractJMSMessage extends AMQMessage implements javax.jms
         }
         return _data;
     }
+
+    protected void checkReadable() throws MessageNotReadableException
+    {
+        if (!_readableMessage)
+        {
+            throw new MessageNotReadableException("You need to call reset() to make the message readable");
+        }
+    }
+
+    protected void checkWritable() throws MessageNotWriteableException
+    {
+        if (_readableMessage)
+        {
+            throw new MessageNotWriteableException("You need to call clearBody() to make the message writable");
+        }
+    }
+
+    protected void checkWritableProperties() throws MessageNotWriteableException
+    {
+        if (_readableProperties)
+        {
+            throw new MessageNotWriteableException("You need to call clearProperties() to make the message writable");
+        }
+    }
+
+    public boolean isReadable()
+    {
+        return _readableMessage;
+    }
+
+    public boolean isWritable()
+    {
+        return !_readableMessage;
+    }
+
+    public void reset() throws JMSException
+    {
+        _readableMessage = true;
+    }
+
 }
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/JMSBytesMessage.java b/java/client/src/main/java/org/apache/qpid/client/message/JMSBytesMessage.java
index 6921b0a4e6..6e1958e40a 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/JMSBytesMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/JMSBytesMessage.java
@@ -37,7 +37,6 @@ public class JMSBytesMessage extends AbstractJMSMessage implements javax.jms.Byt
 {
     private static final String MIME_TYPE = "application/octet-stream";
 
-    private boolean _readable = false;
 
     /**
      * The default initial size of the buffer. The buffer expands automatically.
@@ -65,7 +64,6 @@ public class JMSBytesMessage extends AbstractJMSMessage implements javax.jms.Byt
             _data = ByteBuffer.allocate(DEFAULT_BUFFER_INITIAL_SIZE);
             _data.setAutoExpand(true);
         }
-        _readable = (data != null);
     }
 
     JMSBytesMessage(long messageNbr, ContentHeaderBody contentHeader, ByteBuffer data)
@@ -74,13 +72,11 @@ public class JMSBytesMessage extends AbstractJMSMessage implements javax.jms.Byt
         // TODO: this casting is ugly. Need to review whole ContentHeaderBody idea
         super(messageNbr, (BasicContentHeaderProperties) contentHeader.properties, data);
         getJmsContentHeaderProperties().setContentType(MIME_TYPE);
-        _readable = true;
     }
 
-    public void clearBody() throws JMSException
+    public void clearBodyImpl() throws JMSException
     {
         _data.clear();
-        _readable = false;
     }
 
     public String toBodyString() throws JMSException
@@ -139,13 +135,6 @@ public class JMSBytesMessage extends AbstractJMSMessage implements javax.jms.Byt
         return _data.limit();
     }
 
-    private void checkReadable() throws MessageNotReadableException
-    {
-        if (!_readable)
-        {
-            throw new MessageNotReadableException("You need to call reset() to make the message readable");
-        }
-    }
 
     /**
      * Check that there is at least a certain number of bytes available to read
@@ -161,14 +150,6 @@ public class JMSBytesMessage extends AbstractJMSMessage implements javax.jms.Byt
         }
     }
 
-    private void checkWritable() throws MessageNotWriteableException
-    {
-        if (_readable)
-        {
-            throw new MessageNotWriteableException("You need to call clearBody() to make the message writable");
-        }
-    }
-
     public boolean readBoolean() throws JMSException
     {
         checkReadable();
@@ -392,13 +373,8 @@ public class JMSBytesMessage extends AbstractJMSMessage implements javax.jms.Byt
 
     public void reset() throws JMSException
     {
-        //checkWritable();
+        super.reset();
         _data.flip();
-        _readable = true;
     }
 
-    public boolean isReadable()
-    {
-        return _readable;
-    }
 }
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java b/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java
index 2001573ef9..233f05244f 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/JMSMapMessage.java
@@ -68,13 +68,13 @@ public class JMSMapMessage extends JMSTextMessage implements javax.jms.MapMessag
 
     // AbstractJMSMessage Interface
 
-    public void clearBody() throws JMSException
+    public void clearBodyImpl() throws JMSException
     {
         if (_data != null)
         {
             _data.release();
         }
-        _data = null;
+        _data = null;     
     }
 
     public String toBodyString() throws JMSException
@@ -206,48 +206,55 @@ public class JMSMapMessage extends JMSTextMessage implements javax.jms.MapMessag
 
     public void setBoolean(String string, boolean b) throws JMSException
     {
+        checkWritable();
         _map.setBoolean(string, b);
     }
 
     public void setByte(String string, byte b) throws JMSException
     {
+        checkWritable();
         _map.setByte(string, b);
     }
 
     public void setShort(String string, short i) throws JMSException
     {
+        checkWritable();
         _map.setShort(string, i);
     }
 
     public void setChar(String string, char c) throws JMSException
     {
+        checkWritable();
         _map.setChar(string, c);
     }
 
     public void setInt(String string, int i) throws JMSException
     {
+        checkWritable();
         _map.setInteger(string, i);
     }
 
     public void setLong(String string, long l) throws JMSException
     {
+        checkWritable();
         _map.setLong(string, l);
     }
 
     public void setFloat(String string, float v) throws JMSException
     {
-
+        checkWritable();
         _map.setFloat(string, v);
     }
 
     public void setDouble(String string, double v) throws JMSException
     {
-
+        checkWritable();
         _map.setDouble(string, v);
     }
 
     public void setString(String string, String string1) throws JMSException
     {
+        checkWritable();
         _map.setString(string, string1);
     }
 
@@ -258,11 +265,13 @@ public class JMSMapMessage extends JMSTextMessage implements javax.jms.MapMessag
 
     public void setBytes(String string, byte[] bytes, int i, int i1) throws JMSException
     {
+        checkWritable();
         _map.setBytes(string, bytes, i, i1);
     }
 
     public void setObject(String string, Object object) throws JMSException
     {
+        checkWritable();
         _map.setObject(string, object);
     }
 
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/JMSObjectMessage.java b/java/client/src/main/java/org/apache/qpid/client/message/JMSObjectMessage.java
index 34dd7e9ec1..61f326d52b 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/JMSObjectMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/JMSObjectMessage.java
@@ -36,7 +36,6 @@ import java.nio.charset.CharacterCodingException;
 public class JMSObjectMessage extends AbstractJMSMessage implements ObjectMessage
 {
     static final String MIME_TYPE = "application/java-object-stream";
-    private final boolean _readonly;
 
     private static final int DEFAULT_BUFFER_SIZE = 1024;
 
@@ -56,7 +55,6 @@ public class JMSObjectMessage extends AbstractJMSMessage implements ObjectMessag
             _data = ByteBuffer.allocate(DEFAULT_BUFFER_SIZE);
             _data.setAutoExpand(true);
         }
-        _readonly = (data != null);
         getJmsContentHeaderProperties().setContentType(MIME_TYPE);
     }
 
@@ -66,10 +64,9 @@ public class JMSObjectMessage extends AbstractJMSMessage implements ObjectMessag
     JMSObjectMessage(long messageNbr, ContentHeaderBody contentHeader, ByteBuffer data) throws AMQException
     {
         super(messageNbr, (BasicContentHeaderProperties) contentHeader.properties, data);
-        _readonly = data != null;
     }
 
-    public void clearBody() throws JMSException
+    public void clearBodyImpl() throws JMSException
     {
         if (_data != null)
         {
@@ -90,10 +87,7 @@ public class JMSObjectMessage extends AbstractJMSMessage implements ObjectMessag
 
     public void setObject(Serializable serializable) throws JMSException
     {
-        if (_readonly)
-        {
-            throw new MessageNotWriteableException("Message is not writable.");
-        }
+        checkWritable();
 
         if (_data == null)
         {
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/JMSTextMessage.java b/java/client/src/main/java/org/apache/qpid/client/message/JMSTextMessage.java
index 2624c20105..3061d5a59c 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/JMSTextMessage.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/JMSTextMessage.java
@@ -66,7 +66,7 @@ public class JMSTextMessage extends AbstractJMSMessage implements javax.jms.Text
         setText(text);
     }
 
-    public void clearBody() throws JMSException
+    public void clearBodyImpl() throws JMSException
     {
         if (_data != null)
         {
@@ -93,6 +93,8 @@ public class JMSTextMessage extends AbstractJMSMessage implements javax.jms.Text
 
     public void setText(String string) throws JMSException
     {
+        checkWritable();
+        
         clearBody();
         try
         {
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/basic/BytesMessageTest.java b/java/client/src/test/java/org/apache/qpid/test/unit/basic/BytesMessageTest.java
index 4d37c5d2a6..2983a16e6d 100644
--- a/java/client/src/test/java/org/apache/qpid/test/unit/basic/BytesMessageTest.java
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/basic/BytesMessageTest.java
@@ -135,6 +135,8 @@ public class BytesMessageTest extends TestCase implements MessageListener
             buffer.get(data);
             actual.add(data);
 
+
+            //Check Body Write Status
             try
             {
                 m.writeBoolean(true);
@@ -144,6 +146,41 @@ public class BytesMessageTest extends TestCase implements MessageListener
             {
                 //normal execution
             }
+
+            m.clearBody();
+
+            try
+            {
+                m.writeBoolean(true);
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
+
+
+            //Check property write status
+            try
+            {
+                m.setStringProperty("test", "test");
+                Assert.fail("Message should not be writeable");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                //normal execution
+            }
+
+            m.clearProperties();
+
+            try
+            {
+                m.setStringProperty("test", "test");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
+
         }
 
         assertEqual(messages.iterator(), actual.iterator());
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/basic/MapMessageTest.java b/java/client/src/test/java/org/apache/qpid/test/unit/basic/MapMessageTest.java
index f25d2887ae..a685e75a29 100644
--- a/java/client/src/test/java/org/apache/qpid/test/unit/basic/MapMessageTest.java
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/basic/MapMessageTest.java
@@ -132,16 +132,48 @@ public class MapMessageTest extends TestCase implements MessageListener
             assertEqual(m.getInt("messageNumber"), count);
             assertEqual(m.getBoolean("odd"), count / 2 == 0);
 
-//            try
-//            {
-//                m.setInt("testint", 3);
-//                fail("Message should not be writeable");
-//            }
-//            catch (MessageNotWriteableException mnwe)
-//            {
-//                //normal execution
-//            }
+            try
+            {
+                m.setInt("testint", 3);
+                fail("Message should not be writeable");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                //normal execution
+            }
+
+            m.clearBody();
+
+            try
+            {
+                m.setInt("testint", 3);
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
 
+              //Check property write status
+            try
+            {
+                m.setStringProperty("test", "test");
+                Assert.fail("Message should not be writeable");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                //normal execution
+            }
+
+            m.clearProperties();
+
+            try
+            {
+                m.setStringProperty("test", "test");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
 
             count++;
         }
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/basic/ObjectMessageTest.java b/java/client/src/test/java/org/apache/qpid/test/unit/basic/ObjectMessageTest.java
index dfb1b26454..e7d7159bd8 100644
--- a/java/client/src/test/java/org/apache/qpid/test/unit/basic/ObjectMessageTest.java
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/basic/ObjectMessageTest.java
@@ -125,19 +125,53 @@ public class ObjectMessageTest extends TestCase implements MessageListener
         {
             actual.add(m.getObject());
 
-//            try
-//            {
-//                m.setObject("Test text");
-//                Assert.fail("Message should not be writeable");
-//            }
-//            catch (MessageNotWriteableException mnwe)
-//            {
-//                //normal execution
-//            }
+            try
+            {
+                m.setObject("Test text");
+                Assert.fail("Message should not be writeable");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                //normal execution
+            }
+
+            m.clearBody();
+
+            try
+            {
+                m.setObject("Test text");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
+
+              //Check property write status
+            try
+            {
+                m.setStringProperty("test", "test");
+                Assert.fail("Message should not be writeable");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                //normal execution
+            }
+
+            m.clearProperties();
+
+            try
+            {
+                m.setStringProperty("test", "test");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
 
         }
 
         assertEqual(messages.iterator(), actual.iterator());
+
     }
 
     private static void assertEqual(Iterator expected, Iterator actual)
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/basic/TextMessageTest.java b/java/client/src/test/java/org/apache/qpid/test/unit/basic/TextMessageTest.java
index 04a9185fa6..dfbec81549 100644
--- a/java/client/src/test/java/org/apache/qpid/test/unit/basic/TextMessageTest.java
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/basic/TextMessageTest.java
@@ -122,15 +122,49 @@ public class TextMessageTest extends TestCase implements MessageListener
         {
             actual.add(m.getText());
 
-//            try
-//            {
-//                m.setText("Test text");
-//                Assert.fail("Message should not be writeable");
-//            }
-//            catch (MessageNotWriteableException mnwe)
-//            {
-//                //normal execution
-//            }
+            //Check body write status            
+            try
+            {
+                m.setText("Test text");
+                Assert.fail("Message should not be writeable");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                //normal execution
+            }
+
+            m.clearBody();
+
+            try
+            {
+                m.setText("Test text");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
+
+            //Check property write status
+            try
+            {
+                m.setStringProperty("test", "test");
+                Assert.fail("Message should not be writeable");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                //normal execution
+            }
+
+            m.clearProperties();
+
+            try
+            {
+                m.setStringProperty("test", "test");
+            }
+            catch (MessageNotWriteableException mnwe)
+            {
+                Assert.fail("Message should be writeable");
+            }
 
         }