You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2015/11/07 21:24:09 UTC

svn commit: r1713171 - in /qpid/java/trunk: broker-core/src/main/java/org/apache/qpid/server/logging/messages/ broker-core/src/main/java/org/apache/qpid/server/model/ broker-core/src/main/java/org/apache/qpid/server/transport/ broker-core/src/test/java...

Author: kwall
Date: Sat Nov  7 20:24:09 2015
New Revision: 1713171

URL: http://svn.apache.org/viewvc?rev=1713171&view=rev
Log:
QPID-6824: Include Broker host/port number in CON-1001 message

* included whether the connection is using TLS or not
* reduced the number of CON-1001 messages from three to two to reduce log clutter. The first is now logged imediately after the arrival of the protocol header, the second
  is logged once the user and target virtual host is known.

Modified:
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/ConnectionMessages.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/Protocol.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/transport/AbstractAMQPConnection.java
    qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java
    qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/AMQPConnection_0_10.java
    qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
    qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8.java
    qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/ConnectionMessages.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/ConnectionMessages.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/ConnectionMessages.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/ConnectionMessages.java Sat Nov  7 20:24:09 2015
@@ -69,12 +69,12 @@ public class ConnectionMessages
 
     /**
      * Log a Connection message of the Format:
-     * <pre>CON-1001 : Open[ : Client ID : {0}][ : Protocol Version : {1}][ : Client Version : {2}][ : Client Product : {3}]</pre>
+     * <pre>CON-1001 : Open : Port : {0}({1}) Protocol Version {2}[ : SSL][ : Client ID : {3}][ : Client Version : {4}][ : Client Product : {5}]</pre>
      * Optional values are contained in [square brackets] and are numbered
      * sequentially in the method call.
      *
      */
