Merge branch 'develop' into task/JAL-2196pdbeProperties
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Wed, 5 Oct 2016 15:17:59 +0000 (16:17 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Wed, 5 Oct 2016 15:17:59 +0000 (16:17 +0100)
Conflicts:
src/jalview/datamodel/PDBEntry.java
test/jalview/ws/dbsources/UniprotTest.java

1  2 
src/jalview/appletgui/AlignFrame.java
src/jalview/appletgui/AppletJmol.java
src/jalview/datamodel/PDBEntry.java
src/jalview/gui/Jalview2XML.java
src/jalview/io/StructureFile.java
src/jalview/util/DBRefUtils.java
src/jalview/ws/dbsources/Uniprot.java
test/jalview/datamodel/PDBEntryTest.java
test/jalview/ws/dbsources/UniprotTest.java

@@@ -102,6 -102,7 +102,6 @@@ import java.net.URLEncoder
  import java.util.Arrays;
  import java.util.Deque;
  import java.util.HashMap;
 -import java.util.Hashtable;
  import java.util.List;
  import java.util.Map;
  import java.util.StringTokenizer;
@@@ -702,9 -703,7 +702,7 @@@ public class AlignFrame extends Embmenu
        // Hide everything by the current selection - this is a hack - we do the
        // invert and then hide
        // first check that there will be visible columns after the invert.
-       if ((viewport.getColumnSelection() != null
-               && viewport.getColumnSelection().getSelected() != null && viewport
-               .getColumnSelection().getSelected().size() > 0)
+       if (viewport.hasSelectedColumns()
                || (sg != null && sg.getSize() > 0 && sg.getStartRes() <= sg
                        .getEndRes()))
        {
          hide = true;
          viewport.hideAllSelectedSeqs();
        }
-       else if (!(toggleCols && viewport.getColumnSelection().getSelected()
-               .size() > 0))
+       else if (!(toggleCols && viewport.hasSelectedColumns()))
        {
          viewport.showAllHiddenSeqs();
        }
  
      if (toggleCols)
      {
-       if (viewport.getColumnSelection().getSelected().size() > 0)
+       if (viewport.hasSelectedColumns())
        {
          viewport.hideSelectedColumns();
          if (!toggleSeqs)
        }
        if (needtoadd)
        {
 -        // make a note of the access mode and add
 -        if (pdbentry.getProperty() == null)
 -        {
 -          pdbentry.setProperty(new Hashtable());
 -        }
 -        pdbentry.getProperty().put("protocol", protocol);
 +        pdbentry.setProperty("protocol", protocol);
          toaddpdb.addPDBId(pdbentry);
          alignPanel.getStructureSelectionManager()
                  .registerPDBEntry(pdbentry);
      if (protocol == null || protocol.trim().length() == 0
              || protocol.equals("null"))
      {
 -      protocol = (String) pdb.getProperty().get("protocol");
 +      protocol = (String) pdb.getProperty("protocol");
        if (protocol == null)
        {
          System.err.println("Couldn't work out protocol to open structure: "
@@@ -60,9 -60,12 +60,11 @@@ import java.awt.event.KeyListener
  import java.awt.event.WindowAdapter;
  import java.awt.event.WindowEvent;
  import java.util.ArrayList;
  import java.util.List;
  import java.util.Vector;
  
+ import org.jmol.util.Logger;
  public class AppletJmol extends EmbmenuFrame implements
  // StructureListener,
          KeyListener, ActionListener, ItemListener
        jmb.allocateViewer(renderPanel, true, ap.av.applet.getName()
                + "_jmol_", ap.av.applet.getDocumentBase(),
                ap.av.applet.getCodeBase(), "-applet", scriptWindow, null);
+       Logger.setLogLevel(Logger.LEVEL_WARN);
      } catch (Exception e)
      {
        System.err
          closeViewer();
        }
      });
 -    if (pdbentry.getProperty() == null)
 -    {
 -      pdbentry.setProperty(new Hashtable());
 -      pdbentry.getProperty().put("protocol", protocol);
 -    }
 +    pdbentry.setProperty("protocol", protocol);
 +
      if (pdbentry.getFile() != null)
      {
        // import structure data from pdbentry.getFile based on given protocol
@@@ -22,20 -22,11 +22,21 @@@ package jalview.datamodel
  
  import jalview.util.CaseInsensitiveString;
  
 +import java.util.Collections;
 +import java.util.Enumeration;
  import java.util.Hashtable;
  
  public class PDBEntry
  {
 +
 +  /**
 +   * constant for storing chain code in properties table
 +   */
 +  private static final String CHAIN_ID = "chain_code";
 +
-   Hashtable<String, Object> properties;
++  private Hashtable<String, Object> properties;
++
+   private static final int PDB_ID_LENGTH = 4;
  
    private String file;
  
      }
    }
  
 -  /**
 -   * constant for storing chain code in properties table
 -   */
 -  private static final String CHAIN_ID = "chain_code";
 -
 -  Hashtable properties;
  
-   /*
-    * (non-Javadoc)
-    * 
-    * @see java.lang.Object#equals(java.lang.Object)
+   /**
+    * Answers true if obj is a PDBEntry with the same id and chain code (both
+    * ignoring case), file, type and properties
     */
    @Override
    public boolean equals(Object obj)
        return true;
      }
      PDBEntry o = (PDBEntry) obj;
-     return (type == o.type || (type != null && o.type != null && o.type
-             .equals(type)))
-             && (id == o.id || (id != null && o.id != null && o.id
-                     .equalsIgnoreCase(id)))
-             && (properties == o.properties || (properties != null
-                     && o.properties != null && properties
-                       .equals(o.properties)));
  
+     /*
+      * note that chain code is stored as a property wrapped by a 
+      * CaseInsensitiveString, so we are in effect doing a 
+      * case-insensitive comparison of chain codes
+      */
+     boolean idMatches = id == o.id
+             || (id != null && id.equalsIgnoreCase(o.id));
+     boolean fileMatches = file == o.file
+             || (file != null && file.equals(o.file));
+     boolean typeMatches = type == o.type
+             || (type != null && type.equals(o.type));
+     if (idMatches && fileMatches && typeMatches)
+     {
+       return properties == o.properties
+               || (properties != null && properties.equals(o.properties));
+     }
+     return false;
    }
  
    /**
    public PDBEntry(String pdbId, String chain, PDBEntry.Type type,
            String filePath)
    {
+     init(pdbId, chain, type, filePath);
+   }
+   /**
+    * @param pdbId
+    * @param chain
 -   * @param type
++   * @param entryType
+    * @param filePath
+    */
 -  void init(String pdbId, String chain, PDBEntry.Type type, String filePath)
++  void init(String pdbId, String chain, PDBEntry.Type entryType, String filePath)
+   {
      this.id = pdbId;
--    this.type = type == null ? null : type.toString();
++    this.type = entryType == null ? null : entryType.toString();
      this.file = filePath;
      setChainCode(chain);
    }
      id = entry.id;
      if (entry.properties != null)
      {
 -      properties = (Hashtable) entry.properties.clone();
 +      properties = (Hashtable<String, Object>) entry.properties.clone();
      }
    }
  
+   /**
+    * Make a PDBEntry from a DBRefEntry. The accession code is used for the PDB
+    * id, but if it is 5 characters in length, the last character is removed and
+    * set as the chain code instead.
+    * 
+    * @param dbr
+    */
+   public PDBEntry(DBRefEntry dbr)
+   {
+     if (!DBRefSource.PDB.equals(dbr.getSource()))
+     {
+       throw new IllegalArgumentException("Invalid source: "
+               + dbr.getSource());
+     }
+     String pdbId = dbr.getAccessionId();
+     String chainCode = null;
+     if (pdbId.length() == PDB_ID_LENGTH + 1)
+     {
+       char chain = pdbId.charAt(PDB_ID_LENGTH);
+       if (('a' <= chain && chain <= 'z') || ('A' <= chain && chain <= 'Z'))
+       {
+         pdbId = pdbId.substring(0, PDB_ID_LENGTH);
+         chainCode = String.valueOf(chain);
+       }
+     }
+     init(pdbId, chainCode, null, null);
+   }
 -  public void setFile(String file)
 +  public void setFile(String f)
    {
 -    this.file = file;
 +    this.file = f;
    }
  
    public String getFile()
      return id;
    }
  
 -  public void setProperty(Hashtable property)
 +  public void setProperty(String key, Object value)
    {
 -    this.properties = property;
 +    if (this.properties == null)
 +    {
 +      this.properties = new Hashtable<String, Object>();
 +    }
 +    properties.put(key, value);
    }
  
 -  public Hashtable getProperty()
 +  public Object getProperty(String key)
    {
 -    return properties;
 +    return properties == null ? null : properties.get(key);
 +  }
 +
 +  /**
 +   * Returns an enumeration of the keys of this object's properties (or an empty
 +   * enumeration if it has no properties)
 +   * 
 +   * @return
 +   */
 +  public Enumeration<String> getProperties()
 +  {
 +    if (properties == null)
 +    {
 +      return Collections.emptyEnumeration();
 +    }
 +    return properties.keys();
    }
  
    /**
              : properties.get(CHAIN_ID).toString();
    }
  
 +  /**
 +   * Sets a non-case-sensitive property for the given chain code. Two PDBEntry
 +   * objects which differ only in the case of their chain code are considered
 +   * equal. This avoids duplication of objects in lists of PDB ids.
 +   * 
 +   * @param chainCode
 +   */
    public void setChainCode(String chainCode)
    {
 -    if (properties == null)
 +    if (chainCode == null)
      {
 -      if (chainCode == null)
 -      {
 -        // nothing to do.
 -        return;
 -      }
 -      properties = new Hashtable();
 +      deleteProperty(CHAIN_ID);
      }
 -    if (chainCode == null)
 +    else
 +    {
 +      setProperty(CHAIN_ID, new CaseInsensitiveString(chainCode));
 +    }
 +  }
 +
 +  /**
 +   * Deletes the property with the given key, and returns the deleted value (or
 +   * null)
 +   */
 +  Object deleteProperty(String key)
 +  {
 +    Object result = null;
 +    if (properties != null)
      {
 -      properties.remove(CHAIN_ID);
 -      return;
 +      result = properties.remove(key);
      }
 -    // update property for non-null chainCode
 -    properties.put(CHAIN_ID, new CaseInsensitiveString(chainCode));
 +    return result;
    }
  
    @Override
    }
  
    /**
 +   * Getter provided for Castor binding only. Application code should call
 +   * getProperty() or getProperties() instead.
 +   * 
 +   * @deprecated
 +   * @see #getProperty(String)
 +   * @see #getProperties()
 +   * @see jalview.ws.dbsources.Uniprot#getUniprotEntries
 +   * @return
 +   */
 +  @Deprecated
 +  public Hashtable<String, Object> getProps()
 +  {
 +    return properties;
 +  }
 +
 +  /**
 +   * Setter provided for Castor binding only. Application code should call
 +   * setProperty() instead.
 +   * 
 +   * @deprecated
 +   * @return
 +   */
 +  @Deprecated
 +  public void setProps(Hashtable<String, Object> props)
 +  {
 +    properties = props;
 +  }
 +
 +  /**
-    * update entry with details from another entry concerning the same PDB
-    * ID/file spec.
+    * Answers true if this object is either equivalent to, or can be 'improved'
+    * by, the given entry.
+    * <p>
+    * If newEntry has the same id (ignoring case), and doesn't have a conflicting
+    * file spec or chain code, then update this entry from its file and/or chain
+    * code.
     * 
     * @param newEntry
     * @return true if modifications were made
     */
 -  protected boolean updateFrom(PDBEntry newEntry)
 +  public boolean updateFrom(PDBEntry newEntry)
    {
-     boolean modified = false;
+     if (this.equals(newEntry))
+     {
+       return true;
+     }
+     String newId = newEntry.getId();
+     if (newId == null || getId() == null)
+     {
+       return false; // shouldn't happen
+     }
+     /*
 -     * id (less any chain code) has to match (ignoring case)
++     * id has to match (ignoring case)
+      */
+     if (!getId().equalsIgnoreCase(newId))
+     {
+       return false;
+     }
+     /*
+      * Don't update if associated with different structure files
+      */
+     String newFile = newEntry.getFile();
+     if (newFile != null && getFile() != null && !newFile.equals(getFile()))
+     {
+       return false;
+     }
  
-     if (getFile() == null)
+     /*
+      * Don't update if associated with different chains (ignoring case)
+      */
+     String newChain = newEntry.getChainCode();
+     if (newChain != null && newChain.length() > 0 && getChainCode() != null
+             && getChainCode().length() > 0
+             && !getChainCode().equalsIgnoreCase(newChain))
      {
-       // update file and type of file
-       modified = newEntry.getFile() != null;
-       setFile(newEntry.getFile());
+       return false;
      }
-     if (newEntry.getType() != null && newEntry.getFile() != null
-             && newEntry.getFile().equals(getFile()))
+     /*
+      * set file path if not already set
+      */
+     String newType = newEntry.getType();
+     if (getFile() == null && newFile != null)
+     {
+       setFile(newFile);
+       setType(newType);
+     }
+     /*
+      * set file type if new entry has it and we don't
+      * (for the case where file was not updated)
+      */
+     if (getType() == null && newType != null)
      {
-       setType(newEntry.getType());
+       setType(newType);
      }
-     if (getChainCode() == null
-             || (getChainCode() != null && getChainCode().length() == 0 && newEntry
-                     .getChainCode() != null))
+     /*
+      * set chain if not already set (we excluded differing 
+      * chains earlier) (ignoring case change only)
+      */
+     if (newChain != null && newChain.length() > 0
+             && !newChain.equalsIgnoreCase(getChainCode()))
      {
-       modified |= (getChainCode() == null || !newEntry.getChainCode()
-               .equals(getChainCode()));
-       setChainCode(newEntry.getChainCode());
+       setChainCode(newChain);
      }
-     if (newEntry.properties != null)
+     /*
 -     * copy any new properties; notice this may include chain_code,
 -     * but we excluded differing chain codes earlier
++     * copy any new or modified properties
+      */
 -    if (newEntry.getProperty() != null)
++    Enumeration<String> newProps = newEntry.getProperties();
++    while (newProps.hasMoreElements())
      {
-       for (Entry<String, Object> entry : newEntry.properties.entrySet())
 -      if (properties == null)
++      /*
++       * copy properties unless value matches; this defends against changing
++       * the case of chain_code which is wrapped in a CaseInsensitiveString
++       */
++      String key = newProps.nextElement();
++      Object value = newEntry.getProperty(key);
++      if (!value.equals(getProperty(key)))
        {
-         if (!entry.getValue().equals(getProperty(entry.getKey())))
-         {
-           modified = true;
-         }
-         setProperty(entry.getKey(), entry.getValue());
 -        properties = new Hashtable();
 -      }
 -      for (Object p : newEntry.getProperty().keySet())
 -      {
 -        /*
 -         * copy properties unless value matches; this defends against changing
 -         * the case of chain_code which is wrapped in a CaseInsensitiveString
 -         */
 -        Object value = newEntry.getProperty().get(p);
 -        if (!value.equals(properties.get(p)))
 -        {
 -          properties.put(p, newEntry.getProperty().get(p));
 -        }
++        setProperty(key, value);
        }
      }
-     return modified;
+     return true;
    }
  }
