Release 3.13.1 (#373)

* MODOAIPMH-540: invalidRecordContent error code intro (#372) * Update news.md * [maven-release-plugin] prepare release v3.13.1 * [maven-release-plugin] prepare for next development iteration --------- Co-authored-by: Dmytro Bykov <[email protected]>
folio-org · Apr 5, 2024 · 0687979 · 0687979
1 parent 1deba53
commit 0687979
Show file tree

Hide file tree

Showing 9 changed files with 430 additions and 4 deletions.
diff --git 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
@@ -2,7 +2,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>org.folio</groupId>
   <artifactId>mod-oai-pmh</artifactId>
-  <version>3.13.1-SNAPSHOT</version>
+  <version>3.13.2-SNAPSHOT</version>
 
   <name>OAI-PMH Repository Business Logic</name>
   <description>Business logic to support the Open Archives Initiative Protocol for Metadata Harvesting</description>

diff --git a/ramls/schemas/OAI-PMH.xsd b/ramls/schemas/OAI-PMH.xsd
@@ -89,6 +89,7 @@
       <enumeration value="badResumptionToken"/>
       <enumeration value="noSetHierarchy"/>
       <enumeration value="serviceUnavailable"/>
+      <enumeration value="invalidRecordContent"/>
     </restriction>
   </simpleType>
 

diff --git 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
@@ -472,8 +472,10 @@ protected Future<Response> processRecords(Context ctx, Request request,
     Promise<Response> 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<Response> processRecords(Context ctx, Request request,
     return oaiResponsePromise.future();
   }
 
+  boolean noRecordsFoundResultCheck(Map<String, RecordType> recordsMap, JsonArray items,  VerbType verb){
+    return recordsMap.isEmpty() &&
+      (jsonArrayIsEmpty(items) || (jsonArrayNotEmpty(items) && VerbType.GET_RECORD != verb));
+  }
+
+  boolean conversionIntoJaxbObjectIssueCheck(Map<String, RecordType> 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<Map<String, RecordType>> buildRecords(Context context, Request re
 
     Map<String, RecordType> 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
@@ -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
@@ -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
@@ -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()