Merge branch 'features/JAL-2068groovyAnnotationWorker' into develop
authorJim Procter <jprocter@issues.jalview.org>
Mon, 18 Jul 2016 13:01:49 +0000 (14:01 +0100)
committerJim Procter <jprocter@issues.jalview.org>
Mon, 18 Jul 2016 13:01:49 +0000 (14:01 +0100)
minor conflicts in constructor/imports for unification of Graduated/FeatureColour

19 files changed:
src/jalview/api/AlignCalcManagerI.java
src/jalview/api/AlignCalcWorkerI.java
src/jalview/appletgui/FeatureRenderer.java
src/jalview/gui/AnnotationLabels.java
src/jalview/gui/FeatureRenderer.java
src/jalview/renderer/seqfeatures/FeatureRenderer.java
src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
src/jalview/workers/AlignCalcManager.java
src/jalview/workers/AlignCalcWorker.java
src/jalview/workers/AlignmentAnnotationFactory.java
src/jalview/workers/AnnotationProviderI.java
src/jalview/workers/AnnotationWorker.java
src/jalview/workers/ColumnCounterWorker.java
src/jalview/workers/ConsensusThread.java
src/jalview/workers/ConservationThread.java
src/jalview/workers/StrucConsensusThread.java
src/jalview/ws/jws2/AbstractJabaCalcWorker.java
src/jalview/ws/jws2/SequenceAnnotationWSClient.java
test/jalview/workers/AlignCalcManagerTest.java [new file with mode: 0644]

index 66f4036..9c7f9ab 100644 (file)
@@ -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
@@ -63,7 +55,7 @@ public interface AlignCalcManagerI
    * 
    * @param worker
    */
-  void workerCannotRun(AlignCalcWorkerI worker);
+  void disableWorker(AlignCalcWorkerI worker);
 
   /**
    * indicate that a worker like this may be run on the platform.
@@ -71,7 +63,15 @@ public interface AlignCalcManagerI
    * @param worker
    *          of class to be removed from the execution blacklist
    */
-  void workerMayRun(AlignCalcWorkerI worker);
+  void enableWorker(AlignCalcWorkerI worker);
+
+  /**
+   * Answers true if the worker is disabled from running
+   * 
+   * @param worker
+   * @return
+   */
+  boolean isDisabled(AlignCalcWorkerI worker);
 
   /**
    * launch a new worker
@@ -120,7 +120,7 @@ public interface AlignCalcManagerI
    * 
    * @param workerClass
    */
-  void updateAnnotationFor(Class workerClass);
+  void updateAnnotationFor(Class<? extends AlignCalcWorkerI> 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<AlignCalcWorkerI> 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<AlignCalcWorkerI> getRegisteredWorkersOfClass(
+          Class<? extends AlignCalcWorkerI> workerClass);
 
   /**
    * work out if there is an instance of a worker that is *waiting* to start
@@ -156,6 +147,15 @@ public interface AlignCalcManagerI
    * 
    * @param typeToRemove
    */
-  void removeRegisteredWorkersOfClass(Class typeToRemove);
+  void removeRegisteredWorkersOfClass(
+          Class<? extends AlignCalcWorkerI> 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, e.g. for colour schemes.
+   * 
+   * @param ann
+   */
+  void removeWorkerForAnnotation(AlignmentAnnotation ann);
 }
index 06dc054..85157c4 100644 (file)
@@ -35,17 +35,26 @@ public interface AlignCalcWorkerI extends Runnable
    * @param annot
    * @return
    */
-  public boolean involves(AlignmentAnnotation annot);
+  boolean involves(AlignmentAnnotation annot);
 
   /**
    * Updates the display of calculated annotation values (does not recalculate
-   * the values). This allows for quick redraw of annotations when display
-   * settings are changed.
+   * the values). This allows ßquick redraw of annotations when display settings
+   * are changed.
    */
-  public void updateAnnotation();
+  void updateAnnotation();
 
   /**
-   * Removes any annotation managed by this worker from the alignment
+   * Removes any annotation(s) managed by this worker from the alignment
    */
   void removeAnnotation();
+
+  /**
+   * Answers true if the worker should be deleted entirely when its annotation
+   * is deleted from the display, or false if it should continue to run. Some
+   * workers are required to run for their side-effects.
+   * 
+   * @return
+   */
+  boolean isDeletable();
 }
