From 1e28a196997a1e0b8b74d468bfd3df8ec74c1337 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Mon, 10 Dec 2018 15:29:08 +0000 Subject: [PATCH 1/1] JAL-3048 wip externalise trigger from RunResponse --- src/jalview/gui/AlignExportOptions.java | 4 +- src/jalview/gui/AlignFrame.java | 27 ++--- src/jalview/gui/AlignViewport.java | 6 +- src/jalview/gui/Desktop.java | 7 +- src/jalview/gui/EditNameDialog.java | 6 +- src/jalview/gui/FeatureEditor.java | 4 +- src/jalview/gui/FeatureSettings.java | 16 +-- src/jalview/gui/ImageExporter.java | 4 +- src/jalview/gui/JvOptionPane.java | 6 +- src/jalview/gui/LineartOptions.java | 4 +- src/jalview/gui/TextColourChooser.java | 2 +- src/jalview/gui/UserDefinedColours.java | 77 +++++++------- src/jalview/io/HtmlSvgOutput.java | 4 +- src/jalview/io/JalviewFileChooser.java | 78 +------------- src/jalview/util/dialogrunner/DialogRunner.java | 76 ++------------ src/jalview/util/dialogrunner/DialogRunnerI.java | 12 +-- src/jalview/util/dialogrunner/RunResponse.java | 20 +--- .../util/dialogrunner/DialogRunnerTest.java | 110 +++++++++++--------- .../jalview/util/dialogrunner/RunResponseTest.java | 33 +++--- 19 files changed, 172 insertions(+), 324 deletions(-) diff --git a/src/jalview/gui/AlignExportOptions.java b/src/jalview/gui/AlignExportOptions.java index ecf89cc..db78bee 100644 --- a/src/jalview/gui/AlignExportOptions.java +++ b/src/jalview/gui/AlignExportOptions.java @@ -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); } /** diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java index 7dbed05..a1d97e7 100644 --- a/src/jalview/gui/AlignFrame.java +++ b/src/jalview/gui/AlignFrame.java @@ -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); diff --git a/src/jalview/gui/AlignViewport.java b/src/jalview/gui/AlignViewport.java index ee61308..7b8c6c9 100644 --- a/src/jalview/gui/AlignViewport.java +++ b/src/jalview/gui/AlignViewport.java @@ -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() diff --git a/src/jalview/gui/Desktop.java b/src/jalview/gui/Desktop.java index 6b7a278..1e258b8 100644 --- a/src/jalview/gui/Desktop.java +++ b/src/jalview/gui/Desktop.java @@ -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() diff --git a/src/jalview/gui/EditNameDialog.java b/src/jalview/gui/EditNameDialog.java index bbb7a20..c976545 100644 --- a/src/jalview/gui/EditNameDialog.java +++ b/src/jalview/gui/EditNameDialog.java @@ -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, diff --git a/src/jalview/gui/FeatureEditor.java b/src/jalview/gui/FeatureEditor.java index d75404a..b829ec7 100644 --- a/src/jalview/gui/FeatureEditor.java +++ b/src/jalview/gui/FeatureEditor.java @@ -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; diff --git a/src/jalview/gui/FeatureSettings.java b/src/jalview/gui/FeatureSettings.java index 36af39a..9165a3d 100644 --- a/src/jalview/gui/FeatureSettings.java +++ b/src/jalview/gui/FeatureSettings.java @@ -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); } diff --git a/src/jalview/gui/ImageExporter.java b/src/jalview/gui/ImageExporter.java index 4c2c8ba..0689220 100644 --- a/src/jalview/gui/ImageExporter.java +++ b/src/jalview/gui/ImageExporter.java @@ -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 */ } diff --git a/src/jalview/gui/JvOptionPane.java b/src/jalview/gui/JvOptionPane.java index 92a57ca..b3df2f7 100644 --- a/src/jalview/gui/JvOptionPane.java +++ b/src/jalview/gui/JvOptionPane.java @@ -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; } diff --git a/src/jalview/gui/LineartOptions.java b/src/jalview/gui/LineartOptions.java index 765a1ac..ccf7edb 100644 --- a/src/jalview/gui/LineartOptions.java +++ b/src/jalview/gui/LineartOptions.java @@ -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); } /** diff --git a/src/jalview/gui/TextColourChooser.java b/src/jalview/gui/TextColourChooser.java index 01d9b81..ae9fda9 100644 --- a/src/jalview/gui/TextColourChooser.java +++ b/src/jalview/gui/TextColourChooser.java @@ -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, diff --git a/src/jalview/gui/UserDefinedColours.java b/src/jalview/gui/UserDefinedColours.java index f1ff128..9e928d8 100755 --- a/src/jalview/gui/UserDefinedColours.java +++ b/src/jalview/gui/UserDefinedColours.java @@ -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); } diff --git a/src/jalview/io/HtmlSvgOutput.java b/src/jalview/io/HtmlSvgOutput.java index a943a81..1f82853 100644 --- a/src/jalview/io/HtmlSvgOutput.java +++ b/src/jalview/io/HtmlSvgOutput.java @@ -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 */ } diff --git a/src/jalview/io/JalviewFileChooser.java b/src/jalview/io/JalviewFileChooser.java index 8e0fe30..6fa6016 100755 --- a/src/jalview/io/JalviewFileChooser.java +++ b/src/jalview/io/JalviewFileChooser.java @@ -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); } /** diff --git a/src/jalview/util/dialogrunner/DialogRunner.java b/src/jalview/util/dialogrunner/DialogRunner.java index 59f3c98..b6fd6a2 100644 --- a/src/jalview/util/dialogrunner/DialogRunner.java +++ b/src/jalview/util/dialogrunner/DialogRunner.java @@ -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 - * the actual dialog that will be shown - which will also initiate the - * response chain. */ public class DialogRunner implements DialogRunnerI { private Map> 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 actions = callbacks.get(action.getTrigger()); + List 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 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()) { diff --git a/src/jalview/util/dialogrunner/DialogRunnerI.java b/src/jalview/util/dialogrunner/DialogRunnerI.java index 6914656..cc4a836 100644 --- a/src/jalview/util/dialogrunner/DialogRunnerI.java +++ b/src/jalview/util/dialogrunner/DialogRunnerI.java @@ -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. - * 
-   * dialog.addResponse(newRunResponse(OK_PRessed) { run()...})
-   *     .addResponse(new RunResponse(CANCEL_PRESSED);
-   *
- * - * @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 diff --git a/src/jalview/util/dialogrunner/RunResponse.java b/src/jalview/util/dialogrunner/RunResponse.java index 7cb7e08..b97525b 100644 --- a/src/jalview/util/dialogrunner/RunResponse.java +++ b/src/jalview/util/dialogrunner/RunResponse.java @@ -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; - } } diff --git a/test/jalview/util/dialogrunner/DialogRunnerTest.java b/test/jalview/util/dialogrunner/DialogRunnerTest.java index 7d7e5e8..b89c4bf 100644 --- a/test/jalview/util/dialogrunner/DialogRunnerTest.java +++ b/test/jalview/util/dialogrunner/DialogRunnerTest.java @@ -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 doDialog(String resp) { - runner.handleResponse(resp); + return runner.handleResponse(resp); } @Override - public void handleResponse(Object response) { - // TODO Auto-generated method stub - + public List 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 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> actions = (Map>) PA.getValue(runner, "callbacks"); + Collection registered = actions.get(action.getTrigger()); + return registered != null && registered.contains(action); } } diff --git a/test/jalview/util/dialogrunner/RunResponseTest.java b/test/jalview/util/dialogrunner/RunResponseTest.java index 4942b32..a8ae22e 100644 --- a/test/jalview/util/dialogrunner/RunResponseTest.java +++ b/test/jalview/util/dialogrunner/RunResponseTest.java @@ -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)); } } -- 1.7.10.2