QPID-3816: add client version to connection open logging

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

7 files changed, 72 insertions, 28 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties b/java/broker/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties
index 81ae6f3bd0..8559862a45 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties
+++ b/java/broker/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties
@@ -20,5 +20,6 @@
 
 # 0 - Client id
 # 1 - Protocol Version
-OPEN = CON-1001 : Open[ : Client ID : {0}][ : Protocol Version : {1}]
+# 2 - Client Version
+OPEN = CON-1001 : Open[ : Client ID : {0}][ : Protocol Version : {1}][ : Client Version : {2}]
 CLOSE = CON-1002 : Close
\ No newline at end of file
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 670f83899e..1f59091eba 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
@@ -179,7 +179,7 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
         _configStore = virtualHostRegistry.getConfigStore();
         _id = _configStore.createId();
 
-        _actor.message(ConnectionMessages.OPEN(null, null, false, false));
+        _actor.message(ConnectionMessages.OPEN(null, null, null, false, false, false));
 
         _registry = virtualHostRegistry.getApplicationRegistry();
         initialiseStatistics();
@@ -365,7 +365,7 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
         try
         {
             // Log incomming protocol negotiation request
-            _actor.message(ConnectionMessages.OPEN(null, pi.getProtocolMajor() + "-" + pi.getProtocolMinor(), false, true));
+            _actor.message(ConnectionMessages.OPEN(null, pi.getProtocolMajor() + "-" + pi.getProtocolMinor(), null, false, true, false));
 
             ProtocolVersion pv = pi.checkVersion(); // Fails if not correct
 
@@ -923,16 +923,16 @@ public class AMQProtocolEngine implements ServerProtocolEngine, Managable, AMQPr
         _clientProperties = clientProperties;
         if (_clientProperties != null)
         {
+            _clientVersion = _clientProperties.getString(ConnectionStartProperties.VERSION_0_8);
+
             if (_clientProperties.getString(ConnectionStartProperties.CLIENT_ID_0_8) != null)
             {
                 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));
+                _actor.message(ConnectionMessages.OPEN(clientID, _protocolVersion.toString(), _clientVersion, true, true, true));
             }
-
-            _clientVersion = _clientProperties.getString(ConnectionStartProperties.VERSION_0_8);
         }
     }
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/ProtocolEngine_0_10.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/ProtocolEngine_0_10.java
index a64bf77880..182ef1ed82 100755
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/ProtocolEngine_0_10.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/ProtocolEngine_0_10.java
@@ -90,8 +90,8 @@ public class ProtocolEngine_0_10  extends InputHandler implements ServerProtocol
         _connection.setSender(new Disassembler(sender, MAX_FRAME_SIZE));
 
         // FIXME Two log messages to maintain compatibility with earlier protocol versions
-        _connection.getLogActor().message(ConnectionMessages.OPEN(null, null, false, false));
-        _connection.getLogActor().message(ConnectionMessages.OPEN(null, "0-10", false, true));
+        _connection.getLogActor().message(ConnectionMessages.OPEN(null, null, null, false, false, false));
+        _connection.getLogActor().message(ConnectionMessages.OPEN(null, "0-10", null, false, true, false));
     }
 
     public SocketAddress getRemoteAddress()
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 ca5551163f..c38f3d0761 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
@@ -107,7 +107,7 @@ public class ServerConnection extends Connection implements Managable, AMQConnec
             {
                 _onOpenTask.run();    
             }
-            _actor.message(ConnectionMessages.OPEN(getClientId(), "0-10", true, true));
+            _actor.message(ConnectionMessages.OPEN(getClientId(), "0-10", getClientVersion(), true, true, true));
 
             getVirtualHost().getConnectionRegistry().registerConnection(this);
         }
diff --git a/java/broker/src/test/java/org/apache/qpid/server/logging/messages/AbstractTestMessages.java b/java/broker/src/test/java/org/apache/qpid/server/logging/messages/AbstractTestMessages.java
index 11c8c84892..24e7225d82 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/logging/messages/AbstractTestMessages.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/logging/messages/AbstractTestMessages.java
@@ -63,16 +63,21 @@ public abstract class AbstractTestMessages extends InternalBrokerBaseCase
         return _logger.getLogMessages();
     }
 
