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

[CIRC-2104] Updating error response status and schema for request creation #1477

Merged
merged 9 commits into from
Jun 21, 2024
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