From ea36a0b44f1661422284e55ad5e137a0c81821f1 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Tue, 31 May 2016 17:29:02 +0100 Subject: [PATCH] JAL-2110 refactor + tests to allow a dbref version to update "0" or "source:0" --- src/jalview/api/DBRefEntryI.java | 17 ++++++ src/jalview/datamodel/DBRefEntry.java | 77 ++++++++++++++++++++++++++ src/jalview/datamodel/Sequence.java | 27 +++++---- test/jalview/datamodel/DBRefEntryTest.java | 78 ++++++++++++++++++++++++++ test/jalview/datamodel/SequenceTest.java | 82 ++++++++++++++++++++++++++++ 5 files changed, 267 insertions(+), 14 deletions(-) diff --git a/src/jalview/api/DBRefEntryI.java b/src/jalview/api/DBRefEntryI.java index 490b21a..49858cb 100644 --- a/src/jalview/api/DBRefEntryI.java +++ b/src/jalview/api/DBRefEntryI.java @@ -1,5 +1,6 @@ package jalview.api; +import jalview.datamodel.DBRefEntry; import jalview.datamodel.Mapping; //JBPComment: this is a datamodel API - so it should be in datamodel (it's a peer of SequenceI) @@ -74,4 +75,20 @@ public interface DBRefEntryI public int getEndRes(); public Mapping getMap(); + + /** + * Answers true if this object is either equivalent to, or can be 'improved' + * by, the given entry. Specifically, answers true if + * + * + * @param otherEntry + * @return + */ + public boolean updateFrom(DBRefEntry otherEntry); } diff --git a/src/jalview/datamodel/DBRefEntry.java b/src/jalview/datamodel/DBRefEntry.java index 53642b5..738c4dc 100755 --- a/src/jalview/datamodel/DBRefEntry.java +++ b/src/jalview/datamodel/DBRefEntry.java @@ -98,6 +98,82 @@ public class DBRefEntry implements DBRefEntryI } /** + * Answers true if this object is either equivalent to, or can be 'improved' + * by, the given entry. Specifically, answers true if + * + * + * @param other + * @return + */ + @Override + public boolean updateFrom(DBRefEntry other) + { + if (other == null) + { + return false; + } + if (other == this) + { + return true; + } + + /* + * source must either match or be both null + */ + String otherSource = other.getSource(); + if ((source == null && otherSource != null) + || (source != null && otherSource == null) + || (source != null && !source.equalsIgnoreCase(otherSource))) + { + return false; + } + + /* + * accession id must either match or be both null + */ + String otherAccession = other.getAccessionId(); + if ((accessionId == null && otherAccession != null) + || (accessionId != null && otherAccession == null) + || (accessionId != null && !accessionId.equalsIgnoreCase(otherAccession))) + { + return false; + } + + /* + * if my version is null, "0" or "source:0" then replace with other version, + * otherwise the versions have to match + */ + String otherVersion = other.getVersion(); + if ((version == null || version.equals("0") || version.endsWith(":0")) + && otherVersion != null) + { + setVersion(otherVersion); + } + else + { + if (!version.equalsIgnoreCase(otherVersion)) + { + return false; + } + } + + /* + * if I have no mapping, take that of the other dbref + */ + if (map == null) + { + setMap(other.getMap()); + } + return true; + } + + /** * test for similar DBRef attributes, except for the map object. * * @param entry @@ -106,6 +182,7 @@ public class DBRefEntry implements DBRefEntryI @Override public boolean equalRef(DBRefEntryI entry) { + // TODO is this method and equals() not needed? if (entry == null) { return false; diff --git a/src/jalview/datamodel/Sequence.java b/src/jalview/datamodel/Sequence.java index a61f093..151d8c4 100755 --- a/src/jalview/datamodel/Sequence.java +++ b/src/jalview/datamodel/Sequence.java @@ -966,26 +966,25 @@ public class Sequence extends ASequence implements SequenceI dbrefs = new DBRefEntry[0]; } - int i, iSize = dbrefs.length; - - for (i = 0; i < iSize; i++) + for (DBRefEntryI dbr : dbrefs) { - if (dbrefs[i].equalRef(entry)) + if (dbr.updateFrom(entry)) { - if (entry.getMap() != null) - { - if (dbrefs[i].getMap() == null) - { - // overwrite with 'superior' entry that contains a mapping. - dbrefs[i] = entry; - } - } + /* + * found a dbref that either matched, or could be + * updated from, the new entry - no need to add it + */ return; } } - DBRefEntry[] temp = new DBRefEntry[iSize + 1]; - System.arraycopy(dbrefs, 0, temp, 0, iSize); + /* + * extend the array to make room for one more + */ + // TODO use an ArrayList instead + int j = dbrefs.length; + DBRefEntry[] temp = new DBRefEntry[j + 1]; + System.arraycopy(dbrefs, 0, temp, 0, j); temp[temp.length - 1] = entry; dbrefs = temp; diff --git a/test/jalview/datamodel/DBRefEntryTest.java b/test/jalview/datamodel/DBRefEntryTest.java index b3376a6..ae6dcda 100644 --- a/test/jalview/datamodel/DBRefEntryTest.java +++ b/test/jalview/datamodel/DBRefEntryTest.java @@ -20,7 +20,9 @@ */ package jalview.datamodel; +import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertFalse; +import static org.testng.AssertJUnit.assertSame; import static org.testng.AssertJUnit.assertTrue; import jalview.util.MapList; @@ -60,4 +62,80 @@ public class DBRefEntryTest assertTrue(ref1.equalRef(ref2)); assertTrue(ref2.equalRef(ref1)); } + + /** + * Tests for the method that may update a DBRefEntry from another with a + * mapping or 'real' version + */ + @Test(groups = { "Functional" }) + public void testUpdateFrom() + { + DBRefEntry ref1 = new DBRefEntry("UNIPROT", "1", "V71633"); + + assertFalse(ref1.updateFrom(null)); + + /* + * equivalent other dbref + */ + DBRefEntry ref2 = new DBRefEntry("uniprot", "1", "v71633"); + assertTrue(ref1.updateFrom(ref2)); + assertEquals("UNIPROT", ref1.getSource()); // unchanged + assertEquals("V71633", ref1.getAccessionId()); // unchanged + + /* + * ref1 has no mapping, acquires mapping from ref2 + */ + Mapping map = new Mapping(new MapList(new int[] { 1, 3 }, new int[] { + 1, 1 }, 3, 1)); + ref2.setMap(map); + assertTrue(ref1.updateFrom(ref2)); + assertSame(map, ref1.getMap()); // null mapping updated + + /* + * ref1 has a mapping, does not acquire mapping from ref2 + */ + ref2.setMap(new Mapping(map)); + assertTrue(ref1.updateFrom(ref2)); + assertSame(map, ref1.getMap()); // non-null mapping not updated + + /* + * ref2 has a different source, accession or version + */ + ref2.setSource("pdb"); + assertFalse(ref1.updateFrom(ref2)); + ref2.setSource(ref1.getSource()); + ref2.setAccessionId("P12345"); + assertFalse(ref1.updateFrom(ref2)); + ref2.setAccessionId(ref1.getAccessionId()); + ref1.setVersion("2"); + assertFalse(ref1.updateFrom(ref2)); + + /* + * a non-null version supersedes "0" or "source:0" + */ + ref2.setVersion(null); + assertFalse(ref1.updateFrom(ref2)); + assertEquals("2", ref1.getVersion()); + ref2.setVersion("3"); + ref1.setVersion("0"); + assertTrue(ref1.updateFrom(ref2)); + assertEquals("3", ref1.getVersion()); + ref1.setVersion("UNIPROT:0"); + assertTrue(ref1.updateFrom(ref2)); + assertEquals("3", ref1.getVersion()); + + /* + * version "source:n" with n>0 is not superseded + */ + ref1.setVersion("UNIPROT:1"); + assertFalse(ref1.updateFrom(ref2)); + assertEquals("UNIPROT:1", ref1.getVersion()); + + /* + * version "10" is not superseded + */ + ref1.setVersion("10"); + assertFalse(ref1.updateFrom(ref2)); + assertEquals("10", ref1.getVersion()); + } } diff --git a/test/jalview/datamodel/SequenceTest.java b/test/jalview/datamodel/SequenceTest.java index 03c4a96..17dfcdc 100644 --- a/test/jalview/datamodel/SequenceTest.java +++ b/test/jalview/datamodel/SequenceTest.java @@ -29,6 +29,7 @@ import static org.testng.AssertJUnit.assertTrue; import static org.testng.internal.junit.ArrayAsserts.assertArrayEquals; import jalview.datamodel.PDBEntry.Type; +import jalview.util.MapList; import java.util.ArrayList; import java.util.Arrays; @@ -616,4 +617,85 @@ public class SequenceTest assertEquals(' ', sq.getCharAt(5)); assertEquals(' ', sq.getCharAt(-1)); } + + /** + * Tests for adding (or updating) dbrefs + * + * @see DBRefEntry#updateFrom(DBRefEntry) + */ + @Test(groups = { "Functional" }) + public void testAddDBRef() + { + SequenceI sq = new Sequence("", "abcde"); + assertNull(sq.getDBRefs()); + DBRefEntry dbref = new DBRefEntry("Uniprot", "1", "P00340"); + sq.addDBRef(dbref); + assertEquals(1, sq.getDBRefs().length); + assertSame(dbref, sq.getDBRefs()[0]); + + /* + * change of version - new entry + */ + DBRefEntry dbref2 = new DBRefEntry("Uniprot", "2", "P00340"); + sq.addDBRef(dbref2); + assertEquals(2, sq.getDBRefs().length); + assertSame(dbref, sq.getDBRefs()[0]); + assertSame(dbref2, sq.getDBRefs()[1]); + + /* + * matches existing entry - not added + */ + sq.addDBRef(new DBRefEntry("UNIPROT", "1", "p00340")); + assertEquals(2, sq.getDBRefs().length); + + /* + * different source = new entry + */ + DBRefEntry dbref3 = new DBRefEntry("UniRef", "1", "p00340"); + sq.addDBRef(dbref3); + assertEquals(3, sq.getDBRefs().length); + assertSame(dbref3, sq.getDBRefs()[2]); + + /* + * different ref = new entry + */ + DBRefEntry dbref4 = new DBRefEntry("UniRef", "1", "p00341"); + sq.addDBRef(dbref4); + assertEquals(4, sq.getDBRefs().length); + assertSame(dbref4, sq.getDBRefs()[3]); + + /* + * matching ref with a mapping - map updated + */ + DBRefEntry dbref5 = new DBRefEntry("UniRef", "1", "p00341"); + Mapping map = new Mapping(new MapList(new int[] { 1, 3 }, new int[] { + 1, 1 }, 3, 1)); + dbref5.setMap(map); + sq.addDBRef(dbref5); + assertEquals(4, sq.getDBRefs().length); + assertSame(dbref4, sq.getDBRefs()[3]); + assertSame(map, dbref4.getMap()); + + /* + * 'real' version replaces "0" version + */ + dbref2.setVersion("0"); + DBRefEntry dbref6 = new DBRefEntry(dbref2.getSource(), "3", + dbref2.getAccessionId()); + sq.addDBRef(dbref6); + assertEquals(4, sq.getDBRefs().length); + assertSame(dbref2, sq.getDBRefs()[1]); + assertEquals("3", dbref2.getVersion()); + + /* + * 'real' version replaces "source:0" version + */ + dbref3.setVersion("Uniprot:0"); + DBRefEntry dbref7 = new DBRefEntry(dbref3.getSource(), "3", + dbref3.getAccessionId()); + sq.addDBRef(dbref7); + assertEquals(4, sq.getDBRefs().length); + assertSame(dbref3, sq.getDBRefs()[2]); + assertEquals("3", dbref2.getVersion()); + } } -- 1.7.10.2