JAL-3690 separate startup and poll code in SeqAnnotationCalcWorker

diff --git a/src/jalview/api/PollableAlignCalcWorkerI.java b/src/jalview/api/PollableAlignCalcWorkerI.java
index 9697187..25074ec 100644 (file)
--- a/src/jalview/api/PollableAlignCalcWorkerI.java
+++ b/src/jalview/api/PollableAlignCalcWorkerI.java
@@ -14,5 +14,5 @@ public interface PollableAlignCalcWorkerI extends AlignCalcWorkerI
   
   public void cancel();
   
-  public void cleanUp();
+  public void done();
 }

diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java
index 9e0fe6f..cc81eef 100644 (file)
--- a/src/jalview/gui/AlignFrame.java
+++ b/src/jalview/gui/AlignFrame.java
@@ -991,6 +991,12 @@ public class AlignFrame extends GAlignFrame
   {
     progressBar.setProgressBar(message, id);
   }
+  
+  @Override
+  public void removeProgressBar(long id)
+  {
+    progressBar.removeProgressBar(id);
+  }
 
   @Override
   public void registerHandler(final long id,

diff --git a/src/jalview/gui/Desktop.java b/src/jalview/gui/Desktop.java
index 63b621e..e9bf09f 100644 (file)
--- a/src/jalview/gui/Desktop.java
+++ b/src/jalview/gui/Desktop.java
@@ -2520,6 +2520,13 @@ public class Desktop extends jalview.jbgui.GDesktop
       progressBars.put(Long.valueOf(id), addProgressPanel(message));
     }
   }
+  
+  @Override
+  public void removeProgressBar(long id)
+  {
+    //TODO
+    throw new UnsupportedOperationException("not implemented");
+  }
 
   /*
    * (non-Javadoc)

diff --git a/src/jalview/gui/IProgressIndicator.java b/src/jalview/gui/IProgressIndicator.java
index 35bd871..c250aca 100644 (file)
--- a/src/jalview/gui/IProgressIndicator.java
+++ b/src/jalview/gui/IProgressIndicator.java
@@ -56,4 +56,11 @@ public interface IProgressIndicator
    */
   boolean operationInProgress();
 
+  /**
+   * Remove progress bar with a given id from the panel.
+   * 
+   * @param id
+   */
+  void removeProgressBar(long id);
+
 }

diff --git a/src/jalview/gui/PCAPanel.java b/src/jalview/gui/PCAPanel.java
index 96a8b0d..a2114f0 100644 (file)
--- a/src/jalview/gui/PCAPanel.java
+++ b/src/jalview/gui/PCAPanel.java
@@ -609,6 +609,12 @@ public class PCAPanel extends GPCAPanel
     // // setMenusForViewport();
     // validate();
   }
+  
+  @Override
+  public void removeProgressBar(long id)
+  {
+    progressBar.removeProgressBar(id);
+  }
 
   @Override
   public void registerHandler(final long id,

diff --git a/src/jalview/gui/ProgressBar.java b/src/jalview/gui/ProgressBar.java
index 011d810..abf096f 100644 (file)
--- a/src/jalview/gui/ProgressBar.java
+++ b/src/jalview/gui/ProgressBar.java
@@ -166,6 +166,23 @@ public class ProgressBar implements IProgressIndicator
     });
 
   }