-    public static LogMessage OPEN(String param1, String param2, String param3, String param4, boolean opt1, boolean opt2, boolean opt3, boolean opt4)
+    public static LogMessage OPEN(String param1, String param2, String param3, String param4, String param5, String param6, boolean opt1, boolean opt2, boolean opt3, boolean opt4)
     {
         String rawMessage = _messages.getString("OPEN");
         StringBuffer msg = new StringBuffer();
@@ -88,7 +88,7 @@ public class ConnectionMessages
         if (parts.length > 1)
         {
 
-            // Add Option : : Client ID : {0}.
+            // Add Option : : SSL.
             end = parts[1].indexOf(']');
             if (opt1)
             {
@@ -98,7 +98,7 @@ public class ConnectionMessages
             // Use 'end + 1' to remove the ']' from the output
             msg.append(parts[1].substring(end + 1));
 
-            // Add Option : : Protocol Version : {1}.
+            // Add Option : : Client ID : {3}.
             end = parts[2].indexOf(']');
             if (opt2)
             {
@@ -108,7 +108,7 @@ public class ConnectionMessages
             // Use 'end + 1' to remove the ']' from the output
             msg.append(parts[2].substring(end + 1));
 
-            // Add Option : : Client Version : {2}.
+            // Add Option : : Client Version : {4}.
             end = parts[3].indexOf(']');
             if (opt3)
             {
@@ -118,7 +118,7 @@ public class ConnectionMessages
             // Use 'end + 1' to remove the ']' from the output
             msg.append(parts[3].substring(end + 1));
 
-            // Add Option : : Client Product : {3}.
+            // Add Option : : Client Product : {5}.
             end = parts[4].indexOf(']');
             if (opt4)
             {
@@ -131,7 +131,7 @@ public class ConnectionMessages
 
         rawMessage = msg.toString();
 
-        final Object[] messageArguments = {param1, param2, param3, param4};
+        final Object[] messageArguments = {param1, param2, param3, param4, param5, param6};
         // Create a new MessageFormat to ensure thread safety.
         // Sharing a MessageFormat and using applyPattern is not thread safe
         MessageFormat formatter = new MessageFormat(rawMessage, _currentLocale);

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/messages/Connection_logmessages.properties Sat Nov  7 20:24:09 2015
@@ -18,10 +18,7 @@
 #
 # Default File used for all non-defined locales.
 
-# 0 - Client id
-# 1 - Protocol Version
-# 2 - Client Version
-OPEN = CON-1001 : Open[ : Client ID : {0}][ : Protocol Version : {1}][ : Client Version : {2}][ : Client Product : {3}]
+OPEN = CON-1001 : Open : Destination : {0}({1}) : Protocol Version : {2}[ : SSL][ : Client ID : {3}][ : Client Version : {4}][ : Client Product : {5}]
 CLOSE = CON-1002 : Close
 IDLE_CLOSE = CON-1003 : Closed due to inactivity
 DROPPED_CONNECTION = CON-1004 : Connection dropped

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/Protocol.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/Protocol.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/Protocol.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/Protocol.java Sat Nov  7 20:24:09 2015
@@ -20,25 +20,30 @@
  */
 package org.apache.qpid.server.model;
 
-import java.util.Collection;
-import java.util.EnumSet;
-
 public enum Protocol
 {
-    AMQP_0_8(ProtocolType.AMQP),
-    AMQP_0_9(ProtocolType.AMQP),
-    AMQP_0_9_1(ProtocolType.AMQP),
-    AMQP_0_10(ProtocolType.AMQP),
-    AMQP_1_0(ProtocolType.AMQP),
+    AMQP_0_8(ProtocolType.AMQP, "0-8"),
+    AMQP_0_9(ProtocolType.AMQP, "0-9"),
+    AMQP_0_9_1(ProtocolType.AMQP, "0-9-1"),
+    AMQP_0_10(ProtocolType.AMQP, "0-10"),
+    AMQP_1_0(ProtocolType.AMQP, "1.0"),
     JMX_RMI(ProtocolType.JMX),
     HTTP(ProtocolType.HTTP),
     RMI(ProtocolType.RMI);
 
     private final ProtocolType _protocolType;
 
-    private Protocol(ProtocolType type)
+    private final String _protocolVersion;
+
+    Protocol(ProtocolType type)
+    {
+        this(type, null);
+    }
+
+    Protocol(ProtocolType type, String version)
     {
         _protocolType =  type;
+        _protocolVersion = version;
     }
 
     public ProtocolType getProtocolType()
@@ -46,46 +51,17 @@ public enum Protocol
         return _protocolType;
     }
 
-    public boolean isAMQP()
+    public String getProtocolVersion()
     {
-        return _protocolType == ProtocolType.AMQP;
+        return _protocolVersion;
     }
 
-    public static Protocol valueOfObject(Object protocolObject)
+    public boolean isAMQP()
     {
-        Protocol protocol;
-        if (protocolObject instanceof Protocol)
-        {
-            protocol = (Protocol) protocolObject;
-        }
-        else
-        {
-            try
-            {
-                protocol = Protocol.valueOf(String.valueOf(protocolObject));
-            }
-            catch (Exception e)
-            {
-                throw new IllegalArgumentException("Can't convert '" + protocolObject
-                        + "' to one of the supported protocols: " + EnumSet.allOf(Protocol.class), e);
-            }
-        }
-        return protocol;
-    }
-
-    public static boolean hasAmqpProtocol(Collection<Protocol> protocols)
-    {
-        for (Protocol protocol : protocols)
-        {
-            if (protocol.isAMQP())
-            {
-                return true;
-            }
-        }
-        return false;
+        return _protocolType == ProtocolType.AMQP;
     }
 
-    public static enum ProtocolType
+    public enum ProtocolType
     {
         AMQP, HTTP, JMX, RMI
     }

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/transport/AbstractAMQPConnection.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/transport/AbstractAMQPConnection.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/transport/AbstractAMQPConnection.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/transport/AbstractAMQPConnection.java Sat Nov  7 20:24:09 2015
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.transport;
 
+import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.security.AccessControlContext;
 import java.security.AccessController;
@@ -443,7 +444,7 @@ public abstract class AbstractAMQPConnec
         return Thread.currentThread() == _ioThread;
     }
 
-    private <T> T runAsSubject(PrivilegedAction<T> action)
+    protected <T> T runAsSubject(PrivilegedAction<T> action)
     {
         return Subject.doAs(_subject, action);
     }
@@ -598,6 +599,39 @@ public abstract class AbstractAMQPConnec
                 getSubject());
     }
 
+    protected void logConnectionOpen()
+    {
+        runAsSubject(new PrivilegedAction<Object>()
+        {
+            @Override
+            public Object run()
+            {
+                SocketAddress localAddress = _network.getLocalAddress();
+                final String localAddressStr;
+                if (localAddress instanceof InetSocketAddress)
+                {
+                    InetSocketAddress inetAddress = (InetSocketAddress) localAddress;
+                    localAddressStr = inetAddress.getAddress().getHostAddress() + ":" + inetAddress.getPort();
+                }
+                else
+                {
+                    localAddressStr = localAddress.toString();
+                }
+                getEventLogger().message(ConnectionMessages.OPEN(getPort().getName(),
+                                                                 localAddressStr,
+                                                                 getProtocol().getProtocolVersion(),
+                                                                 getClientId(),
+                                                                 getClientVersion(),
+                                                                 getClientProduct(),
+                                                                 getTransport().isSecure(),
+                                                                 getClientId() != null,
+                                                                 getClientVersion() != null,
+                                                                 getClientProduct() != null));
+                return null;
+            }
+        });
+    }
+
     public abstract List<? extends AMQSessionModel<?>> getSessionModels();
 
     @Override

Modified: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java (original)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java Sat Nov  7 20:24:09 2015
@@ -27,93 +27,61 @@ import java.util.List;
  */
 public class ConnectionMessagesTest extends AbstractTestMessages
 {
-    public void testConnectionOpen_WithClientIDProtocolVersionClientVersionClientProduct()
+    public void testConnectionOpen_WithOptional()
     {
+        String port = "myport";
+        String local = "127.0.0.1:5672";
+        String protocolVersion = "0-8";
         String clientID = "client";
-        String protocolVersion = "8-0";
         String clientVersion = "1.2.3_4";
         String clientProduct = "myProduct";
 
-        _logMessage = ConnectionMessages.OPEN(clientID, protocolVersion, clientVersion, clientProduct , true, true, true, true);
+        _logMessage = ConnectionMessages.OPEN(port, local, protocolVersion, clientID, clientVersion, clientProduct , false, true, true, true);
         List<Object> log = performLog();
 
-        String[] expected = {"Open :", "Client ID", clientID,
+        String[] expected = {"Open",
+                             ": Destination", "myport(127.0.0.1:5672)",
                              ": Protocol Version :", protocolVersion,
+                             ": Client ID", clientID,
                              ": Client Version :", clientVersion,
                              ": Client Product :", clientProduct};
 
         validateLogMessage(log, "CON-1001", expected);
     }
 
-    public void testConnectionOpen_WithClientIDNoProtocolVersionNoClientVersionNoClientProduct()
+    public void testConnectionOpen()
     {
-        String clientID = "client";        
+        String port = "myport";
+        String local = "127.0.0.1:5672";
+        String protocolVersion = "0-8";
 
-        _logMessage = ConnectionMessages.OPEN(clientID, null, null, null, true, false, false, false);
+        _logMessage = ConnectionMessages.OPEN(port, local, protocolVersion, null, null, null , false, false, false, false);
         List<Object> log = performLog();
 
-        String[] expected = {"Open :", "Client ID", clientID};
+        String[] expected = {"Open",
+                ": Destination", "myport(127.0.0.1:5672)",
+                ": Protocol Version :", protocolVersion};
 
         validateLogMessage(log, "CON-1001", expected);
     }
 
-    public void testConnectionOpen_WithNOClientIDProtocolVersionNoClientVersionNoClientProduct()
+    public void testSslConnectionOpen()
     {
-        String protocolVersion = "8-0";
+        String port = "myport";
+        String local = "127.0.0.1:5672";
+        String protocolVersion = "0-8";
 
-        _logMessage = ConnectionMessages.OPEN(null, protocolVersion, null, null , false, true, false, false);
+        _logMessage = ConnectionMessages.OPEN(port, local, protocolVersion, null, null, null , true, false, false, false);
         List<Object> log = performLog();
 
-        String[] expected = {"Open", ": Protocol Version :", protocolVersion};
+        String[] expected = {"Open",
+                ": Destination", "myport(127.0.0.1:5672)",
+                ": Protocol Version :", protocolVersion,
+                ": SSL"};
 
         validateLogMessage(log, "CON-1001", expected);
     }
 
-    public void testConnectionOpen_WithNOClientIDNoProtocolVersionClientVersionNoClientProduct()
-    {
-        String clientVersion = "1.2.3_4";
-
-        _logMessage = ConnectionMessages.OPEN(null, null, clientVersion, null , false, false, true, false);
-        List<Object> log = performLog();
-
-        String[] expected = {"Open", ": Client Version :", clientVersion};
-
-        validateLogMessage(log, "CON-1001", expected);
-    }
-
-    public void testConnectionOpen_WithNOClientIDNoProtocolVersionNullClientVersionNoClientProduct()
-    {
-        String clientVersion = null;
-
-        _logMessage = ConnectionMessages.OPEN(null, null, clientVersion , null , false, false, true, false);
-        List<Object> log = performLog();
-
-        String[] expected = {"Open", ": Client Version :", clientVersion};
-
-        validateLogMessage(log, "CON-1001", true, expected);
-    }
-
-    public void testConnectionOpen_WithNOClientIDNoProtocolVersionNoClientVersionNullClientProduct()
-    {
-        String clientProduct = null;
-
-        _logMessage = ConnectionMessages.OPEN(null, null, null , clientProduct , false, false, false, true);
-        List<Object> log = performLog();
-
-        String[] expected = {"Open", ": Client Product :", clientProduct};
-
-        validateLogMessage(log, "CON-1001", true, expected);
-    }
-
-    public void testConnectionOpen_WithNoClientIDNoProtocolVersionNoClientVersionNoClientProduct()
-    {
-        _logMessage = ConnectionMessages.OPEN(null, null, null, null, false, false, false, false);
-        List<Object> log = performLog();
-
-        String[] expected = {"Open"};
-
-        validateLogMessage(log, "CON-1001", expected);
-    }
 
     public void testConnectionClose()
     {

Modified: qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/AMQPConnection_0_10.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/AMQPConnection_0_10.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/AMQPConnection_0_10.java (original)
+++ qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/AMQPConnection_0_10.java Sat Nov  7 20:24:09 2015
@@ -107,20 +107,14 @@ public class AMQPConnection_0_10 extends
             @Override
             public Object run()
             {
-                _connection.getEventLogger()
-                        .message(ConnectionMessages.OPEN(null, null, null, null, false, false, false, false));
-
                 _connection.setNetworkConnection(_network);
                 _disassembler = new ServerDisassembler(wrapSender(_network.getSender()), Constant.MIN_MAX_FRAME_SIZE);
                 _connection.setSender(_disassembler);
                 _connection.addFrameSizeObserver(_disassembler);
-                // FIXME Two log messages to maintain compatibility with earlier protocol versions
-                _connection.getEventLogger()
-                        .message(ConnectionMessages.OPEN(null, "0-10", null, null, false, true, false, false));
-
                 return null;
             }
         }, getAccessControllerContext());
+        logConnectionOpen();
     }
 
     private ByteBufferSender wrapSender(final ByteBufferSender sender)
@@ -350,4 +344,9 @@ public class AMQPConnection_0_10 extends
         return _connection.getEventLogger();
     }
 
+    @Override
+    public void logConnectionOpen()
+    {
+        super.logConnectionOpen();
+    }
 }

Modified: qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java (original)
+++ qpid/java/trunk/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java Sat Nov  7 20:24:09 2015
@@ -140,15 +140,7 @@ public class ServerConnection extends Co
 
         if (state == State.OPEN)
         {
-            getEventLogger().message(ConnectionMessages.OPEN(getClientId(),
-                    "0-10",
-                    getClientVersion(),
-                    getClientProduct(),
-                    true,
-                    true,
-                    true,
-                    true));
-
+            _amqpConnection.logConnectionOpen();
             _amqpConnection.virtualHostAssociated();
         }
 

Modified: qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8.java (original)
+++ qpid/java/trunk/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQPConnection_0_8.java Sat Nov  7 20:24:09 2015
@@ -202,24 +202,9 @@ public class AMQPConnection_0_8
 
         _network = network;
         _sender = network.getSender();
-        runAsSubject(new PrivilegedAction<Void>()
-        {
-
-            @Override
-            public Void run()
-            {
-
-                getEventLogger().message(ConnectionMessages.OPEN(null, null, null, null, false, false, false, false));
-
-                return null;
-            }
-        });
         _closeWhenNoRoute = getBroker().getConnection_closeWhenNoRoute();
-    }
 
