Better checking of state before doing certain operations.

src/net/java/sip/communicator/impl/protocol/irc/IrcStack.java

@@ -29,6 +29,15 @@
 /**
  * An implementation of IRC using the irc-api library.
  *
+ * TODO Do we need to cancel any join channel operations still in progress?
+ *
+ * TODO Is there a chance that any Listeners will remain active after
+ * disconnecting?
+ *
+ * TODO Create a separate session object that stores: IRCApi instance +
+ * connection state + channel list container, such that it is easy to check for
+ * connectivity and manage state.
+ *
  * @author Danny van Heumen
  */
 public class IrcStack
@@ -125,9 +134,8 @@ public IrcStack(final ProtocolProviderServiceIrcImpl parentProvider,
      */
     public boolean isConnected()
     {
-        final IRCApi irc = this.session.get();
+        return (this.connectionState != null && this.connectionState
-        return (irc != null && this.connectionState != null
+            .isConnected());
-            && this.connectionState.isConnected());
     }
 
     /**
@@ -155,8 +163,7 @@ public void connect(final String host, final int port,
         final String password, final boolean secureConnection,
         final boolean autoNickChange) throws Exception
     {
-        if (this.session.get() != null && this.connectionState != null
+        if (isConnected())
-            && this.connectionState.isConnected())
         {
             return;
         }
@@ -180,8 +187,8 @@ public void connect(final String host, final int port,
         {
             final IRCApi irc = new IRCApiImpl(true);
             this.params.setServer(server);
-            irc.addListener(new ServerListener());
             this.session.set(irc);
+            irc.addListener(new ServerListener());
 
             if (LOGGER.isTraceEnabled())
             {
@@ -341,6 +348,11 @@ public void disconnect()
             return;
         }
 
+        // TODO consider skipping the leave-channel operations, since it is not
+        // necessary for IRC. This means that channel members actually get the
+        // quit message, instead of just a part message. (We do need to clean up
+        // afterwards, though, such as clearing the joined channel list and
+        // closing any open listeners.)
         synchronized (this.joined)
         {
             // Leave all joined channels.
@@ -415,6 +427,22 @@ public String getNick()
         }
     }
 
+    /**
+     * Get the currently active nick name.
+     *
+     * @return returns the nick name currently active for this connection, or
+     *         null if no connection is currently active (i.e. we are not
+     *         connected to an IRC server).
+     */
+    public String getActiveNick()
+    {
+        if (this.connectionState == null || !this.connectionState.isConnected())
+        {
+            return null;
+        }
+        return this.connectionState.getNickname();
+    }
+
     /**
      * Set the user's new nick name.
      *
@@ -476,13 +504,13 @@ public boolean isJoined(final ChatRoomIrcImpl chatroom)
     public List<String> getServerChatRoomList()
     {
         LOGGER.trace("Start retrieve server chat room list.");
-        final IRCApi irc = this.session.get();
+        if (!isConnected())
-
-        if (irc == null)
         {
-            throw new IllegalStateException("irc instance is not available");
+            throw new IllegalStateException("Not connected to an IRC server.");
         }
 
+        final IRCApi irc = this.session.get();
+
         // TODO Currently, not using an API library method for listing
         // channels, since it isn't available.
         synchronized (this.channellist)
@@ -505,7 +533,7 @@ public List<String> getServerChatRoomList()
                         irc.rawMessage("LIST");
                         while (!listSignal.isDone())
                         {
-                            LOGGER.trace("Start waiting for list ...");
+                            LOGGER.trace("Waiting for list ...");
                             listSignal.wait();
                         }
                         LOGGER.trace("Done waiting for list.");
@@ -770,7 +798,7 @@ private void leave(final String chatRoomName)
     {
         if (!isConnected())
         {
-            return;
+            throw new IllegalStateException("Not connected to an IRC server.");
         }
 
         final IRCApi irc = this.session.get();
@@ -831,7 +859,7 @@ public void invite(final String memberId, final ChatRoomIrcImpl chatroom)
     {
         if (!isConnected())
         {
-            return;
+            throw new IllegalStateException("Not connected to an IRC server.");
         }
 
         final IRCApi irc = this.session.get();
@@ -871,6 +899,10 @@ public void command(final Contact contact, final MessageIrcImpl message)
      */
     private void command(final String source, final String message)
     {
+        if (!isConnected())
+        {
+            throw new IllegalStateException("Not connected to IRC server.");
+        }
         final IRCApi irc = this.session.get();
         final String msg = message.toLowerCase();
         if (msg.startsWith("/msg "))
@@ -926,6 +958,10 @@ else if (msg.startsWith("/join "))
      */
     public void message(final ChatRoomIrcImpl chatroom, final String message)
     {
+        if (!isConnected())
+        {
+            throw new IllegalStateException("Not connected to an IRC server.");
+        }
         final IRCApi irc = this.session.get();
         String target = chatroom.getIdentifier();
         irc.message(target, message);
@@ -939,6 +975,10 @@ public void message(final ChatRoomIrcImpl chatroom, final String message)
      */
     public void message(final Contact contact, final Message message)
     {
+        if (!isConnected())
+        {
+            throw new IllegalStateException("Not connected to an IRC server.");
+        }
         final String target = contact.getAddress();
         try
         {
@@ -967,6 +1007,10 @@ public void message(final Contact contact, final Message message)
     public void grant(final ChatRoomIrcImpl chatRoom, final String userAddress,
         final Mode mode)
     {
+        if (!isConnected())
+        {
+            throw new IllegalStateException("Not connected to an IRC server.");
+        }
         if (mode.getRole() == null)
         {
             throw new IllegalArgumentException(
@@ -987,6 +1031,10 @@ public void grant(final ChatRoomIrcImpl chatRoom, final String userAddress,
     public void revoke(final ChatRoomIrcImpl chatRoom,
         final String userAddress, final Mode mode)
     {
+        if (!isConnected())
+        {
+            throw new IllegalStateException("Not connected to an IRC server.");
+        }
         if (mode.getRole() == null)
         {
             throw new IllegalArgumentException(
@@ -1296,9 +1344,14 @@ public void onUserAction(final UserActionMsg msg)
         @Override
         public void onUserQuit(final QuitMessage msg)
         {
-            // FIXME Disabled user presence status updates, probably not going
+            // FIXME Re-enabled/Disabled user presence status updates, probably
-            // to do user presence, since we cannot detect all changes and Jitsi
+            // not going to do user presence, since we cannot detect all changes
-            // does act upon different presence statuses.
+            // and Jitsi does act upon different presence statuses.
+
+            // FIXME Off-line contact still gets sent a message. Is this desired
+            // behavior?
+
+            // FIXME Disable listener on receiving local user quit message.
 
             // final String userNick = msg.getSource().getNick();
             // final Contact user =
@@ -1563,6 +1616,7 @@ public void onChannelKick(final ChannelKick msg)
         @Override
         public void onUserQuit(final QuitMessage msg)
         {
+            // FIXME handle local user quitting messages correctly.
             String user = msg.getSource().getNick();
             ChatRoomMember member = this.chatroom.getChatRoomMember(user);
             if (member != null)
@@ -1684,6 +1738,10 @@ public void onChannelNotice(final ChannelNotice msg)
          */
         private void leaveChatRoom()
         {
+            if (!IrcStack.this.isConnected())
+            {
+                return;
+            }
             final IRCApi irc = IrcStack.this.session.get();
             irc.deleteListener(this);
             IrcStack.this.joined.remove(this.chatroom.getIdentifier());
@@ -1896,6 +1954,8 @@ private boolean isThisChatRoom(final String chatRoomName)
          */
         private boolean isMe(final IRCUser user)
         {
+            // FIXME in case of disconnected, connectionState == null, so
+            // results in NPE.
             return IrcStack.this.connectionState.getNickname().equals(
                 user.getNick());
         }
@@ -1908,6 +1968,8 @@ private boolean isMe(final IRCUser user)
          */
         private boolean isMe(final String name)
         {
+            // FIXME in case of disconnected, connectionState == null, so
+            // results in NPE.
             return IrcStack.this.connectionState.getNickname().equals(name);
         }
     }