From: kiramt <k.mourao@dundee.ac.uk>
Date: Tue, 10 Jan 2017 13:40:32 +0000 (+0000)
Subject: JAL-2316 Separate label/db name in UrlProviders. Unit test updates.
X-Git-Tag: Release_2_10_3b1~346^2~9
X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=64f19a3315b24ae0084cdc5ca8d47c9485040588

JAL-2316 Separate label/db name in UrlProviders. Unit test updates.
---

diff --git a/src/jalview/urls/CustomUrlProvider.java b/src/jalview/urls/CustomUrlProvider.java
index b10d816..07f4068 100644
--- a/src/jalview/urls/CustomUrlProvider.java
+++ b/src/jalview/urls/CustomUrlProvider.java
@@ -37,7 +37,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.StringTokenizer;
-import java.util.Vector;
 
 /**
  * 
@@ -167,15 +166,15 @@ public class CustomUrlProvider extends UrlProviderImpl
     {
       urls.remove(SRS_LABEL);
       UrlLink link = new UrlLink(UrlConstants.DEFAULT_STRING);
-      link.setDescription(UrlConstants.DEFAULT_LABEL);
+      link.setLabel(UrlConstants.DEFAULT_LABEL);
       urls.put(UrlConstants.DEFAULT_LABEL, link);
     }
   }
 
   @Override
-  public Vector<String> getLinksForMenu()
+  public List<String> getLinksForMenu()
   {
-    Vector<String> links = new Vector<String>();
+    List<String> links = new ArrayList<String>();
     Iterator<Map.Entry<String, UrlLink>> it = selectedUrls.entrySet()
             .iterator();
     while (it.hasNext())
@@ -298,14 +297,14 @@ public class CustomUrlProvider extends UrlProviderImpl
         if (link.getIsSelected())
         {
           selurls.put(link.getId(),
-                  new UrlLink(link.getName(), link.getUrl(), link.getName()));
+                  new UrlLink(link.getDescription(), link.getUrl(), link.getDescription()));
         }
         else
         {
           unselurls
                   .put(link.getId(),
-                          new UrlLink(link.getName(), link.getUrl(), link
-                                  .getName()));
+                          new UrlLink(link.getDescription(), link.getUrl(), link
+                                  .getDescription()));
         }
         // sort out primary and selected ids
         if (link.getIsPrimary())
@@ -327,7 +326,7 @@ public class CustomUrlProvider extends UrlProviderImpl
             && (!selectedUrls.containsKey(UrlConstants.DEFAULT_LABEL)))
     {
       UrlLink link = new UrlLink(UrlConstants.DEFAULT_STRING);
-      link.setDescription(UrlConstants.DEFAULT_LABEL);
+      link.setLabel(UrlConstants.DEFAULT_LABEL);
       selectedUrls.put(UrlConstants.DEFAULT_LABEL, link);
     }
     primaryUrl = UrlConstants.DEFAULT_LABEL;
diff --git a/src/jalview/urls/IdentifiersUrlProvider.java b/src/jalview/urls/IdentifiersUrlProvider.java
index 0219b54..a966b07 100644
--- a/src/jalview/urls/IdentifiersUrlProvider.java
+++ b/src/jalview/urls/IdentifiersUrlProvider.java
@@ -35,7 +35,6 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.StringTokenizer;
-import java.util.Vector;
 
 import org.json.simple.JSONArray;
 import org.json.simple.JSONObject;
@@ -170,12 +169,12 @@ public class IdentifiersUrlProvider extends UrlProviderImpl
   }
 
   @Override
-  public Vector<String> getLinksForMenu()
+  public List<String> getLinksForMenu()
   {
-    Vector<String> links = new Vector<String>();
+    List<String> links = new ArrayList<String>();
     for (String key : selectedUrls)
     {
-      links.add(urls.get(key).toString());
+      links.add(urls.get(key).toStringWithTarget());
     }
     return links;
   }
diff --git a/src/jalview/urls/UrlLinkDisplay.java b/src/jalview/urls/UrlLinkDisplay.java
index 814370e..0eabff7 100644
--- a/src/jalview/urls/UrlLinkDisplay.java
+++ b/src/jalview/urls/UrlLinkDisplay.java
@@ -87,14 +87,14 @@ public class UrlLinkDisplay
     return id;
   }
 
-  public String getName()
+  public String getDescription()
   {
     return link.getLabel();
   }
 
   public String getDBName()
   {
-    return link.getDBName();
+    return link.getTarget();
   }
 
   public String getUrl()
@@ -112,19 +112,19 @@ public class UrlLinkDisplay
     return isSelected;
   }
 
-  public void setName(String name)
+  public void setDBName(String name)
   {
-    link.setLabel(name);
+    link.setTarget(name);
   }
 
   public void setUrl(String rowUrl)
   {
-    link = new UrlLink(getName(), rowUrl, getName());
+    link = new UrlLink(getDescription(), rowUrl, getDBName());
   }
 
   public void setDescription(String desc)
   {
-    link.setDescription(desc);
+    link.setLabel(desc);
   }
 
   public void setIsDefault(boolean rowDefault)
@@ -150,7 +150,7 @@ public class UrlLinkDisplay
     case SELECTED:
       return isSelected;
     case NAME:
-      return getName();
+      return getDescription();
     case DATABASE:
       return getDBName();
     default:
@@ -177,7 +177,7 @@ public class UrlLinkDisplay
     case NAME:
       setDescription((String) value);
     case DATABASE:
-      setName((String) value);
+      setDBName((String) value);
       break;
     default:
       // do nothing
diff --git a/src/jalview/util/UrlLink.java b/src/jalview/util/UrlLink.java
index d95edb2..0529c73 100644
--- a/src/jalview/util/UrlLink.java
+++ b/src/jalview/util/UrlLink.java
@@ -45,6 +45,10 @@ public class UrlLink
    * documentation todo.
    */
 
+  private static final String EQUALS = "=";
+
+  private static final String SPACE = " ";
+
   private String urlSuffix;
 
   private String urlPrefix;
