JAL-2744 refactor PopupMenu constructor to enable adding feature details

diff --git a/src/jalview/gui/IdPanel.java b/src/jalview/gui/IdPanel.java
index 3cc0ed3..f0aefb1 100755 (executable)
--- a/src/jalview/gui/IdPanel.java
+++ b/src/jalview/gui/IdPanel.java
@@ -331,7 +331,8 @@ public class IdPanel extends JPanel
      *  and any non-positional features
      */
     List<String> nlinks = Preferences.sequenceUrlLinks.getLinksForMenu();
-    for (SequenceFeature sf : sq.getFeatures().getNonPositionalFeatures())
+    List<SequenceFeature> features = sq.getFeatures().getNonPositionalFeatures();
+    for (SequenceFeature sf : features)
     {
       if (sf.links != null)
       {
@@ -342,7 +343,7 @@ public class IdPanel extends JPanel
       }
     }
 
-    PopupMenu pop = new PopupMenu(alignPanel, sq, nlinks,
+    PopupMenu pop = new PopupMenu(alignPanel, sq, features,
             Preferences.getGroupURLLinks());
     pop.show(this, e.getX(), e.getY());
   }

diff --git a/src/jalview/gui/PopupMenu.java b/src/jalview/gui/PopupMenu.java
index 846ba64..21db7d7 100644 (file)
--- a/src/jalview/gui/PopupMenu.java
+++ b/src/jalview/gui/PopupMenu.java
@@ -34,7 +34,6 @@ import jalview.datamodel.Annotation;
 import jalview.datamodel.DBRefEntry;
 import jalview.datamodel.HiddenColumns;
 import jalview.datamodel.PDBEntry;
-import jalview.datamodel.Sequence;
 import jalview.datamodel.SequenceFeature;
 import jalview.datamodel.SequenceGroup;
 import jalview.datamodel.SequenceI;
@@ -176,25 +175,31 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
    * Creates a new PopupMenu object.
    * 
    * @param ap
-   *          DOCUMENT ME!
    * @param seq
-   *          DOCUMENT ME!
+   * @param features
+   *          non-positional features (for seq not null), or positional features
+   *          at residue (for seq equal to null)
    */
-  public PopupMenu(final AlignmentPanel ap, Sequence seq,
-          List<String> links)
+  public PopupMenu(final AlignmentPanel ap, SequenceI seq,
+          List<SequenceFeature> features)
   {
-    this(ap, seq, links, null);
+    this(ap, seq, features, null);
   }
 
   /**
+   * Constructor
    * 
-   * @param ap
+   * @param alignPanel
    * @param seq
-   * @param links
+   *          the sequence under the cursor if in the Id panel, null if in the
+   *          sequence panel
+   * @param features
+   *          non-positional features if in the Id panel, features at the
+   *          clicked residue if in the sequence panel
    * @param groupLinks
    */
-  public PopupMenu(final AlignmentPanel ap, final SequenceI seq,
-          List<String> links, List<String> groupLinks)
+  public PopupMenu(final AlignmentPanel alignPanel, final SequenceI seq,
+          List<SequenceFeature> features, List<String> groupLinks)
   {
     // /////////////////////////////////////////////////////////
     // If this is activated from the sequence panel, the user may want to
@@ -202,7 +207,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
     //
     // If from the IDPanel, we must display the sequence menu
     // ////////////////////////////////////////////////////////
-    this.ap = ap;
+    this.ap = alignPanel;
     sequence = seq;
 
     for (String ff : FileFormats.getInstance().getWritableFormats(true))
@@ -237,9 +242,9 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
     /*
      * And repeat for the current selection group (if there is one):
      */
-    final List<SequenceI> selectedGroup = (ap.av.getSelectionGroup() == null
+    final List<SequenceI> selectedGroup = (alignPanel.av.getSelectionGroup() == null
             ? Collections.<SequenceI> emptyList()
-            : ap.av.getSelectionGroup().getSequences());
+            : alignPanel.av.getSelectionGroup().getSequences());
     buildAnnotationTypesMenus(groupShowAnnotationsMenu,
             groupHideAnnotationsMenu, selectedGroup);
     configureReferenceAnnotationsMenu(groupAddReferenceAnnotations,
@@ -257,7 +262,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
     if (seq != null)
     {
       sequenceMenu.setText(sequence.getName());
-      if (seq == ap.av.getAlignment().getSeqrep())
+      if (seq == alignPanel.av.getAlignment().getSeqrep())
       {
         makeReferenceSeq.setText(
                 MessageManager.getString("action.unmark_as_reference"));
@@ -268,7 +273,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
                 MessageManager.getString("action.set_as_reference"));
       }
 
-      if (!ap.av.getAlignment().isNucleotide())
+      if (!alignPanel.av.getAlignment().isNucleotide())
       {
         remove(rnaStructureMenu);
       }
@@ -279,7 +284,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
          * add menu items to 2D-render any alignment or sequence secondary
          * structure annotation
          */
-        AlignmentAnnotation[] aas = ap.av.getAlignment()
+        AlignmentAnnotation[] aas = alignPanel.av.getAlignment()
                 .getAlignmentAnnotation();
         if (aas != null)
         {
@@ -299,7 +304,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
                 @Override
                 public void actionPerformed(ActionEvent e)
                 {
-                  new AppVarna(seq, aa, ap);
+                  new AppVarna(seq, aa, alignPanel);
                 }
               });
               rnaStructureMenu.add(menuItem);
@@ -328,7 +333,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
                 public void actionPerformed(ActionEvent e)
                 {
                   // TODO: VARNA does'nt print gaps in the sequence
-                  new AppVarna(seq, aa, ap);
+                  new AppVarna(seq, aa, alignPanel);
                 }
               });
               rnaStructureMenu.add(menuItem);
