Skip to content

Commit

Permalink
Merge branch 'master' into CIRC-1933
Browse files Browse the repository at this point in the history
  • Loading branch information
OleksandrVidinieiev authored Dec 4, 2023
2 parents e83c30b + 4e1ec3f commit 07f5e32
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 55 deletions.
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,23 @@ system property variables in this order, and uses the default 9801 as fallback.

The Docker container exposes port 9801.

## Environment variables

Module integrates with Kafka in order to consume/publish domain events. This integration can
be configured using the following environment variables:

| Variable name | Default value |
|--------------------|-------------------|
| KAFKA_HOST | localhost |
| KAFKA_PORT | 9092 |
| REPLICATION FACTOR | 1 |
| MAX_REQUEST_SIZE | 4000000 |
| ENV | folio |
| OKAPI_URL | http://okapi:9130 |

If a variable is not present, its default values is used as a fallback. If this configuration is
invalid, the module will start, but Kafka integration will not work.

## Design Notes

### Known Limitations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ private Future<KafkaConsumerWrapper<String, String>> createConsumer(DomainEventT
.processRecordErrorHandler((t, r) -> log.error("Failed to process event: {}", r, t))
.build();


return moduleIdProvider.getModuleId()
.onSuccess(moduleId -> log.info("createConsumer:: moduleId={}", moduleId))
.compose(moduleId -> consumer.start(handler, moduleId))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import static java.lang.String.format;
import static org.folio.circulation.domain.representations.RequestProperties.PICKUP_SERVICE_POINT_ID;
import static org.folio.circulation.domain.representations.RequestProperties.REQUEST_TYPE;
import static org.folio.circulation.support.ErrorCode.INSTANCE_ALREADY_REQUESTED;
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.ValidationErrorFailure.failedValidation;
import static org.folio.circulation.support.results.Result.of;
import static org.folio.circulation.support.results.Result.succeeded;
Expand Down Expand Up @@ -254,31 +257,38 @@ private static Result<RequestAndRelatedRecords> alreadyRequestedFailure(
Request requestBeingPlaced = requestAndRelatedRecords.getRequest();
HashMap<String, String> parameters = new HashMap<>();
String message;
ErrorCode errorCode;

if (requestBeingPlaced.isTitleLevel()) {
if (existingRequest.isTitleLevel()) {
parameters.put(REQUESTER_ID, requestBeingPlaced.getUserId());
parameters.put(INSTANCE_ID, requestBeingPlaced.getInstanceId());

message = requestBeingPlaced.getOperation() == Operation.MOVE
? "Not allowed to move title level page request to the same item"
: "This requester already has an open request for this instance";
if (requestBeingPlaced.getOperation() == Operation.MOVE) {
message = "Not allowed to move title level page request to the same item";
errorCode = MOVING_REQUEST_TO_THE_SAME_ITEM;
} else {
message = "This requester already has an open request for this instance";
errorCode = INSTANCE_ALREADY_REQUESTED;
}
} else {
parameters.put(REQUESTER_ID, requestBeingPlaced.getUserId());
parameters.put(INSTANCE_ID, requestBeingPlaced.getInstanceId());

message = "This requester already has an open request for one of the instance's items";
errorCode = ITEM_OF_THIS_INSTANCE_ALREADY_REQUESTED;
}
} else {
parameters.put(REQUESTER_ID, requestBeingPlaced.getUserId());
parameters.put(ITEM_ID, requestBeingPlaced.getItemId());
parameters.put(REQUEST_ID, requestBeingPlaced.getId());

message = "This requester already has an open request for this item";
errorCode = ITEM_ALREADY_REQUESTED;
}
log.info("alreadyRequestedFailure:: message: {}", message);
log.info("alreadyRequestedFailure:: message: {}, errorCode: {}", message, errorCode);

return failedValidation(message, parameters, ITEM_ALREADY_REQUESTED);
return failedValidation(message, parameters, errorCode);
}

static boolean isTheSameRequester(RequestAndRelatedRecords it, Request that) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ private CompletableFuture<Result<MultipleRecords<Item>>> fetchItems(Collection<S

return finder.findByIds(dcbItemIds)
.thenApply(mapResult(identityMap::add))
.thenApply(mapResult(records -> records.mapRecords(mapper::toDomain)));
.thenApply(r -> r.map(records -> records.mapRecords(mapper::toDomain)))
.thenApply(r -> r.mapFailure(failure -> succeeded(MultipleRecords.empty())));
}