@@ -80,23 +84,38 @@ public class UrlLink
       dynamic = true;
       usesDBaccession = true;
 
-      sep = parseTargetAndLabel(sep, psqid, link);
+      sep = parseLabel(sep, psqid, link);
 
-      parseUrl(link, DB_ACCESSION, psqid, sep);
+      int endOfRegex = parseUrl(link, DB_ACCESSION, psqid, sep);
+      parseTarget(link, sep, endOfRegex);
     }
     else if (nsqid > -1)
     {
       dynamic = true;
-      sep = parseTargetAndLabel(sep, nsqid, link);
+      sep = parseLabel(sep, nsqid, link);
+
+      int endOfRegex = parseUrl(link, SEQUENCE_ID, nsqid, sep);
 
-      parseUrl(link, SEQUENCE_ID, nsqid, sep);
+      parseTarget(link, sep, endOfRegex);
     }
     else
     {
-      target = link.substring(0, sep);
-      sep = link.lastIndexOf(SEP);
-      label = link.substring(0, sep);
-      urlPrefix = link.substring(sep + 1).trim();
+      label = link.substring(0, sep).trim();
+
+      // if there's a third element in the url link string
+      // it is the target name, otherwise target=label
+      int lastsep = link.lastIndexOf(SEP);
+      if (lastsep != sep)
+      {
+        urlPrefix = link.substring(sep + 1, lastsep).trim();
+        target = link.substring(lastsep + 1).trim();
+      }
+      else
+      {
+        urlPrefix = link.substring(sep + 1).trim();
+        target = label;
+      }
+
       regexReplace = null; // implies we trim any prefix if necessary //
       urlSuffix = null;
     }
@@ -117,14 +136,13 @@ public class UrlLink
    */
   public UrlLink(String name, String url, String desc)
   {
-    this(name + SEP + url);
-    dbname = desc;
+    this(name + SEP + url + SEP + desc);
   }
 
   /**
    * @return the url_suffix
    */
-  public String getUrl_suffix()
+  public String getUrlSuffix()
   {
     return urlSuffix;
   }
@@ -132,7 +150,7 @@ public class UrlLink
   /**
    * @return the url_prefix
    */
-  public String getUrl_prefix()
+  public String getUrlPrefix()
   {
     return urlPrefix;
   }
@@ -153,21 +171,13 @@ public class UrlLink
     return label;
   }
 
-  /**
-   * @return the name of this link's associated database
-   */
-  public String getDBName()
-  {
-    return dbname;
-  }
-
   public String getUrlWithToken()
   {
     String var = (usesDBaccession ? DB_ACCESSION : SEQUENCE_ID);
 
     return urlPrefix
-            + (dynamic ? (DELIM + var + ((regexReplace != null) ? "="
-                    + regexReplace + "=" + DELIM : DELIM)) : "")
+            + (dynamic ? (DELIM + var + ((regexReplace != null) ? EQUALS
+                    + regexReplace + EQUALS + DELIM : DELIM)) : "")
             + ((urlSuffix == null) ? "" : urlSuffix);
   }
 
@@ -227,13 +237,13 @@ public class UrlLink
   }
 
   /**
-   * Set the description
+   * Set the target
    * 
    * @param desc
    */
-  public void setDescription(String desc)
+  public void setTarget(String desc)
   {
-    this.dbname = desc;
+    target = desc;
   }
 
   /**
@@ -277,7 +287,7 @@ public class UrlLink
                       + rg.stringMatched(s) + "'");
             }
             // try to collate subgroup matches
-            Vector subs = new Vector();
+            Vector<String> subs = new Vector<String>();
             // have to loop through submatches, collating them at top level
             // match
             int s = 0; // 1;
@@ -321,7 +331,7 @@ public class UrlLink
             String[] res = new String[subs.size()];
             for (int r = 0, rs = subs.size(); r < rs; r++)
             {
-              res[r] = (String) subs.elementAt(r);
+              res[r] = subs.elementAt(r);
             }
             subs.removeAllElements();
             return res;
@@ -354,6 +364,15 @@ public class UrlLink
   }
 
   /**
+   * @return delimited string containing label, url and target
+   */
+  public String toStringWithTarget()
+  {
+    return label + SEP + getUrlWithToken() + SEP + target;
+  }
+
+  /**
+   * Parse the label from the link string
    * 
    * @param firstSep
    *          Location of first occurrence of separator in link string
@@ -363,7 +382,7 @@ public class UrlLink
    *          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)
+  protected int parseLabel(int firstSep, int psqid, String link)
   {
     int p = firstSep;
     int sep = firstSep;
@@ -375,21 +394,44 @@ public class UrlLink
     // Assuming that the URL itself does not contain any SEP symbols
     // sep now contains last pipe symbol position prior to any regex symbols
     label = link.substring(0, sep);
-    if (label.indexOf(SEP) > -1)
-    {
-      // SEP terminated database name / www target at start of Label
-      target = label.substring(0, label.indexOf(SEP));
-    }
-    else if (label.indexOf(" ") > 2)
+
+    return sep;
+  }
+
+  /**
+   * Parse the target from the link string
+   * 
+   * @param link
+   *          Link string containing database name and url
+   * @param sep
+   *          Location of first separator symbol
+   * @param endOfRegex
+   *          Location of end of any regular expression in link string
+   */
+  protected void parseTarget(String link, int sep, int endOfRegex)
+  {
+    int lastsep = link.lastIndexOf(SEP);
+
+    if ((lastsep != sep) && (lastsep > endOfRegex))
     {
-      // space separated Label - matches database name
-      target = label.substring(0, label.indexOf(" "));
+      // final element in link string is the target
+      target = link.substring(lastsep + 1).trim();
     }
     else
     {
       target = label;
     }
-    return sep;
+
+    if (target.indexOf(SEP) > -1)
+    {
+      // SEP terminated database name / www target at start of Label
+      target = target.substring(0, target.indexOf(SEP));
+    }
+    else if (target.indexOf(SPACE) > 2)
+    {
+      // space separated label - first word matches database name
+      target = target.substring(0, target.indexOf(SPACE));
+    }
   }
 
   /**
@@ -403,8 +445,9 @@ public class UrlLink
    *          Position of id or name in link string
    * @param sep
    *          Position of separator in link string
+   * @return Location of end of any regex in link string
    */
