[MODSOURCE-732] Change logic of DELETE record endpoint (#596)

descriptors/ModuleDescriptor-template.json

@@ -54,7 +54,7 @@
     },
     {
       "id": "source-storage-records",
-      "version": "3.2",
+      "version": "3.3",
       "handlers": [
         {
           "methods": [

mod-source-record-storage-server/src/main/java/org/folio/dao/util/IdType.java

@@ -7,7 +7,8 @@ public enum IdType {
   AUTHORITY("authorityId"),
   EXTERNAL("externalId"),
   // NOTE: not really external id but is default from dto
-  RECORD("matchedId");
+  RECORD("matchedId"),
+  SRS_RECORD("id");
 
   private final String idField;
 

mod-source-record-storage-server/src/main/java/org/folio/rest/impl/SourceStorageRecordsImpl.java

@@ -15,7 +15,6 @@
 
 import org.folio.dataimport.util.ExceptionHelper;
 import org.folio.rest.jaxrs.model.Record;
-import org.folio.rest.jaxrs.model.Record.State;
 import org.folio.rest.jaxrs.resource.SourceStorageRecords;
 import org.folio.rest.tools.utils.TenantTool;
 import org.folio.services.RecordService;
@@ -115,14 +114,11 @@ public void putSourceStorageRecordsGenerationById(String matchedId, Record entit
   }
 
   @Override
-  public void deleteSourceStorageRecordsById(String id, Map<String, String> okapiHeaders,
+  public void deleteSourceStorageRecordsById(String id, String idType, Map<String, String> okapiHeaders,
       Handler<AsyncResult<Response>> asyncResultHandler, Context vertxContext) {
     vertxContext.runOnContext(v -> {
       try {
-        recordService.getRecordById(id, tenantId)
-          .map(recordOptional -> recordOptional.orElseThrow(() -> new NotFoundException(format(NOT_FOUND_MESSAGE, Record.class.getSimpleName(), id))))
-            .compose(record -> record.getState().equals(State.DELETED) ? Future.succeededFuture(true)
-              : recordService.updateRecord(record.withState(State.DELETED), tenantId).map(r -> true))
+        recordService.deleteRecordById(id, toExternalIdType(idType), tenantId).map(r -> true)
             .map(updated -> DeleteSourceStorageRecordsByIdResponse.respond204()).map(Response.class::cast)
             .otherwise(ExceptionHelper::mapExceptionToResponse).onComplete(asyncResultHandler);
       } catch (Exception e) {

mod-source-record-storage-server/src/main/java/org/folio/services/RecordService.java

@@ -253,4 +253,6 @@ public interface RecordService {
    * @return void future
    */
   Future<Void> updateRecordsState(String matchedId, RecordState state, RecordType recordType, String tenantId);
+
+  Future<Void> deleteRecordById(String id, IdType idType, String tenantId);
 }

mod-source-record-storage-server/src/main/java/org/folio/services/RecordServiceImpl.java

@@ -35,17 +35,18 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.folio.dao.util.IdType;
+import org.folio.dao.util.ParsedRecordDaoUtil;
 import org.folio.dao.util.RecordDaoUtil;
+import org.folio.dao.util.RecordType;
+import org.folio.dao.util.SnapshotDaoUtil;
 import org.folio.okapi.common.GenericCompositeFuture;
 import org.folio.services.exceptions.DuplicateRecordException;
 import org.jooq.Condition;
 import org.jooq.OrderField;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.folio.dao.RecordDao;
-import org.folio.dao.util.IdType;
-import org.folio.dao.util.RecordType;
-import org.folio.dao.util.SnapshotDaoUtil;
 import org.folio.rest.jaxrs.model.FetchParsedRecordsBatchRequest;
 import org.folio.rest.jaxrs.model.FieldRange;
 import org.folio.rest.jaxrs.model.MarcBibCollection;
@@ -75,6 +76,8 @@ public class RecordServiceImpl implements RecordService {
   private static final String DUPLICATE_RECORD_MSG = "Incoming file may contain duplicates";
   private static final String MATCHED_ID_NOT_EQUAL_TO_999_FIELD = "Matched id (%s) not equal to 999ff$s (%s) field";
   private static final String RECORD_WITH_GIVEN_MATCHED_ID_NOT_FOUND = "Record with given matched id (%s) not found";
+  private static final String NOT_FOUND_MESSAGE = "%s with id '%s' was not found";
+  private static final Character DELETED_LEADER_RECORD_STATUS = 'd';
   public static final String UPDATE_RECORD_DUPLICATE_EXCEPTION = "Incoming record could be a duplicate, incoming record generation should not be the same as matched record generation and the execution of job should be started after of creating the previous record generation";
   public static final char SUBFIELD_S = 's';
   public static final char INDICATOR = 'f';
@@ -300,6 +303,19 @@ public Future<Void> updateRecordsState(String matchedId, RecordState state, Reco
     return recordDao.updateRecordsState(matchedId, state, recordType, tenantId);
   }
 
+  @Override
+  public Future<Void> deleteRecordById(String id, IdType idType, String tenantId) {
+    return recordDao.getRecordByExternalId(id, idType, tenantId)
+      .map(recordOptional -> recordOptional.orElseThrow(() -> new NotFoundException(format(NOT_FOUND_MESSAGE, Record.class.getSimpleName(), id))))
+      .map(record -> {
+        record.withState(Record.State.DELETED);
+        record.setAdditionalInfo(record.getAdditionalInfo().withSuppressDiscovery(true));
+        ParsedRecordDaoUtil.updateLeaderStatus(record.getParsedRecord(), DELETED_LEADER_RECORD_STATUS);
+        return record;
+      })
+      .compose(record -> updateRecord(record, tenantId)).map(r -> null);
+  }
+
   private Future<Record> setMatchedIdForRecord(Record record, String tenantId) {
     String marcField999s = getFieldFromMarcRecord(record, TAG_999, INDICATOR, INDICATOR, SUBFIELD_S);
     if (marcField999s != null) {

mod-source-record-storage-server/src/test/java/org/folio/rest/impl/RecordApiTest.java

@@ -21,6 +21,7 @@
 import io.vertx.ext.unit.TestContext;
 import io.vertx.ext.unit.junit.VertxUnitRunner;
 import org.apache.http.HttpStatus;
+import org.folio.dao.util.ParsedRecordDaoUtil;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -760,7 +761,7 @@ public void shouldReturnNotFoundOnDeleteWhenRecordDoesNotExist() {
   }
 
   @Test
-  public void shouldDeleteExistingMarcRecordOnDelete(TestContext testContext) {
+  public void shouldDeleteExistingMarcRecordOnDeleteByRecordId(TestContext testContext) {
     postSnapshots(testContext, snapshot_2);
 
     Async async = testContext.async();
@@ -783,13 +784,19 @@ public void shouldDeleteExistingMarcRecordOnDelete(TestContext testContext) {
     async.complete();
 
     async = testContext.async();
-    RestAssured.given()
+    Response deletedResponse = RestAssured.given()
       .spec(spec)
       .when()
-      .get(SOURCE_STORAGE_RECORDS_PATH + "/" + parsed.getId())
-      .then()
-      .statusCode(HttpStatus.SC_OK)
-      .body("deleted", is(true));
+      .get(SOURCE_STORAGE_RECORDS_PATH + "/" + parsed.getId());
+    Assert.assertEquals(HttpStatus.SC_OK, deletedResponse.getStatusCode());
+    Record deletedRecord = deletedResponse.body().as(Record.class);
+
+    Assert.assertEquals(true, deletedRecord.getDeleted());
+    Assert.assertEquals(Record.State.DELETED, deletedRecord.getState());
+    Assert.assertEquals("d", deletedRecord.getLeaderRecordStatus());
+    Assert.assertEquals(true, deletedRecord.getAdditionalInfo().getSuppressDiscovery());
+    Assert.assertEquals("d", ParsedRecordDaoUtil.getLeaderStatus(deletedRecord.getParsedRecord()));
+
     async.complete();
 
     async = testContext.async();
@@ -818,10 +825,131 @@ public void shouldDeleteExistingMarcRecordOnDelete(TestContext testContext) {
       .get(SOURCE_STORAGE_RECORDS_PATH + "/" + errorRecord.getId())
       .then()
       .statusCode(HttpStatus.SC_OK)
-      .body("deleted", is(true));
+      .body("deleted", is(true))
+      .body("state", is("DELETED"))
+      .body("additionalInfo.suppressDiscovery", is(true));
     async.complete();
   }
 
+  @Test
+  public void shouldDeleteExistingMarcRecordOnDeleteByInstanceId(TestContext testContext) {
+    postSnapshots(testContext, snapshot_1);
+
+    String srsId = UUID.randomUUID().toString();
+    String instanceId = UUID.randomUUID().toString();
+
+    ParsedRecord parsedRecord = new ParsedRecord().withId(srsId)
+      .withContent(new JsonObject().put("leader", "01542ccm a2200361   4500")
+        .put("fields", new JsonArray().add(new JsonObject().put("999", new JsonObject()
+          .put("subfields", new JsonArray().add(new JsonObject().put("s", srsId)).add(new JsonObject().put("i", instanceId)))))));
+
+    Record newRecord = new Record()
+      .withId(srsId)
+      .withSnapshotId(snapshot_1.getJobExecutionId())
+      .withRecordType(Record.RecordType.MARC_BIB)
+      .withRawRecord(rawMarcRecord)
+      .withParsedRecord(parsedRecord)
+      .withState(Record.State.ACTUAL)
+      .withExternalIdsHolder(new ExternalIdsHolder()
+        .withInstanceId(instanceId))
+      .withMatchedId(UUID.randomUUID().toString());
+
+    Async async = testContext.async();
+    Response createParsed = RestAssured.given()
+      .spec(spec)
+      .body(newRecord)
+      .when()
+      .post(SOURCE_STORAGE_RECORDS_PATH);
+    assertThat(createParsed.statusCode(), is(HttpStatus.SC_CREATED));
+    Record parsed = createParsed.body().as(Record.class);
+    async.complete();
+
+    async = testContext.async();
+    RestAssured.given()
+      .spec(spec)
+      .when()
+      .delete(SOURCE_STORAGE_RECORDS_PATH + "/" + instanceId + "?idType=INSTANCE")
+      .then()
+      .statusCode(HttpStatus.SC_NO_CONTENT);
+    async.complete();
+
+    async = testContext.async();
+    Response deletedResponse = RestAssured.given()
+      .spec(spec)
+      .when()
+      .get(SOURCE_STORAGE_RECORDS_PATH + "/" + parsed.getId());
+    Assert.assertEquals(HttpStatus.SC_OK, deletedResponse.getStatusCode());
+    Record deletedRecord = deletedResponse.body().as(Record.class);
+
+    Assert.assertEquals(true, deletedRecord.getDeleted());
+    Assert.assertEquals(Record.State.DELETED, deletedRecord.getState());
+    Assert.assertEquals("d", deletedRecord.getLeaderRecordStatus());
+    Assert.assertEquals(true, deletedRecord.getAdditionalInfo().getSuppressDiscovery());
+    Assert.assertEquals("d", ParsedRecordDaoUtil.getLeaderStatus(deletedRecord.getParsedRecord()));
+
+    async.complete();
+  }
+
+  @Test
+  public void shouldReturnNotFoundIfTryingToDeleteRecordWithStateNotActual(TestContext testContext) {
+    postSnapshots(testContext, snapshot_1);
+
+    Record newRecord1 = new Record()
+      .withId(UUID.randomUUID().toString())
+      .withSnapshotId(snapshot_1.getJobExecutionId())
+      .withRecordType(Record.RecordType.MARC_BIB)
+      .withRawRecord(rawMarcRecord)
+      .withParsedRecord(parsedMarcRecord)
+      .withState(Record.State.OLD)
+      .withMatchedId(UUID.randomUUID().toString());
+
+    Async async = testContext.async();
+    Response createParsed = RestAssured.given()
+      .spec(spec)
+      .body(newRecord1)
+      .when()
+      .post(SOURCE_STORAGE_RECORDS_PATH);
+    assertThat(createParsed.statusCode(), is(HttpStatus.SC_CREATED));
+    async.complete();
+
+    async = testContext.async();
+    RestAssured.given()
+      .spec(spec)
+      .when()
+      .delete(SOURCE_STORAGE_RECORDS_PATH + "/" + newRecord1.getId())
+      .then()
+      .statusCode(HttpStatus.SC_NOT_FOUND);
+    async.complete();
+
+    Record newRecord2 = new Record()
+      .withId(UUID.randomUUID().toString())
+      .withSnapshotId(snapshot_1.getJobExecutionId())
+      .withRecordType(Record.RecordType.MARC_BIB)
+      .withRawRecord(rawMarcRecord)
+      .withParsedRecord(parsedMarcRecord)
+      .withState(Record.State.DELETED)
+      .withMatchedId(UUID.randomUUID().toString());
+
+    async = testContext.async();
+    Response createParsed2 = RestAssured.given()
+      .spec(spec)
+      .body(newRecord2)
+      .when()
+      .post(SOURCE_STORAGE_RECORDS_PATH);
+    assertThat(createParsed2.statusCode(), is(HttpStatus.SC_CREATED));
+    async.complete();
+
+    async = testContext.async();
+    RestAssured.given()
+      .spec(spec)
+      .when()
+      .delete(SOURCE_STORAGE_RECORDS_PATH + "/" + newRecord2.getId())
+      .then()
+      .statusCode(HttpStatus.SC_NOT_FOUND);
+    async.complete();
+  }
+
+
   @Test
   public void shouldDeleteExistingEdifactRecordOnDelete(TestContext testContext) {
     postSnapshots(testContext, snapshot_3);

mod-source-record-storage-server/src/test/java/org/folio/rest/impl/SourceStorageStreamApiTest.java

@@ -1067,7 +1067,8 @@ public void shouldReturnIdOnSearchMarcRecordIdsWhenRecordWasDeleted(TestContext
       .statusCode(HttpStatus.SC_NO_CONTENT);
     async.complete();
     MarcRecordSearchRequest searchRequest = new MarcRecordSearchRequest();
-    searchRequest.setLeaderSearchExpression("p_05 = 'c' and p_06 = 'c' and p_07 = 'm'");
+    searchRequest.setLeaderSearchExpression("p_05 = 'd' and p_06 = 'c' and p_07 = 'm'");
+    searchRequest.setSuppressFromDiscovery(true);
     searchRequest.setFieldsSearchExpression("001.value = '393893' and 005.value ^= '2014110' and 035.ind1 = '#'");
     searchRequest.setDeleted(true);
     // when

ramls/source-record-storage-records.raml

@@ -88,6 +88,12 @@ resourceTypes:
             application/json:
               type: record
     delete:
+      queryParameters:
+        idType:
+          description: Type of Id for Record lookup
+          type: string
+          example: INSTANCE
+          default: SRS_RECORD
       responses:
         204:
     /formatted: