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 @@ -72,6 +72,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 Down Expand Up @@ -99,6 +100,13 @@ private static Result<AllowedServicePointsRequest> buildRequest(RoutingContext r
useStubItem);
errors.add(String.format("useStubItem is not a valid boolean: %s.", useStubItem));
}
if (ecsRequestRouting != null && !"true".equals(ecsRequestRouting)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are validating boolean string twice now, do you think this check deserves a separate method? Something like isIvalidBooleanString(String boolean)

&& !"false".equals(ecsRequestRouting)) {

log.warn("validateAllowedServicePointsRequest:: ecsRequestRouting is not a valid boolean: {}",
ecsRequestRouting);
errors.add(String.format("ecsRequestRouting is not a valid boolean: %s.", ecsRequestRouting));
}
// Checking parameter combinations
boolean allowedCombinationOfParametersDetected = false;

Expand Down Expand Up @@ -137,7 +145,7 @@ 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 JsonObject toJson(Map<RequestType, Set<AllowedServicePoint>> allowedServicePoints) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,20 +145,25 @@ private CompletableFuture<Result<User>> fetchUser(AllowedServicePointsRequest re
private CompletableFuture<Result<Map<RequestType, Set<AllowedServicePoint>>>>
getAllowedServicePoints(AllowedServicePointsRequest request, User user, Collection<Item> items) {

String indexName = request.isEcsRequestRouting()
? "ecsRequestRouting"
: "pickupLocation";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are now searching either by ecsRequestRouting or pickupLocation. Do we not care about the value of pickupLocation when fetching ECS routing service points?

if (items.isEmpty() && request.isForTitleLevelRequest()) {
log.info("getAllowedServicePoints:: requested instance has no items");
return getAllowedServicePointsForTitleWithNoItems(request);
return getAllowedServicePointsForTitleWithNoItems(request, indexName);
}

BiFunction<RequestPolicy, Set<Item>, CompletableFuture<Result<Map<RequestType,
Set<AllowedServicePoint>>>>> mappingFunction = request.isImplyingItemStatusIgnore()
? this::extractAllowedServicePointsIgnoringItemStatus
: this::extractAllowedServicePointsConsideringItemStatus;
? (requestPolicy, itemsSet) -> extractAllowedServicePointsIgnoringItemStatus(
requestPolicy, itemsSet, indexName)
: (requestPolicy, itemsSet) -> extractAllowedServicePointsConsideringItemStatus(
requestPolicy, itemsSet, indexName);

if (request.isUseStubItem()) {
return requestPolicyRepository.lookupRequestPolicy(user)
.thenCompose(r -> r.after(policy -> extractAllowedServicePointsIgnoringItemStatus(
policy, new HashSet<>())));
policy, new HashSet<>(), indexName)));
}

return requestPolicyRepository.lookupRequestPolicies(items, user)
Expand All @@ -168,20 +173,20 @@ private CompletableFuture<Result<User>> fetchUser(AllowedServicePointsRequest re
}

private CompletableFuture<Result<Map<RequestType, Set<AllowedServicePoint>>>>
getAllowedServicePointsForTitleWithNoItems(AllowedServicePointsRequest request) {
getAllowedServicePointsForTitleWithNoItems(AllowedServicePointsRequest request, String indexName) {

if (request.isForTitleLevelRequest() && request.getOperation() == CREATE) {
log.info("getAllowedServicePointsForTitleWithNoItems:: checking TLR settings");
return configurationRepository.lookupTlrSettings()
.thenCompose(r -> r.after(this::considerTlrSettings));
.thenCompose(r -> r.after(tlrSettings -> considerTlrSettings(tlrSettings, indexName)));
}

log.info("getAllowedServicePointsForTitleWithNoItems:: no need to check TLR-settings");
return ofAsync(emptyMap());
}

private CompletableFuture<Result<Map<RequestType, Set<AllowedServicePoint>>>> considerTlrSettings(
TlrSettingsConfiguration tlrSettings) {
TlrSettingsConfiguration tlrSettings, String indexName) {

if (!tlrSettings.isTitleLevelRequestsFeatureEnabled() ||
tlrSettings.isTlrHoldShouldFollowCirculationRules()) {
Expand All @@ -192,7 +197,7 @@ private CompletableFuture<Result<Map<RequestType, Set<AllowedServicePoint>>>> co

log.info("considerTlrSettings:: allowing all pickup locations for Hold");

return fetchAllowedServicePoints()
return fetchAllowedServicePoints(indexName)
.thenApply(r -> r.map(sp -> sp.isEmpty() ? emptyMap() : Map.of(HOLD, sp)));
}

Expand Down Expand Up @@ -231,20 +236,22 @@ private Result<Item> refuseIfItemIsNotFound(Item item, AllowedServicePointsReque
}

private CompletableFuture<Result<Map<RequestType, Set<AllowedServicePoint>>>>
extractAllowedServicePointsIgnoringItemStatus(RequestPolicy requestPolicy, Set<Item> items) {
extractAllowedServicePointsIgnoringItemStatus(RequestPolicy requestPolicy, Set<Item> items,
String indexName) {

return extractAllowedServicePoints(requestPolicy, items, true);
return extractAllowedServicePoints(requestPolicy, items, true, indexName);
}

private CompletableFuture<Result<Map<RequestType, Set<AllowedServicePoint>>>>
extractAllowedServicePointsConsideringItemStatus(RequestPolicy requestPolicy, Set<Item> items) {
extractAllowedServicePointsConsideringItemStatus(RequestPolicy requestPolicy, Set<Item> items,
String indexName) {

return extractAllowedServicePoints(requestPolicy, items, false);
return extractAllowedServicePoints(requestPolicy, items, false, indexName);
}

private CompletableFuture<Result<Map<RequestType, Set<AllowedServicePoint>>>>
extractAllowedServicePoints(RequestPolicy requestPolicy, Set<Item> items,
boolean ignoreItemStatus) {
boolean ignoreItemStatus, String indexName) {

log.debug("extractAllowedServicePoints:: parameters requestPolicy: {}, items: {}, " +
"ignoreItemStatus: {}", requestPolicy, items, ignoreItemStatus);
Expand Down Expand Up @@ -280,22 +287,22 @@ private Result<Item> refuseIfItemIsNotFound(Item item, AllowedServicePointsReque
.collect(Collectors.toCollection(ArrayList::new)); // collect into a mutable list

// TODO: fetch service points on a later stage, we only need IDs here
return fetchServicePoints(requestTypesAllowedByPolicy, servicePointAllowedByPolicy)
return fetchServicePoints(requestTypesAllowedByPolicy, servicePointAllowedByPolicy, indexName)
.thenApply(r -> r.map(servicePoints -> groupAllowedServicePointsByRequestType(
requestTypesAllowedByPolicyAndStatus, servicePoints, servicePointAllowedByPolicy)));
}

private CompletableFuture<Result<Set<AllowedServicePoint>>> fetchServicePoints(
List<RequestType> requestTypesAllowedByPolicy,
Map<RequestType, Set<String>> servicePointsAllowedByPolicy) {
Map<RequestType, Set<String>> servicePointsAllowedByPolicy, String indexName) {

Set<String> allowedServicePointsIds = servicePointsAllowedByPolicy.values().stream()
.flatMap(Collection::stream)
.collect(Collectors.toSet());

return requestTypesAllowedByPolicy.size() == servicePointsAllowedByPolicy.size()
? fetchPickupLocationServicePointsByIds(allowedServicePointsIds)
: fetchAllowedServicePoints();
? fetchPickupLocationServicePointsByIds(allowedServicePointsIds, indexName)
: fetchAllowedServicePoints(indexName);
}

private Map<RequestType, Set<AllowedServicePoint>> groupAllowedServicePointsByRequestType(
Expand Down Expand Up @@ -359,20 +366,22 @@ private Map<RequestType, Set<AllowedServicePoint>> combineAllowedServicePoints(
}, () -> new EnumMap<>(RequestType.class)));
}

private CompletableFuture<Result<Set<AllowedServicePoint>>> fetchAllowedServicePoints() {
return servicePointRepository.fetchPickupLocationServicePoints()
private CompletableFuture<Result<Set<AllowedServicePoint>>> fetchAllowedServicePoints(
String indexName) {

return servicePointRepository.fetchServicePointsByIndexName(indexName)
.thenApply(r -> r.map(servicePoints -> servicePoints.stream()
.map(AllowedServicePoint::new)
.collect(Collectors.toSet())));
}

private CompletableFuture<Result<Set<AllowedServicePoint>>> fetchPickupLocationServicePointsByIds(
Set<String> ids) {
Set<String> ids, String indexName) {

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
Loading