Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion descriptors/ModuleDescriptor-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
},
{
"id": "source-storage-records",
"version": "3.2",
"version": "3.3",
"handlers": [
{
"methods": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions ramls/source-record-storage-records.raml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading