Improved error handling for commands.

In case of bad command usage, throw an IllegalArgumentException.
IllegalArgumentException will be caught and handled specially. Whenever
IAE is thrown, it will be caught and command.help() will be called so
that we can receive additional usage instructions. The error and the
help information will be thrown inside a BadCommandInvocationException
which will be handled by the appropriate OperationSet.

In case of such an event, a system message will be fired with the usage
instructions as to inform the user on how to use the command.  Other
exceptions will be logged as an error, as they are not expected and
considered to be an implementation issue.

11 files changed, 177 insertions, 7 deletions

diff --git a/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImpl.java b/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImpl.java
index ac63644..1d30fc8 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImpl.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImpl.java
@@ -1024,6 +1024,20 @@ public class ChatRoomIrcImpl
                             + "to a bug in the implementation.", new Date(),
                         message);
                 }
+                catch (BadCommandInvocationException e)
+                {
+                    MessageIrcImpl helpMessage =
+                        new MessageIrcImpl(
+                            e.getHelp(),
+                            OperationSetBasicInstantMessaging
+                                .DEFAULT_MIME_TYPE,
+                            OperationSetBasicInstantMessaging
+                                .DEFAULT_MIME_ENCODING,
+                            "Command usage:");
+                    this.fireMessageReceivedEvent(helpMessage, this.user,
+                        new Date(),
+                        MessageReceivedEvent.SYSTEM_MESSAGE_RECEIVED);
+                }
             }
             else
             {
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/Command.java b/src/net/java/sip/communicator/impl/protocol/irc/Command.java
index dd0534a..3f9af8e 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/Command.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/Command.java
@@ -11,26 +11,36 @@ import net.java.sip.communicator.impl.protocol.irc.exception.*;
 /**
  * Interface for the implementation of individual IRC commands.
  *
+ * <p>
  * This interface defines the format for the implementation of commands that can
  * be called for via the messaging input field.
+ * </p>
  *
+ * <p>
  * A new command object is instantiated for each encounter. Then
  * {@link #execute} is called to execute the command.
+ * </p>
  *
+ * <p>
  * Instantiation will be done by the CommandFactory. The CommandFactory expects
  * to find a constructor with the following types as arguments (in order):
+ * </p>
  * <ol>
  * <li>{@link ProtocolProviderServiceIrcImpl}</li>
  * <li>{@link IrcConnection}</li>
  * </ol>
  *
+ * <p>
  * If no suitable constructor is found, an {@link BadCommandException} will be
  * thrown.
+ * </p>
  *
+ * <p>
  * For more advanced IRC commands, one can register listeners with the
  * underlying IRC client. This way you can intercept IRC messages as they are
  * received. Using this method you can send messages, receive replies and act on
  * (other) events.
+ * </p>
  *
  * FIXME Additionally, describe the AbstractIrcMessageListener type once this
  * implementation is merged with Jitsi main line.
@@ -43,9 +53,26 @@ public interface Command
      * Execute the command using the full line.
      *
      * @param source the source channel/user from which the message is sent.
-     * (Note that for a normal message this would then be the target/receiver of
-     * the message too.)
+     *            (Note that for a normal message this would then be the
+     *            target/receiver of the message too.)
      * @param line the command message line
+     * @throws IllegalArgumentException Special meaning has been given to
+     *             IllegalArgumentException: it signals bad usage of a command.
+     *             Jitsi will consequently call {@link #help()} to query a help
+     *             message that will be passed on as a system message to the
+     *             user.
      */
     void execute(String source, String line);
+
+    /**
+     * Return help information to output.
+     *
+     * {@link #help()} is called whenever a command execution fails with
+     * {@link IllegalArgumentException}. IllegalArgumentException suggests that
+     * the command was called incorrectly. The help information will then be
+     * displayed to explain the user how to use the command.
+     *
+     * @return returns help information to display
+     */
+    String help();
 }
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/MessageManager.java b/src/net/java/sip/communicator/impl/protocol/irc/MessageManager.java
index c33b4db..f30cd0a 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/MessageManager.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/MessageManager.java
@@ -128,10 +128,15 @@ public class MessageManager
      * @throws UnsupportedCommandException for unknown or unsupported commands
      * @throws BadCommandException in case of incompatible command or bad
      *             implementation
+     * @throws BadCommandInvocationException in case of bad usage of the
+     *             command. An exception will be thrown that contains the root
+     *             cause and optionally a help text containing usage information
+     *             for that particular command.
      */
     public void command(final ChatRoomIrcImpl chatroom, final String message)
         throws UnsupportedCommandException,
