Mina client/server session id's

[ataylor]

I have a slight problem with the way we use the mina session id's.

currently when a connection is created in MessagingServerImpl we use the client session id to register it with the ConnectionManager. i.e.

public CreateConnectionResponse createConnection(final String username, final String password,
 final long remotingClientSessionID, final String clientAddress,
 final int incrementVersion,
 final PacketReturner sender)
 throws Exception
 {
 log.trace("creating a new connection for user " + username);

 if(version.getIncrementingVersion() < incrementVersion)
 {
 throw new MessagingException(MessagingException.INCOMPATIBLE_CLIENT_SERVER_VERSIONS,
 "client not compatible with version: " + version.getFullVersion());
 }
 // Authenticate. Successful autentication will place a new SubjectContext on thread local,
 // which will be used in the authorization process. However, we need to make sure we clean
 // up thread local immediately after we used the information, otherwise some other people
 // security my be screwed up, on account of thread local security stack being corrupted.

 securityStore.authenticate(username, password);

 long id = remotingService.getDispatcher().generateID();

 final ServerConnection connection =
 new ServerConnectionImpl(id, username, password,
 remotingClientSessionID, clientAddress,
 remotingService.getDispatcher(), resourceManager, storageManager,
 queueSettingsRepository,
 postOffice, securityStore, connectionManager);

 remotingService.getDispatcher().register(new ServerConnectionPacketHandler(connection));

 return new CreateConnectionResponse(connection.getID(), version);
 }

The problem is when we try to clean up connections using the MinaSessionListener the session that is passed to the public void sessionDestroyed(IoSession session) method is the session on the server side.
This can easily be fixed by using the serversession id like this:

final ServerConnection connection =
 new ServerConnectionImpl(id, username, password,
 sender.getSessionID(), clientAddress,
 remotingService.getDispatcher(), resourceManager, storageManager,
 queueSettingsRepository,
 postOffice, securityStore, connectionManager);

however this still means that on the client side any connection exceptions thrown will still be using the client session id. At the minute this isn't a problem as it only used by the client side exception listeners but it may further on.

[timfox]

I'm not following this.

I understood everything up to the last sentence:

"however this still means that on the client side any connection exceptions thrown will still be using the client session id. At the minute this isn't a problem as it only used by the client side exception listeners but it may further on. "

Can you elaborate?

[ataylor]

private class JMSFailureListener implements RemotingSessionListener
{
public void sessionDestroyed(long sessionID, MessagingException me)
{
if (me == null)
return;

JMSException je = new JMSException(me.toString());

je.initCause(me);

exceptionListener.onException(je);
}

}

see JBossConnection:

private class JMSFailureListener implements RemotingSessionListener
 {
 public void sessionDestroyed(long sessionID, MessagingException me)
 {
 if (me == null)
 return;

 JMSException je = new JMSException(me.toString());

 je.initCause(me);

 exceptionListener.onException(je);
 }

 }

The session id is not even used here, but if it was logged it may get confusing if someone was trying to match up client and server logs.

[timfox]

I don't think the session id should be exposed in the remoting session listener anyway- this is an implementation detail that is of no use to the user.

Also there is only one session per connection anyway :)

We should also rename the method from sessionDestroyed() to onFailure() ot something similar, which makes more sense to me.

[ataylor]

I don't think the session id should be exposed in the remoting session listener anyway- this is an implementation detail that is of no use to the user.

Ive left this in for now since the connection manager is reliant on it.