index 3f87549..81d8ef5 100644 (file)
@@ -70,8 +70,8 @@ public class FeatureRenderer extends
    */
   public FeatureRenderer(AlignmentViewport av)
   {
-    super();
-    this.av = av;
+    super(av);
+
   }
 
   static String lastFeatureAdded;
index e27b919..5b01b9b 100755 (executable)
@@ -244,6 +244,7 @@ public class AnnotationLabels extends JPanel implements MouseListener,
     else if (evt.getActionCommand().equals(DELETE))
     {
       ap.av.getAlignment().deleteAnnotation(aa[selectedRow]);
+      ap.av.getCalcManager().removeWorkerForAnnotation(aa[selectedRow]);
     }
     else if (evt.getActionCommand().equals(SHOWALL))
     {
index 1bf7453..727c13b 100644 (file)
@@ -75,9 +75,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)
index 5d3aa2d..b007365 100644 (file)
@@ -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;
@@ -52,6 +53,16 @@ public class FeatureRenderer extends FeatureRendererModel
 
   private Integer currentColour;
 
+  /**
+   * Constructor given a viewport
+   * 
+   * @param viewport
+   */
+  public FeatureRenderer(AlignViewportI viewport)
+  {
+    this.av = viewport;
+  }
+
   protected void updateAvConfig()
   {
     av_charHeight = av.getCharHeight();
index a4e4348..d813fe2 100644 (file)
@@ -29,7 +29,6 @@ import jalview.datamodel.SequenceI;
 import jalview.renderer.seqfeatures.FeatureRenderer;
 import jalview.schemes.FeatureColour;
 import jalview.schemes.UserColourScheme;
-import jalview.viewmodel.AlignmentViewport;
 
 import java.awt.Color;
 import java.beans.PropertyChangeListener;
@@ -64,7 +63,7 @@ public abstract class FeatureRendererModel implements
   protected PropertyChangeSupport changeSupport = new PropertyChangeSupport(
           this);
 
-  protected AlignmentViewport av;
+  protected AlignViewportI av;
 
   @Override
   public AlignViewportI getViewport()
index 1063706..191e8c7 100644 (file)
@@ -34,23 +34,48 @@ import java.util.Set;
 
 public class AlignCalcManager implements AlignCalcManagerI
 {
-  private volatile List<AlignCalcWorkerI> restartable = Collections
-          .synchronizedList(new ArrayList<AlignCalcWorkerI>());
+  /*
+   * list of registered workers
+   */
+  private volatile List<AlignCalcWorkerI> restartable;
 
-  private volatile List<Class> blackList = Collections
-          .synchronizedList(new ArrayList<Class>());
+  /*
+   * types of worker _not_ to run (for example, because they have
+   * previously thrown errors)
+   */
+  private volatile List<Class<? extends AlignCalcWorkerI>> blackList;
 
-  /**
+  /*
    * global record of calculations in progress
    */
-  private volatile Map<Class, AlignCalcWorkerI> inProgress = Collections
-          .synchronizedMap(new Hashtable<Class, AlignCalcWorkerI>());
+  private volatile List<AlignCalcWorkerI> inProgress;
 
-  /**
+  /*
    * record of calculations pending or in progress in the current context
    */
-  private volatile Map<Class, List<AlignCalcWorkerI>> updating = Collections
-          .synchronizedMap(new Hashtable<Class, List<AlignCalcWorkerI>>());
+  private volatile Map<Class<? extends AlignCalcWorkerI>, List<AlignCalcWorkerI>> updating;
+
+  /*
+   * workers that have run to completion so are candidates for visual-only 
+   * update of their results
+   */
+  private HashSet<AlignCalcWorkerI> canUpdate;
+
+  /**
+   * Constructor
+   */
+  public AlignCalcManager()
+  {
+    restartable = Collections
+            .synchronizedList(new ArrayList<AlignCalcWorkerI>());
+    blackList = Collections
+            .synchronizedList(new ArrayList<Class<? extends AlignCalcWorkerI>>());
+    inProgress = Collections
+            .synchronizedList(new ArrayList<AlignCalcWorkerI>());
+    updating = Collections
+            .synchronizedMap(new Hashtable<Class<? extends AlignCalcWorkerI>, List<AlignCalcWorkerI>>());
+    canUpdate = new HashSet<AlignCalcWorkerI>();
+  }
 
   @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<AlignCalcWorkerI> 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<AlignCalcWorkerI> canUpdate = new HashSet<AlignCalcWorkerI>();
-
   @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<AlignCalcWorkerI> upd = updating.get(worker.getClass());
       if (upd != null)
       {
@@ -167,7 +162,7 @@ public class AlignCalcManager implements AlignCalcManagerI
   }
 
   @Override
-  public void workerCannotRun(AlignCalcWorkerI worker)
+  public void disableWorker(AlignCalcWorkerI worker)
   {
     synchronized (blackList)
     {
@@ -175,22 +170,24 @@ public class AlignCalcManager implements AlignCalcManagerI
     }
   }
 
-  public boolean isBlackListed(Class workerType)
+  @Override
+  public boolean isDisabled(AlignCalcWorkerI worker)
   {
     synchronized (blackList)
     {
-      return blackList.contains(workerType);
+      return blackList.contains(worker.getClass());
     }
   }
 
   @Override
   public void startWorker(AlignCalcWorkerI worker)
   {
-    // System.err.println("Starting "+worker.getClass());
-    // new Exception("").printStackTrace();
-    Thread tw = new Thread(worker);
-    tw.setName(worker.getClass().toString());
-    tw.start();
+    if (!isDisabled(worker))
+    {
+      Thread tw = new Thread(worker);
+      tw.setName(worker.getClass().toString());
+      tw.start();
+    }
   }
 
   @Override
@@ -199,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);
     }
   }
 