-  protected void parseUrl(String link, String varName, int sqidPos, int sep)
+  protected int parseUrl(String link, String varName, int sqidPos, int sep)
   {
     urlPrefix = link.substring(sep + 1, sqidPos).trim();
 
@@ -447,7 +490,14 @@ public class UrlLink
       // verify format is really correct.
       if (link.indexOf(DELIM + varName + DELIM) == sqidPos)
       {
-        urlSuffix = link.substring(sqidPos + startLength - 1);
+        int lastsep = link.lastIndexOf(SEP);
+        if (lastsep < sqidPos + startLength - 1)
+        {
+          // the last SEP character was before the regex, ignore
+          lastsep = link.length();
+        }
+        urlSuffix = link.substring(sqidPos + startLength - 1, lastsep)
+                .trim();
         regexReplace = null;
       }
       else
@@ -456,6 +506,8 @@ public class UrlLink
                 + link;
       }
     }
+
+    return p;
   }
 
   /**
@@ -489,11 +541,11 @@ public class UrlLink
    */
   protected void createStaticLink(Map<String, List<String>> linkset)
   {
-    if (!linkset.containsKey(label + SEP + getUrl_prefix()))
+    if (!linkset.containsKey(label + SEP + getUrlPrefix()))
     {
       // Add a non-dynamic link
-      linkset.put(label + SEP + getUrl_prefix(),
-              Arrays.asList(target, label, null, getUrl_prefix()));
+      linkset.put(label + SEP + getUrlPrefix(),
+              Arrays.asList(target, label, null, getUrlPrefix()));
     }
   }
 
@@ -575,82 +627,4 @@ public class UrlLink
       }
     }
   }
-
-  private static void testUrls(UrlLink ul, String idstring, String[] urls)
-  {
-
-    if (urls == null)
-    {
-      System.out.println("Created NO urls.");
-    }
-    else
-    {
-      System.out.println("Created " + (urls.length / 2) + " Urls.");
-      for (int uls = 0; uls < urls.length; uls += 2)
-      {
-        System.out.println("URL Replacement text : " + urls[uls]
-                + " : URL : " + urls[uls + 1]);
-      }
-    }
-  }
-
-  public static void main(String argv[])
-  {
-    String[] links = new String[] {
-    /*
-     * "AlinkT|Target|http://foo.foo.soo/",
-     * "myUrl1|http://$SEQUENCE_ID=/[0-9]+/=$.someserver.org/foo",
-     * "myUrl2|http://$SEQUENCE_ID=/(([0-9]+).+([A-Za-z]+))/=$.someserver.org/foo"
-     * ,
-     * "myUrl3|http://$SEQUENCE_ID=/([0-9]+).+([A-Za-z]+)/=$.someserver.org/foo"
-     * , "myUrl4|target|http://$SEQUENCE_ID$.someserver.org/foo|too",
-     * "PF1|http://us.expasy.org/cgi-bin/niceprot.pl?$SEQUENCE_ID=/(?:PFAM:)?(.+)/=$"
-     * ,
-     * "PF2|http://us.expasy.org/cgi-bin/niceprot.pl?$SEQUENCE_ID=/(PFAM:)?(.+)/=$"
-     * ,
-     * "PF3|http://us.expasy.org/cgi-bin/niceprot.pl?$SEQUENCE_ID=/PFAM:(.+)/=$"
-     * , "NOTFER|http://notfer.org/$SEQUENCE_ID=/(?<!\\s)(.+)/=$",
-     */
-    "NESTED|http://nested/$" + DB_ACCESSION
-            + "=/^(?:Label:)?(?:(?:gi\\|(\\d+))|([^:]+))/=$/nested" };
-    String[] idstrings = new String[] {
-    /*
-     * //"LGUL_human", //"QWIQW_123123", "uniprot|why_do+_12313_foo",
-     * //"123123312", "123123 ABCDE foo", "PFAM:PF23943",
-     */
-    "Label:gi|9234|pdb|102L|A" };
-    // TODO: test the setLabel method.
-    for (int i = 0; i < links.length; i++)
-    {
-      UrlLink ul = new UrlLink(links[i]);
-      if (ul.isValid())
-      {
-        System.out.println("\n\n\n");
-        System.out.println("Link " + i + " " + links[i] + " : "
-                + ul.toString());
-        System.out.println(" pref : "
-                + ul.getUrl_prefix()
-                + "\n suf : "
-                + ul.getUrl_suffix()
-                + "\n : "
-                + ((ul.getRegexReplace() != null) ? ul.getRegexReplace()
-                        : ""));
-        for (int ids = 0; ids < idstrings.length; ids++)
-        {
-          System.out.println("ID String : " + idstrings[ids]
-                  + "\nWithout onlyIfMatches:");
-          String[] urls = ul.makeUrls(idstrings[ids], false);
-          testUrls(ul, idstrings[ids], urls);
-          System.out.println("With onlyIfMatches set.");
-          urls = ul.makeUrls(idstrings[ids], true);
-          testUrls(ul, idstrings[ids], urls);
-        }
-      }
-      else
-      {
-        System.err.println("Invalid URLLink : " + links[i] + " : "
-                + ul.getInvalidMessage());
-      }
-    }
-  }
 }
