From 43d326c3a7616aedecfce6f7980b3831ec25243a Mon Sep 17 00:00:00 2001 From: gmungoc Date: Mon, 17 Aug 2015 12:09:19 +0100 Subject: [PATCH] JAL-1829 JAL-1830 refactoring for correct mapping (de-)registration --- src/jalview/gui/AlignFrame.java | 27 +++-- src/jalview/gui/AlignViewport.java | 70 +++++++------ src/jalview/gui/Desktop.java | 13 ++- .../structure/StructureSelectionManager.java | 105 ++++++++++++++++---- .../structure/StructureSelectionManagerTest.java | 43 ++++---- 5 files changed, 174 insertions(+), 84 deletions(-) diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java index b37b442..83fb7c0 100644 --- a/src/jalview/gui/AlignFrame.java +++ b/src/jalview/gui/AlignFrame.java @@ -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 diff --git a/src/jalview/gui/AlignViewport.java b/src/jalview/gui/AlignViewport.java index 9ed953f..aedfe12 100644 --- a/src/jalview/gui/AlignViewport.java +++ b/src/jalview/gui/AlignViewport.java @@ -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 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() diff --git a/src/jalview/gui/Desktop.java b/src/jalview/gui/Desktop.java index 6a95363..0e1811d 100644 --- a/src/jalview/gui/Desktop.java +++ b/src/jalview/gui/Desktop.java @@ -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) diff --git a/src/jalview/structure/StructureSelectionManager.java b/src/jalview/structure/StructureSelectionManager.java index ac14b52..cf60490 100644 --- a/src/jalview/structure/StructureSelectionManager.java +++ b/src/jalview/structure/StructureSelectionManager.java @@ -20,6 +20,22 @@ */ 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 seqMappingRefCounts = new HashMap(); + private Map seqMappingRefCounts = new HashMap(); private List commandListeners = new ArrayList(); @@ -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()); + } } diff --git a/test/jalview/structure/StructureSelectionManagerTest.java b/test/jalview/structure/StructureSelectionManagerTest.java index e3612a5..0162308 100644 --- a/test/jalview/structure/StructureSelectionManagerTest.java +++ b/test/jalview/structure/StructureSelectionManagerTest.java @@ -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)); } } -- 1.7.10.2