-        BadCommandException
+        BadCommandException,
+        BadCommandInvocationException
     {
         if (!this.connectionState.isConnected())
         {
@@ -147,10 +152,15 @@ public class MessageManager
      * @param message the command message
      * @throws UnsupportedCommandException for unknown or unsupported commands
      * @throws BadCommandException in case of a bad command implementation
+     * @throws BadCommandInvocationException in case of bad usage of the
+     *             command. An exception will be thrown that contains the root
+     *             cause and optionally a help text containing usage information
+     *             for that particular command.
      */
     public void command(final Contact contact, final MessageIrcImpl message)
         throws UnsupportedCommandException,
-        BadCommandException
+        BadCommandException,
+        BadCommandInvocationException
     {
         if (!this.connectionState.isConnected())
         {
@@ -168,10 +178,15 @@ public class MessageManager
      *             be found
      * @throws BadCommandException in case of an incompatible command or a bad
      *             implementation
+     * @throws BadCommandInvocationException in case of bad usage of the
+     *             command. An exception will be thrown that contains the root
+     *             cause and optionally a help text containing usage
+     *             information for that particular command.
      */
     private void command(final String source, final String message)
         throws UnsupportedCommandException,
-        BadCommandException
+        BadCommandException,
+        BadCommandInvocationException
     {
         final String msg = message.toLowerCase();
         final int end = msg.indexOf(' ');
@@ -187,10 +202,13 @@ public class MessageManager
         final Command cmd = this.commandFactory.createCommand(command);
         try
         {
-            // FIXME catch IllegalArgumentException and print help information
-            // for command
             cmd.execute(source, msg);
         }
+        catch (IllegalArgumentException e)
+        {
+            final String help = cmd.help();
+            throw new BadCommandInvocationException(msg, help, e);
+        }
         catch (RuntimeException e)
         {
             LOGGER.error(
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/OperationSetBasicInstantMessagingIrcImpl.java b/src/net/java/sip/communicator/impl/protocol/irc/OperationSetBasicInstantMessagingIrcImpl.java
index 6def900..f522e21 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/OperationSetBasicInstantMessagingIrcImpl.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/OperationSetBasicInstantMessagingIrcImpl.java
@@ -6,6 +6,8 @@
  */
 package net.java.sip.communicator.impl.protocol.irc;
 
+import java.util.*;
+
 import net.java.sip.communicator.impl.protocol.irc.exception.*;
 import net.java.sip.communicator.service.protocol.*;
 import net.java.sip.communicator.service.protocol.event.*;
@@ -142,6 +144,25 @@ public class OperationSetBasicInstantMessagingIrcImpl
                             fireMessageDeliveryFailed(message, to,
                                 MessageDeliveryFailedEvent.INTERNAL_ERROR);
                         }
+                        catch (BadCommandInvocationException e)
+                        {
+                            MessageIrcImpl helpMessage =
+                                new MessageIrcImpl(
+                                    e.getHelp(),
+                                    OperationSetBasicInstantMessaging
+                                        .DEFAULT_MIME_TYPE,
+                                    OperationSetBasicInstantMessaging
+                                        .DEFAULT_MIME_ENCODING,
+                                    "Command usage:");
+                            MessageReceivedEvent helpEvent =
+                                new MessageReceivedEvent(
+                                    helpMessage,
+                                    to,
+                                    new Date(),
+                                    MessageReceivedEvent
+                                        .SYSTEM_MESSAGE_RECEIVED);
+                            fireMessageEvent(helpEvent);
+                        }
                     }
                     else
                     {
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/exception/BadCommandInvocationException.java b/src/net/java/sip/communicator/impl/protocol/irc/exception/BadCommandInvocationException.java
new file mode 100644
index 0000000..bc8a223
--- /dev/null
+++ b/src/net/java/sip/communicator/impl/protocol/irc/exception/BadCommandInvocationException.java
@@ -0,0 +1,27 @@
+package net.java.sip.communicator.impl.protocol.irc.exception;
+
+
+public class BadCommandInvocationException
+    extends Exception
+{
+    private final String line;
+    private final String help;
+
+    public BadCommandInvocationException(final String line, final String help, final Throwable cause)
+    {
+        super("The command failed because of incorrect usage: "
+            + cause.getMessage(), cause);
+        this.line = line;
+        this.help = help;
+    }
+
+    public String getLine()
+    {
+        return this.line;
+    }
+
+    public String getHelp()
+    {
+        return this.help;
+    }
+}
diff --git a/src/net/java/sip/communicator/plugin/irccommands/command/Join.java b/src/net/java/sip/communicator/plugin/irccommands/command/Join.java
index 20c9c72..d84d93b 100644
--- a/src/net/java/sip/communicator/plugin/irccommands/command/Join.java
+++ b/src/net/java/sip/communicator/plugin/irccommands/command/Join.java
@@ -76,4 +76,13 @@ public class Join implements Command
         }
         this.connection.getClient().joinChannel(channel, password);
     }
+
+    /**
+     * Command usage instructions.
+     */
+    @Override
+    public String help()
+    {
+        return "Usage: /join <channel> [password]";
+    }
 }
diff --git a/src/net/java/sip/communicator/plugin/irccommands/command/Me.java b/src/net/java/sip/communicator/plugin/irccommands/command/Me.java
index cbbb869..c9f7c47 100644
--- a/src/net/java/sip/communicator/plugin/irccommands/command/Me.java
+++ b/src/net/java/sip/communicator/plugin/irccommands/command/Me.java
@@ -64,4 +64,13 @@ public class Me
         }
         this.connection.getClient().act(source, message);
     }
+
+    /**
+     * Command usage instructions.
+     */
+    @Override
+    public String help()
+    {
+        return "Usage: /me <message>";
+    }
 }
