From 18475c6a6ddf36913be5db92bd85323b03bb4316 Mon Sep 17 00:00:00 2001 From: RomanChernetskyi <111740564+RomanChernetskyi@users.noreply.github.com> Date: Wed, 7 Feb 2024 12:01:30 +0200 Subject: [PATCH] [MODSOURCE-732] Change logic of DELETE record endpoint (#596) --- descriptors/ModuleDescriptor-template.json | 2 +- .../main/java/org/folio/dao/util/IdType.java | 3 +- .../rest/impl/SourceStorageRecordsImpl.java | 8 +- .../org/folio/services/RecordService.java | 2 + .../org/folio/services/RecordServiceImpl.java | 22 ++- .../org/folio/rest/impl/RecordApiTest.java | 142 +++++++++++++++++- .../rest/impl/SourceStorageStreamApiTest.java | 3 +- ramls/source-record-storage-records.raml | 6 + 8 files changed, 169 insertions(+), 19 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 39370f10b..2c274fcb8 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -54,7 +54,7 @@ }, { "id": "source-storage-records", - "version": "3.2", + "version": "3.3", "handlers": [ { "methods": [ diff --git a/mod-source-record-storage-server/src/main/java/org/folio/dao/util/IdType.java b/mod-source-record-storage-server/src/main/java/org/folio/dao/util/IdType.java index 5ba04bd17..83b631798 100644 --- a/mod-source-record-storage-server/src/main/java/org/folio/dao/util/IdType.java +++ b/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; diff --git a/mod-source-record-storage-server/src/main/java/org/folio/rest/impl/SourceStorageRecordsImpl.java b/mod-source-record-storage-server/src/main/java/org/folio/rest/impl/SourceStorageRecordsImpl.java index 70bd30db8..509a95bc3 100644 --- a/mod-source-record-storage-server/src/main/java/org/folio/rest/impl/SourceStorageRecordsImpl.java +++ b/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 okapiHeaders, + public void deleteSourceStorageRecordsById(String id, String idType, Map okapiHeaders, Handler> 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) { diff --git a/mod-source-record-storage-server/src/main/java/org/folio/services/RecordService.java b/mod-source-record-storage-server/src/main/java/org/folio/services/RecordService.java index 8b7cf1899..f49ce7528 100644 --- a/mod-source-record-storage-server/src/main/java/org/folio/services/RecordService.java +++ b/mod-source-record-storage-server/src/main/java/org/folio/services/RecordService.java @@ -253,4 +253,6 @@ public interface RecordService { * @return void future */ Future updateRecordsState(String matchedId, RecordState state, RecordType recordType, String tenantId); + + Future deleteRecordById(String id, IdType idType, String tenantId); } diff --git a/mod-source-record-storage-server/src/main/java/org/folio/services/RecordServiceImpl.java b/mod-source-record-storage-server/src/main/java/org/folio/services/RecordServiceImpl.java index 505ffed11..2f922c3b1 100644 --- a/mod-source-record-storage-server/src/main/java/org/folio/services/RecordServiceImpl.java +++ b/mod-source-record-storage-server/src/main/java/org/folio/services/RecordServiceImpl.java @@ -35,7 +35,11 @@ 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; @@ -43,9 +47,6 @@ 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 updateRecordsState(String matchedId, RecordState state, Reco return recordDao.updateRecordsState(matchedId, state, recordType, tenantId); } + @Override + public Future 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 setMatchedIdForRecord(Record record, String tenantId) { String marcField999s = getFieldFromMarcRecord(record, TAG_999, INDICATOR, INDICATOR, SUBFIELD_S); if (marcField999s != null) { diff --git a/mod-source-record-storage-server/src/test/java/org/folio/rest/impl/RecordApiTest.java b/mod-source-record-storage-server/src/test/java/org/folio/rest/impl/RecordApiTest.java index 3761a3ef9..89c9021c2 100644 --- a/mod-source-record-storage-server/src/test/java/org/folio/rest/impl/RecordApiTest.java +++ b/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); diff --git a/mod-source-record-storage-server/src/test/java/org/folio/rest/impl/SourceStorageStreamApiTest.java b/mod-source-record-storage-server/src/test/java/org/folio/rest/impl/SourceStorageStreamApiTest.java index cf232129b..a5fe13b10 100644 --- a/mod-source-record-storage-server/src/test/java/org/folio/rest/impl/SourceStorageStreamApiTest.java +++ b/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 diff --git a/ramls/source-record-storage-records.raml b/ramls/source-record-storage-records.raml index de6517f41..d5890f655 100644 --- a/ramls/source-record-storage-records.raml +++ b/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: