diff --git a/pass-core-doi-service/pom.xml b/pass-core-doi-service/pom.xml index 49071e25..e27ace80 100644 --- a/pass-core-doi-service/pom.xml +++ b/pass-core-doi-service/pom.xml @@ -49,6 +49,11 @@ commons-io test + + com.squareup.okhttp3 + mockwebserver + test + diff --git a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ElideConnector.java b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ElideConnector.java index 9958276f..c715ef0e 100644 --- a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ElideConnector.java +++ b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ElideConnector.java @@ -37,6 +37,7 @@ import org.eclipse.pass.object.model.Journal; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; /** * This class manages Journal objects related to a journal lookup on Crossref - creating or updating @@ -44,6 +45,7 @@ * * @author jrm */ +@Component public class ElideConnector { private static final Logger LOG = LoggerFactory.getLogger(ElideConnector.class); diff --git a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiService.java b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiService.java index f453beea..3fcd1027 100644 --- a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiService.java +++ b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiService.java @@ -16,12 +16,7 @@ */ package org.eclipse.pass.doi.service; -import static java.lang.Thread.sleep; - -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Set; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -35,9 +30,6 @@ * @author jrm */ public abstract class ExternalDoiService { - private final Set activeJobSet = new HashSet<>(); - private final Set syncActiveJobSet = Collections.synchronizedSet(activeJobSet); - final static String MAILTO = "pass@jhu.edu"; /** @@ -56,13 +48,13 @@ public abstract class ExternalDoiService { * A key, value map of query parameters used by the external service; null if there aren't any. * @return the map */ - public abstract HashMap parameterMap(); + public abstract Map parameterMap(); /** * A key, value map of headers used by the external service; null if there aren't any. * @return the map */ - public abstract HashMap headerMap(); + public abstract Map headerMap(); /** * A method to transform the raw external service's JSON response to suit the UI requirements @@ -89,62 +81,4 @@ String verify(String doi) { Matcher matcher = pattern.matcher(suffix); return matcher.matches() ? suffix : null; } - - /** - * this simply protects the external service from a person hammering on a request thinking - * it wasn't processed, when it really is just slow coming back - * - * @param doi the doi to check active - * @return whether the doi lookup is still active - */ - boolean isAlreadyActive(String doi) { - //check cache map for existence of doi - //put doi on map if absent - - if (syncActiveJobSet.contains(doi)) { - return true; - } else { - // this DOI is not actively being processed - // let's temporarily prohibit new requests for this DOI - syncActiveJobSet.add(doi); - //longest time we expect it should take to create a Journal object, in - //milliseconds - int cachePeriod = 30000; - Thread t = new Thread(new ExternalDoiService.ExpiringLock(doi, cachePeriod)); - t.start(); - } - return false; - } - - /** - * Once a doi has been processed, this method removes it from the locked list. - * @param doi the doi - */ - void unlockDoi(String doi) { - syncActiveJobSet.remove(doi); - } - - /** - * A class to manage locking so that an active process for a DOI will finish executing before - * another one begins - */ - public class ExpiringLock implements Runnable { - private final String key; - private final int duration; - - ExpiringLock(String key, int duration) { - this.key = key; - this.duration = duration; - } - - @Override - public void run() { - try { - sleep(duration); - syncActiveJobSet.remove(key); - } catch (InterruptedException e) { - syncActiveJobSet.remove(key); - } - } - } } diff --git a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnector.java b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnector.java index 5fad6647..3779cf24 100644 --- a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnector.java +++ b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnector.java @@ -19,7 +19,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import java.io.IOException; -import java.io.StringReader; +import java.io.Reader; import java.util.Objects; import jakarta.json.Json; @@ -34,14 +34,17 @@ import okhttp3.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; /** * A class which manages the retrieval of JSON from external DOI services (Unpaywall, Crossref) * * @author jrm */ +@Component public class ExternalDoiServiceConnector { private static final Logger LOG = LoggerFactory.getLogger(ExternalDoiServiceConnector.class); + static final String HTTP_STATUS_CODE = "HTTP_STATUS_CODE"; private final OkHttpClient client; @@ -54,54 +57,50 @@ public class ExternalDoiServiceConnector { } /** - * consult external service to get a json object for a supplied doi + * Consult external service to get a json object for a supplied doi. * * @param doi - the supplied doi string, prefix trimmed if necessary - * @return a string representing the works object if successful; an empty string if not found; null if IO exception + * @return a JSON object if successful, + * null if an error occurs interacting with the external service, + * {error: "error message", HTTP_STATUS_CODE: http status code} if + * the external service returns an error status code */ JsonObject retrieveMetadata(String doi, ExternalDoiService service) { HttpUrl.Builder urlBuilder = Objects.requireNonNull(HttpUrl.parse(service.baseUrl() + doi)).newBuilder(); - if ( service.parameterMap() != null ) { - for ( String key : service.parameterMap().keySet() ) { + if (service.parameterMap() != null) { + for (String key : service.parameterMap().keySet()) { urlBuilder.addQueryParameter(key, service.parameterMap().get(key)); } } - String url = urlBuilder.build().toString(); + Request.Builder requestBuilder = new Request.Builder().url(urlBuilder.build()); - Request.Builder requestBuilder = new Request.Builder() - .url(url); - if ( service.headerMap() != null ) { + if (service.headerMap() != null) { requestBuilder.headers(Headers.of(service.headerMap())); } - Request okHttpRequest = requestBuilder.build(); + Request okHttpRequest = requestBuilder.build(); Call call = client.newCall(okHttpRequest); - JsonReader reader; - JsonObject metadataJsonObject; - String responseString = null; try (Response okHttpResponse = call.execute()) { - responseString = Objects.requireNonNull(okHttpResponse.body()).string(); - reader = Json.createReader(new StringReader(responseString)); - metadataJsonObject = reader.readObject(); - reader.close(); - - service.unlockDoi(doi); - - return metadataJsonObject; - } catch (JsonParsingException e) { - if (responseString != null) { - return Json.createObjectBuilder() - .add("error", responseString) - .build(); + if (okHttpResponse.isSuccessful()) { + try (Reader reader = okHttpResponse.body().charStream(); + JsonReader jsonReader = Json.createReader(reader)) { + return jsonReader.readObject(); + } catch (JsonParsingException e) { + LOG.error("Error parsing JSON of external service: " + okHttpRequest.url(), e); + return null; + } } + + // Set response as the error field and save the status code. + return Json.createObjectBuilder().add("error", okHttpResponse.body().string()). + add(HTTP_STATUS_CODE, Json.createValue(okHttpResponse.code())).build(); } catch (IOException e) { - LOG.error(e.getMessage(), e); + LOG.error("Error accessing external service: " + okHttpRequest.url(), e); + return null; } - return null; } - } diff --git a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/PassDoiServiceController.java b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/PassDoiServiceController.java index 98cf13fc..ae2e496a 100644 --- a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/PassDoiServiceController.java +++ b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/PassDoiServiceController.java @@ -17,15 +17,15 @@ package org.eclipse.pass.doi.service; import java.io.IOException; -import java.io.OutputStream; +import java.io.Writer; -import com.yahoo.elide.RefreshableElide; import jakarta.json.Json; import jakarta.json.JsonObject; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; @@ -43,14 +43,14 @@ public class PassDoiServiceController { private final ExternalDoiService xrefDoiService; private final ExternalDoiService unpaywallDoiService; - /** - * @param refreshableElide the RefreshableElide - */ - public PassDoiServiceController(RefreshableElide refreshableElide) { - this.elideConnector = new ElideConnector(refreshableElide); - this.externalDoiServiceConnector = new ExternalDoiServiceConnector(); - this.xrefDoiService = new XrefDoiService(); - this.unpaywallDoiService = new UnpaywallDoiService(); + public PassDoiServiceController(ElideConnector elideConnector, + ExternalDoiServiceConnector externalDoiServiceConnector, + @Qualifier("xrefDoiService") ExternalDoiService xrefDoiService, + @Qualifier("unpaywallDoiService") ExternalDoiService unpaywallDoiService) { + this.elideConnector = elideConnector; + this.externalDoiServiceConnector = externalDoiServiceConnector; + this.xrefDoiService = xrefDoiService; + this.unpaywallDoiService = unpaywallDoiService; } /** @@ -71,64 +71,50 @@ protected void getXrefMetadata(HttpServletRequest request, HttpServletResponse r //we will call out to crossref and collect the work JSON object //the value of this parameter is expected to be already URIencoded - String doi = request.getParameter("doi"); + String doi = externalService.verify(request.getParameter("doi")); //stage 1: verify doi is valid - if (externalService.verify(doi) == null) { + if (doi == null) { // do not have have a valid xref doi - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { JsonObject jsonObject = Json.createObjectBuilder() .add("error", "Supplied DOI is not in valid DOI format.") .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(400); return; } } - //Stage 2: make sure we don't already have a request being processed for this doi - if (externalService.isAlreadyActive(doi)) { - // return already processing error (429>) - try (OutputStream out = response.getOutputStream()) { - String message = "There is already an active request for " + doi; - JsonObject jsonObject = Json.createObjectBuilder() - .add("error", message + "; try again later.") - .build(); - out.write(jsonObject.toString().getBytes()); - response.setStatus(429); - LOG.warn(message); - } - } - - //stage 3: try to get crossref record, catch errors first, and halt processing + //stage 2: try to get crossref record, catch errors first, and halt processing JsonObject xrefJsonObject = externalDoiServiceConnector.retrieveMetadata(doi, externalService); + if (xrefJsonObject == null) { - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { String message = "There was an error getting the metadata from " + externalService.name() + " for " + doi; JsonObject jsonObject = Json.createObjectBuilder() .add("error", message) .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(500); - LOG.warn(message); } - } else if (xrefJsonObject.getJsonString("error") != null) { - int responseCode; + } else if (xrefJsonObject.containsKey("error")) { + int responseCode = xrefJsonObject.getInt(ExternalDoiServiceConnector.HTTP_STATUS_CODE); String message; - if (xrefJsonObject.getString("error").equals("Resource not found.")) { - responseCode = 404; + + if (responseCode == 404) { message = "The resource for DOI " + doi + " could not be found on " + externalService.name() + "."; } else { - responseCode = 500; message = "A record for this resource could not be returned from " + externalService.name() + ": " + - xrefJsonObject.getJsonString("error"); + xrefJsonObject.getJsonString("error"); } - try (OutputStream out = response.getOutputStream()) { + + try (Writer out = response.getWriter()) { JsonObject jsonObject = Json.createObjectBuilder() .add("error", message) .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(responseCode); LOG.warn(message); } @@ -137,25 +123,25 @@ protected void getXrefMetadata(HttpServletRequest request, HttpServletResponse r String journalId = elideConnector.resolveJournal(xrefJsonObject); if (journalId != null) { - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { JsonObject jsonObject = Json.createObjectBuilder() .add("journal-id", journalId) .add("crossref", externalService.processObject(xrefJsonObject)) .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(200); } } else { // journal id is null - this should never happen unless Crosssref journal is insufficient // for example, if a book doi ws supplied which has no issns - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { String message = "Insufficient information to locate or specify a journal entry."; JsonObject jsonObject = Json.createObjectBuilder() .add("error", message) .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(422); LOG.warn(message); } @@ -182,79 +168,51 @@ protected void getUnpaywallMetadata(HttpServletRequest request, HttpServletRespo //we will call out to unpaywall and collect the JSON object //the value of this parameter is expected to be already URIencoded - String doi = request.getParameter("doi"); + String doi = externalService.verify(request.getParameter("doi")); //stage 1: verify doi is valid - if (externalService.verify(doi) == null) { + if (doi == null) { // do not have have a valid doi - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { JsonObject jsonObject = Json.createObjectBuilder() .add("error", "Supplied DOI is not in valid DOI format.") .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(400); return; } } - //Stage 2: make sure we don't already have a request being processed for this doi - if (externalService.isAlreadyActive(doi)) { - // return already processing error (429) - try (OutputStream out = response.getOutputStream()) { - String message = "There is already an active request for " + doi; - JsonObject jsonObject = Json.createObjectBuilder() - .add("error", message + "; try again later.") - .build(); - out.write(jsonObject.toString().getBytes()); - response.setStatus(429); - LOG.warn(message); - } - } - - //stage 3: try to get unpaywall record, catch errors first, and halt processing + //stage 2: try to get unpaywall record, catch errors first, and halt processing JsonObject unpaywallJsonObject = externalDoiServiceConnector.retrieveMetadata(doi, externalService); if (unpaywallJsonObject == null) { - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { String message = "There was an error getting the metadata from " + externalService.name() + " for " + doi; JsonObject jsonObject = Json.createObjectBuilder() .add("error", message) .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(500); - LOG.warn(message); - } - } else if ( - unpaywallJsonObject.containsKey("error") && - unpaywallJsonObject.getValue("/error").toString().equals("true") - ) { - int responseCode; - String message; - - if (unpaywallJsonObject.getValue("/HTTP_status_code") != null && - unpaywallJsonObject.getValue("/message") != null) { - - responseCode = Integer.parseInt(unpaywallJsonObject.getValue("/HTTP_status_code").toString()); - message = unpaywallJsonObject.getValue("/message").toString(); - } else { - responseCode = 500; - message = "A record for this resource could not be returned from Unpaywall: " + - unpaywallJsonObject.getJsonString("error"); } + } else if (unpaywallJsonObject.containsKey("error")) { + int responseCode = unpaywallJsonObject.getInt(ExternalDoiServiceConnector.HTTP_STATUS_CODE); + String message = "A record for this resource could not be returned from Unpaywall: " + + unpaywallJsonObject.getJsonString("error"); - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { JsonObject jsonObject = Json.createObjectBuilder() .add("error", message) .build(); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(responseCode); LOG.warn(message); } } else { // have a non-empty JSON string to process - try (OutputStream out = response.getOutputStream()) { + try (Writer out = response.getWriter()) { JsonObject jsonObject = externalService.processObject(unpaywallJsonObject); - out.write(jsonObject.toString().getBytes()); + out.write(jsonObject.toString()); response.setStatus(200); } } diff --git a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/UnpaywallDoiService.java b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/UnpaywallDoiService.java index 92759223..dda7a66e 100644 --- a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/UnpaywallDoiService.java +++ b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/UnpaywallDoiService.java @@ -20,18 +20,21 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.HashMap; +import java.util.Map; import jakarta.json.Json; import jakarta.json.JsonArray; import jakarta.json.JsonArrayBuilder; import jakarta.json.JsonObject; import jakarta.json.JsonValue; +import org.springframework.stereotype.Service; /** * The UnpaywallDoiService class is an implementation of the ExternalDoiService abstract class to interface with * the Unpaywall API. The Unpaywall API is a RESTful API that returns JSON metadata for a given DOI. More information * about the Unpaywall API can be found here: Unpaywall API */ +@Service public class UnpaywallDoiService extends ExternalDoiService { String UNPAYWALL_BASEURI = "https://api.unpaywall.org/v2/"; @@ -48,7 +51,7 @@ public String baseUrl() { } @Override - public HashMap parameterMap() { + public Map parameterMap() { HashMap parameterMap = new HashMap<>(); String agent = System.getenv("PASS_DOI_SERVICE_MAILTO") != null ? System.getenv( "PASS_DOI_SERVICE_MAILTO") : MAILTO; @@ -57,7 +60,7 @@ public HashMap parameterMap() { } @Override - public HashMap headerMap() { + public Map headerMap() { return null; } diff --git a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/XrefDoiService.java b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/XrefDoiService.java index 752a874d..101bfa59 100644 --- a/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/XrefDoiService.java +++ b/pass-core-doi-service/src/main/java/org/eclipse/pass/doi/service/XrefDoiService.java @@ -33,6 +33,7 @@ import jakarta.json.JsonObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; /** * The XrefDoiService class is an implementation of the ExternalDoiService abstract class to interface with the Crossref @@ -44,6 +45,7 @@ * address. The default email address used by is pass@jhu.edu and can be overridden by setting the environment variable * PASS_DOI_SERVICE_MAILTO */ +@Service public class XrefDoiService extends ExternalDoiService { private final static String XREF_BASEURI = "https://api.crossref.org/v1/works/"; private final static Logger LOG = LoggerFactory.getLogger(XrefDoiService.class); diff --git a/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnectorIntegrationTest.java b/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnectorIntegrationTest.java new file mode 100644 index 00000000..cea0e971 --- /dev/null +++ b/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnectorIntegrationTest.java @@ -0,0 +1,96 @@ +/* + * + * Copyright 2019 Johns Hopkins University + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.eclipse.pass.doi.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import jakarta.json.JsonObject; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +/** + * Integration test for the connector. This test class is @Disableed in production, and is only used to + * verify behavior against live services when either the connector class or the external APIs + * are changed + * + * @author jrm + */ +@Disabled +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class ExternalDoiServiceConnectorIntegrationTest { + + private final ExternalDoiServiceConnector underTest = new ExternalDoiServiceConnector(); + private final ExternalDoiService xrefService = new XrefDoiService(); + private final ExternalDoiService unpaywallService = new UnpaywallDoiService(); + + /** + * test that hitting the Crossref API with a doi returns the expected JSON object + */ + @Test + void testXrefLookup() { + String realDoi = "10.4137/cmc.s38446"; + JsonObject blob = underTest.retrieveMetadata(realDoi, xrefService); + //these results will differ by a timestamp - but a good check is that they return the same journal objects + JsonObject object = JsonTestObjectsUtil.xrefTestJsonObject(); + + assertNotNull(blob.getJsonObject("message").getJsonArray("ISSN")); + assertEquals(blob.getJsonObject("message").getJsonArray("ISSN"), + object.getJsonObject("message").getJsonArray("ISSN")); + } + + /** + * test that hitting the Unpaywall API with a doi returns the expected JSON object + */ + @Test + void testUnpaywallLookup() { + String realDoi = "10.4137/cmc.s38446"; + JsonObject blob = underTest.retrieveMetadata(realDoi, unpaywallService); + + JsonObject object = JsonTestObjectsUtil.unpaywallTestJsonObject(); + + assertNotNull(blob.getJsonString("doi")); + assertEquals(blob.getJsonString("doi"), object.getJsonString("doi")); + + } + + /** + * Test that sending bad doi to Xref results in an error. + */ + @Test + void testBadXrefDoiLookup() { + String badDoi = "10.1212/abc.DEF"; + JsonObject blob = underTest.retrieveMetadata(badDoi, xrefService); + assertTrue(blob.containsKey("error")); + assertEquals(404, blob.getInt(ExternalDoiServiceConnector.HTTP_STATUS_CODE)); + } + + /** + * Test that sending a bad doi to Unpaywall results in an error. + */ + @Test + void testBadUnpaywallDoiLookup() { + String badDoi = "10.1212/abc.DEF"; + JsonObject blob = underTest.retrieveMetadata(badDoi, unpaywallService); + assertTrue(blob.containsKey("error")); + assertEquals(404, blob.getInt(ExternalDoiServiceConnector.HTTP_STATUS_CODE)); + } + +} + diff --git a/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnectorTest.java b/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnectorTest.java index 8334be1a..68423e4e 100644 --- a/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnectorTest.java +++ b/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/ExternalDoiServiceConnectorTest.java @@ -1,6 +1,6 @@ /* * - * Copyright 2019 Johns Hopkins University + * Copyright 2025 Johns Hopkins University * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,77 +18,104 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.io.IOException; +import java.util.Map; import jakarta.json.JsonObject; -import org.junit.jupiter.api.Disabled; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.SocketPolicy; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestInstance; /** - * Unit tests for the connector. This test class is @Disableed in production, and is only used to - * verify behavior against live services when either the connector class or the external APIs - * are changed - * - * @author jrm + * Unit tests that check the behavior of retrieveMetadata by mocking the external service. */ -@Disabled -@TestInstance(TestInstance.Lifecycle.PER_CLASS) public class ExternalDoiServiceConnectorTest { + private ExternalDoiServiceConnector underTest = new ExternalDoiServiceConnector(); - private final ExternalDoiServiceConnector underTest = new ExternalDoiServiceConnector(); - private final ExternalDoiService xrefService = new XrefDoiService(); - private final ExternalDoiService unpaywallService = new UnpaywallDoiService(); + private ExternalDoiService mockService(String baseUrl) { + return new ExternalDoiService() { + @Override + public JsonObject processObject(JsonObject object) { + return object; + } - /** - * test that hitting the Crossref API with a doi returns the expected JSON object - */ - @Test - public void testXrefLookup() { - String realDoi = "10.4137/cmc.s38446"; - JsonObject blob = underTest.retrieveMetadata(realDoi, xrefService); - //these results will differ by a timestamp - but a good check is that they return the same journal objects - JsonObject object = JsonTestObjectsUtil.xrefTestJsonObject(); - - assertNotNull(blob.getJsonObject("message").getJsonArray("ISSN")); - assertEquals(blob.getJsonObject("message").getJsonArray("ISSN"), - object.getJsonObject("message").getJsonArray("ISSN")); + @Override + public Map parameterMap() { + return Map.of(); + } + + @Override + public String name() { + return "test"; + } + + @Override + public Map headerMap() { + return Map.of(); + } + + @Override + public String baseUrl() { + return baseUrl; + } + }; } - /** - * test that hitting the Unpaywall API with a doi returns the expected JSON object - */ @Test - public void testUnpaywallLookup() { - String realDoi = "10.4137/cmc.s38446"; //"10.1038/nature12373"; - JsonObject blob = underTest.retrieveMetadata(realDoi, unpaywallService); + void testRetrieveMetadataNotFound() throws IOException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(new MockResponse().setResponseCode(404)); - JsonObject object = JsonTestObjectsUtil.unpaywallTestJsonObject(); + ExternalDoiService service = mockService(server.url("/").toString()); - assertNotNull(blob.getJsonString("doi")); - assertEquals(blob.getJsonString("doi"), object.getJsonString("doi")); + JsonObject result = underTest.retrieveMetadata("10.123/abc", service); + assertNotNull(result); + assertEquals(404, result.getInt(ExternalDoiServiceConnector.HTTP_STATUS_CODE)); + } } - /** - * test that a bad doi gives the required error message - */ @Test - public void testBadXrefDoiLookup() { - String badDoi = "10.1212/abc.DEF"; - JsonObject blob = underTest.retrieveMetadata(badDoi, xrefService); - assertEquals("Resource not found.", blob.getString("error")); + void testRetrieveMetadataBadIO() throws IOException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START)); + + ExternalDoiService service = mockService(server.url("/").toString()); + + JsonObject result = underTest.retrieveMetadata("10.123/abc", service); + + assertNull(result); + } } - /** - * test that a bad doi gives the required error message - */ @Test - public void testBadUnpaywallDoiLookup() { - String badDoi = "10.1212/abc.DEF"; - JsonObject blob = underTest.retrieveMetadata(badDoi, unpaywallService); - assertEquals("true",blob.getValue("/error").toString()); - assertEquals("404", blob.getValue("/HTTP_status_code").toString()); + void testRetrieveMetadataBadJson() throws IOException { + try (MockWebServer server = new MockWebServer()) { + server.enqueue(new MockResponse().setBody("This is not JSON")); + + ExternalDoiService service = mockService(server.url("/").toString()); + + JsonObject result = underTest.retrieveMetadata("10.123/abc", service); + + assertNull(result); + } } -} + @Test + void testRetrieveMetadataSuccess() throws IOException { + try (MockWebServer server = new MockWebServer()) { + String json = "{\"foo\":\"bar\"}"; + server.enqueue(new MockResponse().setBody(json)); + ExternalDoiService service = mockService(server.url("/").toString()); + + JsonObject result = underTest.retrieveMetadata("10.4137/cmc.s38446", service); + + assertNotNull(result); + assertEquals("bar", result.getString("foo")); + } + } +} diff --git a/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/PassDoiServiceControllerTest.java b/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/PassDoiServiceControllerTest.java new file mode 100644 index 00000000..4bd587f2 --- /dev/null +++ b/pass-core-doi-service/src/test/java/org/eclipse/pass/doi/service/PassDoiServiceControllerTest.java @@ -0,0 +1,285 @@ +/* + * + * Copyright 2025 Johns Hopkins University + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.eclipse.pass.doi.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringReader; +import java.io.StringWriter; + +import jakarta.json.Json; +import jakarta.json.JsonObject; +import jakarta.json.JsonReader; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * These unit tests check how the controller handles success and error conditions from the external + * Crossref and Unpaywall services by mocking those services. + * + * There is an integration test in pass-core-main called DoiServiceTest that actually hits the live + * external services. + */ +class PassDoiServiceControllerTest { + private PassDoiServiceController controller; + private ElideConnector elideConnector; + private ExternalDoiServiceConnector externalDoiServiceConnector; + private ExternalDoiService xrefDoiService; + private ExternalDoiService unpaywallDoiService; + + @BeforeEach + public void setUp() { + elideConnector = mock(ElideConnector.class); + externalDoiServiceConnector = mock(ExternalDoiServiceConnector.class); + xrefDoiService = mock(ExternalDoiService.class); + unpaywallDoiService = mock(ExternalDoiService.class); + + controller = new PassDoiServiceController( + elideConnector, + externalDoiServiceConnector, + xrefDoiService, + unpaywallDoiService + ); + } + + @Test + void testGetXrefMetadata_Success() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "10.1234/5678"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(xrefDoiService.verify(doi)).thenReturn(doi); + + JsonObject xrefJson = Json.createObjectBuilder().add("a", "b").build(); + JsonObject processedJson = Json.createObjectBuilder().add("processed", "true").build(); + + when(externalDoiServiceConnector.retrieveMetadata(doi, xrefDoiService)).thenReturn(xrefJson); + when(elideConnector.resolveJournal(xrefJson)).thenReturn("journal-1"); + when(xrefDoiService.processObject(xrefJson)).thenReturn(processedJson); + + controller.getXrefMetadata(request, response); + + verify(response).setStatus(200); + verify(response).setContentType("application/json"); + + JsonObject result = parseJson(stringWriter.toString()); + assertEquals("journal-1", result.getString("journal-id")); + assertEquals(processedJson, result.getJsonObject("crossref")); + } + + @Test + void testGetXrefMetadata_InvalidDoi() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "invalid-doi"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(xrefDoiService.verify(doi)).thenReturn(null); + + controller.getXrefMetadata(request, response); + + verify(response).setStatus(400); + JsonObject result = parseJson(stringWriter.toString()); + assertEquals("Supplied DOI is not in valid DOI format.", result.getString("error")); + } + + @Test + void testGetXrefMetadata_ServiceError_NullCheck() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "10.1234/5678"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(xrefDoiService.verify(doi)).thenReturn(doi); + when(xrefDoiService.name()).thenReturn("Crossref"); + + when(externalDoiServiceConnector.retrieveMetadata(doi, xrefDoiService)).thenReturn(null); + + controller.getXrefMetadata(request, response); + + verify(response).setStatus(500); + JsonObject result = parseJson(stringWriter.toString()); + assertEquals("There was an error getting the metadata from Crossref for " + doi, result.getString("error")); + } + + @Test + void testGetXrefMetadata_ServiceError_404() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "10.1234/5678"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(xrefDoiService.verify(doi)).thenReturn(doi); + when(xrefDoiService.name()).thenReturn("Crossref"); + + JsonObject errorJson = Json.createObjectBuilder() + .add("error", "Not Found") + .add(ExternalDoiServiceConnector.HTTP_STATUS_CODE, 404) + .build(); + + when(externalDoiServiceConnector.retrieveMetadata(doi, xrefDoiService)).thenReturn(errorJson); + + controller.getXrefMetadata(request, response); + + verify(response).setStatus(404); + JsonObject result = parseJson(stringWriter.toString()); + assertEquals("The resource for DOI " + doi + " could not be found on Crossref.", result.getString("error")); + } + + @Test + void testGetXrefMetadata_NoJournalId() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "10.1234/5678"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(xrefDoiService.verify(doi)).thenReturn(doi); + + JsonObject xrefJson = Json.createObjectBuilder().add("a", "b").build(); + + when(externalDoiServiceConnector.retrieveMetadata(doi, xrefDoiService)).thenReturn(xrefJson); + when(elideConnector.resolveJournal(xrefJson)).thenReturn(null); + + controller.getXrefMetadata(request, response); + + verify(response).setStatus(422); + JsonObject result = parseJson(stringWriter.toString()); + assertEquals("Insufficient information to locate or specify a journal entry.", result.getString("error")); + } + + @Test + void testGetUnpaywallMetadata_Success() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "10.1234/5678"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(unpaywallDoiService.verify(doi)).thenReturn(doi); + + JsonObject unpaywallJson = Json.createObjectBuilder().add("a", "b").build(); + JsonObject processedJson = Json.createObjectBuilder().add("processed", "true").build(); + + when(externalDoiServiceConnector.retrieveMetadata(doi, unpaywallDoiService)).thenReturn(unpaywallJson); + when(unpaywallDoiService.processObject(unpaywallJson)).thenReturn(processedJson); + + controller.getUnpaywallMetadata(request, response); + + verify(response).setStatus(200); + JsonObject result = parseJson(stringWriter.toString()); + assertEquals(processedJson, result); + } + + @Test + void testGetUnpaywallMetadata_InvalidDoi() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "invalid-doi"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(unpaywallDoiService.verify(doi)).thenReturn(null); + + controller.getUnpaywallMetadata(request, response); + + verify(response).setStatus(400); + JsonObject result = parseJson(stringWriter.toString()); + assertEquals("Supplied DOI is not in valid DOI format.", result.getString("error")); + } + + @Test + void testGetUnpaywallMetadata_ServiceError_Null() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "10.1234/5678"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(unpaywallDoiService.verify(doi)).thenReturn(doi); + when(unpaywallDoiService.name()).thenReturn("Unpaywall"); + + when(externalDoiServiceConnector.retrieveMetadata(doi, unpaywallDoiService)).thenReturn(null); + + controller.getUnpaywallMetadata(request, response); + + verify(response).setStatus(500); + JsonObject result = parseJson(stringWriter.toString()); + assertEquals("There was an error getting the metadata from Unpaywall for " + doi, result.getString("error")); + } + + @Test + void testGetUnpaywallMetadata_ServiceError_WithCode() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + StringWriter stringWriter = new StringWriter(); + PrintWriter writer = new PrintWriter(stringWriter); + + String doi = "10.1234/5678"; + when(request.getParameter("doi")).thenReturn(doi); + when(response.getWriter()).thenReturn(writer); + when(unpaywallDoiService.verify(doi)).thenReturn(doi); + + JsonObject errorJson = Json.createObjectBuilder() + .add("error", "Some Error") + .add(ExternalDoiServiceConnector.HTTP_STATUS_CODE, 503) + .build(); + + when(externalDoiServiceConnector.retrieveMetadata(doi, unpaywallDoiService)).thenReturn(errorJson); + + controller.getUnpaywallMetadata(request, response); + + verify(response).setStatus(503); + JsonObject result = parseJson(stringWriter.toString()); + assertTrue(result.getString("error").contains("Some Error")); + } + + private JsonObject parseJson(String json) { + try (JsonReader reader = Json.createReader(new StringReader(json))) { + return reader.readObject(); + } + } +} \ No newline at end of file diff --git a/pass-core-main/src/test/java/org/eclipse/pass/doi/service/DoiServiceTest.java b/pass-core-main/src/test/java/org/eclipse/pass/doi/service/DoiServiceTest.java index 12f684e8..13009706 100644 --- a/pass-core-main/src/test/java/org/eclipse/pass/doi/service/DoiServiceTest.java +++ b/pass-core-main/src/test/java/org/eclipse/pass/doi/service/DoiServiceTest.java @@ -46,6 +46,12 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +/** + * This integration test of the DOI API hits the live CrossRef and Unpaywall services. + * + * There is a unit test in pass-core-doi-service called PassDoiServiceControllerTest which checks + * various failure conditions around integrating with the external services which are not tested here. + */ class DoiServiceTest extends SimpleIntegrationTest { private static final String CREDENTIALS = Credentials.basic(BACKEND_USER, BACKEND_PASSWORD);