+  
+  @Override
+  public void removeProgressBar(final long id)
+  {
+    SwingUtilities.invokeLater(() -> {
+      JPanel progressPanel = progressBars.get(id);
+      if (progressPanel != null)
+      {
+        progressBars.remove(id);
+        if (progressBarHandlers.containsKey(id))
+        {
+          progressBarHandlers.remove(id);
+        }
+        removeRow(progressPanel);
+      }
+    });
+  }
 
   /**
    * Lays out progress bar container hierarchy

diff --git a/src/jalview/gui/StructureChooser.java b/src/jalview/gui/StructureChooser.java
index 33d8c33..3807ebd 100644 (file)
--- a/src/jalview/gui/StructureChooser.java
+++ b/src/jalview/gui/StructureChooser.java
@@ -1386,6 +1386,12 @@ public class StructureChooser extends GStructureChooser
   {
     progressBar.setProgressBar(message, id);
   }
+  
+  @Override
+  public void removeProgressBar(long id)
+  {
+    progressBar.removeProgressBar(id);
+  }
 
   @Override
   public void registerHandler(long id, IProgressIndicatorHandler handler)

diff --git a/src/jalview/gui/WebserviceInfo.java b/src/jalview/gui/WebserviceInfo.java
index 25ade21..03d76ee 100644 (file)
--- a/src/jalview/gui/WebserviceInfo.java
+++ b/src/jalview/gui/WebserviceInfo.java
@@ -928,6 +928,12 @@ public void hyperlinkUpdate(HyperlinkEvent e)
   {
     progressBar.setProgressBar(message, id);
   }
+  
+  @Override
+  public void removeProgressBar(long id)
+  {
+    progressBar.removeProgressBar(id);
+  }
 
   @Override
   public void registerHandler(final long id,

diff --git a/src/jalview/workers/AlignCalcManager.java b/src/jalview/workers/AlignCalcManager.java
index 79ca863..9f8d4e8 100644 (file)
--- a/src/jalview/workers/AlignCalcManager.java
+++ b/src/jalview/workers/AlignCalcManager.java
@@ -185,7 +185,7 @@ public class AlignCalcManager implements AlignCalcManagerI
         try
         {
           worker.run();
-        } catch (Exception e)
+        } catch (Throwable e)
         {
           e.printStackTrace();
         }

diff --git a/src/jalview/workers/AlignCalcManager2.java b/src/jalview/workers/AlignCalcManager2.java
index 7a19321..0057500 100644 (file)
--- a/src/jalview/workers/AlignCalcManager2.java
+++ b/src/jalview/workers/AlignCalcManager2.java
@@ -183,6 +183,8 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
             }
             else if (!completed)
             {
+              Cache.log.debug(format("Polling worker %s",
+                      worker.getClass().getName()));
               if (worker.poll())
               {
                 Cache.log.debug(format("Worker %s finished",
@@ -198,14 +200,12 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
           }
           if (completed)
           {
-            try
-            {
-              future.cancel(false);
-            }
-            catch (NullPointerException ignored)
-            {
-              // extremely unlikely to happen
-            }
+            Cache.log.debug(format("Finalizing completed worker %s",
+                    worker.getClass().getName()));
+            worker.done();
+            // almost impossible, but the future may be null at this point
+            // let it throw NPE to cancel forcefully
+            future.cancel(false);
           }
         }
       };
@@ -224,6 +224,9 @@ public class AlignCalcManager2 implements AlignCalcManagerI2
       task.cancel(false);
       executor.submit(() -> {
         worker.cancel();
+        Cache.log.debug(format("Finalizing cancelled worker %s",
+                worker.getClass().getName()));
+        worker.done();
       });
     }
   }

diff --git a/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java b/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java
index ae03dfb..53b790d 100644 (file)
--- a/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java
+++ b/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java
@@ -26,6 +26,7 @@ import jalview.analysis.SeqsetUtils;
 import jalview.api.AlignViewportI;
 import jalview.api.AlignmentViewPanel;
 import jalview.api.FeatureColourI;
+import jalview.api.PollableAlignCalcWorkerI;
 import jalview.bin.Cache;
 import jalview.datamodel.AlignmentAnnotation;
 import jalview.datamodel.AlignmentI;
@@ -64,7 +65,7 @@ import java.util.List;
 import java.util.Map;
 
 public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
-        implements WSAnnotationCalcManagerI
+        implements WSAnnotationCalcManagerI, PollableAlignCalcWorkerI
 {
 
   protected ServiceWithParameters service;
@@ -162,17 +163,11 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
     return gapMap;
   }
 
-  public SeqAnnotationServiceCalcWorker(AlignViewportI alignViewport,
-          AlignmentViewPanel alignPanel)
-  {
-    super(alignViewport, alignPanel);
-  }
-
   public SeqAnnotationServiceCalcWorker(ServiceWithParameters service,
           AlignFrame alignFrame,
           WsParamSetI preset, List<ArgumentI> paramset)
   {
-    this(alignFrame.getCurrentView(), alignFrame.alignPanel);
+    super(alignFrame.getCurrentView(), alignFrame.alignPanel);
     // TODO: both these fields needed ?
     this.alignFrame = alignFrame;
     this.guiProgress = alignFrame;
@@ -185,6 +180,7 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
               .getEndpoint();
     } catch (ClassCastException cce)
     {
+      annotService = null;
       JvOptionPane.showMessageDialog(Desktop.desktop,
               MessageManager.formatMessage(
                       "label.service_called_is_not_an_annotation_service",
@@ -194,6 +190,7 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
               JvOptionPane.WARNING_MESSAGE);
 
     }
+    cancellable = CancellableI.class.isInstance(annotService);
     // configure submission flags
     proteinAllowed = service.isProteinService();
     nucleotidesAllowed = service.isNucleotideService();
@@ -218,7 +215,8 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
     return annotService != null;
   }
 
-  protected jalview.ws.api.SequenceAnnotationServiceI annotService = null;
+  protected SequenceAnnotationServiceI annotService;
+  protected final boolean cancellable;
 
   volatile JobId rslt = null;
 
@@ -226,8 +224,13 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
 
   private int min_valid_seqs;
 
-  @Override
-  public void run() throws Exception
+
+  private long progressId = -1;
+  JobStateSummary job = null;
+  WebserviceInfo info = null;
+  List<SequenceI> seqs = null;
+  
+  @Override public void startUp() throws Throwable
   {
     if (alignViewport.isClosed())
     {
@@ -239,134 +242,106 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
       return;
     }
 
-    long progressId = -1;
-
-    int serverErrorsLeft = 3;
-    final boolean cancellable = CancellableI.class
-            .isAssignableFrom(annotService.getClass());
     StringBuffer msg = new StringBuffer();
-    JobStateSummary job = new JobStateSummary();
-    WebserviceInfo info = new WebserviceInfo("foo", "bar", false);
-    try
-    {
-      List<SequenceI> seqs = getInputSequences(
-              alignViewport.getAlignment(),
-              bySequence ? alignViewport.getSelectionGroup() : null);
+    job = new JobStateSummary();
+    info = new WebserviceInfo("foo", "bar", false);
 
-      if (seqs == null || !checkValidInputSeqs(seqs))
-      {
-        jalview.bin.Cache.log.debug(
-                "Sequences for analysis service were null or not valid");
-        return;
-      }
+    seqs = getInputSequences(
+            alignViewport.getAlignment(),
+            bySequence ? alignViewport.getSelectionGroup() : null);
 
-      if (guiProgress != null)
-      {
-        guiProgress.setProgressBar(service.getActionText(),
-                progressId = System.currentTimeMillis());
-      }
-      jalview.bin.Cache.log.debug("submitted " + seqs.size()
-              + " sequences to " + service.getActionText());
+    if (seqs == null || !checkValidInputSeqs(seqs))
+    {
+      jalview.bin.Cache.log.debug(
+              "Sequences for analysis service were null or not valid");
+      return;
+    }
 
-      rslt = annotService.submitToService(seqs, getPreset(),
-              getArguments());
-      if (rslt == null)
-      {
-        return;
-      }
-      // TODO: handle job submission error reporting here.
-      Cache.log.debug("Service " + service.getUri() + "\nSubmitted job ID: "
-              + rslt);
-      ;
-      // ///
-      // otherwise, construct WsJob and any UI handlers
-      running = new AnnotationWsJob();
-      running.setJobHandle(rslt);
-      running.setSeqNames(seqNames);
-      running.setStartPos(start);
-      running.setSeqs(seqs);
-      job.updateJobPanelState(info, "", running);
-      if (guiProgress != null)
-      {
-        guiProgress.registerHandler(progressId,
-                new IProgressIndicatorHandler()
-                {
+    if (guiProgress != null)
+    {
+      guiProgress.setProgressBar(service.getActionText(),
+              progressId = System.currentTimeMillis());
+    }
+    jalview.bin.Cache.log.debug("submitted " + seqs.size()
+            + " sequences to " + service.getActionText());
 
-                  @Override
-                  public boolean cancelActivity(long id)
-                  {
-                    ((CancellableI) annotService).cancel(running);
-                    return true;
-                  }
-
-                  @Override
-                  public boolean canCancel()
-                  {
-                    return cancellable;
-                  }
-                });
-      }
-      
-      // ///
-      // and poll for updates until job finishes, fails or becomes stale
-      
-      boolean finished = false;
-      do
-      {
-        Cache.log.debug("Updating status for annotation service.");
-        annotService.updateStatus(running);
-        job.updateJobPanelState(info, "", running);
-        if (running.isSubjobComplete())
-        {
-          Cache.log.debug(
-                  "Finished polling analysis service job: status reported is "
-                          + running.getState());
-          finished = true;
-        }
-        else
-        {
-          Cache.log.debug("Status now " + running.getState());
-        }
+    rslt = annotService.submitToService(seqs, getPreset(),
+            getArguments());
+    if (rslt == null)
+    {
+      return;
+    }
+    // TODO: handle job submission error reporting here.
+    Cache.log.debug("Service " + service.getUri() + "\nSubmitted job ID: "
+            + rslt);
+    ;
+    // ///
+    // otherwise, construct WsJob and any UI handlers
+    running = new AnnotationWsJob();
+    running.setJobHandle(rslt);
+    running.setSeqNames(seqNames);
+    running.setStartPos(start);
+    running.setSeqs(seqs);
+    job.updateJobPanelState(info, "", running);
+    if (guiProgress != null)
+    {
+      guiProgress.registerHandler(progressId,
+              new IProgressIndicatorHandler()
+              {
 
-        // pull any stats - some services need to flush log output before
-        // results are available
-        Cache.log.debug("Updating progress log for annotation service.");
+                @Override
+                public boolean cancelActivity(long id)
+                {
+                  calcMan.cancelWorker(SeqAnnotationServiceCalcWorker.this);
+                  return true;
+                }
 
-        try
-        {
-          annotService.updateJobProgress(running);
-        } catch (Throwable thr)
-        {
-          Cache.log.debug("Ignoring exception during progress update.",
-                  thr);
-        }
-        Cache.log.trace("Result of poll: " + running.getStatus());
+                @Override
+                public boolean canCancel()
+                {
+                  return cancellable;
+                }
+              });
+    }
+  }
+  
+  @Override public boolean poll() throws Throwable
+  {
+    boolean finished = false;
+    
+    Cache.log.debug("Updating status for annotation service.");
+    annotService.updateStatus(running);
+    job.updateJobPanelState(info, "", running);
+    if (running.isSubjobComplete())
+    {
+      Cache.log.debug(
+              "Finished polling analysis service job: status reported is "
+                      + running.getState());
+      finished = true;
+    }
+    else
+    {
+      Cache.log.debug("Status now " + running.getState());
+    }
 
-        if (!finished && !running.isFailed())
-        {
-          try
-          {
-            Cache.log.debug("Analysis service job thread sleeping.");
-            Thread.sleep(200);
-            Cache.log.debug("Analysis service job thread woke.");
-          } catch (InterruptedException x)
-          {
-          }
-          ;
-        }
-      } while (!finished);
+    // pull any stats - some services need to flush log output before
+    // results are available
+    Cache.log.debug("Updating progress log for annotation service.");
 
+    try
+    {
+      annotService.updateJobProgress(running);
+    } catch (Throwable thr)
+    {
+      Cache.log.debug("Ignoring exception during progress update.",
+              thr);
+    }
+    Cache.log.trace("Result of poll: " + running.getStatus());
+    
+    
+    if (finished)
+    {
       Cache.log.debug("Job poll loop exited. Job is " + running.getState());
-      // TODO: need to poll/retry
-      if (serverErrorsLeft > 0)
-      {
-        try
-        {
-          Thread.sleep(200);
-        } catch (InterruptedException x)
-        {
-        }
-      }
       if (running.isFinished())
       {
         // expect there to be results to collect
@@ -466,60 +441,28 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
         }
       }
       Cache.log.debug("Annotation Service Worker thread finished.");
-    }
-// TODO: use service specitic exception handlers
-//    catch (JobSubmissionException x)
-//    {
-//
-//      System.err.println(
-//              "submission error with " + getServiceActionText() + " :");
-//      x.printStackTrace();
-//      calcMan.disableWorker(this);
-//    } catch (ResultNotAvailableException x)
-//    {
-//      System.err.println("collection error:\nJob ID: " + rslt);
-//      x.printStackTrace();
-//      calcMan.disableWorker(this);
-//
-//    } catch (OutOfMemoryError error)
-//    {
-//      calcMan.disableWorker(this);
-//
-//      ap.raiseOOMWarning(getServiceActionText(), error);
-//    } 
-    catch (Throwable x)
-    {
-      calcMan.disableWorker(this);
 
-      System.err
-              .println("Blacklisting worker due to unexpected exception:");
-      x.printStackTrace();
-      throw new Exception(x);
-    } finally
+    }
+    
+    return finished;
+  }
+  
+  @Override public void cancel()
+  {
+    cancelCurrentJob();
+  }
+  
+  @Override public void done()
+  {
+    if (ap != null)
     {
-
-      if (ap != null)
+      if (guiProgress != null && progressId != -1)
       {
-        if (guiProgress != null && progressId != -1)
-        {
-          guiProgress.setProgressBar("", progressId);
-        }
-        // TODO: may not need to paintAlignment again !
-        ap.paintAlignment(false, false);
-      }
-      if (msg.length() > 0)
-      {
-        // TODO: stash message somewhere in annotation or alignment view.
-        // code below shows result in a text box popup
-        /*
-         * jalview.gui.CutAndPasteTransfer cap = new
-         * jalview.gui.CutAndPasteTransfer(); cap.setText(msg.toString());
-         * jalview.gui.Desktop.addInternalFrame(cap,
-         * "Job Status for "+getServiceActionText(), 600, 400);
-         */
+        guiProgress.removeProgressBar(progressId);
       }
+      // TODO: may not need to paintAlignment again !
+      ap.paintAlignment(false, false);
     }
-
   }
 
   /**
@@ -552,7 +495,7 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
     try
     {
       String id = running.getJobId();
-      if (((CancellableI) annotService).cancel(running))
+      if (cancellable && ((CancellableI) annotService).cancel(running))
       {
         System.err.println("Cancelled job " + id);
       }