JAL-1990 suggested revision to separate references to UI components from datamodel... codereviews/JAL-1990_IProgressIndicator_jalview.structure
authorJim Procter <jprocter@issues.jalview.org>
Thu, 16 Jun 2016 14:18:12 +0000 (15:18 +0100)
committerJim Procter <jprocter@issues.jalview.org>
Thu, 16 Jun 2016 14:18:12 +0000 (15:18 +0100)
16 files changed:
src/MCview/AppletPDBCanvas.java
src/MCview/PDBCanvas.java
src/jalview/appletgui/AlignFrame.java
src/jalview/appletgui/AppletJmol.java
src/jalview/appletgui/AppletJmolBinding.java
src/jalview/appletgui/ExtJmol.java
src/jalview/ext/jmol/JalviewJmolBinding.java
src/jalview/gui/AppJmol.java
src/jalview/gui/AppJmolBinding.java
src/jalview/gui/ChimeraViewFrame.java
src/jalview/gui/IProgressIndicator.java
src/jalview/gui/Jalview2XML.java
src/jalview/gui/StructureChooser.java
src/jalview/gui/StructureViewerBase.java
src/jalview/structure/StructureSelectionManager.java
test/jalview/structures/models/AAStructureBindingModelTest.java

index df98833..244421e 100644 (file)
@@ -157,7 +157,7 @@ public class AppletPDBCanvas extends Panel implements MouseListener,
 
     try
     {
-      pdb = ssm.setMapping(seq, chains, pdbentry.getFile(), protocol);
+      pdb = ssm.setMapping(seq, chains, pdbentry.getFile(), protocol, null);
 
       if (protocol.equals(jalview.io.AppletFormatAdapter.PASTE))
       {
index 1c7a1f7..c582293 100644 (file)
@@ -151,7 +151,8 @@ public class PDBCanvas extends JPanel implements MouseListener,
 
     try
     {
-      pdb = ssm.setMapping(seq, chains, pdbentry.getFile(), protocol);
+      pdb = ssm.setMapping(seq, chains, pdbentry.getFile(), protocol,
+              ap.alignFrame);
 
       if (protocol.equals(jalview.io.AppletFormatAdapter.PASTE))
       {
index e5f0053..9ced93b 100644 (file)
@@ -4088,7 +4088,7 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener,
     {
       // register the association(s) and quit, don't create any windows.
       if (StructureSelectionManager.getStructureSelectionManager(applet)
-              .setMapping(seqs, chains, pdb.getFile(), protocol) == null)
+              .setMapping(seqs, chains, pdb.getFile(), protocol, null) == null)
       {
         System.err.println("Failed to map " + pdb.getFile() + " ("
                 + protocol + ") to any sequences");
index 8374721..e00a48d 100644 (file)
@@ -210,7 +210,7 @@ public class AppletJmol extends EmbmenuFrame implements
     {
       reader = StructureSelectionManager.getStructureSelectionManager(
               ap.av.applet).setMapping(seq, chains, pdbentry.getFile(),
-              protocol);
+              protocol, null);
       // PROMPT USER HERE TO ADD TO NEW OR EXISTING VIEW?
       // FOR NOW, LETS JUST OPEN A NEW WINDOW
     }
index 6ec5b4d..0234a1e 100644 (file)
@@ -24,6 +24,7 @@ import jalview.api.AlignmentViewPanel;
 import jalview.datamodel.PDBEntry;
 import jalview.datamodel.SequenceI;
 import jalview.ext.jmol.JalviewJmolBinding;
+import jalview.gui.IProgressIndicator;
 import jalview.structure.StructureSelectionManager;
 
 import java.awt.Container;
@@ -113,12 +114,14 @@ class AppletJmolBinding extends JalviewJmolBinding
     appletJmolBinding.updateTitleAndMenus();
   }
 
+  @Override
   public void updateColours(Object source)
   {
     AlignmentPanel ap = (AlignmentPanel) source;
     colourBySequence(ap);
   }
 
+  @Override
   public void showUrl(String url)
   {
     try
@@ -143,6 +146,7 @@ class AppletJmolBinding extends JalviewJmolBinding
     // do nothing.
   }
 
+  @Override
   public void selectionChanged(BS arg0)
   {
     // TODO Auto-generated method stub
@@ -195,4 +199,11 @@ class AppletJmolBinding extends JalviewJmolBinding
     // TODO Auto-generated method stub
     return null;
   }
+
+  @Override
+  protected IProgressIndicator getIProgressIndicator()
+  {
+    // no progress indicators on the applet
+    return null;
+  }
 }
index 929a871..04096a3 100644 (file)
@@ -26,6 +26,7 @@ import jalview.api.SequenceRenderer;
 import jalview.datamodel.PDBEntry;
 import jalview.datamodel.SequenceI;
 import jalview.ext.jmol.JalviewJmolBinding;
+import jalview.gui.IProgressIndicator;
 
 import java.awt.Container;
 import java.util.ArrayList;
@@ -64,6 +65,14 @@ public class ExtJmol extends JalviewJmolBinding
     notifyFileLoaded(null, null, null, null, 0);
   }
 
+  @Override
+  protected IProgressIndicator getIProgressIndicator()
+  {
+    // no progress indicators on applet (could access javascript for this)
+    return null;
+  }
+
+  @Override
   public void updateColours(Object source)
   {
 
@@ -71,6 +80,7 @@ public class ExtJmol extends JalviewJmolBinding
 
   }
 
+  @Override
   public void showUrl(String arg0)
   {
     showUrl(arg0, "jmol");
@@ -90,6 +100,7 @@ public class ExtJmol extends JalviewJmolBinding
     }
   }
 