-    private <T> T runAsSubject(PrivilegedAction<T> action)
-    {
-        return AccessController.doPrivileged(action, getAccessControllerContext());
+        logConnectionOpen();
     }
 
     @Override
@@ -344,18 +329,7 @@ public class AMQPConnection_0_8
         _decoder.setExpectProtocolInitiation(false);
         try
         {
-            // Log incoming protocol negotiation request
-            getEventLogger().message(ConnectionMessages.OPEN(null,
-                                                             pi.getProtocolMajor() + "-" + pi.getProtocolMinor(),
-                                                             null,
-                                                             null,
-                                                             false,
-                                                             true,
-                                                             false,
-                                                             false));
-
             ProtocolVersion pv = pi.checkVersion(); // Fails if not correct
-
             setProtocolVersion(pv);
 
             StringBuilder mechanismBuilder = new StringBuilder();
@@ -744,15 +718,6 @@ public class AMQPConnection_0_8
             setClientProduct(clientProduct);
             setRemoteProcessPid(remoteProcessPid);
             setClientId(clientId == null ? UUID.randomUUID().toString() : clientId);
-
-            getEventLogger().message(ConnectionMessages.OPEN(getClientId(),
-                                                             _protocolVersion.toString(),
-                                                             getClientVersion(),
-                                                             getClientProduct(),
-                                                             true,
-                                                             true,
-                                                             true,
-                                                             true));
         }
     }
 