private CompletableFuture<Result<Item>> fetchItem(String itemId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.folio.circulation.rules.CirculationRulesException;
import org.folio.circulation.rules.CirculationRulesParser;
import org.folio.circulation.rules.Text2Drools;
import org.folio.circulation.rules.cache.CirculationRulesCache;
import org.folio.circulation.support.Clients;
import org.folio.circulation.support.CollectionResourceClient;
import org.folio.circulation.support.ForwardOnFailure;
Expand Down Expand Up @@ -101,9 +100,6 @@ private void get(RoutingContext routingContext) {
return;
}
JsonObject circulationRules = new JsonObject(response.getBody());
CirculationRulesCache.getInstance()
.buildRules(context.getTenantId(), circulationRules.getString("rulesAsText"));

context.write(ok(circulationRules));
}
catch (Exception e) {
Expand Down Expand Up @@ -158,8 +154,6 @@ private void proceedWithUpdate(Map<String, Set<String>> existingPoliciesIds,

clients.circulationRulesStorage().put(rulesInput.copy())
.thenApply(this::failWhenResponseOtherThanNoContent)
.thenApply(result -> result.map(response -> CirculationRulesCache.getInstance()
.buildRules(webContext.getTenantId(), rulesAsText)))
.thenApply(result -> result.map(response -> noContent()))
.thenAccept(webContext::writeResultToHttpResponse);
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/folio/circulation/support/ErrorCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,7 @@ public enum ErrorCode {
CANNOT_CREATE_PAGE_TLR_WITHOUT_ITEM_ID,
MOVING_REQUEST_TO_THE_SAME_ITEM,
ITEM_ALREADY_REQUESTED,
ITEM_OF_THIS_INSTANCE_ALREADY_REQUESTED,
INSTANCE_ALREADY_REQUESTED,
HOLD_SHELF_REQUESTS_REQUIRE_PICKUP_SERVICE_POINT
}
14 changes: 0 additions & 14 deletions src/test/java/api/CirculationRulesAPITests.java
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
package api;

import static api.support.APITestContext.TENANT_ID;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.core.Is.is;

import java.util.HashSet;
import java.util.Set;
import java.util.UUID;

import org.folio.circulation.rules.cache.CirculationRulesCache;
import org.folio.circulation.support.http.client.Response;
import org.junit.jupiter.api.Test;

import api.support.APITestContext;
import api.support.APITests;
import api.support.builders.LoanPolicyBuilder;
import api.support.builders.LostItemFeePolicyBuilder;
Expand Down Expand Up @@ -432,15 +427,6 @@ void canReportValidationError() {
assertThat(json.getInteger("column"), is(2));
}

@Test
void getRefreshesCirculationRulesCache() {
CirculationRulesCache cache = CirculationRulesCache.getInstance();
cache.dropCache();
assertThat(cache.getRules(TENANT_ID), nullValue());
String rules = circulationRulesFixture.getCirculationRules();
assertThat(cache.getRules(TENANT_ID).getRulesAsText(), equalTo(rules));
}

/** @return rulesAsText field */
private String getRulesText() {
Response response = circulationRulesFixture.getRules();
Expand Down
41 changes: 27 additions & 14 deletions src/test/java/api/requests/RequestsAPICreationTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,11 @@
import static org.folio.circulation.domain.representations.logs.LogEventType.NOTICE_ERROR;
import static org.folio.circulation.domain.representations.logs.LogEventType.REQUEST_CREATED_THROUGH_OVERRIDE;
import static org.folio.circulation.support.ErrorCode.FULFILLMENT_PREFERENCE_IS_NOT_ALLOWED;
import static org.folio.circulation.support.ErrorCode.HOLD_SHELF_REQUESTS_REQUIRE_PICKUP_SERVICE_POINT;
import static org.folio.circulation.support.ErrorCode.HOLD_AND_RECALL_TLR_NOT_ALLOWED_PAGEABLE_AVAILABLE_ITEM_FOUND;
import static org.folio.circulation.support.ErrorCode.HOLD_SHELF_REQUESTS_REQUIRE_PICKUP_SERVICE_POINT;
import static org.folio.circulation.support.ErrorCode.INSTANCE_ALREADY_REQUESTED;
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.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;
Expand Down Expand Up @@ -102,7 +105,6 @@
import java.util.stream.IntStream;
import java.util.stream.Stream;

import api.support.builders.AddInfoRequestBuilder;
import org.apache.http.HttpStatus;
import org.awaitility.Awaitility;
import org.folio.circulation.domain.ItemStatus;
Expand Down Expand Up @@ -132,6 +134,7 @@

import api.support.APITests;
import api.support.TlrFeatureStatus;
import api.support.builders.AddInfoRequestBuilder;
import api.support.builders.Address;
import api.support.builders.CheckInByBarcodeRequestBuilder;
import api.support.builders.HoldingBuilder;
Expand Down Expand Up @@ -697,6 +700,7 @@ void cannotCreateTlrWhenUserAlreadyRequestedTheSameTitle() {
assertThat(postResponse, hasStatus(HTTP_UNPROCESSABLE_ENTITY));
assertThat(postResponse.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for this instance"),
hasCode(INSTANCE_ALREADY_REQUESTED),
hasParameter("requesterId", usersFixture.james().getId().toString()),
hasParameter("instanceId", instanceId.toString()))));
}
Expand Down Expand Up @@ -727,6 +731,7 @@ void cannotCreateTlrWhenUserAlreadyRequestedAnItemFromTheSameTitle() {
assertThat(response.getJson(), hasErrors(1));
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for one of the instance's items"),
hasCode(ITEM_OF_THIS_INSTANCE_ALREADY_REQUESTED),
hasParameter("requesterId", jessica.getId().toString()),
hasParameter("instanceId", item1.getInstanceId().toString()))));
}
Expand Down Expand Up @@ -1497,8 +1502,10 @@ void cannotCreateItemLevelRequestIfTitleLevelRequestForInstanceAlreadyCreated()
patronId, pickupServicePointId, instanceId, secondItem));