+
   @Override
   public SequenceRenderer getSequenceRenderer(AlignmentViewPanel alignment)
   {
@@ -126,6 +137,7 @@ public class ExtJmol extends JalviewJmolBinding
     // ignore
   }
 
+  @Override
   public void selectionChanged(BS arg0)
   {
     System.out.println(arg0);
index 3f0847b..d7da38c 100644 (file)
@@ -27,6 +27,7 @@ import jalview.datamodel.AlignmentI;
 import jalview.datamodel.ColumnSelection;
 import jalview.datamodel.PDBEntry;
 import jalview.datamodel.SequenceI;
+import jalview.gui.IProgressIndicator;
 import jalview.io.AppletFormatAdapter;
 import jalview.io.StructureFile;
 import jalview.schemes.ColourSchemeI;
@@ -1144,7 +1145,8 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel
           // see JAL-623 - need method of matching pasted data up
           {
             pdb = getSsm().setMapping(getSequence()[pe], getChains()[pe],
-                    pdbfile, AppletFormatAdapter.PASTE);
+                    pdbfile, AppletFormatAdapter.PASTE,
+                    getIProgressIndicator());
             getPdbEntry(modelnum).setFile("INLINE" + pdb.getId());
             matches = true;
             foundEntry = true;
@@ -1176,7 +1178,7 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel
             }
             // Explicitly map to the filename used by Jmol ;
             pdb = getSsm().setMapping(getSequence()[pe], getChains()[pe],
-                    fileName, protocol);
+                    fileName, protocol, getIProgressIndicator());
             // pdbentry[pe].getFile(), protocol);
 
           }
@@ -1237,6 +1239,8 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel
     setLoadingFromArchive(false);
   }
 
