JAL-1829 JAL-1830 refactoring for correct mapping (de-)registration
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 17 Aug 2015 11:09:19 +0000 (12:09 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 17 Aug 2015 11:09:19 +0000 (12:09 +0100)
src/jalview/gui/AlignFrame.java
src/jalview/gui/AlignViewport.java
src/jalview/gui/Desktop.java
src/jalview/structure/StructureSelectionManager.java
test/jalview/structure/StructureSelectionManagerTest.java

index b37b442..83fb7c0 100644 (file)
@@ -1497,6 +1497,10 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
 
       if (closeAllTabs)
       {
+        /*
+         * this will raise an INTERNAL_FRAME_CLOSED event and this method will
+         * be called recursively, with the frame now in 'closed' state
+         */
         this.setClosed(true);
       }
     } catch (Exception ex)
@@ -2749,12 +2753,17 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     }
 
     /*
-     * Views share the same edits, undo and redo stacks, mappings.
+     * Views share the same edits undo and redo stacks
      */
     newap.av.setHistoryList(viewport.getHistoryList());
     newap.av.setRedoList(viewport.getRedoList());
-    newap.av.getAlignment().setCodonFrames(
-            viewport.getAlignment().getCodonFrames());
+
+    /*
+     * Views share the same mappings; need to deregister any new mappings
+     * created by copyAlignPanel, and register the new reference to the shared
+     * mappings
+     */
+    newap.av.replaceMappings(viewport.getAlignment());
 
     newap.av.viewName = getNewViewName(viewTitle);
 
@@ -4968,10 +4977,10 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
               "Exception during translation. Please report this !", ex);
       final String msg = MessageManager
               .getString("label.error_when_translating_sequences_submit_bug_report");
-      final String title = MessageManager
+      final String errorTitle = MessageManager
               .getString("label.implementation_error")
               + MessageManager.getString("translation_failed");
-      JOptionPane.showMessageDialog(Desktop.desktop, msg, title,
+      JOptionPane.showMessageDialog(Desktop.desktop, msg, errorTitle,
               JOptionPane.ERROR_MESSAGE);
       return;
     }
@@ -4979,9 +4988,9 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     {
       final String msg = MessageManager
               .getString("label.select_at_least_three_bases_in_at_least_one_sequence_to_cDNA_translation");
-      final String title = MessageManager
+      final String errorTitle = MessageManager
               .getString("label.translation_failed");
-      JOptionPane.showMessageDialog(Desktop.desktop, msg, title,
+      JOptionPane.showMessageDialog(Desktop.desktop, msg, errorTitle,
               JOptionPane.WARNING_MESSAGE);
     }
     else
@@ -4995,8 +5004,7 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
       if (Cache.getDefault(Preferences.ENABLE_SPLIT_FRAME, true))
       {
         final SequenceI[] seqs = viewport.getSelectionAsNewSequence();
-        viewport.openSplitFrame(af, new Alignment(seqs),
-                al.getCodonFrames());
+        viewport.openSplitFrame(af, new Alignment(seqs));
       }
       else
       {
@@ -6088,7 +6096,6 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
       sf.setComplementVisible(this, show);
     }
   }
-
 }
 
 class PrintThread extends Thread
index 9ed953f..aedfe12 100644 (file)
@@ -45,7 +45,6 @@ import jalview.api.AlignViewportI;
 import jalview.api.ViewStyleI;
 import jalview.bin.Cache;
 import jalview.commands.CommandI;
-import jalview.datamodel.AlignedCodonFrame;
 import jalview.datamodel.Alignment;
 import jalview.datamodel.AlignmentI;
 import jalview.datamodel.ColumnSelection;
@@ -71,7 +70,6 @@ import java.awt.Rectangle;
 import java.util.ArrayList;
 import java.util.Hashtable;
 import java.util.List;
-import java.util.Set;
 import java.util.Vector;
 
 import javax.swing.JInternalFrame;