+    protected void validateLogMessage(List<Object> logs, String tag, String[] expected)
+    {
+        validateLogMessage(logs, tag, false, expected);
+    }
+
     /**
-     * Validate that only a single log messasge occured and that the message
+     * Validate that only a single log message occurred and that the message
      * section starts with the specified tag
      *
      * @param logs     the logs generated during test run
      * @param tag      the tag to check for
      * @param expected the expected log messages
-     *
+     * @param useStringForNull replace a null String reference with "null"
      */
-    protected void validateLogMessage(List<Object> logs, String tag, String[] expected)
+    protected void validateLogMessage(List<Object> logs, String tag, boolean useStringForNull, String[] expected)
     {
         assertEquals("Log has incorrect message count", 1, logs.size());
 
@@ -97,6 +102,10 @@ public abstract class AbstractTestMessages extends InternalBrokerBaseCase
         int index = 0;
         for (String text : expected)
         {
+            if(useStringForNull && text == null)
+            {
+                text = "null";
+            }
             index = message.indexOf(text, index);
             assertTrue("Message does not contain expected (" + text + ") text :" + message, index != -1);
             index = index + text.length();
diff --git a/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java b/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java
index 24fccf8446..b2951ae54a 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java
@@ -27,25 +27,27 @@ import java.util.List;
  */
 public class ConnectionMessagesTest extends AbstractTestMessages
 {
-    public void testConnectionOpen_WithClientIDProtocolVersion()
+    public void testConnectionOpen_WithClientIDProtocolVersionClientVersion()
     {
         String clientID = "client";
         String protocolVersion = "8-0";
+        String clientVersion = "1.2.3_4";
 
-        _logMessage = ConnectionMessages.OPEN(clientID, protocolVersion, true , true);
+        _logMessage = ConnectionMessages.OPEN(clientID, protocolVersion, clientVersion, true , true, true);
         List<Object> log = performLog();
 
         String[] expected = {"Open :", "Client ID", clientID,
-                             ": Protocol Version :", protocolVersion};
+                             ": Protocol Version :", protocolVersion,
+                             ": Client Version :", clientVersion};
 
         validateLogMessage(log, "CON-1001", expected);
     }
 
-    public void testConnectionOpen_WithClientIDNoProtocolVersion()
+    public void testConnectionOpen_WithClientIDNoProtocolVersionNoClientVersion()
     {
         String clientID = "client";        
 
-        _logMessage = ConnectionMessages.OPEN(clientID, null,true, false);
+        _logMessage = ConnectionMessages.OPEN(clientID, null, null, true, false, false);
         List<Object> log = performLog();
 
         String[] expected = {"Open :", "Client ID", clientID};
@@ -53,11 +55,11 @@ public class ConnectionMessagesTest extends AbstractTestMessages
         validateLogMessage(log, "CON-1001", expected);
     }
 
-    public void testConnectionOpen_WithNOClientIDProtocolVersion()
+    public void testConnectionOpen_WithNOClientIDProtocolVersionNoClientVersion()
     {
         String protocolVersion = "8-0";
 
-        _logMessage = ConnectionMessages.OPEN(null, protocolVersion, false , true);
+        _logMessage = ConnectionMessages.OPEN(null, protocolVersion, null, false , true, false);
         List<Object> log = performLog();
 
         String[] expected = {"Open", ": Protocol Version :", protocolVersion};
@@ -65,17 +67,39 @@ public class ConnectionMessagesTest extends AbstractTestMessages
         validateLogMessage(log, "CON-1001", expected);
     }
 
-    public void testConnectionOpen_WithNoClientIDNoProtocolVersion()
+    public void testConnectionOpen_WithNOClientIDNoProtocolVersionClientVersion()
     {
-        _logMessage = ConnectionMessages.OPEN(null, null,false, false);
+        String clientVersion = "1.2.3_4";
+
+        _logMessage = ConnectionMessages.OPEN(null, null, clientVersion, false , false, true);
         List<Object> log = performLog();
 
-        String[] expected = {"Open"};
+        String[] expected = {"Open", ": Client Version :", clientVersion};
 
         validateLogMessage(log, "CON-1001", expected);
     }
 
+    public void testConnectionOpen_WithNOClientIDNoProtocolVersionNullClientVersion()
+    {
+        String clientVersion = null;
+
+        _logMessage = ConnectionMessages.OPEN(null, null, clientVersion , false , false, true);
+        List<Object> log = performLog();
+
+        String[] expected = {"Open", ": Client Version :", clientVersion};
+
+        validateLogMessage(log, "CON-1001", true, expected);
+    }
 
+    public void testConnectionOpen_WithNoClientIDNoProtocolVersionNoClientVersion()
+    {
+        _logMessage = ConnectionMessages.OPEN(null, null, null, false, false, false);
+        List<Object> log = performLog();
+
+        String[] expected = {"Open"};
+
+        validateLogMessage(log, "CON-1001", expected);
+    }
 
     public void testConnectionClose()
     {
diff --git a/java/systests/src/main/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java b/java/systests/src/main/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java
index 2c1cebb9b3..ffe25a5cbe 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java
@@ -21,6 +21,9 @@
 package org.apache.qpid.server.logging;
 
 import javax.jms.Connection;
+
+import org.apache.qpid.common.QpidProperties;
+
 import java.util.HashMap;
 import java.util.List;
 import java.util.TreeSet;
@@ -68,7 +71,7 @@ public class ConnectionLoggingTest extends AbstractTestLogging
 
         // MESSAGE [con:1(/127.0.0.1:46927)] CON-1001 : Open
         // MESSAGE [con:1(/127.0.0.1:46927)] CON-1001 : Open : Protocol Version : 0-9
-        // MESSAGE [con:1(/127.0.0.1:46927)] CON-1001 : Open : Client ID : clientid : Protocol Version : 0-9
+        // MESSAGE [con:1(/127.0.0.1:46927)] CON-1001 : Open : Client ID : clientid : Protocol Version : 0-9 : Client Version : 1.2.3_4
         // MESSAGE [con:0(/127.0.0.1:46927)] CON-1002 : Close
 
         HashMap<Integer, List<String>> connectionData = splitResultsOnConnectionID(results);
@@ -87,18 +90,18 @@ public class ConnectionLoggingTest extends AbstractTestLogging
         validateMessageID("CON-1001",log);
 
         // validate the last three CON-1001 messages.
-        //  MESSAGE [con:1(/127.0.0.1:52540)] CON-1001 : Open : Client ID : clientid : Protocol Version : 0-9
-        validateConnectionOpen(results, 0, true, true, clientid);
+        //  MESSAGE [con:1(/127.0.0.1:52540)] CON-1001 : Open : Client ID : clientid : Protocol Version : 0-9 : Client Version : 1.2.3_4
+        validateConnectionOpen(results, 0, true, true, clientid, true, QpidProperties.getReleaseVersion());
 
         //  MESSAGE [con:1(/127.0.0.1:52540)] CON-1001 : Open : Protocol Version : 0-9
-        validateConnectionOpen(results, 1, true, false, null);
+        validateConnectionOpen(results, 1, true, false, null, false, null);
 
         //  MESSAGE [con:1(/127.0.0.1:52540)] CON-1001 : Open
-        validateConnectionOpen(results, 2, false, false, null);
+        validateConnectionOpen(results, 2, false, false, null, false, null);
     }
     
     private void validateConnectionOpen(List<String> results, int positionFromEnd,
-                 boolean protocolVersionPresent, boolean clientIdOptionPresent, String clientIdValue)
+                 boolean protocolVersionPresent, boolean clientIdOptionPresent, String clientIdValue, boolean clientVersionPresent, String clientVersionValue)
     {
         String log = getLogMessageFromEnd(results, positionFromEnd);
         
@@ -115,6 +118,13 @@ public class ConnectionLoggingTest extends AbstractTestLogging
                 protocolVersionPresent, fromMessage(log).contains("Protocol Version :"));
         //fixme there is no way currently to find out the negotiated protocol version
         // The delegate is the versioned class ((AMQConnection)connection)._delegate
+
+        assertEquals("unexpected Client ID option state", clientVersionPresent, fromMessage(log).contains("Client Version :"));
+
+        if(clientVersionPresent && clientVersionValue != null)
+        {
+            assertTrue("Client version value is not present: " + clientVersionValue, fromMessage(log).contains(clientVersionValue));
+        }
     }
 
     /**