Code tidy up. Unit test passes.

diff --git a/src/jalview/util/UrlLink.java b/src/jalview/util/UrlLink.java
index a8c0b32..f9dda09 100644 (file)
--- a/src/jalview/util/UrlLink.java
+++ b/src/jalview/util/UrlLink.java
@@ -63,99 +63,29 @@ public class UrlLink
 
       sep = parseTargetAndLabel(sep, psqid, link);
 
-      // Parse URL : Whole URL string first
-      int p;
-      url_prefix = link.substring(sep + 1, psqid);
-      if (link.indexOf("$" + SEQUENCE_ID + "=/") == psqid
-              && (p = link.indexOf("/=$", psqid + 14)) > psqid + 14)
-      {
-        // Extract Regex and suffix
-        url_suffix = link.substring(p + 3);
-        regexReplace = link.substring(psqid + 14, p);
-        try
-        {
-          com.stevesoft.pat.Regex rg = com.stevesoft.pat.Regex.perlCode("/"
-                  + regexReplace + "/");
-          if (rg == null)
-          {
-            invalidMessage = "Invalid Regular Expression : '"
-                    + regexReplace + "'\n";
-          }
-        } catch (Exception e)
-        {
-          invalidMessage = "Invalid Regular Expression : '" + regexReplace
-                  + "'\n";
-        }
-      }
-      else
-      {
-        regexReplace = null;
-        // verify format is really correct.
-        if (link.indexOf("$" + SEQUENCE_ID + "$") == psqid)
-        {
-          url_suffix = link.substring(psqid + 13);
-          regexReplace = null;
-        }
-        else
-        {
-          invalidMessage = "Warning: invalid regex structure for URL link : "
-                  + link;
-        }
-      }
+      parseUrl(link, SEQUENCE_ID, psqid, sep);
     }
     else if (nsqid > -1)
     {
       dynamic = true;
       sep = parseTargetAndLabel(sep, nsqid, link);
 
-      int p;
-      url_prefix = link.substring(sep + 1, nsqid);
-      if (link.indexOf("$" + SEQUENCE_NAME + "=/") == nsqid
-              && (p = link.indexOf("/=$", nsqid + 16)) > nsqid + 16)
-      {
-        // Extract Regex and suffix
-        url_suffix = link.substring(p + 3);
-        regexReplace = link.substring(nsqid + 16, p);
-        try
-        {
-          com.stevesoft.pat.Regex rg = com.stevesoft.pat.Regex.perlCode("/"
-                  + regexReplace + "/");
-          if (rg == null)
-          {
-            invalidMessage = "Invalid Regular Expression : '"
-                    + regexReplace + "'\n";
-          }
-        } catch (Exception e)
-        {
-          invalidMessage = "Invalid Regular Expression : '" + regexReplace
-                  + "'\n";
-        }
-      }
-      else
-      {
-        regexReplace = null;
-        // verify format is really correct.
-        if (link.indexOf("$" + SEQUENCE_NAME + "$") == nsqid)
-        {
-          url_suffix = link.substring(nsqid + 15);
-          regexReplace = null;
-        }
-        else
-        {
-          invalidMessage = "Warning: invalid regex structure for URL link : "
-                  + link;
-        }
-      }
+      parseUrl(link, SEQUENCE_NAME, nsqid, sep);
     }
     else
     {
       target = link.substring(0, sep);
-      label = link.substring(0, sep = link.lastIndexOf("|"));
+      sep = link.lastIndexOf("|");
+      label = link.substring(0, sep);
       url_prefix = link.substring(sep + 1);
       regexReplace = null; // implies we trim any prefix if necessary //
       // regexReplace=".*\\|?(.*)";
       url_suffix = null;
     }
