JAL-3048 wip externalise trigger from RunResponse
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 10 Dec 2018 15:29:08 +0000 (15:29 +0000)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 10 Dec 2018 15:29:08 +0000 (15:29 +0000)
19 files changed:
src/jalview/gui/AlignExportOptions.java
src/jalview/gui/AlignFrame.java
src/jalview/gui/AlignViewport.java
src/jalview/gui/Desktop.java
src/jalview/gui/EditNameDialog.java
src/jalview/gui/FeatureEditor.java
src/jalview/gui/FeatureSettings.java
src/jalview/gui/ImageExporter.java
src/jalview/gui/JvOptionPane.java
src/jalview/gui/LineartOptions.java
src/jalview/gui/TextColourChooser.java
src/jalview/gui/UserDefinedColours.java
src/jalview/io/HtmlSvgOutput.java
src/jalview/io/JalviewFileChooser.java
src/jalview/util/dialogrunner/DialogRunner.java
src/jalview/util/dialogrunner/DialogRunnerI.java
src/jalview/util/dialogrunner/RunResponse.java
test/jalview/util/dialogrunner/DialogRunnerTest.java
test/jalview/util/dialogrunner/RunResponseTest.java

index ecf89cc..db78bee 100644 (file)
@@ -120,9 +120,9 @@ public class AlignExportOptions extends JPanel
    * 
    * @param action
    */
-  public void setResponseAction(RunResponse action)
+  public void setResponseAction(Object response, RunResponse action)
   {
-    dialog.addResponse(action);
+    dialog.addResponse(response, action);
   }
 
   /**
index 7dbed05..a1d97e7 100644 (file)
@@ -1299,8 +1299,8 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     {
       AlignExportOptions choices = new AlignExportOptions(
               alignPanel.getAlignViewport(), format, options);
-      choices.setResponseAction(outputAction);
-      choices.setResponseAction(cancelAction);
+      choices.setResponseAction(0, outputAction);
+      choices.setResponseAction(1, cancelAction);
       choices.showDialog();
     }
     else
@@ -1359,7 +1359,7 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     {
       AlignExportOptions choices = new AlignExportOptions(
               alignPanel.getAlignViewport(), fileFormat, options);
-      choices.setResponseAction(outputAction);
+      choices.setResponseAction(0, outputAction);
       choices.showDialog();
     }
     else
@@ -1472,9 +1472,8 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
             MessageManager.getString("label.load_jalview_annotations"));
     chooser.setToolTipText(
             MessageManager.getString("label.load_jalview_annotations"));
-    chooser.addResponse(new RunResponse(JalviewFileChooser.APPROVE_OPTION)
+    chooser.addResponse(0, new RunResponse(JalviewFileChooser.APPROVE_OPTION)
     {
-
       @Override
       public void run()
       {
@@ -1482,7 +1481,6 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
         jalview.bin.Cache.setProperty("LAST_DIRECTORY", choice);
         loadJalviewDataFile(chooser.getSelectedFile(), null, null, null);
       }
-
     });
 
     chooser.showOpenDialog(this);
@@ -3966,8 +3964,7 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     chooser.setToolTipText(
             MessageManager.getString("label.load_tree_file"));
 
-    chooser.addResponse(new jalview.util.dialogrunner.RunResponse(
-            JalviewFileChooser.APPROVE_OPTION)
+    chooser.addResponse(0,new RunResponse(JalviewFileChooser.APPROVE_OPTION)
     {
       @Override
       public void run()
@@ -5698,19 +5695,15 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     chooser.setDialogTitle(MessageManager.getString("label.load_vcf_file"));
     chooser.setToolTipText(MessageManager.getString("label.load_vcf_file"));
     final AlignFrame us = this;
-    chooser.addResponse(new RunResponse(JalviewFileChooser.APPROVE_OPTION)
+    chooser.addResponse(0, new RunResponse(JalviewFileChooser.APPROVE_OPTION)
     {
       @Override
       public void run()
       {
-
-        {
-          String choice = chooser.getSelectedFile().getPath();
-          Cache.setProperty("LAST_DIRECTORY", choice);
-          SequenceI[] seqs = viewport.getAlignment().getSequencesArray();
-          new VCFLoader(choice).loadVCF(seqs, us);
-        }
-
+        String choice = chooser.getSelectedFile().getPath();
+        Cache.setProperty("LAST_DIRECTORY", choice);
+        SequenceI[] seqs = viewport.getAlignment().getSequencesArray();
+        new VCFLoader(choice).loadVCF(seqs, us);
       };
     });
     chooser.showOpenDialog(null);
index ee61308..7b8c6c9 100644 (file)
@@ -771,21 +771,21 @@ public class AlignViewport extends AlignmentViewport
      * in reverse order)
      */
     JvOptionPane dialog = JvOptionPane.newOptionDialog(Desktop.desktop)