assertThat(response, hasStatus(HTTP_UNPROCESSABLE_ENTITY));
assertThat(response.getJson(), hasErrorWith(hasMessage(
"This requester already has an open request for this item")));
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for this item"),
hasCode(ITEM_ALREADY_REQUESTED)
)));
}

@Test
Expand All @@ -1517,8 +1524,10 @@ void cannotCreateTitleLevelRequestIfItemLevelRequestAlreadyCreated() {
Response response = requestsClient.attemptCreate(buildPageTitleLevelRequest(
patronId, pickupServicePointId, instanceId));
assertThat(response, hasStatus(HTTP_UNPROCESSABLE_ENTITY));
assertThat(response.getJson(), hasErrorWith(hasMessage(
"This requester already has an open request for one of the instance's items")));
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for one of the instance's items"),
hasCode(ITEM_OF_THIS_INSTANCE_ALREADY_REQUESTED)
)));
}

@Test
Expand Down Expand Up @@ -1556,8 +1565,10 @@ void cannotCreateTwoTitleLevelRequestsForSameInstance() {
Response response = requestsClient.attemptCreate(buildPageTitleLevelRequest(
userId, pickupServicePointId, instanceId));
assertThat(response, hasStatus(HTTP_UNPROCESSABLE_ENTITY));
assertThat(response.getJson(), hasErrorWith(hasMessage(
"This requester already has an open request for this instance")));
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for this instance"),
hasCode(INSTANCE_ALREADY_REQUESTED)
)));
assertThat(requestsClient.getAll(), hasSize(1));
}

Expand All @@ -1575,8 +1586,10 @@ void cannotCreateTwoItemLevelRequestsForSameItem() {
Response response = requestsClient.attemptCreate(buildItemLevelRequest(userId,
pickupServicePointId, instanceId, item));
assertThat(response, hasStatus(HTTP_UNPROCESSABLE_ENTITY));
assertThat(response.getJson(), hasErrorWith(hasMessage(
"This requester already has an open request for this item")));
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for this item"),
hasCode(ITEM_ALREADY_REQUESTED)
)));
assertThat(requestsClient.getAll(), hasSize(1));
}

