Skip to content

Commit

Permalink
Merge pull request #1477 from folio-org/CIRC-2104
Browse files Browse the repository at this point in the history
[CIRC-2104] Updating error response status and schema for request creation
  • Loading branch information
roman-barannyk authored Jun 21, 2024
2 parents ef4e221 + 7f33ac2 commit 7aa8e28
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 18 deletions.
2 changes: 1 addition & 1 deletion descriptors/ModuleDescriptor-template.json
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@
},
{
"id": "circulation",
"version": "14.2",
"version": "14.3",
"handlers": [
{
"methods": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static org.folio.circulation.support.ErrorCode.ITEM_ALREADY_REQUESTED;
import static org.folio.circulation.support.ErrorCode.ITEM_OF_THIS_INSTANCE_ALREADY_REQUESTED;
import static org.folio.circulation.support.ErrorCode.MOVING_REQUEST_TO_THE_SAME_ITEM;
import static org.folio.circulation.support.ErrorCode.REQUEST_NOT_ALLOWED_FOR_PATRON_ITEM_COMBINATION;
import static org.folio.circulation.support.ValidationErrorFailure.failedValidation;
import static org.folio.circulation.support.results.Result.of;
import static org.folio.circulation.support.results.Result.succeeded;
Expand Down Expand Up @@ -116,7 +117,7 @@ private static Result<RequestAndRelatedRecords> failureDisallowedForRequestType(

return failedValidation(
format("%s requests are not allowed for this patron and item combination", requestTypeName),
REQUEST_TYPE, requestTypeName);
REQUEST_TYPE, requestTypeName, REQUEST_NOT_ALLOWED_FOR_PATRON_ITEM_COMBINATION);
}

static Result<RequestAndRelatedRecords> refuseWhenInvalidUserAndPatronGroup(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.folio.circulation.domain.validation;

import static java.util.concurrent.CompletableFuture.completedFuture;
import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_AUTOMATICALLY;
import static org.folio.circulation.support.results.Result.ofAsync;
import static org.folio.circulation.support.results.Result.succeeded;
import static org.folio.circulation.support.utils.LogUtil.listAsString;
Expand All @@ -18,14 +19,14 @@
import org.apache.logging.log4j.Logger;
import org.folio.circulation.domain.AutomatedPatronBlock;
import org.folio.circulation.domain.AutomatedPatronBlocks;
import org.folio.circulation.infrastructure.storage.AutomatedPatronBlocksRepository;
import org.folio.circulation.domain.LoanAndRelatedRecords;
import org.folio.circulation.domain.RequestAndRelatedRecords;
import org.folio.circulation.infrastructure.storage.AutomatedPatronBlocksRepository;
import org.folio.circulation.resources.context.RenewalContext;
import org.folio.circulation.support.Clients;
import org.folio.circulation.support.ValidationErrorFailure;
import org.folio.circulation.support.http.server.ValidationError;
import org.folio.circulation.support.results.Result;
import org.folio.circulation.support.ValidationErrorFailure;

public class AutomatedPatronBlocksValidator {
private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass());
Expand All @@ -43,7 +44,7 @@ public AutomatedPatronBlocksValidator(

public AutomatedPatronBlocksValidator(AutomatedPatronBlocksRepository automatedPatronBlocksRepository) {
this(automatedPatronBlocksRepository, messages -> new ValidationErrorFailure(messages.stream()
.map(message -> new ValidationError(message, new HashMap<>()))
.map(message -> new ValidationError(message, new HashMap<>(), USER_IS_BLOCKED_AUTOMATICALLY))
.collect(Collectors.toList())));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.folio.circulation.domain.validation;

import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_MANUALLY;
import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError;
import static org.folio.circulation.support.fetching.RecordFetching.findWithCqlQuery;
import static org.folio.circulation.support.http.client.CqlQuery.exactMatch;
Expand Down Expand Up @@ -104,7 +105,7 @@ private HttpFailure createUserBlockedValidationError(
final String reason = userManualBlocks.getRecords().stream()
.map(UserManualBlock::getDesc).collect(Collectors.joining(";"));

return singleValidationError(new ValidationError(message, "reason", reason));
return singleValidationError(new ValidationError(message, "reason", reason, USER_IS_BLOCKED_MANUALLY));
}

private boolean isUserBlockedManually(MultipleRecords<UserManualBlock> userManualBlockMultipleRecords,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.folio.circulation.resources;

import static java.util.concurrent.CompletableFuture.completedFuture;
import static java.util.stream.Collectors.toList;
import static org.folio.circulation.domain.InstanceRequestItemsComparer.sortRequestQueues;
import static org.folio.circulation.domain.RequestFulfillmentPreference.HOLD_SHELF;
Expand Down Expand Up @@ -29,11 +30,13 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -159,7 +162,7 @@ private CompletableFuture<Result<Map<Item, ZonedDateTime>>> getLoanItems(
if (unsortedUnavailableItems == null || unsortedUnavailableItems.isEmpty()) {
log.info("getLoanItems:: unsortedUnavailableItems is null or empty");

return CompletableFuture.completedFuture(succeeded(null));
return completedFuture(succeeded(null));
}

Map<Item, CompletableFuture<Result<Loan>>> itemLoanFuturesMap = new HashMap<>();
Expand Down Expand Up @@ -301,23 +304,21 @@ updateUponRequest, new RequestLoanValidator(itemFinder, loanRepository),
new FailFastErrorHandler());

return placeRequest(requestRepresentations, 0, createRequestService,
clients, new ArrayList<>(), repositories);
clients, new HashSet<>(), repositories);
}

private CompletableFuture<Result<RequestAndRelatedRecords>> placeRequest(
List<JsonObject> itemRequests, int startIndex, CreateRequestService createRequestService,
Clients clients, List<String> errors, RequestRelatedRepositories repositories) {
Clients clients, Set<ValidationError> errors, RequestRelatedRepositories repositories) {

log.debug("RequestByInstanceIdResource.placeRequest, startIndex: {}, itemRequestSize: {}",
startIndex, itemRequests.size());

if (startIndex >= itemRequests.size()) {
String aggregateFailures = String.format("%n%s", String.join("%n", errors));
log.warn("placeRequest:: Failed to place a request for the instance. Reasons: {}",
aggregateFailures);
errors);

return CompletableFuture.completedFuture(failedDueToServerError(
"Failed to place a request for the instance. Reasons: " + aggregateFailures));
return completedFuture(failedValidation(errors));
}

JsonObject currentItemRequest = itemRequests.get(startIndex);
Expand All @@ -336,12 +337,14 @@ private CompletableFuture<Result<RequestAndRelatedRecords>> placeRequest(
if (r.succeeded()) {
log.debug("RequestByInstanceIdResource.placeRequest: succeeded creating request for item {}",
currentItemRequest.getString(ITEM_ID));
return CompletableFuture.completedFuture(r);
return completedFuture(r);
} else {
String reason = getErrorMessage(r.cause());
errors.add(reason);
HttpFailure failure = r.cause();
errors.addAll(convertToValidationErrors(failure));

log.debug("Failed to create request for item {} with cause: {}",
currentItemRequest.getString(ITEM_ID), failure);

log.debug("Failed to create request for item {} with reason: {}", currentItemRequest.getString(ITEM_ID), reason);
return placeRequest(itemRequests, startIndex +1,
createRequestService, clients, errors, repositories);
}
Expand Down Expand Up @@ -527,4 +530,19 @@ static String getErrorMessage(HttpFailure failure) {
}
return reason;
}

static Collection<ValidationError> convertToValidationErrors(HttpFailure failure) {
Set<ValidationError> validationErrors = new HashSet<>();
if (failure instanceof ServerErrorFailure serverErrorFailure) {
validationErrors.add(new ValidationError(serverErrorFailure.getReason()));
} else if (failure instanceof ValidationErrorFailure validationErrorFailure) {
validationErrors.addAll(validationErrorFailure.getErrors());
} else if (failure instanceof BadRequestFailure badRequestFailure) {
validationErrors.add(new ValidationError(badRequestFailure.getReason()));
} else if (failure instanceof ForwardOnFailure forwardOnFailure) {
validationErrors.add(new ValidationError(forwardOnFailure.getFailureResponse().getBody()));
}

return validationErrors;
}
}
3 changes: 3 additions & 0 deletions src/main/java/org/folio/circulation/support/ErrorCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ public enum ErrorCode {
ITEM_HAS_OPEN_LOAN,
REQUEST_ALREADY_CLOSED,
REQUEST_NOT_ALLOWED_FOR_PATRON_TITLE_COMBINATION,
REQUEST_NOT_ALLOWED_FOR_PATRON_ITEM_COMBINATION,
REQUEST_PICKUP_SERVICE_POINT_IS_NOT_ALLOWED,
SERVICE_POINT_IS_NOT_PICKUP_LOCATION,
REQUEST_LEVEL_IS_NOT_ALLOWED,
FULFILLMENT_PREFERENCE_IS_NOT_ALLOWED,
REQUESTER_ALREADY_HAS_LOAN_FOR_ONE_OF_INSTANCES_ITEMS,
USER_IS_BLOCKED_MANUALLY,
USER_IS_BLOCKED_AUTOMATICALLY,
REQUESTER_ALREADY_HAS_THIS_ITEM_ON_LOAN,
HOLD_AND_RECALL_TLR_NOT_ALLOWED_PAGEABLE_AVAILABLE_ITEM_FOUND,
USER_CANNOT_BE_PROXY_FOR_THEMSELVES,
Expand Down
33 changes: 33 additions & 0 deletions src/test/java/api/requests/InstanceRequestsAPICreationTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,39 @@ void canCreateTlrWhenAvailableItemExists(String allowedRequestTypesForCheckedOut
is(RequestStatus.OPEN_NOT_YET_FILLED.getValue()));
}

@Test
void shouldReturn422StatusIfNoAvailableItemsAndRequesterAlreadyHasOpenRequestForThisInstance() {
UUID pickupServicePointId = servicePointsFixture.cd1().getId();

ZonedDateTime instanceRequestDate = ZonedDateTime.of(2024, 6, 7, 10, 22, 54, 0, UTC);
ZonedDateTime instanceRequestDateRequestExpirationDate = instanceRequestDate.plusDays(30);
IndividualResource instance = instancesFixture.basedUponDunkirk();
IndividualResource holdings = holdingsFixture.defaultWithHoldings(instance.getId());
IndividualResource locationsResource = locationsFixture.mainFloor();

itemsFixture.basedUponDunkirkWithCustomHoldingAndLocation(holdings.getId(),
locationsResource.getId());
reconfigureTlrFeature(TlrFeatureStatus.ENABLED, true, null, null, null);

IndividualResource instanceRequester = usersFixture.charlotte();
JsonObject requestBody = createInstanceRequestObject(instance.getId(),
instanceRequester.getId(), pickupServicePointId, instanceRequestDate,
instanceRequestDateRequestExpirationDate);

Response postResponse = requestsFixture.attemptToPlaceForInstance(requestBody);
assertEquals(201, postResponse.getStatusCode());

Response secondPostResponse = requestsFixture.attemptToPlaceForInstance(requestBody);
assertEquals(422, secondPostResponse.getStatusCode());
assertThat(secondPostResponse.getJson(), hasErrorWith(allOf(
hasMessage("Cannot create page TLR for this instance ID - no pageable available items found"),
hasParameter("instanceId", instance.getId().toString()))));
assertThat(secondPostResponse.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for this instance"),
hasParameter("requesterId", instanceRequester.getId().toString()),
hasParameter("instanceId", instance.getId().toString()))));
}

private void validateInstanceRequestResponse(JsonObject representation,
UUID pickupServicePointId, UUID instanceId, UUID itemId,
RequestType expectedRequestType) {
Expand Down
7 changes: 6 additions & 1 deletion src/test/java/api/requests/RequestsAPICreationTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
import static org.folio.circulation.support.ErrorCode.REQUESTER_ALREADY_HAS_LOAN_FOR_ONE_OF_INSTANCES_ITEMS;
import static org.folio.circulation.support.ErrorCode.REQUESTER_ALREADY_HAS_THIS_ITEM_ON_LOAN;
import static org.folio.circulation.support.ErrorCode.REQUEST_LEVEL_IS_NOT_ALLOWED;
import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_AUTOMATICALLY;
import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_MANUALLY;
import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime;
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.containsString;
Expand Down Expand Up @@ -2694,7 +2696,8 @@ void cannotCreateRequestWhenRequesterHasActiveRequestManualBlocks() {
assertThat(postResponse.getJson(), hasErrors(1));
assertThat(postResponse.getJson(), hasErrorWith(allOf(
hasMessage("Patron blocked from requesting"),
hasParameter("reason", "Display description")))
hasParameter("reason", "Display description"),
hasCode(USER_IS_BLOCKED_MANUALLY)))
);
}

