Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CIRC-2051 Add ecsRequestRouting parameter to allowed-service-points #1451

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion descriptors/ModuleDescriptor-template.json
Original file line number Diff line number Diff line change
@@ -649,7 +649,7 @@
},
{
"id": "allowed-service-points",
"version": "1.1",
"version": "1.2",
"handlers": [
{
"methods": [
4 changes: 4 additions & 0 deletions ramls/circulation.raml
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -206,22 +206,24 @@ public CompletableFuture<Result<Collection<ServicePoint>>> findServicePointsById
.thenApply(r -> r.map(MultipleRecords::getRecords));
}

public CompletableFuture<Result<Collection<ServicePoint>>> fetchPickupLocationServicePoints() {
public CompletableFuture<Result<Collection<ServicePoint>>> 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<Result<Collection<ServicePoint>>> fetchPickupLocationServicePointsByIds(
Set<String> ids) {
public CompletableFuture<Result<Collection<ServicePoint>>>
fetchPickupLocationServicePointsByIdsAndIndexName(Set<String> ids, String indexName) {

log.debug("filterIdsByServicePointsAndPickupLocationExistence:: parameters ids: {}",
() -> collectionAsString(ids));
log.debug("filterIdsByServicePointsAndPickupLocationExistence:: parameters ids: {}, " +
"indexName: {}", () -> collectionAsString(ids), () -> indexName);

Result<CqlQuery> pickupLocationQuery = exactMatch("pickupLocation", "true");
Result<CqlQuery> pickupLocationQuery = exactMatch(indexName, "true");

return createServicePointsFetcher().findByIdIndexAndQuery(ids, "id", pickupLocationQuery)
.thenApply(r -> r.map(MultipleRecords::getRecords));
Original file line number Diff line number Diff line change
@@ -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<AllowedServicePointsRequest> buildRequest(RoutingContext r
String itemId = queryParams.get("itemId");
String requestId = queryParams.get("requestId");
String useStubItem = queryParams.get("useStubItem");
String ecsRequestRouting = queryParams.get("ecsRequestRouting");

List<String> errors = new ArrayList<>();

@@ -94,11 +96,8 @@ private static Result<AllowedServicePointsRequest> 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<AllowedServicePointsRequest> 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<String> 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<RequestType, Set<AllowedServicePoint>> allowedServicePoints) {
Original file line number Diff line number Diff line change
@@ -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<Result<Map<RequestType, Set<AllowedServicePoint>>>>
@@ -360,7 +364,7 @@ private Map<RequestType, Set<AllowedServicePoint>> combineAllowedServicePoints(
}

private CompletableFuture<Result<Set<AllowedServicePoint>>> 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<Result<Set<AllowedServicePoint>>> 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)
106 changes: 81 additions & 25 deletions src/test/java/api/requests/AllowedServicePointsAPITests.java
Original file line number Diff line number Diff line change
@@ -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<RequestType, Set<UUID>> 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<IndividualResource> 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<QueryStringParameter> 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");
Loading