From 19bf73c8a38e4399d0352d8eda2ff0aa43f30dbc Mon Sep 17 00:00:00 2001 From: Roman Barannyk <53909129+roman-barannyk@users.noreply.github.com> Date: Mon, 15 Apr 2024 12:05:25 +0300 Subject: [PATCH 01/27] [CIRC-2051] Add ecsRequestRouting parameter to allowed-service-points (#1455) * CIRC-2051 add ecsRequestRouting to allowed-service-points * CIRC-2051 refactoring * CIRC-2051 move common validation to separate method * CIRC-2051 fix code smells * CIRC-2051 fix code smell * CIRC-2051 tests refactoring * CIRC-2051 rename test --- descriptors/ModuleDescriptor-template.json | 2 +- ramls/circulation.raml | 4 + .../domain/AllowedServicePointsRequest.java | 1 + .../storage/ServicePointRepository.java | 16 +-- .../AllowedServicePointsResource.java | 21 ++-- .../services/AllowedServicePointsService.java | 10 +- .../AllowedServicePointsAPITests.java | 106 +++++++++++++----- .../LoanDueDatesAfterRecallTests.java | 2 +- .../support/builders/ServicePointBuilder.java | 53 +++++++-- .../fixtures/ServicePointExamples.java | 8 ++ .../fixtures/ServicePointsFixture.java | 6 + 11 files changed, 173 insertions(+), 56 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 8a02b239f2..cf24358336 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -649,7 +649,7 @@ }, { "id": "allowed-service-points", - "version": "1.1", + "version": "1.2", "handlers": [ { "methods": [ diff --git a/ramls/circulation.raml b/ramls/circulation.raml index dca970e8e4..1b527ed222 100644 --- a/ramls/circulation.raml +++ b/ramls/circulation.raml @@ -347,6 +347,10 @@ resourceTypes: description: "When true, allows to apply circulation rules based on patron group only" type: boolean required: false + ecsRequestRouting: + description: "When true, returns only service points with ecsRequestRouting" + type: boolean + required: false responses: 200: description: "List of allowed service points was retrieved successfully" diff --git a/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java b/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java index 1f2a13854c..16ea46f1ea 100644 --- a/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java +++ b/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java @@ -21,6 +21,7 @@ public class AllowedServicePointsRequest { private String itemId; private String requestId; private boolean useStubItem; + private boolean ecsRequestRouting; public boolean isForTitleLevelRequest() { return instanceId != null; diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java index fe9ec5c7e1..c7533cd1fc 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java @@ -206,22 +206,24 @@ public CompletableFuture>> findServicePointsById .thenApply(r -> r.map(MultipleRecords::getRecords)); } - public CompletableFuture>> fetchPickupLocationServicePoints() { + public CompletableFuture>> fetchServicePointsByIndexName( + String indexName) { + return createServicePointsFetcher().find(MultipleCqlIndexValuesCriteria.builder() - .indexName("pickupLocation") + .indexName(indexName) .indexOperator(CqlQuery::matchAny) .value("true") .build()) .thenApply(r -> r.map(MultipleRecords::getRecords)); } - public CompletableFuture>> fetchPickupLocationServicePointsByIds( - Set ids) { + public CompletableFuture>> + fetchPickupLocationServicePointsByIdsAndIndexName(Set ids, String indexName) { - log.debug("filterIdsByServicePointsAndPickupLocationExistence:: parameters ids: {}", - () -> collectionAsString(ids)); + log.debug("filterIdsByServicePointsAndPickupLocationExistence:: parameters ids: {}, " + + "indexName: {}", () -> collectionAsString(ids), () -> indexName); - Result pickupLocationQuery = exactMatch("pickupLocation", "true"); + Result pickupLocationQuery = exactMatch(indexName, "true"); return createServicePointsFetcher().findByIdIndexAndQuery(ids, "id", pickupLocationQuery) .thenApply(r -> r.map(MultipleRecords::getRecords)); diff --git a/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java b/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java index 4b7c9e9f2d..be103e5c11 100644 --- a/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java +++ b/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java @@ -53,7 +53,8 @@ private void get(RoutingContext routingContext) { ofAsync(routingContext) .thenApply(r -> r.next(AllowedServicePointsResource::buildRequest)) - .thenCompose(r -> r.after(new AllowedServicePointsService(clients)::getAllowedServicePoints)) + .thenCompose(r -> r.after(request -> new AllowedServicePointsService( + clients, request.isEcsRequestRouting()).getAllowedServicePoints(request))) .thenApply(r -> r.map(AllowedServicePointsResource::toJson)) .thenApply(r -> r.map(JsonHttpResponse::ok)) .exceptionally(CommonFailures::failedDueToServerError) @@ -72,6 +73,7 @@ private static Result buildRequest(RoutingContext r String itemId = queryParams.get("itemId"); String requestId = queryParams.get("requestId"); String useStubItem = queryParams.get("useStubItem"); + String ecsRequestRouting = queryParams.get("ecsRequestRouting"); List errors = new ArrayList<>(); @@ -94,11 +96,8 @@ private static Result buildRequest(RoutingContext r requestId); errors.add(String.format("Request ID is not a valid UUID: %s.", requestId)); } - if (useStubItem != null && !"true".equals(useStubItem) && !"false".equals(useStubItem)) { - log.warn("validateAllowedServicePointsRequest:: useStubItem is not a valid boolean: {}", - useStubItem); - errors.add(String.format("useStubItem is not a valid boolean: %s.", useStubItem)); - } + validateBoolean(useStubItem, "useStubItem", errors); + validateBoolean(ecsRequestRouting, "ecsRequestRouting", errors); // Checking parameter combinations boolean allowedCombinationOfParametersDetected = false; @@ -137,7 +136,15 @@ private static Result buildRequest(RoutingContext r } return succeeded(new AllowedServicePointsRequest(operation, requesterId, instanceId, itemId, - requestId, Boolean.parseBoolean(useStubItem))); + requestId, Boolean.parseBoolean(useStubItem), Boolean.parseBoolean(ecsRequestRouting))); + } + + private static void validateBoolean(String parameter, String parameterName, List errors) { + if (parameter != null && !"true".equals(parameter) && !"false".equals(parameter)) { + log.warn("validateBoolean:: {} is not a valid boolean: {}", + parameterName, parameter); + errors.add(String.format("%s is not a valid boolean: %s.", parameterName, parameter)); + } } private static JsonObject toJson(Map> allowedServicePoints) { diff --git a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java index 74140f217a..cd59a6d6ea 100644 --- a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java +++ b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java @@ -58,6 +58,8 @@ public class AllowedServicePointsService { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + private static final String ECS_REQUEST_ROUTING_INDEX_NAME = "ecsRequestRouting"; + private static final String PICKUP_LOCATION_INDEX_NAME = "pickupLocation"; private final ItemRepository itemRepository; private final UserRepository userRepository; private final RequestRepository requestRepository; @@ -66,8 +68,9 @@ public class AllowedServicePointsService { private final ItemByInstanceIdFinder itemFinder; private final ConfigurationRepository configurationRepository; private final InstanceRepository instanceRepository; + private final String indexName; - public AllowedServicePointsService(Clients clients) { + public AllowedServicePointsService(Clients clients, boolean isEcsRequestRouting) { itemRepository = new ItemRepository(clients); userRepository = new UserRepository(clients); requestRepository = new RequestRepository(clients); @@ -76,6 +79,7 @@ public AllowedServicePointsService(Clients clients) { configurationRepository = new ConfigurationRepository(clients); instanceRepository = new InstanceRepository(clients); itemFinder = new ItemByInstanceIdFinder(clients.holdingsStorage(), itemRepository); + indexName = isEcsRequestRouting ? ECS_REQUEST_ROUTING_INDEX_NAME : PICKUP_LOCATION_INDEX_NAME; } public CompletableFuture>>> @@ -360,7 +364,7 @@ private Map> combineAllowedServicePoints( } private CompletableFuture>> fetchAllowedServicePoints() { - return servicePointRepository.fetchPickupLocationServicePoints() + return servicePointRepository.fetchServicePointsByIndexName(indexName) .thenApply(r -> r.map(servicePoints -> servicePoints.stream() .map(AllowedServicePoint::new) .collect(Collectors.toSet()))); @@ -372,7 +376,7 @@ private CompletableFuture>> fetchPickupLocationS log.debug("filterIdsByServicePointsAndPickupLocationExistence:: parameters ids: {}", () -> collectionAsString(ids)); - return servicePointRepository.fetchPickupLocationServicePointsByIds(ids) + return servicePointRepository.fetchPickupLocationServicePointsByIdsAndIndexName(ids, indexName) .thenApply(servicePointsResult -> servicePointsResult .map(servicePoints -> servicePoints.stream() .map(AllowedServicePoint::new) diff --git a/src/test/java/api/requests/AllowedServicePointsAPITests.java b/src/test/java/api/requests/AllowedServicePointsAPITests.java index a8da598cd2..0c901d94f2 100644 --- a/src/test/java/api/requests/AllowedServicePointsAPITests.java +++ b/src/test/java/api/requests/AllowedServicePointsAPITests.java @@ -154,8 +154,8 @@ void shouldReturnListOfAllowedServicePointsForRequest(RequestType requestType, .collect(Collectors.toSet())); var response = requestLevel == TITLE - ? get("create", requesterId, instanceId, null, null, null, HttpStatus.SC_OK).getJson() - : get("create", requesterId, null, itemId, null, null, HttpStatus.SC_OK).getJson(); + ? get("create", requesterId, instanceId, null, null, null, null, HttpStatus.SC_OK).getJson() + : get("create", requesterId, null, itemId, null, null, null, HttpStatus.SC_OK).getJson(); assertThat(response, allowedServicePointMatcher(Map.of(requestType, allowedSpInResponse))); } @@ -223,7 +223,7 @@ void shouldReturnListOfAllowedServicePointsForRequestReplacement( var requestId = request == null ? null : request.getId().toString(); var response = - get("replace", null, null, null, requestId, null, HttpStatus.SC_OK).getJson(); + get("replace", null, null, null, requestId, null, null, HttpStatus.SC_OK).getJson(); assertThat(response, allowedServicePointMatcher(Map.of(requestType, allowedSpInResponse))); } @@ -625,7 +625,7 @@ void shouldReturnAllowedServicePointsForAllEnabledRequestTypes() { void getReplaceFailsWhenRequestDoesNotExist() { String requestId = randomId(); Response response = get("replace", null, null, null, requestId, null, - HttpStatus.SC_UNPROCESSABLE_ENTITY); + null, HttpStatus.SC_UNPROCESSABLE_ENTITY); assertThat(response.getJson(), hasErrorWith(hasMessage( String.format("Request with ID %s was not found", requestId)))); } @@ -635,7 +635,7 @@ void getMoveFailsWhenRequestDoesNotExist() { String requestId = randomId(); String itemId = itemsFixture.basedUponNod().getId().toString(); Response response = get("move", null, null, itemId, requestId, null, - HttpStatus.SC_UNPROCESSABLE_ENTITY); + null, HttpStatus.SC_UNPROCESSABLE_ENTITY); assertThat(response.getJson(), hasErrorWith(hasMessage( String.format("Request with ID %s was not found", requestId)))); } @@ -696,58 +696,58 @@ void shouldReturnListOfAllowedServicePointsForRequestMove(RequestLevel requestLe // Valid "move" request var moveResponse = - get("move", null, null, itemToMoveToId, requestId, null, HttpStatus.SC_OK).getJson(); + get("move", null, null, itemToMoveToId, requestId, null, null, HttpStatus.SC_OK).getJson(); assertThat(moveResponse, allowedServicePointMatcher(Map.of(HOLD, List.of(sp2)))); // Invalid "move" requests var invalidMoveResponse1 = get("move", null, null, null, requestId, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse1.getBody(), equalTo("Invalid combination of query parameters")); var invalidMoveResponse2 = get("move", null, null, itemToMoveToId, null, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse2.getBody(), equalTo("Invalid combination of query parameters")); var invalidMoveResponse3 = get("move", null, null, null, null, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse3.getBody(), equalTo("Invalid combination of query parameters")); var invalidMoveResponse4 = get("move", requesterId, null, itemToMoveToId, requestId, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse4.getBody(), equalTo("Invalid combination of query parameters")); var invalidMoveResponse5 = get("move", null, instanceId, itemToMoveToId, requestId, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse5.getBody(), equalTo("Invalid combination of query parameters")); // Valid "replace" request var replaceResponse = - get("replace", null, null, null, requestId, null, HttpStatus.SC_OK).getJson(); + get("replace", null, null, null, requestId, null, null, HttpStatus.SC_OK).getJson(); assertThat(replaceResponse, allowedServicePointMatcher(Map.of(HOLD, List.of(sp2)))); // Invalid "replace" requests var invalidReplaceResponse1 = get("replace", null, null, null, null, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse1.getBody(), equalTo("Invalid combination of query parameters")); var invalidReplaceResponse2 = get("replace", requesterId, null, null, requestId, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse2.getBody(), equalTo("Invalid combination of query parameters")); var invalidReplaceResponse3 = get("replace", null, instanceId, null, requestId, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse3.getBody(), equalTo("Invalid combination of query parameters")); var invalidReplaceResponse4 = get("replace", null, null, requestedItemId, requestId, - null, HttpStatus.SC_BAD_REQUEST); + null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse4.getBody(), equalTo("Invalid combination of query parameters")); var invalidReplaceResponse5 = get("replace", requesterId, instanceId, - requestedItemId, requestId, null, HttpStatus.SC_BAD_REQUEST); + requestedItemId, requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse5.getBody(), equalTo("Invalid combination of query parameters")); } @@ -766,14 +766,16 @@ void shouldUseStubItemParameterInCirculationRuleMatchingWhenPresent() { circulationRulesFixture.updateCirculationRules(createRules("m " + book + "+ g " + patronGroup, "g " + patronGroup)); - var response = getCreateOp(requesterId, instanceId, null, "true", HttpStatus.SC_OK).getJson(); + var response = getCreateOp(requesterId, instanceId, null, "true", null, HttpStatus.SC_OK) + .getJson(); assertThat(response, hasNoJsonPath(PAGE.getValue())); JsonArray allowedServicePoints = response.getJsonArray(HOLD.getValue()); assertServicePointsMatch(allowedServicePoints, List.of(cd1, cd2, cd4, cd5)); allowedServicePoints = response.getJsonArray(RECALL.getValue()); assertServicePointsMatch(allowedServicePoints, List.of(cd1, cd2, cd4, cd5)); - response = getCreateOp(requesterId, instanceId, null, "false", HttpStatus.SC_OK).getJson(); + response = getCreateOp(requesterId, instanceId, null, "false", null, HttpStatus.SC_OK) + .getJson(); assertThat(response, hasNoJsonPath(HOLD.getValue())); assertThat(response, hasNoJsonPath(RECALL.getValue())); allowedServicePoints = response.getJsonArray(PAGE.getValue()); @@ -784,12 +786,62 @@ void shouldUseStubItemParameterInCirculationRuleMatchingWhenPresent() { assertThat(response, hasNoJsonPath(RECALL.getValue())); allowedServicePoints = response.getJsonArray(PAGE.getValue()); assertServicePointsMatch(allowedServicePoints, List.of(cd1, cd2, cd4, cd5)); + } - Response errorResponse = getCreateOp(requesterId, instanceId, null, "invalid", + @Test + void shouldReturnErrorIfUseStubItemIsInvalid() { + Response errorResponse = getCreateOp(UUID.randomUUID().toString(), + UUID.randomUUID().toString(), null, "invalid", null, HttpStatus.SC_BAD_REQUEST); assertThat(errorResponse.getBody(), is("useStubItem is not a valid boolean: invalid.")); } + @Test + void shouldConsiderEcsRequestRoutingParameterForAllowedServicePoints() { + var requesterId = usersFixture.steve().getId().toString(); + var instanceId = itemsFixture.createMultipleItemsForTheSameInstance(2).get(0) + .getInstanceId().toString(); + var cd1 = servicePointsFixture.cd1(); + var cd2 = servicePointsFixture.cd2(); + var cd4 = servicePointsFixture.cd4(); + var cd11 = servicePointsFixture.cd11(); + + final Map> allowedServicePointsInPolicy = new HashMap<>(); + allowedServicePointsInPolicy.put(PAGE, Set.of(cd1.getId(), cd2.getId(), cd11.getId())); + allowedServicePointsInPolicy.put(HOLD, Set.of(cd4.getId(), cd2.getId(), cd11.getId())); + var requestPolicy = requestPoliciesFixture.createRequestPolicyWithAllowedServicePoints( + allowedServicePointsInPolicy, PAGE, HOLD); + policiesActivation.use(PoliciesToActivate.builder().requestPolicy(requestPolicy)); + + var response = getCreateOp(requesterId, instanceId, null, "false", "true", + HttpStatus.SC_OK).getJson(); + JsonArray allowedServicePoints = response.getJsonArray(PAGE.getValue()); + assertServicePointsMatch(allowedServicePoints, List.of(cd11)); + assertThat(response, hasNoJsonPath(HOLD.getValue())); + assertThat(response, hasNoJsonPath(RECALL.getValue())); + + response = getCreateOp(requesterId, instanceId, null, "false", "false", + HttpStatus.SC_OK).getJson(); + allowedServicePoints = response.getJsonArray(PAGE.getValue()); + assertServicePointsMatch(allowedServicePoints, List.of(cd1, cd2)); + assertThat(response, hasNoJsonPath(HOLD.getValue())); + assertThat(response, hasNoJsonPath(RECALL.getValue())); + + response = getCreateOp(requesterId, instanceId, null, "false", null, + HttpStatus.SC_OK).getJson(); + allowedServicePoints = response.getJsonArray(PAGE.getValue()); + assertServicePointsMatch(allowedServicePoints, List.of(cd1, cd2)); + assertThat(response, hasNoJsonPath(HOLD.getValue())); + assertThat(response, hasNoJsonPath(RECALL.getValue())); + } + + @Test + void shouldReturnErrorIfEcsRequestRoutingIsInvalid() { + Response errorResponse = getCreateOp(UUID.randomUUID().toString(), + UUID.randomUUID().toString(), null, null, "invalid", HttpStatus.SC_BAD_REQUEST); + assertThat(errorResponse.getBody(), is("ecsRequestRouting is not a valid boolean: invalid.")); + } + private void assertServicePointsMatch(JsonArray response, List expectedServicePoints) { @@ -814,23 +866,24 @@ private void assertServicePointsMatch(JsonArray response, } private Response getCreateOp(String requesterId, String instanceId, String itemId, - String useStubItem, int expectedStatusCode) { + String useStubItem, String ecsRequestRouting, int expectedStatusCode) { - return get("create", requesterId, instanceId, itemId, null, useStubItem, expectedStatusCode); + return get("create", requesterId, instanceId, itemId, null, useStubItem, + ecsRequestRouting, expectedStatusCode); } private Response getCreateOp(String requesterId, String instanceId, String itemId, int expectedStatusCode) { - return get("create", requesterId, instanceId, itemId, null, null, expectedStatusCode); + return get("create", requesterId, instanceId, itemId, null, null, null, expectedStatusCode); } private Response getReplaceOp(String requestId, int expectedStatusCode) { - return get("replace", null, null, null, requestId, null, expectedStatusCode); + return get("replace", null, null, null, requestId, null, null, expectedStatusCode); } private Response get(String operation, String requesterId, String instanceId, String itemId, - String requestId, String useStubItem, int expectedStatusCode) { + String requestId, String useStubItem, String ecsRequestRouting, int expectedStatusCode) { List queryParams = new ArrayList<>(); queryParams.add(namedParameter("operation", operation)); @@ -849,6 +902,9 @@ private Response get(String operation, String requesterId, String instanceId, St if (useStubItem != null) { queryParams.add(namedParameter("useStubItem", useStubItem)); } + if (ecsRequestRouting != null) { + queryParams.add(namedParameter("ecsRequestRouting", ecsRequestRouting)); + } return restAssuredClient.get(allowedServicePointsUrl(), queryParams, expectedStatusCode, "allowed-service-points"); diff --git a/src/test/java/api/requests/scenarios/LoanDueDatesAfterRecallTests.java b/src/test/java/api/requests/scenarios/LoanDueDatesAfterRecallTests.java index e038b56595..eeaf801853 100644 --- a/src/test/java/api/requests/scenarios/LoanDueDatesAfterRecallTests.java +++ b/src/test/java/api/requests/scenarios/LoanDueDatesAfterRecallTests.java @@ -285,7 +285,7 @@ void recallRequestWithMGDAndRDValuesChangesDueDateToMGDWithCLDDM() { setFallbackPolicies(canCirculateRollingPolicy); - servicePointsFixture.create(new ServicePointBuilder(checkOutServicePointId, "CLDDM Desk", "clddm", "CLDDM Desk Test", null, null, TRUE, null, null)); + servicePointsFixture.create(new ServicePointBuilder(checkOutServicePointId, "CLDDM Desk", "clddm", "CLDDM Desk Test", null, null, TRUE, null, null, null)); // We use the loan date to calculate the minimum guaranteed due date (MGD) final ZonedDateTime loanDate = diff --git a/src/test/java/api/support/builders/ServicePointBuilder.java b/src/test/java/api/support/builders/ServicePointBuilder.java index e2fb90ec3f..480f354a67 100644 --- a/src/test/java/api/support/builders/ServicePointBuilder.java +++ b/src/test/java/api/support/builders/ServicePointBuilder.java @@ -18,6 +18,7 @@ public class ServicePointBuilder extends JsonBuilder implements Builder { private final JsonObject holdShelfExpiryPeriod; private final String holdShelfClosedLibraryDateManagement; + private final Boolean ecsRequestRouting; public ServicePointBuilder( UUID id, @@ -28,7 +29,8 @@ public ServicePointBuilder( Integer shelvingLagTime, Boolean pickupLocation, JsonObject holdShelfExpiryPeriod, - String holdShelfClosedLibraryDateManagement) { + String holdShelfClosedLibraryDateManagement, + Boolean ecsRequestRouting) { this.id = id; this.name = name; this.code = code; @@ -38,6 +40,7 @@ public ServicePointBuilder( this.pickupLocation = pickupLocation; this.holdShelfExpiryPeriod = holdShelfExpiryPeriod; this.holdShelfClosedLibraryDateManagement = holdShelfClosedLibraryDateManagement; + this.ecsRequestRouting = ecsRequestRouting; } public ServicePointBuilder(String name, String code, String discoveryDisplayName) { @@ -50,6 +53,7 @@ public ServicePointBuilder(String name, String code, String discoveryDisplayName null, false, null, + null, null); } @@ -64,8 +68,9 @@ public static ServicePointBuilder from(IndividualResource response) { getIntegerProperty(representation, "shelvingLagTime", null), getBooleanProperty(representation, "pickupLocation"), getObjectProperty(representation, "holdShelfExpiryPeriod"), - getProperty(representation, "holdShelfClosedLibraryDateManagement") - ); + getProperty(representation, "holdShelfClosedLibraryDateManagement"), + getBooleanProperty(representation, "ecsRequestRouting") + ); } @Override @@ -80,6 +85,7 @@ public JsonObject create() { put(servicePoint, "pickupLocation", this.pickupLocation); put(servicePoint, "holdShelfExpiryPeriod", this.holdShelfExpiryPeriod); put(servicePoint, "holdShelfClosedLibraryDateManagement", this.holdShelfClosedLibraryDateManagement); + put(servicePoint, "ecsRequestRouting", this.ecsRequestRouting); return servicePoint; } @@ -94,7 +100,8 @@ public ServicePointBuilder withId(UUID newId) { this.shelvingLagTime, this.pickupLocation, this.holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withName(String newName) { @@ -107,7 +114,8 @@ public ServicePointBuilder withName(String newName) { this.shelvingLagTime, this.pickupLocation, this.holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withCode(String newCode) { @@ -120,7 +128,8 @@ public ServicePointBuilder withCode(String newCode) { this.shelvingLagTime, this.pickupLocation, this.holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withDiscoveryDisplayName(String newDiscoveryDisplayName) { @@ -133,7 +142,8 @@ public ServicePointBuilder withDiscoveryDisplayName(String newDiscoveryDisplayNa this.shelvingLagTime, this.pickupLocation, this.holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withDescription(String newDescription) { @@ -146,7 +156,8 @@ public ServicePointBuilder withDescription(String newDescription) { this.shelvingLagTime, this.pickupLocation, this.holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withShelvingLagTime(Integer newShelvingLagTime) { @@ -159,7 +170,8 @@ public ServicePointBuilder withShelvingLagTime(Integer newShelvingLagTime) { newShelvingLagTime, this.pickupLocation, this.holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withPickupLocation(Boolean newPickupLocation) { @@ -172,7 +184,8 @@ public ServicePointBuilder withPickupLocation(Boolean newPickupLocation) { this.shelvingLagTime, newPickupLocation, this.holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withHoldShelfExpriyPeriod(int duration, String intervalId) { @@ -190,7 +203,8 @@ public ServicePointBuilder withHoldShelfExpriyPeriod(int duration, String interv this.shelvingLagTime, this.pickupLocation, holdShelfExpiryPeriod, - this.holdShelfClosedLibraryDateManagement); + this.holdShelfClosedLibraryDateManagement, + this.ecsRequestRouting); } public ServicePointBuilder withholdShelfClosedLibraryDateManagement(String expirationDateManagement) { @@ -203,6 +217,21 @@ public ServicePointBuilder withholdShelfClosedLibraryDateManagement(String expir this.shelvingLagTime, this.pickupLocation, this.holdShelfExpiryPeriod, - expirationDateManagement); + expirationDateManagement, + this.ecsRequestRouting); + } + + public ServicePointBuilder withEcsRequestRouting(Boolean ecsRequestRouting) { + return new ServicePointBuilder( + this.id, + this.name, + this.code, + this.discoveryDisplayName, + this.description, + this.shelvingLagTime, + this.pickupLocation, + this.holdShelfExpiryPeriod, + this.holdShelfClosedLibraryDateManagement, + ecsRequestRouting); } } diff --git a/src/test/java/api/support/fixtures/ServicePointExamples.java b/src/test/java/api/support/fixtures/ServicePointExamples.java index 05dc048a41..3267a39341 100644 --- a/src/test/java/api/support/fixtures/ServicePointExamples.java +++ b/src/test/java/api/support/fixtures/ServicePointExamples.java @@ -79,4 +79,12 @@ static ServicePointBuilder basedUponCircDesk10() { .withholdShelfClosedLibraryDateManagement(ExpirationDateManagement.MOVE_TO_THE_END_OF_THE_NEXT_OPEN_DAY.name()) .withHoldShelfExpriyPeriod(6, "Months"); } + + static ServicePointBuilder basedUponCircDesk11() { + return new ServicePointBuilder("Circ Desk 11", "cd11", + "Circulation Desk -- Back Entrance") + .withPickupLocation(FALSE) + .withEcsRequestRouting(TRUE) + .withHoldShelfExpriyPeriod(6, "Months"); + } } diff --git a/src/test/java/api/support/fixtures/ServicePointsFixture.java b/src/test/java/api/support/fixtures/ServicePointsFixture.java index aaf734c60f..cc4fb0fce1 100644 --- a/src/test/java/api/support/fixtures/ServicePointsFixture.java +++ b/src/test/java/api/support/fixtures/ServicePointsFixture.java @@ -2,6 +2,7 @@ import static api.support.fixtures.ServicePointExamples.basedUponCircDesk1; import static api.support.fixtures.ServicePointExamples.basedUponCircDesk10; +import static api.support.fixtures.ServicePointExamples.basedUponCircDesk11; import static api.support.fixtures.ServicePointExamples.basedUponCircDesk2; import static api.support.fixtures.ServicePointExamples.basedUponCircDesk3; import static api.support.fixtures.ServicePointExamples.basedUponCircDesk4; @@ -81,6 +82,11 @@ public IndividualResource cd10() { return create(basedUponCircDesk10()); } + public IndividualResource cd11() { + + return create(basedUponCircDesk11()); + } + public IndividualResource create(ServicePointBuilder builder) { return servicePointRecordCreator.createIfAbsent(builder); From 6e3315d5ae518abf71f1107f7b8d7844520944cd Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Tue, 23 Apr 2024 15:18:18 +0300 Subject: [PATCH 02/27] conflicts resolving --- .../requests/AllowedServicePointsAPITests.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/java/api/requests/AllowedServicePointsAPITests.java b/src/test/java/api/requests/AllowedServicePointsAPITests.java index 89655d4675..a2c54736dd 100644 --- a/src/test/java/api/requests/AllowedServicePointsAPITests.java +++ b/src/test/java/api/requests/AllowedServicePointsAPITests.java @@ -22,6 +22,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.core.Is.is; import java.time.ZonedDateTime; import java.util.ArrayList; @@ -927,4 +928,21 @@ private ServicePointBuilder servicePointBuilder() { .withPickupLocation(TRUE) .withHoldShelfExpriyPeriod(30, "Days"); } + + private String createRules(String firstRuleCondition, String secondRuleCondition) { + final var loanPolicy = loanPoliciesFixture.canCirculateRolling().getId().toString(); + final var allowAllRequestPolicy = requestPoliciesFixture.allowAllRequestPolicy() + .getId().toString(); + final var holdAndRecallRequestPolicy = requestPoliciesFixture.allowHoldAndRecallRequestPolicy() + .getId().toString(); + final var noticePolicy = noticePoliciesFixture.activeNotice().getId().toString(); + final var overdueFinePolicy = overdueFinePoliciesFixture.facultyStandard().getId().toString(); + final var lostItemFeePolicy = lostItemFeePoliciesFixture.facultyStandard().getId().toString(); + + return String.join("\n", + "priority: t, s, c, b, a, m, g", + "fallback-policy: l " + loanPolicy + " r " + allowAllRequestPolicy + " n " + noticePolicy + " o " + overdueFinePolicy + " i " + lostItemFeePolicy, + firstRuleCondition + " : l " + loanPolicy + " r " + allowAllRequestPolicy + " n " + noticePolicy + " o " + overdueFinePolicy + " i " + lostItemFeePolicy, + secondRuleCondition + " : l " + loanPolicy + " r " + holdAndRecallRequestPolicy + " n " + noticePolicy + " o " + overdueFinePolicy + " i " + lostItemFeePolicy); + } } From ae6991ded251118da2700852e07003933e8248cf Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Wed, 24 Apr 2024 19:11:39 +0300 Subject: [PATCH 03/27] conflicts resolving --- .../storage/requests/RequestPolicyRepository.java | 10 ++++++++++ .../services/AllowedServicePointsService.java | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java index 60f35c780d..7df5c56de1 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java @@ -15,6 +15,7 @@ import java.util.Collection; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.function.BinaryOperator; import java.util.stream.Collectors; @@ -95,6 +96,15 @@ public CompletableFuture>>> lookupRequestPol .thenCompose(r -> r.after(this::lookupRequestPolicies)); } + public CompletableFuture> lookupRequestPolicy(User user) { + // Circulation rules need to be executed with the patron group parameter only. + // All the item-related parameters should be random UUIDs. + return lookupRequestPolicyId(UUID.randomUUID().toString(), user.getPatronGroupId(), + UUID.randomUUID().toString(), UUID.randomUUID().toString()) + .thenCompose(r -> r.after(this::lookupRequestPolicy)) + .thenApply(result -> result.map(RequestPolicy::from)); + } + private BinaryOperator> itemsMergeOperator() { return (items1, items2) -> Stream.concat(items1.stream(), items2.stream()) .collect(Collectors.toSet()); diff --git a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java index f81df42ef6..cd59a6d6ea 100644 --- a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java +++ b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.EnumMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -158,6 +159,12 @@ private CompletableFuture> fetchUser(AllowedServicePointsRequest re ? this::extractAllowedServicePointsIgnoringItemStatus : this::extractAllowedServicePointsConsideringItemStatus; + if (request.isUseStubItem()) { + return requestPolicyRepository.lookupRequestPolicy(user) + .thenCompose(r -> r.after(policy -> extractAllowedServicePointsIgnoringItemStatus( + policy, new HashSet<>()))); + } + return requestPolicyRepository.lookupRequestPolicies(items, user) .thenCompose(r -> r.after(policies -> allOf(policies, mappingFunction))) .thenApply(r -> r.map(this::combineAllowedServicePoints)); From 32b7e00554d9a4c14143387368c42efc186a2ad5 Mon Sep 17 00:00:00 2001 From: OleksandrVidinieiev <56632770+OleksandrVidinieiev@users.noreply.github.com> Date: Fri, 26 Apr 2024 17:56:19 +0300 Subject: [PATCH 04/27] CIRC-2081: Fetch TLR settings from mod-settings (#1467) * CIRC-2081 Fetch TLR settings from mod-settings * CIRC-2081 Fetch TLR settings from mod-settings * CIRC-2081 Refactoring * CIRC-2081 Add permissions * CIRC-2081 Remove redundant changes, fixture refactoring * CIRC-2081 Revert redundant change * CIRC-2081 Merge SettingsBuilder constructors * CIRC-2081 Add global permissions * CIRC-2081 Remove unused methods --- descriptors/ModuleDescriptor-template.json | 41 +++++++-- .../domain/MoveRequestService.java | 7 +- .../storage/ConfigurationRepository.java | 30 ------- .../storage/SettingsRepository.java | 41 +++++++-- .../resources/ChangeDueDateResource.java | 11 ++- .../resources/CheckInByBarcodeResource.java | 4 +- .../resources/CheckOutByBarcodeResource.java | 2 +- .../RequestByInstanceIdResource.java | 4 +- .../resources/RequestCollectionResource.java | 4 +- .../RequestFromRepresentationService.java | 5 +- .../resources/RequestQueueResource.java | 8 +- .../resources/renewal/RenewalResource.java | 4 +- .../services/AllowedServicePointsService.java | 8 +- .../request/RequestRelatedRepositories.java | 3 + .../java/api/loans/CheckInByBarcodeTests.java | 20 ++--- .../api/loans/CheckOutByBarcodeTests.java | 8 +- .../scenarios/ChangeDueDateAPITests.java | 4 +- .../CheckoutWithRequestScenarioTests.java | 4 +- .../api/queue/RequestQueueResourceTest.java | 2 +- .../AllowedServicePointsAPITests.java | 8 +- .../requests/RequestsAPICreationTests.java | 90 +++++++++---------- .../requests/RequestsAPILoanHistoryTests.java | 2 +- .../requests/RequestsAPILoanRenewalTests.java | 18 ++-- .../requests/RequestsAPIRetrievalTests.java | 2 +- .../requests/RequestsAPIUpdatingTests.java | 2 +- .../scenarios/HoldShelfFulfillmentTests.java | 19 ++-- .../requests/scenarios/MoveRequestTests.java | 24 ++--- src/test/java/api/support/APITests.java | 36 ++++---- .../api/support/builders/SettingsBuilder.java | 2 +- .../java/api/support/fakes/FakeOkapi.java | 1 - .../fixtures/ConfigurationExample.java | 34 ------- .../fixtures/ConfigurationsFixture.java | 41 --------- .../api/support/fixtures/SettingsFixture.java | 61 ++++++++++++- 33 files changed, 284 insertions(+), 266 deletions(-) delete mode 100644 src/test/java/api/support/fixtures/ConfigurationsFixture.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index cf24358336..b4641a9048 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -132,8 +132,7 @@ "methods": ["POST"], "pathPattern": "/circulation/loans/{id}/change-due-date", "permissionsRequired": [ - "circulation.loans.change-due-date.post", - "configuration.entries.collection.get" + "circulation.loans.change-due-date.post" ], "modulePermissions": [ "modperms.circulation.loans.change-due-date.post" @@ -677,7 +676,10 @@ "inventory-storage.instances.item.get", "inventory-storage.instances.collection.get", "configuration.entries.item.get", - "configuration.entries.collection.get" + "configuration.entries.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation" ] } ] @@ -1663,6 +1665,9 @@ "proxiesfor.collection.get", "patron-notice.post", "configuration.entries.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation", "calendar.endpoint.dates.get", "pubsub.publish.post", "circulation-storage.loans-history.collection.get" @@ -1716,7 +1721,8 @@ "checkout-lock-storage.checkout-locks.item.delete", "mod-settings.entries.collection.get", "mod-settings.entries.item.get", - "mod-settings.global.read.mod-circulation" + "mod-settings.global.read.mod-circulation", + "mod-settings.global.read.circulation" ], "visible": false }, @@ -1777,7 +1783,10 @@ "actual-cost-fee-fine-cancel.post", "departments.item.get", "departments.collection.get", - "circulation-storage.loans-history.collection.get" + "circulation-storage.loans-history.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation" ], "visible": false }, @@ -1809,6 +1818,9 @@ "proxiesfor.collection.get", "calendar.endpoint.dates.get", "configuration.entries.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation", "scheduled-notice-storage.scheduled-notices.collection.delete", "scheduled-notice-storage.scheduled-notices.item.post", "patron-notice.post", @@ -1993,6 +2005,9 @@ "scheduled-notice-storage.scheduled-notices.collection.delete", "scheduled-notice-storage.scheduled-notices.item.post", "configuration.entries.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation", "manualblocks.collection.get", "pubsub.publish.post", "automated-patron-blocks.collection.get", @@ -2073,6 +2088,9 @@ "proxiesfor.collection.get", "patron-notice.post", "configuration.entries.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation", "scheduled-notice-storage.scheduled-notices.collection.delete", "scheduled-notice-storage.scheduled-notices.item.post", "pubsub.publish.post", @@ -2116,7 +2134,10 @@ "addresstypes.collection.get", "usergroups.collection.get", "usergroups.item.get", - "pubsub.publish.post" + "pubsub.publish.post", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation" ], "visible": false }, @@ -2151,6 +2172,9 @@ "patron-notice.post", "calendar.endpoint.dates.get", "configuration.entries.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation", "scheduled-notice-storage.scheduled-notices.collection.delete", "scheduled-notice-storage.scheduled-notices.item.post", "manualblocks.collection.get", @@ -2209,7 +2233,10 @@ "addresstypes.collection.get", "pubsub.publish.post", "patron-notice.post", - "circulation-storage.loans-history.collection.get" + "circulation-storage.loans-history.collection.get", + "mod-settings.entries.item.get", + "mod-settings.entries.collection.get", + "mod-settings.global.read.circulation" ], "visible": false }, diff --git a/src/main/java/org/folio/circulation/domain/MoveRequestService.java b/src/main/java/org/folio/circulation/domain/MoveRequestService.java index 3077016093..d13be99cb9 100644 --- a/src/main/java/org/folio/circulation/domain/MoveRequestService.java +++ b/src/main/java/org/folio/circulation/domain/MoveRequestService.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.validation.RequestLoanValidator; import org.folio.circulation.infrastructure.storage.ConfigurationRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.requests.RequestPolicyRepository; import org.folio.circulation.infrastructure.storage.requests.RequestQueueRepository; import org.folio.circulation.infrastructure.storage.requests.RequestRepository; @@ -27,13 +28,14 @@ public class MoveRequestService { private final ConfigurationRepository configurationRepository; private final EventPublisher eventPublisher; private final RequestQueueRepository requestQueueRepository; + private final SettingsRepository settingsRepository; private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); public MoveRequestService(RequestRepository requestRepository, RequestPolicyRepository requestPolicyRepository, UpdateUponRequest updateUponRequest, MoveRequestProcessAdapter moveRequestHelper, RequestLoanValidator requestLoanValidator, RequestNoticeSender requestNoticeSender, ConfigurationRepository configurationRepository, EventPublisher eventPublisher, - RequestQueueRepository requestQueueRepository) { + RequestQueueRepository requestQueueRepository, SettingsRepository settingsRepository) { this.requestRepository = requestRepository; this.requestPolicyRepository = requestPolicyRepository; @@ -44,11 +46,12 @@ public MoveRequestService(RequestRepository requestRepository, RequestPolicyRepo this.configurationRepository = configurationRepository; this.eventPublisher = eventPublisher; this.requestQueueRepository = requestQueueRepository; + this.settingsRepository = settingsRepository; } public CompletableFuture> moveRequest( RequestAndRelatedRecords requestAndRelatedRecords, Request originalRequest) { - return configurationRepository.lookupTlrSettings() + return settingsRepository.lookupTlrSettings() .thenApply(r -> r.map(requestAndRelatedRecords::withTlrSettings)) .thenApply(r -> r.next(RequestServiceUtility::refuseTlrProcessingWhenFeatureIsDisabled)) .thenApply(r -> r.next(records -> RequestServiceUtility.refuseMovingToOrFromHoldTlr(records, diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ConfigurationRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ConfigurationRepository.java index 52a974504d..339c280513 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ConfigurationRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ConfigurationRepository.java @@ -12,7 +12,6 @@ import org.folio.circulation.domain.ConfigurationService; import org.folio.circulation.domain.MultipleRecords; import org.folio.circulation.domain.anonymization.config.LoanAnonymizationConfiguration; -import org.folio.circulation.domain.configuration.TlrSettingsConfiguration; import org.folio.circulation.support.Clients; import org.folio.circulation.support.GetManyRecordsClient; import org.folio.circulation.support.http.client.CqlQuery; @@ -49,13 +48,6 @@ public CompletableFuture> lookupSessionTimeout() { return lookupConfigurations(otherSettingsQuery, applySessionTimeout()); } - public CompletableFuture> lookupTlrSettings() { - Result queryResult = defineModuleNameAndConfigNameFilter( - "SETTINGS", "TLR"); - - return findAndMapFirstConfiguration(queryResult, TlrSettingsConfiguration::from); - } - /** * Gets loan history tenant configuration - settings for loan anonymization * @@ -123,26 +115,4 @@ private Function, Integer> applySessionTimeout() .findSessionTimeout(configurations.getRecords()); } - /** - * Find first configuration and maps it to an object with a provided mapper - */ - private CompletableFuture> findAndMapFirstConfiguration( - Result cqlQueryResult, Function mapper) { - - return cqlQueryResult - .after(query -> configurationClient.getMany(query, DEFAULT_PAGE_LIMIT)) - .thenApply(result -> result.next(r -> from(r, Configuration::new, CONFIGS_KEY))) - .thenApply(result -> result.map(this::findFirstConfigurationAsJsonObject)) - .thenApply(result -> result.map(mapper)); - } - - private JsonObject findFirstConfigurationAsJsonObject( - MultipleRecords configurations) { - - return configurations.getRecords().stream() - .findFirst() - .map(Configuration::getValue) - .map(JsonObject::new) - .orElse(new JsonObject()); - } } diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SettingsRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SettingsRepository.java index 89f2e845ba..d5b2e32b84 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SettingsRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SettingsRepository.java @@ -6,6 +6,7 @@ import org.folio.circulation.domain.Configuration; import org.folio.circulation.domain.MultipleRecords; import org.folio.circulation.domain.configuration.CheckoutLockConfiguration; +import org.folio.circulation.domain.configuration.TlrSettingsConfiguration; import org.folio.circulation.support.Clients; import org.folio.circulation.support.GetManyRecordsClient; import org.folio.circulation.support.http.client.CqlQuery; @@ -13,9 +14,13 @@ import org.folio.circulation.support.results.Result; import java.lang.invoke.MethodHandles; +import java.util.Collection; +import java.util.List; import java.util.concurrent.CompletableFuture; +import static java.util.function.Function.identity; import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; +import static org.folio.circulation.support.http.client.CqlQuery.exactMatchAny; import static org.folio.circulation.support.results.Result.succeeded; public class SettingsRepository { @@ -27,14 +32,10 @@ public SettingsRepository(Clients clients) { } public CompletableFuture> lookUpCheckOutLockSettings() { + log.debug("lookUpCheckOutLockSettings:: fetching checkout lock settings"); try { - log.debug("lookUpCheckOutLockSettings:: fetching checkout lock settings"); - final Result moduleQuery = exactMatch("scope", "mod-circulation"); - final Result configNameQuery = exactMatch("key", "checkoutLockFeature"); - - return moduleQuery.combine(configNameQuery, CqlQuery::and) - .after(cqlQuery -> settingsClient.getMany(cqlQuery, PageLimit.noLimit())) - .thenApply(result -> result.next(response -> MultipleRecords.from(response, Configuration::new, "items"))) + return fetchSettings("mod-circulation", "checkoutLockFeature") + .thenApply(r -> r.map(records -> records.mapRecords(Configuration::new))) .thenApply(r -> r.map(r1 -> r1.getRecords().stream().findFirst() .map(Configuration::getValue) .map(JsonObject::new) @@ -49,4 +50,30 @@ public CompletableFuture> lookUpCheckOutLockSe return CompletableFuture.completedFuture(succeeded(CheckoutLockConfiguration.from(new JsonObject()))); } } + + public CompletableFuture> lookupTlrSettings() { + return fetchSettings("circulation", List.of("generalTlr", "regularTlr")) + .thenApply(r -> r.map(SettingsRepository::extractAndMergeValues)) + .thenApply(r -> r.map(TlrSettingsConfiguration::from)); + } + + private CompletableFuture>> fetchSettings(String scope, String key) { + return fetchSettings(scope, List.of(key)); + } + + private CompletableFuture>> fetchSettings(String scope, + Collection keys) { + + return exactMatch("scope", scope) + .combine(exactMatchAny("key", keys), CqlQuery::and) + .after(query -> settingsClient.getMany(query, PageLimit.noLimit())) + .thenApply(r -> r.next(response -> MultipleRecords.from(response, identity(), "items"))); + } + + private static JsonObject extractAndMergeValues(MultipleRecords entries) { + return entries.getRecords() + .stream() + .map(rec -> rec.getJsonObject("value")) + .reduce(new JsonObject(), JsonObject::mergeIn); + } } diff --git a/src/main/java/org/folio/circulation/resources/ChangeDueDateResource.java b/src/main/java/org/folio/circulation/resources/ChangeDueDateResource.java index ec49593d85..0e43fcbc24 100644 --- a/src/main/java/org/folio/circulation/resources/ChangeDueDateResource.java +++ b/src/main/java/org/folio/circulation/resources/ChangeDueDateResource.java @@ -3,7 +3,9 @@ import static java.util.concurrent.CompletableFuture.completedFuture; import static org.folio.circulation.domain.representations.ChangeDueDateRequest.DUE_DATE; import static org.folio.circulation.domain.representations.LoanProperties.ITEM_ID; -import static org.folio.circulation.resources.handlers.error.CirculationErrorType.*; +import static org.folio.circulation.resources.handlers.error.CirculationErrorType.FAILED_TO_FETCH_USER; +import static org.folio.circulation.resources.handlers.error.CirculationErrorType.FAILED_TO_FIND_SINGLE_OPEN_LOAN; +import static org.folio.circulation.resources.handlers.error.CirculationErrorType.ITEM_DOES_NOT_EXIST; import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError; import static org.folio.circulation.support.json.JsonPropertyFetcher.getDateTimeProperty; import static org.folio.circulation.support.results.MappingFunctions.toFixedValue; @@ -26,7 +28,7 @@ import org.folio.circulation.domain.representations.ChangeDueDateRequest; import org.folio.circulation.domain.validation.ItemStatusValidator; import org.folio.circulation.domain.validation.LoanValidator; -import org.folio.circulation.infrastructure.storage.ConfigurationRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.loans.OverdueFinePolicyRepository; @@ -80,6 +82,8 @@ private CompletableFuture> processChangeDueDate( final var itemRepository = new ItemRepository(clients); final var userRepository = new UserRepository(clients); final var loanRepository = new LoanRepository(clients, itemRepository, userRepository); + final var settingsRepository = new SettingsRepository(clients); + final WebContext webContext = new WebContext(routingContext); final OkapiPermissions okapiPermissions = OkapiPermissions.from(webContext.getHeaders()); @@ -103,13 +107,12 @@ private CompletableFuture> processChangeDueDate( final LoanNoticeSender loanNoticeSender = LoanNoticeSender.using(clients, loanRepository); - final ConfigurationRepository configurationRepository = new ConfigurationRepository(clients); log.info("starting change due date process for loan {}", request.getLoanId()); return succeeded(request) .after(r -> getExistingLoan(loanRepository, r)) .thenApply(LoanValidator::refuseWhenLoanIsClosed) .thenApply(this::toLoanAndRelatedRecords) - .thenComposeAsync(r -> r.combineAfter(configurationRepository::lookupTlrSettings, + .thenComposeAsync(r -> r.combineAfter(settingsRepository::lookupTlrSettings, LoanAndRelatedRecords::withTlrSettings)) .thenComposeAsync(r -> r.after(requestQueueRepository::get)) .thenApply(itemStatusValidator::refuseWhenItemStatusDoesNotAllowDueDateChange) diff --git a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java index a8e75f09ee..a63e63446b 100644 --- a/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java +++ b/src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java @@ -16,6 +16,7 @@ import org.folio.circulation.domain.representations.CheckInByBarcodeResponse; import org.folio.circulation.domain.validation.CheckInValidators; import org.folio.circulation.infrastructure.storage.ConfigurationRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.requests.RequestQueueRepository; @@ -78,6 +79,7 @@ private void checkIn(RoutingContext routingContext) { final RequestNoticeSender requestNoticeSender = RequestNoticeSender.using(clients); final ConfigurationRepository configurationRepository = new ConfigurationRepository(clients); + final SettingsRepository settingsRepository = new SettingsRepository(clients); refuseWhenLoggedInUserNotPresent(context) .next(notUsed -> checkInRequestResult) @@ -87,7 +89,7 @@ private void checkIn(RoutingContext routingContext) { .withItemStatusBeforeCheckIn(item.getStatus())) .thenApply(checkInValidators::refuseWhenItemIsNotAllowedForCheckIn) .thenApply(checkInValidators::refuseWhenClaimedReturnedIsNotResolved) - .thenComposeAsync(r -> r.combineAfter(configurationRepository::lookupTlrSettings, + .thenComposeAsync(r -> r.combineAfter(settingsRepository::lookupTlrSettings, CheckInContext::withTlrSettings)) .thenComposeAsync(r -> r.combineAfter(configurationRepository::findTimeZoneConfiguration, CheckInContext::withTimeZone)) diff --git a/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java b/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java index a4f804b70b..94e16b6721 100644 --- a/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java +++ b/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java @@ -154,7 +154,7 @@ private void checkOut(RoutingContext routingContext) { .thenApply(validators::refuseWhenItemIsAlreadyCheckedOut) .thenApply(validators::refuseWhenItemIsNotAllowedForCheckOut) .thenComposeAsync(validators::refuseWhenItemHasOpenLoans) - .thenComposeAsync(r -> r.combineAfter(configurationRepository::lookupTlrSettings, + .thenComposeAsync(r -> r.combineAfter(settingsRepository::lookupTlrSettings, LoanAndRelatedRecords::withTlrSettings)) .thenComposeAsync(r -> r.after(requestQueueRepository::get)) .thenCompose(validators::refuseWhenRequestedByAnotherPatron) diff --git a/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java b/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java index 43e78519e1..dc539a9097 100644 --- a/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java @@ -60,7 +60,7 @@ import org.folio.circulation.domain.validation.ProxyRelationshipValidator; import org.folio.circulation.domain.validation.RequestLoanValidator; import org.folio.circulation.domain.validation.ServicePointPickupLocationValidator; -import org.folio.circulation.infrastructure.storage.ConfigurationRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.requests.RequestQueueRepository; import org.folio.circulation.resources.handlers.error.FailFastErrorHandler; @@ -118,7 +118,7 @@ private void createInstanceLevelRequests(RoutingContext routingContext) { final var requestBody = routingContext.getBodyAsJson(); - new ConfigurationRepository(clients).lookupTlrSettings() + new SettingsRepository(clients).lookupTlrSettings() .thenCompose(r -> r.after(config -> buildAndPlaceRequests(clients, eventPublisher, repositories, itemFinder, config, requestBody))) .thenApply(r -> r.map(RequestAndRelatedRecords::getRequest)) diff --git a/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java b/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java index aab1681c96..3f19b9241d 100644 --- a/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java @@ -30,6 +30,7 @@ import org.folio.circulation.infrastructure.storage.CalendarRepository; import org.folio.circulation.infrastructure.storage.ConfigurationRepository; import org.folio.circulation.infrastructure.storage.ServicePointRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.LoanPolicyRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; @@ -272,6 +273,7 @@ void move(RoutingContext routingContext) { final var loanPolicyRepository = new LoanPolicyRepository(clients); final var requestPolicyRepository = new RequestPolicyRepository(clients); final var configurationRepository = new ConfigurationRepository(clients); + final var settingsRepository = new SettingsRepository(clients); final var updateUponRequest = new UpdateUponRequest(new UpdateItem(itemRepository, new RequestQueueService(requestPolicyRepository, loanPolicyRepository)), @@ -287,7 +289,7 @@ void move(RoutingContext routingContext) { requestRepository, requestPolicyRepository, updateUponRequest, moveRequestProcessAdapter, new RequestLoanValidator(new ItemByInstanceIdFinder(clients.holdingsStorage(), itemRepository), loanRepository), RequestNoticeSender.using(clients), configurationRepository, eventPublisher, - requestQueueRepository); + requestQueueRepository, settingsRepository); fromFutureResult(requestRepository.getById(id)) .map(request -> request.withOperation(Request.Operation.MOVE)) diff --git a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java index 255b015535..04bc011498 100644 --- a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java +++ b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java @@ -66,6 +66,7 @@ import org.folio.circulation.domain.validation.ServicePointPickupLocationValidator; import org.folio.circulation.infrastructure.storage.ConfigurationRepository; import org.folio.circulation.infrastructure.storage.ServicePointRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.inventory.HoldingsRepository; import org.folio.circulation.infrastructure.storage.inventory.InstanceRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; @@ -95,6 +96,7 @@ class RequestFromRepresentationService { private final LoanRepository loanRepository; private final ServicePointRepository servicePointRepository; private final ConfigurationRepository configurationRepository; + private final SettingsRepository settingsRepository; private final RequestPolicyRepository requestPolicyRepository; private final ProxyRelationshipValidator proxyRelationshipValidator; private final ServicePointPickupLocationValidator pickupLocationValidator; @@ -118,6 +120,7 @@ public RequestFromRepresentationService(Request.Operation operation, this.loanRepository = repositories.getLoanRepository(); this.servicePointRepository = repositories.getServicePointRepository(); this.configurationRepository = repositories.getConfigurationRepository(); + this.settingsRepository = repositories.getSettingsRepository(); this.requestPolicyRepository = repositories.getRequestPolicyRepository(); this.proxyRelationshipValidator = proxyRelationshipValidator; @@ -129,7 +132,7 @@ public RequestFromRepresentationService(Request.Operation operation, CompletableFuture> getRequestFrom(JsonObject representation) { - return configurationRepository.lookupTlrSettings() + return settingsRepository.lookupTlrSettings() .thenCompose(r -> r.after(tlrSettings -> initRequest(operation, tlrSettings, representation))) .thenApply(r -> r.next(this::validateStatus)) .thenApply(r -> r.next(this::validateRequestLevel)) diff --git a/src/main/java/org/folio/circulation/resources/RequestQueueResource.java b/src/main/java/org/folio/circulation/resources/RequestQueueResource.java index 3696483058..430684b83d 100644 --- a/src/main/java/org/folio/circulation/resources/RequestQueueResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestQueueResource.java @@ -27,6 +27,7 @@ import org.folio.circulation.infrastructure.storage.CalendarRepository; import org.folio.circulation.infrastructure.storage.ConfigurationRepository; import org.folio.circulation.infrastructure.storage.ServicePointRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.requests.RequestQueueRepository; @@ -125,6 +126,7 @@ private void reorderQueue(RoutingContext routingContext, RequestQueueType reques final var requestRepository = RequestRepository.using(clients, itemRepository, userRepository, loanRepository); final var configurationRepository = new ConfigurationRepository(clients); + final var settingsRepository = new SettingsRepository(clients); final var requestQueueRepository = new RequestQueueRepository(requestRepository); final UpdateRequestQueue updateRequestQueue = new UpdateRequestQueue( @@ -133,7 +135,7 @@ requestQueueRepository, requestRepository, new ServicePointRepository(clients), getRequestQueueByType(routingContext, requestQueueType, requestQueueRepository); - validateTlrFeatureStatus(configurationRepository, requestQueueType, idParamValue) + validateTlrFeatureStatus(settingsRepository, requestQueueType, idParamValue) .thenCompose(r -> r.after(tlrSettings -> getRequestQueueByType(routingContext, requestQueueType, requestQueueRepository))) .thenApply(r -> r.map(reorderContext::withRequestQueue)) @@ -152,10 +154,10 @@ requestQueueRepository, requestRepository, new ServicePointRepository(clients), } private CompletableFuture> validateTlrFeatureStatus( - ConfigurationRepository configurationRepository, RequestQueueType requestQueueType, + SettingsRepository settingsRepository, RequestQueueType requestQueueType, String idParamValue) { - return configurationRepository.lookupTlrSettings() + return settingsRepository.lookupTlrSettings() .thenApply(r -> r.failWhen( tlrSettings -> succeeded( requestQueueType == FOR_INSTANCE ^ tlrSettings.isTitleLevelRequestsFeatureEnabled()), diff --git a/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java b/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java index 4d60a026aa..e7e278c757 100644 --- a/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java +++ b/src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java @@ -70,6 +70,7 @@ import org.folio.circulation.infrastructure.storage.AutomatedPatronBlocksRepository; import org.folio.circulation.infrastructure.storage.CalendarRepository; import org.folio.circulation.infrastructure.storage.ConfigurationRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.feesandfines.FeeFineOwnerRepository; import org.folio.circulation.infrastructure.storage.feesandfines.FeeFineRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; @@ -150,6 +151,7 @@ private void renew(RoutingContext routingContext) { final LoanRepresentation loanRepresentation = new LoanRepresentation(); final ConfigurationRepository configurationRepository = new ConfigurationRepository(clients); + final SettingsRepository settingsRepository = new SettingsRepository(clients); final LoanScheduledNoticeService scheduledNoticeService = LoanScheduledNoticeService.using(clients); final ReminderFeeScheduledNoticeService scheduledRemindersService = new ReminderFeeScheduledNoticeService(clients); @@ -187,7 +189,7 @@ private void renew(RoutingContext routingContext) { .thenCompose(r -> r.after(ctx -> lookupOverdueFinePolicy(ctx, overdueFinePolicyRepository, errorHandler))) .thenComposeAsync(r -> r.after(ctx -> blockRenewalOfItemsWithReminderFees(ctx, errorHandler))) .thenCompose(r -> r.after(ctx -> lookupLoanPolicy(ctx, loanPolicyRepository, errorHandler))) - .thenCompose(r -> r.combineAfter(configurationRepository::lookupTlrSettings, + .thenCompose(r -> r.combineAfter(settingsRepository::lookupTlrSettings, RenewalContext::withTlrSettings)) .thenComposeAsync(r -> r.after( ctx -> lookupRequestQueue(ctx, requestQueueRepository, errorHandler))) diff --git a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java index cd59a6d6ea..dbff3faa44 100644 --- a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java +++ b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java @@ -41,8 +41,8 @@ import org.folio.circulation.domain.User; import org.folio.circulation.domain.configuration.TlrSettingsConfiguration; import org.folio.circulation.domain.policy.RequestPolicy; -import org.folio.circulation.infrastructure.storage.ConfigurationRepository; import org.folio.circulation.infrastructure.storage.ServicePointRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.inventory.InstanceRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.requests.RequestPolicyRepository; @@ -66,7 +66,7 @@ public class AllowedServicePointsService { private final RequestPolicyRepository requestPolicyRepository; private final ServicePointRepository servicePointRepository; private final ItemByInstanceIdFinder itemFinder; - private final ConfigurationRepository configurationRepository; + private final SettingsRepository settingsRepository; private final InstanceRepository instanceRepository; private final String indexName; @@ -76,7 +76,7 @@ public AllowedServicePointsService(Clients clients, boolean isEcsRequestRouting) requestRepository = new RequestRepository(clients); requestPolicyRepository = new RequestPolicyRepository(clients); servicePointRepository = new ServicePointRepository(clients); - configurationRepository = new ConfigurationRepository(clients); + settingsRepository = new SettingsRepository(clients); instanceRepository = new InstanceRepository(clients); itemFinder = new ItemByInstanceIdFinder(clients.holdingsStorage(), itemRepository); indexName = isEcsRequestRouting ? ECS_REQUEST_ROUTING_INDEX_NAME : PICKUP_LOCATION_INDEX_NAME; @@ -176,7 +176,7 @@ private CompletableFuture> fetchUser(AllowedServicePointsRequest re if (request.isForTitleLevelRequest() && request.getOperation() == CREATE) { log.info("getAllowedServicePointsForTitleWithNoItems:: checking TLR settings"); - return configurationRepository.lookupTlrSettings() + return settingsRepository.lookupTlrSettings() .thenCompose(r -> r.after(this::considerTlrSettings)); } diff --git a/src/main/java/org/folio/circulation/support/request/RequestRelatedRepositories.java b/src/main/java/org/folio/circulation/support/request/RequestRelatedRepositories.java index a7360d40c4..72b4c86b3c 100644 --- a/src/main/java/org/folio/circulation/support/request/RequestRelatedRepositories.java +++ b/src/main/java/org/folio/circulation/support/request/RequestRelatedRepositories.java @@ -2,6 +2,7 @@ import org.folio.circulation.infrastructure.storage.ConfigurationRepository; import org.folio.circulation.infrastructure.storage.ServicePointRepository; +import org.folio.circulation.infrastructure.storage.SettingsRepository; import org.folio.circulation.infrastructure.storage.inventory.HoldingsRepository; import org.folio.circulation.infrastructure.storage.inventory.InstanceRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; @@ -28,6 +29,7 @@ public class RequestRelatedRepositories { private RequestQueueRepository requestQueueRepository; private RequestPolicyRepository requestPolicyRepository; private ConfigurationRepository configurationRepository; + private SettingsRepository settingsRepository; private ServicePointRepository servicePointRepository; private LocationRepository locationRepository; @@ -43,6 +45,7 @@ public RequestRelatedRepositories(Clients clients) { requestQueueRepository = new RequestQueueRepository(requestRepository); requestPolicyRepository = new RequestPolicyRepository(clients); configurationRepository = new ConfigurationRepository(clients); + settingsRepository = new SettingsRepository(clients); servicePointRepository = new ServicePointRepository(clients); locationRepository = LocationRepository.using(clients); } diff --git a/src/test/java/api/loans/CheckInByBarcodeTests.java b/src/test/java/api/loans/CheckInByBarcodeTests.java index 6b8ee96495..c60cc189a3 100644 --- a/src/test/java/api/loans/CheckInByBarcodeTests.java +++ b/src/test/java/api/loans/CheckInByBarcodeTests.java @@ -1637,7 +1637,7 @@ void availableNoticeIsSentUponCheckInWhenRequesterBarcodeWasChanged() { @Test void linkItemToHoldTLRWithHoldShelfWhenCheckedInItemThenFulfilledWithSuccess(){ reconfigureTlrFeature(TlrFeatureStatus.NOT_CONFIGURED); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID instanceId = instancesFixture.basedUponDunkirk().getId(); IndividualResource defaultWithHoldings = holdingsFixture.defaultWithHoldings(instanceId); IndividualResource checkedOutItem = itemsClient.create(buildCheckedOutItemWithHoldingRecordsId(defaultWithHoldings.getId())); @@ -1665,7 +1665,7 @@ void linkItemToHoldTLRWithHoldShelfWhenCheckedInItemThenFulfilledWithSuccess(){ @Test void linkItemToHoldTLRWithDeliveryWhenCheckedInThenFulfilledWithSuccess(){ reconfigureTlrFeature(TlrFeatureStatus.NOT_CONFIGURED); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID instanceId = instancesFixture.basedUponDunkirk().getId(); IndividualResource defaultWithHoldings = holdingsFixture.defaultWithHoldings(instanceId); IndividualResource checkedOutItem = itemsClient.create(buildCheckedOutItemWithHoldingRecordsId(defaultWithHoldings.getId())); @@ -1691,7 +1691,7 @@ void linkItemToHoldTLRWithDeliveryWhenCheckedInThenFulfilledWithSuccess(){ @Test void requestsShouldChangePositionWhenTheyGoInFulfillmentOnCheckIn() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(3); ItemResource firstItem = items.get(0); @@ -1742,7 +1742,7 @@ void requestsShouldChangePositionWhenTheyGoInFulfillmentOnCheckIn() { @Test void canCheckinItemWhenRequestForAnotherItemOfSameInstanceExists() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource firstItem = items.get(0); @@ -1763,7 +1763,7 @@ void canCheckinItemWhenRequestForAnotherItemOfSameInstanceExists() { @Test void canFulFillRecallRequestWhenCheckInAnotherItemOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource firstItem = items.get(0); ItemResource secondItem = items.get(1); @@ -1789,7 +1789,7 @@ void canFulFillRecallRequestWhenCheckInAnotherItemOfSameInstance() { @Test void canFulFillRecallRequestWhenCheckInAnotherItemOfSameInstanceWithMultipleRecallRequests() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(3); ItemResource firstItem = items.get(0); ItemResource secondItem = items.get(1); @@ -1830,7 +1830,7 @@ void canFulFillRecallRequestWhenCheckInAnotherItemOfSameInstanceWithMultipleReca @Test void shouldNotLinkTitleLevelHoldRequestToAnItemUponCheckInWhenItemIsNonRequestable() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource item = itemsFixture.basedUponNod(); checkOutFixture.checkOutByBarcode(item, usersFixture.rebecca()); IndividualResource request = requestsFixture.placeTitleLevelRequest(HOLD, item.getInstanceId(), @@ -1860,7 +1860,7 @@ void shouldNotLinkTitleLevelHoldRequestToAnItemUponCheckInWhenItemIsNonRequestab @Test void shouldNotLinkTitleLevelHoldRequestToAnItemUponCheckInWhenItemIsNonLoanable() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource item = itemsFixture.basedUponNod(); checkOutFixture.checkOutByBarcode(item, usersFixture.rebecca()); IndividualResource request = requestsFixture.placeTitleLevelRequest(HOLD, item.getInstanceId(), @@ -1876,7 +1876,7 @@ void shouldNotLinkTitleLevelHoldRequestToAnItemUponCheckInWhenItemIsNonLoanable( @Test void shouldNotLinkTitleLevelRecallRequestToNewItemUponCheckInWhenItemIsNonRequestable() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID canCirculateLoanTypeId = loanTypesFixture.canCirculate().getId(); UUID readingRoomLoanTypeId = loanTypesFixture.readingRoom().getId(); @@ -1922,7 +1922,7 @@ void shouldNotLinkTitleLevelRecallRequestToNewItemUponCheckInWhenItemIsNonReques @Test void shouldNotLinkTitleLevelRecallRequestToNewItemUponCheckInWhenItemIsNonLoanable() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID canCirculateLoanTypeId = loanTypesFixture.canCirculate().getId(); UUID readingRoomLoanTypeId = loanTypesFixture.readingRoom().getId(); diff --git a/src/test/java/api/loans/CheckOutByBarcodeTests.java b/src/test/java/api/loans/CheckOutByBarcodeTests.java index a827a13001..b7107e04b3 100644 --- a/src/test/java/api/loans/CheckOutByBarcodeTests.java +++ b/src/test/java/api/loans/CheckOutByBarcodeTests.java @@ -2492,7 +2492,7 @@ void canCheckOutUsingAlternateCheckoutRollingLoanPolicy() { @ParameterizedTest @EnumSource(value = TlrFeatureStatus.class, names = {"DISABLED", "NOT_CONFIGURED"}) void titleLevelRequestIsIgnoredWhenTlrFeatureIsNotEnabled(TlrFeatureStatus tlrFeatureStatus) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource item = itemsFixture.basedUponNod(); UserResource borrower = usersFixture.steve(); @@ -2519,7 +2519,7 @@ void titleLevelRequestIsIgnoredWhenTlrFeatureIsNotEnabled(TlrFeatureStatus tlrFe "Title, Title" }) void canFulfilPageAndHoldRequestsWithMixedLevels(String pageRequestLevel, String holdRequestLevel) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource item = itemsFixture.basedUponNod(); UserResource firstRequester = usersFixture.steve(); @@ -2582,7 +2582,7 @@ void canFulfilPageAndHoldRequestsWithMixedLevels(String pageRequestLevel, String @Test void canCheckoutItemWhenTitleLevelPageRequestsExistForDifferentItemsOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(4); UUID instanceId = items.stream().findAny().orElseThrow().getInstanceId(); @@ -2613,7 +2613,7 @@ void canCheckoutItemWhenTitleLevelPageRequestsExistForDifferentItemsOfSameInstan void cannotCheckoutItemWhenTitleLevelPageRequestExistsForSameItem( String firstRequestLevel, String secondRequestLevel) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource randomItem = items.stream().findAny().orElseThrow(); diff --git a/src/test/java/api/loans/scenarios/ChangeDueDateAPITests.java b/src/test/java/api/loans/scenarios/ChangeDueDateAPITests.java index 9c84c684dc..509f1229d5 100644 --- a/src/test/java/api/loans/scenarios/ChangeDueDateAPITests.java +++ b/src/test/java/api/loans/scenarios/ChangeDueDateAPITests.java @@ -436,7 +436,7 @@ void dueDateChangeShouldNotUnsetRenewalFlagValueWhenTlrFeatureEnabled() { assertThat(recalledLoan.getJson().getBoolean("dueDateChangedByRecall"), equalTo(true)); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); requestsClient.create(new RequestBuilder() .recall() .titleRequestLevel() @@ -496,7 +496,7 @@ void dueDateChangeShouldUnsetRenewalFlagValueWhenTlrFeatureDisabledOrNotConfigur assertThat(recalledLoan.getJson().getBoolean("dueDateChangedByRecall"), equalTo(true)); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); requestsClient.create(new RequestBuilder() .recall() diff --git a/src/test/java/api/loans/scenarios/CheckoutWithRequestScenarioTests.java b/src/test/java/api/loans/scenarios/CheckoutWithRequestScenarioTests.java index 6332320521..efcd458121 100644 --- a/src/test/java/api/loans/scenarios/CheckoutWithRequestScenarioTests.java +++ b/src/test/java/api/loans/scenarios/CheckoutWithRequestScenarioTests.java @@ -165,7 +165,7 @@ void checkingOutWithHoldRequestAppliesAlternatePeriodAndScheduledForFixedPolicy( @Test void alternatePeriodShouldBeAppliedWhenRequestQueueContainsHoldTlr() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); var firstItem = items.get(0); var secondItem = items.get(1); @@ -245,7 +245,7 @@ void alternatePeriodShouldBeAppliedWhenRequestQueueContainsHoldTlr() { @Test void alternatePeriodShouldNotBeAppliedWhenRequestQueueContainsHoldIlrForDifferentItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); var firstItem = items.get(0); var secondItem = items.get(1); diff --git a/src/test/java/api/queue/RequestQueueResourceTest.java b/src/test/java/api/queue/RequestQueueResourceTest.java index 98d70164b0..04fb3cffa8 100644 --- a/src/test/java/api/queue/RequestQueueResourceTest.java +++ b/src/test/java/api/queue/RequestQueueResourceTest.java @@ -303,7 +303,7 @@ void shouldGetRequestQueueForItemSuccessfully() { @Test void shouldGetRequestQueueForInstanceSuccessfully() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID isbnIdentifierId = identifierTypesFixture.isbn().getId(); String isbnValue = "9780866989427"; diff --git a/src/test/java/api/requests/AllowedServicePointsAPITests.java b/src/test/java/api/requests/AllowedServicePointsAPITests.java index a2c54736dd..da9b9e83af 100644 --- a/src/test/java/api/requests/AllowedServicePointsAPITests.java +++ b/src/test/java/api/requests/AllowedServicePointsAPITests.java @@ -203,7 +203,7 @@ void shouldReturnListOfAllowedServicePointsForRequestReplacement( .map(UUID::fromString) .collect(Collectors.toSet())); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); var pickupLocationId = allowedSpByPolicy.stream() .findFirst() .map(AllowedServicePoint::getId) @@ -495,7 +495,7 @@ void allPickupLocationsAreReturnedForTitleLevelHoldWhenItIsDisabledAndInstanceHa boolean instanceHasHoldings) { // allow TLR-holds for instances with no holdings/items - configurationsFixture.configureTlrFeature(true, false, null, null, null); + settingsFixture.configureTlrFeature(true, false, null, null, null); IndividualResource sp1 = servicePointsFixture.cd1(); // pickup location IndividualResource sp2 = servicePointsFixture.cd2(); // pickup location @@ -516,7 +516,7 @@ void allPickupLocationsAreReturnedForTitleLevelHoldWhenItIsDisabledAndInstanceHa @Test void noAllowedServicePointsAreReturnedForTitleLevelHoldWhenItIsDisabledAndInstanceHasItems() { // allow TLR-holds for instances with no holdings/items - configurationsFixture.configureTlrFeature(true, false, null, null, null); + settingsFixture.configureTlrFeature(true, false, null, null, null); IndividualResource sp1 = servicePointsFixture.cd1(); // pickup location servicePointsFixture.cd2(); // pickup location @@ -675,7 +675,7 @@ void shouldReturnListOfAllowedServicePointsForRequestMove(RequestLevel requestLe setRequestPolicyWithAllowedServicePoints(PAGE, Set.of(sp1Uuid)); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); IndividualResource request = requestsFixture.place(new RequestBuilder() .withRequestType(PAGE.toString()) diff --git a/src/test/java/api/requests/RequestsAPICreationTests.java b/src/test/java/api/requests/RequestsAPICreationTests.java index ae49db5a36..4dcad03e2f 100644 --- a/src/test/java/api/requests/RequestsAPICreationTests.java +++ b/src/test/java/api/requests/RequestsAPICreationTests.java @@ -191,7 +191,7 @@ public class RequestsAPICreationTests extends APITests { @AfterEach public void afterEach() { mockClockManagerToReturnDefaultDateTime(); - configurationsFixture.deleteTlrFeatureConfig(); + settingsFixture.deleteTlrFeatureSettings(); } @Test @@ -470,7 +470,7 @@ void cannotCreateItemLevelRequestForUnknownInstance(String tlrFeatureStatus, @ParameterizedTest @CsvSource({"Page", "Hold", "Recall"}) void cannotCreateTitleLevelRequestForUnknownInstance(String requestType) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID patronId = usersFixture.charlotte().getId(); final UUID pickupServicePointId = servicePointsFixture.cd1().getId(); @@ -518,7 +518,7 @@ void cannotCreateTitleLevelRequestForUnknownInstance(String requestType) { }) void cannotCreateRequestForUnknownItem(String tlrFeatureEnabledString, String requestType) { if (Boolean.parseBoolean(tlrFeatureEnabledString)) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); } IndividualResource instance = instancesFixture.basedUponDunkirk(); @@ -606,7 +606,7 @@ void canCreateTitleLevelRequestWhenTlrEnabled() { final var items = itemsFixture.createMultipleItemsForTheSameInstance(2); UUID instanceId = items.get(0).getInstanceId(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); IndividualResource requestResource = requestsClient.create(new RequestBuilder() .page() @@ -633,7 +633,7 @@ void cannotCreateRequestWithNonExistentRequestLevelWhenTlrEnabled() { ItemResource item = itemsFixture.basedUponSmallAngryPlanet(); UUID instanceId = item.getInstanceId(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); Response postResponse = requestsClient.attemptCreate(new RequestBuilder() .recall() @@ -740,7 +740,7 @@ void cannotCreateTlrWhenUserAlreadyRequestedAnItemFromTheSameTitle() { @ParameterizedTest @EnumSource(value = RequestType.class, names = {"HOLD", "RECALL"}) void cannotCreateHoldTlrWhenAvailableItemForInstance(RequestType requestType) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource item = items.get(0); @@ -1374,7 +1374,7 @@ void canCreatePagedRequestWhenItemStatusIsAvailable() { void cannotCreateTitleLevelPagedRequestIfThereAreNoAvailableItems() { UUID patronId = usersFixture.charlotte().getId(); final UUID pickupServicePointId = servicePointsFixture.cd1().getId(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID instanceId = instancesFixture.basedUponDunkirk().getId(); IndividualResource defaultWithHoldings = holdingsFixture.defaultWithHoldings(instanceId); @@ -1406,7 +1406,7 @@ void canCreateTlrRecallWhenAvailableItemExistsButPageIsNotAllowedByPolicy() { overdueFinePoliciesFixture.facultyStandard().getId(), lostItemFeePoliciesFixture.facultyStandard().getId()); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final var items = itemsFixture.createMultipleItemsForTheSameInstance(2); var instanceId = items.get(0).getInstanceId(); @@ -1428,7 +1428,7 @@ void canCreateTlrRecallWhenAvailableItemExistsButPageIsNotAllowedByPolicy() { void canCreateTitleLevelPagedRequest() { UUID patronId = usersFixture.charlotte().getId(); final UUID pickupServicePointId = servicePointsFixture.cd1().getId(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); IndividualResource uponDunkirkInstance = instancesFixture.basedUponDunkirk(); UUID instanceId = uponDunkirkInstance.getId(); @@ -1460,7 +1460,7 @@ void canCreateTitleLevelPagedRequest() { void canHaveUserBarcodeInCheckInPublishedEventAfterTitleLevelRequest() { UUID patronId = usersFixture.charlotte().getId(); final UUID pickupServicePointId = servicePointsFixture.cd1().getId(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); IndividualResource uponDunkirkInstance = instancesFixture.basedUponDunkirk(); UUID instanceId = uponDunkirkInstance.getId(); @@ -1493,7 +1493,7 @@ void cannotCreateItemLevelRequestIfTitleLevelRequestForInstanceAlreadyCreated() UUID patronId = usersFixture.charlotte().getId(); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); buildItem(instanceId, "111"); requestsClient.create(buildPageTitleLevelRequest(patronId, pickupServicePointId, instanceId)); @@ -1515,7 +1515,7 @@ void cannotCreateTitleLevelRequestIfItemLevelRequestAlreadyCreated() { UUID patronId = usersFixture.charlotte().getId(); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); buildItem(instanceId, "111"); ItemResource secondItem = buildItem(instanceId, "222"); @@ -1537,7 +1537,7 @@ void canCreateItemLevelRequestAndTitleLevelRequestForDifferentInstances() { UUID patronId = usersFixture.charlotte().getId(); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId= UUID.randomUUID(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); buildItem(instanceId, "111"); requestsClient.create(buildPageTitleLevelRequest(patronId, pickupServicePointId, @@ -1557,7 +1557,7 @@ void cannotCreateTwoTitleLevelRequestsForSameInstance() { UUID userId = usersFixture.charlotte().getId(); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); buildItem(instanceId, "111"); buildItem(instanceId, "222"); @@ -1579,7 +1579,7 @@ void cannotCreateTwoItemLevelRequestsForSameItem() { UUID userId = usersFixture.charlotte().getId(); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource item = buildItem(instanceId, "111"); requestsClient.create(buildItemLevelRequest(userId, pickupServicePointId, @@ -1697,7 +1697,7 @@ void canCreateRecallRequestWhenItemIsCheckedOut() { @Test void tlrRecallShouldPickItemWithLoanWithNextClosestDueDateIfAnotherRecallRequestExists() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); var londonZoneId = ZoneId.of("Europe/London"); var items = itemsFixture.createMultipleItemsForTheSameInstance(3); var firstItem = items.get(0); @@ -1723,7 +1723,7 @@ void tlrRecallShouldPickItemWithLoanWithNextClosestDueDateIfAnotherRecallRequest @Test void tlrRecallShouldPickRecalledLoanWithClosestDueDateIfThereAreNoNotRecalledLoansAndSameAmountOfRecalls() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); var londonZoneId = ZoneId.of("Europe/London"); var items = itemsFixture.createMultipleItemsForTheSameInstance(4); var firstItem = items.get(0); @@ -1769,7 +1769,7 @@ void tlrRecallWithoutLoanShouldPickRecallableItemFromRequestedInstance(String it IndividualResource inTransitPickupServicePoint = servicePointsFixture.cd2(); UUID instanceId = instancesFixture.basedUponDunkirk().getId(); IndividualResource defaultWithHoldings = holdingsFixture.defaultWithHoldings(instanceId); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); if (itemStatus.equals("Paged")) { itemsClient.create(new ItemBuilder() .forHolding(defaultWithHoldings.getId()) @@ -1813,7 +1813,7 @@ void tlrRecallShouldFailWhenRequestHasNoLoanOrRecallableItem(String itemStatus) IndividualResource requestPickupServicePoint = servicePointsFixture.cd1(); UUID instanceId = instancesFixture.basedUponDunkirk().getId(); IndividualResource defaultWithHoldings = holdingsFixture.defaultWithHoldings(instanceId); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); itemsFixture.basedUponDunkirk(holdingBuilder -> holdingBuilder, instanceBuilder -> instanceBuilder.withId(instanceId), itemBuilder -> itemBuilder @@ -2074,7 +2074,7 @@ void canCreateHoldRequestWhenItemIsMissing() { "Lost and paid", "Paged", "In process (non-requestable)", "Intellectual item", "Unavailable", "Restricted", "Unknown", "Awaiting delivery", "Order closed"}) void canCreateTlrHoldRequestWhenInstanceHasItemsWithStatusAllowedForHold(String itemStatus) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final ItemResource item = itemsFixture.basedUponSmallAngryPlanet( builder -> builder.withStatus(itemStatus)); IndividualResource response = requestsFixture.placeTitleLevelHoldShelfRequest(item.getInstanceId(), @@ -3240,7 +3240,7 @@ void cannotCreateItemLevelRequestWithoutInstanceId() { @Test void cannotCreateTitleLevelRequestWithoutInstanceId() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource item = itemsFixture.basedUponNod(); @@ -3297,7 +3297,7 @@ void cannotCreateRequestWithItemIdButNoHoldingsRecordId(RequestLevel requestLeve @Test void recallTlrRequestShouldBeAppliedToLoanWithClosestDueDate() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); @@ -3372,7 +3372,7 @@ void recallTlrRequestShouldBeAppliedToLoanWithClosestDueDate() { void statusOfTlrRequestShouldBeChangedIfAssociatedItemCheckedIn() { UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource firstItem = buildItem(instanceId, "111"); ItemResource secondItem = buildItem(instanceId, "222"); @@ -3400,7 +3400,7 @@ void statusOfTlrRequestShouldBeChangedIfAssociatedItemCheckedIn() { @Test void awaitingPickupNoticeShouldBeSentDuringCheckInWhenItemCreatedAfterHoldTlr() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with available notice") @@ -3432,7 +3432,7 @@ void awaitingPickupNoticeShouldBeSentDuringCheckInWhenItemCreatedAfterHoldTlr() @Test void awaitingPickupNoticeShouldBeSentDuringCheckInWhenItemIsReturnedAndTlrHoldExists() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with available notice") @@ -3462,7 +3462,7 @@ void awaitingPickupNoticeShouldBeSentDuringCheckInWhenItemIsReturnedAndTlrHoldEx @Test void awaitingPickupNoticesShouldBeSentToMultiplePatronsDuringPagedItemsCheckIn() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); NoticePolicyBuilder noticePolicy = new NoticePolicyBuilder() .withName("Policy with available notice") .withLoanNotices(Collections.singletonList(new NoticeConfigurationBuilder() @@ -3578,7 +3578,7 @@ void pageRequestShouldNotBeCreatedIffulfillmentPreferenceIsNotValid(String fulfi @Test void itemCheckOutShouldNotAffectRequestAssociatedWithAnotherItemOfInstance() { UUID instanceId = UUID.randomUUID(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource firstItem = buildItem(instanceId, "111"); ItemResource secondItem = buildItem(instanceId, "222"); ZonedDateTime requestDate = ZonedDateTime.of(2021, 7, 22, 10, 22, 54, 0, UTC); @@ -3613,7 +3613,7 @@ void itemCheckOutShouldNotAffectRequestAssociatedWithAnotherItemOfInstance() { @Test void itemCheckOutRecallRequestCreationShouldProduceNotice() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); JsonObject recallToLoaneeConfiguration = new NoticeConfigurationBuilder() .withTemplateId(UUID.randomUUID()) .withEventType(NoticeEventType.ITEM_RECALLED.getRepresentation()) @@ -3656,7 +3656,7 @@ void itemCheckOutRecallRequestCreationShouldProduceNotice() { @Test void itemCheckOutRecallCancelAgainRecallRequestCreationShouldProduceNotice() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); JsonObject recallToLoaneeConfiguration = new NoticeConfigurationBuilder() .withTemplateId(UUID.randomUUID()) .withEventType(NoticeEventType.ITEM_RECALLED.getRepresentation()) @@ -3713,7 +3713,7 @@ private void verifyNumberOfNoticeEventsForUser(UUID userId, int expectedNoticeEv @ParameterizedTest @ValueSource(ints = {1, 2, 3, 4, 5}) void titleLevelPageRequestIsCreatedForItemClosestToPickupServicePoint(int testCase) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID pickupServicePointId = servicePointsFixture.create(new ServicePointBuilder( "Pickup service point", "PICKUP", "Display name") @@ -3821,7 +3821,7 @@ void pageTlrSucceedsWhenClosestAvailableItemIsNotPageable() { // pickup service point is not requestable. At the same time, other available and requestable // items exist. - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); circulationRulesFixture.updateCirculationRules(differentRequestPoliciesBasedOnMaterialType()); UUID pickupServicePointId = servicePointsFixture.create(new ServicePointBuilder( @@ -3920,7 +3920,7 @@ void holdTlrShouldSucceedWhenAvailableItemsExistButTheyAreNotPageable() { // Hold TLR should be created when available items of the same instance exist, but all of them // are not pageable due to the request policy - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); circulationRulesFixture.updateCirculationRules(differentRequestPoliciesBasedOnMaterialType()); IndividualResource instance = instancesFixture.basedUponDunkirk(); @@ -3965,7 +3965,7 @@ void holdAndRecallTlrShouldFailWhenAvailablePageableItemsExist(RequestType type) // Hold TLR should fail when available items of the same instance exist and some of those // items are pageable (request policy allows page requests) - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); circulationRulesFixture.updateCirculationRules(differentRequestPoliciesBasedOnMaterialType()); IndividualResource instance = instancesFixture.basedUponDunkirk(); @@ -4018,7 +4018,7 @@ void holdAndRecallTlrShouldFailWhenAvailablePageableItemsExist(RequestType type) @Test void recallTlrShouldNotBeCreatedForInstanceWithOnlyAgedToLostItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); @@ -4034,7 +4034,7 @@ void recallTlrShouldNotBeCreatedForInstanceWithOnlyAgedToLostItem() { @Test void recallTlrShouldBeCreatedForOnOrderItemIfInstanceHasOnOrderAndDeclaredLostItems() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); useLostItemPolicy(lostItemFeePoliciesFixture.chargeFee().getId()); UUID pickupServicePointId = servicePointsFixture.cd1().getId(); UUID instanceId = UUID.randomUUID(); @@ -4073,7 +4073,7 @@ void recallTlrShouldBeCreatedForOnOrderItemIfInstanceHasOnOrderAndDeclaredLostIt @Test void recallTlrShouldNotBeCreatedForInstanceWithOnlyDeclaredLostItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); useLostItemPolicy(lostItemFeePoliciesFixture.chargeFee().getId()); UUID instanceId = UUID.randomUUID(); @@ -4094,7 +4094,7 @@ void recallTlrShouldNotBeCreatedForInstanceWithOnlyDeclaredLostItem() { @Test void recallTlrShouldNotBeCreatedForInstanceWithOnlyClaimedReturnedItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); useLostItemPolicy(lostItemFeePoliciesFixture.chargeFee().getId()); UUID instanceId = UUID.randomUUID(); @@ -4115,7 +4115,7 @@ void recallTlrShouldNotBeCreatedForInstanceWithOnlyClaimedReturnedItem() { @Test void recallTlrShouldFailWhenNotAllowedByPolicy() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); circulationRulesFixture.updateCirculationRules(differentRequestPoliciesBasedOnMaterialType()); IndividualResource instance = instancesFixture.basedUponDunkirk(); @@ -4156,7 +4156,7 @@ void recallTlrShouldFailWhenNotAllowedByPolicy() { @Test void holdTlrShouldSucceedEvenWhenPolicyDoesNotAllowHolds() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); circulationRulesFixture.updateCirculationRules(differentRequestPoliciesBasedOnMaterialType()); IndividualResource instance = instancesFixture.basedUponDunkirk(); @@ -4198,7 +4198,7 @@ void holdTlrShouldSucceedEvenWhenPolicyDoesNotAllowHolds() { @Test void titleLevelHoldFailsWhenItShouldFollowCirculationRulesAndNoneOfInstanceItemsAreAllowedForHold() { // enable TLR feature and make Hold requests respect circulation rules - configurationsFixture.configureTlrFeature(true, true, null, null, null); + settingsFixture.configureTlrFeature(true, true, null, null, null); IndividualResource book = materialTypesFixture.book(); IndividualResource video = materialTypesFixture.videoRecording(); @@ -4226,7 +4226,7 @@ void titleLevelHoldFailsWhenItShouldFollowCirculationRulesAndNoneOfInstanceItems @Test void titleLevelHoldIsPlacedWhenItShouldFollowCirculationRulesAndOneOfInstanceItemsIsAllowedForHold() { // enable TLR feature and make Hold requests respect circulation rules - configurationsFixture.configureTlrFeature(true, true, null, null, null); + settingsFixture.configureTlrFeature(true, true, null, null, null); IndividualResource book = materialTypesFixture.book(); IndividualResource video = materialTypesFixture.videoRecording(); @@ -4250,7 +4250,7 @@ void titleLevelHoldIsPlacedWhenItShouldFollowCirculationRulesAndOneOfInstanceIte @Test void titleLevelHoldIsPlacedWhenItCanIgnoreCirculationRulesAndNoneOfInstanceItemsAreAllowedForHold() { // enable TLR feature and make Hold requests ignore circulation rules - configurationsFixture.configureTlrFeature(true, false, null, null, null); + settingsFixture.configureTlrFeature(true, false, null, null, null); IndividualResource book = materialTypesFixture.book(); IndividualResource video = materialTypesFixture.videoRecording(); @@ -4311,7 +4311,7 @@ void itemLevelRequestIsNotPlacedWhenRequestPolicyDisallowsRequestedPickupService void titleLevelRequestIsNotPlacedWhenRequestPolicyDisallowsRequestedPickupServicePoint( RequestType requestType) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final UUID requestPolicyId = UUID.randomUUID(); policiesActivation.use(new RequestPolicyBuilder( @@ -4607,7 +4607,7 @@ void canCreateHoldTlrForInstanceWithNoHoldingsRecords() { @Test void recallTlrShouldSucceedWhenItNeedsToPickLeastRecalledLoanAndRequestsWithNoLoansAreInTheQueueScenario1() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final var patron1 = usersFixture.charlotte(); final var patron2 = usersFixture.jessica(); @@ -4706,7 +4706,7 @@ void recallTlrShouldSucceedWhenItNeedsToPickLeastRecalledLoanAndRequestsWithNoLo @Test void recallTlrShouldSucceedWhenItNeedsToPickLeastRecalledLoanAndRequestsWithNoLoansAreInTheQueueScenario2() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final var patron1 = usersFixture.charlotte(); final var patron2 = usersFixture.jessica(); diff --git a/src/test/java/api/requests/RequestsAPILoanHistoryTests.java b/src/test/java/api/requests/RequestsAPILoanHistoryTests.java index 4767c605d4..85c129025a 100644 --- a/src/test/java/api/requests/RequestsAPILoanHistoryTests.java +++ b/src/test/java/api/requests/RequestsAPILoanHistoryTests.java @@ -45,7 +45,7 @@ void creatingRecallRequestChangesTheOpenLoanForTheSameItem() { @Test void checkOutShouldNotTruncateLoanIfRecallRequestExistsForAnotherItemOfTheSameInstanceIfTlrIsEnabled() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final var items = itemsFixture.createMultipleItemsForTheSameInstance(2); var steve = usersFixture.steve(); var charlotte = usersFixture.charlotte(); diff --git a/src/test/java/api/requests/RequestsAPILoanRenewalTests.java b/src/test/java/api/requests/RequestsAPILoanRenewalTests.java index 20434ac2be..5b82140520 100644 --- a/src/test/java/api/requests/RequestsAPILoanRenewalTests.java +++ b/src/test/java/api/requests/RequestsAPILoanRenewalTests.java @@ -302,7 +302,7 @@ void forbidRenewalLoanByIdWhenLoanProfileIsFixedFirstRequestInQueueIsHoldAndRene @Test void allowRenewalWhenFirstRequestInQueueIsItemLevelHoldForDifferentItemOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); loanPolicyWithRollingProfileAndRenewingIsForbiddenWhenHoldIsPending(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource itemForLoan = items.get(0); @@ -316,7 +316,7 @@ void allowRenewalWhenFirstRequestInQueueIsItemLevelHoldForDifferentItemOfSameIns @Test void allowRenewalWhenFirstRequestInQueueIsTitleLevelHoldForDifferentItemOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); loanPolicyWithRollingProfileAndRenewingIsForbiddenWhenHoldIsPending(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource itemForLoan = items.get(0); @@ -334,7 +334,7 @@ void allowRenewalWhenFirstRequestInQueueIsTitleLevelHoldForDifferentItemOfSameIn @Test void forbidRenewalWhenFirstRequestInQueueIsTitleLevelHoldWithoutItemId() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); loanPolicyWithRollingProfileAndRenewingIsForbiddenWhenHoldIsPending(); ItemResource item = itemsFixture.basedUponNod(); UserResource borrower = usersFixture.charlotte(); @@ -348,7 +348,7 @@ void forbidRenewalWhenFirstRequestInQueueIsTitleLevelHoldWithoutItemId() { @Test void alternateLoanPeriodIsNotUsedWhenFirstRequestInQueueIsItemLevelHoldForDifferentItemOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); useRollingPolicyWithRenewingAllowedForHoldingRequest(); // base loan period - 3 weeks, alternate - 4 weeks List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource itemForLoan = items.get(0); @@ -364,7 +364,7 @@ void alternateLoanPeriodIsNotUsedWhenFirstRequestInQueueIsItemLevelHoldForDiffer @Test void alternateLoanPeriodIsNotUsedForRenewalWhenFirstRequestInQueueIsTitleLevelHoldForDifferentItemOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); useRollingPolicyWithRenewingAllowedForHoldingRequest(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource itemForLoan = items.get(0); @@ -384,7 +384,7 @@ void alternateLoanPeriodIsNotUsedForRenewalWhenFirstRequestInQueueIsTitleLevelHo @Test void alternateLoanPeriodIsUsedForRenewalWhenFirstRequestInQueueIsTitleLevelHoldWithoutItemId() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); useRollingPolicyWithRenewingAllowedForHoldingRequest(); // base loan period - 3 weeks, alternate - 4 weeks ItemResource item = itemsFixture.basedUponNod(); UUID instanceId = item.getInstanceId(); @@ -400,7 +400,7 @@ void alternateLoanPeriodIsUsedForRenewalWhenFirstRequestInQueueIsTitleLevelHoldW @Test void forbidRenewalWhenTitleLevelRecallRequestExistsForSameItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource item = itemsFixture.basedUponNod(); UserResource borrower = usersFixture.james(); checkOutFixture.checkOutByBarcode(item, borrower); @@ -418,7 +418,7 @@ void forbidRenewalWhenTitleLevelRecallRequestExistsForSameItem() { @Test void allowRenewalWhenTitleLevelRecallRequestExistsForDifferentItemOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource itemForLoan = items.get(0); ItemResource itemForRequest = items.get(1); @@ -598,7 +598,7 @@ void forbidRenewalOverrideWhenRecallIsForDifferentItemOfSameInstance() { @Test void forbidRenewalOverrideWhenTitleLevelRecallRequestExistsForDifferentItemOfSameInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); List items = itemsFixture.createMultipleItemsForTheSameInstance(2); ItemResource itemForLoan = items.get(0); ItemResource itemForRequest = items.get(1); diff --git a/src/test/java/api/requests/RequestsAPIRetrievalTests.java b/src/test/java/api/requests/RequestsAPIRetrievalTests.java index 6605d72546..17fdb28469 100644 --- a/src/test/java/api/requests/RequestsAPIRetrievalTests.java +++ b/src/test/java/api/requests/RequestsAPIRetrievalTests.java @@ -244,7 +244,7 @@ void canGetARequestById() { @Test void titleLevelRequestRetrievalById() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); UUID isbnIdentifierId = identifierTypesFixture.isbn().getId(); String isbnValue = "9780866989427"; diff --git a/src/test/java/api/requests/RequestsAPIUpdatingTests.java b/src/test/java/api/requests/RequestsAPIUpdatingTests.java index a0483f1712..ba382cb4f7 100644 --- a/src/test/java/api/requests/RequestsAPIUpdatingTests.java +++ b/src/test/java/api/requests/RequestsAPIUpdatingTests.java @@ -866,7 +866,7 @@ void shouldUseCurrentUserIdAsSourceWhenSendingUpdateRequestMessage() { @Test void editingRecallTlrShouldNotChangeRecalledItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); IndividualResource patron1 = usersFixture.steve(); IndividualResource patron2 = usersFixture.rebecca(); diff --git a/src/test/java/api/requests/scenarios/HoldShelfFulfillmentTests.java b/src/test/java/api/requests/scenarios/HoldShelfFulfillmentTests.java index ede4fbf442..91dceb4c8e 100644 --- a/src/test/java/api/requests/scenarios/HoldShelfFulfillmentTests.java +++ b/src/test/java/api/requests/scenarios/HoldShelfFulfillmentTests.java @@ -24,8 +24,6 @@ import java.time.ZonedDateTime; import java.util.List; import java.util.UUID; -import java.util.stream.Collectors; -import java.util.stream.IntStream; import org.folio.circulation.support.http.client.Response; import org.folio.circulation.support.utils.ClockUtil; @@ -38,14 +36,13 @@ import api.support.APITests; import api.support.TlrFeatureStatus; import api.support.builders.CheckInByBarcodeRequestBuilder; -import api.support.builders.InstanceBuilder; import api.support.http.IndividualResource; import api.support.http.ItemResource; class HoldShelfFulfillmentTests extends APITests { @AfterEach public void afterEach() { - configurationsFixture.deleteTlrFeatureConfig(); + settingsFixture.deleteTlrFeatureSettings(); } @ParameterizedTest @@ -84,7 +81,7 @@ void itemIsReadyForPickUpWhenCheckedInAtPickupServicePoint(TlrFeatureStatus tlrF @ParameterizedTest @ValueSource(ints = {1, 2}) void tlrRequestIsPositionedCorrectlyInUnifiedQueue(int checkedInItemNumber) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final IndividualResource pickupServicePoint = servicePointsFixture.cd1(); @@ -222,7 +219,7 @@ void canBeCheckedOutToRequestingPatronWhenReadyForPickup(TlrFeatureStatus tlrFea @Test void canBeCheckedOutToPatronRequestingTitleWhenReadyForPickup() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final IndividualResource pickupServicePoint = servicePointsFixture.cd1(); @@ -288,7 +285,7 @@ void checkInAtDifferentServicePointPlacesItemInTransit(TlrFeatureStatus tlrFeatu @Test void checkInItemWithTlrRequestAtDifferentServicePointPlacesItemInTransit() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final IndividualResource pickupServicePoint = servicePointsFixture.cd1(); final IndividualResource checkInServicePoint = servicePointsFixture.cd2(); @@ -363,7 +360,7 @@ void canBeCheckedOutToRequestingPatronWhenInTransit(TlrFeatureStatus tlrFeatureS @Test void canCheckoutItemForTitleLevelRequestWhenInTransit() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final IndividualResource pickupServicePoint = servicePointsFixture.cd1(); final IndividualResource checkInServicePoint = servicePointsFixture.cd2(); @@ -430,7 +427,7 @@ void itemIsReadyForPickUpWhenCheckedInAtPickupServicePointAfterTransit(TlrFeatur @Test void itemWithTlrRequestIsReadyForPickUpWhenCheckedInAtPickupServicePointAfterTransit() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final IndividualResource pickupServicePoint = servicePointsFixture.cd1(); final IndividualResource checkInServicePoint = servicePointsFixture.cd2(); @@ -508,7 +505,7 @@ void cannotCheckOutToOtherPatronWhenRequestIsAwaitingPickup(TlrFeatureStatus tlr @Test void cannotCheckOutToOtherPatronWhenTlrRequestIsAwaitingPickup() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); ItemResource smallAngryPlanet = itemsFixture.basedUponSmallAngryPlanet(); IndividualResource james = usersFixture.james(); @@ -580,7 +577,7 @@ void cannotCheckOutToOtherPatronWhenRequestIsInTransitForPickup(TlrFeatureStatus @Test void cannotCheckOutToOtherPatronWhenTlrRequestIsInTransitForPickup() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); final IndividualResource requestServicePoint = servicePointsFixture.cd1(); final IndividualResource checkInServicePoint = servicePointsFixture.cd2(); diff --git a/src/test/java/api/requests/scenarios/MoveRequestTests.java b/src/test/java/api/requests/scenarios/MoveRequestTests.java index 7a53868f15..2f9a30856c 100644 --- a/src/test/java/api/requests/scenarios/MoveRequestTests.java +++ b/src/test/java/api/requests/scenarios/MoveRequestTests.java @@ -184,7 +184,7 @@ void canMoveRequestFromOneItemCopyToAnother() { @Test void itemShouldRemainPagedIfHoldCreatedAfterRequestHasBeenMovedToAnotherItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val items = itemsFixture.createMultipleItemsForTheSameInstance(2); val firstItem = items.get(0); @@ -208,7 +208,7 @@ void itemShouldRemainPagedIfHoldCreatedAfterRequestHasBeenMovedToAnotherItem() { @Test void canMovePageTlrToAvailableItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val firstItem = itemsFixture.basedUponSmallAngryPlanet("89809"); val pageIlrForFirstItem = requestsFixture.placeTitleLevelPageRequest(firstItem.getInstanceId(), @@ -227,7 +227,7 @@ void canMovePageTlrToAvailableItem() { @Test void canMovePageTlrToRecall() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val firstItem = itemsFixture.basedUponSmallAngryPlanet("89809"); val pageTlrForFirstItem = requestsFixture.placeTitleLevelPageRequest(firstItem.getInstanceId(), usersFixture.james()); @@ -249,7 +249,7 @@ void canMovePageTlrToRecall() { @Test void canMoveRecallTlrToAnotherItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val items = itemsFixture.createMultipleItemsForTheSameInstance(2); val firstItem = items.get(0); @@ -269,7 +269,7 @@ void canMoveRecallTlrToAnotherItem() { @Test void canMoveRecallTlrToPage() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val items = itemsFixture.createMultipleItemsForTheSameInstance(2); val firstItem = items.get(0); @@ -289,7 +289,7 @@ void canMoveRecallTlrToPage() { @Test void whenRequestIsMovedItemShouldBecomeAvailableIfThereAreNoRequestsInTheQueueForThisItemIfTlrIsEnabled() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val items = itemsFixture.createMultipleItemsForTheSameInstance(2); val firstItem = items.get(0); @@ -308,7 +308,7 @@ void whenRequestIsMovedItemShouldBecomeAvailableIfThereAreNoRequestsInTheQueueFo @Test void whenRequestIsMovedPositionsShouldBeConsistentWhenTlrIsEnabled() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val items = itemsFixture.createMultipleItemsForTheSameInstance(3); @@ -369,7 +369,7 @@ void whenRequestIsMovedPositionsShouldBeConsistentWhenTlrIsEnabled() { @Test void cannotMoveRequestToAnItemFromDifferentInstance() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val nod = itemsFixture.basedUponNod(); val uprooted = itemsFixture.basedUponUprooted(); @@ -387,7 +387,7 @@ void cannotMoveRequestToAnItemFromDifferentInstance() { @Test void cannotMoveToOrFromHoldTlr() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val items = itemsFixture.createMultipleItemsForTheSameInstance(2); val firstItem = items.get(0); @@ -416,7 +416,7 @@ void cannotMoveToOrFromHoldTlr() { @Test void cannotMoveTlrToTheSameItem() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val item = itemsFixture.basedUponNod(); val jessica = usersFixture.jessica(); @@ -435,7 +435,7 @@ void cannotMoveTlrToTheSameItem() { @Test void cannotMoveTlrWhenFeatureIsDisabled() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); val items = itemsFixture.createMultipleItemsForTheSameInstance(2); val firstItem = items.get(0); @@ -446,7 +446,7 @@ void cannotMoveTlrWhenFeatureIsDisabled() { val firstItemHoldTlr = requestsFixture.placeTitleLevelHoldShelfRequest(firstItem.getInstanceId(), usersFixture.james()); - configurationsFixture.disableTlrFeature(); + settingsFixture.disableTlrFeature(); Response response = requestsFixture.attemptMove(new MoveRequestBuilder(firstItemHoldTlr.getId(), secondItem.getId())); diff --git a/src/test/java/api/support/APITests.java b/src/test/java/api/support/APITests.java index ddc2dce289..6e3493f683 100644 --- a/src/test/java/api/support/APITests.java +++ b/src/test/java/api/support/APITests.java @@ -24,6 +24,16 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; +import org.junit.Assert; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.testcontainers.containers.KafkaContainer; +import org.testcontainers.utility.DockerImageName; + +import api.support.fakes.FakeModNotify; +import api.support.fakes.FakePubSub; +import api.support.fakes.FakeStorageModule; import api.support.fixtures.AddInfoFixture; import api.support.fixtures.AddressTypesFixture; import api.support.fixtures.AgeToLostFixture; @@ -36,7 +46,6 @@ import api.support.fixtures.CirculationItemsFixture; import api.support.fixtures.CirculationRulesFixture; import api.support.fixtures.ClaimItemReturnedFixture; -import api.support.fixtures.ConfigurationsFixture; import api.support.fixtures.DeclareLostFixtures; import api.support.fixtures.DepartmentFixture; import api.support.fixtures.EndPatronSessionClient; @@ -70,16 +79,6 @@ import api.support.fixtures.TenantActivationFixture; import api.support.fixtures.UserManualBlocksFixture; import api.support.fixtures.UsersFixture; -import org.junit.Assert; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; -import org.testcontainers.containers.KafkaContainer; -import org.testcontainers.utility.DockerImageName; - -import api.support.fakes.FakeModNotify; -import api.support.fakes.FakePubSub; -import api.support.fakes.FakeStorageModule; import api.support.fixtures.policies.PoliciesActivationFixture; import api.support.http.IndividualResource; import api.support.http.ResourceClient; @@ -232,9 +231,6 @@ public abstract class APITests { protected final AddressTypesFixture addressTypesFixture = new AddressTypesFixture(ResourceClient.forAddressTypes()); - protected final ConfigurationsFixture configurationsFixture = - new ConfigurationsFixture(configClient); - protected final PatronGroupsFixture patronGroupsFixture = new PatronGroupsFixture(patronGroupsClient); @@ -446,13 +442,13 @@ protected void mockClockManagerToReturnDefaultDateTime() { protected void reconfigureTlrFeature(TlrFeatureStatus tlrFeatureStatus) { if (tlrFeatureStatus == TlrFeatureStatus.ENABLED) { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); } else if (tlrFeatureStatus == TlrFeatureStatus.DISABLED) { - configurationsFixture.disableTlrFeature(); + settingsFixture.disableTlrFeature(); } else { - configurationsFixture.deleteTlrFeatureConfig(); + settingsFixture.deleteTlrFeatureSettings(); } } @@ -468,15 +464,15 @@ protected void reconfigureTlrFeature(TlrFeatureStatus tlrFeatureStatus, UUID cancellationTemplateId, UUID expirationTemplateId) { if (tlrFeatureStatus == TlrFeatureStatus.ENABLED) { - configurationsFixture.configureTlrFeature(true, tlrHoldShouldFollowCirculationRules, + settingsFixture.configureTlrFeature(true, tlrHoldShouldFollowCirculationRules, confirmationTemplateId, cancellationTemplateId, expirationTemplateId); } else if (tlrFeatureStatus == TlrFeatureStatus.DISABLED) { - configurationsFixture.configureTlrFeature(false, tlrHoldShouldFollowCirculationRules, + settingsFixture.configureTlrFeature(false, tlrHoldShouldFollowCirculationRules, confirmationTemplateId, cancellationTemplateId, expirationTemplateId); } else { - configurationsFixture.deleteTlrFeatureConfig(); + settingsFixture.deleteTlrFeatureSettings(); } } diff --git a/src/test/java/api/support/builders/SettingsBuilder.java b/src/test/java/api/support/builders/SettingsBuilder.java index dd954c07aa..380b52fef2 100644 --- a/src/test/java/api/support/builders/SettingsBuilder.java +++ b/src/test/java/api/support/builders/SettingsBuilder.java @@ -8,7 +8,7 @@ public class SettingsBuilder implements Builder { private final JsonObject representation; - public SettingsBuilder(UUID id, String scope, String key, String value) { + public SettingsBuilder(UUID id, String scope, String key, Object value) { this.representation = new JsonObject() .put("id", id) .put("scope", scope) diff --git a/src/test/java/api/support/fakes/FakeOkapi.java b/src/test/java/api/support/fakes/FakeOkapi.java index 9cdfb40600..548d5e78d3 100644 --- a/src/test/java/api/support/fakes/FakeOkapi.java +++ b/src/test/java/api/support/fakes/FakeOkapi.java @@ -406,7 +406,6 @@ public void start(Promise startFuture) throws IOException { .withRootPath("/settings/entries") .withCollectionPropertyName("items") .withChangeMetadata() - .withRecordConstraint(this::userHasAlreadyAcquiredLock) .create().register(router); new FakeStorageModuleBuilder() diff --git a/src/test/java/api/support/fixtures/ConfigurationExample.java b/src/test/java/api/support/fixtures/ConfigurationExample.java index 5f7b9c7011..e06ac904c9 100644 --- a/src/test/java/api/support/fixtures/ConfigurationExample.java +++ b/src/test/java/api/support/fixtures/ConfigurationExample.java @@ -2,10 +2,7 @@ import static org.folio.circulation.support.json.JsonPropertyWriter.write; -import java.util.UUID; - import api.support.builders.ConfigRecordBuilder; -import api.support.builders.TlrSettingsConfigurationBuilder; import io.vertx.core.json.JsonObject; public class ConfigurationExample { @@ -40,37 +37,6 @@ public static ConfigRecordBuilder schedulerNoticesLimitConfiguration(String limi DEFAULT_NOTIFICATION_SCHEDULER_CONFIG_NAME, limit); } - public static ConfigRecordBuilder tlrFeatureEnabled() { - return new ConfigRecordBuilder("SETTINGS", "TLR", - new TlrSettingsConfigurationBuilder() - .withTitleLevelRequestsFeatureEnabled(true) - .create() - .encodePrettily()); - } - - public static ConfigRecordBuilder tlrFeatureDisabled() { - return new ConfigRecordBuilder("SETTINGS", "TLR", - new TlrSettingsConfigurationBuilder() - .withTitleLevelRequestsFeatureEnabled(false) - .create() - .encodePrettily()); - } - - public static ConfigRecordBuilder tlrFeatureConfiguration(boolean isTlrEnabled, - boolean holdShouldFollowCirculationRules, UUID confirmationTemplateId, - UUID cancellationTemplateId, UUID expirationTemplateId) { - - return new ConfigRecordBuilder("SETTINGS", "TLR", - new TlrSettingsConfigurationBuilder() - .withTitleLevelRequestsFeatureEnabled(isTlrEnabled) - .withTlrHoldShouldFollowCirculationRules(holdShouldFollowCirculationRules) - .withConfirmationPatronNoticeTemplateId(confirmationTemplateId) - .withCancellationPatronNoticeTemplateId(cancellationTemplateId) - .withExpirationPatronNoticeTemplateId(expirationTemplateId) - .create() - .encodePrettily()); - } - private static JsonObject combinedTimeZoneConfig(String timezone) { final JsonObject encodedValue = new JsonObject(); write(encodedValue, "locale", US_LOCALE); diff --git a/src/test/java/api/support/fixtures/ConfigurationsFixture.java b/src/test/java/api/support/fixtures/ConfigurationsFixture.java deleted file mode 100644 index f42e903052..0000000000 --- a/src/test/java/api/support/fixtures/ConfigurationsFixture.java +++ /dev/null @@ -1,41 +0,0 @@ -package api.support.fixtures; - -import java.util.UUID; - -import api.support.http.ResourceClient; - -public class ConfigurationsFixture { - private final ResourceClient client; - private UUID tlrConfigurationEntryId = null; - - public ConfigurationsFixture(ResourceClient client) { - this.client = client; - } - - public void enableTlrFeature() { - deleteTlrFeatureConfig(); - tlrConfigurationEntryId = client.create(ConfigurationExample.tlrFeatureEnabled()).getId(); - } - - public void disableTlrFeature() { - deleteTlrFeatureConfig(); - tlrConfigurationEntryId = client.create(ConfigurationExample.tlrFeatureDisabled()).getId(); - } - - public void deleteTlrFeatureConfig() { - if (tlrConfigurationEntryId != null) { - client.delete(tlrConfigurationEntryId); - tlrConfigurationEntryId = null; - } - } - - public void configureTlrFeature(boolean isTlrFeatureEnabled, boolean tlrHoldShouldFollowCirculationRules, - UUID confirmationTemplateId, UUID cancellationTemplateId, UUID expirationTemplateId) { - - deleteTlrFeatureConfig(); - tlrConfigurationEntryId = client.create(ConfigurationExample.tlrFeatureConfiguration( - isTlrFeatureEnabled, tlrHoldShouldFollowCirculationRules, confirmationTemplateId, - cancellationTemplateId, expirationTemplateId)) - .getId(); - } -} diff --git a/src/test/java/api/support/fixtures/SettingsFixture.java b/src/test/java/api/support/fixtures/SettingsFixture.java index 23166fcf83..41220f073d 100644 --- a/src/test/java/api/support/fixtures/SettingsFixture.java +++ b/src/test/java/api/support/fixtures/SettingsFixture.java @@ -1,12 +1,15 @@ package api.support.fixtures; +import java.util.List; +import java.util.UUID; + import api.support.builders.SettingsBuilder; import api.support.http.ResourceClient; import io.vertx.core.json.JsonObject; -import java.util.UUID; - public class SettingsFixture { + private static final UUID GENERAL_TLR_SETTINGS_ID = UUID.randomUUID(); + private static final UUID REGULAR_TLR_SETTINGS_ID = UUID.randomUUID(); private final ResourceClient settingsClient; @@ -27,4 +30,58 @@ private SettingsBuilder buildCheckoutLockFeatureSettings(boolean checkoutFeature .encodePrettily() ); } + + public void enableTlrFeature() { + createGeneralTlrSettings(true, false); + } + + public void disableTlrFeature() { + createGeneralTlrSettings(false, false); + } + + public void deleteTlrFeatureSettings() { + settingsClient.delete(GENERAL_TLR_SETTINGS_ID); + settingsClient.delete(REGULAR_TLR_SETTINGS_ID); + } + + public void configureTlrFeature(boolean isTlrFeatureEnabled, boolean tlrHoldShouldFollowCirculationRules, + UUID confirmationTemplateId, UUID cancellationTemplateId, UUID expirationTemplateId) { + + deleteTlrFeatureSettings(); + createGeneralTlrSettings(isTlrFeatureEnabled, tlrHoldShouldFollowCirculationRules); + createRegularTlrSettings(confirmationTemplateId, cancellationTemplateId, expirationTemplateId); + } + + private void createGeneralTlrSettings(boolean isTlrFeatureEnabled, + boolean tlrHoldShouldFollowCirculationRules) { + + JsonObject value = new JsonObject() + .put("titleLevelRequestsFeatureEnabled", isTlrFeatureEnabled) + .put("createTitleLevelRequestsByDefault", false) + .put("tlrHoldShouldFollowCirculationRules", tlrHoldShouldFollowCirculationRules); + + SettingsBuilder builder = new SettingsBuilder(GENERAL_TLR_SETTINGS_ID, "circulation", + "generalTlr", value); + + settingsClient.create(builder); + } + + private void createRegularTlrSettings(UUID confirmationTemplateId, UUID cancellationTemplateId, + UUID expirationTemplateId) { + + JsonObject regularTlrValue = new JsonObject() + .put("cancellationPatronNoticeTemplateId", cancellationTemplateId) + .put("confirmationPatronNoticeTemplateId", confirmationTemplateId) + .put("expirationPatronNoticeTemplateId", expirationTemplateId); + + SettingsBuilder builder = new SettingsBuilder(REGULAR_TLR_SETTINGS_ID, "circulation", + "regularTlr", regularTlrValue); + + settingsClient.create(builder); + } + + public List getAll() { + return settingsClient.getAll(); + } + } From 3167b2027627875836a608f341023342b89061c7 Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Tue, 30 Apr 2024 18:37:57 +0500 Subject: [PATCH 05/27] CIRC-2072 Create a facade for instance search --- descriptors/ModuleDescriptor-template.json | 15 ++++ .../circulation/CirculationVerticle.java | 2 + .../circulation/domain/InstanceExtended.java | 56 +++++++++++++++ .../org/folio/circulation/domain/Item.java | 11 +++ .../ItemSummaryRepresentation.java | 1 + .../storage/SearchRepository.java | 69 +++++++++++++++++++ .../resources/ItemsByInstanceResource.java | 43 ++++++++++++ .../folio/circulation/support/Clients.java | 12 ++++ 8 files changed, 209 insertions(+) create mode 100644 src/main/java/org/folio/circulation/domain/InstanceExtended.java create mode 100644 src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java create mode 100644 src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index b4641a9048..14a15aca2b 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -166,6 +166,21 @@ } ] }, + { + "id": "instance-items", + "version": "0.1", + "handlers": [ + { + "methods": ["GET"], + "pathPattern": "/circulation/items-by-instance/{id}", + "permissionsRequired": [ + ], + "modulePermissions": [ + "search.instances.collection.get" + ] + } + ] + }, { "id": "add-info", "version": "0.1", diff --git a/src/main/java/org/folio/circulation/CirculationVerticle.java b/src/main/java/org/folio/circulation/CirculationVerticle.java index cd6960808b..e106a7691e 100644 --- a/src/main/java/org/folio/circulation/CirculationVerticle.java +++ b/src/main/java/org/folio/circulation/CirculationVerticle.java @@ -20,6 +20,7 @@ import org.folio.circulation.resources.FeeFineNotRealTimeScheduledNoticeProcessingResource; import org.folio.circulation.resources.FeeFineScheduledNoticeProcessingResource; import org.folio.circulation.resources.HealthResource; +import org.folio.circulation.resources.ItemsByInstanceResource; import org.folio.circulation.resources.ItemsInTransitResource; import org.folio.circulation.resources.LoanAnonymizationResource; import org.folio.circulation.resources.LoanCirculationRulesEngineResource; @@ -92,6 +93,7 @@ public void start(Promise startFuture) { new RequestCollectionResource(client).register(router); new RequestQueueResource(client).register(router); new RequestByInstanceIdResource(client).register(router); + new ItemsByInstanceResource(client).register(router); new RequestHoldShelfClearanceResource( "/circulation/requests-reports/hold-shelf-clearance/:servicePointId", client) diff --git a/src/main/java/org/folio/circulation/domain/InstanceExtended.java b/src/main/java/org/folio/circulation/domain/InstanceExtended.java new file mode 100644 index 0000000000..fca63fda6b --- /dev/null +++ b/src/main/java/org/folio/circulation/domain/InstanceExtended.java @@ -0,0 +1,56 @@ +package org.folio.circulation.domain; + +import io.vertx.core.json.JsonArray; +import io.vertx.core.json.JsonObject; +import lombok.NonNull; +import lombok.ToString; +import lombok.Value; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.circulation.domain.representations.ItemSummaryRepresentation; +import org.folio.circulation.storage.mappers.ItemMapper; + +import java.lang.invoke.MethodHandles; +import java.util.Collection; +import java.util.List; + +import static org.folio.circulation.support.json.JsonObjectArrayPropertyFetcher.mapToList; +import static org.folio.circulation.support.json.JsonPropertyWriter.write; + +@Value +@ToString(onlyExplicitlyIncluded = true) +public class InstanceExtended { + + private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + + JsonObject representation; + + @ToString.Include + String id; + @ToString.Include + String title; + + @NonNull Collection items; + + public static InstanceExtended from(JsonObject representation) { + return new InstanceExtended(representation, representation.getString("id"), + representation.getString("title"), mapItems(representation)); + } + + private static List mapItems(JsonObject representation) { + return mapToList(representation, "items", new ItemMapper()::toDomain); + } + + public InstanceExtended changeItems(Collection items) { + JsonArray itemsArray = new JsonArray(); + for (Item item : items) { + itemsArray.add(new ItemSummaryRepresentation().createItemSummary(item)); + } + write(representation, "items", itemsArray); + return new InstanceExtended(representation, id, title, items); + } + + public JsonObject toJson() { + return representation; + } +} diff --git a/src/main/java/org/folio/circulation/domain/Item.java b/src/main/java/org/folio/circulation/domain/Item.java index c27a296ef1..6ad60b2932 100644 --- a/src/main/java/org/folio/circulation/domain/Item.java +++ b/src/main/java/org/folio/circulation/domain/Item.java @@ -415,4 +415,15 @@ public String getLendingLibraryCode() { public String getDcbItemTitle() { return getProperty(itemRepresentation, "instanceTitle"); } + + public String getTenantId() { + return getProperty(itemRepresentation, "tenantId"); + } + + public Item changeTenantId(String tenantId) { + if(tenantId != null) { + write(itemRepresentation, "tenantId", tenantId); + } + return this; + } } diff --git a/src/main/java/org/folio/circulation/domain/representations/ItemSummaryRepresentation.java b/src/main/java/org/folio/circulation/domain/representations/ItemSummaryRepresentation.java index efb1db2699..3e6c89dde3 100644 --- a/src/main/java/org/folio/circulation/domain/representations/ItemSummaryRepresentation.java +++ b/src/main/java/org/folio/circulation/domain/representations/ItemSummaryRepresentation.java @@ -44,6 +44,7 @@ public JsonObject createItemSummary(Item item) { write(itemSummary, "copyNumber", item.getCopyNumber()); write(itemSummary, CALL_NUMBER_COMPONENTS, createCallNumberComponents(item.getCallNumberComponents())); + write(itemSummary, "tenantId", item.getTenantId()); JsonObject status = new JsonObject() .put("name", item.getStatus().getValue()); diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java new file mode 100644 index 0000000000..49153d2b4a --- /dev/null +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -0,0 +1,69 @@ +package org.folio.circulation.infrastructure.storage; + +import lombok.AllArgsConstructor; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.circulation.domain.InstanceExtended; +import org.folio.circulation.domain.Item; +import org.folio.circulation.domain.MultipleRecords; +import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; +import org.folio.circulation.support.Clients; +import org.folio.circulation.support.CollectionResourceClient; +import org.folio.circulation.support.http.client.Response; +import org.folio.circulation.support.results.Result; + +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; + +import static org.folio.circulation.support.results.ResultBinding.flatMapResult; +import static org.folio.circulation.support.results.ResultBinding.mapResult; + +@AllArgsConstructor +public class SearchRepository { + + private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + + private final ItemRepository itemRepository; + private final CollectionResourceClient searchClient; + + + public SearchRepository(Clients clients) { + this(new ItemRepository(clients), clients.searchClient()); + } + + public CompletableFuture> getInstanceWithItems(String instanceId) { + return searchClient.getManyWithQueryStringParameters(Map.of("expandAll", + "true", "query", String.format("id==%s", instanceId))) + .thenApply(flatMapResult(this::mapResponseToInstances)) + .thenApply(mapResult(MultipleRecords::firstOrNull)) + .thenCompose(r -> r.map(this::updateItemDetails) + .orElse(CompletableFuture.completedFuture(null))); + } + + private Result> mapResponseToInstances(Response response) { + return MultipleRecords.from(response, InstanceExtended::from, "instances"); + } + + private CompletableFuture> updateItemDetails(InstanceExtended searchInstance) { + List> futures = new ArrayList<>(); + List updatedItems = new ArrayList<>(); + + searchInstance.getItems().forEach(item -> { + var tenantId = item.getTenantId(); + CompletableFuture updateFuture = itemRepository.fetchById(item.getItemId()) + .thenCompose(itemRepository::fetchItemRelatedRecords) + .thenAccept(updatedItem -> { + synchronized (updatedItems) { + updatedItems.add(updatedItem.value().changeTenantId(tenantId)); + } + }); + futures.add(updateFuture); + }); + + return CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])) + .thenApply(v -> Result.of(() -> searchInstance.changeItems(updatedItems))); + } +} diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java new file mode 100644 index 0000000000..75f5435a38 --- /dev/null +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -0,0 +1,43 @@ +package org.folio.circulation.resources; + +import io.vertx.core.http.HttpClient; +import io.vertx.ext.web.Router; +import io.vertx.ext.web.RoutingContext; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.circulation.domain.InstanceExtended; +import org.folio.circulation.infrastructure.storage.SearchRepository; +import org.folio.circulation.support.Clients; +import org.folio.circulation.support.RouteRegistration; +import org.folio.circulation.support.http.server.JsonHttpResponse; +import org.folio.circulation.support.http.server.WebContext; + +import java.lang.invoke.MethodHandles; + +public class ItemsByInstanceResource extends Resource { + + private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + public ItemsByInstanceResource(HttpClient client) { + super(client); + } + + @Override + public void register(Router router) { + RouteRegistration routeRegistration = new RouteRegistration( + "/circulation/items-by-instance", router); + + routeRegistration.get(this::getInstanceItems); + } + + private void getInstanceItems(RoutingContext routingContext) { + final WebContext context = new WebContext(routingContext); + final Clients clients = Clients.create(context, client); + String instanceId = routingContext.pathParam("id"); + log.debug("getInstanceItems:: instanceId: " + instanceId); + final var searchRepository = new SearchRepository(clients); + searchRepository.getInstanceWithItems(instanceId) + .thenApply(r -> r.map(InstanceExtended::toJson)) + .thenApply(r -> r.map(JsonHttpResponse::ok)) + .thenAccept(context::writeResultToHttpResponse); + } +} diff --git a/src/main/java/org/folio/circulation/support/Clients.java b/src/main/java/org/folio/circulation/support/Clients.java index 3ffc41941a..bf7dc48056 100644 --- a/src/main/java/org/folio/circulation/support/Clients.java +++ b/src/main/java/org/folio/circulation/support/Clients.java @@ -68,6 +68,7 @@ public class Clients { private final CollectionResourceClient departmentClient; private final CollectionResourceClient checkOutLockStorageClient; private final CollectionResourceClient circulationItemClient; + private final CollectionResourceClient searchClient; private final GetManyRecordsClient settingsStorageClient; public static Clients create(WebContext context, HttpClient httpClient) { @@ -136,6 +137,7 @@ private Clients(OkapiHttpClient client, WebContext context) { checkOutLockStorageClient = createCheckoutLockClient(client, context); settingsStorageClient = createSettingsStorageClient(client, context); circulationItemClient = createCirculationItemClient(client, context); + searchClient = createSearchClient(client, context); } catch(MalformedURLException e) { throw new InvalidOkapiLocationException(context.getOkapiLocation(), e); @@ -374,6 +376,10 @@ public CollectionResourceClient circulationItemClient() { return circulationItemClient; } + public CollectionResourceClient searchClient() { + return searchClient; + } + private static CollectionResourceClient getCollectionResourceClient( OkapiHttpClient client, WebContext context, String path) @@ -801,6 +807,12 @@ private CollectionResourceClient createCirculationItemClient( return getCollectionResourceClient(client, context, "/circulation-item"); } + private CollectionResourceClient createSearchClient( + OkapiHttpClient client, WebContext context) throws MalformedURLException { + + return getCollectionResourceClient(client, context, "/search/instances"); + } + private GetManyRecordsClient createSettingsStorageClient( OkapiHttpClient client, WebContext context) throws MalformedURLException { From 6676cde5347f61bd1aaa3cc845bd6b317b2e0ea9 Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Thu, 2 May 2024 17:03:50 +0500 Subject: [PATCH 06/27] CIRC-2072 Create a facade for instance search --- .../folio/circulation/resources/ItemsByInstanceResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index 75f5435a38..607ca3c907 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -33,7 +33,7 @@ private void getInstanceItems(RoutingContext routingContext) { final WebContext context = new WebContext(routingContext); final Clients clients = Clients.create(context, client); String instanceId = routingContext.pathParam("id"); - log.debug("getInstanceItems:: instanceId: " + instanceId); + log.debug("getInstanceItems:: instanceId {}", instanceId); final var searchRepository = new SearchRepository(clients); searchRepository.getInstanceWithItems(instanceId) .thenApply(r -> r.map(InstanceExtended::toJson)) From 1c1cd46046693a569300395066471238eaeacefb Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Mon, 13 May 2024 18:18:32 +0500 Subject: [PATCH 07/27] CIRC-2072 Added API test --- .../circulation/domain/InstanceExtended.java | 8 ++--- .../resources/ItemsByInstanceResource.java | 9 +++++- .../java/api/ItemsByInstanceResourceTest.java | 24 +++++++++++++++ src/test/java/api/support/APITests.java | 2 ++ .../api/support/builders/SearchBuilder.java | 29 +++++++++++++++++++ .../java/api/support/fakes/FakeOkapi.java | 6 ++++ .../api/support/fakes/FakeStorageModule.java | 3 +- .../api/support/fixtures/SearchFixture.java | 22 ++++++++++++++ .../java/api/support/http/InterfaceUrls.java | 7 +++++ .../java/api/support/http/ResourceClient.java | 4 +++ 10 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 src/test/java/api/ItemsByInstanceResourceTest.java create mode 100644 src/test/java/api/support/builders/SearchBuilder.java create mode 100644 src/test/java/api/support/fixtures/SearchFixture.java diff --git a/src/main/java/org/folio/circulation/domain/InstanceExtended.java b/src/main/java/org/folio/circulation/domain/InstanceExtended.java index fca63fda6b..d159d2b6a0 100644 --- a/src/main/java/org/folio/circulation/domain/InstanceExtended.java +++ b/src/main/java/org/folio/circulation/domain/InstanceExtended.java @@ -25,16 +25,12 @@ public class InstanceExtended { JsonObject representation; - @ToString.Include String id; - @ToString.Include - String title; @NonNull Collection items; public static InstanceExtended from(JsonObject representation) { - return new InstanceExtended(representation, representation.getString("id"), - representation.getString("title"), mapItems(representation)); + return new InstanceExtended(representation, representation.getString("id"), mapItems(representation)); } private static List mapItems(JsonObject representation) { @@ -47,7 +43,7 @@ public InstanceExtended changeItems(Collection items) { itemsArray.add(new ItemSummaryRepresentation().createItemSummary(item)); } write(representation, "items", itemsArray); - return new InstanceExtended(representation, id, title, items); + return new InstanceExtended(representation, id, items); } public JsonObject toJson() { diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index 607ca3c907..73eea2a62c 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -1,6 +1,7 @@ package org.folio.circulation.resources; import io.vertx.core.http.HttpClient; +import io.vertx.core.json.JsonObject; import io.vertx.ext.web.Router; import io.vertx.ext.web.RoutingContext; import org.apache.logging.log4j.LogManager; @@ -36,8 +37,14 @@ private void getInstanceItems(RoutingContext routingContext) { log.debug("getInstanceItems:: instanceId {}", instanceId); final var searchRepository = new SearchRepository(clients); searchRepository.getInstanceWithItems(instanceId) - .thenApply(r -> r.map(InstanceExtended::toJson)) + .thenApply(r -> r.map(this::toJson)) .thenApply(r -> r.map(JsonHttpResponse::ok)) .thenAccept(context::writeResultToHttpResponse); } + + private JsonObject toJson(InstanceExtended instanceExtended) { + if (instanceExtended != null) + return instanceExtended.toJson(); + return new JsonObject(); + } } diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java new file mode 100644 index 0000000000..222b301426 --- /dev/null +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -0,0 +1,24 @@ +package api; + +import api.support.APITests; +import org.folio.circulation.support.http.client.Response; +import org.junit.jupiter.api.Test; +import java.util.UUID; +import static api.support.http.InterfaceUrls.itemsByInstanceUrl; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; + +public class ItemsByInstanceResourceTest extends APITests { + @Test + void canGetInstanceById() { + UUID instanceId = UUID.randomUUID(); + searchFixture.basedUponDunkirk(instanceId); + Response response = get(instanceId.toString(), 200); + assertThat(response.getStatusCode(), is(200)); + } + + private Response get(String instanceId, int expectedStatusCode) { + return restAssuredClient.get(itemsByInstanceUrl(instanceId), expectedStatusCode, + "items-by-instance-request"); + } +} diff --git a/src/test/java/api/support/APITests.java b/src/test/java/api/support/APITests.java index 6e3493f683..09858b2693 100644 --- a/src/test/java/api/support/APITests.java +++ b/src/test/java/api/support/APITests.java @@ -24,6 +24,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; +import api.support.fixtures.SearchFixture; import org.junit.Assert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -296,6 +297,7 @@ public abstract class APITests { protected final DepartmentFixture departmentFixture = new DepartmentFixture(); protected final CheckOutLockFixture checkOutLockFixture = new CheckOutLockFixture(); protected final SettingsFixture settingsFixture = new SettingsFixture(); + protected final SearchFixture searchFixture = new SearchFixture(); protected APITests() { this(true, false); diff --git a/src/test/java/api/support/builders/SearchBuilder.java b/src/test/java/api/support/builders/SearchBuilder.java new file mode 100644 index 0000000000..5856973650 --- /dev/null +++ b/src/test/java/api/support/builders/SearchBuilder.java @@ -0,0 +1,29 @@ +package api.support.builders; + +import io.vertx.core.json.JsonObject; +import java.util.List; + +public class SearchBuilder extends JsonBuilder implements Builder { + private final int totalRecords; + private final JsonObject instance; + + public SearchBuilder(JsonObject instance) { + this.totalRecords = 1; + this.instance = instance; + } + + @Override + public JsonObject create() { + final JsonObject search = new JsonObject(); + + put(search, "totalRecords", totalRecords); + put(search, "instances", List.of(instance)); + + return search; + } + + public SearchBuilder withItems(List items) { + put(instance, "items", items); + return new SearchBuilder(instance); + } +} diff --git a/src/test/java/api/support/fakes/FakeOkapi.java b/src/test/java/api/support/fakes/FakeOkapi.java index 548d5e78d3..b8d720227a 100644 --- a/src/test/java/api/support/fakes/FakeOkapi.java +++ b/src/test/java/api/support/fakes/FakeOkapi.java @@ -414,6 +414,12 @@ public void start(Promise startFuture) throws IOException { .withChangeMetadata() .create().register(router); + new FakeStorageModuleBuilder() + .withRootPath("/search/instances") + .withCollectionPropertyName("instances") + .withChangeMetadata() + .create().register(router); + new FakeFeeFineOperationsModule().register(router); server.requestHandler(router) diff --git a/src/test/java/api/support/fakes/FakeStorageModule.java b/src/test/java/api/support/fakes/FakeStorageModule.java index a3cf6cdff1..baaaad21e2 100644 --- a/src/test/java/api/support/fakes/FakeStorageModule.java +++ b/src/test/java/api/support/fakes/FakeStorageModule.java @@ -631,7 +631,8 @@ private void checkForUnexpectedQueryParameters(RoutingContext routingContext) { boolean isValidParameter = queryParameter.contains("query") || queryParameter.contains("offset") || isContainsQueryParameter(queryParameter) || - queryParameter.contains("limit"); + queryParameter.contains("limit") || + queryParameter.contains("expandAll"); return !isValidParameter; }) diff --git a/src/test/java/api/support/fixtures/SearchFixture.java b/src/test/java/api/support/fixtures/SearchFixture.java new file mode 100644 index 0000000000..533c5e4f47 --- /dev/null +++ b/src/test/java/api/support/fixtures/SearchFixture.java @@ -0,0 +1,22 @@ +package api.support.fixtures; + +import api.support.builders.InstanceBuilder; +import api.support.builders.SearchBuilder; +import api.support.http.ResourceClient; +import java.util.List; +import java.util.UUID; + +public class SearchFixture { + + private final ResourceClient searchClient; + + public SearchFixture() { + this.searchClient = ResourceClient.forSearchClient(); + } + + public void basedUponDunkirk(UUID instanceId) { + SearchBuilder builder = new SearchBuilder(new InstanceBuilder("Dunkirk", + UUID.randomUUID()).withId(instanceId).create()).withItems(List.of(ItemExamples.basedUponDunkirk(UUID.randomUUID(), UUID.randomUUID()).create())); + searchClient.create(builder); + } +} diff --git a/src/test/java/api/support/http/InterfaceUrls.java b/src/test/java/api/support/http/InterfaceUrls.java index fedf39d696..7399b0c542 100644 --- a/src/test/java/api/support/http/InterfaceUrls.java +++ b/src/test/java/api/support/http/InterfaceUrls.java @@ -334,4 +334,11 @@ public static URL settingsStorageUrl() { return APITestContext.viaOkapiModuleUrl("/settings/entries"); } + public static URL searchUrl(String subPath) { + return APITestContext.viaOkapiModuleUrl("/search/instances" + subPath); + } + + public static URL itemsByInstanceUrl(String subPath) { + return circulationModuleUrl("/circulation/items-by-instance/" + subPath); + } } diff --git a/src/test/java/api/support/http/ResourceClient.java b/src/test/java/api/support/http/ResourceClient.java index 844fff542d..d4a713d491 100644 --- a/src/test/java/api/support/http/ResourceClient.java +++ b/src/test/java/api/support/http/ResourceClient.java @@ -272,6 +272,10 @@ public static ResourceClient forActualCostRecordsStorage() { return new ResourceClient(InterfaceUrls::actualCostRecordsStorageUrl, "actualCostRecords"); } + public static ResourceClient forSearchClient() { + return new ResourceClient(InterfaceUrls::searchUrl, "instances"); + } + private ResourceClient(UrlMaker urlMaker, String collectionArrayPropertyName) { this.urlMaker = urlMaker; this.collectionArrayPropertyName = collectionArrayPropertyName; From 6797c1b58aaebae94d3e7b917b4c5e9c535f1bee Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Wed, 15 May 2024 17:55:15 +0500 Subject: [PATCH 08/27] CIRC-2072 Added API test --- .../storage/SearchRepository.java | 37 ++++++++----------- .../java/api/support/fakes/FakeOkapi.java | 1 + 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 49153d2b4a..21647db11d 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -7,17 +7,15 @@ import org.folio.circulation.domain.Item; import org.folio.circulation.domain.MultipleRecords; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; +import org.folio.circulation.support.AsyncCoordinationUtil; import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; import org.folio.circulation.support.http.client.Response; import org.folio.circulation.support.results.Result; - import java.lang.invoke.MethodHandles; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; - +import static org.folio.circulation.support.results.Result.emptyAsync; import static org.folio.circulation.support.results.ResultBinding.flatMapResult; import static org.folio.circulation.support.results.ResultBinding.mapResult; @@ -35,12 +33,12 @@ public SearchRepository(Clients clients) { } public CompletableFuture> getInstanceWithItems(String instanceId) { + log.debug("getInstanceWithItems:: instanceId {}", instanceId); return searchClient.getManyWithQueryStringParameters(Map.of("expandAll", "true", "query", String.format("id==%s", instanceId))) .thenApply(flatMapResult(this::mapResponseToInstances)) .thenApply(mapResult(MultipleRecords::firstOrNull)) - .thenCompose(r -> r.map(this::updateItemDetails) - .orElse(CompletableFuture.completedFuture(null))); + .thenCompose(r -> r.after(this::updateItemDetails)); } private Result> mapResponseToInstances(Response response) { @@ -48,22 +46,17 @@ private Result> mapResponseToInstances(Respons } private CompletableFuture> updateItemDetails(InstanceExtended searchInstance) { - List> futures = new ArrayList<>(); - List updatedItems = new ArrayList<>(); - - searchInstance.getItems().forEach(item -> { - var tenantId = item.getTenantId(); - CompletableFuture updateFuture = itemRepository.fetchById(item.getItemId()) - .thenCompose(itemRepository::fetchItemRelatedRecords) - .thenAccept(updatedItem -> { - synchronized (updatedItems) { - updatedItems.add(updatedItem.value().changeTenantId(tenantId)); - } - }); - futures.add(updateFuture); - }); + log.debug("updateItemDetails:: searchInstance {}", searchInstance); + if (searchInstance == null) { + return emptyAsync(); + } + return AsyncCoordinationUtil.allOf(searchInstance.getItems(), this::fetchItemDetails) + .thenApply(r -> r.map(searchInstance::changeItems)); + } - return CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])) - .thenApply(v -> Result.of(() -> searchInstance.changeItems(updatedItems))); + private CompletableFuture> fetchItemDetails(Item searchItem) { + return itemRepository.fetchById(searchItem.getItemId()) + .thenComposeAsync(itemRepository::fetchItemRelatedRecords) + .thenApply(r -> r.map(item -> item.changeTenantId(searchItem.getTenantId()))); } } diff --git a/src/test/java/api/support/fakes/FakeOkapi.java b/src/test/java/api/support/fakes/FakeOkapi.java index b8d720227a..a2c78074ff 100644 --- a/src/test/java/api/support/fakes/FakeOkapi.java +++ b/src/test/java/api/support/fakes/FakeOkapi.java @@ -418,6 +418,7 @@ public void start(Promise startFuture) throws IOException { .withRootPath("/search/instances") .withCollectionPropertyName("instances") .withChangeMetadata() + .withQueryParameters("query", "expandAll") .create().register(router); new FakeFeeFineOperationsModule().register(router); From eac3e54eb412d9819198f9d44a53b56e848df6aa Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Mon, 20 May 2024 15:36:40 +0500 Subject: [PATCH 09/27] CIRC-2072 Create a facade for instance search --- descriptors/ModuleDescriptor-template.json | 2 +- ...tanceExtended.java => SearchInstance.java} | 10 +- .../storage/SearchRepository.java | 16 +-- .../resources/ItemsByInstanceResource.java | 29 +++-- .../java/api/ItemsByInstanceResourceTest.java | 8 +- src/test/java/api/support/APITests.java | 4 +- .../api/support/builders/SearchBuilder.java | 29 ----- .../builders/SearchInstanceBuilder.java | 22 ++++ .../java/api/support/fakes/FakeOkapi.java | 7 +- .../api/support/fakes/FakeSearchModule.java | 100 ++++++++++++++++++ ...ixture.java => SearchInstanceFixture.java} | 8 +- .../java/api/support/http/InterfaceUrls.java | 2 +- 12 files changed, 169 insertions(+), 68 deletions(-) rename src/main/java/org/folio/circulation/domain/{InstanceExtended.java => SearchInstance.java} (79%) delete mode 100644 src/test/java/api/support/builders/SearchBuilder.java create mode 100644 src/test/java/api/support/builders/SearchInstanceBuilder.java create mode 100644 src/test/java/api/support/fakes/FakeSearchModule.java rename src/test/java/api/support/fixtures/{SearchFixture.java => SearchInstanceFixture.java} (70%) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 14a15aca2b..b180fdbef3 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -172,7 +172,7 @@ "handlers": [ { "methods": ["GET"], - "pathPattern": "/circulation/items-by-instance/{id}", + "pathPattern": "/circulation/items-by-instance", "permissionsRequired": [ ], "modulePermissions": [ diff --git a/src/main/java/org/folio/circulation/domain/InstanceExtended.java b/src/main/java/org/folio/circulation/domain/SearchInstance.java similarity index 79% rename from src/main/java/org/folio/circulation/domain/InstanceExtended.java rename to src/main/java/org/folio/circulation/domain/SearchInstance.java index d159d2b6a0..3f82792380 100644 --- a/src/main/java/org/folio/circulation/domain/InstanceExtended.java +++ b/src/main/java/org/folio/circulation/domain/SearchInstance.java @@ -19,7 +19,7 @@ @Value @ToString(onlyExplicitlyIncluded = true) -public class InstanceExtended { +public class SearchInstance { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); @@ -29,21 +29,21 @@ public class InstanceExtended { @NonNull Collection items; - public static InstanceExtended from(JsonObject representation) { - return new InstanceExtended(representation, representation.getString("id"), mapItems(representation)); + public static SearchInstance from(JsonObject representation) { + return new SearchInstance(representation, representation.getString("id"), mapItems(representation)); } private static List mapItems(JsonObject representation) { return mapToList(representation, "items", new ItemMapper()::toDomain); } - public InstanceExtended changeItems(Collection items) { + public SearchInstance changeItems(Collection items) { JsonArray itemsArray = new JsonArray(); for (Item item : items) { itemsArray.add(new ItemSummaryRepresentation().createItemSummary(item)); } write(representation, "items", itemsArray); - return new InstanceExtended(representation, id, items); + return new SearchInstance(representation, id, items); } public JsonObject toJson() { diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 21647db11d..85e1d7e26c 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -3,7 +3,7 @@ import lombok.AllArgsConstructor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.folio.circulation.domain.InstanceExtended; +import org.folio.circulation.domain.SearchInstance; import org.folio.circulation.domain.Item; import org.folio.circulation.domain.MultipleRecords; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; @@ -13,6 +13,7 @@ import org.folio.circulation.support.http.client.Response; import org.folio.circulation.support.results.Result; import java.lang.invoke.MethodHandles; +import java.net.URLEncoder; import java.util.Map; import java.util.concurrent.CompletableFuture; import static org.folio.circulation.support.results.Result.emptyAsync; @@ -32,20 +33,21 @@ public SearchRepository(Clients clients) { this(new ItemRepository(clients), clients.searchClient()); } - public CompletableFuture> getInstanceWithItems(String instanceId) { - log.debug("getInstanceWithItems:: instanceId {}", instanceId); + public CompletableFuture> getInstanceWithItems(String query) { + log.debug("getInstanceWithItems:: query {}", query); return searchClient.getManyWithQueryStringParameters(Map.of("expandAll", - "true", "query", String.format("id==%s", instanceId))) + "true", "query", URLEncoder.encode(query, + java.nio.charset.StandardCharsets.UTF_8))) .thenApply(flatMapResult(this::mapResponseToInstances)) .thenApply(mapResult(MultipleRecords::firstOrNull)) .thenCompose(r -> r.after(this::updateItemDetails)); } - private Result> mapResponseToInstances(Response response) { - return MultipleRecords.from(response, InstanceExtended::from, "instances"); + private Result> mapResponseToInstances(Response response) { + return MultipleRecords.from(response, SearchInstance::from, "instances"); } - private CompletableFuture> updateItemDetails(InstanceExtended searchInstance) { + private CompletableFuture> updateItemDetails(SearchInstance searchInstance) { log.debug("updateItemDetails:: searchInstance {}", searchInstance); if (searchInstance == null) { return emptyAsync(); diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index 73eea2a62c..b5a8c96ed3 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -6,14 +6,13 @@ import io.vertx.ext.web.RoutingContext; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.folio.circulation.domain.InstanceExtended; +import org.folio.circulation.domain.SearchInstance; import org.folio.circulation.infrastructure.storage.SearchRepository; import org.folio.circulation.support.Clients; -import org.folio.circulation.support.RouteRegistration; import org.folio.circulation.support.http.server.JsonHttpResponse; import org.folio.circulation.support.http.server.WebContext; - import java.lang.invoke.MethodHandles; +import java.util.List; public class ItemsByInstanceResource extends Resource { @@ -24,25 +23,25 @@ public ItemsByInstanceResource(HttpClient client) { @Override public void register(Router router) { - RouteRegistration routeRegistration = new RouteRegistration( - "/circulation/items-by-instance", router); - - routeRegistration.get(this::getInstanceItems); + router.get("/circulation/items-by-instance") + .handler(this::getInstanceItems); } private void getInstanceItems(RoutingContext routingContext) { final WebContext context = new WebContext(routingContext); final Clients clients = Clients.create(context, client); - String instanceId = routingContext.pathParam("id"); - log.debug("getInstanceItems:: instanceId {}", instanceId); - final var searchRepository = new SearchRepository(clients); - searchRepository.getInstanceWithItems(instanceId) - .thenApply(r -> r.map(this::toJson)) - .thenApply(r -> r.map(JsonHttpResponse::ok)) - .thenAccept(context::writeResultToHttpResponse); + List queryParams = routingContext.queryParam("query"); + if (!queryParams.isEmpty()) { + final var searchRepository = new SearchRepository(clients); + searchRepository.getInstanceWithItems(queryParams.get(0)) + .thenApply(r -> r.map(this::toJson)) + .thenApply(r -> r.map(JsonHttpResponse::ok)) + .thenAccept(context::writeResultToHttpResponse); + } } - private JsonObject toJson(InstanceExtended instanceExtended) { + private JsonObject toJson(SearchInstance instanceExtended) { + log.debug("toJson:: instanceExtended: {}", instanceExtended); if (instanceExtended != null) return instanceExtended.toJson(); return new JsonObject(); diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index 222b301426..b87b146b69 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -13,12 +13,14 @@ public class ItemsByInstanceResourceTest extends APITests { void canGetInstanceById() { UUID instanceId = UUID.randomUUID(); searchFixture.basedUponDunkirk(instanceId); - Response response = get(instanceId.toString(), 200); + Response response = + get(String.format("query=(id==%s)", instanceId), 200); assertThat(response.getStatusCode(), is(200)); + assertThat(response.getJson().getString("id"), is(instanceId.toString())); } - private Response get(String instanceId, int expectedStatusCode) { - return restAssuredClient.get(itemsByInstanceUrl(instanceId), expectedStatusCode, + private Response get(String query, int expectedStatusCode) { + return restAssuredClient.get(itemsByInstanceUrl(query), expectedStatusCode, "items-by-instance-request"); } } diff --git a/src/test/java/api/support/APITests.java b/src/test/java/api/support/APITests.java index 09858b2693..8020fb431a 100644 --- a/src/test/java/api/support/APITests.java +++ b/src/test/java/api/support/APITests.java @@ -24,7 +24,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; -import api.support.fixtures.SearchFixture; +import api.support.fixtures.SearchInstanceFixture; import org.junit.Assert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -297,7 +297,7 @@ public abstract class APITests { protected final DepartmentFixture departmentFixture = new DepartmentFixture(); protected final CheckOutLockFixture checkOutLockFixture = new CheckOutLockFixture(); protected final SettingsFixture settingsFixture = new SettingsFixture(); - protected final SearchFixture searchFixture = new SearchFixture(); + protected final SearchInstanceFixture searchFixture = new SearchInstanceFixture(); protected APITests() { this(true, false); diff --git a/src/test/java/api/support/builders/SearchBuilder.java b/src/test/java/api/support/builders/SearchBuilder.java deleted file mode 100644 index 5856973650..0000000000 --- a/src/test/java/api/support/builders/SearchBuilder.java +++ /dev/null @@ -1,29 +0,0 @@ -package api.support.builders; - -import io.vertx.core.json.JsonObject; -import java.util.List; - -public class SearchBuilder extends JsonBuilder implements Builder { - private final int totalRecords; - private final JsonObject instance; - - public SearchBuilder(JsonObject instance) { - this.totalRecords = 1; - this.instance = instance; - } - - @Override - public JsonObject create() { - final JsonObject search = new JsonObject(); - - put(search, "totalRecords", totalRecords); - put(search, "instances", List.of(instance)); - - return search; - } - - public SearchBuilder withItems(List items) { - put(instance, "items", items); - return new SearchBuilder(instance); - } -} diff --git a/src/test/java/api/support/builders/SearchInstanceBuilder.java b/src/test/java/api/support/builders/SearchInstanceBuilder.java new file mode 100644 index 0000000000..f90cb83d68 --- /dev/null +++ b/src/test/java/api/support/builders/SearchInstanceBuilder.java @@ -0,0 +1,22 @@ +package api.support.builders; + +import io.vertx.core.json.JsonObject; +import java.util.List; + +public class SearchInstanceBuilder extends JsonBuilder implements Builder { + + private final JsonObject searchInstance; + public SearchInstanceBuilder(JsonObject searchInstance) { + this.searchInstance = searchInstance; + } + + @Override + public JsonObject create() { + return searchInstance; + } + + public SearchInstanceBuilder withItems(List items) { + put(searchInstance, "items", items); + return new SearchInstanceBuilder(searchInstance); + } +} diff --git a/src/test/java/api/support/fakes/FakeOkapi.java b/src/test/java/api/support/fakes/FakeOkapi.java index a2c78074ff..0a5e6604bf 100644 --- a/src/test/java/api/support/fakes/FakeOkapi.java +++ b/src/test/java/api/support/fakes/FakeOkapi.java @@ -231,6 +231,12 @@ public void start(Promise startFuture) throws IOException { FakeCalendarOkapi.registerCalendarSurroundingDates(router); registerFakeStorageLoansAnonymize(router); + new FakeSearchModule().register(router); + new FakeStorageModuleBuilder() + .withRecordName(FakeSearchModule.recordTypeName) + .withRootPath("/search/instances") + .create().register(router); + new FakeStorageModuleBuilder() .withRecordName("institution") .withRootPath("/location-units/institutions") @@ -416,7 +422,6 @@ public void start(Promise startFuture) throws IOException { new FakeStorageModuleBuilder() .withRootPath("/search/instances") - .withCollectionPropertyName("instances") .withChangeMetadata() .withQueryParameters("query", "expandAll") .create().register(router); diff --git a/src/test/java/api/support/fakes/FakeSearchModule.java b/src/test/java/api/support/fakes/FakeSearchModule.java new file mode 100644 index 0000000000..d77ae1fcbc --- /dev/null +++ b/src/test/java/api/support/fakes/FakeSearchModule.java @@ -0,0 +1,100 @@ +package api.support.fakes; + +import io.vertx.core.buffer.Buffer; +import io.vertx.core.http.HttpServerResponse; +import io.vertx.core.json.Json; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.web.Router; +import io.vertx.ext.web.RoutingContext; +import lombok.SneakyThrows; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.circulation.support.http.server.ClientErrorResponse; +import org.folio.circulation.support.http.server.WebContext; +import org.folio.circulation.support.results.Result; +import java.lang.invoke.MethodHandles; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import static java.lang.String.format; +import static org.folio.circulation.support.results.CommonFailures.failedDueToServerError; + +public class FakeSearchModule { + + private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + public static final String recordTypeName = "search-instance"; + private final String idRegexp = "id\\s*(==|!=|>|>=|<|<=|\\|=|\\|=)\\s*([a-f0-9\\-]+)"; + private final Storage storage; + private final Pattern idPattern; + + private final String rootPath = "/search/instances"; + + public FakeSearchModule() { + this.idPattern = Pattern.compile(idRegexp); + this.storage = Storage.getStorage(); + } + + @SneakyThrows + public void register(Router router) { + router.get("/search/instances").handler(this::getById); + } + + private void getById(RoutingContext routingContext) { + WebContext context = new WebContext(routingContext); + + Result idParsingResult = getIdParameter(routingContext); + + if(idParsingResult.failed()) { + idParsingResult.cause().writeTo(routingContext.response()); + return; + } + + Map resourcesForTenant = getResourcesForTenant(context); + + final String id = idParsingResult.value().toString(); + + if(resourcesForTenant.containsKey(id)) { + final JsonObject resourceRepresentation = resourcesForTenant.get(id); + + final JsonObject searchResult = new JsonObject(); + + searchResult.put("totalRecords", 1); + searchResult.put("instances", List.of(resourceRepresentation)); + + log.debug("Found {} resource: {}", recordTypeName, + searchResult.encodePrettily()); + + HttpServerResponse response = routingContext.response(); + Buffer buffer = Buffer.buffer(Json.encodePrettily(searchResult), "UTF-8"); + + response.setStatusCode(200); + response.putHeader("content-type", "application/json; charset=utf-8"); + response.putHeader("content-length", Integer.toString(buffer.length())); + + response.write(buffer); + response.end(); + } + else { + log.debug("Failed to find {} resource: {}", recordTypeName, + idParsingResult); + + ClientErrorResponse.notFound(routingContext.response()); + } + } + + private Result getIdParameter(RoutingContext routingContext) { + final String query = routingContext.request().getParam("query"); + Matcher matcher = idPattern.matcher(query); + String id = matcher.find() ? matcher.group(2) : null; + + return Result.of(() -> UUID.fromString(id)) + .mapFailure(r -> failedDueToServerError(format( + "ID parameter \"%s\" is not a valid UUID", id))); + } + + private Map getResourcesForTenant(WebContext context) { + return storage.getTenantResources(rootPath, context.getTenantId()); + } +} diff --git a/src/test/java/api/support/fixtures/SearchFixture.java b/src/test/java/api/support/fixtures/SearchInstanceFixture.java similarity index 70% rename from src/test/java/api/support/fixtures/SearchFixture.java rename to src/test/java/api/support/fixtures/SearchInstanceFixture.java index 533c5e4f47..1b88d32b3c 100644 --- a/src/test/java/api/support/fixtures/SearchFixture.java +++ b/src/test/java/api/support/fixtures/SearchInstanceFixture.java @@ -1,21 +1,21 @@ package api.support.fixtures; import api.support.builders.InstanceBuilder; -import api.support.builders.SearchBuilder; +import api.support.builders.SearchInstanceBuilder; import api.support.http.ResourceClient; import java.util.List; import java.util.UUID; -public class SearchFixture { +public class SearchInstanceFixture { private final ResourceClient searchClient; - public SearchFixture() { + public SearchInstanceFixture() { this.searchClient = ResourceClient.forSearchClient(); } public void basedUponDunkirk(UUID instanceId) { - SearchBuilder builder = new SearchBuilder(new InstanceBuilder("Dunkirk", + SearchInstanceBuilder builder = new SearchInstanceBuilder(new InstanceBuilder("Dunkirk", UUID.randomUUID()).withId(instanceId).create()).withItems(List.of(ItemExamples.basedUponDunkirk(UUID.randomUUID(), UUID.randomUUID()).create())); searchClient.create(builder); } diff --git a/src/test/java/api/support/http/InterfaceUrls.java b/src/test/java/api/support/http/InterfaceUrls.java index 7399b0c542..958623e21b 100644 --- a/src/test/java/api/support/http/InterfaceUrls.java +++ b/src/test/java/api/support/http/InterfaceUrls.java @@ -339,6 +339,6 @@ public static URL searchUrl(String subPath) { } public static URL itemsByInstanceUrl(String subPath) { - return circulationModuleUrl("/circulation/items-by-instance/" + subPath); + return circulationModuleUrl("/circulation/items-by-instance?" + subPath); } } From 1b29087482f3f43a36c1df543a55889e3d36e5b3 Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Tue, 21 May 2024 17:25:56 +0500 Subject: [PATCH 10/27] CIRC-2072 Create a facade for instance search --- src/main/java/org/folio/circulation/domain/Item.java | 2 +- .../java/org/folio/circulation/domain/SearchInstance.java | 3 --- src/test/java/api/support/fakes/FakeOkapi.java | 6 ------ 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/Item.java b/src/main/java/org/folio/circulation/domain/Item.java index 6ad60b2932..c0b15b4ae6 100644 --- a/src/main/java/org/folio/circulation/domain/Item.java +++ b/src/main/java/org/folio/circulation/domain/Item.java @@ -421,7 +421,7 @@ public String getTenantId() { } public Item changeTenantId(String tenantId) { - if(tenantId != null) { + if(tenantId != null && itemRepresentation != null) { write(itemRepresentation, "tenantId", tenantId); } return this; diff --git a/src/main/java/org/folio/circulation/domain/SearchInstance.java b/src/main/java/org/folio/circulation/domain/SearchInstance.java index 3f82792380..b80d231c20 100644 --- a/src/main/java/org/folio/circulation/domain/SearchInstance.java +++ b/src/main/java/org/folio/circulation/domain/SearchInstance.java @@ -22,11 +22,8 @@ public class SearchInstance { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); - JsonObject representation; - String id; - @NonNull Collection items; public static SearchInstance from(JsonObject representation) { diff --git a/src/test/java/api/support/fakes/FakeOkapi.java b/src/test/java/api/support/fakes/FakeOkapi.java index 0a5e6604bf..82144daa90 100644 --- a/src/test/java/api/support/fakes/FakeOkapi.java +++ b/src/test/java/api/support/fakes/FakeOkapi.java @@ -420,12 +420,6 @@ public void start(Promise startFuture) throws IOException { .withChangeMetadata() .create().register(router); - new FakeStorageModuleBuilder() - .withRootPath("/search/instances") - .withChangeMetadata() - .withQueryParameters("query", "expandAll") - .create().register(router); - new FakeFeeFineOperationsModule().register(router); server.requestHandler(router) From a6e440ea8fe87949d5b9604654799039e9e9b88d Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Thu, 23 May 2024 15:49:53 +0500 Subject: [PATCH 11/27] CIRC-2072 Create a facade for instance search --- descriptors/ModuleDescriptor-template.json | 1 + .../org/folio/circulation/domain/Item.java | 2 +- .../circulation/domain/SearchInstance.java | 6 +++--- .../storage/SearchRepository.java | 18 +++++++++--------- .../builders/SearchInstanceBuilder.java | 3 ++- .../api/support/fakes/FakeSearchModule.java | 5 +++-- .../fixtures/SearchInstanceFixture.java | 7 +++++-- 7 files changed, 24 insertions(+), 18 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index b180fdbef3..7a749525a6 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -174,6 +174,7 @@ "methods": ["GET"], "pathPattern": "/circulation/items-by-instance", "permissionsRequired": [ + "circulation.items-by-instance.get" ], "modulePermissions": [ "search.instances.collection.get" diff --git a/src/main/java/org/folio/circulation/domain/Item.java b/src/main/java/org/folio/circulation/domain/Item.java index c0b15b4ae6..83e0f85aaf 100644 --- a/src/main/java/org/folio/circulation/domain/Item.java +++ b/src/main/java/org/folio/circulation/domain/Item.java @@ -421,7 +421,7 @@ public String getTenantId() { } public Item changeTenantId(String tenantId) { - if(tenantId != null && itemRepresentation != null) { + if(itemRepresentation != null) { write(itemRepresentation, "tenantId", tenantId); } return this; diff --git a/src/main/java/org/folio/circulation/domain/SearchInstance.java b/src/main/java/org/folio/circulation/domain/SearchInstance.java index b80d231c20..5957eb6cab 100644 --- a/src/main/java/org/folio/circulation/domain/SearchInstance.java +++ b/src/main/java/org/folio/circulation/domain/SearchInstance.java @@ -1,5 +1,8 @@ package org.folio.circulation.domain; +import static org.folio.circulation.support.json.JsonObjectArrayPropertyFetcher.mapToList; +import static org.folio.circulation.support.json.JsonPropertyWriter.write; + import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import lombok.NonNull; @@ -14,9 +17,6 @@ import java.util.Collection; import java.util.List; -import static org.folio.circulation.support.json.JsonObjectArrayPropertyFetcher.mapToList; -import static org.folio.circulation.support.json.JsonPropertyWriter.write; - @Value @ToString(onlyExplicitlyIncluded = true) public class SearchInstance { diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 85e1d7e26c..5f5f72f4de 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -1,34 +1,34 @@ package org.folio.circulation.infrastructure.storage; +import static org.folio.circulation.support.StringUtil.urlEncode; +import static org.folio.circulation.support.results.Result.emptyAsync; +import static org.folio.circulation.support.results.ResultBinding.flatMapResult; +import static org.folio.circulation.support.results.ResultBinding.mapResult; + import lombok.AllArgsConstructor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.folio.circulation.domain.SearchInstance; import org.folio.circulation.domain.Item; import org.folio.circulation.domain.MultipleRecords; +import org.folio.circulation.domain.SearchInstance; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.support.AsyncCoordinationUtil; import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; import org.folio.circulation.support.http.client.Response; import org.folio.circulation.support.results.Result; + import java.lang.invoke.MethodHandles; -import java.net.URLEncoder; import java.util.Map; import java.util.concurrent.CompletableFuture; -import static org.folio.circulation.support.results.Result.emptyAsync; -import static org.folio.circulation.support.results.ResultBinding.flatMapResult; -import static org.folio.circulation.support.results.ResultBinding.mapResult; @AllArgsConstructor public class SearchRepository { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); - private final ItemRepository itemRepository; private final CollectionResourceClient searchClient; - public SearchRepository(Clients clients) { this(new ItemRepository(clients), clients.searchClient()); } @@ -36,8 +36,8 @@ public SearchRepository(Clients clients) { public CompletableFuture> getInstanceWithItems(String query) { log.debug("getInstanceWithItems:: query {}", query); return searchClient.getManyWithQueryStringParameters(Map.of("expandAll", - "true", "query", URLEncoder.encode(query, - java.nio.charset.StandardCharsets.UTF_8))) + "true", "query", urlEncode(query, + java.nio.charset.StandardCharsets.UTF_8.name()))) .thenApply(flatMapResult(this::mapResponseToInstances)) .thenApply(mapResult(MultipleRecords::firstOrNull)) .thenCompose(r -> r.after(this::updateItemDetails)); diff --git a/src/test/java/api/support/builders/SearchInstanceBuilder.java b/src/test/java/api/support/builders/SearchInstanceBuilder.java index f90cb83d68..3a57b1299e 100644 --- a/src/test/java/api/support/builders/SearchInstanceBuilder.java +++ b/src/test/java/api/support/builders/SearchInstanceBuilder.java @@ -1,11 +1,12 @@ package api.support.builders; -import io.vertx.core.json.JsonObject; import java.util.List; +import io.vertx.core.json.JsonObject; public class SearchInstanceBuilder extends JsonBuilder implements Builder { private final JsonObject searchInstance; + public SearchInstanceBuilder(JsonObject searchInstance) { this.searchInstance = searchInstance; } diff --git a/src/test/java/api/support/fakes/FakeSearchModule.java b/src/test/java/api/support/fakes/FakeSearchModule.java index d77ae1fcbc..26b6e4f53f 100644 --- a/src/test/java/api/support/fakes/FakeSearchModule.java +++ b/src/test/java/api/support/fakes/FakeSearchModule.java @@ -1,5 +1,8 @@ package api.support.fakes; +import static java.lang.String.format; +import static org.folio.circulation.support.results.CommonFailures.failedDueToServerError; + import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpServerResponse; import io.vertx.core.json.Json; @@ -18,8 +21,6 @@ import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; -import static java.lang.String.format; -import static org.folio.circulation.support.results.CommonFailures.failedDueToServerError; public class FakeSearchModule { diff --git a/src/test/java/api/support/fixtures/SearchInstanceFixture.java b/src/test/java/api/support/fixtures/SearchInstanceFixture.java index 1b88d32b3c..71aa8fdb90 100644 --- a/src/test/java/api/support/fixtures/SearchInstanceFixture.java +++ b/src/test/java/api/support/fixtures/SearchInstanceFixture.java @@ -15,8 +15,11 @@ public SearchInstanceFixture() { } public void basedUponDunkirk(UUID instanceId) { - SearchInstanceBuilder builder = new SearchInstanceBuilder(new InstanceBuilder("Dunkirk", - UUID.randomUUID()).withId(instanceId).create()).withItems(List.of(ItemExamples.basedUponDunkirk(UUID.randomUUID(), UUID.randomUUID()).create())); + SearchInstanceBuilder builder = new SearchInstanceBuilder( + new InstanceBuilder( + "Dunkirk", UUID.randomUUID()).withId(instanceId).create()) + .withItems(List.of(ItemExamples.basedUponDunkirk(UUID.randomUUID(), + UUID.randomUUID()).create())); searchClient.create(builder); } } From 1c88c01f7bcffb092968ba6c9f6689c09eae9368 Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Fri, 24 May 2024 11:44:59 +0500 Subject: [PATCH 12/27] CIRC-2072 Create a facade for instance search --- src/test/java/api/ItemsByInstanceResourceTest.java | 2 +- src/test/java/api/requests/AllowedServicePointsAPITests.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index b87b146b69..7c976c4329 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -8,7 +8,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; -public class ItemsByInstanceResourceTest extends APITests { +class ItemsByInstanceResourceTest extends APITests { @Test void canGetInstanceById() { UUID instanceId = UUID.randomUUID(); diff --git a/src/test/java/api/requests/AllowedServicePointsAPITests.java b/src/test/java/api/requests/AllowedServicePointsAPITests.java index da9b9e83af..d6df3fdf83 100644 --- a/src/test/java/api/requests/AllowedServicePointsAPITests.java +++ b/src/test/java/api/requests/AllowedServicePointsAPITests.java @@ -35,7 +35,6 @@ import org.apache.http.HttpStatus; import org.folio.circulation.domain.ItemStatus; -import org.folio.circulation.domain.Request; import org.folio.circulation.domain.RequestLevel; import org.folio.circulation.domain.RequestType; import org.folio.circulation.support.http.client.Response; From be46a6716bc4ab41588ce8cdd0bfaad415b00c54 Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Fri, 24 May 2024 18:05:38 +0500 Subject: [PATCH 13/27] CIRC-2072 Create a facade for instance search --- .../infrastructure/storage/SearchRepository.java | 2 +- .../circulation/resources/ItemsByInstanceResource.java | 9 +++++---- src/test/java/api/ItemsByInstanceResourceTest.java | 7 ++++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 5f5f72f4de..7d6c207e50 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -48,7 +48,7 @@ private Result> mapResponseToInstances(Response } private CompletableFuture> updateItemDetails(SearchInstance searchInstance) { - log.debug("updateItemDetails:: searchInstance {}", searchInstance); + log.debug("updateItemDetails:: searchInstance {}", () -> searchInstance); if (searchInstance == null) { return emptyAsync(); } diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index b5a8c96ed3..3bac5505fc 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -17,6 +17,7 @@ public class ItemsByInstanceResource extends Resource { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + public ItemsByInstanceResource(HttpClient client) { super(client); } @@ -32,8 +33,7 @@ private void getInstanceItems(RoutingContext routingContext) { final Clients clients = Clients.create(context, client); List queryParams = routingContext.queryParam("query"); if (!queryParams.isEmpty()) { - final var searchRepository = new SearchRepository(clients); - searchRepository.getInstanceWithItems(queryParams.get(0)) + new SearchRepository(clients).getInstanceWithItems(queryParams.get(0)) .thenApply(r -> r.map(this::toJson)) .thenApply(r -> r.map(JsonHttpResponse::ok)) .thenAccept(context::writeResultToHttpResponse); @@ -41,9 +41,10 @@ private void getInstanceItems(RoutingContext routingContext) { } private JsonObject toJson(SearchInstance instanceExtended) { - log.debug("toJson:: instanceExtended: {}", instanceExtended); - if (instanceExtended != null) + log.debug("toJson:: instanceExtended: {}", () -> instanceExtended); + if (instanceExtended != null) { return instanceExtended.toJson(); + } return new JsonObject(); } } diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index 7c976c4329..2ffa2b05a5 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -1,12 +1,13 @@ package api; +import static api.support.http.InterfaceUrls.itemsByInstanceUrl; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; + import api.support.APITests; import org.folio.circulation.support.http.client.Response; import org.junit.jupiter.api.Test; import java.util.UUID; -import static api.support.http.InterfaceUrls.itemsByInstanceUrl; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.Is.is; class ItemsByInstanceResourceTest extends APITests { @Test From 9d6832078fd78f73ddc03336594a7aebb88e594f Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Mon, 27 May 2024 15:39:50 +0500 Subject: [PATCH 14/27] CIRC-2072 Create a facade for instance search --- .../circulation/domain/SearchInstance.java | 17 ++++----- .../storage/SearchRepository.java | 18 +++++----- .../resources/ItemsByInstanceResource.java | 16 ++++----- .../java/api/ItemsByInstanceResourceTest.java | 6 ++-- .../builders/SearchInstanceBuilder.java | 1 + .../api/support/fakes/FakeSearchModule.java | 36 ++++++++++--------- .../api/support/fakes/FakeStorageModule.java | 4 +-- .../fixtures/SearchInstanceFixture.java | 5 +-- 8 files changed, 56 insertions(+), 47 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/SearchInstance.java b/src/main/java/org/folio/circulation/domain/SearchInstance.java index 5957eb6cab..809a121181 100644 --- a/src/main/java/org/folio/circulation/domain/SearchInstance.java +++ b/src/main/java/org/folio/circulation/domain/SearchInstance.java @@ -3,19 +3,20 @@ import static org.folio.circulation.support.json.JsonObjectArrayPropertyFetcher.mapToList; import static org.folio.circulation.support.json.JsonPropertyWriter.write; -import io.vertx.core.json.JsonArray; -import io.vertx.core.json.JsonObject; -import lombok.NonNull; -import lombok.ToString; -import lombok.Value; +import java.lang.invoke.MethodHandles; +import java.util.Collection; +import java.util.List; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.representations.ItemSummaryRepresentation; import org.folio.circulation.storage.mappers.ItemMapper; -import java.lang.invoke.MethodHandles; -import java.util.Collection; -import java.util.List; +import io.vertx.core.json.JsonArray; +import io.vertx.core.json.JsonObject; +import lombok.NonNull; +import lombok.ToString; +import lombok.Value; @Value @ToString(onlyExplicitlyIncluded = true) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 7d6c207e50..d32a9aa7d3 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -1,11 +1,14 @@ package org.folio.circulation.infrastructure.storage; import static org.folio.circulation.support.StringUtil.urlEncode; -import static org.folio.circulation.support.results.Result.emptyAsync; +import static org.folio.circulation.support.results.Result.failed; import static org.folio.circulation.support.results.ResultBinding.flatMapResult; import static org.folio.circulation.support.results.ResultBinding.mapResult; -import lombok.AllArgsConstructor; +import java.lang.invoke.MethodHandles; +import java.util.Map; +import java.util.concurrent.CompletableFuture; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.Item; @@ -13,14 +16,13 @@ import org.folio.circulation.domain.SearchInstance; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.support.AsyncCoordinationUtil; +import org.folio.circulation.support.BadRequestFailure; import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; import org.folio.circulation.support.http.client.Response; import org.folio.circulation.support.results.Result; -import java.lang.invoke.MethodHandles; -import java.util.Map; -import java.util.concurrent.CompletableFuture; +import lombok.AllArgsConstructor; @AllArgsConstructor public class SearchRepository { @@ -36,8 +38,7 @@ public SearchRepository(Clients clients) { public CompletableFuture> getInstanceWithItems(String query) { log.debug("getInstanceWithItems:: query {}", query); return searchClient.getManyWithQueryStringParameters(Map.of("expandAll", - "true", "query", urlEncode(query, - java.nio.charset.StandardCharsets.UTF_8.name()))) + "true", "query", urlEncode(query))) .thenApply(flatMapResult(this::mapResponseToInstances)) .thenApply(mapResult(MultipleRecords::firstOrNull)) .thenCompose(r -> r.after(this::updateItemDetails)); @@ -50,7 +51,8 @@ private Result> mapResponseToInstances(Response private CompletableFuture> updateItemDetails(SearchInstance searchInstance) { log.debug("updateItemDetails:: searchInstance {}", () -> searchInstance); if (searchInstance == null) { - return emptyAsync(); + return CompletableFuture.completedFuture(failed(new BadRequestFailure( + "Search result is empty"))); } return AsyncCoordinationUtil.allOf(searchInstance.getItems(), this::fetchItemDetails) .thenApply(r -> r.map(searchInstance::changeItems)); diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index 3bac5505fc..841d5a401e 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -1,9 +1,8 @@ package org.folio.circulation.resources; -import io.vertx.core.http.HttpClient; -import io.vertx.core.json.JsonObject; -import io.vertx.ext.web.Router; -import io.vertx.ext.web.RoutingContext; +import java.lang.invoke.MethodHandles; +import java.util.List; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.SearchInstance; @@ -11,8 +10,11 @@ import org.folio.circulation.support.Clients; import org.folio.circulation.support.http.server.JsonHttpResponse; import org.folio.circulation.support.http.server.WebContext; -import java.lang.invoke.MethodHandles; -import java.util.List; + +import io.vertx.core.http.HttpClient; +import io.vertx.core.json.JsonObject; +import io.vertx.ext.web.Router; +import io.vertx.ext.web.RoutingContext; public class ItemsByInstanceResource extends Resource { @@ -32,12 +34,10 @@ private void getInstanceItems(RoutingContext routingContext) { final WebContext context = new WebContext(routingContext); final Clients clients = Clients.create(context, client); List queryParams = routingContext.queryParam("query"); - if (!queryParams.isEmpty()) { new SearchRepository(clients).getInstanceWithItems(queryParams.get(0)) .thenApply(r -> r.map(this::toJson)) .thenApply(r -> r.map(JsonHttpResponse::ok)) .thenAccept(context::writeResultToHttpResponse); - } } private JsonObject toJson(SearchInstance instanceExtended) { diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index 2ffa2b05a5..19105c6d00 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -4,10 +4,12 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; -import api.support.APITests; +import java.util.UUID; + import org.folio.circulation.support.http.client.Response; import org.junit.jupiter.api.Test; -import java.util.UUID; + +import api.support.APITests; class ItemsByInstanceResourceTest extends APITests { @Test diff --git a/src/test/java/api/support/builders/SearchInstanceBuilder.java b/src/test/java/api/support/builders/SearchInstanceBuilder.java index 3a57b1299e..92e67bdeb3 100644 --- a/src/test/java/api/support/builders/SearchInstanceBuilder.java +++ b/src/test/java/api/support/builders/SearchInstanceBuilder.java @@ -1,6 +1,7 @@ package api.support.builders; import java.util.List; + import io.vertx.core.json.JsonObject; public class SearchInstanceBuilder extends JsonBuilder implements Builder { diff --git a/src/test/java/api/support/fakes/FakeSearchModule.java b/src/test/java/api/support/fakes/FakeSearchModule.java index 26b6e4f53f..b2a88d9458 100644 --- a/src/test/java/api/support/fakes/FakeSearchModule.java +++ b/src/test/java/api/support/fakes/FakeSearchModule.java @@ -3,6 +3,19 @@ import static java.lang.String.format; import static org.folio.circulation.support.results.CommonFailures.failedDueToServerError; +import java.lang.invoke.MethodHandles; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.circulation.support.http.server.ClientErrorResponse; +import org.folio.circulation.support.http.server.WebContext; +import org.folio.circulation.support.results.Result; + import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpServerResponse; import io.vertx.core.json.Json; @@ -10,30 +23,19 @@ import io.vertx.ext.web.Router; import io.vertx.ext.web.RoutingContext; import lombok.SneakyThrows; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.folio.circulation.support.http.server.ClientErrorResponse; -import org.folio.circulation.support.http.server.WebContext; -import org.folio.circulation.support.results.Result; -import java.lang.invoke.MethodHandles; -import java.util.List; -import java.util.Map; -import java.util.UUID; -import java.util.regex.Matcher; -import java.util.regex.Pattern; public class FakeSearchModule { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); public static final String recordTypeName = "search-instance"; - private final String idRegexp = "id\\s*(==|!=|>|>=|<|<=|\\|=|\\|=)\\s*([a-f0-9\\-]+)"; + private static final String ID_REGEXP = "id\\s*(==|!=|>|>=|<|<=|\\|=|\\|=)\\s*" + + "([a-f0-9\\-]+)"; private final Storage storage; private final Pattern idPattern; - - private final String rootPath = "/search/instances"; + private static final String ROOT_PATH = "/search/instances"; public FakeSearchModule() { - this.idPattern = Pattern.compile(idRegexp); + this.idPattern = Pattern.compile(ID_REGEXP); this.storage = Storage.getStorage(); } @@ -56,7 +58,7 @@ private void getById(RoutingContext routingContext) { final String id = idParsingResult.value().toString(); - if(resourcesForTenant.containsKey(id)) { + if (resourcesForTenant.containsKey(id)) { final JsonObject resourceRepresentation = resourcesForTenant.get(id); final JsonObject searchResult = new JsonObject(); @@ -96,6 +98,6 @@ private Result getIdParameter(RoutingContext routingContext) { } private Map getResourcesForTenant(WebContext context) { - return storage.getTenantResources(rootPath, context.getTenantId()); + return storage.getTenantResources(ROOT_PATH, context.getTenantId()); } } diff --git a/src/test/java/api/support/fakes/FakeStorageModule.java b/src/test/java/api/support/fakes/FakeStorageModule.java index baaaad21e2..07af148cf0 100644 --- a/src/test/java/api/support/fakes/FakeStorageModule.java +++ b/src/test/java/api/support/fakes/FakeStorageModule.java @@ -329,7 +329,7 @@ private void getById(RoutingContext routingContext) { final String id = idParsingResult.value().toString(); - if(resourcesForTenant.containsKey(id)) { + if (resourcesForTenant.containsKey(id)) { final JsonObject resourceRepresentation = resourcesForTenant.get(id); log.debug("Found {} resource: {}", recordTypeName, @@ -444,7 +444,7 @@ private void delete(RoutingContext routingContext) { Map resourcesForTenant = getResourcesForTenant(context); - if(resourcesForTenant.containsKey(id)) { + if (resourcesForTenant.containsKey(id)) { resourcesForTenant.remove(id); noContent().writeTo(routingContext.response()); diff --git a/src/test/java/api/support/fixtures/SearchInstanceFixture.java b/src/test/java/api/support/fixtures/SearchInstanceFixture.java index 71aa8fdb90..a9498447a4 100644 --- a/src/test/java/api/support/fixtures/SearchInstanceFixture.java +++ b/src/test/java/api/support/fixtures/SearchInstanceFixture.java @@ -1,10 +1,11 @@ package api.support.fixtures; +import java.util.List; +import java.util.UUID; + import api.support.builders.InstanceBuilder; import api.support.builders.SearchInstanceBuilder; import api.support.http.ResourceClient; -import java.util.List; -import java.util.UUID; public class SearchInstanceFixture { From af5a9f78e7dafedc3f6b1ce416a90f272e417a0c Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Mon, 27 May 2024 19:05:57 +0500 Subject: [PATCH 15/27] CIRC-2072 Create a facade for instance search --- .../infrastructure/storage/SearchRepository.java | 11 ++++++++--- .../resources/ItemsByInstanceResource.java | 4 +--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index d32a9aa7d3..9f97e2614e 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -6,6 +6,7 @@ import static org.folio.circulation.support.results.ResultBinding.mapResult; import java.lang.invoke.MethodHandles; +import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -35,10 +36,14 @@ public SearchRepository(Clients clients) { this(new ItemRepository(clients), clients.searchClient()); } - public CompletableFuture> getInstanceWithItems(String query) { - log.debug("getInstanceWithItems:: query {}", query); + public CompletableFuture> getInstanceWithItems(List queryParams) { + log.debug("getInstanceWithItems:: query {}", queryParams); + if (queryParams.isEmpty()) { + return CompletableFuture.completedFuture(failed(new BadRequestFailure( + "query is empty"))); + } return searchClient.getManyWithQueryStringParameters(Map.of("expandAll", - "true", "query", urlEncode(query))) + "true", "query", urlEncode(queryParams.get(0)))) .thenApply(flatMapResult(this::mapResponseToInstances)) .thenApply(mapResult(MultipleRecords::firstOrNull)) .thenCompose(r -> r.after(this::updateItemDetails)); diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index 841d5a401e..ef6286314e 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -1,7 +1,6 @@ package org.folio.circulation.resources; import java.lang.invoke.MethodHandles; -import java.util.List; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -33,8 +32,7 @@ public void register(Router router) { private void getInstanceItems(RoutingContext routingContext) { final WebContext context = new WebContext(routingContext); final Clients clients = Clients.create(context, client); - List queryParams = routingContext.queryParam("query"); - new SearchRepository(clients).getInstanceWithItems(queryParams.get(0)) + new SearchRepository(clients).getInstanceWithItems(routingContext.queryParam("query")) .thenApply(r -> r.map(this::toJson)) .thenApply(r -> r.map(JsonHttpResponse::ok)) .thenAccept(context::writeResultToHttpResponse); From f86281cd418f4ecabafa6d69cecf3aba621bf7a7 Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Mon, 27 May 2024 19:13:21 +0500 Subject: [PATCH 16/27] CIRC-2072 Create a facade for instance search --- .../circulation/infrastructure/storage/SearchRepository.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 9f97e2614e..bb8b3635a9 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -65,7 +65,6 @@ private CompletableFuture> updateItemDetails(SearchInstan private CompletableFuture> fetchItemDetails(Item searchItem) { return itemRepository.fetchById(searchItem.getItemId()) - .thenComposeAsync(itemRepository::fetchItemRelatedRecords) .thenApply(r -> r.map(item -> item.changeTenantId(searchItem.getTenantId()))); } } From 0702200f949d9e9292b00f6c18d87997272e8a7b Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Mon, 27 May 2024 20:29:11 +0500 Subject: [PATCH 17/27] CIRC-2072 Create a facade for instance search --- src/main/java/org/folio/circulation/domain/Item.java | 2 +- .../circulation/resources/ItemsByInstanceResource.java | 8 ++++---- src/test/java/api/support/fakes/FakeSearchModule.java | 2 +- src/test/java/api/support/fakes/FakeStorageModule.java | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/Item.java b/src/main/java/org/folio/circulation/domain/Item.java index 83e0f85aaf..926a348be1 100644 --- a/src/main/java/org/folio/circulation/domain/Item.java +++ b/src/main/java/org/folio/circulation/domain/Item.java @@ -421,7 +421,7 @@ public String getTenantId() { } public Item changeTenantId(String tenantId) { - if(itemRepresentation != null) { + if (itemRepresentation != null) { write(itemRepresentation, "tenantId", tenantId); } return this; diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index ef6286314e..882ebaf29c 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -38,10 +38,10 @@ private void getInstanceItems(RoutingContext routingContext) { .thenAccept(context::writeResultToHttpResponse); } - private JsonObject toJson(SearchInstance instanceExtended) { - log.debug("toJson:: instanceExtended: {}", () -> instanceExtended); - if (instanceExtended != null) { - return instanceExtended.toJson(); + private JsonObject toJson(SearchInstance searchInstance) { + log.debug("toJson:: searchInstance: {}", () -> searchInstance); + if (searchInstance != null) { + return searchInstance.toJson(); } return new JsonObject(); } diff --git a/src/test/java/api/support/fakes/FakeSearchModule.java b/src/test/java/api/support/fakes/FakeSearchModule.java index b2a88d9458..db1e07319a 100644 --- a/src/test/java/api/support/fakes/FakeSearchModule.java +++ b/src/test/java/api/support/fakes/FakeSearchModule.java @@ -49,7 +49,7 @@ private void getById(RoutingContext routingContext) { Result idParsingResult = getIdParameter(routingContext); - if(idParsingResult.failed()) { + if (idParsingResult.failed()) { idParsingResult.cause().writeTo(routingContext.response()); return; } diff --git a/src/test/java/api/support/fakes/FakeStorageModule.java b/src/test/java/api/support/fakes/FakeStorageModule.java index 07af148cf0..4cf5b18b69 100644 --- a/src/test/java/api/support/fakes/FakeStorageModule.java +++ b/src/test/java/api/support/fakes/FakeStorageModule.java @@ -320,7 +320,7 @@ private void getById(RoutingContext routingContext) { Result idParsingResult = getIdParameter(routingContext); - if(idParsingResult.failed()) { + if (idParsingResult.failed()) { idParsingResult.cause().writeTo(routingContext.response()); return; } From 0b2acfd77f22435a741829541d4fb928c67ea7d3 Mon Sep 17 00:00:00 2001 From: MagzhanArtykov Date: Tue, 28 May 2024 14:04:14 +0500 Subject: [PATCH 18/27] CIRC-2072 Create a facade for instance search --- src/test/java/api/ItemsByInstanceResourceTest.java | 7 ++++++- .../java/api/support/fixtures/SearchInstanceFixture.java | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index 19105c6d00..b644e8fb95 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -10,16 +10,21 @@ import org.junit.jupiter.api.Test; import api.support.APITests; +import api.support.http.ItemResource; class ItemsByInstanceResourceTest extends APITests { @Test void canGetInstanceById() { UUID instanceId = UUID.randomUUID(); - searchFixture.basedUponDunkirk(instanceId); + ItemResource itemResource = itemsFixture.basedUponDunkirk(); + searchFixture.basedUponDunkirk(instanceId, itemResource); Response response = get(String.format("query=(id==%s)", instanceId), 200); assertThat(response.getStatusCode(), is(200)); assertThat(response.getJson().getString("id"), is(instanceId.toString())); + assertThat(response.getJson().getJsonArray("items").size(), is(1)); + assertThat(response.getJson().getJsonArray("items").getJsonObject(0).getString("id"), + is(itemResource.getId().toString())); } private Response get(String query, int expectedStatusCode) { diff --git a/src/test/java/api/support/fixtures/SearchInstanceFixture.java b/src/test/java/api/support/fixtures/SearchInstanceFixture.java index a9498447a4..e1bb144753 100644 --- a/src/test/java/api/support/fixtures/SearchInstanceFixture.java +++ b/src/test/java/api/support/fixtures/SearchInstanceFixture.java @@ -5,6 +5,7 @@ import api.support.builders.InstanceBuilder; import api.support.builders.SearchInstanceBuilder; +import api.support.http.ItemResource; import api.support.http.ResourceClient; public class SearchInstanceFixture { @@ -15,12 +16,11 @@ public SearchInstanceFixture() { this.searchClient = ResourceClient.forSearchClient(); } - public void basedUponDunkirk(UUID instanceId) { + public void basedUponDunkirk(UUID instanceId, ItemResource itemResource) { SearchInstanceBuilder builder = new SearchInstanceBuilder( new InstanceBuilder( "Dunkirk", UUID.randomUUID()).withId(instanceId).create()) - .withItems(List.of(ItemExamples.basedUponDunkirk(UUID.randomUUID(), - UUID.randomUUID()).create())); + .withItems(List.of(itemResource.getJson())); searchClient.create(builder); } } From 3f6bd4f9dace149f962a02e412b38c5f905d5cc8 Mon Sep 17 00:00:00 2001 From: OleksandrVidinieiev <56632770+OleksandrVidinieiev@users.noreply.github.com> Date: Thu, 6 Jun 2024 17:20:04 +0300 Subject: [PATCH 19/27] CIRC-2101: Fetch item details across tenants (#1474) * CIRC-2101 Fetch items across tenants * CIRC-2101 Fetch items across tenants * CIRC-2101 Remove @Setter from static field * CIRC-2101 Fix formatting * CIRC-2101 Fetch items across tenants * CIRC-2101 Remove @Setter from static field * CIRC-2101 Fix formatting * CIRC-2101 Add missing permission set * CIRC-2101 Reuse item repository for items in same tenant --- descriptors/ModuleDescriptor-template.json | 8 ++- .../storage/SearchRepository.java | 33 +++++++-- .../resources/ItemsByInstanceResource.java | 4 +- .../folio/circulation/support/Clients.java | 4 ++ .../client/VertxWebClientOkapiHttpClient.java | 3 + .../support/http/server/WebContext.java | 6 +- .../java/api/ItemsByInstanceResourceTest.java | 72 ++++++++++++++++--- src/test/java/api/support/APITestContext.java | 12 +++- .../api/support/RestAssuredConfiguration.java | 2 +- 9 files changed, 121 insertions(+), 23 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 7a749525a6..dee5864e0e 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -1536,6 +1536,11 @@ "displayName": "circulation settings - Read configuration", "description": "To read the configuration from mod settings." }, + { + "permissionName": "circulation.items-by-instance.get", + "displayName": "circulation - get items by instance", + "description": "get items by instance" + }, { "permissionName": "circulation.all", "displayName": "circulation - all permissions", @@ -1577,7 +1582,8 @@ "circulation.requests.allowed-service-points.get", "circulation.inventory.items-in-transit-report.get", "circulation.pick-slips.get", - "circulation.search-slips.get" + "circulation.search-slips.get", + "circulation.items-by-instance.get" ] }, { diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index bb8b3635a9..a999c2158d 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -6,9 +6,11 @@ import static org.folio.circulation.support.results.ResultBinding.mapResult; import java.lang.invoke.MethodHandles; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -21,19 +23,24 @@ import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; import org.folio.circulation.support.http.client.Response; +import org.folio.circulation.support.http.server.WebContext; import org.folio.circulation.support.results.Result; +import io.vertx.core.http.HttpClient; import lombok.AllArgsConstructor; @AllArgsConstructor public class SearchRepository { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); - private final ItemRepository itemRepository; + private final WebContext webContext; + private final HttpClient httpClient; private final CollectionResourceClient searchClient; - public SearchRepository(Clients clients) { - this(new ItemRepository(clients), clients.searchClient()); + public SearchRepository(WebContext webContext, HttpClient httpClient) { + this.webContext = webContext; + this.httpClient = httpClient; + this.searchClient = Clients.create(webContext, httpClient).searchClient(); } public CompletableFuture> getInstanceWithItems(List queryParams) { @@ -59,11 +66,27 @@ private CompletableFuture> updateItemDetails(SearchInstan return CompletableFuture.completedFuture(failed(new BadRequestFailure( "Search result is empty"))); } - return AsyncCoordinationUtil.allOf(searchInstance.getItems(), this::fetchItemDetails) + + Map> itemsByTenant = searchInstance.getItems() + .stream() + .collect(Collectors.groupingBy(Item::getTenantId)); + + log.info("updateItemDetails:: fetching item details from tenants: {}", itemsByTenant::keySet); + + return AsyncCoordinationUtil.allOf(itemsByTenant, this::fetchItemDetails) + .thenApply(r -> r.map(lists -> lists.stream().flatMap(Collection::stream).toList())) .thenApply(r -> r.map(searchInstance::changeItems)); } - private CompletableFuture> fetchItemDetails(Item searchItem) { + private CompletableFuture>> fetchItemDetails(String tenantId, List items) { + ItemRepository itemRepository = new ItemRepository(Clients.create(webContext, httpClient, tenantId)); + + return AsyncCoordinationUtil.allOf(items, item -> fetchItemDetails(item, itemRepository)); + } + + private CompletableFuture> fetchItemDetails(Item searchItem, + ItemRepository itemRepository) { + return itemRepository.fetchById(searchItem.getItemId()) .thenApply(r -> r.map(item -> item.changeTenantId(searchItem.getTenantId()))); } diff --git a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java index 882ebaf29c..381808e951 100644 --- a/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java +++ b/src/main/java/org/folio/circulation/resources/ItemsByInstanceResource.java @@ -6,7 +6,6 @@ import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.SearchInstance; import org.folio.circulation.infrastructure.storage.SearchRepository; -import org.folio.circulation.support.Clients; import org.folio.circulation.support.http.server.JsonHttpResponse; import org.folio.circulation.support.http.server.WebContext; @@ -31,8 +30,7 @@ public void register(Router router) { private void getInstanceItems(RoutingContext routingContext) { final WebContext context = new WebContext(routingContext); - final Clients clients = Clients.create(context, client); - new SearchRepository(clients).getInstanceWithItems(routingContext.queryParam("query")) + new SearchRepository(context, client).getInstanceWithItems(routingContext.queryParam("query")) .thenApply(r -> r.map(this::toJson)) .thenApply(r -> r.map(JsonHttpResponse::ok)) .thenAccept(context::writeResultToHttpResponse); diff --git a/src/main/java/org/folio/circulation/support/Clients.java b/src/main/java/org/folio/circulation/support/Clients.java index bf7dc48056..a035bd904e 100644 --- a/src/main/java/org/folio/circulation/support/Clients.java +++ b/src/main/java/org/folio/circulation/support/Clients.java @@ -75,6 +75,10 @@ public static Clients create(WebContext context, HttpClient httpClient) { return new Clients(context.createHttpClient(httpClient), context); } + public static Clients create(WebContext context, HttpClient httpClient, String tenantId) { + return new Clients(context.createHttpClient(httpClient, tenantId), context); + } + private Clients(OkapiHttpClient client, WebContext context) { try { requestsStorageClient = createRequestsStorageClient(client, context); diff --git a/src/main/java/org/folio/circulation/support/http/client/VertxWebClientOkapiHttpClient.java b/src/main/java/org/folio/circulation/support/http/client/VertxWebClientOkapiHttpClient.java index e4815a30bf..a203f4386f 100644 --- a/src/main/java/org/folio/circulation/support/http/client/VertxWebClientOkapiHttpClient.java +++ b/src/main/java/org/folio/circulation/support/http/client/VertxWebClientOkapiHttpClient.java @@ -27,7 +27,9 @@ import io.vertx.ext.web.client.HttpResponse; import io.vertx.ext.web.client.WebClient; import io.vertx.core.http.HttpMethod; +import lombok.extern.log4j.Log4j2; +@Log4j2 public class VertxWebClientOkapiHttpClient implements OkapiHttpClient { private static final Duration DEFAULT_TIMEOUT = Duration.of(20, SECONDS); private static final String ACCEPT = HttpHeaderNames.ACCEPT.toString(); @@ -184,6 +186,7 @@ public CompletableFuture> delete(String url, } private HttpRequest withStandardHeaders(HttpRequest request) { + log.debug("withStandardHeaders:: url={}, tenantId={}", request.uri(), tenantId); return request .putHeader(ACCEPT, "application/json, text/plain") .putHeader(OKAPI_URL, okapiUrl.toString()) diff --git a/src/main/java/org/folio/circulation/support/http/server/WebContext.java b/src/main/java/org/folio/circulation/support/http/server/WebContext.java index a38173b438..df6ec49778 100644 --- a/src/main/java/org/folio/circulation/support/http/server/WebContext.java +++ b/src/main/java/org/folio/circulation/support/http/server/WebContext.java @@ -76,6 +76,10 @@ public URL getOkapiBasedUrl(String path) throws MalformedURLException { } public OkapiHttpClient createHttpClient(HttpClient httpClient) { + return createHttpClient(httpClient, getTenantId()); + } + + public OkapiHttpClient createHttpClient(HttpClient httpClient, String tenantId) { URL okapiUrl; try { @@ -86,7 +90,7 @@ public OkapiHttpClient createHttpClient(HttpClient httpClient) { } return VertxWebClientOkapiHttpClient.createClientUsing(httpClient, - okapiUrl, getTenantId(), getOkapiToken(), getUserId(), + okapiUrl, tenantId, getOkapiToken(), getUserId(), getRequestId()); } diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index b644e8fb95..71d3ce18b6 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -1,30 +1,80 @@ package api; +import static api.support.APITestContext.clearTempTenantId; +import static api.support.APITestContext.setTempTenantId; import static api.support.http.InterfaceUrls.itemsByInstanceUrl; +import static api.support.matchers.JsonObjectMatcher.hasJsonPath; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.core.Is.is; +import java.util.List; import java.util.UUID; import org.folio.circulation.support.http.client.Response; import org.junit.jupiter.api.Test; import api.support.APITests; -import api.support.http.ItemResource; +import api.support.builders.SearchInstanceBuilder; +import api.support.http.IndividualResource; +import api.support.http.ResourceClient; +import api.support.matchers.UUIDMatcher; +import io.vertx.core.json.JsonArray; +import io.vertx.core.json.JsonObject; class ItemsByInstanceResourceTest extends APITests { + + private static final String TENANT_ID_COLLEGE = "college"; + private static final String TENANT_ID_UNIVERSITY = "university"; + @Test void canGetInstanceById() { - UUID instanceId = UUID.randomUUID(); - ItemResource itemResource = itemsFixture.basedUponDunkirk(); - searchFixture.basedUponDunkirk(instanceId, itemResource); - Response response = - get(String.format("query=(id==%s)", instanceId), 200); - assertThat(response.getStatusCode(), is(200)); - assertThat(response.getJson().getString("id"), is(instanceId.toString())); - assertThat(response.getJson().getJsonArray("items").size(), is(1)); - assertThat(response.getJson().getJsonArray("items").getJsonObject(0).getString("id"), - is(itemResource.getId().toString())); + IndividualResource instance = instancesFixture.basedUponDunkirk(); + UUID instanceId = instance.getId(); + + // create item in tenant "college" + setTempTenantId(TENANT_ID_COLLEGE); + IndividualResource collegeLocation = locationsFixture.mainFloor(); + IndividualResource collegeHoldings = holdingsFixture.defaultWithHoldings(instanceId); + IndividualResource collegeItem = itemsFixture.createItemWithHoldingsAndLocation( + collegeHoldings.getId(), collegeLocation.getId()); + clearTempTenantId(); + + // create item in tenant "university" + setTempTenantId(TENANT_ID_UNIVERSITY); + IndividualResource universityLocation = locationsFixture.thirdFloor(); + IndividualResource universityHoldings = holdingsFixture.defaultWithHoldings(instanceId); + IndividualResource universityItem = itemsFixture.createItemWithHoldingsAndLocation( + universityHoldings.getId(), universityLocation.getId()); + clearTempTenantId(); + + // make sure neither item exists in current tenant + assertThat(itemsFixture.getById(collegeItem.getId()).getResponse().getStatusCode(), is(404)); + assertThat(itemsFixture.getById(universityItem.getId()).getResponse().getStatusCode(), is(404)); + + List searchItems = List.of( + collegeItem.getJson().put("tenantId", TENANT_ID_COLLEGE), + universityItem.getJson().put("tenantId", TENANT_ID_UNIVERSITY)); + + JsonObject searchInstance = new SearchInstanceBuilder(instance.getJson()) + .withItems(searchItems) + .create(); + + ResourceClient.forSearchClient().create(searchInstance); + Response response = get(String.format("query=(id==%s)", instanceId), 200); + JsonObject responseJson = response.getJson(); + JsonArray items = responseJson.getJsonArray("items"); + + assertThat(responseJson.getString("id"), UUIDMatcher.is(instanceId)); + assertThat(items, iterableWithSize(2)); + assertThat(items, hasItem(allOf( + hasJsonPath("id", UUIDMatcher.is(collegeItem.getId())), + hasJsonPath("tenantId", is(TENANT_ID_COLLEGE))))); + assertThat(items, hasItem(allOf( + hasJsonPath("id", UUIDMatcher.is(universityItem.getId())), + hasJsonPath("tenantId", is(TENANT_ID_UNIVERSITY))))); } private Response get(String query, int expectedStatusCode) { diff --git a/src/test/java/api/support/APITestContext.java b/src/test/java/api/support/APITestContext.java index 167f11e91e..0f7890eb34 100644 --- a/src/test/java/api/support/APITestContext.java +++ b/src/test/java/api/support/APITestContext.java @@ -9,6 +9,7 @@ import java.net.URL; import java.time.ZonedDateTime; import java.time.temporal.ChronoUnit; +import java.util.Optional; import java.util.Properties; import java.util.Random; import java.util.concurrent.CompletableFuture; @@ -40,6 +41,7 @@ public class APITestContext { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); public static final String TENANT_ID = "test_tenant"; + public static String tempTenantId; private static String USER_ID = "79ff2a8b-d9c3-5b39-ad4a-0a84025ab085"; private static final String TOKEN = "eyJhbGciOiJIUzUxMiJ9eyJzdWIiOiJhZG1pbiIsInVzZXJfaWQiOiI3OWZmMmE4Yi1kOWMzLTViMzktYWQ0YS0wYTg0MDI1YWIwODUiLCJ0ZW5hbnQiOiJ0ZXN0X3RlbmFudCJ9BShwfHcNClt5ZXJ8ImQTMQtAM1sQEnhsfWNmXGsYVDpuaDN3RVQ9"; @@ -66,7 +68,15 @@ static String getToken() { } public static String getTenantId() { - return TENANT_ID; + return Optional.ofNullable(tempTenantId).orElse(TENANT_ID); + } + + public static void setTempTenantId(String tenantId) { + tempTenantId = tenantId; + } + + public static void clearTempTenantId() { + setTempTenantId(null); } public static String getUserId() { diff --git a/src/test/java/api/support/RestAssuredConfiguration.java b/src/test/java/api/support/RestAssuredConfiguration.java index bae905f073..e14bf8db7f 100644 --- a/src/test/java/api/support/RestAssuredConfiguration.java +++ b/src/test/java/api/support/RestAssuredConfiguration.java @@ -27,7 +27,7 @@ public static RequestSpecification standardHeaders(OkapiHeaders okapiHeaders) { final HashMap headers = new HashMap<>(); headers.put(OKAPI_URL, okapiHeaders.getUrl().toString()); - headers.put(TENANT, okapiHeaders.getTenantId()); + headers.put(TENANT, APITestContext.getTenantId()); headers.put(TOKEN, okapiHeaders.getToken()); headers.put(REQUEST_ID, okapiHeaders.getRequestId()); headers.put(OKAPI_PERMISSIONS, okapiHeaders.getOkapiPermissions()); From ea175649e33aa22cbe298b75e2a7182f5b669779 Mon Sep 17 00:00:00 2001 From: Alexander Kurash Date: Wed, 3 Jul 2024 20:28:48 +0300 Subject: [PATCH 20/27] CIRC-2109 Pass additional includeRoutingServicePoints parameter when needed (#1478) * CIRC-2109 Initial implementation * CIRC-2109 Apply suggestions from code review Co-authored-by: Roman Barannyk <53909129+roman-barannyk@users.noreply.github.com> * CIRC-2109 Support for fake routing service points * CIRC-2109 Remove unneeded method * CIRC-2109 Add Override annotation * CIRC-2109 Apply suggestions from code review Co-authored-by: Roman Barannyk <53909129+roman-barannyk@users.noreply.github.com> * CIRC-2019 Update src/test/java/api/support/fakes/FakeCQLToJSONInterpreter.java Co-authored-by: Roman Barannyk <53909129+roman-barannyk@users.noreply.github.com> * CIRC-2109 Fix github suggestion commit * CIRC-2109 Code review 1 * CIRC-2109 Revert FakeStorageModuleBuilder --------- Co-authored-by: Roman Barannyk <53909129+roman-barannyk@users.noreply.github.com> --- .../storage/ServicePointRepository.java | 10 +++- .../services/AllowedServicePointsService.java | 2 +- .../folio/circulation/support/Clients.java | 25 ++++++++++ .../support/CollectionResourceClient.java | 6 +-- .../CustomParamCollectionResourceClient.java | 46 +++++++++++++++++++ .../client/IncludeRoutingServicePoints.java | 33 +++++++++++++ .../fakes/FakeCQLToJSONInterpreter.java | 21 +++++++++ .../java/api/support/fakes/FakeOkapi.java | 5 +- .../api/support/fakes/FakeStorageModule.java | 10 ++-- 9 files changed, 148 insertions(+), 10 deletions(-) create mode 100644 src/main/java/org/folio/circulation/support/CustomParamCollectionResourceClient.java create mode 100644 src/main/java/org/folio/circulation/support/http/client/IncludeRoutingServicePoints.java diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java index c7533cd1fc..7225491566 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ServicePointRepository.java @@ -41,7 +41,15 @@ public class ServicePointRepository { private final CollectionResourceClient servicePointsStorageClient; public ServicePointRepository(Clients clients) { - servicePointsStorageClient = clients.servicePointsStorage(); + this(clients, false); + } + + public ServicePointRepository(Clients clients, boolean includeRoutingServicePoints) { + if (includeRoutingServicePoints) { + servicePointsStorageClient = clients.routingServicePointsStorage(); + } else { + servicePointsStorageClient = clients.servicePointsStorage(); + } } public CompletableFuture> getServicePointById(UUID id) { diff --git a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java index dbff3faa44..c5ee898a43 100644 --- a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java +++ b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java @@ -75,7 +75,7 @@ public AllowedServicePointsService(Clients clients, boolean isEcsRequestRouting) userRepository = new UserRepository(clients); requestRepository = new RequestRepository(clients); requestPolicyRepository = new RequestPolicyRepository(clients); - servicePointRepository = new ServicePointRepository(clients); + servicePointRepository = new ServicePointRepository(clients, isEcsRequestRouting); settingsRepository = new SettingsRepository(clients); instanceRepository = new InstanceRepository(clients); itemFinder = new ItemByInstanceIdFinder(clients.holdingsStorage(), itemRepository); diff --git a/src/main/java/org/folio/circulation/support/Clients.java b/src/main/java/org/folio/circulation/support/Clients.java index 671cede9f1..0dcc883a34 100644 --- a/src/main/java/org/folio/circulation/support/Clients.java +++ b/src/main/java/org/folio/circulation/support/Clients.java @@ -4,7 +4,9 @@ import org.folio.circulation.rules.CirculationRulesProcessor; import org.folio.circulation.services.PubSubPublishingService; +import org.folio.circulation.support.http.client.IncludeRoutingServicePoints; import org.folio.circulation.support.http.client.OkapiHttpClient; +import org.folio.circulation.support.http.client.QueryParameter; import org.folio.circulation.support.http.server.WebContext; import io.vertx.core.http.HttpClient; @@ -40,6 +42,7 @@ public class Clients { private final CollectionResourceClient circulationRulesStorageClient; private final CollectionResourceClient requestPoliciesStorageClient; private final CollectionResourceClient servicePointsStorageClient; + private final CollectionResourceClient routingServicePointsStorageClient; private final CollectionResourceClient calendarStorageClient; private final CollectionResourceClient patronGroupsStorageClient; private final CollectionResourceClient patronNoticePolicesStorageClient; @@ -113,6 +116,8 @@ private Clients(OkapiHttpClient client, WebContext context) { requestPoliciesStorageClient = createRequestPoliciesStorageClient(client, context); fixedDueDateSchedulesStorageClient = createFixedDueDateSchedulesStorageClient(client, context); servicePointsStorageClient = createServicePointsStorageClient(client, context); + routingServicePointsStorageClient = createServicePointsStorageWithCustomParam(client, + context, IncludeRoutingServicePoints.enabled()); patronGroupsStorageClient = createPatronGroupsStorageClient(client, context); calendarStorageClient = createCalendarStorageClient(client, context); patronNoticePolicesStorageClient = createPatronNoticePolicesStorageClient(client, context); @@ -246,6 +251,10 @@ public CollectionResourceClient servicePointsStorage() { return servicePointsStorageClient; } + public CollectionResourceClient routingServicePointsStorage() { + return routingServicePointsStorageClient; + } + public CollectionResourceClient patronGroupsStorage() { return patronGroupsStorageClient; } @@ -398,6 +407,14 @@ private static CollectionResourceClient getCollectionResourceClient( return new CollectionResourceClient(client, context.getOkapiBasedUrl(path)); } + private static CollectionResourceClient getCollectionResourceClientWithCustomParam( + OkapiHttpClient client, WebContext context, String path, QueryParameter customParam) + throws MalformedURLException { + + return new CustomParamCollectionResourceClient(client, context.getOkapiBasedUrl(path), + customParam); + } + public CollectionResourceClient noticeTemplatesClient() { return noticeTemplatesClient; } @@ -640,6 +657,14 @@ private CollectionResourceClient createServicePointsStorageClient( return getCollectionResourceClient(client, context, "/service-points"); } + private CollectionResourceClient createServicePointsStorageWithCustomParam( + OkapiHttpClient client, WebContext context, QueryParameter customParam) + throws MalformedURLException { + + return getCollectionResourceClientWithCustomParam(client, context, "/service-points", + customParam); + } + private CollectionResourceClient createPatronGroupsStorageClient( OkapiHttpClient client, WebContext context) throws MalformedURLException { diff --git a/src/main/java/org/folio/circulation/support/CollectionResourceClient.java b/src/main/java/org/folio/circulation/support/CollectionResourceClient.java index a5c827108c..0b3db44c9b 100644 --- a/src/main/java/org/folio/circulation/support/CollectionResourceClient.java +++ b/src/main/java/org/folio/circulation/support/CollectionResourceClient.java @@ -18,8 +18,8 @@ import io.vertx.core.json.JsonObject; public class CollectionResourceClient implements GetManyRecordsClient { - private final OkapiHttpClient client; - private final URL collectionRoot; + final OkapiHttpClient client; + final URL collectionRoot; public CollectionResourceClient(OkapiHttpClient client, URL collectionRoot) { this.collectionRoot = collectionRoot; @@ -109,7 +109,7 @@ public CompletableFuture> getMany(CqlQuery cqlQuery, return client.get(collectionRoot, cqlQuery, pageLimit, offset); } - private String individualRecordUrl(String id) { + String individualRecordUrl(String id) { return format("%s/%s", collectionRoot, id); } } diff --git a/src/main/java/org/folio/circulation/support/CustomParamCollectionResourceClient.java b/src/main/java/org/folio/circulation/support/CustomParamCollectionResourceClient.java new file mode 100644 index 0000000000..2c765e3acf --- /dev/null +++ b/src/main/java/org/folio/circulation/support/CustomParamCollectionResourceClient.java @@ -0,0 +1,46 @@ +package org.folio.circulation.support; + +import java.net.URL; +import java.util.concurrent.CompletableFuture; + +import org.folio.circulation.support.http.client.CqlQuery; +import org.folio.circulation.support.http.client.Offset; +import org.folio.circulation.support.http.client.OkapiHttpClient; +import org.folio.circulation.support.http.client.PageLimit; +import org.folio.circulation.support.http.client.QueryParameter; +import org.folio.circulation.support.http.client.Response; +import org.folio.circulation.support.results.Result; + +public class CustomParamCollectionResourceClient extends CollectionResourceClient { + + private QueryParameter customQueryParameter; + + public CustomParamCollectionResourceClient(OkapiHttpClient client, URL collectionRoot, + QueryParameter customQueryParameter) { + + super(client, collectionRoot); + this.customQueryParameter = customQueryParameter; + } + + @Override + public CompletableFuture> get() { + return client.get(collectionRoot.toString(), customQueryParameter); + } + + @Override + public CompletableFuture> get(PageLimit pageLimit) { + return client.get(collectionRoot, pageLimit, customQueryParameter); + } + + @Override + public CompletableFuture> get(String id) { + return client.get(individualRecordUrl(id), customQueryParameter); + } + + @Override + public CompletableFuture> getMany(CqlQuery cqlQuery, + PageLimit pageLimit, Offset offset) { + + return client.get(collectionRoot, cqlQuery, pageLimit, offset, customQueryParameter); + } +} diff --git a/src/main/java/org/folio/circulation/support/http/client/IncludeRoutingServicePoints.java b/src/main/java/org/folio/circulation/support/http/client/IncludeRoutingServicePoints.java new file mode 100644 index 0000000000..ff9d531cad --- /dev/null +++ b/src/main/java/org/folio/circulation/support/http/client/IncludeRoutingServicePoints.java @@ -0,0 +1,33 @@ +package org.folio.circulation.support.http.client; + +import static java.lang.String.format; + +public class IncludeRoutingServicePoints implements QueryParameter { + + private static final String PARAM_NAME = "includeRoutingServicePoints"; + private final Boolean value; + + public static IncludeRoutingServicePoints enabled() { + return new IncludeRoutingServicePoints(true); + } + + private IncludeRoutingServicePoints(Boolean value) { + this.value = value; + } + + @Override + public void consume(QueryStringParameterConsumer consumer) { + if (value != null) { + consumer.consume(PARAM_NAME, value.toString()); + } + } + + @Override + public String toString() { + if (value == null) { + return format("No %s", PARAM_NAME); + } + + return format("%s = \"%s\"", PARAM_NAME, value); + } +} diff --git a/src/test/java/api/support/fakes/FakeCQLToJSONInterpreter.java b/src/test/java/api/support/fakes/FakeCQLToJSONInterpreter.java index 5f9808ea09..8abd49675e 100644 --- a/src/test/java/api/support/fakes/FakeCQLToJSONInterpreter.java +++ b/src/test/java/api/support/fakes/FakeCQLToJSONInterpreter.java @@ -14,13 +14,34 @@ import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.folio.circulation.support.http.server.WebContext; import io.vertx.core.json.JsonObject; public class FakeCQLToJSONInterpreter { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + public List execute(Collection records, String query, + WebContext context) { + + var initiallyFilteredRecords = execute(records, query); + + // Routing SP filtering + String includeRoutingServicePointsParam = context.getStringParameter( + "includeRoutingServicePoints"); + if (Boolean.parseBoolean(includeRoutingServicePointsParam)) { + return records.stream() + .filter(json -> json.containsKey("ecsRequestRouting") + ? json.getBoolean("ecsRequestRouting") + : false) + .toList(); + } + + return initiallyFilteredRecords; + } + public List execute(Collection records, String query) { + final var queryAndSort = splitQueryAndSort(query); final var cqlPredicate = new CqlPredicate(queryAndSort.left); diff --git a/src/test/java/api/support/fakes/FakeOkapi.java b/src/test/java/api/support/fakes/FakeOkapi.java index ae7ab05909..c9b7c96836 100644 --- a/src/test/java/api/support/fakes/FakeOkapi.java +++ b/src/test/java/api/support/fakes/FakeOkapi.java @@ -23,11 +23,11 @@ import java.util.Objects; import org.apache.commons.lang3.exception.ExceptionUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.folio.circulation.support.ValidationErrorFailure; import org.folio.circulation.support.http.client.OkapiHttpClient; import org.folio.circulation.support.results.Result; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import io.vertx.core.AbstractVerticle; import io.vertx.core.Promise; @@ -282,6 +282,7 @@ public void start(Promise startFuture) throws IOException { .withRootPath("/service-points") .withRequiredProperties("name", "code", "discoveryDisplayName") .withUniqueProperties("name") + .withQueryParameters("includeRoutingServicePoints") .withChangeMetadata() .disallowCollectionDelete() .create() diff --git a/src/test/java/api/support/fakes/FakeStorageModule.java b/src/test/java/api/support/fakes/FakeStorageModule.java index 4cf5b18b69..d0193bd02e 100644 --- a/src/test/java/api/support/fakes/FakeStorageModule.java +++ b/src/test/java/api/support/fakes/FakeStorageModule.java @@ -378,8 +378,8 @@ private void getMany(RoutingContext routingContext) { Map resourcesForTenant = getResourcesForTenant(context); - List filteredItems = new FakeCQLToJSONInterpreter() - .execute(resourcesForTenant.values(), query); + List filteredItems = getFakeCQLToJSONInterpreter() + .execute(resourcesForTenant.values(), query, context); List pagedItems = filteredItems.stream() .skip(offset) @@ -408,6 +408,10 @@ private void getMany(RoutingContext routingContext) { response.end(); } + FakeCQLToJSONInterpreter getFakeCQLToJSONInterpreter() { + return new FakeCQLToJSONInterpreter(); + } + private void empty(RoutingContext routingContext) { WebContext context = new WebContext(routingContext); @@ -430,7 +434,7 @@ private void deleteMany(RoutingContext routingContext) { Map resourcesForTenant = getResourcesForTenant(context); - new FakeCQLToJSONInterpreter() + getFakeCQLToJSONInterpreter() .execute(resourcesForTenant.values(), query) .forEach(item -> resourcesForTenant.remove(item.getString("id"))); From 5bd43b6f64f68d357b698c07412852977671ccef Mon Sep 17 00:00:00 2001 From: Maksat <144414992+Maksat-Galymzhan@users.noreply.github.com> Date: Fri, 19 Jul 2024 20:12:52 +0500 Subject: [PATCH 21/27] CIRC-2117 Return empty result when search doesn't find anything (#1485) * CIRC-2117: return empty if no instance found --- .../infrastructure/storage/SearchRepository.java | 7 ++++--- src/test/java/api/ItemsByInstanceResourceTest.java | 12 ++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index a999c2158d..ed441ce461 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -1,6 +1,7 @@ package org.folio.circulation.infrastructure.storage; import static org.folio.circulation.support.StringUtil.urlEncode; +import static org.folio.circulation.support.results.Result.emptyAsync; import static org.folio.circulation.support.results.Result.failed; import static org.folio.circulation.support.results.ResultBinding.flatMapResult; import static org.folio.circulation.support.results.ResultBinding.mapResult; @@ -62,9 +63,9 @@ private Result> mapResponseToInstances(Response private CompletableFuture> updateItemDetails(SearchInstance searchInstance) { log.debug("updateItemDetails:: searchInstance {}", () -> searchInstance); - if (searchInstance == null) { - return CompletableFuture.completedFuture(failed(new BadRequestFailure( - "Search result is empty"))); + if (searchInstance == null || searchInstance.getId() == null) { + log.info("updateItemDetails:: searchInstance is empty"); + return emptyAsync(); } Map> itemsByTenant = searchInstance.getItems() diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index 71d3ce18b6..08d072d432 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -4,6 +4,7 @@ import static api.support.APITestContext.setTempTenantId; import static api.support.http.InterfaceUrls.itemsByInstanceUrl; import static api.support.matchers.JsonObjectMatcher.hasJsonPath; +import static org.folio.HttpStatus.HTTP_OK; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.hasItem; @@ -77,6 +78,17 @@ void canGetInstanceById() { hasJsonPath("tenantId", is(TENANT_ID_UNIVERSITY))))); } + @Test + void canGetEmptyResult() { + UUID instanceId = UUID.randomUUID(); + + ResourceClient.forSearchClient().replace(instanceId, new JsonObject()); + Response response = get(String.format("query=(id==%s)", instanceId), HTTP_OK.toInt()); + JsonObject responseJson = response.getJson(); + + assertThat(responseJson.isEmpty(), is(true)); + } + private Response get(String query, int expectedStatusCode) { return restAssuredClient.get(itemsByInstanceUrl(query), expectedStatusCode, "items-by-instance-request"); From 1bb48aab6dc1c1faa9f83961ccf8337e30c53f80 Mon Sep 17 00:00:00 2001 From: Magzhan Date: Mon, 22 Jul 2024 20:20:12 +0500 Subject: [PATCH 22/27] [CIRC-2116] Allowed SP endpoint should support patronGroupId parameter (#1484) * CIRC-2116 Allowed SP endpoint should support patronGroupId parameter * CIRC-2116 Allowed SP endpoint should support patronGroupId parameter * CIRC-2116 Allowed SP endpoint should support patronGroupId parameter * CIRC-2116 Allowed SP endpoint should support patronGroupId parameter * CIRC-2116 Allowed SP endpoint should support patronGroupId parameter * CIRC-2116 Allowed SP endpoint should support patronGroupId parameter * CIRC-2116 Allowed SP endpoint should support patronGroupId parameter --- .../domain/AllowedServicePointsRequest.java | 1 + .../requests/RequestPolicyRepository.java | 18 ++--- .../AllowedServicePointsResource.java | 25 ++++--- .../rules/CirculationRuleCriteria.java | 5 +- .../services/AllowedServicePointsService.java | 28 +++++--- .../AllowedServicePointsAPITests.java | 69 +++++++++++++------ 6 files changed, 94 insertions(+), 52 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java b/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java index 16ea46f1ea..c0ec5b2f06 100644 --- a/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java +++ b/src/main/java/org/folio/circulation/domain/AllowedServicePointsRequest.java @@ -17,6 +17,7 @@ public class AllowedServicePointsRequest { private Request.Operation operation; private String requesterId; + private String patronGroupId; private String instanceId; private String itemId; private String requestId; diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java index 7df5c56de1..42a1fe5abf 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestPolicyRepository.java @@ -75,18 +75,18 @@ public CompletableFuture> lookupRequestPolicies(Request request) public CompletableFuture> lookupRequestPolicy(Item item, User user) { log.debug("lookupRequestPolicy:: parameters item: {}, user: {}", item, user); return lookupRequestPolicyId(item, user) - .thenComposeAsync(r -> r.after(this::lookupRequestPolicy)) + .thenComposeAsync(r -> r.after(this::lookupRequestPolicyById)) .thenApply(result -> result.map(RequestPolicy::from)); } public CompletableFuture>>> lookupRequestPolicies( - Collection items, User user) { + Collection items, String patronGroupId) { - log.debug("lookupRequestPolicies:: parameters items: {}, user: {}", - items::size, () -> asJson(user)); + log.debug("lookupRequestPolicies:: parameters items: {}, patronGroupId: {}", + items::size, () -> asJson(patronGroupId)); Map> criteriaMap = items.stream() - .map(item -> new CirculationRuleCriteria(item, user)) + .map(item -> new CirculationRuleCriteria(item, patronGroupId)) .collect(toMap(identity(), criteria -> Set.of(criteria.getItem()), itemsMergeOperator())); return allOf(criteriaMap.entrySet(), entry -> lookupRequestPolicyId(entry.getKey()) @@ -96,12 +96,12 @@ public CompletableFuture>>> lookupRequestPol .thenCompose(r -> r.after(this::lookupRequestPolicies)); } - public CompletableFuture> lookupRequestPolicy(User user) { + public CompletableFuture> lookupRequestPolicy(String patronGroupId) { // Circulation rules need to be executed with the patron group parameter only. // All the item-related parameters should be random UUIDs. - return lookupRequestPolicyId(UUID.randomUUID().toString(), user.getPatronGroupId(), + return lookupRequestPolicyId(UUID.randomUUID().toString(), patronGroupId, UUID.randomUUID().toString(), UUID.randomUUID().toString()) - .thenCompose(r -> r.after(this::lookupRequestPolicy)) + .thenCompose(r -> r.after(this::lookupRequestPolicyById)) .thenApply(result -> result.map(RequestPolicy::from)); } @@ -110,7 +110,7 @@ private BinaryOperator> itemsMergeOperator() { .collect(Collectors.toSet()); } - private CompletableFuture> lookupRequestPolicy( + private CompletableFuture> lookupRequestPolicyById( String requestPolicyId) { log.debug("lookupRequestPolicy:: parameters requestPolicyId: {}", requestPolicyId); diff --git a/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java b/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java index 11fa7dff36..1e96d2b916 100644 --- a/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java +++ b/src/main/java/org/folio/circulation/resources/AllowedServicePointsResource.java @@ -75,28 +75,36 @@ private static Result buildRequest(RoutingContext r String requestId = queryParams.get("requestId"); String useStubItem = queryParams.get("useStubItem"); String ecsRequestRouting = queryParams.get("ecsRequestRouting"); + String patronGroupId = queryParams.get("patronGroupId"); List errors = new ArrayList<>(); // Checking UUID validity if (requesterId != null && !isUuid(requesterId)) { - log.warn("Requester ID is not a valid UUID: {}", requesterId); + log.warn("buildRequest:: Requester ID is not a valid UUID: {}", + requesterId); errors.add(String.format("Requester ID is not a valid UUID: %s.", requesterId)); } + if (patronGroupId != null && !isUuid(patronGroupId)) { + log.warn("buildRequest:: Patron Group ID is not a valid UUID: {}", patronGroupId); + errors.add(String.format("Patron Group ID is not a valid UUID: %s.", + patronGroupId)); + } + if (instanceId != null && !isUuid(instanceId)) { - log.warn("Instance ID is not a valid UUID: {}", requesterId); + log.warn("buildRequest:: Instance ID is not a valid UUID: {}", instanceId); errors.add(String.format("Instance ID is not a valid UUID: %s.", instanceId)); } if (itemId != null && !isUuid(itemId)) { - log.warn("Item ID is not a valid UUID: {}", itemId); + log.warn("buildRequest:: Item ID is not a valid UUID: {}", itemId); errors.add(String.format("Item ID is not a valid UUID: %s.", itemId)); } if (requestId != null && !isUuid(requestId)) { - log.warn("Request ID is not a valid UUID: {}", requestId); + log.warn("buildRequest:: Request ID is not a valid UUID: {}", requestId); errors.add(String.format("Request ID is not a valid UUID: %s.", requestId)); } validateBoolean(useStubItem, "useStubItem", errors); @@ -105,14 +113,14 @@ private static Result buildRequest(RoutingContext r boolean allowedCombinationOfParametersDetected = false; - if (operation == Request.Operation.CREATE && requesterId != null && instanceId != null && + if (operation == Request.Operation.CREATE && (requesterId != null || patronGroupId != null) && instanceId != null && itemId == null && requestId == null) { log.info("validateAllowedServicePointsRequest:: TLR request creation case"); allowedCombinationOfParametersDetected = true; } - if (operation == Request.Operation.CREATE && requesterId != null && instanceId == null && + if (operation == Request.Operation.CREATE && (requesterId != null || patronGroupId != null) && instanceId == null && itemId != null && requestId == null) { log.info("validateAllowedServicePointsRequest:: ILR request creation case"); @@ -144,8 +152,9 @@ private static Result buildRequest(RoutingContext r return failed(new BadRequestFailure(errorMessage)); } - return succeeded(new AllowedServicePointsRequest(operation, requesterId, instanceId, itemId, - requestId, Boolean.parseBoolean(useStubItem), Boolean.parseBoolean(ecsRequestRouting))); + return succeeded(new AllowedServicePointsRequest(operation, requesterId, + patronGroupId, instanceId, itemId, requestId, Boolean.parseBoolean(useStubItem), + Boolean.parseBoolean(ecsRequestRouting))); } private static void validateBoolean(String parameter, String parameterName, List errors) { diff --git a/src/main/java/org/folio/circulation/rules/CirculationRuleCriteria.java b/src/main/java/org/folio/circulation/rules/CirculationRuleCriteria.java index 3052577f4f..29f1d903d5 100644 --- a/src/main/java/org/folio/circulation/rules/CirculationRuleCriteria.java +++ b/src/main/java/org/folio/circulation/rules/CirculationRuleCriteria.java @@ -1,7 +1,6 @@ package org.folio.circulation.rules; import org.folio.circulation.domain.Item; -import org.folio.circulation.domain.User; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -20,11 +19,11 @@ public class CirculationRuleCriteria { @EqualsAndHashCode.Exclude private Item item; - public CirculationRuleCriteria(@NonNull Item item, @NonNull User user) { + public CirculationRuleCriteria(@NonNull Item item, @NonNull String patronGroupId) { this.materialTypeId = item.getMaterialTypeId(); this.loanTypeId = item.getLoanTypeId(); this.locationId = item.getEffectiveLocationId(); - this.patronGroupId = user.getPatronGroupId(); + this.patronGroupId = patronGroupId; this.item = item; } } diff --git a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java index c5ee898a43..66b2ed4831 100644 --- a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java +++ b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java @@ -90,8 +90,9 @@ public AllowedServicePointsService(Clients clients, boolean isEcsRequestRouting) return ofAsync(request) .thenCompose(r -> r.after(this::fetchInstance)) .thenCompose(r -> r.after(this::fetchRequest)) - .thenCompose(r -> r.after(this::fetchUser)) - .thenCompose(r -> r.after(user -> getAllowedServicePoints(request, user))); + .thenCompose(r -> r.after(this::getPatronGroupId)) + .thenCompose(r -> r.after(patronGroupId -> getAllowedServicePoints(request, + patronGroupId))); } private CompletableFuture> fetchInstance( @@ -128,26 +129,33 @@ private CompletableFuture> fetchRequest( .thenApply(r -> r.map(allowedServicePointsRequest::updateWithRequestInformation)); } - private CompletableFuture> fetchUser(AllowedServicePointsRequest request) { + private CompletableFuture> getPatronGroupId(AllowedServicePointsRequest request) { + + if (request.getPatronGroupId() != null) { + return ofAsync(request.getPatronGroupId()); + } + final String userId = request.getRequesterId(); return userRepository.getUser(userId) .thenApply(r -> r.failWhen( user -> succeeded(user == null), - user -> notFoundValidationFailure(userId, User.class))); + user -> notFoundValidationFailure(userId, User.class))) + .thenApply(result -> result.map(User::getPatronGroupId)); } private CompletableFuture>>> - getAllowedServicePoints(AllowedServicePointsRequest request, User user) { + getAllowedServicePoints(AllowedServicePointsRequest request, String patronGroupId) { - log.debug("getAllowedServicePoints:: parameters request: {}, user: {}", request, user); + log.debug("getAllowedServicePoints:: parameters request: {}, patronGroupId: {}", request, patronGroupId); return fetchItems(request) - .thenCompose(r -> r.after(items -> getAllowedServicePoints(request, user, items))); + .thenCompose(r -> r.after(items -> getAllowedServicePoints(request, patronGroupId, items))); } private CompletableFuture>>> - getAllowedServicePoints(AllowedServicePointsRequest request, User user, Collection items) { + getAllowedServicePoints(AllowedServicePointsRequest request, String patronGroupId, + Collection items) { if (items.isEmpty() && request.isForTitleLevelRequest()) { log.info("getAllowedServicePoints:: requested instance has no items"); @@ -160,12 +168,12 @@ private CompletableFuture> fetchUser(AllowedServicePointsRequest re : this::extractAllowedServicePointsConsideringItemStatus; if (request.isUseStubItem()) { - return requestPolicyRepository.lookupRequestPolicy(user) + return requestPolicyRepository.lookupRequestPolicy(patronGroupId) .thenCompose(r -> r.after(policy -> extractAllowedServicePointsIgnoringItemStatus( policy, new HashSet<>()))); } - return requestPolicyRepository.lookupRequestPolicies(items, user) + return requestPolicyRepository.lookupRequestPolicies(items, patronGroupId) .thenCompose(r -> r.after(policies -> allOf(policies, mappingFunction))) .thenApply(r -> r.map(this::combineAllowedServicePoints)); // TODO: remove irrelevant request types for REPLACE diff --git a/src/test/java/api/requests/AllowedServicePointsAPITests.java b/src/test/java/api/requests/AllowedServicePointsAPITests.java index 2446e910e0..1407c97b35 100644 --- a/src/test/java/api/requests/AllowedServicePointsAPITests.java +++ b/src/test/java/api/requests/AllowedServicePointsAPITests.java @@ -138,6 +138,7 @@ void shouldReturnListOfAllowedServicePointsForRequest(RequestType requestType, List allowedSpInResponse) { var requesterId = usersFixture.steve().getId().toString(); + var patronGroupId = patronGroupsFixture.regular().getId().toString(); var items = itemsFixture.createMultipleItemForTheSameInstance(1, List.of(ib -> ib.withStatus(itemStatus.getValue()))); var item = items.get(0); @@ -155,8 +156,18 @@ void shouldReturnListOfAllowedServicePointsForRequest(RequestType requestType, .collect(Collectors.toSet())); var response = requestLevel == TITLE - ? get("create", requesterId, instanceId, null, null, null, null, HttpStatus.SC_OK).getJson() - : get("create", requesterId, null, itemId, null, null, null, HttpStatus.SC_OK).getJson(); + ? get("create", requesterId, null, instanceId, null, null, null, null, + HttpStatus.SC_OK).getJson() + : get("create", requesterId, null, null, itemId, null, null, null, + HttpStatus.SC_OK).getJson(); + + assertThat(response, allowedServicePointMatcher(Map.of(requestType, allowedSpInResponse))); + + response = requestLevel == TITLE + ? get("create", null, patronGroupId, instanceId, null, null, null, null, + HttpStatus.SC_OK).getJson() + : get("create", null, patronGroupId, null, itemId, null, null, null, + HttpStatus.SC_OK).getJson(); assertThat(response, allowedServicePointMatcher(Map.of(requestType, allowedSpInResponse))); } @@ -224,7 +235,8 @@ void shouldReturnListOfAllowedServicePointsForRequestReplacement( var requestId = request == null ? null : request.getId().toString(); var response = - get("replace", null, null, null, requestId, null, null, HttpStatus.SC_OK).getJson(); + get("replace", null, null, null, null, requestId, null, null, + HttpStatus.SC_OK).getJson(); assertThat(response, allowedServicePointMatcher(Map.of(requestType, allowedSpInResponse))); } @@ -654,7 +666,7 @@ void allowedServicePointsAreSortedByName() { @Test void getReplaceFailsWhenRequestDoesNotExist() { String requestId = randomId(); - Response response = get("replace", null, null, null, requestId, null, + Response response = get("replace", null, null, null, null, requestId, null, null, HttpStatus.SC_UNPROCESSABLE_ENTITY); assertThat(response.getJson(), hasErrorWith(hasMessage( String.format("Request with ID %s was not found", requestId)))); @@ -664,7 +676,7 @@ void getReplaceFailsWhenRequestDoesNotExist() { void getMoveFailsWhenRequestDoesNotExist() { String requestId = randomId(); String itemId = itemsFixture.basedUponNod().getId().toString(); - Response response = get("move", null, null, itemId, requestId, null, + Response response = get("move", null, null, null, itemId, requestId, null, null, HttpStatus.SC_UNPROCESSABLE_ENTITY); assertThat(response.getJson(), hasErrorWith(hasMessage( String.format("Request with ID %s was not found", requestId)))); @@ -726,57 +738,63 @@ void shouldReturnListOfAllowedServicePointsForRequestMove(RequestLevel requestLe // Valid "move" request var moveResponse = - get("move", null, null, itemToMoveToId, requestId, null, null, HttpStatus.SC_OK).getJson(); + get("move", null, null, null, itemToMoveToId, requestId, null, null, + HttpStatus.SC_OK).getJson(); assertThat(moveResponse, allowedServicePointMatcher(Map.of(HOLD, List.of(sp2)))); // Invalid "move" requests - var invalidMoveResponse1 = get("move", null, null, null, requestId, + var invalidMoveResponse1 = get("move", null, null, null, null, requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse1.getBody(), equalTo("Invalid combination of query parameters")); - var invalidMoveResponse2 = get("move", null, null, itemToMoveToId, null, + var invalidMoveResponse2 = get("move", null, null, null, itemToMoveToId, + null, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse2.getBody(), equalTo("Invalid combination of query parameters")); - var invalidMoveResponse3 = get("move", null, null, null, null, + var invalidMoveResponse3 = get("move", null, null, null, null, null, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse3.getBody(), equalTo("Invalid combination of query parameters")); - var invalidMoveResponse4 = get("move", requesterId, null, itemToMoveToId, requestId, + var invalidMoveResponse4 = get("move", requesterId,null, null, itemToMoveToId, requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse4.getBody(), equalTo("Invalid combination of query parameters")); - var invalidMoveResponse5 = get("move", null, instanceId, itemToMoveToId, requestId, + var invalidMoveResponse5 = get("move", null, null, instanceId, + itemToMoveToId, requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidMoveResponse5.getBody(), equalTo("Invalid combination of query parameters")); // Valid "replace" request var replaceResponse = - get("replace", null, null, null, requestId, null, null, HttpStatus.SC_OK).getJson(); + get("replace", null, null, null, null, requestId, null, null, + HttpStatus.SC_OK).getJson(); assertThat(replaceResponse, allowedServicePointMatcher(Map.of(HOLD, List.of(sp2)))); // Invalid "replace" requests - var invalidReplaceResponse1 = get("replace", null, null, null, null, + var invalidReplaceResponse1 = get("replace", null, null, null, null, null, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse1.getBody(), equalTo("Invalid combination of query parameters")); - var invalidReplaceResponse2 = get("replace", requesterId, null, null, requestId, + var invalidReplaceResponse2 = get("replace", requesterId,null, null, null, requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse2.getBody(), equalTo("Invalid combination of query parameters")); - var invalidReplaceResponse3 = get("replace", null, instanceId, null, requestId, + var invalidReplaceResponse3 = get("replace", null, null, instanceId, null + , requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse3.getBody(), equalTo("Invalid combination of query parameters")); - var invalidReplaceResponse4 = get("replace", null, null, requestedItemId, requestId, + var invalidReplaceResponse4 = get("replace", null, null, null, + requestedItemId, requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse4.getBody(), equalTo("Invalid combination of query parameters")); - var invalidReplaceResponse5 = get("replace", requesterId, instanceId, + var invalidReplaceResponse5 = get("replace", requesterId, null, instanceId, requestedItemId, requestId, null, null, HttpStatus.SC_BAD_REQUEST); assertThat(invalidReplaceResponse5.getBody(), equalTo("Invalid combination of query parameters")); @@ -899,28 +917,35 @@ private void assertServicePointsMatch(JsonArray response, private Response getCreateOp(String requesterId, String instanceId, String itemId, String useStubItem, String ecsRequestRouting, int expectedStatusCode) { - return get("create", requesterId, instanceId, itemId, null, useStubItem, + return get("create", requesterId,null, instanceId, itemId, null, useStubItem, ecsRequestRouting, expectedStatusCode); } private Response getCreateOp(String requesterId, String instanceId, String itemId, int expectedStatusCode) { - return get("create", requesterId, instanceId, itemId, null, null, null, expectedStatusCode); + return get("create", requesterId, null, instanceId, itemId, null, null, null, + expectedStatusCode); } private Response getReplaceOp(String requestId, int expectedStatusCode) { - return get("replace", null, null, null, requestId, null, null, expectedStatusCode); + return get("replace", null, null, null, null, requestId, null, null, + expectedStatusCode); } - private Response get(String operation, String requesterId, String instanceId, String itemId, - String requestId, String useStubItem, String ecsRequestRouting, int expectedStatusCode) { + private Response get(String operation, String requesterId, + String patronGroupId, String instanceId, String itemId, + String requestId, String useStubItem, String ecsRequestRouting, + int expectedStatusCode) { List queryParams = new ArrayList<>(); queryParams.add(namedParameter("operation", operation)); if (requesterId != null) { queryParams.add(namedParameter("requesterId", requesterId)); } + if (patronGroupId != null) { + queryParams.add(namedParameter("patronGroupId", patronGroupId)); + } if (instanceId != null) { queryParams.add(namedParameter("instanceId", instanceId)); } From e9f33990453e55903772976a822fbbda29bcbc8a Mon Sep 17 00:00:00 2001 From: OleksandrVidinieiev <56632770+OleksandrVidinieiev@users.noreply.github.com> Date: Fri, 9 Aug 2024 14:06:09 +0300 Subject: [PATCH 23/27] CIRC-2125: Search title-level requests by both `itemId` and `instanceId` during check-in and check-out (#1487) * CIRC-2125 Search title-level requests also by itemId during check-in and check-out * CIRC-2125 Return empty queue if both itemId and instanceId are null * CIRC-2125 Fix security issues --- .../requests/RequestQueueRepository.java | 47 +++++++++++++------ .../resources/CheckInProcessAdapter.java | 3 +- .../resources/CheckOutByBarcodeResource.java | 14 +++++- .../resources/RequestQueueResource.java | 7 +-- .../services/RequestQueueService.java | 3 +- .../support/http/client/CqlQuery.java | 16 +++++++ .../api/loans/CheckOutByBarcodeTests.java | 40 ++++++++++++++++ .../api/support/builders/InstanceBuilder.java | 2 +- .../fixtures/CirculationItemsFixture.java | 15 ++++-- .../support/http/client/CqlQueryTests.java | 34 ++++++++++++++ 10 files changed, 154 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestQueueRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestQueueRepository.java index bcfa69eec3..e4ecf9fdcb 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestQueueRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestQueueRepository.java @@ -1,5 +1,6 @@ package org.folio.circulation.infrastructure.storage.requests; +import static java.util.Collections.emptyList; import static java.util.concurrent.CompletableFuture.completedFuture; import static org.folio.circulation.domain.RequestLevel.ITEM; import static org.folio.circulation.domain.RequestLevel.TITLE; @@ -7,13 +8,16 @@ import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; import static org.folio.circulation.support.http.client.CqlQuery.exactMatchAny; import static org.folio.circulation.support.http.client.PageLimit.oneThousand; +import static org.folio.circulation.support.results.Result.ofAsync; import static org.folio.circulation.support.results.Result.succeeded; import static org.folio.circulation.support.results.ResultBinding.mapResult; -import static org.folio.circulation.support.utils.LogUtil.collectionAsString; import java.lang.invoke.MethodHandles; import java.util.Collection; +import java.util.EnumSet; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; @@ -60,9 +64,10 @@ public CompletableFuture> get(RequestAndRelated public CompletableFuture> getQueue(TlrSettingsConfiguration tlrSettings, String instanceId, String itemId) { - return tlrSettings != null && tlrSettings.isTitleLevelRequestsFeatureEnabled() - ? getByInstanceId(instanceId) - : getByItemId(itemId); + boolean isTlrEnabled = tlrSettings != null && tlrSettings.isTitleLevelRequestsFeatureEnabled(); + log.info("getQueue:: TLR feature is {}", isTlrEnabled ? "enabled" : "disabled"); + + return isTlrEnabled ? getByInstanceId(instanceId) : getByItemId(itemId); } public CompletableFuture> get(RenewalContext context) { @@ -74,33 +79,47 @@ public CompletableFuture> get(RenewalContext context) { ).thenApply(result -> result.map(context::withRequestQueue)); } + public CompletableFuture> getByInstanceIdAndItemId(String instanceId, + String itemId) { + + return get(itemId, instanceId, EnumSet.of(ITEM, TITLE)); + } + public CompletableFuture> getByInstanceId(String instanceId) { - return get("instanceId", instanceId, List.of(ITEM, TITLE)); + return get(null, instanceId, EnumSet.of(ITEM, TITLE)); } public CompletableFuture> getByItemId(String itemId) { - return get("itemId", itemId, List.of(ITEM)); + return get(itemId, null, EnumSet.of(ITEM)); } - private CompletableFuture> get(String idFieldName, String id, - Collection requestLevels) { + private CompletableFuture> get(String itemId, String instanceId, + EnumSet requestLevels) { - log.debug("get:: parameters idFieldName: {}, id: {}, requestLevels: {}", - () -> idFieldName, () -> id, () -> collectionAsString(requestLevels)); + Map filters = new HashMap<>(); + if (itemId != null) { + filters.put("itemId", itemId); + } + if (instanceId != null) { + filters.put("instanceId", instanceId); + } + if (filters.isEmpty()) { + log.info("get:: itemId and instanceId are null, returning an empty queue"); + return ofAsync(new RequestQueue(emptyList())); + } List requestLevelStrings = requestLevels.stream() .map(RequestLevel::getValue) .collect(Collectors.toList()); - final Result itemIdQuery = exactMatch(idFieldName, id); final Result statusQuery = exactMatchAny("status", RequestStatus.openStates()); final Result requestLevelQuery = exactMatchAny("requestLevel", requestLevelStrings); - return itemIdQuery.combine(statusQuery, CqlQuery::and) + return CqlQuery.exactMatchAny(filters) + .combine(statusQuery, CqlQuery::and) .combine(requestLevelQuery, CqlQuery::and) .map(q -> q.sortBy(ascending("position"))) - .after(query -> requestRepository.findBy(query, - MAXIMUM_SUPPORTED_REQUEST_QUEUE_SIZE)) + .after(q -> requestRepository.findBy(q, MAXIMUM_SUPPORTED_REQUEST_QUEUE_SIZE)) .thenApply(r -> r.map(MultipleRecords::getRecords)) .thenApply(r -> r.map(RequestQueue::new)); } diff --git a/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java b/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java index 9e8b396bb4..f721f48863 100644 --- a/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java +++ b/src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java @@ -173,7 +173,8 @@ CompletableFuture> getRequestQueue(CheckInContext context) return requestQueueRepository.getByItemId(context.getItem().getItemId()); } else { - return requestQueueRepository.getByInstanceId(context.getItem().getInstanceId()); + return requestQueueRepository.getByInstanceIdAndItemId(context.getItem().getInstanceId(), + context.getItem().getItemId()); } } diff --git a/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java b/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java index 94e16b6721..57e144f47f 100644 --- a/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java +++ b/src/main/java/org/folio/circulation/resources/CheckOutByBarcodeResource.java @@ -22,6 +22,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.CheckOutLock; +import org.folio.circulation.domain.Item; import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.LoanAndRelatedRecords; import org.folio.circulation.domain.LoanRepresentation; @@ -156,7 +157,8 @@ private void checkOut(RoutingContext routingContext) { .thenComposeAsync(validators::refuseWhenItemHasOpenLoans) .thenComposeAsync(r -> r.combineAfter(settingsRepository::lookupTlrSettings, LoanAndRelatedRecords::withTlrSettings)) - .thenComposeAsync(r -> r.after(requestQueueRepository::get)) + .thenComposeAsync(r -> r.combineAfter(l -> getRequestQueue(l, requestQueueRepository), + LoanAndRelatedRecords::withRequestQueue)) .thenCompose(validators::refuseWhenRequestedByAnotherPatron) .thenComposeAsync(r -> r.after(l -> lookupLoanPolicy(l, loanPolicyRepository, errorHandler))) .thenComposeAsync(validators::refuseWhenItemLimitIsReached) @@ -390,4 +392,14 @@ private Result calculateDefaultInitialDueDate( .map(loan::changeDueDate) .map(loanAndRelatedRecords::withLoan); } + + private CompletableFuture> getRequestQueue( + LoanAndRelatedRecords loanAndRelatedRecords, RequestQueueRepository requestQueueRepository) { + + Item item = loanAndRelatedRecords.getItem(); + + return loanAndRelatedRecords.getTlrSettings().isTitleLevelRequestsFeatureEnabled() + ? requestQueueRepository.getByInstanceIdAndItemId(item.getInstanceId(), item.getItemId()) + : requestQueueRepository.getByItemId(item.getItemId()); + } } diff --git a/src/main/java/org/folio/circulation/resources/RequestQueueResource.java b/src/main/java/org/folio/circulation/resources/RequestQueueResource.java index 430684b83d..059eb284c5 100644 --- a/src/main/java/org/folio/circulation/resources/RequestQueueResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestQueueResource.java @@ -98,10 +98,7 @@ private void getQueue(RoutingContext routingContext, RequestQueueType requestQue final RequestRepresentation requestRepresentation = new RequestRepresentation(); - CompletableFuture> requestQueue = getRequestQueueByType(routingContext, - requestQueueType, requestQueueRepository); - - requestQueue + getRequestQueueByType(routingContext, requestQueueType, requestQueueRepository) .thenApply(r -> r.map(queue -> new MultipleRecords<>(queue.getRequests(), queue.size()))) .thenApply(r -> r.map(requests -> requests.asJson(requestRepresentation::extendedRepresentation, "requests"))) @@ -133,8 +130,6 @@ private void reorderQueue(RoutingContext routingContext, RequestQueueType reques requestQueueRepository, requestRepository, new ServicePointRepository(clients), configurationRepository, RequestQueueService.using(clients), new CalendarRepository(clients)); - getRequestQueueByType(routingContext, requestQueueType, requestQueueRepository); - validateTlrFeatureStatus(settingsRepository, requestQueueType, idParamValue) .thenCompose(r -> r.after(tlrSettings -> getRequestQueueByType(routingContext, requestQueueType, requestQueueRepository))) diff --git a/src/main/java/org/folio/circulation/services/RequestQueueService.java b/src/main/java/org/folio/circulation/services/RequestQueueService.java index d110aee4fd..32d3a2b4b3 100644 --- a/src/main/java/org/folio/circulation/services/RequestQueueService.java +++ b/src/main/java/org/folio/circulation/services/RequestQueueService.java @@ -74,7 +74,8 @@ private CompletableFuture> isItemLevelRequestFulfillableByItem(I protected CompletableFuture> isTitleLevelRequestFulfillableByItem(Item item, Request request) { - if (!StringUtils.equals(request.getInstanceId(), item.getInstanceId())) { + if (!StringUtils.equals(request.getItemId(), item.getItemId()) && + !StringUtils.equals(request.getInstanceId(), item.getInstanceId())) { return ofAsync(false); } diff --git a/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java b/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java index e8f87d6e0a..368d09e170 100644 --- a/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java +++ b/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java @@ -13,7 +13,9 @@ import java.net.URLEncoder; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.folio.circulation.support.CqlSortBy; @@ -69,6 +71,20 @@ public static Result exactMatchAny(String indexName, Collection exactMatchAny(Map indicesToValues) { + String rawQuery = indicesToValues.entrySet() + .stream() + .filter(entry -> entry.getValue() != null) + .map(entry -> String.format("%s==\"%s\"", entry.getKey(), entry.getValue())) + .collect(Collectors.joining(" or ")); + + if (rawQuery.isEmpty()) { + return failedDueToServerError("Cannot generate empty CQL query"); + } + + return Result.of(() -> new CqlQuery("(" + rawQuery + ")", none())); + } + /** * Uses greater than ('>'), as not equals operator ('<>') is not supported in CQL at present */ diff --git a/src/test/java/api/loans/CheckOutByBarcodeTests.java b/src/test/java/api/loans/CheckOutByBarcodeTests.java index b7107e04b3..3735304f64 100644 --- a/src/test/java/api/loans/CheckOutByBarcodeTests.java +++ b/src/test/java/api/loans/CheckOutByBarcodeTests.java @@ -36,6 +36,7 @@ import static api.support.matchers.RequestMatchers.hasPosition; import static api.support.matchers.RequestMatchers.isClosedFilled; import static api.support.matchers.RequestMatchers.isOpenAwaitingPickup; +import static api.support.matchers.RequestMatchers.isOpenInTransit; import static api.support.matchers.ResponseStatusCodeMatcher.hasStatus; import static api.support.matchers.TextDateTimeMatcher.isEquivalentTo; import static api.support.matchers.TextDateTimeMatcher.withinSecondsAfter; @@ -2752,6 +2753,45 @@ void concurrentCheckoutsWhenCheckoutLockFeatureEnabled() throws InterruptedExcep contains("Patron has reached maximum limit of 1 items for loan type"))); } + @Test + void circulationItemCheckOutUpdatesPrimaryEcsRequestStatus() { + settingsFixture.enableTlrFeature(); + UUID itemId = UUID.randomUUID(); + String itemBarcode = "item_barcode"; + UUID pickupServicePointId = servicePointsFixture.cd1().getId(); + UserResource requester = usersFixture.steve(); + IndividualResource realInstance = instancesFixture.basedUponDunkirk(); + + // place title-level hold on instance with no items + IndividualResource initialRequest = requestsFixture.placeTitleLevelHoldShelfRequest( + realInstance.getId(), requester, ZonedDateTime.now(), pickupServicePointId); + UUID requestId = initialRequest.getId(); + + // create circulation item which has same ID as the "real" item, but different holdingsId, instanceId, etc. + UUID dcbInstanceId = UUID.randomUUID(); + IndividualResource dcbHoldings = holdingsFixture.defaultWithHoldings(dcbInstanceId); + final IndividualResource circulationItem = circulationItemsFixture.createCirculationItem( + itemId, itemBarcode, dcbHoldings.getId(), locationsFixture.mainFloor().getId(), "DCB instance"); + + // update request same way DCB does it when a borrowing transaction is created + requestsStorageClient.replace(requestId, + requestsStorageClient.get(requestId) + .getJson() + .put("itemId", itemId.toString()) + .put("holdingsRecordId", dcbHoldings.getId().toString()) + .put("item", new JsonObject().put("barcode", itemBarcode))); + + UUID randomServicePointId = servicePointsFixture.cd2().getId(); + checkInFixture.checkInByBarcode(circulationItem, randomServicePointId); + assertThat(requestsFixture.getById(requestId).getJson(), isOpenInTransit()); + + checkInFixture.checkInByBarcode(circulationItem, pickupServicePointId); + assertThat(requestsFixture.getById(requestId).getJson(), isOpenAwaitingPickup()); + + checkOutFixture.checkOutByBarcode(circulationItem, requester); + assertThat(requestsFixture.getById(requestId).getJson(), isClosedFilled()); + } + private IndividualResource placeRequest(String requestLevel, ItemResource item, IndividualResource requester) { diff --git a/src/test/java/api/support/builders/InstanceBuilder.java b/src/test/java/api/support/builders/InstanceBuilder.java index 2d9c80ca73..b849c16a34 100644 --- a/src/test/java/api/support/builders/InstanceBuilder.java +++ b/src/test/java/api/support/builders/InstanceBuilder.java @@ -27,7 +27,7 @@ public InstanceBuilder(String title, UUID instanceTypeId) { this(UUID.randomUUID(), title, instanceTypeId); } - private InstanceBuilder(UUID id, String title, UUID instanceTypeId) { + public InstanceBuilder(UUID id, String title, UUID instanceTypeId) { this(id, title, new JsonArray(), instanceTypeId, Collections.emptyList(), new JsonArray(), new JsonArray()); } diff --git a/src/test/java/api/support/fixtures/CirculationItemsFixture.java b/src/test/java/api/support/fixtures/CirculationItemsFixture.java index 2444d9d89e..74c248a9c4 100644 --- a/src/test/java/api/support/fixtures/CirculationItemsFixture.java +++ b/src/test/java/api/support/fixtures/CirculationItemsFixture.java @@ -21,9 +21,18 @@ public CirculationItemsFixture( } public IndividualResource createCirculationItem(String barcode, UUID holdingId, UUID locationId, String instanceTitle) { - CirculationItemsBuilder circulationItemsBuilder = new CirculationItemsBuilder().withBarcode(barcode).withHoldingId(holdingId) - .withLoanType(loanTypesFixture.canCirculate().getId()).withMaterialType(materialTypesFixture.book().getId()) - .withLocationId(locationId).withInstanceTitle(instanceTitle); + return createCirculationItem(UUID.randomUUID(), barcode, holdingId, locationId, instanceTitle); + } + + public IndividualResource createCirculationItem(UUID itemId, String barcode, UUID holdingId, UUID locationId, String instanceTitle) { + CirculationItemsBuilder circulationItemsBuilder = new CirculationItemsBuilder() + .withItemId(itemId) + .withBarcode(barcode) + .withHoldingId(holdingId) + .withLoanType(loanTypesFixture.canCirculate().getId()) + .withMaterialType(materialTypesFixture.book().getId()) + .withLocationId(locationId) + .withInstanceTitle(instanceTitle); return circulationItemClient.create(circulationItemsBuilder); } diff --git a/src/test/java/org/folio/circulation/support/http/client/CqlQueryTests.java b/src/test/java/org/folio/circulation/support/http/client/CqlQueryTests.java index 349b294343..0ddc0e11a7 100644 --- a/src/test/java/org/folio/circulation/support/http/client/CqlQueryTests.java +++ b/src/test/java/org/folio/circulation/support/http/client/CqlQueryTests.java @@ -2,6 +2,7 @@ import static java.lang.String.format; import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; import static org.folio.circulation.support.CqlSortBy.ascending; import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; import static org.folio.circulation.support.http.client.CqlQuery.exactMatchAny; @@ -12,15 +13,20 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import java.time.ZonedDateTime; import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; import org.folio.circulation.support.CqlSortBy; import org.folio.circulation.support.CqlSortClause; import org.folio.circulation.support.ServerErrorFailure; import org.folio.circulation.support.results.Result; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; class CqlQueryTests { @Test @@ -153,4 +159,32 @@ void shouldBeEqualToQueryWithSameDefinition() { assertThat(firstQuery.equals(secondQuery), is(true)); } + + @ParameterizedTest + @CsvSource(nullValues={"null"}, value = { + "index1, value1, index2, value2, (index1==\"value1\" or index2==\"value2\")", + "null, null, index2, value2, (index2==\"value2\")", + "index1, value1, null, null, (index1==\"value1\")", + }) + void canMatchByAnyIndex(String index1, String value1, String index2, String value2, + String expectedResult) { + + Map filters = new HashMap<>(); + filters.put(index1, value1); + filters.put(index2, value2); + + String actualResult = exactMatchAny(filters) + .value() + .asText(); + assertThat(actualResult, equalTo(expectedResult)); + } + + @Test + void exactMatchAnyFailsWhenListOfIndicesIsEmpty() { + Result result = exactMatchAny(emptyMap()); + assertThat("Failed result expected", result.failed()); + assertThat(result.cause(), instanceOf(ServerErrorFailure.class)); + assertThat(((ServerErrorFailure) result.cause()).getReason(), + equalTo("Cannot generate empty CQL query")); + } } From c53bbcd1c0a1cb9e440629f80b6e5809d84ee7e5 Mon Sep 17 00:00:00 2001 From: OleksandrVidinieiev <56632770+OleksandrVidinieiev@users.noreply.github.com> Date: Thu, 29 Aug 2024 18:41:22 +0300 Subject: [PATCH 24/27] CIRC-2137 Allow operation `replace` for instance with no items (#1489) --- .../services/AllowedServicePointsService.java | 10 +++++----- .../requests/AllowedServicePointsAPITests.java | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java index 66b2ed4831..ef86ae8b28 100644 --- a/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java +++ b/src/main/java/org/folio/circulation/services/AllowedServicePointsService.java @@ -157,11 +157,6 @@ private CompletableFuture> getPatronGroupId(AllowedServicePointsR getAllowedServicePoints(AllowedServicePointsRequest request, String patronGroupId, Collection items) { - if (items.isEmpty() && request.isForTitleLevelRequest()) { - log.info("getAllowedServicePoints:: requested instance has no items"); - return getAllowedServicePointsForTitleWithNoItems(request); - } - BiFunction, CompletableFuture>>>> mappingFunction = request.isImplyingItemStatusIgnore() ? this::extractAllowedServicePointsIgnoringItemStatus @@ -173,6 +168,11 @@ private CompletableFuture> getPatronGroupId(AllowedServicePointsR policy, new HashSet<>()))); } + if (items.isEmpty() && request.isForTitleLevelRequest()) { + log.info("getAllowedServicePoints:: requested instance has no items"); + return getAllowedServicePointsForTitleWithNoItems(request); + } + return requestPolicyRepository.lookupRequestPolicies(items, patronGroupId) .thenCompose(r -> r.after(policies -> allOf(policies, mappingFunction))) .thenApply(r -> r.map(this::combineAllowedServicePoints)); diff --git a/src/test/java/api/requests/AllowedServicePointsAPITests.java b/src/test/java/api/requests/AllowedServicePointsAPITests.java index 1407c97b35..189f25927c 100644 --- a/src/test/java/api/requests/AllowedServicePointsAPITests.java +++ b/src/test/java/api/requests/AllowedServicePointsAPITests.java @@ -241,6 +241,22 @@ void shouldReturnListOfAllowedServicePointsForRequestReplacement( assertThat(response, allowedServicePointMatcher(Map.of(requestType, allowedSpInResponse))); } + @Test + void shouldReturnListOfAllowedServicePointsForHoldRequestReplacementWhenInstanceHasNoItems() { + settingsFixture.configureTlrFeature(true, false, null, null, null); + var requester = usersFixture.steve(); + var instanceId = instancesFixture.basedUponDunkirk().getId(); + var servicePointId = servicePointsFixture.cd1().getId(); + setRequestPolicyWithAllowedServicePoints(HOLD, Set.of(servicePointId)); + IndividualResource request = requestsFixture.placeTitleLevelHoldShelfRequest( + instanceId, requester, ZonedDateTime.now(), servicePointId); + + var response = get("replace", null, null, null, null, request.getId().toString(), "true", null, + HttpStatus.SC_OK).getJson(); + var expectedServicePoint = new AllowedServicePoint(servicePointId.toString(), "Circ Desk 1"); + assertThat(response, allowedServicePointMatcher(Map.of(HOLD, List.of(expectedServicePoint)))); + } + public static Object[] shouldReturnOnlyExistingAllowedServicePointForRequestParameters() { String sp1Id = randomId(); String sp2Id = randomId(); From 0907443a7e72c1e31021377a06750ebd74a2bbe7 Mon Sep 17 00:00:00 2001 From: Alexander Kurash Date: Fri, 27 Sep 2024 18:59:24 +0300 Subject: [PATCH 25/27] CIRC-2151 Change ECS Primary request validation (#1496) * CIRC-2149 Primary phase - bypass some validation * CIRC-2149 Add logging * CIRC-2149 Add instance items logging * CIRC-2149 Increase limit for holdings * CIRC-2149 Primary ECS TLR Hold - no policy check * CIRC-2151 Add a test * CIRC-2151 Fix tests * CIRC-2151 Test policy check skipping for ECS Hold * CIRC-2151 Remove unneeded assert * CIRC-2151 Fix code smells --- .../domain/CreateRequestService.java | 5 + .../circulation/domain/EcsRequestPhase.java | 37 +++++++ .../org/folio/circulation/domain/Request.java | 5 + .../representations/RequestProperties.java | 1 + .../resources/RequestCollectionResource.java | 8 ++ .../RequestFromRepresentationService.java | 13 ++- .../services/ItemForTlrService.java | 7 ++ .../storage/ItemByInstanceIdFinder.java | 13 ++- .../requests/RequestsAPICreationTests.java | 104 ++++++++++++++++++ .../requests/RequestsAPIRetrievalTests.java | 1 - src/test/java/api/support/APITestContext.java | 2 +- .../api/support/builders/RequestBuilder.java | 9 +- .../java/api/support/fakes/FakeOkapi.java | 2 +- .../api/support/fakes/FakeSearchModule.java | 6 +- 14 files changed, 204 insertions(+), 9 deletions(-) create mode 100644 src/main/java/org/folio/circulation/domain/EcsRequestPhase.java diff --git a/src/main/java/org/folio/circulation/domain/CreateRequestService.java b/src/main/java/org/folio/circulation/domain/CreateRequestService.java index 3893a9dd3a..66ec8fff43 100644 --- a/src/main/java/org/folio/circulation/domain/CreateRequestService.java +++ b/src/main/java/org/folio/circulation/domain/CreateRequestService.java @@ -227,6 +227,11 @@ private CompletableFuture> checkPolicy( boolean tlrFeatureEnabled = request.getTlrSettingsConfiguration().isTitleLevelRequestsFeatureEnabled(); if (tlrFeatureEnabled && request.isTitleLevel() && request.isHold()) { + if (request.getEcsRequestPhase() == EcsRequestPhase.PRIMARY) { + log.warn("checkPolicy:: ECS TLR primary Hold detected, skipping policy check"); + return ofAsync(() -> records); + } + log.info("checkPolicy:: checking policy for title-level hold"); return completedFuture(checkPolicyForTitleLevelHold(records)); } diff --git a/src/main/java/org/folio/circulation/domain/EcsRequestPhase.java b/src/main/java/org/folio/circulation/domain/EcsRequestPhase.java new file mode 100644 index 0000000000..8272931b2e --- /dev/null +++ b/src/main/java/org/folio/circulation/domain/EcsRequestPhase.java @@ -0,0 +1,37 @@ +package org.folio.circulation.domain; + +import static org.apache.commons.lang3.StringUtils.equalsIgnoreCase; + +import java.util.Arrays; + +public enum EcsRequestPhase { + NONE(""), + PRIMARY("Primary"), + SECONDARY("Secondary"); + + public final String value; + + public static EcsRequestPhase from(String value) { + return Arrays.stream(values()) + .filter(status -> status.nameMatches(value)) + .findFirst() + .orElse(NONE); + } + + EcsRequestPhase(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + public boolean nameMatches(String value) { + return equalsIgnoreCase(getValue(), value); + } + + @Override + public String toString() { + return value; + } +} diff --git a/src/main/java/org/folio/circulation/domain/Request.java b/src/main/java/org/folio/circulation/domain/Request.java index 0774075a5b..7497d32d33 100644 --- a/src/main/java/org/folio/circulation/domain/Request.java +++ b/src/main/java/org/folio/circulation/domain/Request.java @@ -17,6 +17,7 @@ import static org.folio.circulation.domain.representations.RequestProperties.CANCELLATION_REASON_ID; import static org.folio.circulation.domain.representations.RequestProperties.CANCELLATION_REASON_NAME; import static org.folio.circulation.domain.representations.RequestProperties.CANCELLATION_REASON_PUBLIC_DESCRIPTION; +import static org.folio.circulation.domain.representations.RequestProperties.ECS_REQUEST_PHASE; import static org.folio.circulation.domain.representations.RequestProperties.HOLDINGS_RECORD_ID; import static org.folio.circulation.domain.representations.RequestProperties.HOLD_SHELF_EXPIRATION_DATE; import static org.folio.circulation.domain.representations.RequestProperties.INSTANCE_ID; @@ -261,6 +262,10 @@ public RequestType getRequestType() { return RequestType.from(getProperty(requestRepresentation, REQUEST_TYPE)); } + public EcsRequestPhase getEcsRequestPhase() { + return EcsRequestPhase.from(getProperty(requestRepresentation, ECS_REQUEST_PHASE)); + } + boolean allowedForItem() { return RequestTypeItemStatusWhiteList.canCreateRequestForItem(getItem().getStatus(), getRequestType()); } diff --git a/src/main/java/org/folio/circulation/domain/representations/RequestProperties.java b/src/main/java/org/folio/circulation/domain/representations/RequestProperties.java index cf3be67cb1..c571d6b314 100644 --- a/src/main/java/org/folio/circulation/domain/representations/RequestProperties.java +++ b/src/main/java/org/folio/circulation/domain/representations/RequestProperties.java @@ -9,6 +9,7 @@ private RequestProperties() { } public static final String HOLDINGS_RECORD_ID = "holdingsRecordId"; public static final String REQUEST_LEVEL = "requestLevel"; public static final String REQUEST_TYPE = "requestType"; + public static final String ECS_REQUEST_PHASE = "ecsRequestPhase"; public static final String PROXY_USER_ID = "proxyUserId"; public static final String POSITION = "position"; public static final String HOLD_SHELF_EXPIRATION_DATE = "holdShelfExpirationDate"; diff --git a/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java b/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java index 3f19b9241d..572d973396 100644 --- a/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java @@ -8,6 +8,10 @@ import static org.folio.circulation.support.results.MappingFunctions.toFixedValue; import static org.folio.circulation.support.results.MappingFunctions.when; +import java.lang.invoke.MethodHandles; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.CreateRequestService; import org.folio.circulation.domain.MoveRequestProcessAdapter; import org.folio.circulation.domain.MoveRequestService; @@ -57,6 +61,8 @@ import io.vertx.ext.web.RoutingContext; public class RequestCollectionResource extends CollectionResource { + private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); + public RequestCollectionResource(HttpClient client) { super(client, "/circulation/requests"); } @@ -74,6 +80,8 @@ void create(RoutingContext routingContext) { final var representation = routingContext.getBodyAsJson(); + log.info("create:: {}", representation); + final var eventPublisher = new EventPublisher(routingContext); RequestRelatedRepositories repositories = new RequestRelatedRepositories(clients); diff --git a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java index 04bc011498..3762c7d927 100644 --- a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java +++ b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java @@ -51,6 +51,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.folio.circulation.domain.EcsRequestPhase; import org.folio.circulation.domain.Item; import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.MultipleRecords; @@ -252,7 +253,12 @@ private CompletableFuture> fetchItemAndLoan( Request request = records.getRequest(); Function>> itemAndLoanFetchingFunction; - if (request.isTitleLevel() && request.isPage()) { + log.info("fetchItemAndLoan:: Request phase is {}", request.getEcsRequestPhase().value); + if (request.getEcsRequestPhase() == EcsRequestPhase.PRIMARY) { + log.info("fetchItemAndLoan:: Primary ECS request detected, using default item fetcher"); + itemAndLoanFetchingFunction = this::fetchItemAndLoanDefault; + } + else if (request.isTitleLevel() && request.isPage()) { itemAndLoanFetchingFunction = this::fetchItemAndLoanForPageTlr; } else if (request.isTitleLevel() && request.isRecall()) { @@ -537,6 +543,11 @@ private Result refuseToCreateTlrLinkedToAnItem(Result request) } private Result validateAbsenceOfItemLinkInTlr(Request request) { + if (request.getEcsRequestPhase() == EcsRequestPhase.PRIMARY) { + log.info("validateAbsenceOfItemLinkInTlr:: Primary ECS request detected, skipping"); + return of(() -> request); + } + String itemId = request.getItemId(); String holdingsRecordId = request.getHoldingsRecordId(); diff --git a/src/main/java/org/folio/circulation/services/ItemForTlrService.java b/src/main/java/org/folio/circulation/services/ItemForTlrService.java index 3a306991a7..d385c39842 100644 --- a/src/main/java/org/folio/circulation/services/ItemForTlrService.java +++ b/src/main/java/org/folio/circulation/services/ItemForTlrService.java @@ -1,5 +1,6 @@ package org.folio.circulation.services; +import static java.lang.String.format; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; @@ -17,6 +18,7 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.function.Function; +import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -41,6 +43,11 @@ public static ItemForTlrService using(RequestRelatedRepositories repositories) { } public List findAvailablePageableItems(Request request) { + log.info("findAvailablePageableItems:: instance items: {}", + () -> request.getInstanceItems().stream() + .map(item -> format("(%s, %s, %s)", item.getItemId(), item.getBarcode(), item.getStatus().getValue())) + .collect(Collectors.joining(", ")) + ); return request.getInstanceItems() .stream() diff --git a/src/main/java/org/folio/circulation/storage/ItemByInstanceIdFinder.java b/src/main/java/org/folio/circulation/storage/ItemByInstanceIdFinder.java index 7cdeebf960..511e443125 100644 --- a/src/main/java/org/folio/circulation/storage/ItemByInstanceIdFinder.java +++ b/src/main/java/org/folio/circulation/storage/ItemByInstanceIdFinder.java @@ -7,23 +7,29 @@ import static org.folio.circulation.support.json.JsonKeys.byId; import static org.folio.circulation.support.results.Result.succeeded; +import java.lang.invoke.MethodHandles; import java.util.Collection; import java.util.List; import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.Item; import org.folio.circulation.domain.MultipleRecords; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.support.FindWithCqlQuery; import org.folio.circulation.support.GetManyRecordsClient; import org.folio.circulation.support.http.client.CqlQuery; +import org.folio.circulation.support.http.client.PageLimit; import org.folio.circulation.support.results.Result; import io.vertx.core.json.JsonObject; public class ItemByInstanceIdFinder { + private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); private final GetManyRecordsClient holdingsStorageClient; private final ItemRepository itemRepository; @@ -41,7 +47,8 @@ public CompletableFuture>> getItemsByInstanceId(UUID ins final FindWithCqlQuery fetcher = findWithCqlQuery( holdingsStorageClient, "holdingsRecords", identity()); - return fetcher.findByQuery(CqlQuery.exactMatch("instanceId", instanceId.toString())) + return fetcher.findByQuery(CqlQuery.exactMatch("instanceId", instanceId.toString()), + PageLimit.oneThousand()) .thenCompose(r -> getItems(r, failWhenNoHoldingsRecordsFound)); } @@ -50,6 +57,10 @@ private CompletableFuture>> getItems( boolean failWhenNoHoldingsRecordsFound) { return holdingsRecordsResult.after(holdingsRecords -> { + log.info("getItems:: holdings records: {}", holdingsRecords.getRecords().stream() + .map(h -> h.getString("id")) + .collect(Collectors.joining(", "))); + if (holdingsRecords == null || holdingsRecords.isEmpty()) { if (failWhenNoHoldingsRecordsFound) { return completedFuture(failedValidation( diff --git a/src/test/java/api/requests/RequestsAPICreationTests.java b/src/test/java/api/requests/RequestsAPICreationTests.java index 4dcad03e2f..b92753935b 100644 --- a/src/test/java/api/requests/RequestsAPICreationTests.java +++ b/src/test/java/api/requests/RequestsAPICreationTests.java @@ -3815,6 +3815,110 @@ void titleLevelPageRequestIsCreatedForItemClosestToPickupServicePoint(int testCa assertThat(request.getJson().getString("itemId"), is(expectedItemId)); } + @Test + void primaryTlrCreationSkipsClosestServicePointLogicAndPoliciesIgnoredForHoldTlr() { + settingsFixture.configureTlrFeature(true, true, null, null, null); + + policiesActivation.use(new RequestPolicyBuilder( + UUID.randomUUID(), + List.of(PAGE), + "Test request policy", + "Test description", + null + )); + + UUID pickupServicePointId = servicePointsFixture.create(new ServicePointBuilder( + "Pickup service point", "PICKUP", "Display name") + .withPickupLocation(Boolean.TRUE)) + .getId(); + + UUID anotherServicePointId = servicePointsFixture.create(new ServicePointBuilder( + "Another service point", "OTHER", "Display name") + .withPickupLocation(Boolean.TRUE)) + .getId(); + + UUID institutionId = locationsFixture.createInstitution("Institution").getId(); + UUID campusIdA = locationsFixture.createCampus("Campus A", institutionId).getId(); + UUID campusIdB = locationsFixture.createCampus("Campus B", institutionId).getId(); + UUID libraryIdA1 = locationsFixture.createLibrary("Library A1", campusIdA).getId(); + UUID libraryIdB1 = locationsFixture.createLibrary("Library B1", campusIdB).getId(); + + UUID sameLibraryLocationId = locationsFixture.createLocation(new LocationBuilder() + .withName("Location in same library") + .withCode("3") + .forInstitution(institutionId) + .forCampus(campusIdA) + .forLibrary(libraryIdA1) + .withPrimaryServicePoint(anotherServicePointId) + .servedBy(anotherServicePointId)) + .getId(); + + UUID anotherLibraryLocationId = locationsFixture.createLocation(new LocationBuilder() + .withName("Location in another library") + .withCode("3") + .forInstitution(institutionId) + .forCampus(campusIdB) + .forLibrary(libraryIdB1) + .withPrimaryServicePoint(anotherServicePointId) + .servedBy(anotherServicePointId)) + .getId(); + + UUID instanceId = instancesFixture.basedUponDunkirk().getId(); + UUID holdingsId = holdingsFixture.defaultWithHoldings(instanceId).getId(); + + // Closest item + UUID closestItemId = itemsFixture.basedUponDunkirkWithCustomHoldingAndLocation( + holdingsId, sameLibraryLocationId).getId(); + + UUID expectedItemId = itemsFixture.basedUponDunkirkWithCustomHoldingAndLocation( + holdingsId, anotherLibraryLocationId).getId(); + + var requestBuilder = new RequestBuilder() + .page() + .fulfillToHoldShelf() + .titleRequestLevel() + .withInstanceId(instanceId) + .withItemId(expectedItemId) + .withHoldingsRecordId(holdingsId) + .withRequestDate(ZonedDateTime.now()) + .withRequesterId(usersFixture.steve().getId()) + .withPickupServicePointId(pickupServicePointId); + + // Request without ECS phase should fail + requestsFixture.attemptPlace(requestBuilder); + + // The same request with Primary ECS phase should succeed because validation is skipped + IndividualResource request = requestsFixture.place( + requestBuilder.withEcsRequestPhase("Primary")); + + assertThat(request.getJson().getString("itemId"), is(expectedItemId)); + + // To make sure there are no Available items left + requestsFixture.place( + requestBuilder + .withRequesterId(usersFixture.jessica().getId()) + .withItemId(closestItemId) + .withEcsRequestPhase("Primary")); + + // Placing TLR Hold request + var requestBuilderTlrHold = new RequestBuilder() + .hold() + .fulfillToHoldShelf() + .titleRequestLevel() + .withInstanceId(instanceId) + .withNoHoldingsRecordId() + .withNoItemId() + .withRequestDate(ZonedDateTime.now()) + .withRequesterId(usersFixture.steve().getId()) + .withPickupServicePointId(pickupServicePointId); + + // Request without ECS phase should fail + requestsFixture.attemptPlace(requestBuilderTlrHold); + + // The same request with Primary ECS phase should succeed because policy check is skipped + requestsFixture.place(requestBuilderTlrHold.withEcsRequestPhase("Primary")); + } + @Test void pageTlrSucceedsWhenClosestAvailableItemIsNotPageable() { // Page TLR should succeed when multiple available items exist, but the closest item to the diff --git a/src/test/java/api/requests/RequestsAPIRetrievalTests.java b/src/test/java/api/requests/RequestsAPIRetrievalTests.java index 17fdb28469..642e402be4 100644 --- a/src/test/java/api/requests/RequestsAPIRetrievalTests.java +++ b/src/test/java/api/requests/RequestsAPIRetrievalTests.java @@ -37,7 +37,6 @@ import api.support.APITests; import api.support.MultipleJsonRecords; -import api.support.TlrFeatureStatus; import api.support.builders.Address; import api.support.builders.ItemBuilder; import api.support.builders.RequestBuilder; diff --git a/src/test/java/api/support/APITestContext.java b/src/test/java/api/support/APITestContext.java index 0f7890eb34..c007163b71 100644 --- a/src/test/java/api/support/APITestContext.java +++ b/src/test/java/api/support/APITestContext.java @@ -60,7 +60,7 @@ public class APITestContext { private static final int PORT = nextFreePort(); private static String fakeOkapiDeploymentId; - private static Boolean useOkapiForStorage; + private static Boolean useOkapiForStorage = false; private static Boolean useOkapiForInitialRequests; static String getToken() { diff --git a/src/test/java/api/support/builders/RequestBuilder.java b/src/test/java/api/support/builders/RequestBuilder.java index 2514482a1e..4fc3b4115f 100644 --- a/src/test/java/api/support/builders/RequestBuilder.java +++ b/src/test/java/api/support/builders/RequestBuilder.java @@ -63,6 +63,7 @@ public class RequestBuilder extends JsonBuilder implements Builder { private final Tags tags; private final String patronComments; private final BlockOverrides blockOverrides; + private final String ecsRequestPhase; public RequestBuilder() { this(UUID.randomUUID(), @@ -89,6 +90,7 @@ public RequestBuilder() { null, null, null, + null, null); } @@ -120,7 +122,8 @@ public static RequestBuilder from(IndividualResource response) { getUUIDProperty(representation, "pickupServicePointId"), new Tags((toStream(representation.getJsonObject("tags"), "tagList").collect(toList()))), getProperty(representation, "patronComments"), - null + null, + getProperty(representation, "ecsRequestPhase") ); } @@ -191,6 +194,10 @@ public JsonObject create() { } } + if (ecsRequestPhase != null) { + put(request, "ecsRequestPhase", ecsRequestPhase); + } + return request; } diff --git a/src/test/java/api/support/fakes/FakeOkapi.java b/src/test/java/api/support/fakes/FakeOkapi.java index c9b7c96836..97c8d29b71 100644 --- a/src/test/java/api/support/fakes/FakeOkapi.java +++ b/src/test/java/api/support/fakes/FakeOkapi.java @@ -233,7 +233,7 @@ public void start(Promise startFuture) throws IOException { new FakeSearchModule().register(router); new FakeStorageModuleBuilder() - .withRecordName(FakeSearchModule.recordTypeName) + .withRecordName(FakeSearchModule.RECORD_TYPE_NAME) .withRootPath("/search/instances") .create().register(router); diff --git a/src/test/java/api/support/fakes/FakeSearchModule.java b/src/test/java/api/support/fakes/FakeSearchModule.java index db1e07319a..e4c4eafd45 100644 --- a/src/test/java/api/support/fakes/FakeSearchModule.java +++ b/src/test/java/api/support/fakes/FakeSearchModule.java @@ -27,7 +27,7 @@ public class FakeSearchModule { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); - public static final String recordTypeName = "search-instance"; + public static final String RECORD_TYPE_NAME = "search-instance"; private static final String ID_REGEXP = "id\\s*(==|!=|>|>=|<|<=|\\|=|\\|=)\\s*" + "([a-f0-9\\-]+)"; private final Storage storage; @@ -66,7 +66,7 @@ private void getById(RoutingContext routingContext) { searchResult.put("totalRecords", 1); searchResult.put("instances", List.of(resourceRepresentation)); - log.debug("Found {} resource: {}", recordTypeName, + log.debug("Found {} resource: {}", RECORD_TYPE_NAME, searchResult.encodePrettily()); HttpServerResponse response = routingContext.response(); @@ -80,7 +80,7 @@ private void getById(RoutingContext routingContext) { response.end(); } else { - log.debug("Failed to find {} resource: {}", recordTypeName, + log.debug("Failed to find {} resource: {}", RECORD_TYPE_NAME, idParsingResult); ClientErrorResponse.notFound(routingContext.response()); From 50a3e6fae6c778c3bdc964bdc5e76b8b2b02b956 Mon Sep 17 00:00:00 2001 From: alexanderkurash Date: Fri, 18 Oct 2024 23:59:48 +0300 Subject: [PATCH 26/27] Fix tests after merging with master branch --- src/test/java/api/loans/CheckInByBarcodeTests.java | 4 ++-- src/test/java/api/requests/RequestsAPICreationTests.java | 4 ++-- src/test/java/api/requests/StaffSlipsTests.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/api/loans/CheckInByBarcodeTests.java b/src/test/java/api/loans/CheckInByBarcodeTests.java index 827afb8362..6df1466060 100644 --- a/src/test/java/api/loans/CheckInByBarcodeTests.java +++ b/src/test/java/api/loans/CheckInByBarcodeTests.java @@ -1666,7 +1666,7 @@ void linkItemToHoldTLRWithHoldShelfWhenCheckedInItemThenFulfilledWithSuccess(){ @Test void checkInItemWhenServicePointHasChangedToNoPickupLocation() { - configurationsFixture.enableTlrFeature(); + reconfigureTlrFeature(TlrFeatureStatus.ENABLED); var instanceId = instancesFixture.basedUponDunkirk().getId(); var defaultWithHoldings = holdingsFixture.defaultWithHoldings(instanceId); var checkedOutItem = itemsClient.create(buildCheckedOutItemWithHoldingRecordsId( @@ -1683,7 +1683,7 @@ void checkInItemWhenServicePointHasChangedToNoPickupLocation() { ServicePointBuilder changedServicePoint = new ServicePointBuilder( servicePointsFixture.cd1().getId(), servicePointName, servicePointCode, discoveryDisplayName, - description, shelvingLagTime, Boolean.FALSE, null, KEEP_THE_CURRENT_DUE_DATE.name()); + description, shelvingLagTime, Boolean.FALSE, null, KEEP_THE_CURRENT_DUE_DATE.name(), false); // Update existing service point servicePointsFixture.update(servicePointCode, changedServicePoint); diff --git a/src/test/java/api/requests/RequestsAPICreationTests.java b/src/test/java/api/requests/RequestsAPICreationTests.java index fbc18bf18a..e0ad4032ed 100644 --- a/src/test/java/api/requests/RequestsAPICreationTests.java +++ b/src/test/java/api/requests/RequestsAPICreationTests.java @@ -645,7 +645,7 @@ void createTitleLevelRequestWhenTlrEnabledSetLocation(String locationCode) { final var items = itemsFixture.createMultipleItemsForTheSameInstance(2); UUID instanceId = items.get(0).getInstanceId(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); IndividualResource requestResource = requestsClient.create(new RequestBuilder() .page() @@ -669,7 +669,7 @@ void createTitleLevelRequestWhenTlrEnabledSetLocationNoItems() { final var items = itemsFixture.createMultipleItemsForTheSameInstance(2); UUID instanceId = items.get(0).getInstanceId(); - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); Response response = requestsClient.attemptCreate( new RequestBuilder() diff --git a/src/test/java/api/requests/StaffSlipsTests.java b/src/test/java/api/requests/StaffSlipsTests.java index d4be9194cc..190143a52d 100644 --- a/src/test/java/api/requests/StaffSlipsTests.java +++ b/src/test/java/api/requests/StaffSlipsTests.java @@ -546,7 +546,7 @@ void responseContainsSlipsWhenServicePointHasManyLocations(SlipsType slipsType) @Test void responseContainsSearchSlipsForTLR() { - configurationsFixture.enableTlrFeature(); + settingsFixture.enableTlrFeature(); var servicePointId = servicePointsFixture.cd1().getId(); var steve = usersFixture.steve(); var instance = instancesFixture.basedUponDunkirk(); From 179730836c920449727a5b1f9a479a170db196a2 Mon Sep 17 00:00:00 2001 From: Oleksandr Vidinieiev Date: Fri, 8 Nov 2024 11:57:03 +0200 Subject: [PATCH 27/27] CIRC-2153 Post-merge fixes --- descriptors/ModuleDescriptor-template.json | 2 +- .../folio/circulation/resources/RequestCollectionResource.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 689e529e11..94d99bd635 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -665,7 +665,7 @@ }, { "id": "allowed-service-points", - "version": "1.2", + "version": "1.1", "handlers": [ { "methods": [ diff --git a/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java b/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java index 572d973396..a222818735 100644 --- a/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestCollectionResource.java @@ -80,7 +80,7 @@ void create(RoutingContext routingContext) { final var representation = routingContext.getBodyAsJson(); - log.info("create:: {}", representation); + log.debug("create:: {}", representation); final var eventPublisher = new EventPublisher(routingContext);