diff --git a/src/net/java/sip/communicator/plugin/irccommands/command/Mode.java b/src/net/java/sip/communicator/plugin/irccommands/command/Mode.java
index f738ae6..4fbae8e 100644
--- a/src/net/java/sip/communicator/plugin/irccommands/command/Mode.java
+++ b/src/net/java/sip/communicator/plugin/irccommands/command/Mode.java
@@ -66,4 +66,13 @@ public class Mode implements Command
         }
         this.connection.getClient().changeMode(source + " " + rawModeString);
     }
+
+    /**
+     * Usage instructions.
+     */
+    @Override
+    public String help()
+    {
+        return "Usage: /mode <mode> [params ...]";
+    }
 }
diff --git a/src/net/java/sip/communicator/plugin/irccommands/command/Msg.java b/src/net/java/sip/communicator/plugin/irccommands/command/Msg.java
index 7e10659..2e9ef6f 100644
--- a/src/net/java/sip/communicator/plugin/irccommands/command/Msg.java
+++ b/src/net/java/sip/communicator/plugin/irccommands/command/Msg.java
@@ -79,4 +79,13 @@ public class Msg implements Command
         }
         this.connection.getClient().message(target, message);
     }
+
+    /**
+     * Usage instructions.
+     */
+    @Override
+    public String help()
+    {
+        return "Usage: /msg <user> <message>";
+    }
 }
diff --git a/src/net/java/sip/communicator/plugin/irccommands/command/Nick.java b/src/net/java/sip/communicator/plugin/irccommands/command/Nick.java
index 39582cd..4216835 100644
--- a/src/net/java/sip/communicator/plugin/irccommands/command/Nick.java
+++ b/src/net/java/sip/communicator/plugin/irccommands/command/Nick.java
@@ -66,4 +66,13 @@ public class Nick implements Command
             this.connection.getIdentityManager().setNick(newNick);
         }
     }
+
+    /**
+     * Usage instructions.
+     */
+    @Override
+    public String help()
+    {
+        return "Usage: /nick <new-nick>";
+    }
 }
diff --git a/test/net/java/sip/communicator/impl/protocol/irc/CommandFactoryTest.java b/test/net/java/sip/communicator/impl/protocol/irc/CommandFactoryTest.java
index a47f0d4..18a88b8 100644
--- a/test/net/java/sip/communicator/impl/protocol/irc/CommandFactoryTest.java
+++ b/test/net/java/sip/communicator/impl/protocol/irc/CommandFactoryTest.java
@@ -73,6 +73,12 @@ public class CommandFactoryTest
             @Override
             public void execute(String source, String line)
             {
+            }
+
+            @Override
+            public String help()
+            {
+                return null;
             }};
         CommandFactory.registerCommand("test", Test.class);
         CommandFactory.registerCommand("foo", anotherType.getClass());
@@ -115,6 +121,12 @@ public class CommandFactoryTest
         public void execute(String source, String line)
         {
         }
+
+        @Override
+        public String help()
+        {
+            return null;
+        }
     }
 
     public void testConstructionNullProvider()
@@ -290,6 +302,12 @@ public class CommandFactoryTest
         public void execute(String source, String line)
         {
         }
+
+        @Override
+        public String help()
+        {
+            return null;
+        }
     }
 
     public abstract static class BadImplementation implements Command