@@ -416,16 +414,43 @@ public class AlignViewport extends AlignmentViewport implements
    */
   public void setAlignment(AlignmentI align)
   {
-    if (alignment != null && alignment.getCodonFrames() != null)
+    replaceMappings(align);
+    this.alignment = align;
+  }
+
+  /**
+   * Replace any codon mappings for this viewport with those for the given
+   * viewport
+   * 
+   * @param align
+   */
+  public void replaceMappings(AlignmentI align)
+  {
+    StructureSelectionManager ssm = StructureSelectionManager
+            .getStructureSelectionManager(Desktop.instance);
+
+    /*
+     * Deregister current mappings (if any)
+     */
+    if (alignment != null)
     {
-      StructureSelectionManager.getStructureSelectionManager(
-              Desktop.instance).removeMappings(alignment.getCodonFrames());
+      ssm.removeMappings(alignment.getCodonFrames());
     }
-    this.alignment = align;
-    if (alignment != null && alignment.getCodonFrames() != null)
+
+    /*
+     * Register new mappings (if any)
+     */
+    if (align != null)
     {
-      StructureSelectionManager.getStructureSelectionManager(
-              Desktop.instance).addMappings(alignment.getCodonFrames());
+      ssm.addMappings(align.getCodonFrames());
+    }
+
+    /*
+     * replace mappings on our alignment
+     */
+    if (alignment != null && align != null)
+    {
+      alignment.setCodonFrames(align.getCodonFrames());
     }
   }
 
@@ -884,9 +909,8 @@ public class AlignViewport extends AlignmentViewport implements
     AlignmentUtils.mapProteinToCdna(protein, cdna);
 
     /*
-     * Create the AlignFrame for the added alignment. Note this will include the
-     * cDNA consensus annotation if it is protein (because the alignment holds
-     * mappings to nucleotide)
+     * Create the AlignFrame for the added alignment. If it is protein, mappings
+     * are registered with StructureSelectionManager as a side-effect.
      */
     AlignFrame newAlignFrame = new AlignFrame(al, AlignFrame.DEFAULT_WIDTH,
             AlignFrame.DEFAULT_HEIGHT);
@@ -921,18 +945,9 @@ public class AlignViewport extends AlignmentViewport implements
     if (openSplitPane)
     {
       al.alignAs(thisAlignment);
-      protein = openSplitFrame(newAlignFrame, thisAlignment,
-              protein.getCodonFrames());
+      protein = openSplitFrame(newAlignFrame, thisAlignment);
     }
 
-    /*
-     * Register the mappings (held on the protein alignment) with the
-     * StructureSelectionManager (for mouseover linking).
-     */
-    final StructureSelectionManager ssm = StructureSelectionManager
-            .getStructureSelectionManager(Desktop.instance);
-    ssm.addMappings(protein.getCodonFrames());
-
     return true;
   }
 
@@ -944,16 +959,14 @@ public class AlignViewport extends AlignmentViewport implements
    *          containing a new alignment to be shown
    * @param complement
    *          cdna/protein complement alignment to show in the other split half
-   * @param mappings
    * @return the protein alignment in the split frame
    */
   protected AlignmentI openSplitFrame(AlignFrame newAlignFrame,
-          AlignmentI complement, Set<AlignedCodonFrame> mappings)
+          AlignmentI complement)
   {
     /*
      * Make a new frame with a copy of the alignment we are adding to. If this
-     * is protein, the new frame will have a cDNA consensus annotation row
-     * added.
+     * is protein, the mappings to cDNA will be registered with StructureSelectionManager as a side-effect.
      */
     AlignFrame copyMe = new AlignFrame(complement,
             AlignFrame.DEFAULT_WIDTH, AlignFrame.DEFAULT_HEIGHT);
@@ -964,9 +977,6 @@ public class AlignViewport extends AlignmentViewport implements
             : newAlignFrame;
     final AlignFrame cdnaFrame = al.isNucleotide() ? newAlignFrame
             : copyMe;
-    AlignmentI protein = proteinFrame.viewport.getAlignment();
-    protein.setCodonFrames(mappings);
-
     cdnaFrame.setVisible(true);
     proteinFrame.setVisible(true);
     String linkedTitle = MessageManager
@@ -978,7 +988,7 @@ public class AlignViewport extends AlignmentViewport implements
     JInternalFrame splitFrame = new SplitFrame(cdnaFrame, proteinFrame);
     Desktop.addInternalFrame(splitFrame, linkedTitle, -1, -1);
 
-    return protein;
+    return proteinFrame.viewport.getAlignment();
   }
 
   public AnnotationColumnChooser getAnnotationColumnSelectionState()