Expand Down Expand Up @@ -2891,6 +2894,8 @@ void requestRefusedWhenAutomatedBlockExistsForPatron() {
hasMessage(MAX_NUMBER_OF_ITEMS_CHARGED_OUT_MESSAGE)));
assertThat(response.getJson(), hasErrorWith(
hasMessage(MAX_OUTSTANDING_FEE_FINE_BALANCE_MESSAGE)));
assertThat(response.getJson(), hasErrorWith(
hasCode(USER_IS_BLOCKED_AUTOMATICALLY)));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,34 @@ void getExpectedErrorMessages() {
assertEquals("fakeResponseFailure", errorMessage);
}

@Test
void shouldConvertToValidationErrorForServerErrorFailure() {
ServerErrorFailure serverErrorFailure = new ServerErrorFailure("Server failed");
Collection<ValidationError> errors = RequestByInstanceIdResource.convertToValidationErrors(serverErrorFailure);

assertEquals(1, errors.size());
assertTrue(errors.contains(new ValidationError("Server failed")));
}

@Test
void shouldConvertToValidationErrorForBadRequestFailure() {
BadRequestFailure badRequestFailure = new BadRequestFailure("Bad request");
Collection<ValidationError> errors = RequestByInstanceIdResource.convertToValidationErrors(badRequestFailure);

assertEquals(1, errors.size());
assertTrue(errors.contains(new ValidationError("Bad request")));
}

@Test
void shouldConvertToValidationErrorForForwardOnFailure() {
Response failureResponse = new Response(422, "test body", "json");
ForwardOnFailure forwardFailure = new ForwardOnFailure(failureResponse);
Collection<ValidationError> errors = RequestByInstanceIdResource.convertToValidationErrors(forwardFailure);

assertEquals(1, errors.size());
assertEquals("test body", errors.stream().toList().get(0).getMessage());
}

public static JsonObject getJsonInstanceRequest(UUID pickupServicePointId) {
ZonedDateTime requestDate = ZonedDateTime.of(2017, 7, 22, 10, 22, 54, 0, UTC);
ZonedDateTime requestExpirationDate = requestDate.plusDays(30);
Expand Down

0 comments on commit 7aa8e28

Please sign in to comment.