Action wrappers now extend action groups

Groups now encapsulate instead of extending their ArrayLists, to limit the number of ways it can be modified
Various resulting tweaks
Fixes issue 631

git-svn-id: http://svn.dmdirc.com/trunk@3859 00569f92-eb28-0410-84fd-f71c24880f

src/com/dmdirc/Main.java

@@ -53,7 +53,7 @@ public final class Main {
     public static final String VERSION = "SVN";
 
     /** The SVN revision that this build was made from. */
-    public static final int SVN_REVISION = 3628; // 3601;
+    public static final int SVN_REVISION = 3853; // 3853; // 3853; // 3853; // 3628; // 3601;
 
     /** Stores the update channel that this version came from, if any. */
     public static final UpdateChannel UPDATE_CHANNEL = UpdateChannel.NONE;

src/com/dmdirc/actions/ActionGroup.java

@@ -23,15 +23,19 @@
 package com.dmdirc.actions;
 
 import com.dmdirc.config.prefs.PreferencesSetting;
+
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 
 /**
  * Represents a group of actions, along with their meta-data.
  * 
  * @author chris
  */
-public class ActionGroup extends ArrayList<Action> {
+public class ActionGroup implements Iterable<Action> {
     
     /**
      * A version number for this class. It should be changed whenever the class
@@ -40,6 +44,9 @@ public class ActionGroup extends ArrayList<Action> {
      */
     private static final long serialVersionUID = 1;    
     
+    /** The actions in this group. */
+    private final List<Action> actions = new ArrayList<Action>();
+    
     /** The name of this action group. */
     private final String name;
     
@@ -159,5 +166,76 @@ public class ActionGroup extends ArrayList<Action> {
     public void setComponent(final int component) {
         this.component = component;
     }
+    
+    /**
+     * Determines the size of this group.
+     * 
+     * @return The size of this group
+     */
+    public int size() {
+        return actions.size();
+    }
+
+    /**
+     * Removes the specified action from this group.
+     * 
+     * @param action The action to be removed
+     */
+    public void remove(final Action action) {
+        actions.remove(action);
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public Iterator<Action> iterator() {
+        return actions.iterator();
+    }
+
+    /**
+     * Retrieves the action at the specified index.
+     * 
+     * @param index The index of the action to return
+     * @return The action at the specified index
+     */
+    public Action get(final int index) {
+        return actions.get(index);
+    }
 
+    /**
+     * Determines if this group contains the specified action.
+     * 
+     * @param action The action to search for
+     * @return True if the action is contained in this list, false otherwise
+     */
+    public boolean contains(final Action action) {
+        return actions.contains(action);
+    }
+
+    /**
+     * Removes all actions from this group.
+     */
+    public void clear() {
+        for (Action action : new ArrayList<Action>(actions)) {
+            remove(action);
+        }
+    }
+
+    /**
+     * Adds the specified action to this group.
+     * 
+     * @param action The action to be added
+     */
+    public void add(final Action action) {
+        actions.add(action);
+    }
+    
+    /**
+     * Retrieves a copy of the list of all actions in this group.
+     * 
+     * @return A list of actions in this group
+     */
+    public List<Action> getActions() {
+        return new ArrayList<Action>(actions);
+    }
+    
 }

src/com/dmdirc/actions/ActionManager.java

@@ -27,7 +27,6 @@ import com.dmdirc.Precondition;
 import com.dmdirc.actions.interfaces.ActionComparison;
 import com.dmdirc.actions.interfaces.ActionComponent;
 import com.dmdirc.actions.interfaces.ActionType;
-import com.dmdirc.actions.wrappers.ActionWrapper;
 import com.dmdirc.actions.wrappers.AliasWrapper;
 import com.dmdirc.actions.wrappers.PerformWrapper;
 import com.dmdirc.config.IdentityManager;
@@ -65,10 +64,6 @@ public final class ActionManager {
     private final static List<ActionComparison> actionComparisons
             = new ArrayList<ActionComparison>();
 
-    /** A list of all action wrappers that have registered with us. */
-    private final static List<ActionWrapper> actionWrappers
-            = new ArrayList<ActionWrapper>();
-
     /** A map linking types and a list of actions that're registered for them. */
     private final static MapList<ActionType, Action> actions
             = new MapList<ActionType, Action>();
@@ -102,8 +97,8 @@ public final class ActionManager {
         registerActionComparisons(CoreActionComparison.values());
         registerActionComponents(CoreActionComponent.values());
 
-        registerWrapper(AliasWrapper.getAliasWrapper());
-        registerWrapper(PerformWrapper.getPerformWrapper());
+        registerGroup(AliasWrapper.getAliasWrapper());
+        registerGroup(PerformWrapper.getPerformWrapper());
        
         // Register a listener for the closing event, so we can save actions
         addListener(new ActionListener() {
@@ -150,20 +145,12 @@ public final class ActionManager {
     }
 
     /**
-     * Registers the specified action wrapper with the manager.
+     * Registers the specified group of actions with the manager.
      *
-     * @param wrapper The wrapper to be registered
+     * @param group The group of actions to be registered
      */
-    @Precondition({
-        "The specified wrapper is not null",
-        "The specified wrapper's group name is non-null and not empty"
-    })
-    public static void registerWrapper(final ActionWrapper wrapper) {
-        Logger.assertTrue(wrapper != null);
-        Logger.assertTrue(wrapper.getGroupName() != null);
-        Logger.assertTrue(!wrapper.getGroupName().isEmpty());
-
-        actionWrappers.add(wrapper);
+    public static void registerGroup(final ActionGroup group) {
+        groups.put(group.getName(), group);
     }
 
     /**
@@ -232,10 +219,9 @@ public final class ActionManager {
      */
     public static void loadActions() {
         actions.clear();
-        groups.clear();
 
-        for (ActionWrapper wrapper : actionWrappers) {
-            wrapper.clearActions();
+        for (ActionGroup group : groups.values()) {
+            group.clear();
         }
 
         final File dir = new File(getDirectory());
@@ -272,41 +258,6 @@ public final class ActionManager {
         }
     }
 
-    /**
-     * Retrieves the action wrapper with the specified group name.
-     *
-     * @param name The group name to find
-     * @return An ActionWrapper with the specified group name, or null
-     */
-    @Precondition("The specified name is non-null and not empty")
-    private static ActionWrapper getWrapper(final String name) {
-        Logger.assertTrue(name != null);
-        Logger.assertTrue(!name.isEmpty());
-
-        for (ActionWrapper wrapper : actionWrappers) {
-            if (name.equals(wrapper.getGroupName())) {
-                return wrapper;
-            }
-        }
-
-        return null;
-    }
-
-    /**
-     * Determines whether the specified group name is one used by an action
-     * wrapper.
-     *
-     * @param name The group name to test
-     * @return True if the group is part of a wrapper, false otherwise
-     */
-    @Precondition("The specified name is non-null and not empty")
-    private static boolean isWrappedGroup(final String name) {
-        Logger.assertTrue(name != null);
-        Logger.assertTrue(!name.isEmpty());
-
-        return getWrapper(name) != null;
-    }
-
     /**
      * Loads action files from a specified group directory.
      *
@@ -317,7 +268,9 @@ public final class ActionManager {
         Logger.assertTrue(dir != null);
         Logger.assertTrue(dir.isDirectory());
         
-        groups.put(dir.getName(), new ActionGroup(dir.getName()));
+        if (!groups.containsKey(dir.getName())) {
+            groups.put(dir.getName(), new ActionGroup(dir.getName()));
+        }
 
         for (File file : dir.listFiles()) {
             new Action(dir.getName(), file.getName());
@@ -337,11 +290,7 @@ public final class ActionManager {
             actions.add(trigger, action);
         }
 
-        if (isWrappedGroup(action.getGroup())) {
-            getWrapper(action.getGroup()).registerAction(action);
-        } else {
-            getGroup(action.getGroup()).add(action);
-        }
+        getGroup(action.getGroup()).add(action);
     }
 
     /**
@@ -494,7 +443,7 @@ public final class ActionManager {
         Logger.assertTrue(!group.isEmpty());
         Logger.assertTrue(groups.containsKey(group));
 
-        for (Action action : new ArrayList<Action>(groups.get(group))) {
+        for (Action action : groups.get(group).getActions()) {
             unregisterAction(action);
         }
 
@@ -541,7 +490,7 @@ public final class ActionManager {
 
         makeGroup(newName);
 
-        for (Action action : new ArrayList<Action>(groups.get(oldName))) {
+        for (Action action : groups.get(oldName).getActions()) {
             action.setGroup(newName);
             getGroup(oldName).remove(action);
             getGroup(newName).add(action);

src/com/dmdirc/actions/wrappers/ActionWrapper.java

@@ -1,82 +0,0 @@
-/*
- * Copyright (c) 2006-2008 Chris Smith, Shane Mc Cormack, Gregory Holmes
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- */
-
-package com.dmdirc.actions.wrappers;
-
-import com.dmdirc.actions.Action;
-
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * An action wrapper deals with a specific group of actions in order to provide
- * an even more user-friendly experience.
- * 
- * @author chris
- */
-public abstract class ActionWrapper {
-    
-    /** A list of registered actions. */
-    protected final List<Action> actions = new ArrayList<Action>();
-
-    /**
-     * Registers the specified action with this manager.
-     * 
-     * @param action The action to be registered
-     */
-    public void registerAction(final Action action) {
-        actions.add(action);
-    }
-    
-    /**
-     * Unregisters the specified action with this manager.
-     * 
-     * @param action The action to be unregistered
-     */
-    public void unregisterAction(final Action action) {
-        actions.remove(action);
-    }
-    
-    /**
-     * Removes all actions from this manager.
-     */
-    public void clearActions() {
-        actions.clear();
-    }
-    
-    /**
-     * Retrieves a list of actions registered with this wrapper.
-     * 
-     * @return A list of registered actions
-     */
-    public List<Action> getActions() {
-        return actions;
-    }
-    
-    /**
-     * Retrieve the group name that this wrapper is using.
-     * 
-     * @return This wrapper's group name
-     */
-    public abstract String getGroupName();
-    
-}

src/com/dmdirc/actions/wrappers/Alias.java

@@ -227,7 +227,7 @@ public final class Alias implements Serializable {
      */
     public Action createAction() {
         return new Action(
-                AliasWrapper.getAliasWrapper().getGroupName(),
+                AliasWrapper.getAliasWrapper().getName(),
                 getName(),
                 new ActionType[] {CoreActionType.UNKNOWN_COMMAND, },
                 getResponse(),

src/com/dmdirc/actions/wrappers/AliasWrapper.java

@@ -26,12 +26,13 @@ import com.dmdirc.Server;
 import com.dmdirc.ServerManager;
 import com.dmdirc.actions.Action;
 import com.dmdirc.actions.ActionCondition;
+import com.dmdirc.actions.ActionGroup;
 import com.dmdirc.actions.CoreActionType;
 import com.dmdirc.commandparser.CommandManager;
 import com.dmdirc.logger.ErrorLevel;
 import com.dmdirc.logger.Logger;
-
 import com.dmdirc.ui.input.TabCompletionType;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -40,7 +41,7 @@ import java.util.List;
  *
  * @author chris
  */
-public final class AliasWrapper extends ActionWrapper {
+public final class AliasWrapper extends ActionGroup {
     
     /** Singleton instance of the alias wrapper. */
     private static AliasWrapper me;
@@ -52,7 +53,7 @@ public final class AliasWrapper extends ActionWrapper {
      * Creates a new instance of AliasWrapper.
      */
     private AliasWrapper() {
-        super();
+        super("aliases");
     }
     
     /**
@@ -79,9 +80,9 @@ public final class AliasWrapper extends ActionWrapper {
     
     /** {@inheritDoc} */
     @Override
-    public void registerAction(final Action action) {
+    public void add(final Action action) {
         if (action.getTriggers()[0].equals(CoreActionType.UNKNOWN_COMMAND)) {
-            super.registerAction(action);
+            super.add(action);
             
             final String commandName = getCommandName(action);
             
@@ -97,9 +98,9 @@ public final class AliasWrapper extends ActionWrapper {
     
     /** {@inheritDoc} */
     @Override
-    public void unregisterAction(final Action action) {
+    public void remove(final Action action) {
         if (action.getTriggers()[0].equals(CoreActionType.UNKNOWN_COMMAND)) {
-            super.unregisterAction(action);
+            super.remove(action);
             
             final String commandName = getCommandName(action);
             
@@ -111,12 +112,6 @@ public final class AliasWrapper extends ActionWrapper {
         }
     }
     
-    /** {@inheritDoc} */
-    @Override
-    public String getGroupName() {
-        return "aliases";
-    }
-    
     /**
      * Retrieves the command name of the specified alias action.
      *

src/com/dmdirc/actions/wrappers/PerformWrapper.java

@@ -25,6 +25,7 @@ package com.dmdirc.actions.wrappers;
 import com.dmdirc.actions.Action;
 import com.dmdirc.actions.interfaces.ActionComponent;
 import com.dmdirc.actions.ActionCondition;
+import com.dmdirc.actions.ActionGroup;
 import com.dmdirc.actions.interfaces.ActionType;
 import com.dmdirc.actions.CoreActionComparison;
 import com.dmdirc.actions.CoreActionComponent;
@@ -40,7 +41,7 @@ import java.util.List;
  *
  * @author Chris
  */
-public class PerformWrapper extends ActionWrapper {
+public class PerformWrapper extends ActionGroup {
     
     /** A singleton instance of the Perform Wrapper. */
     private static PerformWrapper me = new PerformWrapper();
@@ -49,7 +50,7 @@ public class PerformWrapper extends ActionWrapper {
      * Creates a new instance of PerformWrapper.
      */
     private PerformWrapper() {
-        super();
+        super("performs");
     }
     
     /**
@@ -61,20 +62,15 @@ public class PerformWrapper extends ActionWrapper {
         return me;
     }
     
-    /** {@inheritDoc} */
-    public String getGroupName() {
-        return "performs";
-    }
-    
     /** {@inheritDoc} */
     @Override
-    public void registerAction(final Action action) {
+    public void add(final Action action) {
         if (action.getTriggers().length == 1
                 && action.getTriggers()[0] == CoreActionType.SERVER_CONNECTED
                 && action.getConditions().size() == 1
                 && (action.getConditions().get(0).getComponent() == CoreActionComponent.SERVER_NETWORK
                 || action.getConditions().get(0).getComponent() == CoreActionComponent.SERVER_NAME)) {
-            super.registerAction(action);
+            super.add(action);
         } else {
             Logger.userError(ErrorLevel.MEDIUM, "Invalid perform action: " + action.getName());
         }
@@ -138,7 +134,7 @@ public class PerformWrapper extends ActionWrapper {
         
         conditions.add(new ActionCondition(0, component, CoreActionComparison.STRING_EQUALS, server + network));
         
-        return new Action(getGroupName(), server + network, new ActionType[]{CoreActionType.SERVER_CONNECTED},
+        return new Action(getName(), server + network, new ActionType[]{CoreActionType.SERVER_CONNECTED},
                 new String[0], conditions, null);
     }
     
@@ -151,7 +147,7 @@ public class PerformWrapper extends ActionWrapper {
      * @return The matching action if one exists, or null
      */    
     private Action getAction(final ActionComponent component, final String target) {
-        for (Action action : actions) {
+        for (Action action : this) {
             if (action.getConditions().get(0).getComponent() == component
                     && action.getConditions().get(0).getTarget().equalsIgnoreCase(target)) {
                 return action;

src/com/dmdirc/commandparser/commands/global/AliasCommand.java

@@ -68,7 +68,7 @@ public final class AliasCommand extends GlobalCommand implements IntelligentComm
             return;
         }
         
-        for (Action alias : AliasWrapper.getAliasWrapper().getActions()) {
+        for (Action alias : AliasWrapper.getAliasWrapper()) {
             if (AliasWrapper.getCommandName(alias).substring(1).equalsIgnoreCase(args[0])) {
                 sendLine(origin, isSilent, FORMAT_ERROR, "Alias '" + args[0] + "' already exists.");
                 return;
@@ -89,7 +89,7 @@ public final class AliasCommand extends GlobalCommand implements IntelligentComm
      * @return True if the alias was deleted, false otherwise
      */
     private boolean doRemove(final String name) {
-        for (Action alias : AliasWrapper.getAliasWrapper().getActions()) {
+        for (Action alias : AliasWrapper.getAliasWrapper()) {
             if (AliasWrapper.getCommandName(alias).substring(1).equalsIgnoreCase(name)) {
                 alias.delete();
                 ActionManager.unregisterAction(alias);
@@ -128,7 +128,7 @@ public final class AliasCommand extends GlobalCommand implements IntelligentComm
             res.add("--remove");
             res.excludeAll();
         } else if (arg == 1 && previousArgs.get(0).equals("--remove")) {
-            for (Action alias : AliasWrapper.getAliasWrapper().getActions()) {
+            for (Action alias : AliasWrapper.getAliasWrapper()) {
                 res.add(alias.getName());
             }   
         } else if (arg >= 1 && !previousArgs.get(0).equals("--remove")) {

src/com/dmdirc/ui/swing/dialogs/actionseditor/ActionsEditorDialog.java

@@ -24,6 +24,7 @@ package com.dmdirc.ui.swing.dialogs.actionseditor;
 
 import com.dmdirc.Main;
 import com.dmdirc.actions.Action;
+import com.dmdirc.actions.ActionGroup;
 import com.dmdirc.actions.ActionManager;
 import com.dmdirc.actions.interfaces.ActionType;
 import com.dmdirc.ui.swing.MainFrame;
@@ -324,9 +325,7 @@ public final class ActionsEditorDialog extends StandardDialog implements
      * @return true iif there are duplicate names
      */
     private boolean checkDuplicateName(final String name) {
-        final List<Action> actions = ActionManager.getGroups().get(group);
-
-        for (Action loopAction : actions) {
+        for (Action loopAction : ActionManager.getGroups().get(group)) {
             if (loopAction.getName().equals(name) && !loopAction.equals(action)) {
                 return true;
             }

src/com/dmdirc/ui/swing/dialogs/actionsmanager/ActionTableModel.java

@@ -167,7 +167,7 @@ public class ActionTableModel extends AbstractTableModel {
             if (group == null) {
                 actions = new ArrayList<Action>();
             } else {
-                actions = new ArrayList<Action>(group);
+                actions = group.getActions();
             }
             fireTableDataChanged();
         }

src/com/dmdirc/ui/swing/dialogs/actionsmanager/ActionsGroupPanel.java

@@ -35,6 +35,7 @@ import java.awt.event.ActionListener;
 import java.awt.event.MouseAdapter;
 import java.awt.event.MouseEvent;
 
+import java.util.ArrayList;
 import javax.swing.JButton;
 import javax.swing.JOptionPane;
 import javax.swing.JPanel;
@@ -123,7 +124,7 @@ public final class ActionsGroupPanel extends JPanel implements ActionListener,
      */
     private void initComponents() {
         scrollPane = new JScrollPane();
-        model = new ActionTableModel(group);
+        model = new ActionTableModel(group == null ? new ArrayList<Action>() : group.getActions());
         table = new PackingTable(model, false, scrollPane, false) {
 
             /**

src/com/dmdirc/ui/swing/dialogs/aliases/AliasManagerDialog.java

@@ -192,9 +192,8 @@ public final class AliasManagerDialog extends StandardDialog implements
      */
     private List<Alias> getTableData() {
         final List<Alias> aliases = new ArrayList<Alias>();
-        final List<Action> actions = AliasWrapper.getAliasWrapper().getActions();
         
-        for (Action loopAction : actions) {
+        for (Action loopAction : AliasWrapper.getAliasWrapper()) {
             final List<ActionCondition> arguments = loopAction.getConditions();
             
             ActionCondition argument;
@@ -326,8 +325,7 @@ public final class AliasManagerDialog extends StandardDialog implements
     
     /** Saves the aliases. */
     private void save() {
-        final List<Action> actions = new ArrayList<Action>(
-                AliasWrapper.getAliasWrapper().getActions());
+        final List<Action> actions = AliasWrapper.getAliasWrapper().getActions();
         final List<Alias> aliases = tableModel.getAliases();
         
         final List<Alias> newAliases = new ArrayList<Alias>();
@@ -395,8 +393,7 @@ public final class AliasManagerDialog extends StandardDialog implements
      * @return Corresponding action or null if none found
      */
     private Action getAction(final Alias alias) {
-        final List<Action> actions = new ArrayList<Action>(
-                AliasWrapper.getAliasWrapper().getActions());
+        final List<Action> actions = AliasWrapper.getAliasWrapper().getActions();
         Action action = null;
         
         for (Action loopAction : actions) {

test/com/dmdirc/actions/ActionManagerTest.java

@@ -77,8 +77,8 @@ public class ActionManagerTest extends junit.framework.TestCase {
                 new File(ActionManager.getDirectory() + "unit-test").isDirectory());        
         assertSame("renameGroup must move actions to new group",
                 action, ActionManager.getGroup("unit-test").get(0));
-        assertTrue("renameGroup must remove actions from old group",
-                ActionManager.getGroup("unit-test-two").isEmpty());
+        assertEquals("renameGroup must remove actions from old group",
+                0, ActionManager.getGroup("unit-test-two").size());
     }
     
     @Test