index 6a95363..0e1811d 100644 (file)
@@ -265,10 +265,10 @@ public class Desktop extends jalview.jbgui.GDesktop implements
       delegate.openFrame(f);
     }
 
+    @Override
     public void resizeFrame(JComponent f, int newX, int newY, int newWidth,
             int newHeight)
     {
-      Rectangle b = desktop.getBounds();
       if (newY < 0)
       {
         newY = 0;
@@ -1301,7 +1301,17 @@ public class Desktop extends jalview.jbgui.GDesktop implements
     if (v_client != null)
     {
       // TODO clear binding to vamsas document objects on close_all
+    }
 
+    /*
+     * reset state of singleton objects as appropriate (clear down session state
+     * when all windows are closed)
+     */
+    StructureSelectionManager ssm = StructureSelectionManager
+            .getStructureSelectionManager(this);
+    if (ssm != null)
+    {
+      ssm.resetAll();
     }
   }
 
@@ -2287,6 +2297,7 @@ public class Desktop extends jalview.jbgui.GDesktop implements
       }
     }
 
+    @Override
     public void paintComponent(Graphics g)
     {
       if (showMemoryUsage && g != null && df != null)
index ac14b52..cf60490 100644 (file)
  */
 package jalview.structure;
 
+import jalview.analysis.AlignSeq;
+import jalview.api.StructureSelectionManagerProvider;
+import jalview.commands.CommandI;
+import jalview.commands.EditCommand;
+import jalview.commands.OrderCommand;
+import jalview.datamodel.AlignedCodonFrame;
+import jalview.datamodel.AlignmentAnnotation;
+import jalview.datamodel.AlignmentI;
+import jalview.datamodel.Annotation;
+import jalview.datamodel.PDBEntry;
+import jalview.datamodel.SearchResults;
+import jalview.datamodel.SequenceI;
+import jalview.io.AppletFormatAdapter;
+import jalview.util.MappingUtils;
+import jalview.util.MessageManager;
+
 import java.io.PrintStream;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -37,22 +53,6 @@ import MCview.Atom;
 import MCview.PDBChain;
 import MCview.PDBfile;
 
-import jalview.analysis.AlignSeq;
-import jalview.api.StructureSelectionManagerProvider;
-import jalview.commands.CommandI;
-import jalview.commands.EditCommand;
-import jalview.commands.OrderCommand;
-import jalview.datamodel.AlignedCodonFrame;
-import jalview.datamodel.AlignmentAnnotation;
-import jalview.datamodel.AlignmentI;
-import jalview.datamodel.Annotation;
-import jalview.datamodel.PDBEntry;
-import jalview.datamodel.SearchResults;
-import jalview.datamodel.SequenceI;
-import jalview.io.AppletFormatAdapter;
-import jalview.util.MappingUtils;
-import jalview.util.MessageManager;
-
 public class StructureSelectionManager
 {
   public final static String NEWLINE = System.lineSeparator();
@@ -76,7 +76,7 @@ public class StructureSelectionManager
    * Reference counters for the above mappings. Remove mappings when ref count
    * goes to zero.
    */
-  Map<AlignedCodonFrame, Integer> seqMappingRefCounts = new HashMap<AlignedCodonFrame, Integer>();
+  private Map<AlignedCodonFrame, Integer> seqMappingRefCounts = new HashMap<AlignedCodonFrame, Integer>();
 
   private List<CommandListener> commandListeners = new ArrayList<CommandListener>();
 
@@ -721,13 +721,16 @@ public class StructureSelectionManager
               {
                 results = MappingUtils.buildSearchResults(seq, index,
                         seqmappings);
-              }
+               }
               if (handlingVamsasMo)
               {
                 results.addResult(seq, index, index);
 
               }
-              seqListener.highlightSequence(results);
+              if (!results.isEmpty())
+              {
+                seqListener.highlightSequence(results);
+              }
             }
           }
         }
