diff --git a/NEWS.md b/NEWS.md index b1f3b1c45..853553855 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,14 @@ ## 3.14.0 - Unreleased +## 3.13.1 (Released) + +This release includes bug fixes for incorrect characters + +[Full Changelog](https://github.com/folio-org/mod-oai-pmh/compare/v3.13.0...v3.13.1) + +### Bug fixes +* [MODOAIPMH-540](https://issues.folio.org/browse/MODOAIPMH-540) Error while converting record to xml + ## 3.13.0 (Released) This release includes dependency updates and minor fixes diff --git a/pom.xml b/pom.xml index eb00bb715..5537d7b9b 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.folio mod-oai-pmh - 3.13.1-SNAPSHOT + 3.13.2-SNAPSHOT OAI-PMH Repository Business Logic Business logic to support the Open Archives Initiative Protocol for Metadata Harvesting diff --git a/ramls/schemas/OAI-PMH.xsd b/ramls/schemas/OAI-PMH.xsd index cb1fd4830..953d29fe1 100644 --- a/ramls/schemas/OAI-PMH.xsd +++ b/ramls/schemas/OAI-PMH.xsd @@ -89,6 +89,7 @@ + diff --git a/src/main/java/org/folio/oaipmh/Constants.java b/src/main/java/org/folio/oaipmh/Constants.java index 8a1dfabc5..21238c70c 100644 --- a/src/main/java/org/folio/oaipmh/Constants.java +++ b/src/main/java/org/folio/oaipmh/Constants.java @@ -103,6 +103,7 @@ private Constants() { public static final String LIST_ILLEGAL_ARGUMENTS_ERROR = "Verb '%s', argument 'resumptionToken' is exclusive, no others maybe specified with it."; public static final String INVALID_RESUMPTION_TOKEN = "Verb '%s', argument resumptionToken is invalid"; public static final String NO_RECORD_FOUND_ERROR = "There were no records found matching the search criteria"; + public static final String INVALID_CHARACTER_IN_THE_RECORD = "Invalid character in the record."; public static final String BAD_DATESTAMP_FORMAT_ERROR = "Bad datestamp format for '%s=%s' argument."; public static final String RECORD_METADATA_PREFIX_PARAM_ERROR = "The request is missing required arguments. There is no metadataPrefix."; public static final String RECORD_NOT_FOUND_ERROR = "No matching identifier in repository."; diff --git a/src/main/java/org/folio/oaipmh/helpers/AbstractGetRecordsHelper.java b/src/main/java/org/folio/oaipmh/helpers/AbstractGetRecordsHelper.java index de9dee1f7..eb37d82e5 100644 --- a/src/main/java/org/folio/oaipmh/helpers/AbstractGetRecordsHelper.java +++ b/src/main/java/org/folio/oaipmh/helpers/AbstractGetRecordsHelper.java @@ -472,8 +472,10 @@ protected Future processRecords(Context ctx, Request request, Promise oaiResponsePromise = Promise.promise(); buildRecords(ctx, request, items).onSuccess(recordsMap -> { Response response; - if (recordsMap.isEmpty()) { + if (noRecordsFoundResultCheck(recordsMap, items, request.getVerb())) { response = buildNoRecordsFoundOaiResponse(oaipmh, request); + } else if (conversionIntoJaxbObjectIssueCheck(recordsMap, items, request.getVerb())) { + response = conversionIntoJaxbObjectIssueResponse(oaipmh, request); } else { addRecordsToOaiResponse(oaipmh, recordsMap.values()); addResumptionTokenToOaiResponse(oaipmh, resumptionToken); @@ -484,6 +486,22 @@ protected Future processRecords(Context ctx, Request request, return oaiResponsePromise.future(); } + boolean noRecordsFoundResultCheck(Map recordsMap, JsonArray items, VerbType verb){ + return recordsMap.isEmpty() && + (jsonArrayIsEmpty(items) || (jsonArrayNotEmpty(items) && VerbType.GET_RECORD != verb)); + } + + boolean conversionIntoJaxbObjectIssueCheck(Map recordsMap, JsonArray items, VerbType verb){ + return recordsMap.isEmpty() && jsonArrayNotEmpty(items) && VerbType.GET_RECORD == verb; + } + + private boolean jsonArrayNotEmpty(JsonArray ja){ + return ja != null && !ja.isEmpty(); + } + private boolean jsonArrayIsEmpty(JsonArray ja){ + return !jsonArrayNotEmpty(ja); + } + /** * Builds {@link Map} with storage id as key and {@link RecordType} with populated header if there is any, * otherwise empty map is returned @@ -496,7 +514,7 @@ private Future> buildRecords(Context context, Request re Map recordsMap = new ConcurrentHashMap<>(); - if (records != null && !records.isEmpty()) { + if (jsonArrayNotEmpty(records)) { RecordMetadataManager metadataManager = RecordMetadataManager.getInstance(); // Using LinkedHashMap just to rely on order returned by storage service records.stream() diff --git a/src/main/java/org/folio/oaipmh/helpers/AbstractHelper.java b/src/main/java/org/folio/oaipmh/helpers/AbstractHelper.java index 831b0103e..03ac45a31 100644 --- a/src/main/java/org/folio/oaipmh/helpers/AbstractHelper.java +++ b/src/main/java/org/folio/oaipmh/helpers/AbstractHelper.java @@ -64,6 +64,7 @@ import static org.folio.oaipmh.Constants.LIST_NO_REQUIRED_PARAM_ERROR; import static org.folio.oaipmh.Constants.NEXT_RECORD_ID_PARAM; import static org.folio.oaipmh.Constants.NO_RECORD_FOUND_ERROR; +import static org.folio.oaipmh.Constants.INVALID_CHARACTER_IN_THE_RECORD; import static org.folio.oaipmh.Constants.OFFSET_PARAM; import static org.folio.oaipmh.Constants.REPOSITORY_MAX_RECORDS_PER_RESPONSE; import static org.folio.oaipmh.Constants.REPOSITORY_RECORDS_SOURCE; @@ -87,6 +88,7 @@ import static org.openarchives.oai._2.OAIPMHerrorcodeType.CANNOT_DISSEMINATE_FORMAT; import static org.openarchives.oai._2.OAIPMHerrorcodeType.ID_DOES_NOT_EXIST; import static org.openarchives.oai._2.OAIPMHerrorcodeType.NO_RECORDS_MATCH; +import static org.openarchives.oai._2.OAIPMHerrorcodeType.INVALID_RECORD_CONTENT; /** * Abstract helper implementation that provides some common methods. @@ -112,6 +114,10 @@ public abstract class AbstractHelper implements VerbHelper { protected ErrorsService errorsService; + public Response conversionIntoJaxbObjectIssueResponse(OAIPMH oaipmh, Request request) { + oaipmh.withErrors(createInvalidJsonContentError()); + return getResponseHelper().buildFailureResponse(oaipmh, request); + } public Response buildNoRecordsFoundOaiResponse(OAIPMH oaipmh, Request request) { oaipmh.withErrors(createNoRecordsFoundError()); return getResponseHelper().buildFailureResponse(oaipmh, request); @@ -134,6 +140,9 @@ public static Response buildNoRecordsFoundOaiResponse(OAIPMH oaipmh, Request req protected static OAIPMHerrorType createNoRecordsFoundError() { return new OAIPMHerrorType().withCode(NO_RECORDS_MATCH).withValue(NO_RECORD_FOUND_ERROR); } + protected static OAIPMHerrorType createInvalidJsonContentError() { + return new OAIPMHerrorType().withCode(INVALID_RECORD_CONTENT).withValue(INVALID_CHARACTER_IN_THE_RECORD); + } public static ResponseHelper getResponseHelper() { return responseHelper; diff --git a/src/test/java/org/folio/rest/impl/OaiPmhImplTest.java b/src/test/java/org/folio/rest/impl/OaiPmhImplTest.java index 7e0dc9a2b..f3973004c 100644 --- a/src/test/java/org/folio/rest/impl/OaiPmhImplTest.java +++ b/src/test/java/org/folio/rest/impl/OaiPmhImplTest.java @@ -42,6 +42,7 @@ import org.folio.rest.tools.utils.ModuleName; import org.folio.rest.tools.utils.NetworkUtils; import org.folio.spring.SpringContextUtil; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -179,6 +180,7 @@ import static org.openarchives.oai._2.OAIPMHerrorcodeType.BAD_RESUMPTION_TOKEN; import static org.openarchives.oai._2.OAIPMHerrorcodeType.CANNOT_DISSEMINATE_FORMAT; import static org.openarchives.oai._2.OAIPMHerrorcodeType.ID_DOES_NOT_EXIST; +import static org.openarchives.oai._2.OAIPMHerrorcodeType.INVALID_RECORD_CONTENT; import static org.openarchives.oai._2.OAIPMHerrorcodeType.NO_RECORDS_MATCH; import static org.openarchives.oai._2.VerbType.GET_RECORD; import static org.openarchives.oai._2.VerbType.IDENTIFY; @@ -755,6 +757,20 @@ void headerDatestampOfGetRecordVerbShouldCorrespondToGranularitySetting(Metadata System.setProperty(REPOSITORY_TIME_GRANULARITY, timeGranularity); } + @Test + void invalidCharacterInTheRecordTest() { + MetadataPrefix prefix = MARC21XML; + String identifier = IDENTIFIER_PREFIX + OkapiMockServer.EXISTING_IDENTIFIER_WITH_INVALID_CHARACTER; + + RequestSpecification request = createBaseRequest() + .with() + .param(VERB_PARAM, GET_RECORD.value()) + .param(IDENTIFIER_PARAM, identifier) + .param(METADATA_PREFIX_PARAM, prefix.getName()); + + OAIPMH oaipmh = verify404InvalidCharacterInTheRecord(request, GET_RECORD); + } + private void verifyHeaderDateStamp(OAIPMH oaipmh, VerbType verbType, String timeGranularity) { String verb = verbType.value(); if (verb.equals(LIST_RECORDS.value())) { @@ -1771,6 +1787,19 @@ private OAIPMH verify200WithXml(RequestSpecification request, VerbType verb) { return oaipmh; } + private OAIPMH verify404InvalidCharacterInTheRecord(RequestSpecification request, VerbType verb) { + String response = verifyWithCodeWithXml(request, 404); + + // Unmarshal string to OAIPMH and verify required data presents + OAIPMH oaipmh = ResponseConverter.getInstance().stringToOaiPmh(response); + + verifyBaseResponse(oaipmh, verb); + + Assertions.assertEquals(INVALID_RECORD_CONTENT, oaipmh.getErrors().get(0).getCode()); + + return oaipmh; + } + private void verify500(RequestSpecification request) { String response = request .when() diff --git a/src/test/java/org/folio/rest/impl/OkapiMockServer.java b/src/test/java/org/folio/rest/impl/OkapiMockServer.java index 40923a359..40485cc49 100644 --- a/src/test/java/org/folio/rest/impl/OkapiMockServer.java +++ b/src/test/java/org/folio/rest/impl/OkapiMockServer.java @@ -60,6 +60,7 @@ public class OkapiMockServer { public static final String TEST_USER_ID = "30fde4be-2d1a-4546-8d6c-b468caca2720"; static final String EXISTING_IDENTIFIER = "existing-identifier"; + static final String EXISTING_IDENTIFIER_WITH_INVALID_CHARACTER = "id-of-existing-instance-with-invalid-character"; static final String INSTANCE_ID_GET_RECORD_MARC21_FROM_INVENTORY_INVALID_DATA = "existing-invalid-data-identifier"; static final String RECORD_IDENTIFIER_MARC21_WITH_HOLDINGS = "00000000-0000-4a89-a2f9-78ce3145e4fc"; static final String RECORD_IDENTIFIER_INSTANCE_NOT_FOUND = "fb3e23e5-eb7f-4b8b-b531-40e74ec9c6e9"; @@ -135,6 +136,7 @@ public class OkapiMockServer { private static final String SRS_RECORD_WITH_NON_EXISTING_INSTANCE_JSON = "/srs_record_with_non_existing_instance.json"; private static final String INSTANCES_0 = "/instances_0.json"; private static final String INSTANCES_1 = "/instances_1.json"; + private static final String INSTANCES_1_WITH_INVALID_CHARACTER = "/instances_1_with_invalid_character.json"; private static final String INSTANCES_1_NO_RECORD_SOURCE = "/instances_1_withNoRecordSource.json"; private static final String INSTANCES_3 = "/instances_3.json"; @@ -516,7 +518,9 @@ private void handleRecordStorageResultGetResponse(RoutingContext ctx) { } else if (uri.contains(SRS_RECORD_WITH_NEW_METADATA_DATE)) { String json = getJsonObjectFromFileAsString(SOURCE_STORAGE_RESULT_URI + SRS_RECORD); successResponse(ctx, json.replaceAll("REPLACE_ME", NEW_METADATA_DATE_FORMAT)); - } else { + } else if (uri.contains(String.format("%s=%s", ID_PARAM, EXISTING_IDENTIFIER_WITH_INVALID_CHARACTER))) { + successResponse(ctx, getJsonObjectFromFileAsString(SOURCE_STORAGE_RESULT_URI + INSTANCES_1_WITH_INVALID_CHARACTER));} + else { successResponse(ctx, getJsonObjectFromFileAsString(SOURCE_STORAGE_RESULT_URI + INSTANCES_10_TOTAL_RECORDS_11)); } logger.info("Mock returns http status code: {}", ctx.response() diff --git a/src/test/resources/source-storage/source-records/instances_1_with_invalid_character.json b/src/test/resources/source-storage/source-records/instances_1_with_invalid_character.json new file mode 100644 index 000000000..2fd4213c7 --- /dev/null +++ b/src/test/resources/source-storage/source-records/instances_1_with_invalid_character.json @@ -0,0 +1,355 @@ +{ + "sourceRecords": [ + { + "recordId": "3c4ae3f3-b460-4a89-a2f9-78ce3145e4fc", + "snapshotId": "dcca0c83-f338-4434-8770-e33dc31629b8", + "recordType": "MARC", + "rawRecord": { + "id": "f53a1d59-9149-4871-bf2f-807cf689e735", + "content": "donde es posible avanzar. monto representativo" + }, + "parsedRecord": { + "id": "e5eef701-8880-47eb-a7c3-8b2dddd1ebe3", + "content": { + "leader": "\u000b01344nja a2200289 c 4500", + "fields": [ + { + "001": "1011162431" + }, + { + "003": "DE-601" + }, + { + "005": "20180118183625.0" + }, + { + "007": "su uuuuuuuuuuu" + }, + { + "008": "180118s2017 000 0 ger d" + }, + { + "035": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "a": "(DE-599)GBV1011162431" + } + ] + } + }, + { + "040": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "b": "ger" + }, + { + "c": "GBVCP" + }, + { + "e": "rda" + } + ] + } + }, + { + "041": { + "ind1": "0", + "ind2": " ", + "subfields": [ + { + "a": "ger" + } + ] + } + }, + { + "100": { + "ind1": "1", + "ind2": " ", + "subfields": [ + { + "a": "Bach, Johann Sebastian" + }, + { + "e": "KomponistIn" + }, + { + "4": "cmp" + }, + { + "0": "(DE-601)134579348" + }, + { + "0": "(DE-588)11850553X" + } + ] + } + }, + { + "240": { + "ind1": "1", + "ind2": "0", + "subfields": [ + { + "0": "(DE-601)701589477" + }, + { + "0": "(DE-588)300007736" + }, + { + "a": "Ich habe genung" + } + ] + } + }, + { + "245": { + "ind1": "1", + "ind2": "0", + "subfields": [ + { + "a": "Cantatas for bass" + }, + { + "n": "4" + }, + { + "p": "Ich habe genug : BWV 82 / Johann Sebastian Bach ; Matthias Goerne, baritone ; Freiburger Barockorchester, Gottfried von der Goltz, violin and conductor" + } + ] + } + }, + { + "246": { + "ind1": "1", + "ind2": "3", + "subfields": [ + { + "i": "Abweichender Titel" + }, + { + "a": "Ich habe genung" + } + ] + } + }, + { + "300": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "a": "Track 10-14" + } + ] + } + }, + { + "336": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "a": "aufgeführte Musik" + }, + { + "b": "prm" + }, + { + "2": "rdacontent" + } + ] + } + }, + { + "337": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "a": "audio" + }, + { + "b": "s" + }, + { + "2": "rdamedia" + } + ] + } + }, + { + "338": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "a": "Audiodisk" + }, + { + "b": "sd" + }, + { + "2": "rdacarrier" + } + ] + } + }, + { + "700": { + "ind1": "1", + "ind2": " ", + "subfields": [ + { + "a": "Arfken, Katharina" + }, + { + "e": "InstrumentalmusikerIn" + }, + { + "4": "itr" + }, + { + "0": "(DE-601)576364940" + }, + { + "0": "(DE-588)135158265" + } + ] + } + }, + { + "700": { + "ind1": "1", + "ind2": " ", + "subfields": [ + { + "a": "Goltz, Gottfried von der" + }, + { + "e": "DirigentIn" + }, + { + "4": "cnd" + }, + { + "0": "(DE-601)081724969" + }, + { + "0": "(DE-588)122080912" + } + ] + } + }, + { + "710": { + "ind1": "2", + "ind2": " ", + "subfields": [ + { + "a": "Freiburger Barockorchester" + }, + { + "e": "InstrumentalmusikerIn" + }, + { + "4": "itr" + }, + { + "0": "(DE-601)12121060X" + }, + { + "0": "(DE-588)5066798-1" + } + ] + } + }, + { + "773": { + "ind1": "0", + "ind2": " ", + "subfields": [ + { + "w": "(DE-601)895161729" + }, + { + "t": "Cantatas for bass, Bach, Johann Sebastian. - Arles : Harmonia Mundi" + } + ] + } + }, + { + "900": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "a": "GBV" + }, + { + "b": "SBB-PK Berlin <1+1A>" + } + ] + } + }, + { + "954": { + "ind1": " ", + "ind2": " ", + "subfields": [ + { + "0": "SBB-PK Berlin <1+1A>" + }, + { + "a": "11" + }, + { + "b": "1742288871" + }, + { + "c": "01" + }, + { + "x": "0001" + } + ] + } + }, + { + "999": { + "ind1": "f", + "ind2": "f", + "subfields": [ + { + "s": "3c4ae3f3-b460-4a89-a2f9-78ce3145e4fc" + }, + { + "i": "00000000-0000-4a89-a2f9-78ce3145e4fc" + } + ] + } + } + ] + } + }, + "externalIdsHolder": { + "instanceId": "00000000-0000-4a89-a2f9-78ce3145e4fc" + }, + "additionalInfo": { + "suppressDiscovery": false + }, + "metadata": { + "createdDate": "2018-11-20T07:23:11.172+0000", + "createdByUserId": "9df9917d-5c28-5326-82c6-e590e3b56092", + "updatedDate": "2018-11-20T07:23:11.172+0000", + "updatedByUserId": "9df9917d-5c28-5326-82c6-e590e3b56092" + } + } + ], + "totalRecords": 1 +}