QPID-3814: ensure the 0-10 client sends its version number during ConnectionStart(Ok) process, align properties better across protocol versions, general tidy up of the property handling

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

9 files changed, 48 insertions, 73 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionStartOkMethodHandler.java b/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionStartOkMethodHandler.java
index d6e4569e44..162e4e0215 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionStartOkMethodHandler.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionStartOkMethodHandler.java
@@ -80,10 +80,7 @@ public class ConnectionStartOkMethodHandler implements StateAwareMethodListener<
 
             final AuthenticationResult authResult = authMgr.authenticate(ss, body.getResponse());
             //save clientProperties
-            if (session.getClientProperties() == null)
-            {
-                session.setClientProperties(body.getClientProperties());
-            }
+            session.setClientProperties(body.getClientProperties());
 
             MethodRegistry methodRegistry = session.getMethodRegistry();
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java
index 93954108fa..670f83899e 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolEngine.java
@@ -27,8 +27,8 @@ import org.apache.qpid.AMQConnectionException;
 import org.apache.qpid.AMQException;
 import org.apache.qpid.AMQSecurityException;
 import org.apache.qpid.codec.AMQCodecFactory;
-import org.apache.qpid.common.ClientProperties;
 import org.apache.qpid.framing.*;
+import org.apache.qpid.properties.ConnectionStartProperties;
 import org.apache.qpid.protocol.AMQConstant;
 import org.apache.qpid.protocol.AMQMethodEvent;
 import org.apache.qpid.protocol.AMQMethodListener;
@@ -87,8 +87,6 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
 {
     private static final Logger _logger = Logger.getLogger(AMQProtocolEngine.class);
 
-    private static final String CLIENT_PROPERTIES_INSTANCE = ClientProperties.instance.toString();
-
     // to save boxing the channelId and looking up in a map... cache in an array the low numbered
     // channels.  This value must be of the form 2^x - 1.
     private static final int CHANNEL_CACHE_SIZE = 0xff;
@@ -96,7 +94,7 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
 
     private AMQShortString _contextKey;
 
-    private AMQShortString _clientVersion = null;
+    private String _clientVersion = null;
 
     private VirtualHost _virtualHost;
 
@@ -133,7 +131,6 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
     private ProtocolOutputConverter _protocolOutputConverter;
     private Subject _authorizedSubject;
     private MethodDispatcher _dispatcher;
-    private ProtocolSessionIdentifier _sessionIdentifier;
 
     private final long _sessionID;
 
@@ -921,31 +918,22 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
         _saslServer = saslServer;
     }
 
-    public FieldTable getClientProperties()
-    {
-        return _clientProperties;
-    }
-
     public void setClientProperties(FieldTable clientProperties)
     {
         _clientProperties = clientProperties;
         if (_clientProperties != null)
         {
-            if (_clientProperties.getString(CLIENT_PROPERTIES_INSTANCE) != null)
+            if (_clientProperties.getString(ConnectionStartProperties.CLIENT_ID_0_8) != null)
             {
-                String clientID = _clientProperties.getString(CLIENT_PROPERTIES_INSTANCE);
+                String clientID = _clientProperties.getString(ConnectionStartProperties.CLIENT_ID_0_8);
                 setContextKey(new AMQShortString(clientID));
 
                 // Log the Opening of the connection for this client
                 _actor.message(ConnectionMessages.OPEN(clientID, _protocolVersion.toString(), true, true));
             }
 
-            if (_clientProperties.getString(ClientProperties.version.toString()) != null)
-            {
-                _clientVersion = new AMQShortString(_clientProperties.getString(ClientProperties.version.toString()));
-            }
+            _clientVersion = _clientProperties.getString(ConnectionStartProperties.VERSION_0_8);
         }
-        _sessionIdentifier = new ProtocolSessionIdentifier(this);
     }
 
     private void setProtocolVersion(ProtocolVersion pv)
@@ -1154,14 +1142,9 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
         return _lastReceivedTime;
     }
 