@@ -944,6 +947,10 @@ public class StructureSelectionManager
       {
         seqmappings.remove(acf);
         seqMappingRefCounts.remove(acf);
+        if (seqmappings.isEmpty())
+        { // debug
+          System.out.println("All mappings removed");
+        }
       }
     }
   }
@@ -985,6 +992,50 @@ public class StructureSelectionManager
     }
   }
 
+  /**
+   * Resets this object to its initial state by removing all registered
+   * listeners, codon mappings, PDB file mappings
+   */
+  public void resetAll()
+  {
+    if (mappings != null)
+    {
+      mappings.clear();
+    }
+    if (seqmappings != null)
+    {
+      seqmappings.clear();
+    }
+    if (seqMappingRefCounts != null)
+    {
+      seqMappingRefCounts.clear();
+    }
+    if (sel_listeners != null)
+    {
+      sel_listeners.clear();
+    }
+    if (listeners != null)
+    {
+      listeners.clear();
+    }
+    if (commandListeners != null)
+    {
+      commandListeners.clear();
+    }
+    if (view_listeners != null)
+    {
+      view_listeners.clear();
+    }
+    if (pdbFileNameId != null)
+    {
+      pdbFileNameId.clear();
+    }
+    if (pdbIdFileName != null)
+    {
+      pdbIdFileName.clear();
+    }
+  }
+
   public void addSelectionListener(SelectionListener selecter)
   {
     if (!sel_listeners.contains(selecter))
@@ -1136,4 +1187,20 @@ public class StructureSelectionManager
     }
     return null;
   }
+
+  /**
+   * Returns the reference count for a mapping
+   * 
+   * @param acf
+   * @return
+   */
+  protected int getMappingReferenceCount(AlignedCodonFrame acf)
+  {
+    if (seqMappingRefCounts == null)
+    {
+      return 0;
+    }
+    Integer count = seqMappingRefCounts.get(acf);
+    return (count == null ? 0 : count.intValue());
+  }
 }
index e3612a5..0162308 100644 (file)
@@ -33,8 +33,7 @@ public class StructureSelectionManagerTest
     ssm.addMapping(acf1);
     assertEquals(1, ssm.seqmappings.size());
     assertTrue(ssm.seqmappings.contains(acf1));
-    assertEquals(1, ssm.seqMappingRefCounts.size());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf1).intValue());
+    assertEquals(1, ssm.getMappingReferenceCount(acf1));
 
     /*
      * A second mapping.
@@ -43,9 +42,8 @@ public class StructureSelectionManagerTest
     assertEquals(2, ssm.seqmappings.size());
     assertTrue(ssm.seqmappings.contains(acf1));
     assertTrue(ssm.seqmappings.contains(acf2));
-    assertEquals(2, ssm.seqMappingRefCounts.size());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf1).intValue());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf2).intValue());
+    assertEquals(1, ssm.getMappingReferenceCount(acf1));
+    assertEquals(1, ssm.getMappingReferenceCount(acf2));
 
     /*
      * A second reference to the first mapping.
@@ -54,9 +52,8 @@ public class StructureSelectionManagerTest
     assertEquals(2, ssm.seqmappings.size());
     assertTrue(ssm.seqmappings.contains(acf1));
     assertTrue(ssm.seqmappings.contains(acf2));
-    assertEquals(2, ssm.seqMappingRefCounts.size());
-    assertEquals(2, ssm.seqMappingRefCounts.get(acf1).intValue());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf2).intValue());
+    assertEquals(2, ssm.getMappingReferenceCount(acf1));
+    assertEquals(1, ssm.getMappingReferenceCount(acf2));
   }
 
   @Test(groups ={ "Functional" })
@@ -83,10 +80,9 @@ public class StructureSelectionManagerTest
     assertTrue(ssm.seqmappings.contains(acf1));
     assertTrue(ssm.seqmappings.contains(acf2));
     assertTrue(ssm.seqmappings.contains(acf3));
-    assertEquals(3, ssm.seqMappingRefCounts.size());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf1).intValue());
-    assertEquals(2, ssm.seqMappingRefCounts.get(acf2).intValue());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf3).intValue());
+    assertEquals(1, ssm.getMappingReferenceCount(acf1));
+    assertEquals(2, ssm.getMappingReferenceCount(acf2));
+    assertEquals(1, ssm.getMappingReferenceCount(acf3));
   }
 
   @Test(groups ={ "Functional" })
@@ -94,15 +90,16 @@ public class StructureSelectionManagerTest
   {
     AlignedCodonFrame acf1 = new AlignedCodonFrame();
     AlignedCodonFrame acf2 = new AlignedCodonFrame();
-    ssm.addMapping(acf1);
 
     /*
      * Add one and remove it.
      */