Expand Down Expand Up @@ -2127,10 +2140,10 @@ void cannotCreateTwoRequestsFromTheSameUserForTheSameItem() {

assertThat(response, hasStatus(HTTP_UNPROCESSABLE_ENTITY));
assertThat(response.getJson(), hasErrors(1));
assertThat(
response.getJson(),
hasErrorWith(hasMessage("This requester already has an open request for this item"))
);
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("This requester already has an open request for this item"),
hasCode(ITEM_ALREADY_REQUESTED)
)));
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/api/requests/scenarios/MoveRequestTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import static org.folio.circulation.domain.representations.ItemProperties.CALL_NUMBER_COMPONENTS;
import static org.folio.circulation.domain.representations.RequestProperties.REQUEST_TYPE;
import static org.folio.circulation.domain.representations.logs.LogEventType.REQUEST_MOVED;
import static org.folio.circulation.support.ErrorCode.ITEM_ALREADY_REQUESTED;
import static org.folio.circulation.support.ErrorCode.MOVING_REQUEST_TO_THE_SAME_ITEM;
import static org.folio.circulation.support.utils.ClockUtil.getClock;
import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime;
import static org.folio.circulation.support.utils.ClockUtil.setClock;
Expand Down Expand Up @@ -428,7 +428,7 @@ void cannotMoveTlrToTheSameItem() {
Response response = requestsFixture.attemptMove(new MoveRequestBuilder(nodPage.getId(), item.getId()));
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("Not allowed to move title level page request to the same item"),
hasCode(ITEM_ALREADY_REQUESTED),
hasCode(MOVING_REQUEST_TO_THE_SAME_ITEM),
hasParameter("requesterId", jessica.getId().toString()),
hasParameter("instanceId", item.getInstanceId().toString()))));
}
Expand Down
5 changes: 3 additions & 2 deletions src/test/java/api/support/Wait.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.awaitility.Awaitility.waitAtMost;

import java.util.Collection;
import java.util.Objects;
import java.util.concurrent.Callable;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
Expand All @@ -24,8 +25,8 @@ public static <T> Collection<T> waitForSize(Callable<Collection<T>> supplier, in
return waitForValue(supplier, (Predicate<Collection<T>>) c -> c.size() == expectedSize);
}

public static <T> T waitForValue(Callable<T> valueSupplier, T expectedValue) {
return waitForValue(valueSupplier, (Predicate<T>) actualValue -> actualValue == expectedValue);
public static <T> T waitForValue(Callable<T> valueSupplier, T expected) {
return waitForValue(valueSupplier, (Predicate<T>) actual -> Objects.equals(actual, expected));
}

public static <T> T waitForValue(Callable<T> valueSupplier, Predicate<T> valuePredicate) {
Expand Down
12 changes: 11 additions & 1 deletion src/test/java/api/support/fixtures/CirculationRulesFixture.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package api.support.fixtures;

import static api.support.APITestContext.getTenantId;
import static api.support.RestAssuredResponseConversion.toResponse;
import static api.support.http.InterfaceUrls.circulationRulesStorageUrl;
import static api.support.http.InterfaceUrls.circulationRulesUrl;
Expand All @@ -15,11 +16,13 @@
import java.util.List;
import java.util.UUID;

import org.apache.http.HttpStatus;
import org.folio.circulation.rules.ItemLocation;
import org.folio.circulation.rules.ItemType;
import org.folio.circulation.rules.LoanType;
import org.folio.circulation.rules.PatronGroup;
import org.folio.circulation.rules.Policy;
import org.folio.circulation.rules.cache.CirculationRulesCache;
import org.folio.circulation.support.http.client.Response;

import api.support.RestAssuredClient;
Expand All @@ -45,11 +48,18 @@ public String getCirculationRules() {
}

public Response putRules(String body) {
return toResponse(restAssuredClient
Response response = toResponse(restAssuredClient
.beginRequest("put-circulation-rules")
.body(body)
.when().put(circulationRulesUrl())
.then().extract().response());

if (response.getStatusCode() == HttpStatus.SC_NO_CONTENT) {
String rulesAsText = new JsonObject(body).getString("rulesAsText");
CirculationRulesCache.getInstance().buildRules(getTenantId(), rulesAsText);
}

return response;
}

public void updateCirculationRules(UUID loanPolicyId, UUID requestPolicyId,
Expand Down
Loading

0 comments on commit 07f5e32

Please sign in to comment.