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
Expand Up @@ -649,7 +649,7 @@
},
{
"id": "allowed-service-points",
"version": "1.1",
"version": "1.2",
"handlers": [
{
"methods": [
Expand Down
4 changes: 4 additions & 0 deletions ramls/circulation.raml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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<>();

Expand All @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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>>>>
Expand Down Expand Up @@ -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())));
Expand All @@ -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)
Expand Down
106 changes: 81 additions & 25 deletions src/test/java/api/requests/AllowedServicePointsAPITests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
Expand Down Expand Up @@ -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)));
}
Expand Down Expand Up @@ -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))));
}
Expand All @@ -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))));
}
Expand Down Expand Up @@ -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"));
}
Expand All @@ -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());
Expand All @@ -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) {

Expand All @@ -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));
Expand All @@ -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");
Expand Down
Loading