-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mock Entrez PMIDlookup #112
Changes from 12 commits
a28113e
4e45c33
63c19e9
af4b0b2
cdd8be1
24d068e
c788731
83eecf1
53764c0
9285537
35f496a
a17245d
df22988
a196970
8084e37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ | |
<name>NIHMS Data Transform/Load</name> | ||
|
||
<properties> | ||
<wiremock-version>3.3.1</wiremock-version> | ||
<maven-model.version>3.9.6</maven-model.version> | ||
</properties> | ||
|
||
|
@@ -130,6 +131,12 @@ | |
<version>${commons-io.version}</version> | ||
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.wiremock</groupId> | ||
<artifactId>wiremock-standalone</artifactId> | ||
<version>${wiremock-version}</version> | ||
<scope>test</scope> | ||
</dependency> | ||
</dependencies> | ||
|
||
<build> | ||
|
@@ -155,6 +162,38 @@ | |
</execution> | ||
</executions> | ||
</plugin> | ||
|
||
<plugin> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not needed, it is declared in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-dependency-plugin</artifactId> | ||
<executions> | ||
<execution> | ||
<id>analyze-dependencies</id> | ||
<goals> | ||
<goal>analyze-only</goal> | ||
</goals> | ||
<phase>test-compile</phase> | ||
<configuration> | ||
<failOnWarning>true</failOnWarning> | ||
<ignoredUsedUndeclaredDependencies> | ||
<!-- These both come from junit-jupiter, so version is tied to that direct dependency --> | ||
<ignoredUsedUndeclaredDependency>org.junit.jupiter:junit-jupiter-api:</ignoredUsedUndeclaredDependency> | ||
<!-- Comes from httpclient, transitive deps --> | ||
<ignoredUsedUndeclaredDependency>org.apache.httpcomponents:httpcore:</ignoredUsedUndeclaredDependency> | ||
<!-- These come from testcontainers junit-jupiter --> | ||
<ignoredUsedUndeclaredDependency>org.testcontainers::</ignoredUsedUndeclaredDependency> | ||
</ignoredUsedUndeclaredDependencies> | ||
<ignoredNonTestScopedDependencies> | ||
<!-- junit-jupiter is a module containing the junit api jars used directly --> | ||
<ignoredNonTestScopedDependency>org.junit.jupiter:junit-jupiter-api:</ignoredNonTestScopedDependency> | ||
<!-- Needed for when data-transform jar is created --> | ||
<ignoredNonTestScopedDependency>org.json:json:</ignoredNonTestScopedDependency> | ||
</ignoredNonTestScopedDependencies> | ||
</configuration> | ||
</execution> | ||
</executions> | ||
</plugin> | ||
|
||
</plugins> | ||
</build> | ||
</project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
import org.json.JSONObject; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.stereotype.Component; | ||
|
||
/** | ||
|
@@ -54,8 +55,11 @@ public class PmidLookup { | |
* delayed responses. | ||
* https://www.ncbi.nlm.nih.gov/books/NBK25497/ | ||
*/ | ||
private static final String DEFAULT_ENTREZ_PATH = "https://eutils.ncbi.nlm.nih.gov/entrez/eutils/esummary" + | ||
".fcgi?db=pubmed&retmode=json&rettype=abstract&id=%s"; | ||
/*private static final String DEFAULT_ENTREZ_PATH = "https://eutils.ncbi.nlm.nih.gov/entrez/eutils/esummary" + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can delete this too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
".fcgi?db=pubmed&retmode=json&rettype=abstract&id=%s";*/ | ||
@Value("${pmc.entrez.service.url}") | ||
private String ENTREZ_PATH; | ||
|
||
private static final Long DEFAULT_ENTREZ_TIME_OUT = Long.valueOf("400"); | ||
|
||
private static final String JSON_ERROR_KEY = "error"; | ||
|
@@ -92,6 +96,7 @@ public JSONObject retrievePubMedRecordAsJson(String pmid) { | |
jsonRecord = retrieveJsonFromApi(pmid); | ||
if (jsonRecord == null) { | ||
// pause and retry once to allow for API limitations | ||
LOG.info("Entrez URL:",ENTREZ_PATH); | ||
LOG.info("Pausing before trying to pull PMID {} from Entrez again", pmid); | ||
TimeUnit.MILLISECONDS.sleep(DEFAULT_ENTREZ_TIME_OUT); | ||
jsonRecord = retrieveJsonFromApi(pmid); | ||
|
@@ -111,7 +116,7 @@ public JSONObject retrievePubMedRecordAsJson(String pmid) { | |
*/ | ||
private JSONObject retrieveJsonFromApi(String pmid) { | ||
JSONObject root; | ||
String path = String.format(DEFAULT_ENTREZ_PATH, pmid); | ||
String path = String.format(ENTREZ_PATH, pmid); | ||
try { | ||
HttpClient client = HttpClientBuilder | ||
.create() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,41 +15,96 @@ | |
*/ | ||
package org.eclipse.pass.loader.nihms.entrez; | ||
|
||
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; | ||
import static com.github.tomakehurst.wiremock.client.WireMock.get; | ||
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; | ||
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
||
import java.io.IOException; | ||
import java.net.URISyntaxException; | ||
import java.nio.charset.StandardCharsets; | ||
|
||
import com.github.tomakehurst.wiremock.client.WireMock; | ||
import com.github.tomakehurst.wiremock.junit5.WireMockTest; | ||
import org.apache.commons.io.IOUtils; | ||
import org.eclipse.pass.loader.nihms.NihmsTransformLoadCLIRunner; | ||
import org.json.JSONObject; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.boot.test.context.SpringBootTest; | ||
import org.springframework.boot.test.mock.mockito.MockBean; | ||
import org.springframework.test.context.TestPropertySource; | ||
|
||
/** | ||
* @author Karen Hanson | ||
* @version $Id$ | ||
*/ | ||
@SpringBootTest | ||
@TestPropertySource("classpath:test-application.properties") | ||
@WireMockTest(httpPort = 9911) | ||
public class EntrezPmidLookupTest { | ||
|
||
@Autowired | ||
protected PmidLookup pmidLookup; | ||
|
||
// Needed so tests can run after application starts | ||
@MockBean private NihmsTransformLoadCLIRunner nihmsTransformLoadCLIRunner; | ||
|
||
@Value("${pmc.entrez.service.url}") | ||
private String ENTREZ_PATH; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can be deleted, not being used looks like. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
@Test | ||
public void testGetEntrezRecordJson() { | ||
PmidLookup apiService = new PmidLookup(); | ||
public void testGetEntrezRecordJson() throws IOException, URISyntaxException { | ||
String entrez = IOUtils.toString(getClass().getClassLoader(). | ||
getResourceAsStream("pmidrecord.json"), StandardCharsets.UTF_8); | ||
|
||
String pmid = "29249144"; | ||
String pmid = "11111111"; | ||
|
||
JSONObject pmr = apiService.retrievePubMedRecordAsJson(pmid); | ||
assertTrue(pmr.getString("source").contains("Proteome")); | ||
stubFor(get(urlPathEqualTo("/entrez/eutils/esummary.fcgi")) | ||
.withQueryParam("db", WireMock.equalTo("pubmed")) | ||
.withQueryParam("retmode", WireMock.equalTo("json")) | ||
.withQueryParam("rettype", WireMock.equalTo("abstract")) | ||
.withQueryParam("id", WireMock.equalTo(pmid)) | ||
.willReturn(aResponse().withStatus(200).withBody(entrez))); | ||
|
||
JSONObject pmr = pmidLookup.retrievePubMedRecordAsJson(pmid); | ||
assertTrue(pmr.getString("source").contains("Journal A")); | ||
} | ||
|
||
@Test | ||
public void testGetPubMedRecord() { | ||
PmidLookup pmidLookup = new PmidLookup(); | ||
String pmid = "29249144"; | ||
public void testGetPubMedRecord() throws IOException { | ||
String entrez = IOUtils.toString(getClass().getClassLoader(). | ||
getResourceAsStream("pmidrecord.json"), StandardCharsets.UTF_8); | ||
|
||
String pmid = "11111111"; | ||
|
||
stubFor(get(urlPathEqualTo("/entrez/eutils/esummary.fcgi")) | ||
.withQueryParam("db", WireMock.equalTo("pubmed")) | ||
.withQueryParam("retmode", WireMock.equalTo("json")) | ||
.withQueryParam("rettype", WireMock.equalTo("abstract")) | ||
.withQueryParam("id", WireMock.equalTo(pmid)) | ||
.willReturn(aResponse().withStatus(200).withBody(entrez))); | ||
|
||
PubMedEntrezRecord record = pmidLookup.retrievePubMedRecord(pmid); | ||
assertEquals("10.1021/acs.jproteome.7b00775", record.getDoi()); | ||
assertEquals("10.1000/a.abcd.1234", record.getDoi()); | ||
} | ||
|
||
@Test | ||
public void testGetPubMedRecordWithHighAsciiChars() { | ||
PmidLookup pmidLookup = new PmidLookup(); | ||
String pmid = "27648456"; | ||
public void testGetPubMedRecordWithHighAsciiChars() throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A suggestion to reduce code is to update There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other tests use this pmidrecord.json such as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. This now includes the refactored NihmsSubmissionEtlITBase that has the member variables, |
||
String entrez = IOUtils.toString(getClass().getClassLoader(). | ||
getResourceAsStream("pmid_record_ascii.json"), StandardCharsets.UTF_8); | ||
|
||
String pmid = "11111111"; | ||
|
||
stubFor(get(urlPathEqualTo("/entrez/eutils/esummary.fcgi")) | ||
.withQueryParam("db", WireMock.equalTo("pubmed")) | ||
.withQueryParam("retmode", WireMock.equalTo("json")) | ||
.withQueryParam("rettype", WireMock.equalTo("abstract")) | ||
.withQueryParam("id", WireMock.equalTo(pmid)) | ||
.willReturn(aResponse().withStatus(200).withBody(entrez))); | ||
|
||
PubMedEntrezRecord record = pmidLookup.retrievePubMedRecord(pmid); | ||
assertEquals("10.1002/acn3.333", record.getDoi()); | ||
assertEquals("Age-dependent effects of APOE ε4 in preclinical Alzheimer's disease.", record.getTitle()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to remain here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this one I was getting compilation errors. I think there was a recent mvn change or something. I will put it back in to get the error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. This has been reverted, but will be looked into in the future, per our discussion.