-            .addResponse(new RunResponse(0)
+            .addResponse(0, new RunResponse(0)
             {
               @Override
               public void run()
               {
                   addDataToAlignment(al);
               }
-            }).addResponse(new RunResponse(1)
+            }).addResponse(1, new RunResponse(1)
             {
               @Override
               public void run()
               {
                 us.openLinkedAlignmentAs(al, title, true);
               }
-            }).addResponse(new RunResponse(2)
+            }).addResponse(2, new RunResponse(2)
             {
               @Override
               public void run()
index 6b7a278..1e258b8 100644 (file)
@@ -1125,9 +1125,8 @@ public class Desktop extends jalview.jbgui.GDesktop
             MessageManager.getString("label.open_local_file"));
     chooser.setToolTipText(MessageManager.getString("action.open"));
 
-    chooser.addResponse(new RunResponse(JalviewFileChooser.APPROVE_OPTION)
+    chooser.addResponse(0, new RunResponse(JalviewFileChooser.APPROVE_OPTION)
     {
-
       @Override
       public void run()
       {
@@ -1263,7 +1262,7 @@ public class Desktop extends jalview.jbgui.GDesktop
       }};
     String dialogOption = MessageManager
             .getString("label.input_alignment_from_url");
-    JvOptionPane.newOptionDialog(desktop).addResponse(action)
+    JvOptionPane.newOptionDialog(desktop).addResponse(0, action)
             .showInternalDialog(panel, dialogOption,
                     JvOptionPane.YES_NO_CANCEL_OPTION,
                     JvOptionPane.PLAIN_MESSAGE, null, options,
@@ -1746,7 +1745,7 @@ public class Desktop extends jalview.jbgui.GDesktop
             "Jalview Project");
     chooser.setFileView(new JalviewFileView());
     chooser.setDialogTitle(MessageManager.getString("label.restore_state"));
-    chooser.addResponse(new RunResponse(JalviewFileChooser.APPROVE_OPTION)
+    chooser.addResponse(0, new RunResponse(JalviewFileChooser.APPROVE_OPTION)
     {
       @Override
       public void run()
index bbb7a20..c976545 100644 (file)
@@ -107,9 +107,7 @@ public class EditNameDialog
   }
 
   /**
-   * Shows the dialog, and runs the response action if OK is selected. Note the
-   * RunResponse should be constructed to act on dialog return value
-   * JvOptionPane.OK_OPTION.
+   * Shows the dialog, and runs the response action if OK is selected
    * 
    * @param action
    */
@@ -118,7 +116,7 @@ public class EditNameDialog
   {
     Object[] options = new Object[] { MessageManager.getString("action.ok"),
         MessageManager.getString("action.cancel") };
-    JvOptionPane.newOptionDialog(parent).addResponse(action)
+    JvOptionPane.newOptionDialog(parent).addResponse(0, action)
             .showInternalDialog(panel, title,
                     JvOptionPane.YES_NO_CANCEL_OPTION,
                     JvOptionPane.PLAIN_MESSAGE, null, options,
index d75404a..b829ec7 100644 (file)
@@ -414,10 +414,10 @@ public class FeatureEditor
      * also for Delete if applicable (when amending features)
      */
     JvOptionPane dialog = JvOptionPane.newOptionDialog(Desktop.desktop)
-            .addResponse(okAction).addResponse(cancelAction);
+            .addResponse(0, okAction).addResponse(2, cancelAction);
     if (!forCreate)
     {
-      dialog.addResponse(getDeleteAction());
+      dialog.addResponse(1, getDeleteAction());
     }
 
     String title = null;
index 36af39a..9165a3d 100644 (file)
@@ -898,13 +898,15 @@ public class FeatureSettings extends JPanel
     chooser.setDialogTitle(
             MessageManager.getString("label.load_feature_colours"));
     chooser.setToolTipText(MessageManager.getString("action.load"));
-    chooser.addResponse(new RunResponse(JalviewFileChooser.APPROVE_OPTION){
-
-               @Override
-               public void run() {
-                     File file = chooser.getSelectedFile();
-                     load(file);
-               }});
+    chooser.addResponse(0, new RunResponse(JalviewFileChooser.APPROVE_OPTION)
+    {
+         @Override
+         public void run() 
+         {
+           File file = chooser.getSelectedFile();
+               load(file);
+         }
+       });
     chooser.showOpenDialog(this);
   }
 
index 4c2c8ba..0689220 100644 (file)
@@ -145,7 +145,7 @@ public class ImageExporter
       };
       LineartOptions epsOption = new LineartOptions(TYPE.EPS.getName(),
               textSelected);
-      epsOption.setResponseAction(new RunResponse(JOptionPane.NO_OPTION)
+      epsOption.setResponseAction(1, new RunResponse(JOptionPane.NO_OPTION)
       {
         @Override
         public void run()
@@ -155,7 +155,7 @@ public class ImageExporter
                   imageType.getName()), messageId);
         }
       });
-      epsOption.setResponseAction(okAction);
+      epsOption.setResponseAction(0, okAction);
       epsOption.showDialog();
       /* no code here - JalviewJS cannot execute it */
     }
index 92a57ca..b3df2f7 100644 (file)
@@ -822,12 +822,12 @@ public class JvOptionPane extends JOptionPane implements DialogRunnerI,
     {
       runner.handleResponse(response);
     }
-    
   }
+    
   @Override
-  public JvOptionPane addResponse(RunResponse action)
+  public JvOptionPane addResponse(Object response, RunResponse action)
   {
-    runner.addResponse(action);
+    runner.addResponse(response, action);
     return this;
   }
 
index 765a1ac..ccf7edb 100644 (file)
@@ -96,9 +96,9 @@ public class LineartOptions extends JPanel
    * 
    * @param action
    */
-  public void setResponseAction(RunResponse action)
+  public void setResponseAction(Object response, RunResponse action)
   {
-    dialog.addResponse(action);
+    dialog.addResponse(response, action);
   }
 
   /**
index 01d9b81..ae9fda9 100644 (file)
@@ -158,7 +158,7 @@ public class TextColourChooser
         restoreInitialSettings();
       }
     };
-    JvOptionPane.newOptionDialog(alignPanel).addResponse(action)
+    JvOptionPane.newOptionDialog(alignPanel).addResponse(1, action)
             .showInternalDialog(bigpanel, title,
                     JvOptionPane.YES_NO_CANCEL_OPTION,
                     JvOptionPane.PLAIN_MESSAGE, null, options,
index f1ff128..9e928d8 100755 (executable)
@@ -645,43 +645,46 @@ public class UserDefinedColours extends GUserDefinedColours
     chooser.setDialogTitle(
             MessageManager.getString("label.load_colour_scheme"));
     chooser.setToolTipText(MessageManager.getString("action.load"));
-    chooser.addResponse(new RunResponse(JalviewFileChooser.APPROVE_OPTION) {
-               @Override
-               public void run() {
-                   File choice = chooser.getSelectedFile();
-                   Cache.setProperty(LAST_DIRECTORY, choice.getParent());
-
-                   UserColourScheme ucs = ColourSchemeLoader
-                           .loadColourScheme(choice.getAbsolutePath());
-                   Color[] colors = ucs.getColours();
-                   schemeName.setText(ucs.getSchemeName());
-
-                   if (ucs.getLowerCaseColours() != null)
-                   {
-                     caseSensitive.setSelected(true);
-                     lcaseColour.setEnabled(true);
-                     resetButtonPanel(true);
-                     for (int i = 0; i < lowerCaseButtons.size(); i++)
-                     {
-                       JButton button = lowerCaseButtons.get(i);
-                       button.setBackground(ucs.getLowerCaseColours()[i]);
-                     }
-                   }
-                   else
-                   {
-                     caseSensitive.setSelected(false);
-                     lcaseColour.setEnabled(false);
-                     resetButtonPanel(false);
-                   }
-
-                   for (int i = 0; i < upperCaseButtons.size(); i++)
-                   {
-                     JButton button = upperCaseButtons.get(i);
-                     button.setBackground(colors[i]);
-                   }
-
-                   addNewColourScheme(choice.getPath());
-               }});
+    chooser.addResponse(0, new RunResponse(JalviewFileChooser.APPROVE_OPTION) 
+    {
+         @Override
+         public void run() 
+         {
+           File choice = chooser.getSelectedFile();
+           Cache.setProperty(LAST_DIRECTORY, choice.getParent());
+
+           UserColourScheme ucs = ColourSchemeLoader
+                   .loadColourScheme(choice.getAbsolutePath());
+           Color[] colors = ucs.getColours();
+           schemeName.setText(ucs.getSchemeName());
+
+           if (ucs.getLowerCaseColours() != null)
+               {
+                  caseSensitive.setSelected(true);
+                  lcaseColour.setEnabled(true);
+                  resetButtonPanel(true);
+                  for (int i = 0; i < lowerCaseButtons.size(); i++)
+                  {
+                    JButton button = lowerCaseButtons.get(i);
+                    button.setBackground(ucs.getLowerCaseColours()[i]);
+                  }
+                }
+                else
+                {
+                  caseSensitive.setSelected(false);
+                  lcaseColour.setEnabled(false);
+                  resetButtonPanel(false);
+                }
+
+                for (int i = 0; i < upperCaseButtons.size(); i++)
+                {
+                  JButton button = upperCaseButtons.get(i);
+                  button.setBackground(colors[i]);
+                }
+
+                addNewColourScheme(choice.getPath());
+          }
+       });
     
     chooser.showOpenDialog(this);
   }
index a943a81..1f82853 100644 (file)
@@ -260,7 +260,7 @@ public class HtmlSvgOutput extends HTMLOutput
       if (renderStyle.equalsIgnoreCase("Prompt each time") && !isHeadless())
       {
         LineartOptions svgOption = new LineartOptions("HTML", textOption);
-        svgOption.setResponseAction(new RunResponse(JOptionPane.NO_OPTION)
+        svgOption.setResponseAction(1, new RunResponse(JOptionPane.NO_OPTION)
         {
           @Override
           public void run()
@@ -270,7 +270,7 @@ public class HtmlSvgOutput extends HTMLOutput
                     getDescription()));
           }
         });
-        svgOption.setResponseAction(okAction);
+        svgOption.setResponseAction(0, okAction);
         svgOption.showDialog();
         /* no code here - JalviewJS cannot execute it */
       }
