Improved the Command - CommandFactory implementation.

* Moved from 'init' method to constructor.
* Expected format for the constructor described in the Command
interface comments.
* Distinguish between exceptions that occur during construction of the
command, log these and inform the user of an error; and exception
occurring during the execution of the command.
* Better error handling for commands.

To do:
* Add 'help' method to the Command interface.
* Catch IllegalArgumentException and automatically display help()
information to the user.

11 files changed, 172 insertions, 53 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 9761591..ac63644 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImpl.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImpl.java
@@ -1012,6 +1012,18 @@ public class ChatRoomIrcImpl
                             .UNSUPPORTED_OPERATION,
                         e.getMessage(), new Date(), message);
                 }
+                catch (BadCommandException e)
+                {
+                    LOGGER.error("An error occurred while constructing "
+                        + "the command. This is most likely due to a bug "
+                        + "in the implementation of the command. Message: "
+                        + message + "'", e);
+                    this.fireMessageDeliveryFailedEvent(
+                        ChatRoomMessageDeliveryFailedEvent.INTERNAL_ERROR,
+                        "Command cannot be executed. This is most likely due "
+                            + "to a bug in the implementation.", new Date(),
+                        message);
+                }
             }
             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 f9fa97b..dd0534a 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/Command.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/Command.java
@@ -6,36 +6,40 @@
  */
 package net.java.sip.communicator.impl.protocol.irc;
 