-    public ProtocolSessionIdentifier getSessionIdentifier()
-    {
-        return _sessionIdentifier;
-    }
-
     public String getClientVersion()
     {
-        return (_clientVersion == null) ? null : _clientVersion.toString();
+        return _clientVersion;
     }
 
     public Boolean isIncoming()
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSession.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSession.java
index d0dd78052f..b68f6097e0 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSession.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSession.java
@@ -20,9 +20,13 @@
  */
 package org.apache.qpid.server.protocol;
 
+import java.util.List;
+
+import javax.security.auth.Subject;
+import javax.security.sasl.SaslServer;
+
 import org.apache.qpid.AMQConnectionException;
 import org.apache.qpid.AMQException;
-import org.apache.qpid.common.ClientProperties;
 import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.framing.FieldTable;
 import org.apache.qpid.framing.MethodDispatcher;
@@ -35,10 +39,6 @@ import org.apache.qpid.server.security.AuthorizationHolder;
 import org.apache.qpid.server.subscription.ClientDeliveryMethod;
 import org.apache.qpid.server.virtualhost.VirtualHost;
 
-import javax.security.auth.Subject;
-import javax.security.sasl.SaslServer;
-import java.util.List;
-
 
 public interface AMQProtocolSession extends AMQVersionAwareProtocolSession, AuthorizationHolder, AMQConnectionModel
 {
@@ -60,28 +60,6 @@ public interface AMQProtocolSession extends AMQVersionAwareProtocolSession, Auth
 
     long getLastReceivedTime();
 
-    public static final class ProtocolSessionIdentifier
-    {
-        private final Object _sessionIdentifier;
-        private final Object _sessionInstance;
-
-        ProtocolSessionIdentifier(AMQProtocolSession session)
-        {
-            _sessionIdentifier = session.getClientIdentifier();
-            _sessionInstance = session.getClientProperties() == null ? null : session.getClientProperties().getObject(ClientProperties.instance.toAMQShortString());
-        }
-
-        public Object getSessionIdentifier()
-        {
-            return _sessionIdentifier;
-        }
-
-        public Object getSessionInstance()
-        {
-            return _sessionInstance;
-        }
-    }
-
     public static interface Task
     {
         public void doTask(AMQProtocolSession session) throws AMQException;
@@ -192,9 +170,6 @@ public interface AMQProtocolSession extends AMQVersionAwareProtocolSession, Auth
      */
     void setSaslServer(SaslServer saslServer);
 
-
-    FieldTable getClientProperties();
-
     void setClientProperties(FieldTable clientProperties);
 
     Object getClientIdentifier();
@@ -217,8 +192,6 @@ public interface AMQProtocolSession extends AMQVersionAwareProtocolSession, Auth
 
     public MethodDispatcher getMethodDispatcher();
 
-    public ProtocolSessionIdentifier getSessionIdentifier();
-
     String getClientVersion();
 
     long getLastIoTime();
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java
index 105aa1f260..e70720600e 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQProtocolSessionMBean.java
@@ -92,7 +92,7 @@ public class AMQProtocolSessionMBean extends AbstractAMQManagedConnectionObject
 
     public String getVersion()
     {
-        return (_protocolSession.getClientVersion() == null) ? null : _protocolSession.getClientVersion().toString();
+        return _protocolSession.getClientVersion();
     }
 
     public Date getLastIoTime()
diff --git a/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionImpl.java b/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionImpl.java
index c410b822a5..0f6bc976de 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionImpl.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/subscription/SubscriptionImpl.java
@@ -24,7 +24,6 @@ import org.apache.log4j.Logger;
 
 import org.apache.qpid.AMQException;
 import org.apache.qpid.common.AMQPFilterTypes;
-import org.apache.qpid.common.ClientProperties;
 import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.framing.FieldTable;
 import org.apache.qpid.server.AMQChannel;
@@ -320,9 +319,6 @@ public abstract class SubscriptionImpl implements Subscription, FlowCreditManage
 
     private final Boolean _autoClose;
 
-
-    private static final String CLIENT_PROPERTIES_INSTANCE = ClientProperties.instance.toString();
-
     private AMQQueue _queue;
     private final AtomicBoolean _deleted = new AtomicBoolean(false);
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java
index 2e458b22b4..ca5551163f 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java
@@ -488,4 +488,14 @@ public class ServerConnection extends Connection implements Managable, AMQConnec
             _mBean = null;
         }
     }
