From: gmungoc Date: Tue, 3 May 2016 09:58:15 +0000 (+0100) Subject: JAL-2068 minimised jalview.gui imports, test for removeAnnotation X-Git-Tag: Release_2_10_0~138^2~7^2~1 X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=f8e603128476ca6e093ea2fc65435d1294978c53;hp=25551bf7f81cd62e7b427f5787f2879eab2afed7;p=jalview.git JAL-2068 minimised jalview.gui imports, test for removeAnnotation --- diff --git a/src/jalview/api/AlignCalcManagerI.java b/src/jalview/api/AlignCalcManagerI.java index b502f86..9c7f9ab 100644 --- a/src/jalview/api/AlignCalcManagerI.java +++ b/src/jalview/api/AlignCalcManagerI.java @@ -35,14 +35,6 @@ public interface AlignCalcManagerI void notifyStart(AlignCalcWorkerI worker); /** - * check if a calculation of this type is already active - * - * @param worker - * @return - */ - boolean alreadyDoing(AlignCalcWorkerI worker); - - /** * tell manager that worker is now processing data * * @param worker @@ -74,6 +66,14 @@ public interface AlignCalcManagerI void enableWorker(AlignCalcWorkerI worker); /** + * Answers true if the worker is disabled from running + * + * @param worker + * @return + */ + boolean isDisabled(AlignCalcWorkerI worker); + + /** * launch a new worker * * @param worker @@ -120,7 +120,7 @@ public interface AlignCalcManagerI * * @param workerClass */ - void updateAnnotationFor(Class workerClass); + void updateAnnotationFor(Class workerClass); /** * return any registered workers of the given class @@ -128,17 +128,8 @@ public interface AlignCalcManagerI * @param workerClass * @return null or one or more workers of the given class */ - List getRegisteredWorkersOfClass(Class workerClass); - - /** - * start any workers of the given class - * - * @param workerClass - * @return false if no workers of given class were registered (note - - * blacklisted classes cannot be restarted, so this method will return - * true for blacklisted workers) - */ - boolean startRegisteredWorkersOfClass(Class workerClass); + List getRegisteredWorkersOfClass( + Class workerClass); /** * work out if there is an instance of a worker that is *waiting* to start @@ -156,12 +147,13 @@ public interface AlignCalcManagerI * * @param typeToRemove */ - void removeRegisteredWorkersOfClass(Class typeToRemove); + void removeRegisteredWorkersOfClass( + Class typeToRemove); /** * Removes the worker that produces the given annotation, provided it is * marked as 'deletable'. Some workers may need to continue to run as the - * results of their calculations are needed elsewhere e.g. for colour schemes. + * results of their calculations are needed, e.g. for colour schemes. * * @param ann */ diff --git a/src/jalview/appletgui/FeatureRenderer.java b/src/jalview/appletgui/FeatureRenderer.java index 4391fa2..7ae333e 100644 --- a/src/jalview/appletgui/FeatureRenderer.java +++ b/src/jalview/appletgui/FeatureRenderer.java @@ -61,8 +61,7 @@ public class FeatureRenderer extends */ public FeatureRenderer(AlignmentViewport av) { - super(); - this.av = av; + super(av); setTransparencyAvailable(!System.getProperty("java.version") .startsWith("1.1")); diff --git a/src/jalview/gui/FeatureRenderer.java b/src/jalview/gui/FeatureRenderer.java index 1cf15ac..61e99ac 100644 --- a/src/jalview/gui/FeatureRenderer.java +++ b/src/jalview/gui/FeatureRenderer.java @@ -73,9 +73,8 @@ public class FeatureRenderer extends */ public FeatureRenderer(AlignmentPanel ap) { - super(); + super(ap.av); this.ap = ap; - this.av = ap.av; if (ap != null && ap.getSeqPanel() != null && ap.getSeqPanel().seqCanvas != null && ap.getSeqPanel().seqCanvas.fr != null) diff --git a/src/jalview/renderer/seqfeatures/FeatureRenderer.java b/src/jalview/renderer/seqfeatures/FeatureRenderer.java index 2276913..8c248d2 100644 --- a/src/jalview/renderer/seqfeatures/FeatureRenderer.java +++ b/src/jalview/renderer/seqfeatures/FeatureRenderer.java @@ -20,6 +20,7 @@ */ package jalview.renderer.seqfeatures; +import jalview.api.AlignViewportI; import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; import jalview.viewmodel.seqfeatures.FeatureRendererModel; @@ -50,6 +51,16 @@ public class FeatureRenderer extends FeatureRendererModel boolean av_validCharWidth, av_isShowSeqFeatureHeight; + /** + * Constructor given a viewport + * + * @param viewport + */ + public FeatureRenderer(AlignViewportI viewport) + { + this.av = viewport; + } + protected void updateAvConfig() { av_charHeight = av.getCharHeight(); diff --git a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java index 848f565..0d317e6 100644 --- a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java +++ b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java @@ -28,7 +28,6 @@ import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; import jalview.renderer.seqfeatures.FeatureRenderer; import jalview.schemes.GraduatedColor; -import jalview.viewmodel.AlignmentViewport; import java.awt.Color; import java.beans.PropertyChangeListener; @@ -66,7 +65,7 @@ public abstract class FeatureRendererModel implements protected PropertyChangeSupport changeSupport = new PropertyChangeSupport( this); - protected AlignmentViewport av; + protected AlignViewportI av; /* * map holds per feature type, {{min, max}, {min, max}} feature score diff --git a/src/jalview/workers/AlignCalcManager.java b/src/jalview/workers/AlignCalcManager.java index 9ec6a1c..191e8c7 100644 --- a/src/jalview/workers/AlignCalcManager.java +++ b/src/jalview/workers/AlignCalcManager.java @@ -34,23 +34,48 @@ import java.util.Set; public class AlignCalcManager implements AlignCalcManagerI { - private volatile List restartable = Collections - .synchronizedList(new ArrayList()); + /* + * list of registered workers + */ + private volatile List restartable; - private volatile List blackList = Collections - .synchronizedList(new ArrayList()); + /* + * types of worker _not_ to run (for example, because they have + * previously thrown errors) + */ + private volatile List> blackList; - /** + /* * global record of calculations in progress */ - private volatile Map inProgress = Collections - .synchronizedMap(new Hashtable()); + private volatile List inProgress; - /** + /* * record of calculations pending or in progress in the current context */ - private volatile Map> updating = Collections - .synchronizedMap(new Hashtable>()); + private volatile Map, List> updating; + + /* + * workers that have run to completion so are candidates for visual-only + * update of their results + */ + private HashSet canUpdate; + + /** + * Constructor + */ + public AlignCalcManager() + { + restartable = Collections + .synchronizedList(new ArrayList()); + blackList = Collections + .synchronizedList(new ArrayList>()); + inProgress = Collections + .synchronizedList(new ArrayList()); + updating = Collections + .synchronizedMap(new Hashtable, List>()); + canUpdate = new HashSet(); + } @Override public void notifyStart(AlignCalcWorkerI worker) @@ -72,15 +97,6 @@ public class AlignCalcManager implements AlignCalcManagerI } } - @Override - public boolean alreadyDoing(AlignCalcWorkerI worker) - { - synchronized (inProgress) - { - return inProgress.containsKey(worker.getClass()); - } - } - /* * (non-Javadoc) * @@ -108,52 +124,31 @@ public class AlignCalcManager implements AlignCalcManagerI } } - // TODO make into api method if needed ? - public int numberLive(AlignCalcWorkerI worker) - { - synchronized (updating) - { - List upd = updating.get(worker.getClass()); - if (upd == null) - { - return 0; - } - ; - return upd.size(); - } - } - @Override public boolean notifyWorking(AlignCalcWorkerI worker) { synchronized (inProgress) { - // TODO: decide if we should throw exceptions here if multiple workers - // start to work - if (inProgress.get(worker.getClass()) != null) + if (inProgress.contains(worker)) { - if (false) - { - System.err - .println("Warning: Multiple workers are running of type " - + worker.getClass()); - } - return false; + System.err.println("Implementation error: duplicate run of worker " + + worker); + } + else + { + inProgress.add(worker); } - inProgress.put(worker.getClass(), worker); } return true; } - private final HashSet canUpdate = new HashSet(); - @Override public void workerComplete(AlignCalcWorkerI worker) { synchronized (inProgress) { - // System.err.println("Worker "+worker.getClass()+" marked as complete."); - inProgress.remove(worker.getClass()); + // System.err.println("Worker " + worker + " marked as complete."); + inProgress.remove(worker); List upd = updating.get(worker.getClass()); if (upd != null) { @@ -171,22 +166,23 @@ public class AlignCalcManager implements AlignCalcManagerI { synchronized (blackList) { - blackList.add(worker); + blackList.add(worker.getClass()); } } - public boolean isBlackListed(AlignCalcWorkerI worker) + @Override + public boolean isDisabled(AlignCalcWorkerI worker) { synchronized (blackList) { - return blackList.contains(worker); + return blackList.contains(worker.getClass()); } } @Override public void startWorker(AlignCalcWorkerI worker) { - if (!isBlackListed(worker)) + if (!isDisabled(worker)) { Thread tw = new Thread(worker); tw.setName(worker.getClass().toString()); @@ -200,7 +196,7 @@ public class AlignCalcManager implements AlignCalcManagerI synchronized (inProgress) {// System.err.println("isWorking : worker "+(worker!=null ? // worker.getClass():"null")+ " "+hashCode()); - return worker != null && inProgress.get(worker.getClass()) == worker; + return worker != null && inProgress.contains(worker); } } @@ -244,7 +240,7 @@ public class AlignCalcManager implements AlignCalcManagerI { synchronized (inProgress) { - for (AlignCalcWorkerI worker : inProgress.values()) + for (AlignCalcWorkerI worker : inProgress) { if (worker.involves(alignmentAnnotation)) { @@ -269,7 +265,8 @@ public class AlignCalcManager implements AlignCalcManagerI } @Override - public void updateAnnotationFor(Class workerClass) + public void updateAnnotationFor( + Class workerClass) { AlignCalcWorkerI[] workers; @@ -288,59 +285,35 @@ public class AlignCalcManager implements AlignCalcManagerI @Override public List getRegisteredWorkersOfClass( - Class workerClass) + Class workerClass) { List workingClass = new ArrayList(); - AlignCalcWorkerI[] workers; synchronized (canUpdate) { - workers = canUpdate.toArray(new AlignCalcWorkerI[canUpdate.size()]); - } - for (AlignCalcWorkerI worker : workers) - { - if (workerClass.equals(worker.getClass())) + for (AlignCalcWorkerI worker : canUpdate) { - workingClass.add(worker); + if (workerClass.equals(worker.getClass())) + { + workingClass.add(worker); + } } } return (workingClass.size() == 0) ? null : workingClass; } @Override - public boolean startRegisteredWorkersOfClass(Class workerClass) - { - List workers = getRegisteredWorkersOfClass(workerClass); - if (workers == null) - { - return false; - } - for (AlignCalcWorkerI worker : workers) - { - if (!isPending(worker)) - { - startWorker(worker); - } - else - { - System.err.println("Pending exists for " + workerClass); - } - } - return true; - } - - @Override public void enableWorker(AlignCalcWorkerI worker) { synchronized (blackList) { - blackList.remove(worker); + blackList.remove(worker.getClass()); } } @Override - public void removeRegisteredWorkersOfClass(Class typeToRemove) + public void removeRegisteredWorkersOfClass( + Class typeToRemove) { - List workers = getRegisteredWorkersOfClass(typeToRemove); List removable = new ArrayList(); Set toremovannot = new HashSet(); synchronized (restartable) @@ -405,12 +378,23 @@ public class AlignCalcManager implements AlignCalcManagerI } /* - * remove all references to the workers + * remove all references to deleted workers so any references + * they hold to annotation data can be garbage collected */ for (AlignCalcWorkerI worker : toRemove) { restartable.remove(worker); - blackList.remove(worker); + blackList.remove(worker.getClass()); + inProgress.remove(worker); + canUpdate.remove(worker); + synchronized (updating) + { + List upd = updating.get(worker.getClass()); + if (upd != null) + { + upd.remove(worker); + } + } } } } diff --git a/src/jalview/workers/AlignmentAnnotationFactory.java b/src/jalview/workers/AlignmentAnnotationFactory.java index 37f3ca5..ee48c73 100644 --- a/src/jalview/workers/AlignmentAnnotationFactory.java +++ b/src/jalview/workers/AlignmentAnnotationFactory.java @@ -1,5 +1,7 @@ package jalview.workers; +import jalview.api.AlignViewportI; +import jalview.api.AlignmentViewPanel; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.Annotation; import jalview.gui.AlignFrame; @@ -12,9 +14,9 @@ import java.awt.Color; * such as Groovy) to 'register and forget' an alignment annotation calculator.
* Currently supports two flavours of calculator: *
    - *
  • a 'feature counter' which can count any desired property derivable from + *
  • a simple 'feature counter' which counts any desired score derivable from * residue value and any sequence features at each position of the alignment
    • - *
  • a 'general purpose' calculator which computes one more complete + *
  • a 'general purpose' calculator which computes one or more complete * AlignmentAnnotation objects
    • *
