QPID-333 Message Properties on non Qpid Messages are not preserved

Updated MessageConverter to have a constructor that takes a Message type. 

Updated MessageConverterTest to use the new NonQpidMessage to test it out.

JMSHeaderAdapter.java - whitespace changes and comment noting that null return is required.

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

5 files changed, 493 insertions, 47 deletions

diff --git a/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java b/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java
index 4426a7deee..f56fc40360 100644
--- a/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java
+++ b/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer.java
@@ -388,9 +388,7 @@ public class BasicMessageProducer extends Closeable implements org.apache.qpid.j
             }
             else
             {
-                //TODO; Do we really want to create an empty message here ?
-                newMessage = (AbstractJMSMessage) _session.createMessage();
-                return new MessageConverter(newMessage).getConvertedMessage();
+                newMessage = new MessageConverter(message).getConvertedMessage();
             }
 
             if (newMessage != null)
diff --git a/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java b/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
index 3086e5b90a..a1e2640f21 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
@@ -269,7 +269,7 @@ public final class JMSHeaderAdapter
                         s = String.valueOf(o);

                     }

                 }

-            }

+            }//else return s // null; 

         }

 

         return s;

@@ -508,16 +508,16 @@ public final class JMSHeaderAdapter
 

             // JMS invalid names

             if ((propertyName.equals("NULL")

-                    || propertyName.equals("TRUE")

-                    || propertyName.equals("FALSE")

-                    || propertyName.equals("NOT")

-                    || propertyName.equals("AND")

-                    || propertyName.equals("OR")

-                    || propertyName.equals("BETWEEN")

-                    || propertyName.equals("LIKE")

-                    || propertyName.equals("IN")

-                    || propertyName.equals("IS")

-                    || propertyName.equals("ESCAPE")))

+                 || propertyName.equals("TRUE")

+                 || propertyName.equals("FALSE")

+                 || propertyName.equals("NOT")

+                 || propertyName.equals("AND")

+                 || propertyName.equals("OR")

+                 || propertyName.equals("BETWEEN")

+                 || propertyName.equals("LIKE")

+                 || propertyName.equals("IN")

+                 || propertyName.equals("IS")

+                 || propertyName.equals("ESCAPE")))

             {

                 throw new IllegalArgumentException("Identifier '" + propertyName + "' is not allowed in JMS");

             }

diff --git a/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java b/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java
index 58089f595b..f30b8ddc9d 100644
--- a/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java
+++ b/java/client/src/main/java/org/apache/qpid/client/message/MessageConverter.java
@@ -25,7 +25,8 @@ import org.apache.log4j.Logger;
 import javax.jms.*;
 import java.util.Enumeration;
 