+import net.java.sip.communicator.impl.protocol.irc.exception.*;
+
 /**
  * Interface for the implementation of individual IRC commands.
  *
  * This interface defines the format for the implementation of commands that can
  * be called for via the messaging input field.
  *
- * A command is instantiated for each encounter. {@link #init} will be upon
- * instantiation to set up the state before actually performing the command.
- * Then {@link #execute} is called in order to execute the command.
+ * A new command object is instantiated for each encounter. Then
+ * {@link #execute} is called to execute the command.
+ *
+ * Instantiation will be done by the CommandFactory. The CommandFactory expects
+ * to find a constructor with the following types as arguments (in order):
+ * <ol>
+ * <li>{@link ProtocolProviderServiceIrcImpl}</li>
+ * <li>{@link IrcConnection}</li>
+ * </ol>
+ *
+ * If no suitable constructor is found, an {@link BadCommandException} will be
+ * thrown.
+ *
+ * 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.
+ *
+ * FIXME Additionally, describe the AbstractIrcMessageListener type once this
+ * implementation is merged with Jitsi main line.
  *
  * @author Danny van Heumen
  */
 public interface Command
 {
     /**
-     * Initialize this instance of a command. Initialization may include
-     * registering listeners for the current connection such that a command can
-     * act upon the server's response.
-     *
-     * {@link #init} is guaranteed to be called before the command gets
-     * executed and thus can be used to initialize any dependencies or listeners
-     * that are required for the command to be executed.
-     *
-     * @param provider the protocol provider service
-     * @param connection the IRC connection instance
-     */
-    void init(ProtocolProviderServiceIrcImpl provider,
-            IrcConnection connection);
-
-    /**
      * Execute the command using the full line.
      *
      * @param source the source channel/user from which the message is sent.
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/CommandFactory.java b/src/net/java/sip/communicator/impl/protocol/irc/CommandFactory.java
index bf83690..5266156 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/CommandFactory.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/CommandFactory.java
@@ -6,6 +6,7 @@
  */
 package net.java.sip.communicator.impl.protocol.irc;
 
+import java.lang.reflect.*;
 import java.util.*;
 import java.util.Map.Entry;
 
@@ -15,6 +16,9 @@ import net.java.sip.communicator.util.*;
 /**
  * Command factory.
  *
+ * TODO Consider implementing 1 "management" command for querying the registered
+ * commands at runtime and maybe some other things. Not very urgent, though.
+ *
  * @author Danny van Heumen
  */
 public class CommandFactory
@@ -134,9 +138,12 @@ public class CommandFactory
      * @param command the command to look up and instantiate
      * @return returns a command instance
      * @throws UnsupportedCommandException in case command cannot be found
+     * @throws BadCommandException In case of a incompatible command or bad
+     *             implementation.
      */
     public Command createCommand(final String command)
-            throws UnsupportedCommandException
+        throws UnsupportedCommandException,
+        BadCommandException
     {
         if (command == null || command.isEmpty())
         {
@@ -148,29 +155,40 @@ public class CommandFactory
         {
             throw new UnsupportedCommandException(command);
         }
-        final Command cmd;
         try
         {
-            // FIXME change to construct with provider and connection instance,
-            // remove init() method from interface
-            cmd = type.newInstance();
-            cmd.init(this.provider, this.connection);
-            return cmd;
+            Constructor<? extends Command> cmdCtor =
+                type.getConstructor(ProtocolProviderServiceIrcImpl.class,
+                    IrcConnection.class);
+            return cmdCtor.newInstance(this.provider, this.connection);
+        }
+        catch (NoSuchMethodException e)
+        {
+            throw new BadCommandException(command, type,
+                "There is no compatible constructor for instantiating this "
+                    + "command.", e);
+        }
+        catch (InstantiationException e)
+        {
+            throw new BadCommandException(command, type,
+                "Unable to instantiate this command class.", e);
+        }
+        catch (IllegalAccessException e)
+        {
+            throw new BadCommandException(command, type,
+                "Unable to access the constructor of this command class.", e);
         }
-        catch (InstantiationException ex)
+        catch (InvocationTargetException e)
         {
-            throw new IllegalStateException(
-                    "A bad command implementation has been registered. It fails"
-                    + " to instantiate.",
-                    ex);
+            throw new BadCommandException(command, type,
+                "An exception occurred while executing the constructor.", e);
         }
-        catch (IllegalAccessException ex)
+        catch (IllegalArgumentException e)
         {
-            throw new IllegalStateException(
-                    "A bad command implementation has been registered. It does "
-                    + "not allow access to its constructor and therefore cannot"
-                    + " be instantiated.",
-                    ex);
+            throw new BadCommandException(command, type,
+                "Invalid arguments were passed to the "
+                    + "constructor. This may be a bug in the CommandFactory "
+                    + "implementation.", e);
         }
     }
 }
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 a2fec4e..ba32402 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/MessageManager.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/MessageManager.java
@@ -24,6 +24,8 @@ import com.ircclouds.irc.api.state.*;
  * TODO Implement messaging service for offline messages and such. (MemoServ -
  * message relaying services)
  *
+ * TODO move IRC commands to separate package
+ *
  * @author Danny van Heumen
  */
 public class MessageManager
@@ -141,9 +143,12 @@ public class MessageManager
      * @param chatroom the chat room
      * @param message the command message
      * @throws UnsupportedCommandException for unknown or unsupported commands
+     * @throws BadCommandException in case of incompatible command or bad
+     *             implementation
      */
     public void command(final ChatRoomIrcImpl chatroom, final String message)
-            throws UnsupportedCommandException
+        throws UnsupportedCommandException,
+        BadCommandException
     {
         if (!this.connectionState.isConnected())
         {
@@ -158,9 +163,11 @@ public class MessageManager
      * @param contact the chat room
      * @param message the command message
      * @throws UnsupportedCommandException for unknown or unsupported commands
+     * @throws BadCommandException in case of a bad command implementation
      */
     public void command(final Contact contact, final MessageIrcImpl message)
-            throws UnsupportedCommandException
+        throws UnsupportedCommandException,
+        BadCommandException
     {
         if (!this.connectionState.isConnected())
         {
@@ -174,9 +181,14 @@ public class MessageManager
      *
      * @param source Source contact or chat room from which the message is sent.
      * @param message Command message
+     * @throws UnsupportedCommandException in case a suitable command could not
+     *             be found
+     * @throws BadCommandException in case of an incompatible command or a bad
+     *             implementation
      */
     private void command(final String source, final String message)
-            throws UnsupportedCommandException
+        throws UnsupportedCommandException,
+        BadCommandException
     {
         final String msg = message.toLowerCase();
         final int end = msg.indexOf(' ');
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 efbc63d..6def900 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/OperationSetBasicInstantMessagingIrcImpl.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/OperationSetBasicInstantMessagingIrcImpl.java
@@ -134,6 +134,14 @@ public class OperationSetBasicInstantMessagingIrcImpl
                                 MessageDeliveryFailedEvent
                                     .UNSUPPORTED_OPERATION);
                         }
+                        catch (BadCommandException e)
+                        {
+                            LOGGER.error("Error during command execution. "
+                                + "This is most likely due to a bug in the "
+                                + "implementation of the command.", e);
+                            fireMessageDeliveryFailed(message, to,
+                                MessageDeliveryFailedEvent.INTERNAL_ERROR);
+                        }
                     }
                     else
                     {
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/command/Join.java b/src/net/java/sip/communicator/impl/protocol/irc/command/Join.java
index cf2aa77..5d92d16 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/command/Join.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/command/Join.java
@@ -24,7 +24,7 @@ public class Join implements Command
     /**
      * Instance of the IRC connection.
      */
-    private IrcConnection connection;
+    private final IrcConnection connection;
 
     /**
      * Initialization of the /join command. Join a channel.
@@ -32,9 +32,8 @@ public class Join implements Command
      * @param provider the provider instance
      * @param connection the IRC connection instance
      */
-    @Override
-    public void init(final ProtocolProviderServiceIrcImpl provider,
-            final IrcConnection connection)
+    public Join(final ProtocolProviderServiceIrcImpl provider,
+        final IrcConnection connection)
     {
         if (connection == null)
         {
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/command/Me.java b/src/net/java/sip/communicator/impl/protocol/irc/command/Me.java
index 2c2e9bf..52997e1 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/command/Me.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/command/Me.java
@@ -15,7 +15,7 @@ import net.java.sip.communicator.impl.protocol.irc.*;
  * @author Danny van Heumen
  */
 public class Me
-        implements Command
+    implements Command
 {
     /**
      * Me command prefix index.
@@ -25,7 +25,7 @@ public class Me
     /**
      * IRC connection instance.
      */
-    private IrcConnection connection;
+    private final IrcConnection connection;
 
     /**
      * Initialization of the /me command.
@@ -33,8 +33,7 @@ public class Me
      * @param provider the provider instance
      * @param connection the connection instance
      */
-    @Override
-    public void init(final ProtocolProviderServiceIrcImpl provider,
+    public Me(final ProtocolProviderServiceIrcImpl provider,
             final IrcConnection connection)
     {
         if (connection == null)
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/command/Mode.java b/src/net/java/sip/communicator/impl/protocol/irc/command/Mode.java
index 57ab19f..1d70e6e 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/command/Mode.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/command/Mode.java
@@ -31,8 +31,7 @@ public class Mode implements Command
      * @param provider the provider instance
      * @param connection the connection instance
      */
-    @Override
-    public void init(final ProtocolProviderServiceIrcImpl provider,
+    public Mode(final ProtocolProviderServiceIrcImpl provider,
             final IrcConnection connection)
     {
         if (connection == null)
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/command/Msg.java b/src/net/java/sip/communicator/impl/protocol/irc/command/Msg.java
index 03dda0b..2bfec0e 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/command/Msg.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/command/Msg.java
@@ -31,8 +31,7 @@ public class Msg implements Command
      * @param provider the provider instance
      * @param connection the connection instance
      */
-    @Override
-    public void init(final ProtocolProviderServiceIrcImpl provider,
+    public Msg(final ProtocolProviderServiceIrcImpl provider,
             final IrcConnection connection)
     {
         if (connection == null)
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/command/Nick.java b/src/net/java/sip/communicator/impl/protocol/irc/command/Nick.java
index 7d3d257..3cc99c5 100644
--- a/src/net/java/sip/communicator/impl/protocol/irc/command/Nick.java
+++ b/src/net/java/sip/communicator/impl/protocol/irc/command/Nick.java
@@ -26,8 +26,7 @@ public class Nick implements Command
      * @param provider the provider instance
      * @param connection the connection instance
      */
-    @Override
-    public void init(final ProtocolProviderServiceIrcImpl provider,
+    public Nick(final ProtocolProviderServiceIrcImpl provider,
             final IrcConnection connection)
     {
         if (connection == null)
diff --git a/src/net/java/sip/communicator/impl/protocol/irc/exception/BadCommandException.java b/src/net/java/sip/communicator/impl/protocol/irc/exception/BadCommandException.java
new file mode 100644
index 0000000..89668f4
--- /dev/null
+++ b/src/net/java/sip/communicator/impl/protocol/irc/exception/BadCommandException.java
@@ -0,0 +1,70 @@
+/*
+ * Jitsi, the OpenSource Java VoIP and Instant Messaging client.
+ *
+ * Distributable under LGPL license.
+ * See terms of license at gnu.org.
+ */
+package net.java.sip.communicator.impl.protocol.irc.exception;
+
+import net.java.sip.communicator.impl.protocol.irc.*;
+
+/**
+ * Exception indicating a bad command implementation.
+ *
+ * @author Danny van Heumen
+ */
+public class BadCommandException
+    extends Exception
+{
+    /**
+     * Serialization id.
+     */
+    private static final long serialVersionUID = 1L;
+
+    /**
+     * The command name used to look up the type.
+     */
+    private final String command;
+
+    /**
+     * The command type that caused the exception.
+     */
+    private Class<? extends Command> type;
+
+    /**
+     * Constructor.
+     *
+     * @param command the command
+     * @param type the command implementation
+     * @param message the error message
+     * @param e the cause
+     */
+    public BadCommandException(final String command,
+        final Class<? extends Command> type, final String message,
+        final Throwable e)
+    {
+        super("An error occurred while preparing command '" + command + "' ("
+            + type.getCanonicalName() + "): " + message, e);
+        this.command = command;
+    }
+
+    /**
+     * Get the command.
+     *
+     * @return returns the name of the failed command
+     */
+    public String getCommand()
+    {
+        return this.command;
+    }
+
+    /**
+     * Get the type of the implementation.
+     *
+     * @return returns the type that implements the command
+     */
+    public Class<? extends Command> getType()
+    {
+        return this.type;
+    }
+}