@@ -802,6 +767,7 @@ public class AMQPConnection_0_8
         getSubject().getPrincipals().add(virtualHost.getPrincipal());
 
         updateAccessControllerContext();
+        logConnectionOpen();
     }
 
     public ProtocolOutputConverter getProtocolOutputConverter()

Modified: qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java?rev=1713171&r1=1713170&r2=1713171&view=diff
==============================================================================
--- qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java (original)
+++ qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/logging/ConnectionLoggingTest.java Sat Nov  7 20:24:09 2015
@@ -24,8 +24,8 @@ import javax.jms.Connection;
 
 import org.apache.qpid.configuration.CommonProperties;
 
-import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.TreeSet;
 
 public class ConnectionLoggingTest extends AbstractTestLogging
@@ -43,7 +43,7 @@ public class ConnectionLoggingTest exten
      * 1. Running Broker
      * 2. Connecting client
      * Output:
-     * <date> CON-1001 : Open : Client ID {0}[ : Protocol Version : {1}] <version>
+     * <date> CON-1001 : Open ....
      *
      * Validation Steps:
      * 1. The CON ID is correct
@@ -69,69 +69,81 @@ public class ConnectionLoggingTest exten
 
         List<String> results = waitAndFindMatches("CON-1001");
 
-        // 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 : Client Version : 1.2.3_4
-        // MESSAGE [con:0(/127.0.0.1:46927)] CON-1002 : Close
+        // MESSAGE [con:1(/127.0.0.1:46927)] CON-1001 : Open : Destination : amqp(127.0.0.1:15672) : Protocol Version : 0-10
+        // MESSAGE [con:1(guest@/127.0.0.1:46927/test)] CON-1001 : Open : Destination : amqp(127.0.0.1:15672) : Protocol Version : 0-10 : Client ID : clientid : Client Version : 6.0.0-SNAPSHOT : Client Product : qpid
+        // MESSAGE [con:1(guest@/127.0.0.1:46927/test)] CON-1002 : Close
 