@@ -353,8 +358,8 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
       });
       add(menuItem);
 
-      if (ap.av.getSelectionGroup() != null
-              && ap.av.getSelectionGroup().getSize() > 1)
+      if (alignPanel.av.getSelectionGroup() != null
+              && alignPanel.av.getSelectionGroup().getSize() > 1)
       {
         menuItem = new JMenuItem(MessageManager
                 .formatMessage("label.represent_group_with", new Object[]
@@ -370,12 +375,12 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
         sequenceMenu.add(menuItem);
       }
 
-      if (ap.av.hasHiddenRows())
+      if (alignPanel.av.hasHiddenRows())
       {
-        final int index = ap.av.getAlignment().findIndex(seq);
+        final int index = alignPanel.av.getAlignment().findIndex(seq);
 
-        if (ap.av.adjustForHiddenSeqs(index)
-                - ap.av.adjustForHiddenSeqs(index - 1) > 1)
+        if (alignPanel.av.adjustForHiddenSeqs(index)
+                - alignPanel.av.adjustForHiddenSeqs(index - 1) > 1)
         {
           menuItem = new JMenuItem(
                   MessageManager.getString("action.reveal_sequences"));
@@ -384,10 +389,10 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
             @Override
             public void actionPerformed(ActionEvent e)
             {
-              ap.av.showSequence(index);
-              if (ap.overviewPanel != null)
+              alignPanel.av.showSequence(index);
+              if (alignPanel.overviewPanel != null)
               {
-                ap.overviewPanel.updateOverviewImage();
+                alignPanel.overviewPanel.updateOverviewImage();
               }
             }
           });
@@ -396,7 +401,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
       }
     }
     // for the case when no sequences are even visible
-    if (ap.av.hasHiddenRows())
+    if (alignPanel.av.hasHiddenRows())
     {
       {
         menuItem = new JMenuItem(
@@ -406,10 +411,10 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
           @Override
           public void actionPerformed(ActionEvent e)
           {
-            ap.av.showAllHiddenSeqs();
-            if (ap.overviewPanel != null)
+            alignPanel.av.showAllHiddenSeqs();
+            if (alignPanel.overviewPanel != null)
             {
-              ap.overviewPanel.updateOverviewImage();
+              alignPanel.overviewPanel.updateOverviewImage();
             }
           }
         });
@@ -418,9 +423,9 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
       }
     }
 
-    SequenceGroup sg = ap.av.getSelectionGroup();
+    SequenceGroup sg = alignPanel.av.getSelectionGroup();
     boolean isDefinedGroup = (sg != null)
-            ? ap.av.getAlignment().getGroups().contains(sg)
+            ? alignPanel.av.getAlignment().getGroups().contains(sg)
             : false;
 
     if (sg != null && sg.getSize() > 0)
@@ -458,7 +463,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
       Hashtable<String, PDBEntry> pdbe = new Hashtable<>(), reppdb = new Hashtable<>();
 
       SequenceI sqass = null;
-      for (SequenceI sq : ap.av.getSequenceSelection())
+      for (SequenceI sq : alignPanel.av.getSequenceSelection())
       {
         Vector<PDBEntry> pes = sq.getDatasetSequence().getAllPDBEntries();
         if (pes != null && pes.size() > 0)
@@ -508,24 +513,50 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
       rnaStructureMenu.setVisible(false);
     }
 
-    if (links != null && links.size() > 0)
-    {
-      addFeatureLinks(seq, links);
-    }
+    addLinks(seq, features);
   }
 
   /**
    * Adds a 'Link' menu item with a sub-menu item for each hyperlink provided.
+   * When seq is not null, these are links for the sequence id, which may be to
+   * external web sites for the sequence accession, and/or links embedded in
+   * non-positional features. When seq is null, only links embedded in the
+   * provided features are added.
    * 
    * @param seq
-   * @param links
+   * @param features
    */