+  protected abstract IProgressIndicator getIProgressIndicator();
+
   public void notifyNewPickingModeMeasurement(int iatom, String strMeasure)
   {
     notifyAtomPicked(iatom, strMeasure, null);
index a1846bc..2f646fb 100644 (file)
@@ -240,6 +240,11 @@ public class AppJmol extends StructureViewerBase
 
   IProgressIndicator progressBar = null;
 
+  @Override
+  protected IProgressIndicator getIProgressIndicator()
+  {
+    return progressBar;
+  }
   /**
    * add a single PDB structure to a new or existing Jmol view
    * 
index 1c54a5e..aebdc21 100644 (file)
@@ -49,6 +49,11 @@ public class AppJmolBinding extends JalviewJmolBinding
   }
 
   @Override
+  protected IProgressIndicator getIProgressIndicator()
+  {
+    return appJmolWindow.progressBar;
+  }
+  @Override
   public FeatureRenderer getFeatureRenderer(AlignmentViewPanel alignment)
   {
     AlignmentPanel ap = (alignment == null) ? appJmolWindow
@@ -113,6 +118,7 @@ public class AppJmolBinding extends JalviewJmolBinding
     // appJmolWindow.repaint();
     javax.swing.SwingUtilities.invokeLater(new Runnable()
     {
+      @Override
       public void run()
       {
         appJmolWindow.updateTitleAndMenus();
@@ -121,6 +127,7 @@ public class AppJmolBinding extends JalviewJmolBinding
     });
   }
 
+  @Override
   public void updateColours(Object source)
   {
     AlignmentPanel ap = (AlignmentPanel) source;
@@ -144,6 +151,7 @@ public class AppJmolBinding extends JalviewJmolBinding
     // msWalltime);
   }
 
+  @Override
   public void showUrl(String url)
   {
     showUrl(url, "jmol");
index f2244d5..b261ca6 100644 (file)
@@ -675,7 +675,8 @@ public class ChimeraViewFrame extends StructureViewerBase
             }
             // Explicitly map to the filename used by Chimera ;
             jmb.getSsm().setMapping(jmb.getSequence()[pos],
-                    jmb.getChains()[pos], pe.getFile(), protocol);
+                    jmb.getChains()[pos], pe.getFile(), protocol,
+                    progressBar);
           } catch (OutOfMemoryError oomerror)
           {
             new OOMWarning(
@@ -720,7 +721,7 @@ public class ChimeraViewFrame extends StructureViewerBase
 
   /**
    * Fetch PDB data and save to a local file. Returns the full path to the file,
-   * or null if fetch fails.
+   * or null if fetch fails. TODO: refactor to common with Jmol ? duplication
    * 
    * @param processingEntry
    * @return
@@ -1215,4 +1216,10 @@ public class ChimeraViewFrame extends StructureViewerBase
   {
     return jmb;
   }
+
+  @Override
+  protected IProgressIndicator getIProgressIndicator()
+  {
+    return progressBar;
+  }
 }
index 981e94c..35bd871 100644 (file)
@@ -34,7 +34,8 @@ public interface IProgressIndicator
    * is removed with a second call with same ID.
    * 
    * @param message
-   *          - displayed message for operation
+   *          - displayed message for operation. Please ensure message is
+   *          internationalised.
    * @param id
    *          - unique handle for this indicator
    */
index a3604d6..79cffdc 100644 (file)
@@ -3969,7 +3969,7 @@ public class Jalview2XML
       String pdbFile = filedat.getFilePath();
       SequenceI[] seq = filedat.getSeqList().toArray(new SequenceI[0]);
       binding.getSsm().setMapping(seq, null, pdbFile,
-              jalview.io.AppletFormatAdapter.FILE);
+              jalview.io.AppletFormatAdapter.FILE, null);
       binding.addSequenceForStructFile(pdbFile, seq);
     }
     // and add the AlignmentPanel's reference to the view panel
index fd2de8e..8dbe0c2 100644 (file)
@@ -717,10 +717,7 @@ public class StructureChooser extends GStructureChooser implements
   @Override
   public void ok_ActionPerformed()
   {
-    final long progressSessionId = System.currentTimeMillis();
     final StructureSelectionManager ssm = ap.getStructureSelectionManager();
-    ssm.setProgressIndicator(this);
-    ssm.setProgressSessionId(progressSessionId);
     new Thread(new Runnable()
     {
       @Override
@@ -765,7 +762,8 @@ public class StructureChooser extends GStructureChooser implements
       }
       SequenceI[] selectedSeqs = selectedSeqsToView
               .toArray(new SequenceI[selectedSeqsToView.size()]);
-          launchStructureViewer(ssm, pdbEntriesToView, ap, selectedSeqs);
+          launchStructureViewer(ssm, pdbEntriesToView, ap,
+                  selectedSeqs);
     }
     else if (currentView == VIEWS_LOCAL_PDB)
     {
@@ -788,7 +786,8 @@ public class StructureChooser extends GStructureChooser implements
       }
       SequenceI[] selectedSeqs = selectedSeqsToView
               .toArray(new SequenceI[selectedSeqsToView.size()]);
-          launchStructureViewer(ssm, pdbEntriesToView, ap, selectedSeqs);
+          launchStructureViewer(ssm, pdbEntriesToView, ap,
+                  selectedSeqs);
     }
     else if (currentView == VIEWS_ENTER_ID)
     {
@@ -830,7 +829,8 @@ public class StructureChooser extends GStructureChooser implements
                       jalview.io.AppletFormatAdapter.FILE,
                       selectedSequence, true, Desktop.instance);
 
-          launchStructureViewer(ssm, new PDBEntry[] { fileEntry }, ap,
+          launchStructureViewer(ssm,
+                  new PDBEntry[] { fileEntry }, ap,
                   new SequenceI[] { selectedSequence });
     }
     mainFrame.dispose();
