QPID-4659 : Address JIRA comments from Robbie Gemmell

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/QPID-4659@1461575 13f79535-47bb-0310-9956-ffa450edef68

12 files changed, 57 insertions, 76 deletions

diff --git a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
index b70f37e7c3..ecda20f780 100644
--- a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
+++ b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
@@ -33,12 +33,14 @@ import org.apache.qpid.framing.ProtocolVersion;
 import org.apache.qpid.framing.abstraction.MessagePublishInfo;
 import org.apache.qpid.server.message.AMQMessageHeader;
 import org.apache.qpid.server.message.EnqueableMessage;
+import org.apache.qpid.server.protocol.v0_10.MessageMetaDataType_0_10;
 import org.apache.qpid.server.protocol.v0_8.MessageMetaData;
 import org.apache.qpid.server.protocol.v0_10.MessageMetaData_0_10;
 import org.apache.qpid.server.message.MessageReference;
 import org.apache.qpid.server.message.ServerMessage;
 import org.apache.qpid.server.model.UUIDGenerator;
 import org.apache.qpid.server.plugin.MessageMetaDataType;
+import org.apache.qpid.server.protocol.v0_8.MessageMetaDataType_0_8;
 import org.apache.qpid.server.store.MessageStore;
 import org.apache.qpid.server.store.StorableMessageMetaData;
 import org.apache.qpid.server.store.StoredMessage;
@@ -130,7 +132,7 @@ public class BDBMessageStoreTest extends org.apache.qpid.server.store.MessageSto
          */
         StorableMessageMetaData storeableMMD_0_8 = bdbStore.getMessageMetaData(messageid_0_8);
 
-        assertEquals("Unexpected message type",MessageMetaDataType.META_DATA_0_8, storeableMMD_0_8.getType());
+        assertEquals("Unexpected message type", MessageMetaDataType_0_8.TYPE, storeableMMD_0_8.getType());
         assertTrue("Unexpected instance type", storeableMMD_0_8 instanceof MessageMetaData);
         MessageMetaData returnedMMD_0_8 = (MessageMetaData) storeableMMD_0_8;
 
@@ -162,7 +164,7 @@ public class BDBMessageStoreTest extends org.apache.qpid.server.store.MessageSto
          */
         StorableMessageMetaData storeableMMD_0_10 = bdbStore.getMessageMetaData(messageid_0_10);
 
-        assertEquals("Unexpected message type",MessageMetaDataType.META_DATA_0_10, storeableMMD_0_10.getType());
+        assertEquals("Unexpected message type", MessageMetaDataType_0_10.TYPE, storeableMMD_0_10.getType());
         assertTrue("Unexpected instance type", storeableMMD_0_10 instanceof MessageMetaData_0_10);
         MessageMetaData_0_10 returnedMMD_0_10 = (MessageMetaData_0_10) storeableMMD_0_10;
 
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_0_10_to_1_0.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_0_10_to_1_0.java
index a0f3228b22..d4b6317091 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_0_10_to_1_0.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_0_10_to_1_0.java
@@ -51,9 +51,20 @@ public class MessageConverter_0_10_to_1_0  extends MessageConverter_to_1_0<Messa
         }
 
         Properties props = new Properties();