-  void addFeatureLinks(final SequenceI seq, List<String> links)
+  void addLinks(final SequenceI seq, List<SequenceFeature> features)
   {
     JMenu linkMenu = new JMenu(MessageManager.getString("action.link"));
+
+    List<String> nlinks = null;
+    if (seq != null)
+    {
+      nlinks = Preferences.sequenceUrlLinks.getLinksForMenu();
+    }
+    else
+    {
+      nlinks = new ArrayList<>();
+    }
+
+    if (features != null)
+    {
+      for (SequenceFeature sf : features)
+      {
+        if (sf.links != null)
+        {
+          for (String link : sf.links)
+          {
+            nlinks.add(link);
+          }
+        }
+      }
+    }
+
     Map<String, List<String>> linkset = new LinkedHashMap<>();
 
-    for (String link : links)
+    for (String link : nlinks)
     {
       UrlLink urlLink = null;
       try
@@ -548,25 +579,18 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
 
     addshowLinks(linkMenu, linkset.values());
 
-    // disable link menu if there are no valid entries
+    // only add link menu if it has entries
     if (linkMenu.getItemCount() > 0)
     {
-      linkMenu.setEnabled(true);
-    }
-    else
-    {
-      linkMenu.setEnabled(false);
-    }
-
-    if (sequence != null)
-    {
-      sequenceMenu.add(linkMenu);
-    }
-    else
-    {
-      add(linkMenu);
+      if (sequence != null)
+      {
+        sequenceMenu.add(linkMenu);
+      }
+      else
+      {
+        add(linkMenu);
+      }
     }
-
   }
 
   /**

diff --git a/src/jalview/gui/SeqPanel.java b/src/jalview/gui/SeqPanel.java
index e99e577..ad80a3e 100644 (file)
--- a/src/jalview/gui/SeqPanel.java
+++ b/src/jalview/gui/SeqPanel.java
@@ -59,7 +59,6 @@ import java.awt.event.MouseListener;
 import java.awt.event.MouseMotionListener;
 import java.awt.event.MouseWheelEvent;
 import java.awt.event.MouseWheelListener;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
@@ -1782,21 +1781,10 @@ public class SeqPanel extends JPanel
     final int column = findColumn(evt);
     final int seq = findSeq(evt);
     SequenceI sequence = av.getAlignment().getSequenceAt(seq);
-    List<SequenceFeature> allFeatures = ap.getFeatureRenderer()
+    List<SequenceFeature> features = ap.getFeatureRenderer()
             .findFeaturesAtColumn(sequence, column + 1);
-    List<String> links = new ArrayList<>();
-    for (SequenceFeature sf : allFeatures)
-    {
-      if (sf.links != null)
-      {
-        for (String link : sf.links)
-        {
-          links.add(link);
-        }
-      }
-    }
 
-    PopupMenu pop = new PopupMenu(ap, null, links);
+    PopupMenu pop = new PopupMenu(ap, null, features);
     pop.show(this, evt.getX(), evt.getY());
   }
 

diff --git a/test/jalview/gui/PopupMenuTest.java b/test/jalview/gui/PopupMenuTest.java
index 335240b..40e624d 100644 (file)
--- a/test/jalview/gui/PopupMenuTest.java
+++ b/test/jalview/gui/PopupMenuTest.java
@@ -26,21 +26,26 @@ import static org.testng.AssertJUnit.assertEquals;
 import static org.testng.AssertJUnit.assertFalse;
 import static org.testng.AssertJUnit.assertTrue;
 
+import jalview.bin.Cache;
 import jalview.datamodel.AlignmentAnnotation;
 import jalview.datamodel.AlignmentI;
 import jalview.datamodel.Annotation;
 import jalview.datamodel.DBRefEntry;
 import jalview.datamodel.DBRefSource;
-import jalview.datamodel.Sequence;
+import jalview.datamodel.SequenceFeature;
 import jalview.datamodel.SequenceI;
 import jalview.io.DataSourceType;
 import jalview.io.FileFormat;
 import jalview.io.FormatAdapter;
+import jalview.urls.api.UrlProviderFactoryI;
+import jalview.urls.desktop.DesktopUrlProviderFactory;
 import jalview.util.MessageManager;
+import jalview.util.UrlConstants;
 
 import java.awt.Component;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import javax.swing.JMenu;
@@ -80,6 +85,25 @@ public class PopupMenuTest
   @BeforeMethod(alwaysRun = true)
   public void setUp() throws IOException
   {
+    Cache.loadProperties("test/jalview/io/testProps.jvprops");
+    String inMenuString = ("EMBL-EBI Search | http://www.ebi.ac.uk/ebisearch/search.ebi?db=allebi&query=$"
+            + SEQUENCE_ID
+            + "$"
+            + "|"
+            + "UNIPROT | http://www.uniprot.org/uniprot/$" + DB_ACCESSION + "$")
+            + "|"
+            + ("INTERPRO | http://www.ebi.ac.uk/interpro/entry/$"
+                    + DB_ACCESSION + "$")
+            + "|"
+            +
+            // Gene3D entry tests for case (in)sensitivity
+            ("Gene3D | http://gene3d.biochem.ucl.ac.uk/Gene3D/search?sterm=$"
+                    + DB_ACCESSION + "$&mode=protein");
+
+    UrlProviderFactoryI factory = new DesktopUrlProviderFactory(
+            UrlConstants.DEFAULT_LABEL, inMenuString, "");
+    Preferences.sequenceUrlLinks = factory.createUrlProvider();
+
     alignment = new FormatAdapter().readFile(TEST_DATA,
             DataSourceType.PASTE, FileFormat.Fasta);
     AlignFrame af = new AlignFrame(alignment, 700, 500);
@@ -495,17 +519,19 @@ public class PopupMenuTest
 
     // add all the dbrefs to the sequences: Uniprot 1 each, Interpro all 3 to
     // seq0, Gene3D to seq1
-    seqs.get(0).addDBRef(refs.get(0));
+    SequenceI seq = seqs.get(0);
+    seq.addDBRef(refs.get(0));
 
-    seqs.get(0).addDBRef(refs.get(1));
-    seqs.get(0).addDBRef(refs.get(2));
-    seqs.get(0).addDBRef(refs.get(3));
+    seq.addDBRef(refs.get(1));
+    seq.addDBRef(refs.get(2));
+    seq.addDBRef(refs.get(3));
     
     seqs.get(1).addDBRef(refs.get(4));
     seqs.get(1).addDBRef(refs.get(5));
     
     // get the Popup Menu for first sequence
-    testee = new PopupMenu(parentPanel, (Sequence) seqs.get(0), links);
+    List<SequenceFeature> noFeatures = Collections.<SequenceFeature> emptyList();
+    testee = new PopupMenu(parentPanel, seq, noFeatures);
     Component[] seqItems = testee.sequenceMenu.getMenuComponents();
     JMenu linkMenu = (JMenu) seqItems[6];
     Component[] linkItems = linkMenu.getMenuComponents();
@@ -519,15 +545,18 @@ public class PopupMenuTest
     // sequence id for each link should match corresponding DB accession id
     for (int i = 1; i < 4; i++)
     {
-      assertEquals(refs.get(i - 1).getSource(), ((JMenuItem) linkItems[i])
+      String msg = seq.getName() + " link[" + i + "]";
+      assertEquals(msg, refs.get(i - 1).getSource(),
+              ((JMenuItem) linkItems[i])
               .getText().split("\\|")[0]);
-      assertEquals(refs.get(i - 1).getAccessionId(),
+      assertEquals(msg, refs.get(i - 1).getAccessionId(),
               ((JMenuItem) linkItems[i])
               .getText().split("\\|")[1]);
     }
 
     // get the Popup Menu for second sequence
-    testee = new PopupMenu(parentPanel, (Sequence) seqs.get(1), links);
+    seq = seqs.get(1);
+    testee = new PopupMenu(parentPanel, seq, noFeatures);
     seqItems = testee.sequenceMenu.getMenuComponents();
     linkMenu = (JMenu) seqItems[6];
     linkItems = linkMenu.getMenuComponents();
@@ -541,9 +570,11 @@ public class PopupMenuTest
     // sequence id for each link should match corresponding DB accession id
     for (int i = 1; i < 3; i++)
     {
-      assertEquals(refs.get(i + 3).getSource(), ((JMenuItem) linkItems[i])
+      String msg = seq.getName() + " link[" + i + "]";
+      assertEquals(msg, refs.get(i + 3).getSource(),
+              ((JMenuItem) linkItems[i])
               .getText().split("\\|")[0].toUpperCase());
-      assertEquals(refs.get(i + 3).getAccessionId(),
+      assertEquals(msg, refs.get(i + 3).getAccessionId(),
               ((JMenuItem) linkItems[i]).getText().split("\\|")[1]);
     }
 
@@ -552,8 +583,7 @@ public class PopupMenuTest
     nomatchlinks.add("NOMATCH | http://www.uniprot.org/uniprot/$"
             + DB_ACCESSION + "$");
 
-    testee = new PopupMenu(parentPanel, (Sequence) seqs.get(0),
-            nomatchlinks);
+    testee = new PopupMenu(parentPanel, seq, noFeatures);
     seqItems = testee.sequenceMenu.getMenuComponents();
     linkMenu = (JMenu) seqItems[6];
     assertFalse(linkMenu.isEnabled());