+
+    public String getClientId()
+    {
+        return getConnectionDelegate().getClientId();
+    }
+
+    public String getClientVersion()
+    {
+        return getConnectionDelegate().getClientVersion();
+    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
index eb845d7cfb..0bf65d388b 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
@@ -21,6 +21,7 @@
 package org.apache.qpid.server.transport;
 
 import org.apache.qpid.common.ServerPropertyNames;
+import org.apache.qpid.properties.ConnectionStartProperties;
 import org.apache.qpid.protocol.ProtocolEngine;
 import org.apache.qpid.server.configuration.BrokerConfig;
 import org.apache.qpid.server.protocol.AMQConnectionModel;
@@ -290,4 +291,14 @@ public class ServerConnectionDelegate extends ServerDelegate
     {
         return _clientProperties;
     }
-}
+
+    public String getClientId()
+    {
+        return _clientProperties == null ? null : (String) _clientProperties.get(ConnectionStartProperties.CLIENT_ID_0_10);
+    }
+
+    public String getClientVersion()
+    {
+        return _clientProperties == null ? null : (String) _clientProperties.get(ConnectionStartProperties.VERSION_0_10);
+    }
+}
\ No newline at end of file
diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionMBean.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionMBean.java
index 3bf799cba6..bb545164fb 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionMBean.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionMBean.java
@@ -20,7 +20,6 @@
  */
 package org.apache.qpid.server.transport;
 
-import org.apache.qpid.common.ClientProperties;
 import org.apache.qpid.management.common.mbeans.annotations.MBeanConstructor;
 import org.apache.qpid.management.common.mbeans.annotations.MBeanDescription;
 import org.apache.qpid.server.logging.actors.CurrentActor;
@@ -76,7 +75,7 @@ public class ServerConnectionMBean extends AbstractAMQManagedConnectionObject
     @Override
     public String getVersion()
     {
-        return String.valueOf(_serverConnection.getConnectionDelegate().getClientProperties().get(ClientProperties.version.toString()));
+        return String.valueOf(_serverConnection.getClientVersion());
     }
 
     @Override
diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
index d3ce94bdc7..5a208aaeaf 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerSession.java
@@ -526,12 +526,12 @@ public class ServerSession extends Session
     
     public Principal getAuthorizedPrincipal()
     {
-        return ((ServerConnection) getConnection()).getAuthorizedPrincipal();
+        return getConnection().getAuthorizedPrincipal();
     }
     
     public Subject getAuthorizedSubject()
     {
-        return ((ServerConnection) getConnection()).getAuthorizedSubject();
+        return getConnection().getAuthorizedSubject();
     }
 
     public void addSessionCloseTask(Task task)
@@ -546,7 +546,7 @@ public class ServerSession extends Session
 
     public Object getReference()
     {
-        return ((ServerConnection) getConnection()).getReference();
+        return getConnection().getReference();
     }
 
     public MessageStore getMessageStore()
@@ -626,7 +626,7 @@ public class ServerSession extends Session
 
     public AMQConnectionModel getConnectionModel()
     {
-        return (ServerConnection) getConnection();
+        return getConnection();
     }
 
     public String getClientID()
@@ -634,6 +634,12 @@ public class ServerSession extends Session
         return getConnection().getClientId();
     }
 
+    @Override
+    public ServerConnection getConnection()
+    {
+        return (ServerConnection) super.getConnection();
+    }
+
     public LogActor getLogActor()
     {
         return _actor;
@@ -715,8 +721,8 @@ public class ServerSession extends Session
 
     public String toLogString()
     {
-        long connectionId = getConnection() instanceof ServerConnection
-                            ? ((ServerConnection) getConnection()).getConnectionId()
+        long connectionId = super.getConnection() instanceof ServerConnection
+                            ? getConnection().getConnectionId()
                             : -1;
 
         String remoteAddress = _connectionConfig instanceof ProtocolEngine