-        HashMap<Integer, List<String>> connectionData = splitResultsOnConnectionID(results);
+        Map<Integer, List<String>> connectionData = splitResultsOnConnectionID(results);
 
         // Get the last Integer from keySet of the ConnectionData
-        int connectionID = new TreeSet<Integer>(connectionData.keySet()).last();
+        int connectionID = new TreeSet<>(connectionData.keySet()).last();
 
         //Use just the data from the last connection for the test
         results = connectionData.get(connectionID);
 
-	    assertEquals("Unexpected CON-1001 messages count", 3, results.size());
+	    assertEquals("Unexpected CON-1001 messages count", 2, results.size());
 
-        String log = getLogMessage(results, 0);
-        //  MESSAGE [con:1(/127.0.0.1:52540)] CON-1001 : Open
-        //1 & 2
-        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 : Client Version : 1.2.3_4 : Client Product : product
-        validateConnectionOpen(results, 0, true, true, clientid, true, CommonProperties.getReleaseVersion(), true, CommonProperties.getProductName());
-
-        //  MESSAGE [con:1(/127.0.0.1:52540)] CON-1001 : Open : Protocol Version : 0-9
-        validateConnectionOpen(results, 1, true, false, null, false, null, false, null);
-
-        //  MESSAGE [con:1(/127.0.0.1:52540)] CON-1001 : Open
-        validateConnectionOpen(results, 2, false, false, null, false, null, false, null);
+        // validate the two CON-1001 messages.
+        // MESSAGE [con:1(guest@/127.0.0.1:46927/test)] CON-1001 : Open : Destination : amqp(127.0.0.1:15672) : Protocol Version : 0-10 : Client ID : clientid : Client Version : 6.0.0-SNAPSHOT : Client Product : qpid
+        validateConnectionOpen(results, 0,
+                               true, getBrokerProtocol().getProtocolVersion(),
+                               true, clientid,
+                               true, CommonProperties.getReleaseVersion(),
+                               true, CommonProperties.getProductName());
+
+        // MESSAGE [con:1(/127.0.0.1:46927)] CON-1001 : Open : Destination : amqp(127.0.0.1:15672) : Protocol Version : 0-10
+        validateConnectionOpen(results, 1,
+                               true, getBrokerProtocol().getProtocolVersion(),
+                               false, null,
+                               false, null,
+                               false, null);
     }
     
     private void validateConnectionOpen(List<String> results, int positionFromEnd,
-                 boolean protocolVersionPresent, boolean clientIdOptionPresent, String clientIdValue,
-                 boolean clientVersionPresent, String clientVersionValue, boolean clientProductPresent, String clientProductValue)
+                                        boolean protocolVersionPresent, final String protocolVersionValue,
+                                        boolean clientIdOptionPresent, String clientIdValue,
+                                        boolean clientVersionPresent, String clientVersionValue,
+                                        boolean clientProductPresent, String clientProductValue)
     {
         String log = getLogMessageFromEnd(results, positionFromEnd);
         
-        validateMessageID("CON-1001",log);
-        
-        assertEquals("unexpected Client ID option state", clientIdOptionPresent, fromMessage(log).contains("Client ID :"));
-        
+        validateMessageID("CON-1001", log);
+
+        String message = fromMessage(log);
+
+        assertTrue("Destination not present", message.contains("Destination :"));
+
+        assertEquals("unexpected Protocol Version option state",
+                     protocolVersionPresent, message.contains("Protocol Version :"));
+        if(protocolVersionPresent && protocolVersionValue != null)
+        {
+            assertTrue("Protocol Version value is not present: " + protocolVersionValue, message.contains(protocolVersionValue));
+        }
+
+        assertEquals("unexpected Client ID option state", clientIdOptionPresent, message.contains("Client ID :"));
+
         if(clientIdOptionPresent && clientIdValue != null)
         {
-            assertTrue("Client ID value is not present: " + clientIdValue, fromMessage(log).contains(clientIdValue));
+            assertTrue("Client ID value is not present: " + clientIdValue, message.contains(clientIdValue));
         }
-        
-        assertEquals("unexpected Protocol Version option state", 
-                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 Version option state", clientVersionPresent, fromMessage(log).contains("Client Version :"));
+
+        assertEquals("unexpected Client Version option state", clientVersionPresent, message.contains(
+                "Client Version :"));
 
         if(clientVersionPresent && clientVersionValue != null)
         {
-            assertTrue("Client version value is not present: " + clientVersionValue, fromMessage(log).contains(clientVersionValue));
+            assertTrue("Client version value is not present: " + clientVersionValue, message.contains(
+                    clientVersionValue));
         }
 
-        assertEquals("unexpected Client Product option state", clientVersionPresent, fromMessage(log).contains("Client Product :"));
+        assertEquals("unexpected Client Product option state", clientVersionPresent, message.contains(
+                "Client Product :"));
 
         if(clientProductPresent && clientProductValue != null)
         {
-            assertTrue("Client product value is not present: " + clientProductValue, fromMessage(log).contains(clientProductValue));
+            assertTrue("Client product value is not present: " + clientProductValue, message.contains(
+                    clientProductValue));
         }
     }
 
@@ -170,8 +182,7 @@ public class ConnectionLoggingTest exten
 
         // Validation
 
-        // We should have at least four messages
-        assertTrue("CON messages not logged:" + results.size(), results.size() >= 4);
+        assertEquals("CON messages not logged:" + results.size(), 3, results.size());
 
         // Validate Close message occurs
         String log = getLogMessageFromEnd(results, 0);



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org