*/ @@ -28,9 +30,13 @@ public class AlignmentAnnotationFactory */ public static void newCalculator(FeatureCounterI counter) { - if (Desktop.getCurrentAlignFrame() != null) + // TODO need an interface for AlignFrame by which to access + // its AlignViewportI and AlignmentViewPanel + AlignFrame currentAlignFrame = Desktop.getCurrentAlignFrame(); + if (currentAlignFrame != null) { - newCalculator(Desktop.getCurrentAlignFrame(), counter); + newCalculator(currentAlignFrame.getViewport(), currentAlignFrame + .getAlignPanels().get(0), counter); } else { @@ -42,14 +48,15 @@ public class AlignmentAnnotationFactory /** * Constructs and registers a new alignment annotation worker * - * @param af - * the AlignFrame for which the annotation is to be calculated + * @param viewport + * @param panel * @param counter * provider of feature counts per alignment position */ - public static void newCalculator(AlignFrame af, FeatureCounterI counter) + public static void newCalculator(AlignViewportI viewport, + AlignmentViewPanel panel, FeatureCounterI counter) { - new ColumnCounterWorker(af, counter); + new ColumnCounterWorker(viewport, panel, counter); } /** @@ -60,9 +67,13 @@ public class AlignmentAnnotationFactory */ public static void newCalculator(AnnotationProviderI calculator) { - if (Desktop.getCurrentAlignFrame() != null) + // TODO need an interface for AlignFrame by which to access + // its AlignViewportI and AlignmentViewPanel + AlignFrame currentAlignFrame = Desktop.getCurrentAlignFrame(); + if (currentAlignFrame != null) { - newCalculator(Desktop.getCurrentAlignFrame(), calculator); + newCalculator(currentAlignFrame.getViewport(), currentAlignFrame + .getAlignPanels().get(0), calculator); } else { @@ -74,15 +85,16 @@ public class AlignmentAnnotationFactory /** * Constructs and registers a new alignment annotation worker * - * @param af - * the AlignFrame for which the annotation is to be calculated + * @param viewport + * @param panel * @param calculator * provider of AlignmentAnnotation for the alignment */ - public static void newCalculator(AlignFrame af, + public static void newCalculator(AlignViewportI viewport, + AlignmentViewPanel panel, AnnotationProviderI calculator) { - new AnnotationWorker(af, calculator); + new AnnotationWorker(viewport, panel, calculator); } /** diff --git a/src/jalview/workers/AnnotationProviderI.java b/src/jalview/workers/AnnotationProviderI.java index 653ff04..bd24461 100644 --- a/src/jalview/workers/AnnotationProviderI.java +++ b/src/jalview/workers/AnnotationProviderI.java @@ -1,8 +1,8 @@ package jalview.workers; +import jalview.api.FeatureRenderer; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.AlignmentI; -import jalview.gui.FeatureRenderer; import java.util.List; diff --git a/src/jalview/workers/AnnotationWorker.java b/src/jalview/workers/AnnotationWorker.java index 901b6fc..efe707a 100644 --- a/src/jalview/workers/AnnotationWorker.java +++ b/src/jalview/workers/AnnotationWorker.java @@ -20,11 +20,11 @@ */ package jalview.workers; +import jalview.api.AlignViewportI; +import jalview.api.AlignmentViewPanel; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.AlignmentI; -import jalview.gui.AlignFrame; -import jalview.gui.AlignmentPanel; -import jalview.gui.FeatureRenderer; +import jalview.renderer.seqfeatures.FeatureRenderer; import java.util.ArrayList; import java.util.List; @@ -47,9 +47,10 @@ class AnnotationWorker extends AlignCalcWorker * @param af * @param counter */ - public AnnotationWorker(AlignFrame af, AnnotationProviderI counter) + public AnnotationWorker(AlignViewportI viewport, + AlignmentViewPanel panel, AnnotationProviderI counter) { - super(af.getViewport(), af.alignPanel); + super(viewport, panel); ourAnnots = new ArrayList(); this.counter = counter; calcMan.registerWorker(this); @@ -85,7 +86,7 @@ class AnnotationWorker extends AlignCalcWorker try { List anns = counter.calculateAnnotation( - alignment, new FeatureRenderer((AlignmentPanel) ap)); + alignment, new FeatureRenderer(alignViewport)); for (AlignmentAnnotation ann : anns) { ann.showAllColLabels = true; diff --git a/src/jalview/workers/ColumnCounterWorker.java b/src/jalview/workers/ColumnCounterWorker.java index 69f4265..947b3c7 100644 --- a/src/jalview/workers/ColumnCounterWorker.java +++ b/src/jalview/workers/ColumnCounterWorker.java @@ -20,14 +20,14 @@ */ package jalview.workers; +import jalview.api.AlignViewportI; +import jalview.api.AlignmentViewPanel; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.AlignmentI; import jalview.datamodel.Annotation; import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; -import jalview.gui.AlignFrame; -import jalview.gui.AlignmentPanel; -import jalview.gui.FeatureRenderer; +import jalview.renderer.seqfeatures.FeatureRenderer; import jalview.util.ColorUtils; import jalview.util.Comparison; @@ -53,9 +53,10 @@ class ColumnCounterWorker extends AlignCalcWorker * @param af * @param counter */ - public ColumnCounterWorker(AlignFrame af, FeatureCounterI counter) + public ColumnCounterWorker(AlignViewportI viewport, + AlignmentViewPanel panel, FeatureCounterI counter) { - super(af.getViewport(), af.alignPanel); + super(viewport, panel); ourAnnots = new ArrayList(); this.counter = counter; calcMan.registerWorker(this); @@ -123,7 +124,7 @@ class ColumnCounterWorker extends AlignCalcWorker */ void computeAnnotations() { - FeatureRenderer fr = new FeatureRenderer((AlignmentPanel) ap); + FeatureRenderer fr = new FeatureRenderer(alignViewport); // TODO use the commented out code once JAL-2075 is fixed // to get adequate performance on genomic length sequence AlignmentI alignment = alignViewport.getAlignment(); diff --git a/test/jalview/workers/AlignCalcManagerTest.java b/test/jalview/workers/AlignCalcManagerTest.java new file mode 100644 index 0000000..735c75d --- /dev/null +++ b/test/jalview/workers/AlignCalcManagerTest.java @@ -0,0 +1,147 @@ +package jalview.workers; + +import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertFalse; +import static org.testng.AssertJUnit.assertTrue; + +import jalview.api.AlignCalcManagerI; +import jalview.api.AlignCalcWorkerI; +import jalview.api.FeatureRenderer; +import jalview.datamodel.Alignment; +import jalview.datamodel.AlignmentAnnotation; +import jalview.datamodel.AlignmentI; +import jalview.datamodel.Annotation; +import jalview.datamodel.Sequence; +import jalview.datamodel.SequenceI; +import jalview.gui.AlignFrame; + +import java.util.Collections; +import java.util.List; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +public class AlignCalcManagerTest +{ + private AlignFrame alignFrame; + + /** + * Test the method that removes a worker associated with an annotation, + * provided the worker is marked as 'deletable' (some workers should continue + * to run even when their results are no longer displayed) + */ + @Test(groups = "Functional") + public void testRemoveWorkerForAnnotation() + { + AlignCalcManagerI acm = alignFrame.getViewport().getCalcManager(); + final AlignmentAnnotation ann1 = new AlignmentAnnotation("Ann1", + "desc", + new Annotation[] {}); + final AlignmentAnnotation ann2 = new AlignmentAnnotation("Ann2", + "desc", + new Annotation[] {}); + + /* + * make two workers for ann1, one deletable, one not + * and no worker for ann2 + */ + AlignCalcWorkerI worker1 = makeWorker(ann1, true); + AlignCalcWorkerI worker2 = makeWorker(ann1, false); + + /* + * The new workers will get run each in their own thread. + * We can't tell here whether they have finished, or not yet started. + * They have to finish to be 'seen' by getRegisteredWorkersOfClass() + * registerWorker adds to the 'restartable' list but + * getRegisteredWorkers reads from the 'canUpdate' list + * (which is only updated after a worker has run) - why? + * So just give workers time to start and finish + */ + synchronized (this) + { + try + { + wait(100); + } catch (InterruptedException e) + { + // + } + } + + List workers = acm.getRegisteredWorkersOfClass(worker1.getClass()); + assertEquals(2, workers.size()); + assertTrue(workers.contains(worker1)); + assertTrue(workers.contains(worker2)); + assertFalse(acm.isDisabled(worker1)); + assertFalse(acm.isDisabled(worker2)); + + /* + * remove workers for ann2 (there aren't any) + */ + acm.removeWorkerForAnnotation(ann2); + assertTrue(acm.getRegisteredWorkersOfClass(worker1.getClass()) + .contains(worker1)); + assertTrue(acm.getRegisteredWorkersOfClass(worker1.getClass()) + .contains(worker2)); + assertFalse(acm.isDisabled(worker1)); + assertFalse(acm.isDisabled(worker2)); + + /* + * remove worker2 for ann1 + * - should delete worker1 but not worker2 + */ + acm.removeWorkerForAnnotation(ann1); + assertEquals(1, acm.getRegisteredWorkersOfClass(worker1.getClass()) + .size()); + assertTrue(acm.getRegisteredWorkersOfClass(worker1.getClass()) + .contains(worker2)); + assertFalse(acm.isDisabled(worker1)); + assertFalse(acm.isDisabled(worker2)); + } + + /** + * Make a worker linked to the given annotation + * + * @param ann + * @param deletable + * @return + */ + AnnotationWorker makeWorker(final AlignmentAnnotation ann, + final boolean deletable) + { + AnnotationProviderI annotationProvider = new AnnotationProviderI() + { + @Override + public List calculateAnnotation(AlignmentI al, + FeatureRenderer fr) + { + return Collections.singletonList(ann); + } + }; + return new AnnotationWorker(alignFrame.getViewport(), + alignFrame.alignPanel, + annotationProvider) + { + @Override + public boolean isDeletable() + { + return deletable; + } + + @Override + public boolean involves(AlignmentAnnotation ann1) + { + return ann == ann1; + } + }; + } + + @BeforeMethod(alwaysRun = true) + public void setUp() + { + AlignmentI al = new Alignment(new SequenceI[] { new Sequence("Seq1", + "ABC") }); + al.setDataset(null); + alignFrame = new AlignFrame(al, 3, 1); + } +}