-public class MessageConverter {
+public class MessageConverter
+{
 
     /**
      * Log4J logger
@@ -118,6 +119,15 @@ public class MessageConverter {
         setMessageProperties(message);
     }
 
+    public MessageConverter(Message message) throws JMSException
+    {
+        //TODO; Do we really want to create an empty message here ?
+        BytesMessage nativeMessage = new JMSBytesMessage();                
+
+        _newMessage = (AbstractJMSMessage) nativeMessage;
+        setMessageProperties(message);
+    }
+
     public AbstractJMSMessage getConvertedMessage()
     {
         return _newMessage;
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/message/MessageConverterTest.java b/java/client/src/test/java/org/apache/qpid/test/unit/message/MessageConverterTest.java
index 6a335b8627..a8a5c7d8b2 100644
--- a/java/client/src/test/java/org/apache/qpid/test/unit/message/MessageConverterTest.java
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/message/MessageConverterTest.java
@@ -31,10 +31,12 @@ import javax.jms.Message;
 import javax.jms.Destination;
 import javax.jms.TextMessage;
 import javax.jms.MapMessage;
+import javax.jms.JMSException;
 import java.util.HashMap;
 
 
-public class MessageConverterTest extends TestCase {
+public class MessageConverterTest extends TestCase
+{
 
     public static final String JMS_CORR_ID = "QPIDID_01";
     public static final int JMS_DELIV_MODE = 1;
@@ -50,53 +52,79 @@ public class MessageConverterTest extends TestCase {
         super.setUp();
         testTextMessage = new JMSTextMessage();
 
-        //Add JMSProperties
-        testTextMessage.setJMSCorrelationID(JMS_CORR_ID);
-        testTextMessage.setJMSDeliveryMode(JMS_DELIV_MODE);
-        testTextMessage.setJMSType(JMS_TYPE);
-        testTextMessage.setJMSReplyTo(JMS_REPLY_TO);
+        //Set Message Text
         testTextMessage.setText("testTextMessage text");
-
-        //Add non-JMS properties
-        testTextMessage.setStringProperty("testProp1","testValue1");
-        testTextMessage.setDoubleProperty("testProp2",Double.MIN_VALUE);
+        setMessageProperties(testTextMessage);
 
         testMapMessage = new JMSMapMessage();
-        testMapMessage.setString("testMapString","testMapStringValue");
-        testMapMessage.setDouble("testMapDouble",Double.MAX_VALUE);
+        testMapMessage.setString("testMapString", "testMapStringValue");
+        testMapMessage.setDouble("testMapDouble", Double.MAX_VALUE);
     }
 
     public void testSetProperties() throws Exception
     {
-        AbstractJMSMessage newMessage = new MessageConverter((TextMessage)testTextMessage).getConvertedMessage();
-
-        //check JMS prop values on newMessage match
-        assertEquals("JMS Correlation ID mismatch",testTextMessage.getJMSCorrelationID(),newMessage.getJMSCorrelationID());
-        assertEquals("JMS Delivery mode mismatch",testTextMessage.getJMSDeliveryMode(),newMessage.getJMSDeliveryMode());
-        assertEquals("JMS Type mismatch",testTextMessage.getJMSType(),newMessage.getJMSType());
-        assertEquals("JMS Reply To mismatch",testTextMessage.getJMSReplyTo(),newMessage.getJMSReplyTo());
-
-        //check non-JMS standard props ok too
-        assertEquals("Test String prop value mismatch",testTextMessage.getStringProperty("testProp1"),
-                    newMessage.getStringProperty("testProp1"));
-        assertEquals("Test Double prop value mismatch",testTextMessage.getDoubleProperty("testProp2"),
-                    newMessage.getDoubleProperty("testProp2"));
+        AbstractJMSMessage newMessage = new MessageConverter((TextMessage) testTextMessage).getConvertedMessage();
+        mesagePropertiesTest(testTextMessage, newMessage);
     }
 
     public void testJMSTextMessageConversion() throws Exception
     {
-        AbstractJMSMessage newMessage = new MessageConverter((TextMessage)testTextMessage).getConvertedMessage();
-        assertEquals("Converted message text mismatch",((JMSTextMessage)newMessage).getText(),testTextMessage.getText());
+        AbstractJMSMessage newMessage = new MessageConverter((TextMessage) testTextMessage).getConvertedMessage();
+        assertEquals("Converted message text mismatch", ((JMSTextMessage) newMessage).getText(), testTextMessage.getText());
     }
 
     public void testJMSMapMessageConversion() throws Exception
     {
-        AbstractJMSMessage newMessage = new MessageConverter((MapMessage)testMapMessage).getConvertedMessage();
-        assertEquals("Converted map message String mismatch",((JMSMapMessage)newMessage).getString("testMapString"),
-                    testMapMessage.getString("testMapString"));
-        assertEquals("Converted map message Double mismatch",((JMSMapMessage)newMessage).getDouble("testMapDouble"),
-                    testMapMessage.getDouble("testMapDouble"));
+        AbstractJMSMessage newMessage = new MessageConverter((MapMessage) testMapMessage).getConvertedMessage();
+        assertEquals("Converted map message String mismatch", ((JMSMapMessage) newMessage).getString("testMapString"),
+                     testMapMessage.getString("testMapString"));
+        assertEquals("Converted map message Double mismatch", ((JMSMapMessage) newMessage).getDouble("testMapDouble"),
+                     testMapMessage.getDouble("testMapDouble"));
+
+    }
+
+    public void testMessageConversion() throws Exception
+    {
+        Message newMessage = new NonQpidMessage();
+        setMessageProperties(newMessage);
+        mesagePropertiesTest(testTextMessage, newMessage);
+    }
+
+    private void setMessageProperties(Message message) throws JMSException
+    {
+        message.setJMSCorrelationID(JMS_CORR_ID);
+        message.setJMSDeliveryMode(JMS_DELIV_MODE);
+        message.setJMSType(JMS_TYPE);
+        message.setJMSReplyTo(JMS_REPLY_TO);
 
+        //Add non-JMS properties
+        message.setStringProperty("testProp1", "testValue1");
+        message.setDoubleProperty("testProp2", Double.MIN_VALUE);
+    }
+
+
+    private void mesagePropertiesTest(Message expectedMessage, Message actualMessage)
+    {
+        try
+        {
+            //check JMS prop values on newMessage match
+            assertEquals("JMS Correlation ID mismatch", expectedMessage.getJMSCorrelationID(), actualMessage.getJMSCorrelationID());
+            assertEquals("JMS Delivery mode mismatch", expectedMessage.getJMSDeliveryMode(), actualMessage.getJMSDeliveryMode());
+            assertEquals("JMS Type mismatch", expectedMessage.getJMSType(), actualMessage.getJMSType());
+            assertEquals("JMS Reply To mismatch", expectedMessage.getJMSReplyTo(), actualMessage.getJMSReplyTo());
+
+            //check non-JMS standard props ok too
+            assertEquals("Test String prop value mismatch", expectedMessage.getStringProperty("testProp1"),
+                         actualMessage.getStringProperty("testProp1"));
+
+            assertEquals("Test Double prop value mismatch", expectedMessage.getDoubleProperty("testProp2"),
+                         actualMessage.getDoubleProperty("testProp2"));
+        }
+        catch (JMSException e)
+        {
+            fail("An error occured testing the property values" + e.getCause());
+            e.printStackTrace();
+        }
     }
 
     protected void tearDown() throws Exception
diff --git a/java/client/src/test/java/org/apache/qpid/test/unit/message/NonQpidMessage.java b/java/client/src/test/java/org/apache/qpid/test/unit/message/NonQpidMessage.java
new file mode 100644
index 0000000000..e992290513
--- /dev/null
+++ b/java/client/src/test/java/org/apache/qpid/test/unit/message/NonQpidMessage.java
@@ -0,0 +1,410 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.    
+ *
+ * 
+ */
+package org.apache.qpid.test.unit.message;
+
+import javax.jms.Message;
+import javax.jms.JMSException;
+import javax.jms.Destination;
+import java.util.Enumeration;
+import java.util.Hashtable;
+
+public class NonQpidMessage implements Message
+{
+    private String _JMSMessageID;
+    private long _JMSTimestamp;
+    private byte[] _JMSCorrelationIDAsBytes;
+    private String _JMSCorrelationID;
+    private Destination _JMSReplyTo;
+    private Destination _JMSDestination;
+    private int _JMSDeliveryMode;
+    private boolean _JMSRedelivered;
+    private String _JMSType;
+    private long _JMSExpiration;
+    private int _JMSPriority;
+    private Hashtable _properties;
+
+    public NonQpidMessage()
+    {
+        _properties = new Hashtable();
+        _JMSPriority = javax.jms.Message.DEFAULT_PRIORITY;
+        _JMSDeliveryMode = javax.jms.Message.DEFAULT_DELIVERY_MODE;
+    }
+
+    public String getJMSMessageID() throws JMSException
+    {
+        return _JMSMessageID;
+    }
+
+    public void setJMSMessageID(String string) throws JMSException
+    {
+        _JMSMessageID = string;
+    }
+
+    public long getJMSTimestamp() throws JMSException
+    {
+        return _JMSTimestamp;
+    }
+
+    public void setJMSTimestamp(long l) throws JMSException
+    {
+        _JMSTimestamp = l;
+    }
+
+    public byte[] getJMSCorrelationIDAsBytes() throws JMSException
+    {
+        return _JMSCorrelationIDAsBytes;
+    }
+
+    public void setJMSCorrelationIDAsBytes(byte[] bytes) throws JMSException
+    {
+        _JMSCorrelationIDAsBytes = bytes;
+    }
+
+    public void setJMSCorrelationID(String string) throws JMSException
+    {
+        _JMSCorrelationID = string;
+    }
+
+    public String getJMSCorrelationID() throws JMSException
+    {
+        return _JMSCorrelationID;
+    }
+
+    public Destination getJMSReplyTo() throws JMSException
+    {
+        return _JMSReplyTo;
+    }
+
+    public void setJMSReplyTo(Destination destination) throws JMSException
+    {
+        _JMSReplyTo = destination;
+    }
+
+    public Destination getJMSDestination() throws JMSException
+    {
+        return _JMSDestination;
+    }
+
+    public void setJMSDestination(Destination destination) throws JMSException
+    {
+        _JMSDestination = destination;
+    }
+
+    public int getJMSDeliveryMode() throws JMSException
+    {
+        return _JMSDeliveryMode;
+    }
+
+    public void setJMSDeliveryMode(int i) throws JMSException
+    {
+        _JMSDeliveryMode = i;
+    }
+
+    public boolean getJMSRedelivered() throws JMSException
+    {
+        return _JMSRedelivered;
+    }
+
+    public void setJMSRedelivered(boolean b) throws JMSException
+    {
+        _JMSRedelivered = b;
+    }
+
+    public String getJMSType() throws JMSException
+    {
+        return _JMSType;
+    }
+
+    public void setJMSType(String string) throws JMSException
+    {
+        _JMSType = string;
+    }
+
+    public long getJMSExpiration() throws JMSException
+    {
+        return _JMSExpiration;
+    }
+
+    public void setJMSExpiration(long l) throws JMSException
+    {
+        _JMSExpiration = l;
+    }
+
+    public int getJMSPriority() throws JMSException
+    {
+        return _JMSPriority;
+    }
+
+    public void setJMSPriority(int i) throws JMSException
+    {
+        _JMSPriority = i;
+    }
+
+    public void clearProperties() throws JMSException
+    {
+        _properties.clear();
+    }
+
+    public boolean propertyExists(String string) throws JMSException
+    {
+        return _properties.containsKey(string);
+    }
+
+    public boolean getBooleanProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Boolean)
+            {
+                return (Boolean) o;
+            }
+            else
+            {
+                return Boolean.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public byte getByteProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Byte)
+            {
+                return (Byte) o;
+            }
+            else
+            {
+                return Byte.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public short getShortProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Short)
+            {
+                return (Short) o;
+            }
+            else
+            {
+                return Short.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public int getIntProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Integer)
+            {
+                return (Integer) o;
+            }
+            else
+            {
+                return Integer.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public long getLongProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Long)
+            {
+                return (Long) o;
+            }
+            else
+            {
+                return Long.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public float getFloatProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Float)
+            {
+                return (Float) o;
+            }
+            else
+            {
+                return Float.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public double getDoubleProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Double)
+            {
+                return (Double) o;
+            }
+            else
+            {
+                return Double.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public String getStringProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof String)
+            {
+                return (String) o;
+            }
+            else
+            {
+                return null;
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public Object getObjectProperty(String string) throws JMSException
+    {
+        if (propertyExists(string))
+        {
+            Object o = _properties.get(string);
+            if (o instanceof Boolean)
+            {
+                return (Boolean) o;
+            }
+            else
+            {
+                return Boolean.valueOf(null);
+            }
+        }
+        else
+        {
+            throw new JMSException("property does not exist: " + string);
+        }
+    }
+
+    public Enumeration getPropertyNames() throws JMSException
+    {
+        return _properties.keys();
+    }
+
+    public void setBooleanProperty(String string, boolean b) throws JMSException
+    {
+        _properties.put(string, b);
+    }
+
+    public void setByteProperty(String string, byte b) throws JMSException
+    {
+        _properties.put(string, b);
+    }
+
+    public void setShortProperty(String string, short i) throws JMSException
+    {
+        _properties.put(string, i);
+    }
+
+    public void setIntProperty(String string, int i) throws JMSException
+    {
+        _properties.put(string, i);
+    }
+
+    public void setLongProperty(String string, long l) throws JMSException
+    {
+        _properties.put(string, l);
+    }
+
+    public void setFloatProperty(String string, float v) throws JMSException
+    {
+        _properties.put(string, v);
+    }
+
+    public void setDoubleProperty(String string, double v) throws JMSException
+    {
+        _properties.put(string, v);
+    }
+
+    public void setStringProperty(String string, String string1) throws JMSException
+    {
+        _properties.put(string, string1);
+    }
+
+    public void setObjectProperty(String string, Object object) throws JMSException
+    {
+        _properties.put(string, object);
+    }
+
+    public void acknowledge() throws JMSException
+    {
+
+    }
+
+    public void clearBody() throws JMSException
+    {
+
+    }
+}