index 8e0fe30..6fa6016 100755 (executable)
@@ -73,80 +73,6 @@ public class JalviewFileChooser extends JFileChooser implements DialogRunnerI,
   File selectedFile = null;
 
   /**
-   * On user selecting a file to save to, this response is run to check if the
-   * file already exists, and if so show a dialog to prompt for confirmation of
-   * overwrite.
-   */
-  RunResponse overwriteCheck = new RunResponse(JalviewFileChooser.APPROVE_OPTION)
-  {
-    @Override
-    public void run()
-       {
-           selectedFile = getSelectedFile();
-       
-           if (selectedFile == null)
-           {
-             // Workaround for Java 9,10 on OSX - no selected file, but there is a
-             // filename typed in
-             // TODO is this needed in Java 8 or 11?
-             try
-             {
-               String filename = ((BasicFileChooserUI) getUI()).getFileName();
-               if (filename != null && filename.length() > 0)
-               {
-                 selectedFile = new File(getCurrentDirectory(), filename);
-               }
-             } catch (Throwable x)
-             {
-               System.err.println(
-                       "Unexpected exception when trying to get filename.");
-               x.printStackTrace();
-             }
-           }
-           if (selectedFile == null)
-           {
-             setReturnValue(JalviewFileChooser.CANCEL_OPTION);
-             return;
-           }
-           // JBP Note - this code was executed regardless of 'SAVE' being pressed
-           // need to see if there were side effects
-           if (getFileFilter() instanceof JalviewFileFilter)
-           {
-             JalviewFileFilter jvf = (JalviewFileFilter) getFileFilter();
-       
-             if (!jvf.accept(getSelectedFile()))
-             {
-               String withExtension = getSelectedFile() + "."
-                       + jvf.getAcceptableExtension();
-               setSelectedFile(new File(withExtension));
-             }
-           }
-           // All good, so we continue to save
-           setReturnValue(JalviewFileChooser.APPROVE_OPTION);
-       
-           // TODO: ENSURE THAT FILES SAVED WITH A ':' IN THE NAME ARE REFUSED AND THE
-           // USER PROMPTED FOR A NEW FILENAME
-           if (!Jalview.isJS())
-           {
-             if (getSelectedFile().exists())
-             {
-               // JAL-3048 - may not need to raise this for browser saves
-               // yes/no cancel
-               int confirm = JvOptionPane.showConfirmDialog(JalviewFileChooser.this,
-                       MessageManager.getString("label.overwrite_existing_file"),
-                       MessageManager.getString("label.file_already_exists"),
-                 JvOptionPane.YES_NO_OPTION);
-       
-             if (confirm != JvOptionPane.YES_OPTION)
-             {
-                  setReturnValue(JalviewFileChooser.CANCEL_OPTION);
-                }
-              }
-            }
-       };
-  };
-
-  /**
    * Factory method to return a file chooser that offers readable alignment file
    * formats
    * 
@@ -575,9 +501,9 @@ public class JalviewFileChooser extends JFileChooser implements DialogRunnerI,
   }
 
   @Override
-  public DialogRunnerI addResponse(RunResponse action)
+  public DialogRunnerI addResponse(Object response, RunResponse action)
   {
-    return runner.addResponse(action);
+    return runner.addResponse(response, action);
   }
 
   /**
index 59f3c98..b6fd6a2 100644 (file)
@@ -26,23 +26,18 @@ import java.util.List;
 import java.util.Map;
 
 /**
- * daft gymnastics to allow Dialogs to extend from a Swing class and use this
- * class to implement chained Response run() definition and execution.
- * 
- * There is probably a better way of doing this.
+ * A helper class that executes registered Runnable actions corresponding to 
+ * user responses in a dialog. This is to enable dialog execution in JalviewJS,
+ * where everything must happen in a single thread of execution. That is, the
+ * dialog has to 'know' all actions that follow a user choice, rather than
+ * returning a response to allow a separate thread to decide the next action.
  * 
  * @author jprocter
- *
- * @param <T>
- *          the actual dialog that will be shown - which will also initiate the
- *          response chain.
  */
 public class DialogRunner implements DialogRunnerI
 {
   private Map<Object, List<RunResponse>> callbacks = new HashMap<>();
 
-  private boolean firstRunWasCalled = false;
-
   /**
    * Constructor
    */
@@ -50,61 +45,19 @@ public class DialogRunner implements DialogRunnerI
   {
   }
 
-  /**
-   * Reset so handleResponse will start response execution
-   */
-  public void resetResponses()
-  {
-    firstRunWasCalled = false;
-  }
-
   @Override
-  public DialogRunnerI addResponse(RunResponse action)
+  public DialogRunnerI addResponse(Object response, RunResponse action)
   {
-    addResponse(false, action);
-    return this;
-  }
-
-  /**
-   * insert a response at the beginning of the chain for the action. Useful to add
-   * pre-action validations local to the Dialog class
-   * 
-   * @param action
-   * @return
-   */
-  public DialogRunnerI setFirstResponse(RunResponse action)
-  {
-    return addResponse(true, action);
-  }
-
-  protected DialogRunnerI addResponse(boolean prePend, RunResponse action)
-  {
-    List<RunResponse> actions = callbacks.get(action.getTrigger());
+       List<RunResponse> actions = callbacks.get(response);
     if (actions == null)
     {
       actions = new ArrayList<>();
-      callbacks.put(action.getTrigger(), actions);
-    }
-    if (prePend)
-    {
-      actions.add(0,action);
-    } else {
-      actions.add(action);
+      callbacks.put(response, actions);
     }
+    actions.add(action);
     return this;
   }
-
-  /**
-   * 
-   * @param action
-   * @return true if action is a registered callback
-   */
-  public boolean isRegistered(RunResponse action)
-  {
-    List<RunResponse> resp = callbacks.get(action.getTrigger());
-    return resp != null && resp.contains(action);
-  }
-
+  
   /**
    * Handles a response by running the chain of registered actions (if any).
    * Answers the list of responses run (in order).
@@ -124,15 +77,6 @@ public class DialogRunner implements DialogRunnerI
       return responsesRun;
     }
     
-    /*
-     * failsafe check for illegal duplicate call(?)
-     */
-    if (firstRunWasCalled)
-    {
-//      return responsesRun;
-    }
-    firstRunWasCalled = true;
-    
        runResponse(response, responsesRun);
     if (responsesRun.isEmpty())
     {
index 6914656..cc4a836 100644 (file)
@@ -31,17 +31,7 @@ import java.util.List;
 public interface DialogRunnerI
 {
 
-  /**
-   * Adds a new response for this dialog, and returns the dialog (this), to allow chaining, eg.
-   * <pre>
-   * dialog.addResponse(newRunResponse(OK_PRessed) { run()...})
-   *     .addResponse(new RunResponse(CANCEL_PRESSED);
-   * </pre>
-   * 
-   * @param action
-   * @return
-   */
-  DialogRunnerI addResponse(RunResponse action);
+  DialogRunnerI addResponse(Object response, RunResponse action);
 
   /**
    * Runs any registered handlers for the given response, and answers the list
index 7cb7e08..b97525b 100644 (file)
@@ -32,17 +32,12 @@ public abstract class RunResponse implements Runnable
   /**
    * Response that triggers the Run method
    */
-  private Object trigger;
+  Object trigger;
 
   /**
    * set by run() on exit
    */
-  private Object returnValue = null;
-
-  /**
-   * set by dialog runner
-   */
-  private boolean wasRun = false;
+  protected Object returnValue = null;
 
   public RunResponse(Object onTrigger)
   {
@@ -56,7 +51,6 @@ public abstract class RunResponse implements Runnable
   
   public void reset()
   {
-    wasRun = false;
     returnValue = null;
   }
 
@@ -75,14 +69,4 @@ public abstract class RunResponse implements Runnable
   {
        returnValue = o;
   }
-  
-  public boolean hasBeenRun()
-  {
-       return wasRun;
-  }
-  
-  public void setRun()
-  {
-    wasRun = true;
-  }
 }
index 7d7e5e8..b89c4bf 100644 (file)
@@ -1,8 +1,17 @@
 package jalview.util.dialogrunner;
 
-import org.testng.Assert;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
 import org.testng.annotations.Test;
 
+import junit.extensions.PA;
+
 public class DialogRunnerTest
 {
   public class MockDialog implements DialogRunnerI
@@ -10,20 +19,20 @@ public class DialogRunnerTest
     DialogRunnerI runner = new DialogRunner();
 
     @Override
-    public DialogRunnerI addResponse(RunResponse action)
+    public DialogRunnerI addResponse(Object response, RunResponse action)
     {
-      return runner.addResponse(action);
+      return runner.addResponse(response, action);
     }
 
-    public void doDialog(String resp)
+    public List<RunResponse> doDialog(String resp)
     {
-      runner.handleResponse(resp);
+      return runner.handleResponse(resp);
     }
 
        @Override
-       public void handleResponse(Object response) {
-               // TODO Auto-generated method stub
-               
+       public List<RunResponse> handleResponse(Object response) 
+       {
+         return null;
        }
   }
 
@@ -43,20 +52,18 @@ public class DialogRunnerTest
 
     ok = new RunResponse("OK")
     {
-
       @Override
       public void run()
       {
-        returned = "DONE";
+        returnValue = "DONE";
       }
     };
     final RunResponse befok = new RunResponse("OK")
     {
-
       @Override
       public void run()
       {
-        returned = "OK";
+         returnValue = "OK";
       }
     };
 
@@ -65,7 +72,7 @@ public class DialogRunnerTest
       @Override
       public void run()
       {
-        returned = r_ddoit;
+         returnValue = r_ddoit;
       }
     };
     help = new RunResponse("HELP")
@@ -73,8 +80,7 @@ public class DialogRunnerTest
       @Override
       public void run()
       {
-        returned = r_needsb;
-
+         returnValue = r_needsb;
       }
     };
     ineed = new RunResponse(r_needsb)
@@ -82,56 +88,64 @@ public class DialogRunnerTest
       @Override
       public void run()
       {
-        returned = ooh;
+         returnValue = ooh;
       }
     };
 
-    Assert.assertFalse(dialog.runner.isRegistered(ok));
+    assertFalse(isRegistered(dialog.runner, ok));
 
-    dialog.addResponse(ok).addResponse(cancel).addResponse(help).addResponse(ineed);
+    dialog.addResponse("OK", ok).addResponse("CANCEL", cancel).
+        addResponse("HELP", help).addResponse(r_needsb, ineed);
 
-    Assert.assertTrue(dialog.runner.isRegistered(ok));
+    assertTrue(isRegistered(dialog.runner, ok));
 
-    Assert.assertFalse(dialog.runner.firstRunWasCalled);
-    dialog.doDialog("OK");
+//    Assert.assertFalse(dialog.runner.firstRunWasCalled);
+    List<RunResponse> actions = dialog.doDialog("OK");
     // OK called, nothing else.
-    Assert.assertTrue(dialog.runner.firstRunWasCalled);
-    Assert.assertTrue(ok.wasRun);
-    Assert.assertEquals(ok.returned, r_done);
-    Assert.assertFalse(cancel.wasRun);
-    Assert.assertEquals(dialog.runner.responses.size(), 2);
+  //  Assert.assertTrue(dialog.runner.firstRunWasCalled);
+    assertTrue(actions.contains(ok));//ok.wasRun);
+    assertEquals(ok.returnValue, r_done);
+    assertFalse(actions.contains(cancel));//cancel.wasRun);
+    assertEquals(actions/*dialog.runner.responses*/.size(), 2);
 
     // do it again - check it doesn't trigger again
-    ok.wasRun = false;
-    dialog.doDialog("OK");
-    Assert.assertFalse(ok.wasRun);
+    //ok.wasRun = false;
+    actions = dialog.doDialog("OK");
+    assertFalse(actions.contains(ok));
 
     // reset - everything false/null
-    dialog.runner.resetResponses();
-    Assert.assertFalse(dialog.runner.firstRunWasCalled);
-    Assert.assertFalse(ok.wasRun);
-    Assert.assertNull(ok.returned);
-    Assert.assertEquals(dialog.runner.responses.size(), 0);
+//    dialog.runner.resetResponses();
+//    Assert.assertFalse(dialog.runner.firstRunWasCalled);
+//    Assert.assertFalse(ok.wasRun);
+//    Assert.assertNull(ok.returned);
+//    Assert.assertEquals(dialog.runner.responses.size(), 0);
 
     // cancel called ..
-    dialog.doDialog("HELP");
-    Assert.assertTrue(dialog.runner.firstRunWasCalled);
-    Assert.assertFalse(ok.wasRun);
-    Assert.assertEquals(ineed.returned, ooh);
-    Assert.assertEquals(dialog.runner.responses.size(), 3);
+    actions = dialog.doDialog("HELP");
+//    Assert.assertTrue(dialog.runner.firstRunWasCalled);
+    assertFalse(actions.contains(ok));//ok.wasRun);
+    assertEquals(ineed.returnValue, ooh);
+    assertEquals(actions/*dialog.runner.responses*/.size(), 3);
 
     // TODO: test prepend and chained execution of tasks for a response.
-    Assert.assertFalse(dialog.runner.isRegistered(befok));
-    dialog.runner.setFirstResponse(befok);
+//    Assert.assertFalse(dialog.runner.isRegistered(befok));
+    dialog.runner.addResponse("OK", befok); //setFirstResponse(befok);
+
+    assertTrue(isRegistered(dialog.runner, befok));
+    assertTrue(isRegistered(dialog.runner, ok));
 
-    Assert.assertTrue(dialog.runner.isRegistered(befok));
-    Assert.assertTrue(dialog.runner.isRegistered(ok));
+//    dialog.runner.resetResponses();
 
-    dialog.runner.resetResponses();
+    actions = dialog.doDialog("OK");
+    assertTrue(actions.contains(befok));//befok.wasRun);
+    assertTrue(actions.contains(ok)); //ok.wasRun);
+    assertEquals(actions/*dialog.runner.responses*/.size(), 3);
+  }
 