diff --git a/test/jalview/urls/AppletUrlProviderFactoryTest.java b/test/jalview/urls/AppletUrlProviderFactoryTest.java
index 5e0c622..2a967e4 100644
--- a/test/jalview/urls/AppletUrlProviderFactoryTest.java
+++ b/test/jalview/urls/AppletUrlProviderFactoryTest.java
@@ -65,7 +65,7 @@ public class AppletUrlProviderFactoryTest {
 
     // first link set correctly
     Assert.assertEquals(allLinks.get(0).getId(), "Test1");
-    Assert.assertEquals(allLinks.get(0).getName(), "Test1");
+    Assert.assertEquals(allLinks.get(0).getDescription(), "Test1");
     Assert.assertEquals(allLinks.get(0).getUrl(),
             "http://identifiers.org/uniprot/$DB_ACCESSION$");
     Assert.assertFalse(allLinks.get(0).getIsPrimary());
@@ -73,7 +73,7 @@ public class AppletUrlProviderFactoryTest {
 
     // second link set correctly
     Assert.assertEquals(allLinks.get(1).getId(), "Test2");
-    Assert.assertEquals(allLinks.get(1).getName(), "Test2");
+    Assert.assertEquals(allLinks.get(1).getDescription(), "Test2");
     Assert.assertEquals(allLinks.get(1).getUrl(), defaultUrl);
     Assert.assertTrue(allLinks.get(1).getIsPrimary());
     Assert.assertTrue(allLinks.get(1).getIsSelected());
diff --git a/test/jalview/urls/DesktopUrlProviderFactoryTest.java b/test/jalview/urls/DesktopUrlProviderFactoryTest.java
index f4bed2e..5610ca3 100644
--- a/test/jalview/urls/DesktopUrlProviderFactoryTest.java
+++ b/test/jalview/urls/DesktopUrlProviderFactoryTest.java
@@ -103,8 +103,8 @@ public class DesktopUrlProviderFactoryTest
     Assert.assertTrue(menulinks
             .contains("Test3|http://test3/$SEQUENCE_ID$"));
     Assert.assertTrue(menulinks
-            .contains("UniProt Knowledgebase|http://identifiers.org/uniprot/$DB_ACCESSION$"));
+            .contains("UniProt Knowledgebase|http://identifiers.org/uniprot/$DB_ACCESSION$|uniprot"));
     Assert.assertTrue(menulinks
-            .contains("InterPro|http://identifiers.org/interpro/$DB_ACCESSION$"));
+            .contains("InterPro|http://identifiers.org/interpro/$DB_ACCESSION$|interpro"));
   }
 }
