Author: ritchiem Date: Mon Aug 3 13:24:29 2009 New Revision: 800363 URL: http://svn.apache.org/viewvc?rev=800363&view=rev Log: QPID-2002 : Change CON-1001 to make client ID optional so that the various stages of Open can be correctly logged. Client ID is not initially provided so we would be unable to log the protocol negotiation without removing this Modified ProtocolVersion Template to include a toString value to make it easier to log.
There are two templates to update one in gentools/templ.java and one in common/templates Exposed verification methods to allow systests to reuse the code Modified: qpid/trunk/qpid/gentools/templ.java/model/ProtocolVersionListClass.vm qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/messages/LogMessages_en_US.properties qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQMinaProtocolSession.java qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/subjects/AbstractTestLogSubject.java qpid/trunk/qpid/java/common/templates/model/ProtocolVersionListClass.vm Modified: qpid/trunk/qpid/gentools/templ.java/model/ProtocolVersionListClass.vm URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/gentools/templ.java/model/ProtocolVersionListClass.vm?rev=800363&r1=800362&r2=800363&view=diff ============================================================================== --- qpid/trunk/qpid/gentools/templ.java/model/ProtocolVersionListClass.vm (original) +++ qpid/trunk/qpid/gentools/templ.java/model/ProtocolVersionListClass.vm Mon Aug 3 13:24:29 2009 @@ -39,12 +39,14 @@ { private final byte _majorVersion; private final byte _minorVersion; + private final String _stringFormat; public ProtocolVersion(byte majorVersion, byte minorVersion) { _majorVersion = majorVersion; _minorVersion = minorVersion; + _stringFormat = _majorVersion+"-"+_minorVersion; } public byte getMajorVersion() @@ -57,6 +59,10 @@ return _minorVersion; } + public String toString() + { + return _stringFormat; + } public int compareTo(Object o) { Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/messages/LogMessages_en_US.properties URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/messages/LogMessages_en_US.properties?rev=800363&r1=800362&r2=800363&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/messages/LogMessages_en_US.properties (original) +++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/messages/LogMessages_en_US.properties Mon Aug 3 13:24:29 2009 @@ -235,7 +235,7 @@ #Connection # 0 - Client id # 1 - Protocol Version -CON-1001 = Open : Client ID {0}[ : Protocol Version : {1}] +CON-1001 = Open[ : Client ID : {0}][ : Protocol Version : {1}] CON-1002 = Close #Channel Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQMinaProtocolSession.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQMinaProtocolSession.java?rev=800363&r1=800362&r2=800363&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQMinaProtocolSession.java (original) +++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQMinaProtocolSession.java Mon Aug 3 13:24:29 2009 @@ -54,6 +54,9 @@ import org.apache.qpid.server.handler.ServerMethodDispatcherImpl; import org.apache.qpid.server.logging.actors.AMQPConnectionActor; import org.apache.qpid.server.logging.actors.CurrentActor; +import org.apache.qpid.server.logging.subjects.ConnectionLogSubject; +import org.apache.qpid.server.logging.LogSubject; +import org.apache.qpid.server.logging.messages.ConnectionMessages; import org.apache.qpid.server.management.Managable; import org.apache.qpid.server.management.ManagedObject; import org.apache.qpid.server.output.ProtocolOutputConverter; @@ -139,6 +142,7 @@ private final long _sessionID = idGenerator.getAndIncrement(); private AMQPConnectionActor _actor; + private LogSubject _logSubject; public ManagedObject getManagedObject() { @@ -156,6 +160,8 @@ _actor = new AMQPConnectionActor(this, virtualHostRegistry.getApplicationRegistry().getRootMessageLogger()); + _actor.message(ConnectionMessages.CON_1001(null, null, false, false)); + try { IoServiceConfig config = session.getServiceConfig(); @@ -171,6 +177,8 @@ } } + // This is only used by two tests that do provide null values for stateManager + // so we can safely remove this and refactor. public AMQMinaProtocolSession(IoSession session, VirtualHostRegistry virtualHostRegistry, AMQCodecFactory codecFactory, AMQStateManager stateManager) throws AMQException { @@ -236,42 +244,45 @@ int channelId = frame.getChannel(); AMQBody body = frame.getBodyFrame(); - if (_logger.isDebugEnabled()) - { - _logger.debug("Frame Received: " + frame); - } - - // Check that this channel is not closing - if (channelAwaitingClosure(channelId)) + CurrentActor.set(_actor); + try { - if ((frame.getBodyFrame() instanceof ChannelCloseOkBody)) + if (_logger.isDebugEnabled()) { - if (_logger.isInfoEnabled()) - { - _logger.info("Channel[" + channelId + "] awaiting closure - processing close-ok"); - } + _logger.debug("Frame Received: " + frame); } - else + + // Check that this channel is not closing + if (channelAwaitingClosure(channelId)) { - if (_logger.isInfoEnabled()) + if ((frame.getBodyFrame() instanceof ChannelCloseOkBody)) { - _logger.info("Channel[" + channelId + "] awaiting closure. Should close socket as client did not close-ok :" + frame); + if (_logger.isInfoEnabled()) + { + _logger.info("Channel[" + channelId + "] awaiting closure - processing close-ok"); + } } + else + { + if (_logger.isInfoEnabled()) + { + _logger.info("Channel[" + channelId + "] awaiting closure. Should close socket as client did not close-ok :" + frame); + } - closeProtocolSession(); - return; + closeProtocolSession(); + return; + } } - } - CurrentActor.set(_actor); - try - { - body.handle(channelId, this); - } - catch (AMQException e) - { - closeChannel(channelId); - throw e; + try + { + body.handle(channelId, this); + } + catch (AMQException e) + { + closeChannel(channelId); + throw e; + } } finally { @@ -285,6 +296,9 @@ ((AMQDecoder) _codecFactory.getDecoder()).setExpectProtocolInitiation(false); try { + // Log incomming protocol negotiation request + _actor.message(ConnectionMessages.CON_1001(null, pi._protocolMajor + "-" + pi._protocolMinor, false, true)); + ProtocolVersion pv = pi.checkVersion(); // Fails if not correct // This sets the protocol version (and hence framing classes) for this session. @@ -643,6 +657,8 @@ if (!_closed) { _closed = true; + + _actor.message(ConnectionMessages.CON_1002()); if (_virtualHost != null) { @@ -770,7 +786,11 @@ { if (_clientProperties.getString(CLIENT_PROPERTIES_INSTANCE) != null) { - setContextKey(new AMQShortString(_clientProperties.getString(CLIENT_PROPERTIES_INSTANCE))); + String clientID = _clientProperties.getString(CLIENT_PROPERTIES_INSTANCE); + setContextKey(new AMQShortString(clientID)); + + // Log the Opening of the connection for this client + _actor.message(ConnectionMessages.CON_1001(clientID, _protocolVersion.toString(), true, true)); } if (_clientProperties.getString(ClientProperties.version.toString()) != null) @@ -829,6 +849,7 @@ _virtualHost = virtualHost; _actor.virtualHostSelected(this); + _logSubject = new ConnectionLogSubject(this); _virtualHost.getConnectionRegistry().registerConnection(this); Modified: qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java?rev=800363&r1=800362&r2=800363&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java (original) +++ qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/messages/ConnectionMessagesTest.java Mon Aug 3 13:24:29 2009 @@ -24,12 +24,12 @@ public class ConnectionMessagesTest extends AbstractTestMessages { - public void testMessage1001_WithProtocolVersion() + public void testMessage1001_WithClientIDProtocolVersion() { String clientID = "client"; String protocolVersion = "8-0"; - _logMessage = ConnectionMessages.CON_1001(clientID, protocolVersion, true); + _logMessage = ConnectionMessages.CON_1001(clientID, protocolVersion, true , true); List<Object> log = performLog(); String[] expected = {"Open :", "Client ID", clientID, @@ -38,11 +38,11 @@ validateLogMessage(log, "CON-1001", expected); } - public void testMessage1001_NoProtocolVersion() + public void testMessage1001_WithClientIDNoProtocolVersion() { String clientID = "client"; - _logMessage = ConnectionMessages.CON_1001(clientID, null, false); + _logMessage = ConnectionMessages.CON_1001(clientID, null,true, false); List<Object> log = performLog(); String[] expected = {"Open :", "Client ID", clientID}; @@ -50,6 +50,29 @@ validateLogMessage(log, "CON-1001", expected); } + public void testMessage1001_WithNOClientIDProtocolVersion() + { + String protocolVersion = "8-0"; + + _logMessage = ConnectionMessages.CON_1001(null, protocolVersion, false , true); + List<Object> log = performLog(); + + String[] expected = {"Open", ": Protocol Version :", protocolVersion}; + + validateLogMessage(log, "CON-1001", expected); + } + + public void testMessage1001_WithNoClientIDNoProtocolVersion() + { + _logMessage = ConnectionMessages.CON_1001(null, null,false, false); + List<Object> log = performLog(); + + String[] expected = {"Open"}; + + validateLogMessage(log, "CON-1001", expected); + } + + public void testMessage1002() { Modified: qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/subjects/AbstractTestLogSubject.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/subjects/AbstractTestLogSubject.java?rev=800363&r1=800362&r2=800363&view=diff ============================================================================== --- qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/subjects/AbstractTestLogSubject.java (original) +++ qpid/trunk/qpid/java/broker/src/test/java/org/apache/qpid/server/logging/subjects/AbstractTestLogSubject.java Mon Aug 3 13:24:29 2009 @@ -172,7 +172,7 @@ * @param message the message to search * @param vhost the vhostName to check against */ - protected void verifyVirtualHost(String message, VirtualHost vhost) + static public void verifyVirtualHost(String message, VirtualHost vhost) { String vhostSlice = getSlice("vh", message); @@ -199,7 +199,7 @@ * * @return the slice if found otherwise null is returned */ - protected String getSlice(String sliceID, String message) + static public String getSlice(String sliceID, String message) { int indexOfSlice = message.indexOf(sliceID + "("); Modified: qpid/trunk/qpid/java/common/templates/model/ProtocolVersionListClass.vm URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/common/templates/model/ProtocolVersionListClass.vm?rev=800363&r1=800362&r2=800363&view=diff ============================================================================== --- qpid/trunk/qpid/java/common/templates/model/ProtocolVersionListClass.vm (original) +++ qpid/trunk/qpid/java/common/templates/model/ProtocolVersionListClass.vm Mon Aug 3 13:24:29 2009 @@ -41,12 +41,14 @@ { private final byte _majorVersion; private final byte _minorVersion; + private final String _stringFormat; public ProtocolVersion(byte majorVersion, byte minorVersion) { _majorVersion = majorVersion; _minorVersion = minorVersion; + _stringFormat = _majorVersion+"-"+_minorVersion; } public byte getMajorVersion() @@ -59,6 +61,11 @@ return _minorVersion; } + public String toString() + { + return _stringFormat; + } + public int compareTo(Object o) { --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:commits-subscr...@qpid.apache.org