-    dialog.doDialog("OK");
-    Assert.assertTrue(befok.wasRun);
-    Assert.assertTrue(ok.wasRun);
-    Assert.assertEquals(dialog.runner.responses.size(), 3);
+  private boolean isRegistered(DialogRunnerI runner, RunResponse action) 
+  {
+       Map<Object, Collection<Object>> actions  = (Map<Object, Collection<Object>>) PA.getValue(runner,  "callbacks");
+       Collection<Object> registered = actions.get(action.getTrigger());
+       return registered != null && registered.contains(action);
   }
 }
index 4942b32..a8ae22e 100644 (file)
@@ -1,6 +1,8 @@
 package jalview.util.dialogrunner;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotEquals;
+import static org.testng.Assert.assertNull;
 
-import org.testng.Assert;
 import org.testng.annotations.Test;
 
 public class RunResponseTest
@@ -8,30 +10,23 @@ public class RunResponseTest
   @Test
   public void testRunResponse()
   {
-
     RunResponse rr = new RunResponse("OK")
     {
       @Override
       public void run()
       {
-        returned = "DONE";
+        returnValue = "DONE";
       }
     };
-    Assert.assertEquals(rr.ourTrigger, "OK");
-    Assert.assertNotEquals(rr.ourTrigger, "NOTOK");
-    Assert.assertNull(rr.returned);
-    Assert.assertFalse(rr.wasRun);
-    // trivial ..
-    rr.wasRun = true;
-    rr.run();
-    Assert.assertTrue(rr.wasRun);
+    assertEquals(rr.trigger, "OK");
+    assertNotEquals(rr.trigger, "NOTOK");
+    assertNull(rr.returnValue);
 
-    Assert.assertEquals(rr.returned, "DONE");
+    assertEquals(rr.returnValue, "DONE");
     rr.reset();
-    Assert.assertNull(rr.returned);
-    Assert.assertFalse(rr.wasRun);
+    assertNull(rr.returnValue);
 
-    Assert.assertEquals(rr.toString(), "Runner for " + "OK");
+    assertEquals(rr.toString(), "Runner for " + "OK");
 
     // just test the other constructors
     RunResponse rr12 = new RunResponse(12)
@@ -39,7 +34,7 @@ public class RunResponseTest
       @Override
       public void run()
       {
-        returned = "DONE";
+        returnValue = "DONE";
       }
     };
     RunResponse rrpi = new RunResponse(new Double(3.142))
@@ -47,10 +42,10 @@ public class RunResponseTest
       @Override
       public void run()
       {
-        returned = "DONE";
+        returnValue = "DONE";
       }
     };
-    Assert.assertEquals(rr12.ourTrigger, Integer.valueOf(12));
-    Assert.assertEquals(rrpi.ourTrigger, Double.valueOf(3.142));
+    assertEquals(rr12.trigger, Integer.valueOf(12));
+    assertEquals(rrpi.trigger, Double.valueOf(3.142));
 }
 }