+
+        /*
+            TODO: the current properties are not currently set:
+
+            absoluteExpiryTime
+            creationTime
+            groupId
+            groupSequence
+            replyToGroupId
+            to
+        */
+
         if(msgProps != null)
         {
-        //        props.setAbsoluteExpiryTime();
             if(msgProps.hasContentEncoding())
             {
                 props.setContentEncoding(Symbol.valueOf(msgProps.getContentEncoding()));
@@ -63,9 +74,7 @@ public class MessageConverter_0_10_to_1_0  extends MessageConverter_to_1_0<Messa
             {
                 props.setCorrelationId(msgProps.getCorrelationId());
             }
-        //        props.setCreationTime();
-        //        props.setGroupId();
-        //        props.setGroupSequence();
+
             if(msgProps.hasMessageId())
             {
                 props.setMessageId(msgProps.getMessageId());
@@ -84,9 +93,9 @@ public class MessageConverter_0_10_to_1_0  extends MessageConverter_to_1_0<Messa
                     props.setContentType(Symbol.valueOf("application/x-java-serialized-object"));
                 }
             }
-        //        props.setReplyToGroupId();
+
             props.setSubject(serverMessage.getRoutingKey());
-        //        props.setTo();
+
             if(msgProps.hasUserId())
             {
                 props.setUserId(new Binary(msgProps.getUserId()));
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_0_10.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_0_10.java
deleted file mode 100644
index 8974d913ae..0000000000
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_10_v1_0/MessageConverter_1_0_to_0_10.java
+++ /dev/null
@@ -1,27 +0,0 @@
-package org.apache.qpid.server.protocol.converter.v0_10_v1_0;
-
-import org.apache.qpid.server.plugin.MessageConverter;
-import org.apache.qpid.server.protocol.v0_10.MessageTransferMessage;
-import org.apache.qpid.server.protocol.v1_0.Message_1_0;
-import org.apache.qpid.server.virtualhost.VirtualHost;
-
-public class MessageConverter_1_0_to_0_10 implements MessageConverter<Message_1_0, MessageTransferMessage>
-{
-    @Override
-    public Class<Message_1_0> getInputClass()
-    {
-        return Message_1_0.class;
-    }
-
-    @Override
-    public Class<MessageTransferMessage> getOutputClass()
-    {
-        return MessageTransferMessage.class;
-    }
-
-    @Override
-    public MessageTransferMessage convert(Message_1_0 message, VirtualHost vhost)
-    {
-        return null;  //TODO - Implement
-    }
-}
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
index 66d59910ed..f35cc725c4 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_0_8_to_1_0.java
@@ -51,6 +51,16 @@ public class MessageConverter_0_8_to_1_0 extends MessageConverter_to_1_0<AMQMess
 
         Properties props = new Properties();
 
+        /*
+            TODO: The following properties are not currently set:
+
+            creationTime
+            groupId
+            groupSequence
+            replyToGroupId
+            to
+        */
+
         props.setContentEncoding(Symbol.valueOf(contentHeader.getEncodingAsString()));
 
         props.setContentType(Symbol.valueOf(contentHeader.getContentTypeAsString()));
@@ -66,9 +76,7 @@ public class MessageConverter_0_8_to_1_0 extends MessageConverter_to_1_0<AMQMess
         {
             props.setCorrelationId(new Binary(correlationId.getBytes()));
         }
-        //        props.setCreationTime();
-        //        props.setGroupId();
-        //        props.setGroupSequence();
+
         final AMQShortString messageId = contentHeader.getMessageId();
         if(messageId != null)
         {
@@ -76,13 +84,12 @@ public class MessageConverter_0_8_to_1_0 extends MessageConverter_to_1_0<AMQMess
         }
         props.setReplyTo(String.valueOf(contentHeader.getReplyTo()));
 
-        //        props.setReplyToGroupId();
         props.setSubject(serverMessage.getRoutingKey());
-        //        props.setTo();
         if(contentHeader.getUserId() != null)
         {
             props.setUserId(new Binary(contentHeader.getUserId().getBytes()));
         }
+
         sections.add(props);
 
         sections.add(new ApplicationProperties(FieldTable.convertToMap(contentHeader.getHeaders())));
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_0_8.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_0_8.java
deleted file mode 100644
index 8e0355a605..0000000000
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/converter/v0_8_v1_0/MessageConverter_1_0_to_0_8.java
+++ /dev/null
@@ -1,27 +0,0 @@
-package org.apache.qpid.server.protocol.converter.v0_8_v1_0;
-
-import org.apache.qpid.server.plugin.MessageConverter;
-import org.apache.qpid.server.protocol.v0_8.AMQMessage;
-import org.apache.qpid.server.protocol.v1_0.Message_1_0;
-import org.apache.qpid.server.virtualhost.VirtualHost;
-
-public class MessageConverter_1_0_to_0_8 implements MessageConverter<Message_1_0, AMQMessage>
-{
-    @Override
-    public Class<Message_1_0> getInputClass()
-    {
-        return Message_1_0.class;
-    }
-
-    @Override
-    public Class<AMQMessage> getOutputClass()
-    {
-        return AMQMessage.class;
-    }
-
-    @Override
-    public AMQMessage convert(Message_1_0 message, VirtualHost vhost)
-    {
-        return null;  //TODO - Implement
-    }
-}
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageMetaDataType_0_10.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageMetaDataType_0_10.java
index dd137c5b7d..82b14c5bcb 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageMetaDataType_0_10.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/MessageMetaDataType_0_10.java
@@ -28,10 +28,12 @@ import org.apache.qpid.server.store.StoredMessage;
 public class MessageMetaDataType_0_10 implements MessageMetaDataType<MessageMetaData_0_10>
 {
 
+    public static final int TYPE = 1;
+
     @Override
     public int ordinal()
     {
-        return 1;
+        return TYPE;
     }
 
     @Override
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
index 33ee397268..e094930e42 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
@@ -469,7 +469,7 @@ public class ServerConnection extends Connection implements AMQConnectionModel,
     @Override
     public String getVirtualHostName()
     {
-        return _virtualHost.getName();
+        return _virtualHost == null ? null : _virtualHost.getName();
     }
 
     public long getSessionCountLimit()
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaDataType_0_8.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaDataType_0_8.java
index 41e051bc4f..d0f115ab9d 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaDataType_0_8.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaDataType_0_8.java
@@ -28,10 +28,12 @@ import org.apache.qpid.server.store.StoredMessage;
 public class MessageMetaDataType_0_8 implements MessageMetaDataType<MessageMetaData>
 {
 
+    public static final int TYPE = 0;
+
     @Override
     public int ordinal()
     {
-        return 0;
+        return TYPE;
     }
 
     @Override
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java
index e6de727705..62b13baac2 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java
@@ -78,8 +78,6 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener<Con
         }
         else
         {
-            session.setVirtualHost(virtualHost);
-
             // Check virtualhost access
             if (!virtualHost.getSecurityManager().accessVirtualhost(virtualHostName, session.getRemoteAddress()))
             {
@@ -90,6 +88,7 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener<Con
                 throw body.getConnectionException(AMQConstant.CONNECTION_FORCED, "Virtual host '" + virtualHost.getName() + "' is not active");
             }
 
+            session.setVirtualHost(virtualHost);
 
             // See Spec (0.8.2). Section  3.1.2 Virtual Hosts
             if (session.getContextKey() == null)
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java
index 657a29aa1a..61fb2a4bdb 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java
@@ -220,7 +220,7 @@ public class Connection_1_0 implements ConnectionEventListener
         @Override
         public String getVirtualHostName()
         {
-            return _vhost.getName();
+            return _vhost == null ? null : _vhost.getName();
         }
 
         @Override
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/MessageMetaDataType_1_0.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/MessageMetaDataType_1_0.java
index 455052ea81..568b0c1c0f 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/MessageMetaDataType_1_0.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/v1_0/MessageMetaDataType_1_0.java
@@ -28,10 +28,12 @@ import org.apache.qpid.server.store.StoredMessage;
 public class MessageMetaDataType_1_0 implements MessageMetaDataType<MessageMetaData_1_0>
 {
 
+    public static final int TYPE = 2;
+
     @Override
     public int ordinal()
     {
-        return 2;
+        return TYPE;
     }
 
     @Override
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/store/MessageMetaDataTypeRegistry.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/store/MessageMetaDataTypeRegistry.java
index 680174d7b9..64f3ab15ee 100755
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/store/MessageMetaDataTypeRegistry.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/store/MessageMetaDataTypeRegistry.java
@@ -29,8 +29,12 @@ public class MessageMetaDataTypeRegistry
 
     static
     {
-        int maxOrdinal = 0;
-        for(MessageMetaDataType type : new QpidServiceLoader<MessageMetaDataType>().instancesOf(MessageMetaDataType.class))
+        int maxOrdinal = -1;
+
+        Iterable<MessageMetaDataType> messageMetaDataTypes =
+                new QpidServiceLoader<MessageMetaDataType>().atLeastOneInstanceOf(MessageMetaDataType.class);
+
+        for(MessageMetaDataType type : messageMetaDataTypes)
         {
             if(type.ordinal()>maxOrdinal)
             {
@@ -40,6 +44,14 @@ public class MessageMetaDataTypeRegistry
         values = new MessageMetaDataType[maxOrdinal+1];
         for(MessageMetaDataType type : new QpidServiceLoader<MessageMetaDataType>().instancesOf(MessageMetaDataType.class))
         {
+            if(values[type.ordinal()] != null)
+            {
+                throw new IllegalStateException("Multiple MessageDataType ("
+                                                 +values[type.ordinal()].getClass().getName()
+                                                 +", "
+                                                 + type.getClass().getName()
+                                                 + ") defined for the same ordinal value: " + type.ordinal());
+            }
             values[type.ordinal()] = type;
         }
     }