+
+    label = label.trim();
+    target = target.trim();
+    target = target.toUpperCase(); // DBRefEntry uppercases DB names
   }
 
   /**
@@ -340,9 +270,20 @@ public class UrlLink
 
   }
 
-  private int parseTargetAndLabel(int sep, int psqid, String link)
+  /**
+   * 
+   * @param firstSep
+   *          Location of first occurrence of separator in link string
+   * @param psqid
+   *          Position of sequence id or name in link string
+   * @param link
+   *          Link string containing database name and url
+   * @return Position of last separator symbol prior to any regex symbols
+   */
+  protected int parseTargetAndLabel(int firstSep, int psqid, String link)
   {
-    int p = sep;
+    int p = firstSep;
+    int sep = firstSep;
     do
     {
       sep = p;
@@ -368,6 +309,72 @@ public class UrlLink
     return sep;
   }
 
+  /**
+   * Parse the URL part of the link string
+   * 
+   * @param link
+   *          Link string containing database name and url
+   * @param varName
+   *          Name of variable in url string (e.g. SEQUENCE_ID, SEQUENCE_NAME)
+   * @param sqidPos
+   *          Position of id or name in link string
+   * @param sep
+   *          Position of separator in link string
+   */
+  protected void parseUrl(String link, String varName, int sqidPos, int sep)
+  {
+    url_prefix = link.substring(sep + 1, sqidPos);
+
+    // delimiter at start of regex: e.g. $SEQUENCE_ID=/
+    String startDelimiter = "$" + varName + "=/";
+
+    // delimiter at end of regex: /=$
+    String endDelimiter = "/=$";
+
+    int startLength = startDelimiter.length();
+
+    // Parse URL : Whole URL string first
+    int p = link.indexOf(endDelimiter, sqidPos + startLength);
+
+    if (link.indexOf(startDelimiter) == sqidPos
+            && (p > sqidPos + startLength))
+    {
+      // Extract Regex and suffix
+      url_suffix = link.substring(p + endDelimiter.length());
+      regexReplace = link.substring(sqidPos + startLength, p);
+      try
+      {
+        com.stevesoft.pat.Regex rg = com.stevesoft.pat.Regex.perlCode("/"
+                + regexReplace + "/");
+        if (rg == null)
+        {
+          invalidMessage = "Invalid Regular Expression : '" + regexReplace
+                  + "'\n";
+        }
+      } catch (Exception e)
+      {
+        invalidMessage = "Invalid Regular Expression : '" + regexReplace
+                + "'\n";
+      }
+    }
+    else
+    {
+      // no regex
+      regexReplace = null;
+      // verify format is really correct.
+      if (link.indexOf("$" + varName + "$") == sqidPos)
+      {
+        url_suffix = link.substring(sqidPos + startLength - 1);
+        regexReplace = null;
+      }
+      else
+      {
+        invalidMessage = "Warning: invalid regex structure for URL link : "
+                + link;
+      }
+    }
+  }
+
   private static void testUrls(UrlLink ul, String idstring, String[] urls)
   {
 

diff --git a/test/jalview/gui/PopupMenuTest.java b/test/jalview/gui/PopupMenuTest.java
index fe52e92..b4e8629 100644 (file)
--- a/test/jalview/gui/PopupMenuTest.java
+++ b/test/jalview/gui/PopupMenuTest.java
@@ -458,7 +458,7 @@ public class PopupMenuTest
     List<DBRefEntry> refs = new ArrayList<DBRefEntry>();
 
     // links as might be added into Preferences | Connections dialog
-    links.add("EMBL-EBI Search | http://www.ebi.ac.uk/ebisearch/search.ebi?db=allebi&query=$SEQUENCE_ID$");
+    links.add("EMBL-EBI Search | http://www.ebi.ac.uk/ebisearch/search.ebi?db=allebi&query=$SEQUENCE_NAME$");
     links.add("UNIPROT | http://www.uniprot.org/uniprot/$SEQUENCE_ID$");
     links.add("INTERPRO | http://www.ebi.ac.uk/interpro/entry/$SEQUENCE_ID$");
     // Gene3D entry tests for case (in)sensitivity
@@ -500,10 +500,11 @@ 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).getSource(), ((JMenuItem) linkItems[i])
-              .getText().split(" \\|")[0]);
-      assertEquals(refs.get(i).getAccessionId(), ((JMenuItem) linkItems[i])
-              .getText().split(" \\|")[1]);
+      assertEquals(refs.get(i - 1).getSource(), ((JMenuItem) linkItems[i])
+              .getText().split("\\|")[0]);
+      assertEquals(refs.get(i - 1).getAccessionId(),
+              ((JMenuItem) linkItems[i])
+              .getText().split("\\|")[1]);
     }
 
     // get the Popup Menu for second sequence
@@ -519,12 +520,12 @@ public class PopupMenuTest
     assertEquals("EMBL-EBI Search", ((JMenuItem) linkItems[0]).getText());
 
     // sequence id for each link should match corresponding DB accession id
-    for (int i = 5; i < 7; i++)
+    for (int i = 1; i < 3; i++)
     {
-      assertEquals(refs.get(i).getSource(), ((JMenuItem) linkItems[i - 4])
-              .getText().split(" \\|")[0]);
-      assertEquals(refs.get(i).getAccessionId(),
-              ((JMenuItem) linkItems[i - 4]).getText().split(" \\|")[1]);
+      assertEquals(refs.get(i + 3).getSource(), ((JMenuItem) linkItems[i])
+              .getText().split("\\|")[0].toUpperCase());
+      assertEquals(refs.get(i + 3).getAccessionId(),
+              ((JMenuItem) linkItems[i]).getText().split("\\|")[1]);
     }