diff --git a/NEWS.md b/NEWS.md index bc2a1e827..07be371ca 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,7 @@ * Requires `API_NAME vX.Y` ### Features +* Unable to delete local Subject types/sources when they are linked to an Instance ([MODINVSTOR-1284](https://folio-org.atlassian.net/browse/MODINVSTOR-1284)) * Modify endpoint for bulk instances upsert with publish events flag ([MODINVSTOR-1283](https://folio-org.atlassian.net/browse/MODINVSTOR-1283)) * Change Kafka event publishing keys for holdings and items ([MODINVSTOR-1281](https://folio-org.atlassian.net/browse/MODINVSTOR-1281)) * Merge custom ECS TLR feature branch into master ([MODINVSTOR-1262](https://folio-org.atlassian.net/browse/MODINVSTOR-1262)) diff --git a/src/main/java/org/folio/persist/InstanceRepository.java b/src/main/java/org/folio/persist/InstanceRepository.java index 39ab01170..b6a7a4cc0 100644 --- a/src/main/java/org/folio/persist/InstanceRepository.java +++ b/src/main/java/org/folio/persist/InstanceRepository.java @@ -10,6 +10,7 @@ import io.vertx.core.Future; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; +import io.vertx.pgclient.PgException; import io.vertx.sqlclient.Row; import io.vertx.sqlclient.RowSet; import io.vertx.sqlclient.RowStream; @@ -20,12 +21,14 @@ import java.util.stream.Collectors; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.commons.lang3.tuple.Pair; import org.folio.cql2pgjson.CQL2PgJSON; import org.folio.cql2pgjson.exception.FieldException; import org.folio.dbschema.ObjectMapperTool; import org.folio.rest.exceptions.BadRequestException; import org.folio.rest.jaxrs.model.Instance; import org.folio.rest.jaxrs.model.ResultInfo; +import org.folio.rest.persist.Conn; import org.folio.rest.persist.SQLConnection; import org.folio.rest.persist.cql.CQLQueryValidationException; import org.folio.rest.persist.cql.CQLWrapper; @@ -35,11 +38,109 @@ public class InstanceRepository extends AbstractRepository { private static final String INSTANCE_SET_VIEW = "instance_set"; private static final String INSTANCE_HOLDINGS_ITEM_VIEW = "instance_holdings_item_view"; private static final String INVENTORY_VIEW_JSONB_FIELD = "inventory_view.jsonb"; + private static final String INSTANCE_SUBJECT_SOURCE_TABLE = "instance_subject_source"; + private static final String INSTANCE_SUBJECT_TYPE_TABLE = "instance_subject_type"; + public InstanceRepository(Context context, Map okapiHeaders) { super(postgresClient(context, okapiHeaders), INSTANCE_TABLE, Instance.class); } + public Future> unlinkInstanceFromSubjectSource(Conn conn, String instanceId) { + try { + String sql = unlinkInstanceFromSubjectSql(INSTANCE_SUBJECT_SOURCE_TABLE, instanceId); + return conn.execute(sql); + } catch (PgException e) { + return Future.failedFuture(new BadRequestException(e.getMessage())); + } + } + + public Future> unlinkInstanceFromSubjectType(Conn conn, String instanceId) { + try { + String sql = unlinkInstanceFromSubjectSql(INSTANCE_SUBJECT_TYPE_TABLE, instanceId); + return conn.execute(sql); + } catch (PgException e) { + return Future.failedFuture(new BadRequestException(e.getMessage())); + } + } + + private String unlinkInstanceFromSubjectSql(String table, String id) { + return String.format("DELETE FROM %s WHERE instance_id = '%s'; ", + postgresClientFuturized.getFullTableName(table), id); + } + + public Future> batchLinkSubjectSource(Conn conn, List> sourcePairs) { + try { + String sql = """ + INSERT INTO %s (instance_id, source_id) + VALUES %s + ON CONFLICT DO NOTHING; + """ + .formatted( + postgresClientFuturized.getFullTableName(INSTANCE_SUBJECT_SOURCE_TABLE), + sourcePairs.stream() + .map(pair -> String.format("('%s', '%s')", pair.getKey(), pair.getValue())) + .collect(Collectors.joining(", ")) + ); + return conn.execute(sql); + } catch (PgException e) { + return Future.failedFuture(new BadRequestException(e.getMessage())); + } + } + + public Future> batchLinkSubjectType(Conn conn, List> typePairs) { + try { + String sql = """ + INSERT INTO %s (instance_id, type_id) + VALUES %s + ON CONFLICT DO NOTHING; + """ + .formatted( + postgresClientFuturized.getFullTableName(INSTANCE_SUBJECT_TYPE_TABLE), + typePairs.stream() + .map(pair -> String.format("('%s', '%s')", pair.getKey(), pair.getValue())) + .collect(Collectors.joining(", ")) + ); + return conn.execute(sql); + } catch (PgException e) { + return Future.failedFuture(new BadRequestException(e.getMessage())); + } + + } + + public Future> batchUnlinkSubjectSource(Conn conn, String instanceId, List sourceIds) { + try { + String sql = """ + DELETE FROM %s WHERE instance_id = '%s' AND source_id IN ( %s ); + """ + .formatted( + postgresClientFuturized.getFullTableName(INSTANCE_SUBJECT_SOURCE_TABLE), + instanceId, + sourceIds.stream().map(id -> "'" + id + "'").collect(Collectors.joining(", ")) + ); + return conn.execute(sql); + } catch (PgException e) { + return Future.failedFuture(new BadRequestException(e.getMessage())); + } + } + + public Future> batchUnlinkSubjectType(Conn conn, String instanceId, List typeIds) { + try { + String sql = """ + DELETE FROM %s WHERE instance_id = '%s' AND type_id IN ( %s ); + """ + .formatted( + postgresClientFuturized.getFullTableName(INSTANCE_SUBJECT_TYPE_TABLE), + instanceId, + typeIds.stream().map(id -> "'" + id + "'") + .collect(Collectors.joining(", ")) + ); + return conn.execute(sql); + } catch (PgException e) { + return Future.failedFuture(new BadRequestException(e.getMessage())); + } + } + public Future> getAllIds(SQLConnection connection) { return postgresClientFuturized.selectStream(connection, "SELECT id FROM " + postgresClientFuturized.getFullTableName(INSTANCE_TABLE)); diff --git a/src/main/java/org/folio/rest/impl/InstanceBatchSyncApi.java b/src/main/java/org/folio/rest/impl/InstanceBatchSyncApi.java index d03bf5117..5c13be59a 100644 --- a/src/main/java/org/folio/rest/impl/InstanceBatchSyncApi.java +++ b/src/main/java/org/folio/rest/impl/InstanceBatchSyncApi.java @@ -1,6 +1,6 @@ package org.folio.rest.impl; -import static org.folio.rest.jaxrs.resource.InstanceStorageBatchSynchronous.PostInstanceStorageBatchSynchronousResponse.respond500WithTextPlain; +import static org.folio.rest.support.EndpointFailureHandler.handleFailure; import io.vertx.core.AsyncResult; import io.vertx.core.Context; @@ -26,7 +26,7 @@ public void postInstanceStorageBatchSynchronous(boolean upsert, InstancesPost en new InstanceService(vertxContext, okapiHeaders) .createInstances(instances.getInstances(), upsert, true, true) - .otherwise(cause -> respond500WithTextPlain(cause.getMessage())) + .onFailure(handleFailure(asyncResultHandler)) .onComplete(asyncResultHandler); } } diff --git a/src/main/java/org/folio/rest/impl/InstanceBatchSyncUnsafeApi.java b/src/main/java/org/folio/rest/impl/InstanceBatchSyncUnsafeApi.java index fcfde1e77..5e0832d8e 100644 --- a/src/main/java/org/folio/rest/impl/InstanceBatchSyncUnsafeApi.java +++ b/src/main/java/org/folio/rest/impl/InstanceBatchSyncUnsafeApi.java @@ -1,6 +1,6 @@ package org.folio.rest.impl; -import static org.folio.rest.jaxrs.resource.InstanceStorageBatchSynchronousUnsafe.PostInstanceStorageBatchSynchronousUnsafeResponse.respond500WithTextPlain; +import static org.folio.rest.support.EndpointFailureHandler.handleFailure; import io.vertx.core.AsyncResult; import io.vertx.core.Context; @@ -25,7 +25,7 @@ public void postInstanceStorageBatchSynchronousUnsafe(InstancesPost entity, Map< new InstanceService(vertxContext, okapiHeaders) .createInstances(instances.getInstances(), true, false, true) - .otherwise(cause -> respond500WithTextPlain(cause.getMessage())) + .onFailure(handleFailure(asyncResultHandler)) .onComplete(asyncResultHandler); } } diff --git a/src/main/java/org/folio/services/instance/InstanceService.java b/src/main/java/org/folio/services/instance/InstanceService.java index 96a3ada2f..df1bf3ab3 100644 --- a/src/main/java/org/folio/services/instance/InstanceService.java +++ b/src/main/java/org/folio/services/instance/InstanceService.java @@ -21,18 +21,28 @@ import io.vertx.core.Future; import io.vertx.core.Promise; import io.vertx.ext.web.RoutingContext; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; import javax.ws.rs.core.Response; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.folio.persist.InstanceMarcRepository; import org.folio.persist.InstanceRelationshipRepository; import org.folio.persist.InstanceRepository; +import org.folio.rest.exceptions.ValidationException; +import org.folio.rest.jaxrs.model.Errors; import org.folio.rest.jaxrs.model.Instance; import org.folio.rest.jaxrs.model.InstanceWithoutPubPeriod; import org.folio.rest.jaxrs.model.InventoryViewInstance; import org.folio.rest.jaxrs.model.InventoryViewInstanceWithoutPubPeriod; +import org.folio.rest.jaxrs.model.Subject; import org.folio.rest.jaxrs.resource.InstanceStorage; +import org.folio.rest.persist.Conn; import org.folio.rest.persist.PostgresClient; import org.folio.rest.support.CqlQuery; import org.folio.rest.support.HridManager; @@ -51,6 +61,7 @@ public class InstanceService { private final HridManager hridManager; private final Context vertxContext; private final Map okapiHeaders; + private final PostgresClient postgresClient; private final InstanceDomainEventPublisher domainEventPublisher; private final InstanceRepository instanceRepository; private final InstanceMarcRepository marcRepository; @@ -61,7 +72,7 @@ public InstanceService(Context vertxContext, Map okapiHeaders) { this.vertxContext = vertxContext; this.okapiHeaders = okapiHeaders; - final PostgresClient postgresClient = postgresClient(vertxContext, okapiHeaders); + postgresClient = postgresClient(vertxContext, okapiHeaders); hridManager = new HridManager(postgresClient); domainEventPublisher = new InstanceDomainEventPublisher(vertxContext, okapiHeaders); instanceRepository = new InstanceRepository(vertxContext, okapiHeaders); @@ -103,19 +114,25 @@ public Future createInstance(Instance entity) { return hridManager.populateHrid(entity) .compose(NotesValidators::refuseLongNotes) .compose(instance -> { - final Promise postResponse = promise(); + final Promise postResponse = promise(); // promise for the entire process - post(INSTANCE_TABLE, instance, okapiHeaders, vertxContext, - InstanceStorage.PostInstanceStorageInstancesResponse.class, postResponse); + // execute post and linking logic within same transaction + return postgresClient.withTrans(conn -> { - return postResponse.future() - // Return the response without waiting for a domain event publish - // to complete. Units of work performed by this service is the same - // but the ordering of the units of work provides a benefit to the - // api client invoking this endpoint. The response is returned - // a little earlier so the api client can continue its processing - // while the domain event publish is satisfied. - .onSuccess(domainEventPublisher.publishCreated()); + Promise postPromise = postInstance(instance); + + // Chain the batchLinkSubjects operation after postPromise completes + return postPromise.future() + .compose(response -> batchLinkSubjects(conn, instance.getId(), instance.getSubjects()) + .map(v -> response) + ); + }).onComplete(transactionResult -> { + if (transactionResult.succeeded()) { + postResponse.complete(transactionResult.result()); + } else { + postResponse.fail(transactionResult.cause()); + } + }).onSuccess(domainEventPublisher.publishCreated()); }) .map(ResponseHandlerUtil::handleHridError); } @@ -129,12 +146,26 @@ public Future createInstances(List instances, boolean upsert .compose(NotesValidators::refuseInstanceLongNotes) .compose(notUsed -> buildBatchOperationContext(upsert, instances, instanceRepository, Instance::getId, publishEvents)) - .compose(batchOperation -> - // Can use instances list here directly because the class is stateful - postSync(INSTANCE_TABLE, instances, MAX_ENTITIES, upsert, optimisticLocking, okapiHeaders, - vertxContext, PostInstanceStorageBatchSynchronousResponse.class) - .onSuccess(domainEventPublisher.publishCreatedOrUpdated(batchOperation)) - ) + .compose(batchOperation -> { + final Promise postResponse = promise(); + + postgresClient.withTrans(conn -> { + + Promise postPromise = postSyncInstance(instances, upsert, optimisticLocking); + + return postPromise.future() + .compose(response -> batchLinkSubjects(conn, batchOperation.getRecordsToBeCreated()) + .map(v -> response)); + }).onComplete(transactionResult -> { + if (transactionResult.succeeded()) { + postResponse.complete(transactionResult.result()); + } else { + postResponse.fail(transactionResult.cause()); + } + }); + return postResponse.future() + .onSuccess(domainEventPublisher.publishCreatedOrUpdated(batchOperation)); + }) .map(ResponseHandlerUtil::handleHridError); } @@ -150,14 +181,155 @@ public Future updateInstance(String id, Instance newInstance) { }) .compose(oldInstance -> { final Promise putResult = promise(); - put(INSTANCE_TABLE, newInstance, id, okapiHeaders, vertxContext, - InstanceStorage.PutInstanceStorageInstancesByInstanceIdResponse.class, putResult); - return putResult.future() + return postgresClient.withTrans(conn -> { + Promise putPromise = putInstance(newInstance, id); + return putPromise.future() + .compose(response -> linkOrUnlinkSubjects(conn, newInstance, oldInstance) + .map(v -> response)); + }).onComplete(transactionResult -> { + if (transactionResult.succeeded()) { + putResult.complete(transactionResult.result()); + } else { + putResult.fail(transactionResult.cause()); + } + }) .onSuccess(domainEventPublisher.publishUpdated(oldInstance)); }); } + /** + * creates instance with separate promise, to use withing single transaction + * alongside with linking subject sources/types. + * */ + private Promise postInstance(Instance instance) { + Promise promise = Promise.promise(); // Promise for the `post` operation + post(INSTANCE_TABLE, instance, okapiHeaders, vertxContext, + InstanceStorage.PostInstanceStorageInstancesResponse.class, + reply -> { + if (reply.succeeded()) { + promise.complete(reply.result()); // Mark `postPromise` as successful + } else { + promise.fail(reply.cause()); // Mark `postPromise` as failed + } + }); + return promise; + } + + private Promise postSyncInstance(List instances, boolean upsert, boolean optimisticLocking) { + Promise promise = Promise.promise(); + postSync(INSTANCE_TABLE, instances, MAX_ENTITIES, upsert, optimisticLocking, okapiHeaders, + vertxContext, PostInstanceStorageBatchSynchronousResponse.class) + .onComplete(response -> { + if (response.succeeded()) { + var result = response.result(); + if (result.getEntity() instanceof Errors errors) { + promise.fail(new ValidationException(errors)); + } else { + promise.complete(result); + } + } else { + promise.fail(response.cause()); + } + }); + return promise; + } + + private Promise putInstance(Instance newInstance, String instanceId) { + Promise promise = Promise.promise(); + put(INSTANCE_TABLE, newInstance, instanceId, okapiHeaders, vertxContext, + InstanceStorage.PutInstanceStorageInstancesByInstanceIdResponse.class, reply -> { + if (reply.succeeded()) { + promise.complete(reply.result()); + } else { + promise.fail(reply.cause()); + } + }); + return promise; + } + + private Future linkOrUnlinkSubjects(Conn conn, Instance newInstance, Instance oldInstance) { + var instanceId = newInstance.getId(); + + if (newInstance.getSubjects().isEmpty()) { + return Future.all( + instanceRepository.unlinkInstanceFromSubjectSource(conn, instanceId), + instanceRepository.unlinkInstanceFromSubjectType(conn, instanceId) + ).mapEmpty(); + } + // Identify the differences between old and new subjects + Set newSubjects = newInstance.getSubjects(); + Set oldSubjects = oldInstance.getSubjects() != null ? oldInstance.getSubjects() : Collections.emptySet(); + + var subjectsToRemove = oldSubjects.stream() + .filter(subject -> !newSubjects.contains(subject)) + .collect(Collectors.toSet()); + + var subjectsToAdd = newSubjects.stream() + .filter(subject -> !oldSubjects.contains(subject)) + .collect(Collectors.toSet()); + + return batchUnlinkSubjects(conn, instanceId, subjectsToRemove) + .compose(v -> batchLinkSubjects(conn, instanceId, subjectsToAdd)); + } + + private Future batchUnlinkSubjects(Conn conn, String instanceId, Set subjectsToRemove) { + if (subjectsToRemove.isEmpty()) { + return Future.succeededFuture(); + } + + var sourceIds = subjectsToRemove.stream() + .map(Subject::getSourceId) + .filter(Objects::nonNull) + .toList(); + + var typeIds = subjectsToRemove.stream() + .map(Subject::getTypeId) + .filter(Objects::nonNull) + .toList(); + + return Future.all( + sourceIds.isEmpty() ? Future.succeededFuture() : + instanceRepository.batchUnlinkSubjectSource(conn, instanceId, sourceIds), + typeIds.isEmpty() ? Future.succeededFuture() : + instanceRepository.batchUnlinkSubjectType(conn, instanceId, typeIds) + ).mapEmpty(); + } + + + private Future batchLinkSubjects(Conn conn, String instanceId, Set subjectsToAdd) { + var sourcePairs = subjectsToAdd.stream() + .filter(subject -> subject.getSourceId() != null) + .map(subject -> Pair.of(instanceId, subject.getSourceId())) + .toList(); + + var typePairs = subjectsToAdd.stream() + .filter(subject -> subject.getTypeId() != null) + .map(subject -> Pair.of(instanceId, subject.getTypeId())) + .toList(); + + if (sourcePairs.isEmpty() && typePairs.isEmpty()) { + return Future.succeededFuture(); // No subjects to link, return success immediately + } + + // Combine both operations into a single future + return Future.all( + sourcePairs.isEmpty() ? Future.succeededFuture() : + instanceRepository.batchLinkSubjectSource(conn, sourcePairs), + typePairs.isEmpty() ? Future.succeededFuture() : + instanceRepository.batchLinkSubjectType(conn, typePairs) + ).mapEmpty(); + } + + private Future batchLinkSubjects(Conn conn, Collection batchInstance) { + return Future.all( + batchInstance.stream() + .map(instance -> batchLinkSubjects(conn, instance.getId(), + instance.getSubjects())) + .toList() + ).mapEmpty(); + } + /** * Deletes all instances but sends only a single domain event (Kafka) message "all records removed", * this is much faster than sending one message for each deleted instance. diff --git a/src/main/java/org/folio/services/subjectsource/SubjectSourceService.java b/src/main/java/org/folio/services/subjectsource/SubjectSourceService.java index a7a73d7dc..e3bbdc8e7 100644 --- a/src/main/java/org/folio/services/subjectsource/SubjectSourceService.java +++ b/src/main/java/org/folio/services/subjectsource/SubjectSourceService.java @@ -81,10 +81,14 @@ public Future update(String id, SubjectSource subjectSource) { public Future delete(String id) { return repository.getById(id) - .compose(oldSubjectSource -> deleteById(SUBJECT_SOURCE, id, okapiHeaders, context, - DeleteSubjectSourcesBySubjectSourceIdResponse.class) - .onSuccess(domainEventService.publishRemoved(oldSubjectSource)) - ); + .compose(oldSubjectSource -> { + if (oldSubjectSource != null) { + return deleteById(SUBJECT_SOURCE, id, okapiHeaders, context, + DeleteSubjectSourcesBySubjectSourceIdResponse.class) + .onSuccess(domainEventService.publishRemoved(oldSubjectSource)); + } + return Future.failedFuture(new NotFoundException("SubjectSource was not found")); + }); } private Future createSubjectSource(SubjectSource subjectSource) { diff --git a/src/main/java/org/folio/services/subjecttype/SubjectTypeService.java b/src/main/java/org/folio/services/subjecttype/SubjectTypeService.java index f58ff490a..96a53e9f6 100644 --- a/src/main/java/org/folio/services/subjecttype/SubjectTypeService.java +++ b/src/main/java/org/folio/services/subjecttype/SubjectTypeService.java @@ -80,10 +80,14 @@ public Future update(String id, SubjectType subjectType) { public Future delete(String id) { return repository.getById(id) - .compose(oldSubjectType -> deleteById(SUBJECT_TYPE, id, okapiHeaders, context, - DeleteSubjectTypesBySubjectTypeIdResponse.class) - .onSuccess(domainEventService.publishRemoved(oldSubjectType)) - ); + .compose(oldSubjectType -> { + if (oldSubjectType != null) { + return deleteById(SUBJECT_TYPE, id, okapiHeaders, context, + DeleteSubjectTypesBySubjectTypeIdResponse.class) + .onSuccess(domainEventService.publishRemoved(oldSubjectType)); + } + return Future.failedFuture(new NotFoundException("SubjectSource was not found")); + }); } private Future createSubjectType(SubjectType subjectType) { diff --git a/src/main/resources/templates/db_scripts/createInstanceSubjectSourceTable.sql b/src/main/resources/templates/db_scripts/createInstanceSubjectSourceTable.sql new file mode 100644 index 000000000..c276a7f6c --- /dev/null +++ b/src/main/resources/templates/db_scripts/createInstanceSubjectSourceTable.sql @@ -0,0 +1,8 @@ + +CREATE TABLE IF NOT EXISTS ${myuniversity}_${mymodule}.instance_subject_source ( + instance_id UUID, + source_id UUID, + CONSTRAINT fk_instance_id FOREIGN KEY(instance_id) REFERENCES ${myuniversity}_${mymodule}.instance(id) ON DELETE CASCADE, + CONSTRAINT fk_source_id FOREIGN KEY(source_id) REFERENCES ${myuniversity}_${mymodule}.subject_source(id), + CONSTRAINT unique_instance_source UNIQUE (instance_id, source_id) +); diff --git a/src/main/resources/templates/db_scripts/createInstanceSubjectTypeTable.sql b/src/main/resources/templates/db_scripts/createInstanceSubjectTypeTable.sql new file mode 100644 index 000000000..86dfa7c9f --- /dev/null +++ b/src/main/resources/templates/db_scripts/createInstanceSubjectTypeTable.sql @@ -0,0 +1,8 @@ + +CREATE TABLE IF NOT EXISTS ${myuniversity}_${mymodule}.instance_subject_type ( + instance_id UUID, + type_id UUID, + CONSTRAINT fk_instance_id FOREIGN KEY(instance_id) REFERENCES ${myuniversity}_${mymodule}.instance(id) ON DELETE CASCADE, + CONSTRAINT fk_type_id FOREIGN KEY(type_id) REFERENCES ${myuniversity}_${mymodule}.subject_type(id), + CONSTRAINT unique_instance_type UNIQUE (instance_id, type_id) +); diff --git a/src/main/resources/templates/db_scripts/schema.json b/src/main/resources/templates/db_scripts/schema.json index 12b5a4ff6..f92d732c5 100644 --- a/src/main/resources/templates/db_scripts/schema.json +++ b/src/main/resources/templates/db_scripts/schema.json @@ -1237,6 +1237,16 @@ "run": "after", "snippetPath": "createIndexBarcodePattern.sql", "fromModuleVersion": "28.0.3" + }, + { + "run": "after", + "snippetPath": "createInstanceSubjectSourceTable.sql", + "fromModuleVersion": "28.1.0" + }, + { + "run": "after", + "snippetPath": "createInstanceSubjectTypeTable.sql", + "fromModuleVersion": "28.1.0" } ] } diff --git a/src/test/java/org/folio/rest/api/AsyncMigrationTest.java b/src/test/java/org/folio/rest/api/AsyncMigrationTest.java index f7e2c1ff2..3a7475b09 100644 --- a/src/test/java/org/folio/rest/api/AsyncMigrationTest.java +++ b/src/test/java/org/folio/rest/api/AsyncMigrationTest.java @@ -130,13 +130,14 @@ public void canMigrateItems() { @Test public void canMigrateInstanceSubjectsAndSeries() { - var numberOfRecords = 10; + var numberOfRecords = 3; IntStream.range(0, numberOfRecords).parallel().forEach(v -> instancesClient.create(new JsonObject() .put("title", "test" + v) .put("source", "MARC") - .put("instanceTypeId", "30fffe0e-e985-4144-b2e2-1e8179bdb41f"))); + .put("instanceTypeId", "535e3160-763a-42f9-b0c0-d8ed7df6e2a2")) + ); var countDownLatch = new CountDownLatch(1); @@ -166,13 +167,13 @@ public void canMigrateInstanceSubjectsAndSeries() { @Test public void canMigrateInstancePublicationPeriod() { - var numberOfRecords = 10; + var numberOfRecords = 4; IntStream.range(0, numberOfRecords).parallel().forEach(v -> instancesClient.create(new JsonObject() .put("title", "test" + v) .put("source", "MARC") - .put("instanceTypeId", "30fffe0e-e985-4144-b2e2-1e8179bdb41f"))); + .put("instanceTypeId", "535e3160-763a-42f9-b0c0-d8ed7df6e2a2"))); var countDownLatch = new CountDownLatch(1); var query = String.format(UPDATE_JSONB_WITH_PUB_PERIOD, TENANT_ID); @@ -181,7 +182,7 @@ public void canMigrateInstancePublicationPeriod() { // check jsonb contains 'publicationPeriod' data RowSet selectResult = runSql(String.format(SELECT_JSONB, TENANT_ID)); - assertEquals(10, selectResult.rowCount()); + assertEquals(4, selectResult.rowCount()); JsonObject jsonbData = selectResult.iterator().next().toJson().getJsonObject("jsonb"); assertNull(jsonbData.getJsonObject("dates")); assertNotNull(jsonbData.getJsonObject("publicationPeriod")); @@ -208,7 +209,7 @@ public void canMigrateInstancePublicationPeriod() { var selectQuery = String.format(SELECT_JSONB, TENANT_ID); RowSet result = runSql(selectQuery); - assertEquals(10, result.rowCount()); + assertEquals(4, result.rowCount()); JsonObject entry = result.iterator().next().toJson(); JsonObject jsonb = entry.getJsonObject("jsonb"); JsonObject dates = jsonb.getJsonObject("dates"); diff --git a/src/test/java/org/folio/rest/api/InstanceStorageTest.java b/src/test/java/org/folio/rest/api/InstanceStorageTest.java index 931d33366..d0a5b443e 100644 --- a/src/test/java/org/folio/rest/api/InstanceStorageTest.java +++ b/src/test/java/org/folio/rest/api/InstanceStorageTest.java @@ -3,6 +3,7 @@ import static java.lang.String.format; import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static java.net.HttpURLConnection.HTTP_NO_CONTENT; import static java.net.HttpURLConnection.HTTP_OK; import static java.util.concurrent.TimeUnit.SECONDS; import static org.awaitility.Awaitility.await; @@ -105,11 +106,11 @@ @RunWith(VertxUnitRunner.class) public class InstanceStorageTest extends TestBaseWithInventoryUtil { + public static final String SUBJECTS_KEY = "subjects"; private static final String INSTANCES_KEY = "instances"; private static final String TOTAL_RECORDS_KEY = "totalRecords"; private static final String METADATA_KEY = "metadata"; private static final String DATES_KEY = "dates"; - private static final String SUBJECTS_KEY = "subjects"; private static final String TAG_VALUE = "test-tag"; private static final String STATUS_UPDATED_DATE_PROPERTY = "statusUpdatedDate"; private static final Logger log = LogManager.getLogger(); @@ -406,6 +407,63 @@ public void cannotCreateAnInstanceWithNotExistingSubjectIds() assertThat(response.getStatusCode(), is(HTTP_BAD_REQUEST)); } + @Test + public void canUpdateAnInstanceUnlinkSubjectSourceAndType() { + + UUID id = UUID.randomUUID(); + var subject = new Subject() + .withSourceId(UUID_INSTANCE_SUBJECT_SOURCE_ID.toString()) + .withTypeId(UUID_INSTANCE_SUBJECT_TYPE_ID.toString()) + .withValue("subject"); + var subjects = new JsonArray().add(subject); + JsonObject instanceToCreate = smallAngryPlanet(id); + instanceToCreate.put(SUBJECTS_KEY, subjects); + + var newId = createInstanceRecord(instanceToCreate); + + assertThat(newId, is(notNullValue())); + + var getResponse = getById(newId); + + assertThat(getResponse.getStatusCode(), is(HTTP_OK)); + var instanceFromGet = getResponse.getJson(); + instanceFromGet.put(SUBJECTS_KEY, null); + + var updatedResponse = update(instanceFromGet); + assertThat(updatedResponse.getStatusCode(), is(HTTP_NO_CONTENT)); + } + + @Test + public void canUpdateAnInstanceLinkAndUnlinkSubjectSourceAndType() { + + UUID id = UUID.randomUUID(); + var subject = new Subject() + .withSourceId(UUID_INSTANCE_SUBJECT_SOURCE_ID.toString()) + .withTypeId(UUID_INSTANCE_SUBJECT_TYPE_ID.toString()) + .withValue("subject"); + var subjects = new JsonArray().add(subject); + JsonObject instanceToCreate = smallAngryPlanet(id); + instanceToCreate.put(SUBJECTS_KEY, subjects); + + var newId = createInstanceRecord(instanceToCreate); + + assertThat(newId, is(notNullValue())); + + var getResponse = getById(newId); + + assertThat(getResponse.getStatusCode(), is(HTTP_OK)); + var instanceFromGet = getResponse.getJson(); + var updatedSubject = new Subject() + .withSourceId("e894d0dc-621d-4b1d-98f6-6f7120eb0d41") + .withTypeId("d6488f88-1e74-40ce-81b5-b19a928ff5b2") + .withValue("subject upd"); + var updatedSubjects = new JsonArray().add(updatedSubject); + instanceFromGet.put(SUBJECTS_KEY, updatedSubjects); + + var updatedResponse = update(instanceFromGet); + assertThat(updatedResponse.getStatusCode(), is(HTTP_NO_CONTENT)); + } + @Test public void cannotPutAnInstanceAtNonexistingLocation() throws InterruptedException, ExecutionException, TimeoutException { diff --git a/src/test/java/org/folio/rest/api/SubjectSourceTest.java b/src/test/java/org/folio/rest/api/SubjectSourceTest.java index 045ddfbb8..3435df985 100644 --- a/src/test/java/org/folio/rest/api/SubjectSourceTest.java +++ b/src/test/java/org/folio/rest/api/SubjectSourceTest.java @@ -1,5 +1,8 @@ package org.folio.rest.api; +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.folio.rest.api.InstanceStorageTest.SUBJECTS_KEY; +import static org.folio.rest.support.ResponseHandler.json; import static org.folio.rest.support.http.InterfaceUrls.subjectSourcesUrl; import static org.folio.utility.ModuleUtility.getClient; import static org.folio.utility.ModuleUtility.prepareTenant; @@ -17,13 +20,12 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import junitparams.JUnitParamsRunner; import lombok.SneakyThrows; import org.folio.okapi.common.XOkapiHeaders; +import org.folio.rest.jaxrs.model.Subject; import org.folio.rest.support.Response; -import org.folio.rest.support.ResponseHandler; import org.folio.rest.support.http.ResourceClient; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -83,9 +85,9 @@ public void cannotCreateSubjectSourceWithDuplicateCode() CompletableFuture postCompleted = new CompletableFuture<>(); getClient().post(subjectSourcesUrl(""), subjectSource.put("name", "Test2"), - TENANT_ID, ResponseHandler.json(postCompleted)); + TENANT_ID, json(postCompleted)); - Response response = postCompleted.get(TIMEOUT, TimeUnit.SECONDS); + Response response = postCompleted.get(TIMEOUT, SECONDS); assertThat(response.getStatusCode(), is(422)); JsonArray errors = response.getJson().getJsonArray("errors"); @@ -238,6 +240,32 @@ public void canUpdateSubjectSourceToLocalAtEcs() { assertEquals(204, response.getStatusCode()); } + @Test + public void cannotDeleteSubjectSourceLinkedToInstance() { + var instanceId = UUID.randomUUID(); + + JsonObject subjectSource = new JsonObject() + .put("name", "Library Test " + UUID_INSTANCE_SUBJECT_SOURCE_ID) + .put("source", "local"); + + var subjectSourceId = createSubjectSource(subjectSource).getJson().getString("id"); + + var instance = instance(instanceId); + var subject = new Subject() + .withSourceId(subjectSourceId) + .withTypeId(UUID_INSTANCE_SUBJECT_TYPE_ID.toString()) + .withValue("subject"); + var subjects = new JsonArray().add(subject); + instance.put(SUBJECTS_KEY, subjects); + + createInstanceRecord(instance); + + Response response = deleteSubjectSource(UUID.fromString(subjectSourceId)); + + assertEquals(400, response.getStatusCode()); + assertTrue(response.getBody().contains("id is still referenced from table instance_subject_source")); + } + private Response createSubjectSource(JsonObject object) { return createSubjectSource(object, TENANT_ID); } @@ -253,4 +281,9 @@ private Response updateSubjectSource(String id, JsonObject object) { private Response updateSubjectSource(String id, JsonObject object, String tenantId) { return subjectSourceClient.attemptToReplace(id, object, tenantId, Map.of(XOkapiHeaders.URL, mockServer.baseUrl())); } + + private Response deleteSubjectSource(UUID id) { + return subjectSourceClient.attemptToDelete(id); + } + } diff --git a/src/test/java/org/folio/rest/api/SubjectTypeTest.java b/src/test/java/org/folio/rest/api/SubjectTypeTest.java index 0d09f0585..0d97607d3 100644 --- a/src/test/java/org/folio/rest/api/SubjectTypeTest.java +++ b/src/test/java/org/folio/rest/api/SubjectTypeTest.java @@ -1,5 +1,6 @@ package org.folio.rest.api; +import static org.folio.rest.api.InstanceStorageTest.SUBJECTS_KEY; import static org.folio.rest.support.http.InterfaceUrls.subjectTypesUrl; import static org.folio.utility.ModuleUtility.getClient; import static org.folio.utility.ModuleUtility.prepareTenant; @@ -8,6 +9,7 @@ import static org.folio.utility.RestUtility.TENANT_ID; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; import static org.junit.jupiter.api.Assertions.assertEquals; import io.vertx.core.json.JsonArray; @@ -21,6 +23,7 @@ import junitparams.JUnitParamsRunner; import lombok.SneakyThrows; import org.folio.okapi.common.XOkapiHeaders; +import org.folio.rest.jaxrs.model.Subject; import org.folio.rest.support.Response; import org.folio.rest.support.ResponseHandler; import org.folio.rest.support.http.ResourceClient; @@ -217,6 +220,56 @@ public void canUpdateSubjectTypeToLocalAtEcs() { assertEquals(204, response.getStatusCode()); } + @Test + public void cannotDeleteSubjectTypeLinkedToInstance() { + var instanceId = UUID.randomUUID(); + + JsonObject subjectType = new JsonObject() + .put("name", "Library Test " + UUID_INSTANCE_SUBJECT_TYPE_ID) + .put("source", "local"); + + var subjectTypeId = createSubjectType(subjectType).getJson().getString("id"); + + var instance = instance(instanceId); + var subject = new Subject() + .withSourceId(UUID_INSTANCE_SUBJECT_SOURCE_ID.toString()) + .withTypeId(subjectTypeId) + .withValue("subject"); + var subjects = new JsonArray().add(subject); + instance.put(SUBJECTS_KEY, subjects); + + createInstanceRecord(instance); + + Response response = deleteSubjectType(UUID.fromString(subjectTypeId)); + + assertEquals(400, response.getStatusCode()); + assertTrue(response.getBody().contains("id is still referenced from table instance_subject_type")); + } + + @Test + public void clearLinksBetweenSubjectTypeAndInstance() { + var instanceId = UUID.randomUUID(); + + JsonObject subjectType = new JsonObject() + .put("name", "Library Test2 " + UUID_INSTANCE_SUBJECT_TYPE_ID) + .put("source", "local"); + + var subjectTypeId = createSubjectType(subjectType).getJson().getString("id"); + + var instance = instance(instanceId); + var subject = new Subject() + .withSourceId(UUID_INSTANCE_SUBJECT_SOURCE_ID.toString()) + .withTypeId(subjectTypeId) + .withValue("subject"); + var subjects = new JsonArray().add(subject); + instance.put(SUBJECTS_KEY, subjects); + + createInstanceRecord(instance); + + var response = deleteInstanceRecord(instanceId); + assertEquals(204, response.getStatusCode()); + } + private Response createSubjectType(JsonObject object) { return createSubjectType(object, TENANT_ID); } @@ -232,4 +285,8 @@ private Response updateSubjectType(String id, JsonObject object) { private Response updateSubjectType(String id, JsonObject object, String tenantId) { return subjectTypeClient.attemptToReplace(id, object, tenantId, Map.of(XOkapiHeaders.URL, mockServer.baseUrl())); } + + private Response deleteSubjectType(UUID id) { + return subjectTypeClient.attemptToDelete(id); + } } diff --git a/src/test/java/org/folio/rest/api/TestBaseWithInventoryUtil.java b/src/test/java/org/folio/rest/api/TestBaseWithInventoryUtil.java index 74a03eecf..70aed5604 100644 --- a/src/test/java/org/folio/rest/api/TestBaseWithInventoryUtil.java +++ b/src/test/java/org/folio/rest/api/TestBaseWithInventoryUtil.java @@ -268,6 +268,14 @@ protected static UUID createInstanceAndHoldingWithCallNumberSuffix(UUID holdings TENANT_ID, Map.of(XOkapiHeaders.URL, mockServer.baseUrl())).getId(); } + public static UUID createInstanceRecord(JsonObject instanceJson) { + return instancesClient.create(instanceJson).getId(); + } + + public Response deleteInstanceRecord(UUID id) { + return instancesClient.attemptToDelete(id); + } + protected static JsonObject instance(UUID id) { return createInstanceRequest( id,