+    ssm.addMapping(acf1);
     ssm.removeMapping(acf1);
     ssm.removeMapping(acf2);
     assertEquals(0, ssm.seqmappings.size());
-    assertEquals(0, ssm.seqMappingRefCounts.size());
+    assertEquals(0, ssm.getMappingReferenceCount(acf1));
+    assertEquals(0, ssm.getMappingReferenceCount(acf2));
 
     /*
      * Add one twice and remove it once.
@@ -114,9 +111,8 @@ public class StructureSelectionManagerTest
     assertEquals(2, ssm.seqmappings.size());
     assertTrue(ssm.seqmappings.contains(acf1));
     assertTrue(ssm.seqmappings.contains(acf2));
-    assertEquals(2, ssm.seqMappingRefCounts.size());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf1).intValue());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf2).intValue());
+    assertEquals(1, ssm.getMappingReferenceCount(acf1));
+    assertEquals(1, ssm.getMappingReferenceCount(acf2));
 
     /*
      * Remove both once more to clear the set.
@@ -124,7 +120,8 @@ public class StructureSelectionManagerTest
     ssm.removeMapping(acf1);
     ssm.removeMapping(acf2);
     assertEquals(0, ssm.seqmappings.size());
-    assertEquals(0, ssm.seqMappingRefCounts.size());
+    assertEquals(0, ssm.getMappingReferenceCount(acf1));
+    assertEquals(0, ssm.getMappingReferenceCount(acf2));
   }
 
   @Test(groups ={ "Functional" })
@@ -159,10 +156,9 @@ public class StructureSelectionManagerTest
     assertTrue(ssm.seqmappings.contains(acf1));
     assertTrue(ssm.seqmappings.contains(acf2));
     assertTrue(ssm.seqmappings.contains(acf3));
-    assertEquals(3, ssm.seqMappingRefCounts.size());
-    assertEquals(2, ssm.seqMappingRefCounts.get(acf1).intValue());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf2).intValue());
-    assertEquals(1, ssm.seqMappingRefCounts.get(acf3).intValue());
+    assertEquals(2, ssm.getMappingReferenceCount(acf1));
+    assertEquals(1, ssm.getMappingReferenceCount(acf2));
+    assertEquals(1, ssm.getMappingReferenceCount(acf3));
 
     /*
      * Remove one ref each to acf2, acf3 - they are removed
@@ -170,7 +166,6 @@ public class StructureSelectionManagerTest
     ssm.removeMappings(set2);
     assertEquals(1, ssm.seqmappings.size());
     assertTrue(ssm.seqmappings.contains(acf1));
-    assertEquals(1, ssm.seqMappingRefCounts.size());
-    assertEquals(2, ssm.seqMappingRefCounts.get(acf1).intValue());
+    assertEquals(2, ssm.getMappingReferenceCount(acf1));
   }
 }