@@ -857,12 +857,17 @@ public class StructureChooser extends GStructureChooser implements
           final PDBEntry[] pdbEntriesToView,
           final AlignmentPanel alignPanel, SequenceI[] sequences)
   {
-    ssm.setProgressBar(MessageManager
-            .getString("status.launching_3d_structure_viewer"));
+    long progressId = sequences.hashCode();
+    setProgressBar(MessageManager
+            .getString("status.launching_3d_structure_viewer"), progressId);
     final StructureViewer sViewer = new StructureViewer(ssm);
+    setProgressBar(null, progressId);
 
     if (SiftsSettings.isMapWithSifts())
     {
+      // TODO: prompt user if there are lots of sequences without dbrefs.
+      // It can take a long time if we need to fetch all dbrefs for all
+      // sequences!
       ArrayList<SequenceI> seqsWithoutSourceDBRef = new ArrayList<SequenceI>();
       for (SequenceI seq : sequences)
       {
@@ -875,10 +880,9 @@ public class StructureChooser extends GStructureChooser implements
       if (!seqsWithoutSourceDBRef.isEmpty())
       {
         int y = seqsWithoutSourceDBRef.size();
-        ssm.setProgressBar(null);
-        ssm.setProgressBar(MessageManager.formatMessage(
+        setProgressBar(MessageManager.formatMessage(
                 "status.fetching_dbrefs_for_sequences_without_valid_refs",
-                y));
+                y), progressId);
         SequenceI[] seqWithoutSrcDBRef = new SequenceI[y];
         int x = 0;
         for (SequenceI fSeq : seqsWithoutSourceDBRef)
@@ -886,6 +890,7 @@ public class StructureChooser extends GStructureChooser implements
           seqWithoutSrcDBRef[x++] = fSeq;
         }
         new DBRefFetcher(seqWithoutSrcDBRef).fetchDBRefs(true);
+        setProgressBar("Fetch complete.", progressId); // todo i18n
       }
     }
     if (pdbEntriesToView.length > 1)
@@ -896,19 +901,18 @@ public class StructureChooser extends GStructureChooser implements
         seqsMap.add(new SequenceI[] { seq });
       }
       SequenceI[][] collatedSeqs = seqsMap.toArray(new SequenceI[0][0]);
-      ssm.setProgressBar(null);
-      ssm.setProgressBar(MessageManager
-              .getString("status.fetching_3d_structures_for_selected_entries"));
+      setProgressBar(MessageManager
+                    .getString("status.fetching_3d_structures_for_selected_entries"), progressId);
       sViewer.viewStructures(pdbEntriesToView, collatedSeqs, alignPanel);
     }
     else
     {
-      ssm.setProgressBar(null);
-      ssm.setProgressBar(MessageManager.formatMessage(
+      setProgressBar(MessageManager.formatMessage(
               "status.fetching_3d_structures_for",
-              pdbEntriesToView[0].getId()));
+              pdbEntriesToView[0].getId()),progressId);
       sViewer.viewStructures(pdbEntriesToView[0], sequences, alignPanel);
     }
+    setProgressBar(null, progressId);
   }
 
   /**
index 7df42fd..41390b1 100644 (file)
@@ -152,6 +152,7 @@ public abstract class StructureViewerBase extends GStructureViewer
     this.ap = alp;
   }
 
+  @Override
   public AlignmentPanel[] getAllAlignmentPanels()
   {
     AlignmentPanel[] t, list = new AlignmentPanel[0];
@@ -267,6 +268,8 @@ public abstract class StructureViewerBase extends GStructureViewer
 
   protected abstract AAStructureBindingModel getBindingModel();
 
+  protected abstract IProgressIndicator getIProgressIndicator();
+
   /**
    * add a new structure (with associated sequences and chains) to this viewer,
    * retrieving it if necessary first.
@@ -291,6 +294,7 @@ public abstract class StructureViewerBase extends GStructureViewer
         // queue.
         new Thread(new Runnable()
         {
+          @Override
           public void run()
           {
             while (worker != null && worker.isAlive() && _started)
@@ -411,7 +415,7 @@ public abstract class StructureViewerBase extends GStructureViewer
      * create the mappings
      */
     apanel.getStructureSelectionManager().setMapping(seq, chains,
-            pdbFilename, AppletFormatAdapter.FILE);
+            pdbFilename, AppletFormatAdapter.FILE, getIProgressIndicator());
 
     /*
      * alert the FeatureRenderer to show new (PDB RESNUM) features
index fb96b22..f813411 100644 (file)
@@ -70,12 +70,8 @@ public class StructureSelectionManager
 
   private boolean addTempFacAnnot = false;
 
-  private IProgressIndicator progressIndicator;
-
   private SiftsClient siftsClient = null;
 
-  private long progressSessionId;
-
   /*
    * Set of any registered mappings between (dataset) sequences.
    */