@@@ -20,6 -20,7 +20,7 @@@
   */
  package jalview.gui;
  
+ import jalview.analysis.Conservation;
  import jalview.api.FeatureColourI;
  import jalview.api.ViewStyleI;
  import jalview.api.structures.JalviewStructureDisplayI;
@@@ -982,16 -983,17 +983,16 @@@ public class Jalview2XM
              }
            }
  
 -          if (entry.getProperty() != null && !entry.getProperty().isEmpty())
 +          Enumeration<String> props = entry.getProperties();
 +          if (props.hasMoreElements())
            {
              PdbentryItem item = new PdbentryItem();
 -            Hashtable properties = entry.getProperty();
 -            Enumeration en2 = properties.keys();
 -            while (en2.hasMoreElements())
 +            while (props.hasMoreElements())
              {
                Property prop = new Property();
 -              String key = en2.nextElement().toString();
 +              String key = props.nextElement();
                prop.setName(key);
 -              prop.setValue(properties.get(key).toString());
 +              prop.setValue(entry.getProperty(key).toString());
                item.addProperty(prop);
              }
              pdb.addPdbentryItem(item);
              }
              if (ids[p].getPdbentryItem() != null)
              {
 -              entry.setProperty(new Hashtable());
                for (PdbentryItem item : ids[p].getPdbentryItem())
                {
                  for (Property pr : item.getProperty())
                  {
 -                  entry.getProperty().put(pr.getName(), pr.getValue());
 +                  entry.setProperty(pr.getName(), pr.getValue());
                  }
                }
              }
          }
          if (jGroup.getConsThreshold() != 0)
          {
-           jalview.analysis.Conservation c = new jalview.analysis.Conservation(
-                   "All", ResidueProperties.propHash, 3,
+           Conservation c = new Conservation("All", 3,
                    sg.getSequences(null), 0, sg.getWidth() - 1);
            c.calculate();
            c.verdict(false, 25);
@@@ -15,6 -15,7 +15,6 @@@ import jalview.structure.StructureImpor
  import java.awt.Color;
  import java.io.IOException;
  import java.lang.reflect.Constructor;
 -import java.util.Hashtable;
  import java.util.List;
  import java.util.Vector;
  
@@@ -91,6 -92,7 +91,6 @@@ public abstract class StructureFile ext
    {
    }
  
 -  @SuppressWarnings("rawtypes")
    protected SequenceI postProcessChain(PDBChain chain)
    {
      SequenceI pdbSequence = chain.sequence;
      PDBEntry entry = new PDBEntry();
      entry.setId(getId());
      entry.setType(getStructureFileType());
 -    entry.setProperty(new Hashtable());
      if (chain.id != null)
      {
 -      entry.setChainCode(String.valueOf(chain.id));
 +      entry.setChainCode(chain.id);
      }
      if (inFile != null)
      {
        Class cl = Class.forName("jalview.ext.jmol.JmolParser");
        if (cl != null)
        {
-         final Constructor constructor = cl.getConstructor(new Class[] {
-             boolean.class, boolean.class, boolean.class, FileParse.class });
-         final Object[] args = new Object[] { visibleChainAnnotation,
-             predictSecondaryStructure, externalSecondaryStructure,
-             new FileParse(getDataName(), type) };
+         final Constructor constructor = cl
+                 .getConstructor(new Class[] { FileParse.class });
+         final Object[] args = new Object[] { new FileParse(getDataName(),
+                 type) };
  
          StructureImportSettings.setShowSeqFeatures(false);
          StructureImportSettings.setVisibleChainAnnotation(false);
@@@ -26,8 -26,10 +26,9 @@@ import jalview.datamodel.PDBEntry
  import jalview.datamodel.SequenceI;
  
  import java.util.ArrayList;
+ import java.util.Arrays;
  import java.util.HashMap;
  import java.util.HashSet;
 -import java.util.Hashtable;
  import java.util.List;
  import java.util.Map;
  import java.util.Set;
@@@ -300,7 -302,8 +301,8 @@@ public class DBRefUtil
      @Override
      public boolean matches(DBRefEntry refa, DBRefEntry refb)
      {
-       if (refa.getSource() != null && refb.getSource() != null
+       if (refa.getSource() != null
+               && refb.getSource() != null
                && DBRefUtils.getCanonicalName(refb.getSource()).equals(
                        DBRefUtils.getCanonicalName(refa.getSource())))
        {
      @Override
      public boolean matches(DBRefEntry refa, DBRefEntry refb)
      {
-       if (refa.getSource() != null && refb.getSource() != null
+       if (refa.getSource() != null
+               && refb.getSource() != null
                && DBRefUtils.getCanonicalName(refb.getSource()).equals(
                        DBRefUtils.getCanonicalName(refa.getSource())))
        {
      @Override
      public boolean matches(DBRefEntry refa, DBRefEntry refb)
      {
-       if (refa.getSource() != null && refb.getSource() != null
+       if (refa.getSource() != null
+               && refb.getSource() != null
                && DBRefUtils.getCanonicalName(refb.getSource()).equals(
                        DBRefUtils.getCanonicalName(refa.getSource())))
        {
      @Override
      public boolean matches(DBRefEntry refa, DBRefEntry refb)
      {
-       if (refa.getSource() != null && refb.getSource() != null
+       if (refa.getSource() != null
+               && refb.getSource() != null
                && DBRefUtils.getCanonicalName(refb.getSource()).equals(
                        DBRefUtils.getCanonicalName(refa.getSource())))
        {
            PDBEntry pdbr = new PDBEntry();
            pdbr.setId(pdbid);
            pdbr.setType(PDBEntry.Type.PDB);
 -          pdbr.setProperty(new Hashtable());
            pdbr.setChainCode(chaincode);
 -          // pdbr.getProperty().put("CHAIN", chaincode);
            seq.addPDBId(pdbr);
          }
          else
      return matches;
    }
  
+   /**
+    * promote direct database references to primary for nucleotide or protein
+    * sequences if they have an appropriate primary ref
+    * <table>
+    * <tr>
+    * <th>Seq Type</th>
+    * <th>Primary DB</th>
+    * <th>Direct which will be promoted</th>
+    * </tr>
+    * <tr align=center>
+    * <td>peptides</td>
+    * <td>Ensembl</td>
+    * <td>Uniprot</td>
+    * </tr>
+    * <tr align=center>
+    * <td>peptides</td>
+    * <td>Ensembl</td>
+    * <td>Uniprot</td>
+    * </tr>
+    * <tr align=center>
+    * <td>dna</td>
+    * <td>Ensembl</td>
+    * <td>ENA</td>
+    * </tr>
+    * </table>
+    * 
+    * @param sequence
+    */
+   public static void ensurePrimaries(SequenceI sequence)
+   {
+     List<DBRefEntry> pr = sequence.getPrimaryDBRefs();
+     if (pr.size() == 0)
+     {
+       // nothing to do
+       return;
+     }
+     List<DBRefEntry> selfs = new ArrayList<DBRefEntry>();
+     {
+       DBRefEntry[] selfArray = selectDbRefs(!sequence.isProtein(),
+               sequence.getDBRefs());
+       if (selfArray == null || selfArray.length == 0)
+       {
+         // nothing to do
+         return;
+       }
+       selfs.addAll(Arrays.asList(selfArray));
+     }
+     // filter non-primary refs
+     for (DBRefEntry p : pr)
+     {
+       while (selfs.contains(p))
+       {
+         selfs.remove(p);
+       }
+     }
+     List<DBRefEntry> toPromote = new ArrayList<DBRefEntry>();
+     for (DBRefEntry p : pr)
+     {
+       List<String> promType = new ArrayList<String>();
+       if (sequence.isProtein())
+       {
+         switch (getCanonicalName(p.getSource()))
+         {
+         case DBRefSource.UNIPROT:
+           // case DBRefSource.UNIPROTKB:
+           // case DBRefSource.UP_NAME:
+           // search for and promote ensembl
+           promType.add(DBRefSource.ENSEMBL);
+           break;
+         case DBRefSource.ENSEMBL:
+           // search for and promote Uniprot
+           promType.add(DBRefSource.UNIPROT);
+           break;
+         }
+       }
+       else
+       {
+         // TODO: promote transcript refs
+       }
+       // collate candidates and promote them
+       DBRefEntry[] candidates = selectRefs(
+               selfs.toArray(new DBRefEntry[0]),
+               promType.toArray(new String[0]));
+       if (candidates != null)
+       {
+         for (DBRefEntry cand : candidates)
+         {
+           if (cand.hasMap())
+           {
+             if (cand.getMap().getTo() != null
+                     && cand.getMap().getTo() != sequence)
+             {
+               // can't promote refs with mappings to other sequences
+               continue;
+             }
+             if (cand.getMap().getMap().getFromLowest() != sequence
+                     .getStart()
+                     && cand.getMap().getMap().getFromHighest() != sequence
+                             .getEnd())
+             {
+               // can't promote refs with mappings from a region of this sequence
+               // - eg CDS
+               continue;
+             }
+           }
+           // and promote
+           cand.setVersion(p.getVersion() + " (promoted)");
+           selfs.remove(cand);
+           toPromote.add(cand);
+           if (!cand.isPrimaryCandidate())
+           {
+             System.out.println("Warning: Couldn't promote dbref "
+                     + cand.toString() + " for sequence "
+                     + sequence.toString());
+           }
+         }
+       }
+     }
+   }
  }
@@@ -222,6 -222,40 +222,38 @@@ public class Uniprot extends DbSourcePr
        {
          onlyPdbEntries.addElement(pdb);
        }
+       if ("EMBL".equals(pdb.getType()))
+       {
+         // look for a CDS reference and add it, too.
 -        String cdsId = (String) pdb.getProperty()
 -                .get("protein sequence ID");
++        String cdsId = (String) pdb.getProperty("protein sequence ID");
+         if (cdsId != null && cdsId.trim().length() > 0)
+         {
+           // remove version
+           String[] vrs = cdsId.split("\\.");
+           dbr = new DBRefEntry(DBRefSource.EMBLCDS, vrs.length > 1 ? vrs[1]
+                   : DBRefSource.UNIPROT + ":" + dbVersion, vrs[0]);
+           dbRefs.add(dbr);
+         }
+       }
+       if ("Ensembl".equals(pdb.getType()))
+       {
+         /*UniprotXML
+          * <dbReference type="Ensembl" id="ENST00000321556">
+         * <molecule id="Q9BXM7-1"/>
+         * <property type="protein sequence ID" value="ENSP00000364204"/>
+         * <property type="gene ID" value="ENSG00000158828"/>
+         * </dbReference> 
+          */
 -        String cdsId = (String) pdb.getProperty()
 -                .get("protein sequence ID");
++        String cdsId = (String) pdb.getProperty("protein sequence ID");
+         if (cdsId != null && cdsId.trim().length() > 0)
+         {
+           dbr = new DBRefEntry(DBRefSource.ENSEMBL, DBRefSource.UNIPROT
+                   + ":" + dbVersion, cdsId.trim());
+           dbRefs.add(dbr);
+         }
+       }
      }
  
      sequence.setPDBId(onlyPdbEntries);
          sequence.addSequenceFeature(sf);
        }
      }
-     sequence.setDBRefs(dbRefs.toArray(new DBRefEntry[0]));
+     for (DBRefEntry dbr : dbRefs)
+     {
+       sequence.addDBRef(dbr);
+     }
      return sequence;
    }
  
@@@ -1,4 -1,5 +1,5 @@@
  /*
+     assertEquals(case7, case9);
   * Jalview - A Sequence Alignment Editor and Viewer ($$Version-Rel$$)
   * Copyright (C) $$Year-Rel$$ The Jalview Authors
   * 
@@@ -23,8 -24,16 +24,14 @@@ package jalview.datamodel
  import static org.testng.Assert.assertEquals;
  import static org.testng.Assert.assertFalse;
  import static org.testng.Assert.assertNotEquals;
+ import static org.testng.Assert.assertNotSame;
  import static org.testng.Assert.assertNull;
+ import static org.testng.Assert.assertSame;
+ import static org.testng.Assert.assertTrue;
+ import static org.testng.Assert.fail;
+ import jalview.datamodel.PDBEntry.Type;
  
 -import java.util.Hashtable;
 -
  //import org.testng.Assert;
  import org.testng.annotations.AfterMethod;
  import org.testng.annotations.BeforeMethod;
@@@ -49,23 -58,34 +56,34 @@@ public class PDBEntryTes
      PDBEntry pdbEntry = new PDBEntry("1xyz", "A", PDBEntry.Type.PDB,
              "x/y/z/File");
  
+     // id comparison is not case sensitive
      PDBEntry case1 = new PDBEntry("1XYZ", "A", PDBEntry.Type.PDB,
              "x/y/z/File");
+     // chain code comparison is not case sensitive
      PDBEntry case2 = new PDBEntry("1xyz", "a", PDBEntry.Type.PDB,
              "x/y/z/File");
+     // different type
      PDBEntry case3 = new PDBEntry("1xyz", "A", PDBEntry.Type.FILE,
              "x/y/z/File");
+     // different everything
      PDBEntry case4 = new PDBEntry(null, null, null, null);
+     // null id
      PDBEntry case5 = new PDBEntry(null, "A", PDBEntry.Type.PDB,
              "x/y/z/File");
+     // null chain
      PDBEntry case6 = new PDBEntry("1xyz", null, PDBEntry.Type.PDB,
              "x/y/z/File");
+     // null type
      PDBEntry case7 = new PDBEntry("1xyz", "A", null, "x/y/z/File");
+     // null file
      PDBEntry case8 = new PDBEntry("1xyz", "A", PDBEntry.Type.PDB, null);
+     // identical to case7
      PDBEntry case9 = new PDBEntry("1xyz", "A", null, "x/y/z/File");
+     // different file only
+     PDBEntry case10 = new PDBEntry("1xyz", "A", null, "a/b/c/File");
  
      /*
-      * assertions will invoke PDBEntry.equals()
+      * assertEquals will invoke PDBEntry.equals()
       */
      assertFalse(pdbEntry.equals(null));
      assertFalse(pdbEntry.equals("a"));
      assertNotEquals(case5, pdbEntry);
      assertNotEquals(case6, pdbEntry);
      assertNotEquals(case7, pdbEntry);
-     assertEquals(case8, pdbEntry);
+     assertNotEquals(case8, pdbEntry);
      assertEquals(case7, case9);
+     assertNotEquals(case9, case10);
+     // add properties
 -    case7.getProperty().put("hello", "world");
++    case7.setProperty("hello", "world");
+     assertNotEquals(case7, case9);
 -    case9.getProperty().put("hello", "world");
++    case9.setProperty("hello", "world");
+     assertEquals(case7, case9);
 -    case9.getProperty().put("hello", "WORLD");
++    case9.setProperty("hello", "WORLD");
+     assertNotEquals(case7, case9);
  
      /*
       * change string wrapper property to string...
       */
 -    case1.getProperty().put("chain_code", "a");
 +    case1.setProperty("chain_code", "a");
      assertFalse(pdbEntry.equals(case1));
      assertFalse(case1.equals(pdbEntry));
    }
      pdbEntry.setChainCode(null);
      assertNull(pdbEntry.getChainCode());
    }
+   @Test(groups = { "Functional" })
+   public void testGetType()
+   {
+     assertSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("FILE"));
+     assertSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("File"));
+     assertSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("file"));
+     assertNotSame(PDBEntry.Type.FILE, PDBEntry.Type.getType("file "));
+   }
+   @Test(groups = { "Functional" })
+   public void testTypeMatches()
+   {
+     // TODO Type.matches() is not used - delete?
+     assertTrue(PDBEntry.Type.FILE.matches("FILE"));
+     assertTrue(PDBEntry.Type.FILE.matches("File"));
+     assertTrue(PDBEntry.Type.FILE.matches("file"));
+     assertFalse(PDBEntry.Type.FILE.matches("FILE "));
+   }
+   @Test(groups = { "Functional" })
+   public void testUpdateFrom()
+   {
+     PDBEntry pdb1 = new PDBEntry("3A6S", null, null, null);
+     PDBEntry pdb2 = new PDBEntry("3A6S", null, null, null);
+     assertTrue(pdb1.updateFrom(pdb2));
+     /*
+      * mismatch of pdb id not allowed
+      */
+     pdb2 = new PDBEntry("1A70", "A", null, null);
+     assertFalse(pdb1.updateFrom(pdb2));
+     assertNull(pdb1.getChainCode());
+     /*
+      * match of pdb id is not case sensitive
+      */
+     pdb2 = new PDBEntry("3a6s", "A", null, null);
+     assertTrue(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getChainCode(), "A");
+     assertEquals(pdb1.getId(), "3A6S");
+     /*
+      * add chain - with differing case for id
+      */
+     pdb1 = new PDBEntry("3A6S", null, null, null);
+     pdb2 = new PDBEntry("3a6s", "A", null, null);
+     assertTrue(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getChainCode(), "A");
+     /*
+      * change of chain is not allowed
+      */
+     pdb2 = new PDBEntry("3A6S", "B", null, null);
+     assertFalse(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getChainCode(), "A");
+     /*
+      * change chain from null
+      */
+     pdb1 = new PDBEntry("3A6S", null, null, null);
+     pdb2 = new PDBEntry("3A6S", "B", null, null);
+     assertTrue(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getChainCode(), "B");
+     /*
+      * set file and type
+      */
+     pdb2 = new PDBEntry("3A6S", "B", Type.FILE, "filePath");
+     assertTrue(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getFile(), "filePath");
+     assertEquals(pdb1.getType(), Type.FILE.toString());
+     /*
+      * change of file is not allowed
+      */
+     pdb1 = new PDBEntry("3A6S", null, null, "file1");
+     pdb2 = new PDBEntry("3A6S", "A", null, "file2");
+     assertFalse(pdb1.updateFrom(pdb2));
+     assertNull(pdb1.getChainCode());
+     assertEquals(pdb1.getFile(), "file1");
+     /*
+      * set type without change of file
+      */
+     pdb1 = new PDBEntry("3A6S", null, null, "file1");
+     pdb2 = new PDBEntry("3A6S", null, Type.PDB, "file1");
+     assertTrue(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getType(), Type.PDB.toString());
+     /*
+      * set file with differing case of id and chain code
+      */
+     pdb1 = new PDBEntry("3A6S", "A", null, null);
+     pdb2 = new PDBEntry("3a6s", "a", Type.PDB, "file1");
+     assertTrue(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getType(), Type.PDB.toString());
+     assertEquals(pdb1.getId(), "3A6S"); // unchanged
+     assertEquals(pdb1.getFile(), "file1"); // updated
+     assertEquals(pdb1.getChainCode(), "A"); // unchanged
+     /*
+      * changing nothing returns true
+      */
+     pdb1 = new PDBEntry("3A6S", "A", Type.PDB, "file1");
+     pdb2 = new PDBEntry("3A6S", null, null, null);
+     assertTrue(pdb1.updateFrom(pdb2));
+     assertEquals(pdb1.getChainCode(), "A");
+     assertEquals(pdb1.getType(), Type.PDB.toString());
+     assertEquals(pdb1.getFile(), "file1");
+     /*
+      * add and update properties only
+      */
+     pdb1 = new PDBEntry("3A6S", null, null, null);
+     pdb2 = new PDBEntry("3A6S", null, null, null);
 -    // ughh properties not null if chain code has been set...
 -    // JAL-2196 addresses this
 -    pdb1.properties = new Hashtable();
 -    pdb2.properties = new Hashtable();
 -    pdb1.properties.put("destination", "mars");
 -    pdb1.properties.put("hello", "world");
 -    pdb2.properties.put("hello", "moon");
 -    pdb2.properties.put("goodbye", "world");
++    pdb1.setProperty("destination", "mars");
++    pdb1.setProperty("hello", "world");
++    pdb2.setProperty("hello", "moon");
++    pdb2.setProperty("goodbye", "world");
+     assertTrue(pdb1.updateFrom(pdb2));
 -    assertEquals(pdb1.properties.get("destination"), "mars");
 -    assertEquals(pdb1.properties.get("hello"), "moon");
 -    assertEquals(pdb1.properties.get("goodbye"), "world");
++    assertEquals(pdb1.getProperty("destination"), "mars");
++    assertEquals(pdb1.getProperty("hello"), "moon");
++    assertEquals(pdb1.getProperty("goodbye"), "world");
+     /*
+      * add properties only
+      */
+     pdb1 = new PDBEntry("3A6S", null, null, null);
+     pdb2 = new PDBEntry("3A6S", null, null, null);
 -    pdb2.properties = new Hashtable();
 -    pdb2.properties.put("hello", "moon");
++    pdb2.setProperty("hello", "moon");
+     assertTrue(pdb1.updateFrom(pdb2));
 -    assertEquals(pdb1.properties.get("hello"), "moon");
++    assertEquals(pdb1.getProperty("hello"), "moon");
+   }
+   @Test(groups = { "Functional" })
+   public void testConstructor_fromDbref()
+   {
+     PDBEntry pdb = new PDBEntry(new DBRefEntry("PDB", "0", "1A70"));
+     assertEquals(pdb.getId(), "1A70");
+     assertNull(pdb.getChainCode());
+     assertNull(pdb.getType());
+     assertNull(pdb.getFile());
+     /*
+      * from dbref with chain code appended
+      */
+     pdb = new PDBEntry(new DBRefEntry("PDB", "0", "1A70B"));
+     assertEquals(pdb.getId(), "1A70");
+     assertEquals(pdb.getChainCode(), "B");
+     /*
+      * from dbref with overlong accession
+      */
+     pdb = new PDBEntry(new DBRefEntry("PDB", "0", "1A70BC"));
+     assertEquals(pdb.getId(), "1A70BC");
+     assertNull(pdb.getChainCode());
+     /*
+      * from dbref which is not for PDB
+      */
+     try
+     {
+       pdb = new PDBEntry(new DBRefEntry("PDBe", "0", "1A70"));
+       fail("Expected exception");
+     } catch (IllegalArgumentException e)
+     {
+       // expected;
+     }
+   }
  }
  package jalview.ws.dbsources;
  
  import static org.testng.AssertJUnit.assertEquals;
 +import static org.testng.AssertJUnit.assertFalse;
+ import static org.testng.AssertJUnit.assertNotNull;
  import static org.testng.AssertJUnit.assertNull;
  
  import jalview.datamodel.PDBEntry;
  import jalview.datamodel.SequenceFeature;
+ import jalview.datamodel.SequenceI;
  import jalview.datamodel.UniprotEntry;
  
  import java.io.Reader;
@@@ -47,6 -48,7 +49,7 @@@ public class UniprotTes
            + "<protein><recommendedName><fullName>Mitogen-activated protein kinase 13</fullName><fullName>Henry</fullName></recommendedName></protein>"
            + "<dbReference type=\"PDB\" id=\"2FSQ\"><property type=\"method\" value=\"X-ray\"/><property type=\"resolution\" value=\"1.40\"/></dbReference>"
            + "<dbReference type=\"PDBsum\" id=\"2FSR\"/>"
+           + "<dbReference type=\"EMBL\" id=\"AE007869\"><property type=\"protein sequence ID\" value=\"AAK85932.1\"/><property type=\"molecule type\" value=\"Genomic_DNA\"/></dbReference>"
            + "<feature type=\"signal peptide\" evidence=\"7\"><location><begin position=\"1\"/><end position=\"18\"/></location></feature>"
            + "<feature type=\"propeptide\" description=\"Activation peptide\" id=\"PRO_0000027399\" evidence=\"9 16 17 18\"><location><begin position=\"19\"/><end position=\"20\"/></location></feature>"
            + "<feature type=\"chain\" description=\"Granzyme B\" id=\"PRO_0000027400\"><location><begin position=\"21\"/><end position=\"247\"/></location></feature>"
       * Check cross-references
       */
      Vector<PDBEntry> xrefs = entry.getDbReference();
-     assertEquals(2, xrefs.size());
+     assertEquals(3, xrefs.size());
  
      PDBEntry xref = xrefs.get(0);
      assertEquals("2FSQ", xref.getId());
      assertEquals("PDB", xref.getType());
 -    assertEquals(2, xref.getProperty().size());
 -    assertEquals("X-ray", xref.getProperty().get("method"));
 -    assertEquals("1.40", xref.getProperty().get("resolution"));
 +    assertEquals("X-ray", xref.getProperty("method"));
 +    assertEquals("1.40", xref.getProperty("resolution"));
  
      xref = xrefs.get(1);
      assertEquals("2FSR", xref.getId());
      assertEquals("PDBsum", xref.getType());
 -    assertNull(xref.getProperty());
 +    assertFalse(xref.getProperties().hasMoreElements());
+     xref = xrefs.get(2);
+     assertEquals("AE007869", xref.getId());
+     assertEquals("EMBL", xref.getType());
 -    assertNotNull(xref.getProperty());
+     assertEquals("AAK85932.1",
 -            (String) xref.getProperty().get("protein sequence ID"));
++ xref.getProperty("protein sequence ID"));
+     assertEquals("Genomic_DNA",
 -            (String) xref.getProperty().get("molecule type"));
 -    assertEquals(2, xref.getProperty().size());
 -
++ xref.getProperty("molecule type"));
    }
  
+   @Test(groups = { "Functional" })
+   public void testGetUniprotSequence()
+   {
+     UniprotEntry entry = new Uniprot().getUniprotEntries(
+             new StringReader(UNIPROT_XML)).get(0);
+     SequenceI seq = new Uniprot().uniprotEntryToSequenceI(entry);
+     assertNotNull(seq);
+     assertEquals(6, seq.getDBRefs().length); // 2*Uniprot, PDB, PDBsum, 2*EMBL
+   }
    /**
     * Test the method that formats the sequence id
     */