diff --git a/CHANGELOG b/CHANGELOG index 3b16037af4e80e89e3a1719d29e925c8292f661c..50cbc396910c7abfe7a6a48248050690d91cdc50 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,9 @@ minerva (12.2.0~beta.3) unstable; urgency=medium * Bug fix: Icons are still not properly loaded on Safari (#661) + * Bug fix: fetching information about publication with invalid (non numeric) + pubmed id triggered exception (#737) + + -- Piotr Gawron <piotr.gawron@uni.lu> Wed, 6 Mar 2019 17:00:00 +0200 minerva (12.2.0~beta.2) unstable; urgency=medium * Bug fix: order of the overlays is defined explicitly also for general diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImpl.java index ed861f04d7d492efad523f1d8288a8ea4171931d..1ebfa3e314dd3c1b2d86fa27b890bf5afea5df84 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImpl.java @@ -11,6 +11,7 @@ import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; +import org.apache.commons.lang3.math.NumberUtils; import org.apache.log4j.Logger; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -21,6 +22,7 @@ import lcsb.mapviewer.annotation.services.PubmedParser; import lcsb.mapviewer.annotation.services.PubmedSearchException; import lcsb.mapviewer.api.BaseRestImpl; import lcsb.mapviewer.api.QueryException; +import lcsb.mapviewer.common.comparator.IntegerComparator; import lcsb.mapviewer.common.exception.InvalidArgumentException; import lcsb.mapviewer.model.map.BioEntity; import lcsb.mapviewer.model.map.MiriamData; @@ -89,7 +91,7 @@ public class PublicationsRestImpl extends BaseRestImpl { this.searchService = searchService; } - private enum SortColumn { + enum SortColumn { PUBMED_ID("pubmedId"), YEAR("year"), JOURNAL("journal"), @@ -122,8 +124,8 @@ public class PublicationsRestImpl extends BaseRestImpl { List<Map.Entry<MiriamData, List<BioEntity>>> filteredList = new ArrayList<>(); for (Map.Entry<MiriamData, List<BioEntity>> entry : publications.entrySet()) { - Set<Model> publicationModels= new HashSet<>(); - for (BioEntity bioEntity: entry.getValue()) { + Set<Model> publicationModels = new HashSet<>(); + for (BioEntity bioEntity : entry.getValue()) { publicationModels.add(bioEntity.getModel()); } if (isSearchResult(entry.getKey(), search, publicationModels)) { @@ -159,7 +161,7 @@ public class PublicationsRestImpl extends BaseRestImpl { return result; } - private Comparator<Entry<MiriamData, List<BioEntity>>> getComparatorForColumn(SortColumn sortColumnEnum, + Comparator<Entry<MiriamData, List<BioEntity>>> getComparatorForColumn(SortColumn sortColumnEnum, String sortOrder) { final int orderFactor; if (sortOrder.toLowerCase().equals("desc")) { @@ -171,13 +173,16 @@ public class PublicationsRestImpl extends BaseRestImpl { return null; } else if (sortColumnEnum.equals(SortColumn.PUBMED_ID)) { return new Comparator<Map.Entry<MiriamData, List<BioEntity>>>() { + IntegerComparator integerComparator = new IntegerComparator(); + @Override public int compare(Entry<MiriamData, List<BioEntity>> o1, Entry<MiriamData, List<BioEntity>> o2) { - Integer id1 = Integer.valueOf(o1.getKey().getResource()); - Integer id2 = Integer.valueOf(o2.getKey().getResource()); - return id1.compareTo(id2) * orderFactor; + Integer id1 = extractPubmedId(o1.getKey()); + Integer id2 = extractPubmedId(o2.getKey()); + return integerComparator.compare(id1, id2) * orderFactor; } + }; } else if (sortColumnEnum.equals(SortColumn.YEAR)) { return new Comparator<Map.Entry<MiriamData, List<BioEntity>>>() { @@ -310,4 +315,13 @@ public class PublicationsRestImpl extends BaseRestImpl { return pubmedParser.getPubmedArticleById(Integer.valueOf(resource)); } + private Integer extractPubmedId(MiriamData md) { + if (NumberUtils.isDigits(md.getResource())) { + return Integer.valueOf(md.getResource()); + } else { + return null; + } + + } + } diff --git a/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImplTest.java b/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImplTest.java index 0cfca04523e45e92496b967c467e4f3a10a6169a..ee528a95c08a5c9b5191931731e219f29f5162ba 100644 --- a/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImplTest.java +++ b/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/publications/PublicationsRestImplTest.java @@ -1,13 +1,18 @@ package lcsb.mapviewer.api.projects.models.publications; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import java.util.ArrayList; +import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.TreeMap; +import java.util.Map.Entry; import org.apache.log4j.Logger; import org.junit.After; @@ -19,6 +24,10 @@ import org.springframework.beans.factory.annotation.Autowired; import lcsb.mapviewer.api.QueryException; import lcsb.mapviewer.api.RestTestFunctions; +import lcsb.mapviewer.api.projects.models.publications.PublicationsRestImpl.SortColumn; +import lcsb.mapviewer.model.map.BioEntity; +import lcsb.mapviewer.model.map.MiriamData; +import lcsb.mapviewer.model.map.MiriamType; import lcsb.mapviewer.model.map.model.Model; import lcsb.mapviewer.services.interfaces.IModelService; @@ -124,4 +133,48 @@ public class PublicationsRestImplTest extends RestTestFunctions { return _projectRestImpl; } + @Test + public void testComparatorForColumnWithInvalidData() { + Entry<MiriamData, List<BioEntity>> valid = new Entry<MiriamData, List<BioEntity>>() { + + @Override + public List<BioEntity> setValue(List<BioEntity> value) { + return null; + } + + @Override + public List<BioEntity> getValue() { + return new ArrayList<>(); + } + + @Override + public MiriamData getKey() { + return new MiriamData(MiriamType.PUBMED, "12345"); + } + }; + Entry<MiriamData, List<BioEntity>> invalid = new Entry<MiriamData, List<BioEntity>>() { + + @Override + public List<BioEntity> setValue(List<BioEntity> value) { + return null; + } + + @Override + public List<BioEntity> getValue() { + return new ArrayList<>(); + } + + @Override + public MiriamData getKey() { + return new MiriamData(MiriamType.PUBMED, ""); + } + }; + for (SortColumn sortColumn : SortColumn.values()) { + Comparator<Entry<MiriamData, List<BioEntity>>> comparator = _projectRestImpl.getComparatorForColumn(sortColumn, + "desc"); + assertNotNull(comparator.compare(valid, invalid)); + + } + } + }