@@ -321,9 +317,11 @@ public class StructureSelectionManager
    * @return null or the structure data parsed as a pdb file
    */
   synchronized public StructureFile setMapping(SequenceI[] sequence,
-          String[] targetChains, String pdbFile, String protocol)
+          String[] targetChains, String pdbFile, String protocol,
+          IProgressIndicator progress)
   {
-    return setMapping(true, sequence, targetChains, pdbFile, protocol);
+    return computeMapping(true, sequence, targetChains, pdbFile, protocol,
+            progress);
   }
 
 
@@ -338,7 +336,7 @@ public class StructureSelectionManager
    *          - one or more sequences to be mapped to pdbFile
    * @param targetChainIds
    *          - optional chain specification for mapping each sequence to pdb
-   *          (may be nill, individual elements may be nill)
+   *          (may be null, individual elements may be null)
    * @param pdbFile
    *          - structure data resource
    * @param protocol
@@ -350,6 +348,16 @@ public class StructureSelectionManager
           String pdbFile,
           String protocol)
   {
+    return computeMapping(forStructureView, sequenceArray, targetChainIds,
+            pdbFile, protocol, null);
+  }
+
+  synchronized public StructureFile computeMapping(
+          boolean forStructureView, SequenceI[] sequenceArray,
+          String[] targetChainIds, String pdbFile, String protocol,
+          IProgressIndicator progress)
+  {
+    long progressSessionId = System.currentTimeMillis() * 3;
     /*
      * There will be better ways of doing this in the future, for now we'll use
      * the tried and tested MCview pdb mapping
@@ -496,13 +504,14 @@ public class StructureSelectionManager
       {
         pdbFile = "INLINE" + pdb.getId();
       }
-
       ArrayList<StructureMapping> seqToStrucMapping = new ArrayList<StructureMapping>();
       if (isMapUsingSIFTs)
       {
-        setProgressBar(null);
-        setProgressBar(MessageManager
-                .getString("status.obtaining_mapping_with_sifts"));
+        if (progress!=null) {
+          progress.setProgressBar(MessageManager
+                .getString("status.obtaining_mapping_with_sifts"),
+                  progressSessionId);
+        }
         jalview.datamodel.Mapping sqmpping = maxAlignseq
                 .getMappingFromS1(false);
         if (targetChainId != null && !targetChainId.trim().isEmpty())
@@ -559,17 +568,23 @@ public class StructureSelectionManager
       }
       else
       {
-        setProgressBar(null);
-        setProgressBar(MessageManager
-                .getString("status.obtaining_mapping_with_nw_alignment"));
+        if (progress != null)
+        {
+          progress.setProgressBar(MessageManager
+                                 .getString("status.obtaining_mapping_with_nw_alignment"),
+                  progressSessionId);
+        }
         seqToStrucMapping.add(getNWMappings(seq, pdbFile, maxChainId,
                 maxChain, pdb, maxAlignseq));
       }
-
       if (forStructureView)
       {
         mappings.addAll(seqToStrucMapping);
       }
+      if (progress != null)
+      {
+        progress.setProgressBar(null, progressSessionId);
+      }
     }
     return pdb;
   }
@@ -1294,35 +1309,6 @@ public class StructureSelectionManager
     return null;
   }
 
-  public IProgressIndicator getProgressIndicator()
-  {
-    return progressIndicator;
-  }
-
-  public void setProgressIndicator(IProgressIndicator progressIndicator)
-  {
-    this.progressIndicator = progressIndicator;
-  }
-
-  public long getProgressSessionId()
-  {
-    return progressSessionId;
-  }
-
-  public void setProgressSessionId(long progressSessionId)
-  {
-    this.progressSessionId = progressSessionId;
-  }
-
-  public void setProgressBar(String message)
-  {
-    if (progressIndicator == null)
-    {
-      return;
-    }
-    progressIndicator.setProgressBar(message, progressSessionId);
-  }
-
   public List<AlignedCodonFrame> getSequenceMappings()
   {
     return seqmappings;
index bb81992..e82bfea 100644 (file)
@@ -98,11 +98,11 @@ public class AAStructureBindingModelTest
     StructureSelectionManager ssm = new StructureSelectionManager();
 
     ssm.setMapping(new SequenceI[] { seq1 }, null, PDB_1,
-            AppletFormatAdapter.PASTE);
+            AppletFormatAdapter.PASTE, null);
     ssm.setMapping(new SequenceI[] { seq2 }, null, PDB_2,
-            AppletFormatAdapter.PASTE);
+            AppletFormatAdapter.PASTE, null);
     ssm.setMapping(new SequenceI[] { seq3 }, null, PDB_3,
-            AppletFormatAdapter.PASTE);
+            AppletFormatAdapter.PASTE, null);
 
     testee = new AAStructureBindingModel(ssm, pdbFiles, seqs, chains, null)
     {