diff --git a/test/jalview/urls/IdentifiersUrlProviderTest.java b/test/jalview/urls/IdentifiersUrlProviderTest.java
index 107dcda..9c8c96a 100644
--- a/test/jalview/urls/IdentifiersUrlProviderTest.java
+++ b/test/jalview/urls/IdentifiersUrlProviderTest.java
@@ -50,9 +50,9 @@ public class IdentifiersUrlProviderTest
           + "\"prefix\":\"ena.embl\",\"url\":\"http://identifiers.org/ena.embl\"}]";
   
   private static final String[] dlinks = {
-      "UniProt Knowledgebase|http://identifiers.org/uniprot/$DB_ACCESSION$",
-      "InterPro|http://identifiers.org/interpro/$DB_ACCESSION$",
-      "ENA|http://identifiers.org/ena.embl/$DB_ACCESSION$" };
+      "UniProt Knowledgebase|http://identifiers.org/uniprot/$DB_ACCESSION$|uniprot",
+      "InterPro|http://identifiers.org/interpro/$DB_ACCESSION$|interpro",
+      "ENA|http://identifiers.org/ena.embl/$DB_ACCESSION$|ena.embl" };
 
   private static final String[] dlinks1 = {
       "MIR:00000011|http://identifiers.org/interpro/$DB_ACCESSION$",
diff --git a/test/jalview/urls/UrlLinkDisplayTest.java b/test/jalview/urls/UrlLinkDisplayTest.java
index e8a585c..8c50082 100644
--- a/test/jalview/urls/UrlLinkDisplayTest.java
+++ b/test/jalview/urls/UrlLinkDisplayTest.java
@@ -20,16 +20,14 @@ public class UrlLinkDisplayTest {
   @Test(groups = { "Functional" })
   public void getValue()
   {
-    UrlLink link = new UrlLink("Test Url",
-            "http://identifiers.org/$DB_ACCESSION$",
- "TestName");
+    UrlLink link = new UrlLink("Test Name",
+            "http://identifiers.org/$DB_ACCESSION$", "TestDB");
     UrlLinkDisplay u = new UrlLinkDisplay("Test", link, false, false);
 
     Assert.assertFalse((boolean) u.getValue(UrlLinkDisplay.PRIMARY));
     Assert.assertEquals(u.getValue(UrlLinkDisplay.ID), "Test");
-    Assert.assertEquals(u.getValue(UrlLinkDisplay.DATABASE), "Test Url");
-    Assert.assertEquals(u.getValue(UrlLinkDisplay.NAME),
- "TestName");
+    Assert.assertEquals(u.getValue(UrlLinkDisplay.DATABASE), "TestDB");
+    Assert.assertEquals(u.getValue(UrlLinkDisplay.NAME), "Test Name");
     Assert.assertFalse((boolean) u.getValue(UrlLinkDisplay.SELECTED));
     Assert.assertEquals(u.getValue(UrlLinkDisplay.URL),
             "http://identifiers.org/$DB_ACCESSION$");
@@ -69,40 +67,37 @@ public class UrlLinkDisplayTest {
   public void testName()
   {
     UrlLink link = new UrlLink("Test Url",
-            "http://identifiers.org/$DB_ACCESSION$",
- "TestName");
+            "http://identifiers.org/$DB_ACCESSION$", "TestName");
     UrlLinkDisplay u = new UrlLinkDisplay("Test", link, false, false);
 
     // Name initially as input in link
-    Assert.assertEquals(u.getName(), "Test Url");
+    Assert.assertEquals(u.getDBName(), "TestName");
 
     // Setting updates name
-    u.setName("New Name");
-    Assert.assertEquals(u.getName(), "New Name");
+    u.setDBName("NewName");
+    Assert.assertEquals(u.getDBName(), "NewName");
   }
 
   @Test(groups = { "Functional" })
   public void testDescription()
   {
-    UrlLink link = new UrlLink("Test Url",
-            "http://identifiers.org/$DB_ACCESSION$",
- "TestName");
+    UrlLink link = new UrlLink("Test Name",
+            "http://identifiers.org/$DB_ACCESSION$", "TestDB");
     UrlLinkDisplay u = new UrlLinkDisplay("Test", link, false, false);
 
     // Desc initially as input in link
-    Assert.assertEquals(u.getDBName(), "TestName");
+    Assert.assertEquals(u.getDescription(), "Test Name");
 
     // Setting updates name
     u.setDescription("New Desc");
-    Assert.assertEquals(u.getDBName(), "New Desc");
+    Assert.assertEquals(u.getDescription(), "New Desc");
   }
 
   @Test(groups = { "Functional" })
   public void testUrl()
   {
-    UrlLink link = new UrlLink("Test Url",
-            "http://identifiers.org/$DB_ACCESSION$",
- "TestName");
+    UrlLink link = new UrlLink("Test Name",
+            "http://identifiers.org/$DB_ACCESSION$", "TestDB");
     UrlLinkDisplay u = new UrlLinkDisplay("Test", link, false, false);
 
     // Url initially as input in link
@@ -116,16 +111,14 @@ public class UrlLinkDisplayTest {
   @Test(groups = { "Functional" })
   public void testGetSetValue()
   {
-    UrlLink link = new UrlLink("Test Url",
-            "http://identifiers.org/$DB_ACCESSION$",
- "TestName");
+    UrlLink link = new UrlLink("Test Name",
+            "http://identifiers.org/$DB_ACCESSION$", "TestDB");
     UrlLinkDisplay u = new UrlLinkDisplay("Test", link, false, false);
 
     Assert.assertFalse((boolean) u.getValue(UrlLinkDisplay.PRIMARY));
     Assert.assertFalse((boolean) u.getValue(UrlLinkDisplay.SELECTED));
-    Assert.assertEquals(u.getValue(UrlLinkDisplay.DATABASE), "Test Url");
-    Assert.assertEquals(u.getValue(UrlLinkDisplay.NAME),
- "TestName");
+    Assert.assertEquals(u.getValue(UrlLinkDisplay.DATABASE), "TestDB");
+    Assert.assertEquals(u.getValue(UrlLinkDisplay.NAME), "Test Name");
     Assert.assertEquals(u.getValue(UrlLinkDisplay.ID), "Test");
     Assert.assertEquals(u.getValue(UrlLinkDisplay.URL),
             "http://identifiers.org/$DB_ACCESSION$");
@@ -139,8 +132,8 @@ public class UrlLinkDisplayTest {
     u.setValue(UrlLinkDisplay.NAME, "New Desc");
     Assert.assertEquals(u.getValue(UrlLinkDisplay.NAME), "New Desc");
 
-    u.setValue(UrlLinkDisplay.DATABASE, "New Name");
-    Assert.assertEquals(u.getValue(UrlLinkDisplay.DATABASE), "New Name");
+    u.setValue(UrlLinkDisplay.DATABASE, "NewName");
+    Assert.assertEquals(u.getValue(UrlLinkDisplay.DATABASE), "NewName");
 
     u.setValue(UrlLinkDisplay.ID, "New ID");
     Assert.assertEquals(u.getValue(UrlLinkDisplay.ID), "New ID");
diff --git a/test/jalview/urls/UrlLinkTableModelTest.java b/test/jalview/urls/UrlLinkTableModelTest.java
index e5086fd..4b2ea6e 100644
--- a/test/jalview/urls/UrlLinkTableModelTest.java
+++ b/test/jalview/urls/UrlLinkTableModelTest.java
@@ -105,8 +105,9 @@ public class UrlLinkTableModelTest {
   @Test(groups = { "Functional" })
   public void testInitialisation()
   {
-    int defaultCol = 3;
-    int nameCol = 0;
+    int defaultCol = 4;
+    int dbCol = 0;
+    int descCol = 1;
 
     UrlLinkTableModel m = new UrlLinkTableModel(prov);
 
@@ -124,8 +125,11 @@ public class UrlLinkTableModelTest {
       if (isDefault)
       {
         count++;
-        String defaultName = (String) m.getValueAt(row, nameCol);
-        Assert.assertEquals(defaultName, "TEST2");
+        String defaultDBName = (String) m.getValueAt(row, dbCol);
+        Assert.assertEquals(defaultDBName, "TEST2");
+
+        String defaultDesc = (String) m.getValueAt(row, descCol);
+        Assert.assertEquals(defaultDesc, "TEST2");
       }
     }
     Assert.assertEquals(count, 1);
@@ -140,7 +144,7 @@ public class UrlLinkTableModelTest {
     UrlLinkTableModel m = new UrlLinkTableModel(prov);
 
     // correct numbers of column and rows
-    Assert.assertEquals(m.getColumnCount(), 4);
+    Assert.assertEquals(m.getColumnCount(), 5);
     Assert.assertEquals(m.getRowCount(), 10);
   }
 
@@ -154,19 +158,22 @@ public class UrlLinkTableModelTest {
 
     // check column names
     Assert.assertEquals(m.getColumnName(0),
-            MessageManager.formatMessage("label.name"));
+            MessageManager.formatMessage("label.database"));
     Assert.assertEquals(m.getColumnName(1),
-            MessageManager.formatMessage("label.url"));
+            MessageManager.formatMessage("label.name"));
     Assert.assertEquals(m.getColumnName(2),
-            MessageManager.formatMessage("label.inmenu"));
+            MessageManager.formatMessage("label.url"));
     Assert.assertEquals(m.getColumnName(3),
+            MessageManager.formatMessage("label.inmenu"));
+    Assert.assertEquals(m.getColumnName(4),
             MessageManager.formatMessage("label.primary"));
 
     // check column classes
     Assert.assertEquals(m.getColumnClass(0), String.class);
     Assert.assertEquals(m.getColumnClass(1), String.class);
-    Assert.assertEquals(m.getColumnClass(2), Boolean.class);
+    Assert.assertEquals(m.getColumnClass(2), String.class);
     Assert.assertEquals(m.getColumnClass(3), Boolean.class);
+    Assert.assertEquals(m.getColumnClass(4), Boolean.class);
   }
 
   /*
@@ -181,9 +188,10 @@ public class UrlLinkTableModelTest {
 
     // check table has new row inserted
     Assert.assertEquals(m.getValueAt(10, 0), "newname");
-    Assert.assertEquals(m.getValueAt(10, 1), "newurl");
-    Assert.assertEquals(m.getValueAt(10, 2), true);
-    Assert.assertEquals(m.getValueAt(10, 3), false);
+    Assert.assertEquals(m.getValueAt(10, 1), "newname");
+    Assert.assertEquals(m.getValueAt(10, 2), "newurl");
+    Assert.assertEquals(m.getValueAt(10, 3), true);
+    Assert.assertEquals(m.getValueAt(10, 4), false);
 
     // check data source has new row insrte
     Assert.assertTrue(prov.getLinksForMenu().contains(
@@ -231,22 +239,29 @@ public class UrlLinkTableModelTest {
     }
 
     // set new values, one in each row
-    m.setValueAt("namechanged", 6, 0);
-    m.setValueAt("urlchanged", 7, 1);
-    m.setValueAt(false, 8, 2);
-    m.setValueAt(true, 6, 3);
+    m.setValueAt("dbnamechanged", 6, 0);
+    m.setValueAt("descchanged", 6, 1);
+    m.setValueAt("urlchanged", 7, 2);
+    m.setValueAt(false, 8, 3);
+    m.setValueAt(true, 6, 4);
+
+    m.setValueAt("dbnamechanged", 5, 0);
 
     // check values updated in table
-    Assert.assertEquals(m.getValueAt(6, 0), "namechanged");
-    Assert.assertEquals(m.getValueAt(7, 1), "urlchanged");
-    Assert.assertFalse((boolean) m.getValueAt(8, 2));
-    Assert.assertTrue((boolean) m.getValueAt(6, 3));
-    Assert.assertFalse((boolean) m.getValueAt(olddefault, 3));
+    Assert.assertEquals(m.getValueAt(6, 0), "descchanged"); // custom url can't
+                                                            // change db name
+    Assert.assertEquals(m.getValueAt(6, 1), "descchanged");
+    Assert.assertEquals(m.getValueAt(7, 2), "urlchanged");
+    Assert.assertFalse((boolean) m.getValueAt(8, 3));
+    Assert.assertTrue((boolean) m.getValueAt(6, 4));
+    Assert.assertFalse((boolean) m.getValueAt(olddefault, 4));
+
+    Assert.assertEquals(m.getValueAt(5, 0), "dbnamechanged");
 
     // check default row is exactly one row still
     for (int row = 0; row < m.getRowCount(); row++)
     {
-      isDefault = (boolean) m.getValueAt(row, 3);
+      isDefault = (boolean) m.getValueAt(row, 4);
 
       // if isDefault is true, row is 9
       // if isDefault is false, row is not 9
@@ -254,13 +269,14 @@ public class UrlLinkTableModelTest {
     }
 
     // check table updated
-    Assert.assertTrue(prov.writeUrlsAsString(true).contains("namechanged" +SEP + m.getValueAt(6, 1)));
-    Assert.assertTrue(prov.writeUrlsAsString(true).contains(m.getValueAt(7,0) + SEP + "urlchanged"));
+    Assert.assertTrue(prov.writeUrlsAsString(true).contains(
+            "descchanged" + SEP + m.getValueAt(6, 2)));
+    Assert.assertTrue(prov.writeUrlsAsString(true).contains(
+            m.getValueAt(7, 1) + SEP + "urlchanged"));
     Assert.assertTrue(prov.writeUrlsAsString(false).contains(
-            (String) m.getValueAt(8, 0)));
-    Assert.assertEquals(prov.getPrimaryUrl("seqid"), m.getValueAt(6, 1)
+            (String) m.getValueAt(8, 1)));
+    Assert.assertEquals(prov.getPrimaryUrl("seqid"), m.getValueAt(6, 2)
             .toString().replace(DELIM + SEQUENCE_ID + DELIM, "seqid"));
-
   }
 
   /*
@@ -275,15 +291,16 @@ public class UrlLinkTableModelTest {
     {
       Assert.assertFalse(m.isCellEditable(row, 0));
       Assert.assertFalse(m.isCellEditable(row, 1));
-      Assert.assertTrue(m.isCellEditable(row, 2));
+      Assert.assertFalse(m.isCellEditable(row, 2));
+      Assert.assertTrue(m.isCellEditable(row, 3));
 
       if ((row == 4) || (row == 6) || (row == 7))
       {
-        Assert.assertTrue(m.isCellEditable(row, 3));
+        Assert.assertTrue(m.isCellEditable(row, 4));
       }
       else
       {
-        Assert.assertFalse(m.isCellEditable(row, 3));
+        Assert.assertFalse(m.isCellEditable(row, 4));
       }
     }
   }
diff --git a/test/jalview/urls/UrlProviderTest.java b/test/jalview/urls/UrlProviderTest.java
index be40c81..29327d7 100644
--- a/test/jalview/urls/UrlProviderTest.java
+++ b/test/jalview/urls/UrlProviderTest.java
@@ -90,9 +90,9 @@ public class UrlProviderTest {
     Assert.assertTrue(menulinks
             .contains("Test3|http://test3/$SEQUENCE_ID$"));
     Assert.assertTrue(menulinks
-            .contains("UniProt Knowledgebase|http://identifiers.org/uniprot/$DB_ACCESSION$"));
+            .contains("UniProt Knowledgebase|http://identifiers.org/uniprot/$DB_ACCESSION$|uniprot"));
     Assert.assertTrue(menulinks
-            .contains("InterPro|http://identifiers.org/interpro/$DB_ACCESSION$"));
+            .contains("InterPro|http://identifiers.org/interpro/$DB_ACCESSION$|interpro"));
     Assert.assertTrue(menulinks.contains(UrlConstants.DEFAULT_LABEL
             + UrlConstants.SEP + emblUrl));
   }
diff --git a/test/jalview/util/UrlLinkTest.java b/test/jalview/util/UrlLinkTest.java
index d07206f..4092cf2 100644
--- a/test/jalview/util/UrlLinkTest.java
+++ b/test/jalview/util/UrlLinkTest.java
@@ -75,8 +75,8 @@ public class UrlLinkTest
             + DELIM + URL_SUFFIX);
     assertEquals(DB, ul.getTarget());
     assertEquals(DB, ul.getLabel());
-    assertEquals(URL_PREFIX, ul.getUrl_prefix());
-    assertEquals(URL_SUFFIX, ul.getUrl_suffix());
+    assertEquals(URL_PREFIX, ul.getUrlPrefix());
+    assertEquals(URL_SUFFIX, ul.getUrlSuffix());
     assertTrue(ul.isDynamic());
     assertFalse(ul.usesDBAccession());
     assertNull(ul.getRegexReplace());
@@ -88,8 +88,8 @@ public class UrlLinkTest
             + URL_SUFFIX);
     assertEquals(DB, ul.getTarget());
     assertEquals(DB, ul.getLabel());
-    assertEquals(URL_PREFIX, ul.getUrl_prefix());
-    assertEquals(URL_SUFFIX, ul.getUrl_suffix());
+    assertEquals(URL_PREFIX, ul.getUrlPrefix());
+    assertEquals(URL_SUFFIX, ul.getUrlSuffix());
     assertTrue(ul.isDynamic());
     assertTrue(ul.usesDBAccession());
     assertNull(ul.getRegexReplace());
@@ -100,7 +100,7 @@ public class UrlLinkTest
     ul = new UrlLink(DB + SEP + URL_PREFIX + URL_SUFFIX.substring(1));
     assertEquals(DB, ul.getTarget());
     assertEquals(DB, ul.getLabel());
-    assertEquals(URL_PREFIX + URL_SUFFIX.substring(1), ul.getUrl_prefix());
+    assertEquals(URL_PREFIX + URL_SUFFIX.substring(1), ul.getUrlPrefix());
     assertFalse(ul.isDynamic());
     assertFalse(ul.usesDBAccession());
     assertNull(ul.getRegexReplace());
@@ -119,8 +119,8 @@ public class UrlLinkTest
             + REGEX_NESTED + DELIM + URL_SUFFIX);
     assertEquals(DB, ul.getTarget());
     assertEquals(DB, ul.getLabel());
-    assertEquals(URL_PREFIX, ul.getUrl_prefix());
-    assertEquals(URL_SUFFIX, ul.getUrl_suffix());
+    assertEquals(URL_PREFIX, ul.getUrlPrefix());
+    assertEquals(URL_SUFFIX, ul.getUrlSuffix());
     assertTrue(ul.isDynamic());
     assertFalse(ul.usesDBAccession());
     assertEquals(REGEX_NESTED.substring(2, REGEX_NESTED.length() - 2),
@@ -133,8 +133,8 @@ public class UrlLinkTest
             + REGEX_NESTED + DELIM + URL_SUFFIX);
     assertEquals(DB, ul.getTarget());
     assertEquals(DB, ul.getLabel());
-    assertEquals(URL_PREFIX, ul.getUrl_prefix());
-    assertEquals(URL_SUFFIX, ul.getUrl_suffix());
+    assertEquals(URL_PREFIX, ul.getUrlPrefix());
+    assertEquals(URL_SUFFIX, ul.getUrlSuffix());
     assertTrue(ul.isDynamic());
     assertTrue(ul.usesDBAccession());
     assertEquals(REGEX_NESTED.substring(2, REGEX_NESTED.length() - 2),
@@ -147,8 +147,8 @@ public class UrlLinkTest
             + REGEX_RUBBISH + DELIM + URL_SUFFIX);
     assertEquals(DB, ul.getTarget());
     assertEquals(DB, ul.getLabel());
-    assertEquals(URL_PREFIX, ul.getUrl_prefix());
-    assertEquals(URL_SUFFIX, ul.getUrl_suffix());
+    assertEquals(URL_PREFIX, ul.getUrlPrefix());
+    assertEquals(URL_SUFFIX, ul.getUrlSuffix());
     assertTrue(ul.isDynamic());
     assertTrue(ul.usesDBAccession());
     assertEquals(REGEX_RUBBISH.substring(2, REGEX_RUBBISH.length() - 2),
@@ -277,10 +277,10 @@ public class UrlLinkTest
     String key = DB + SEP + URL_PREFIX;
     assertEquals(1, linkset.size());
     assertTrue(linkset.containsKey(key));
-    assertEquals(linkset.get(key).get(0), DB);
-    assertEquals(linkset.get(key).get(1), DB);
-    assertEquals(linkset.get(key).get(2), null);
-    assertEquals(linkset.get(key).get(3), URL_PREFIX);
+    assertEquals(DB, linkset.get(key).get(0));
+    assertEquals(DB, linkset.get(key).get(1));
+    assertEquals(null, linkset.get(key).get(2));
+    assertEquals(URL_PREFIX, linkset.get(key).get(3));
   }
 
   /**
@@ -297,10 +297,10 @@ public class UrlLinkTest
     String key = DB + SEP + URL_PREFIX + URL_SUFFIX;
     assertEquals(1, linkset.size());
     assertTrue(linkset.containsKey(key));
-    assertEquals(linkset.get(key).get(0), DB);
-    assertEquals(linkset.get(key).get(1), DB);
-    assertEquals(linkset.get(key).get(2), null);
-    assertEquals(linkset.get(key).get(3), URL_PREFIX + URL_SUFFIX);
+    assertEquals(DB, linkset.get(key).get(0));
+    assertEquals(DB, linkset.get(key).get(1));
+    assertEquals(null, linkset.get(key).get(2));
+    assertEquals(URL_PREFIX + URL_SUFFIX, linkset.get(key).get(3));
   }
 
   /**
@@ -348,11 +348,11 @@ public class UrlLinkTest
             + URL_SUFFIX;
     assertEquals(1, linkset.size());
     assertTrue(linkset.containsKey(key));
-    assertEquals(linkset.get(key).get(0), DB);
-    assertEquals(linkset.get(key).get(1), DB);
-    assertEquals(linkset.get(key).get(2), seq0.getName());
-    assertEquals(linkset.get(key).get(3), URL_PREFIX + seq0.getName()
-            + URL_SUFFIX);
+    assertEquals(DB, linkset.get(key).get(0));
+    assertEquals(DB, linkset.get(key).get(1));
+    assertEquals(seq0.getName(), linkset.get(key).get(2));
+    assertEquals(URL_PREFIX + seq0.getName() + URL_SUFFIX, linkset.get(key)
+            .get(3));
 
     // Test where link takes a db annotation id and only has one dbref
     ul = new UrlLink(links.get(1));
@@ -360,14 +360,14 @@ public class UrlLinkTest
     ul.createLinksFromSeq(seq0, linkset);
 
     key = "P83527|http://www.uniprot.org/uniprot/P83527";
-    assertEquals(1, linkset.size());
+    assertEquals(linkset.size(), 1);
     assertTrue(linkset.containsKey(key));
-    assertEquals(linkset.get(key).get(0), DBRefSource.UNIPROT);
-    assertEquals(linkset.get(key).get(1), DBRefSource.UNIPROT + SEP
-            + "P83527");
-    assertEquals(linkset.get(key).get(2), "P83527");
-    assertEquals(linkset.get(key).get(3),
-            "http://www.uniprot.org/uniprot/P83527");
+    assertEquals(DBRefSource.UNIPROT, linkset.get(key).get(0));
+    assertEquals(DBRefSource.UNIPROT + SEP + "P83527", linkset.get(key)
+            .get(1));
+    assertEquals("P83527", linkset.get(key).get(2));
+    assertEquals("http://www.uniprot.org/uniprot/P83527", linkset.get(key)
+            .get(3));
 
     // Test where link takes a db annotation id and has multiple dbrefs
     ul = new UrlLink(links.get(2));
@@ -378,27 +378,27 @@ public class UrlLinkTest
     // check each link made it in correctly
     key = "IPR001041|http://www.ebi.ac.uk/interpro/entry/IPR001041";
     assertTrue(linkset.containsKey(key));
-    assertEquals(linkset.get(key).get(0), "INTERPRO");
-    assertEquals(linkset.get(key).get(1), "INTERPRO" + SEP + "IPR001041");
-    assertEquals(linkset.get(key).get(2), "IPR001041");
-    assertEquals(linkset.get(key).get(3),
-            "http://www.ebi.ac.uk/interpro/entry/IPR001041");
+    assertEquals("INTERPRO", linkset.get(key).get(0));
+    assertEquals("INTERPRO" + SEP + "IPR001041", linkset.get(key).get(1));
+    assertEquals("IPR001041", linkset.get(key).get(2));
+    assertEquals("http://www.ebi.ac.uk/interpro/entry/IPR001041", linkset
+            .get(key).get(3));
 
     key = "IPR006058|http://www.ebi.ac.uk/interpro/entry/IPR006058";
     assertTrue(linkset.containsKey(key));
-    assertEquals(linkset.get(key).get(0), "INTERPRO");
-    assertEquals(linkset.get(key).get(1), "INTERPRO" + SEP + "IPR006058");
-    assertEquals(linkset.get(key).get(2), "IPR006058");
-    assertEquals(linkset.get(key).get(3),
-            "http://www.ebi.ac.uk/interpro/entry/IPR006058");
+    assertEquals("INTERPRO", linkset.get(key).get(0));
+    assertEquals("INTERPRO" + SEP + "IPR006058", linkset.get(key).get(1));
+    assertEquals("IPR006058", linkset.get(key).get(2));
+    assertEquals("http://www.ebi.ac.uk/interpro/entry/IPR006058", linkset
+            .get(key).get(3));
 
     key = "IPR012675|http://www.ebi.ac.uk/interpro/entry/IPR012675";
     assertTrue(linkset.containsKey(key));
-    assertEquals(linkset.get(key).get(0), "INTERPRO");
-    assertEquals(linkset.get(key).get(1), "INTERPRO" + SEP + "IPR012675");
-    assertEquals(linkset.get(key).get(2), "IPR012675");
-    assertEquals(linkset.get(key).get(3),
-            "http://www.ebi.ac.uk/interpro/entry/IPR012675");
+    assertEquals("INTERPRO", linkset.get(key).get(0));
+    assertEquals("INTERPRO" + SEP + "IPR012675", linkset.get(key).get(1));
+    assertEquals("IPR012675", linkset.get(key).get(2));
+    assertEquals("http://www.ebi.ac.uk/interpro/entry/IPR012675", linkset
+            .get(key).get(3));
 
     // Test where there are no matching dbrefs for the link
     ul = new UrlLink(DB + SEP + URL_PREFIX + DELIM + DB_ACCESSION + DELIM
@@ -408,4 +408,35 @@ public class UrlLinkTest
     assertTrue(linkset.isEmpty());
   }
 
+  /**
+   * Test links where label and target are both included
+   */
+  @Test(groups = { "Functional" })
+  public void testLinksWithTargets()
+  {
+    UrlLink ul = new UrlLink(
+            "Protein Data Bank | http://www.identifiers.org/pdb/$"
+                    + DB_ACCESSION + "$" + " | pdb");
+
+    assertEquals("Protein Data Bank", ul.getLabel());
+    assertEquals("pdb", ul.getTarget());
+    assertEquals("http://www.identifiers.org/pdb/$" + DB_ACCESSION + "$",
+            ul.getUrlWithToken());
+
+    assertEquals("Protein Data Bank|http://www.identifiers.org/pdb/$"
+            + DB_ACCESSION + "$" + "|pdb", ul.toStringWithTarget());
+
+    ul = new UrlLink("Protein Data Bank",
+            "http://www.identifiers.org/pdb/$" + DB_ACCESSION + "$", "pdb");
+
+    assertEquals("Protein Data Bank", ul.getLabel());
+    assertEquals("pdb", ul.getTarget());
+    assertEquals("http://www.identifiers.org/pdb/$" + DB_ACCESSION + "$",
+            ul.getUrlWithToken());
+
+    assertEquals("Protein Data Bank|http://www.identifiers.org/pdb/$"
+            + DB_ACCESSION + "$" + "|pdb", ul.toStringWithTarget());
+
+  }
+
 }