diff --git a/src/main/java/org/folio/service/BatchResourceService.java b/src/main/java/org/folio/service/BatchResourceService.java index 34774afe..53f6e90b 100644 --- a/src/main/java/org/folio/service/BatchResourceService.java +++ b/src/main/java/org/folio/service/BatchResourceService.java @@ -65,8 +65,9 @@ public Future executeBatchUpdate( if (updateResult.failed()) { log.warn("Batch update rejected", updateResult.cause()); + // Rollback transaction and keep original cause. postgresClient.rollbackTx(connectionResult, rollback -> { - onFinishHandler.handle(Future.failedFuture(updateResult.cause())); + onFinishHandler.handle(failedFuture(updateResult.cause())); promise.fail(updateResult.cause()); }); } else { diff --git a/src/test/java/org/folio/rest/api/RequestBatchAPITest.java b/src/test/java/org/folio/rest/api/RequestBatchAPITest.java index 51ee6366..2ff06acf 100644 --- a/src/test/java/org/folio/rest/api/RequestBatchAPITest.java +++ b/src/test/java/org/folio/rest/api/RequestBatchAPITest.java @@ -1,13 +1,16 @@ package org.folio.rest.api; +import static org.awaitility.Awaitility.await; import static org.folio.rest.api.RequestsApiTest.requestStorageUrl; import static org.folio.rest.api.StorageTestSuite.TENANT_ID; import static org.folio.rest.api.StorageTestSuite.storageUrl; +import static org.folio.rest.support.kafka.FakeKafkaConsumer.getRequestQueueReorderingEvents; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasItems; -import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import java.net.URL; import java.util.Arrays; @@ -16,6 +19,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.function.Function; + import org.folio.rest.impl.RequestsBatchAPI; import org.folio.rest.jaxrs.model.Request; import org.folio.rest.support.ApiTests; @@ -52,8 +56,8 @@ public void checkIdsAfterEach() { public void canUpdateRequestPositionsInBatch() throws Exception { UUID itemId = UUID.randomUUID(); - JsonObject firstRequest = createRequestForItemAtPosition(itemId, 1); - JsonObject secondRequest = createRequestForItemAtPosition(itemId, 2); + JsonObject firstRequest = createRequestAtPosition(itemId, null, 1); + JsonObject secondRequest = createRequestAtPosition(itemId, null, 2); ReorderRequest firstReorderRequest = new ReorderRequest(firstRequest, 2); ReorderRequest secondReorderRequest = new ReorderRequest(secondRequest, 1); @@ -81,8 +85,8 @@ public void canUpdateRequestPositionsInBatch() throws Exception { public void canCloseRequestsInBatch() throws Exception { UUID itemId = UUID.randomUUID(); - JsonObject firstRequest = createRequestForItemAtPosition(itemId, 1); - JsonObject secondRequest = createRequestForItemAtPosition(itemId, 2); + JsonObject firstRequest = createRequestAtPosition(itemId, null, 1); + JsonObject secondRequest = createRequestAtPosition(itemId, null, 2); firstRequest.put("status", Request.Status.CLOSED_FILLED.value()); firstRequest.remove("position"); @@ -111,8 +115,8 @@ public void canCloseRequestsInBatch() throws Exception { public void canUpdateRequestFulfillmentPreferenceInBatch() throws Exception { UUID itemId = UUID.randomUUID(); - JsonObject firstRequest = createRequestForItemAtPosition(itemId, 1); - JsonObject secondRequest = createRequestForItemAtPosition(itemId, 2); + JsonObject firstRequest = createRequestAtPosition(itemId, null, 1); + JsonObject secondRequest = createRequestAtPosition(itemId, null, 2); firstRequest.put("fulfillmentPreference", "Delivery"); secondRequest.put("fulfillmentPreference", "Delivery"); @@ -153,8 +157,8 @@ public void willAbortBatchUpdateOnPopulateMetadataException() throws Exception { public void willAbortBatchUpdateForRequestsAtTheSamePositionInAnItemsQueue() throws Exception { UUID itemId = UUID.randomUUID(); - JsonObject firstRequest = createRequestForItemAtPosition(itemId, 1); - JsonObject secondRequest = createRequestForItemAtPosition(itemId, 2); + JsonObject firstRequest = createRequestAtPosition(itemId, null, 1); + JsonObject secondRequest = createRequestAtPosition(itemId, null, 2); JsonResponse reorderResponse = attemptReorderRequests(ResponseHandler::json, new ReorderRequest(firstRequest, 1), @@ -170,8 +174,8 @@ public void willAbortBatchUpdateForRequestsAtTheSamePositionInAnItemsQueue() thr public void willAbortBatchUpdateWhenOnlyOnePositionIsModified() throws Exception { UUID itemId = UUID.randomUUID(); - JsonObject firstRequest = createRequestForItemAtPosition(itemId, 1); - JsonObject secondRequest = createRequestForItemAtPosition(itemId, 2); + JsonObject firstRequest = createRequestAtPosition(itemId, null, 1); + JsonObject secondRequest = createRequestAtPosition(itemId, null, 2); JsonResponse reorderResponse = attemptReorderRequests(ResponseHandler::json, new ReorderRequest(firstRequest, 2) @@ -186,8 +190,8 @@ public void willAbortBatchUpdateWhenOnlyOnePositionIsModified() throws Exception public void willAbortBatchUpdateOnNullPointerExceptionDueToNoIdInRequest() throws Exception { UUID itemId = UUID.randomUUID(); - JsonObject firstRequest = createRequestForItemAtPosition(itemId, 1); - JsonObject secondRequest = createRequestForItemAtPosition(itemId, 2); + JsonObject firstRequest = createRequestAtPosition(itemId, null, 1); + JsonObject secondRequest = createRequestAtPosition(itemId, null, 2); JsonObject firstRequestCopy = firstRequest.copy(); firstRequestCopy.remove("id"); @@ -205,7 +209,7 @@ public void willAbortBatchUpdateOnNullPointerExceptionDueToNoIdInRequest() throw public void cannotInjectSqlThroughRequestId() throws Exception { UUID itemId = UUID.randomUUID(); - JsonObject firstRequest = createRequestForItemAtPosition(itemId, 1); + JsonObject firstRequest = createRequestAtPosition(itemId, null, 1); JsonObject firstRequestCopy = firstRequest.copy(); firstRequestCopy.put("id", "1'; DELETE FROM request where id::text is not '1"); @@ -219,6 +223,37 @@ public void cannotInjectSqlThroughRequestId() throws Exception { assertRequestsNotUpdated(itemId, firstRequest); } + @Test + public void canUpdateRequestPositionsInBatchForTheInstance() throws Exception { + UUID instanceId = UUID.randomUUID(); + + JsonObject firstRequest = createRequestAtPosition(null, instanceId, 1); + JsonObject secondRequest = createRequestAtPosition(null, instanceId, 2); + + ReorderRequest firstReorderRequest = new ReorderRequest(firstRequest, 2); + ReorderRequest secondReorderRequest = new ReorderRequest(secondRequest, 1); + + reorderRequests( + firstReorderRequest, + secondReorderRequest + ); + + JsonObject requestsForInstanceReply = getAllRequestsForInstance(instanceId); + assertThat(requestsForInstanceReply.getInteger("totalRecords"), is(2)); + JsonArray requestsFromDb = requestsForInstanceReply.getJsonArray("requests"); + assertThat(requestsFromDb.size(), is(2)); + JsonObject [] r = new JsonObject [] { requestsFromDb.getJsonObject(0), requestsFromDb.getJsonObject(1) }; + if (r[0].getInteger("position") == 2) { + r = new JsonObject [] { r[1], r[0] }; + } + assertThat(r[0].getInteger("position"), is(1)); + assertThat(r[1].getInteger("position"), is(2)); + assertThat(r[0].getString("id"), is(secondRequest.getString("id"))); + assertThat(r[1].getString("id"), is(firstRequest.getString("id"))); + + assertCreateRequestQueueReorderingEvent(); + } + private JsonObject getAllRequestsForItem(UUID itemId) throws Exception { CompletableFuture getRequestsCompleted = new CompletableFuture<>(); @@ -228,10 +263,30 @@ private JsonObject getAllRequestsForItem(UUID itemId) throws Exception { return getRequestsCompleted.get(5, TimeUnit.SECONDS).getJson(); } - private JsonObject createRequestForItemAtPosition(UUID itemId, int position) throws Exception { - JsonObject request = createEntity( - new RequestRequestBuilder() + private JsonObject getAllRequestsForInstance(UUID instanceId) throws Exception { + CompletableFuture getRequestsCompleted = new CompletableFuture<>(); + + client.get(requestStorageUrl() + String.format("?query=instanceId==%s", instanceId), + TENANT_ID, ResponseHandler.json(getRequestsCompleted)); + + return getRequestsCompleted.get(5, TimeUnit.SECONDS).getJson(); + } + + private JsonObject createRequestAtPosition(UUID itemId, UUID instanceId, int position) + throws Exception { + + RequestRequestBuilder requestBuilder = null; + if (instanceId != null) { + requestBuilder = new RequestRequestBuilder() + .withInstanceId(instanceId) + .withRequestLevel("Title"); + } else { + requestBuilder = new RequestRequestBuilder() .withItemId(itemId) + .withRequestLevel("Item"); + } + JsonObject request = createEntity( + requestBuilder .withPosition(position) .recall() .toHoldShelf() @@ -240,7 +295,11 @@ private JsonObject createRequestForItemAtPosition(UUID itemId, int position) thr requestStorageUrl() ).getJson(); - assertThat(request.getString("itemId"), is(itemId.toString())); + if (itemId != null) { + assertThat(request.getString("itemId"), is(itemId.toString())); + } else { + assertThat(request.getString("instanceId"), is(instanceId.toString())); + } assertThat(request.getInteger("position"), is(position)); return request; @@ -327,6 +386,10 @@ private void assertValidationError( errorMessageMatcher); } + public static void assertCreateRequestQueueReorderingEvent() { + await().until(() -> getRequestQueueReorderingEvents().size(), greaterThan(0)); + } + /** * Holder for request reorder operation. Holds request and new position for it. */ diff --git a/src/test/java/org/folio/rest/support/kafka/FakeKafkaConsumer.java b/src/test/java/org/folio/rest/support/kafka/FakeKafkaConsumer.java index da59490d..8a136eb7 100644 --- a/src/test/java/org/folio/rest/support/kafka/FakeKafkaConsumer.java +++ b/src/test/java/org/folio/rest/support/kafka/FakeKafkaConsumer.java @@ -27,6 +27,7 @@ public final class FakeKafkaConsumer { private static final String REQUEST_TOPIC_NAME = "folio.test_tenant.circulation.request"; private static final String CHECKIN_TOPIC_NAME = "folio.test_tenant.circulation.check-in"; private static final String CIRCULATION_RULES_TOPIC_NAME = "folio.test_tenant.circulation.rules"; + private static final String REQUEST_QUEUE_REORDERING_TOPIC_NAME = "folio.test_tenant.circulation.request-queue-reordering"; private static final Map>> loanEvents = new ConcurrentHashMap<>(); @@ -36,19 +37,21 @@ public final class FakeKafkaConsumer { new ConcurrentHashMap<>(); private static final Map>> circulationRulesEvents = new ConcurrentHashMap<>(); - + private static final Map>> requestQueueReorderingEvents = + new ConcurrentHashMap<>(); private static final Map>>> topicToEvents = Map.of( LOAN_TOPIC_NAME, loanEvents, REQUEST_TOPIC_NAME, requestEvents, CHECKIN_TOPIC_NAME, checkInEvents, - CIRCULATION_RULES_TOPIC_NAME, circulationRulesEvents + CIRCULATION_RULES_TOPIC_NAME, circulationRulesEvents, + REQUEST_QUEUE_REORDERING_TOPIC_NAME, requestQueueReorderingEvents ); public FakeKafkaConsumer consume(Vertx vertx) { final KafkaConsumer consumer = create(vertx, consumerProperties()); consumer.subscribe(Set.of(LOAN_TOPIC_NAME, REQUEST_TOPIC_NAME, CHECKIN_TOPIC_NAME, - CIRCULATION_RULES_TOPIC_NAME)); + CIRCULATION_RULES_TOPIC_NAME, REQUEST_QUEUE_REORDERING_TOPIC_NAME)); consumer.handler(message -> { var recordEvents = topicToEvents.get(message.topic()); @@ -69,6 +72,7 @@ public static void removeAllEvents() { requestEvents.clear(); checkInEvents.clear(); circulationRulesEvents.clear(); + requestQueueReorderingEvents.clear(); } public static int getAllPublishedLoanCount() { @@ -94,6 +98,13 @@ public static Collection> getCirculation .orElseGet(Collections::emptyList); } + public static Collection> getRequestQueueReorderingEvents() { + return requestQueueReorderingEvents.values() + .stream() + .findFirst() + .orElseGet(Collections::emptyList); + } + public static KafkaConsumerRecord getFirstLoanEvent(String loanId) { return getFirstEvent(getLoanEvents(loanId)); }