@@ -243,7 +240,7 @@ public class AlignCalcManager implements AlignCalcManagerI
   {
     synchronized (inProgress)
     {
-      for (AlignCalcWorkerI worker : inProgress.values())
+      for (AlignCalcWorkerI worker : inProgress)
       {
         if (worker.involves(alignmentAnnotation))
         {
@@ -268,7 +265,8 @@ public class AlignCalcManager implements AlignCalcManagerI
   }
 
   @Override
-  public void updateAnnotationFor(Class workerClass)
+  public void updateAnnotationFor(
+          Class<? extends AlignCalcWorkerI> workerClass)
   {
 
     AlignCalcWorkerI[] workers;
@@ -287,62 +285,35 @@ public class AlignCalcManager implements AlignCalcManagerI
 
   @Override
   public List<AlignCalcWorkerI> getRegisteredWorkersOfClass(
-          Class workerClass)
+          Class<? extends AlignCalcWorkerI> workerClass)
   {
     List<AlignCalcWorkerI> workingClass = new ArrayList<AlignCalcWorkerI>();
-    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<AlignCalcWorkerI> 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 workerMayRun(AlignCalcWorkerI worker)
+  public void enableWorker(AlignCalcWorkerI worker)
   {
     synchronized (blackList)
     {
-      if (blackList.contains(worker.getClass()))
-      {
-        blackList.remove(worker.getClass());
-      }
+      blackList.remove(worker.getClass());
     }
   }
 
   @Override
-  public void removeRegisteredWorkersOfClass(Class typeToRemove)
+  public void removeRegisteredWorkersOfClass(
+          Class<? extends AlignCalcWorkerI> typeToRemove)
   {
-    List<AlignCalcWorkerI> workers = getRegisteredWorkersOfClass(typeToRemove);
     List<AlignCalcWorkerI> removable = new ArrayList<AlignCalcWorkerI>();
     Set<AlignCalcWorkerI> toremovannot = new HashSet<AlignCalcWorkerI>();
     synchronized (restartable)
@@ -382,4 +353,48 @@ public class AlignCalcManager implements AlignCalcManagerI
      * else { System.err.println("Pending exists for " + workerClass); } }
      */
   }
+
+  /**
+   * Deletes the worker that update the given annotation, provided it is marked
+   * as deletable.
+   */
+  @Override
+  public void removeWorkerForAnnotation(AlignmentAnnotation ann)
+  {
+    /*
+     * first just find those to remove (to avoid
+     * ConcurrentModificationException)
+     */
+    List<AlignCalcWorkerI> toRemove = new ArrayList<AlignCalcWorkerI>();
+    for (AlignCalcWorkerI worker : restartable)
+    {
+      if (worker.involves(ann))
+      {
+        if (worker.isDeletable())
+        {
+          toRemove.add(worker);
+        }
+      }
+    }
+
+    /*
+     * 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.getClass());
+      inProgress.remove(worker);
+      canUpdate.remove(worker);
+      synchronized (updating)
+      {
+        List<AlignCalcWorkerI> upd = updating.get(worker.getClass());
+        if (upd != null)
+        {
+          upd.remove(worker);
+        }
+      }
+    }
+  }
 }
index 48e3604..7719c88 100644 (file)
@@ -97,4 +97,10 @@ public abstract class AlignCalcWorker implements AlignCalcWorkerI
   // TODO: allow GUI to query workers associated with annotation to add items to
   // annotation label panel popup menu
 
+  @Override
+  public boolean isDeletable()
+  {
+    return false;
+  }
+
 }
index 520b232..f804a19 100644 (file)
@@ -1,5 +1,7 @@
 package jalview.workers;
 
+import jalview.api.AlignViewportI;
+import jalview.api.AlignmentViewPanel;
 import jalview.bin.Jalview;
 import jalview.datamodel.AlignmentAnnotation;
 import jalview.datamodel.Annotation;
@@ -12,9 +14,9 @@ import java.awt.Color;
  * such as Groovy) to 'register and forget' an alignment annotation calculator. <br>
  * Currently supports two flavours of calculator:
  * <ul>
- * <li>a 'feature counter' which can count any desired property derivable from
+ * <li>a simple 'feature counter' which counts any desired score derivable from
  * residue value and any sequence features at each position of the alignment</li>
- * <li>a 'general purpose' calculator which computes one more complete
+ * <li>a 'general purpose' calculator which computes one or more complete
  * AlignmentAnnotation objects</li>
  * </ul>
  */
@@ -28,9 +30,13 @@ public class AlignmentAnnotationFactory
    */
   public static void newCalculator(FeatureCounterI counter)
   {
-    if (Jalview.getCurrentAlignFrame() != null)
+    // TODO need an interface for AlignFrame by which to access
+    // its AlignViewportI and AlignmentViewPanel
+    AlignFrame currentAlignFrame = Jalview.getCurrentAlignFrame() ;
+    if (currentAlignFrame != null)
     {
-      newCalculator(Jalview.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 (Jalview.getCurrentAlignFrame() != null)
+    // TODO need an interface for AlignFrame by which to access
+    // its AlignViewportI and AlignmentViewPanel
+    AlignFrame currentAlignFrame = Jalview.getCurrentAlignFrame() ;
+    if (currentAlignFrame != null)
     {
-      newCalculator(Jalview.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);
   }
 
   /**
index 653ff04..bd24461 100644 (file)
@@ -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;
 
index fbf7531..efe707a 100644 (file)
  */
 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;
 
 /**
  * A class to create and update one or more alignment annotations, given a
- * 'calculator'.
- * 
+ * 'calculator'. Intended to support a 'plug-in' annotation worker which
+ * implements the AnnotationProviderI interface.
  */
 class AnnotationWorker extends AlignCalcWorker
 {
@@ -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<AlignmentAnnotation>();
     this.counter = counter;
     calcMan.registerWorker(this);
@@ -85,7 +86,7 @@ class AnnotationWorker extends AlignCalcWorker
         try
         {
           List<AlignmentAnnotation> anns = counter.calculateAnnotation(
-                  alignment, new FeatureRenderer((AlignmentPanel) ap));
+                  alignment, new FeatureRenderer(alignViewport));
           for (AlignmentAnnotation ann : anns)
           {
             ann.showAllColLabels = true;
@@ -102,7 +103,7 @@ class AnnotationWorker extends AlignCalcWorker
     } catch (OutOfMemoryError error)
     {
       ap.raiseOOMWarning("calculating annotations", error);
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
     } finally
     {
       calcMan.workerComplete(this);
@@ -133,4 +134,15 @@ class AnnotationWorker extends AlignCalcWorker
   {
     // do nothing
   }
+
+  /**
+   * Answers true to indicate that if this worker's annotation is deleted from
+   * the display, the worker should also be removed. This prevents it running
+   * and recreating the annotation when the alignment changes.
+   */
+  @Override
+  public boolean isDeletable()
+  {
+    return true;
+  }
 }
index 6f4a4f3..947b3c7 100644 (file)
  */
 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<AlignmentAnnotation>();
     this.counter = counter;
     calcMan.registerWorker(this);
@@ -103,7 +104,7 @@ class ColumnCounterWorker extends AlignCalcWorker
     } catch (OutOfMemoryError error)
     {
       ap.raiseOOMWarning("calculating feature counts", error);
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
     } finally
     {
       calcMan.workerComplete(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();
@@ -222,4 +223,15 @@ class ColumnCounterWorker extends AlignCalcWorker
   {
     // do nothing
   }
+
+  /**
+   * Answers true to indicate that if this worker's annotation is deleted from
+   * the display, the worker should also be removed. This prevents it running
+   * and recreating the annotation when the alignment changes.
+   */
+  @Override
+  public boolean isDeletable()
+  {
+    return true;
+  }
 }
index 14e2a31..5f0ec84 100644 (file)
@@ -96,7 +96,7 @@ public class ConsensusThread extends AlignCalcWorker
       }
     } catch (OutOfMemoryError error)
     {
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
       ap.raiseOOMWarning("calculating consensus", error);
     } finally
     {
index 1075e4d..5c303fd 100644 (file)
@@ -108,7 +108,7 @@ public class ConservationThread extends AlignCalcWorker
     } catch (OutOfMemoryError error)
     {
       ap.raiseOOMWarning("calculating conservation", error);
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
       // alignViewport.conservation = null;
       // this.alignViewport.quality = null;
 
index 4249112..5ed2885 100644 (file)
@@ -129,7 +129,7 @@ public class StrucConsensusThread extends AlignCalcWorker
       updateResultAnnotation(true);
     } catch (OutOfMemoryError error)
     {
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
 
       // consensus = null;
       // hconsensus = null;
index 897aa1e..b33df0c 100644 (file)
@@ -331,23 +331,23 @@ public abstract class AbstractJabaCalcWorker extends AlignCalcWorker
       System.err.println("submission error with " + getServiceActionText()
               + " :");
       x.printStackTrace();
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
     } catch (ResultNotAvailableException x)
     {
       System.err.println("collection error:\nJob ID: " + rslt);
       x.printStackTrace();
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
 
     } catch (OutOfMemoryError error)
     {
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
 
       // consensus = null;
       // hconsensus = null;
       ap.raiseOOMWarning(getServiceActionText(), error);
     } catch (Exception x)
     {
-      calcMan.workerCannotRun(this);
+      calcMan.disableWorker(this);
 
       // consensus = null;
       // hconsensus = null;
index 187540c..2af31bb 100644 (file)
@@ -122,7 +122,7 @@ public class SequenceAnnotationWSClient extends Jws2Client
         }
         // reinstate worker if it was blacklisted (might have happened due to
         // invalid parameters)
-        alignFrame.getViewport().getCalcManager().workerMayRun(worker);
+        alignFrame.getViewport().getCalcManager().enableWorker(worker);
         worker.updateParameters(this.preset, paramset);
       }
     }
diff --git a/test/jalview/workers/AlignCalcManagerTest.java b/test/jalview/workers/AlignCalcManagerTest.java
new file mode 100644 (file)
index 0000000..735c75d
--- /dev/null
@@ -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<AlignCalcWorkerI> 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<AlignmentAnnotation> 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);
+  }
+}