Skip to content

Commit

Permalink
Support paging in /items-by-holdings-id API (MODINV-838) (#603)
Browse files Browse the repository at this point in the history 
* Support paging in /items-by-holdings-id API (MODINV-838)
  • Loading branch information
@nielserik
nielserik authored Jul 12, 2023
1 parent dfa12b8 commit 022755f
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 67 deletions.
3 changes: 2 additions & 1 deletion ramls/inventory.raml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ resourceTypes:
schemaItem: item
exampleItem: !include examples/items_get.json
get:
is: [searchable: {description: "query by holdings record ID. This is a mandatory query parameter.
is: [pageable: {description: "The default page size is 200. Maximum page size is 10000."},
searchable: {description: "query by holdings record ID. This is a mandatory query parameter.
An optional parameter, 'relations', can be passed outside of the query to restrict what
Items are returned based on their type of relationship with the holdings record.
Possible values of the 'relations' parameter are: 'onlyBoundWiths', 'onlyBoundWithsSkipDirectlyLinkedItem'",
Expand Down
118 changes: 55 additions & 63 deletions src/main/java/org/folio/inventory/resources/ItemsByHoldingsRecordId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
import io.vertx.ext.web.Router;
import io.vertx.ext.web.RoutingContext;
import io.vertx.ext.web.client.WebClient;
import org.apache.commons.lang.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.inventory.common.WebContext;
import org.folio.inventory.common.api.request.PagingParameters;
import org.folio.inventory.storage.Storage;
import org.folio.inventory.storage.external.CollectionResourceClient;
import org.folio.inventory.storage.external.CqlQuery;
import org.folio.inventory.storage.external.MultipleRecordsFetchClient;
import org.folio.inventory.support.http.client.OkapiHttpClient;
import org.folio.inventory.support.http.server.ClientErrorResponse;
import org.folio.inventory.support.http.server.FailureResponseConsumer;
Expand All @@ -24,7 +23,6 @@
import java.net.URL;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

import static java.lang.String.format;
import static org.folio.inventory.support.CqlHelper.buildQueryByIds;
Expand All @@ -36,7 +34,6 @@ public class ItemsByHoldingsRecordId extends Items
private static final String RELATIVE_ITEMS_FOR_HOLDINGS_PATH = "/inventory/items-by-holdings-id";
// Supporting API
private static final String BOUND_WITH_PARTS_STORAGE_PATH = "/inventory-storage/bound-with-parts";
private static final String ITEM_STORAGE_PATH = "/item-storage/items";
private static final String RELATION_PARAM_ONLY_BOUND_WITHS = "onlyBoundWiths";
private static final String RELATION_PARAM_ONLY_BOUND_WITHS_SKIP_DIRECTLY_LINKED_ITEM = "onlyBoundWithsSkipDirectlyLinkedItem";

Expand All @@ -51,17 +48,17 @@ public void register( Router router )
}

/**
* Finds the item IDs of all items under the given holdings record ID;
* then also finds all bound-with parts that includes this holdings record/title;
* then passes the holdings record ID as well as the bound-with item IDs, if any, on to
* {@link #joinAndRespondWithManyItems(RoutingContext,WebContext,List,String,String) joinAndRespondWithManyItems}
* for that method to retrieve all the actual Item objects.
* Finds bound-with parts involving the given holdings record and passes
* the bound-with item IDs on to
* {@link #respondWithRegularItemsAndBoundWithItems(RoutingContext,WebContext,List,String,String,PagingParameters) joinAndRespondWithManyItems}
* for that method to retrieve the actual Item objects together with Items directly attached to the holdings record.
*/
private void getBoundWithItems( RoutingContext routingContext) {
WebContext context = new WebContext(routingContext);

String queryByHoldingsRecordId = context.getStringParameter("query", null);
String relationsParam = context.getStringParameter( "relations", null );
PagingParameters pagingParameters = getPagingParameters(context);

if (queryByHoldingsRecordId == null || !queryByHoldingsRecordId.contains( "holdingsRecordId" )) {
ClientErrorResponse.badRequest(routingContext.response(),
Expand Down Expand Up @@ -89,68 +86,55 @@ private void getBoundWithItems( RoutingContext routingContext) {

String holdingsRecordId = keyVal[1];

CollectionResourceClient itemsClient =
getCollectionResourceRepository( routingContext, context, ITEM_STORAGE_PATH );
itemsClient.getMany("holdingsRecordId=="+holdingsRecordId,
1000000,
0,
response -> {
List<String> holdingsRecordsItemIds = response.getJson()
.getJsonArray( "items" ).stream()
.map(item -> ((JsonObject) item).getString("id"))
.collect( Collectors.toList());

CollectionResourceClient boundWithPartsClient =
getCollectionResourceRepository(
routingContext,
context,
BOUND_WITH_PARTS_STORAGE_PATH);

MultipleRecordsFetchClient itemsFetcher = MultipleRecordsFetchClient.builder()
.withCollectionPropertyName("boundWithParts")
.withExpectedStatus(200)
.withCollectionResourceClient(boundWithPartsClient)
.build();

BoundWithPartsCql boundWithPartsCql = new BoundWithPartsCql(holdingsRecordId);
itemsFetcher.find(holdingsRecordsItemIds, boundWithPartsCql::byHoldingsRecordIdOrListOfItemIds)
.thenAccept(boundWithParts ->
joinAndRespondWithManyItems(routingContext, context, boundWithParts, holdingsRecordId, relationsParam));
});
CollectionResourceClient boundWithPartsClient =
getCollectionResourceRepository(
routingContext,
context,
BOUND_WITH_PARTS_STORAGE_PATH);

if (boundWithPartsClient != null) {
boundWithPartsClient.getMany("holdingsRecordId==" + holdingsRecordId,
100,
0,
response -> {
List<String> boundWithItemIds = response.getJson()
.getJsonArray("boundWithParts").stream()
.map(part -> ((JsonObject) part).getString("itemId"))
.toList();
respondWithRegularItemsAndBoundWithItems(routingContext, context, boundWithItemIds, holdingsRecordId, relationsParam, pagingParameters);
});
}

}

/**
* Retrieves Inventory Items that are directly related to the provided holdings record ID
* or that are bound-with items containing the holdings record ID/title.
* @param boundWithParts list of bound-with entries for bound-withs containing the
* @param boundWithItemIds list of bound-with entries for bound-withs containing the
* given holdings record/title
* @param holdingsRecordId the ID of the given holdings record
* @param relationsParam optional parameter indicating a sub-set of items to retrieve -
* if omitted retrieve all, but otherwise retrieve only bound-with items
* -- that contains the holdings record/title -- with or without the bound-with
* directly linked to the given holdings-records.
*/
private void joinAndRespondWithManyItems(RoutingContext routingContext,
WebContext webContext,
List<JsonObject> boundWithParts,
String holdingsRecordId,
String relationsParam) {
private void respondWithRegularItemsAndBoundWithItems(RoutingContext routingContext,
WebContext webContext,
List<String> boundWithItemIds,
String holdingsRecordId,
String relationsParam,
PagingParameters pagingParameters) {
String itemQuery;
List<String> itemIds;
boolean onlyBoundWiths = relationsParam != null &&
( relationsParam.equals(RELATION_PARAM_ONLY_BOUND_WITHS ) ||
relationsParam.equals(
RELATION_PARAM_ONLY_BOUND_WITHS_SKIP_DIRECTLY_LINKED_ITEM ) );
boolean skipDirectlyLinkedItem = relationsParam != null && relationsParam.equals(
RELATION_PARAM_ONLY_BOUND_WITHS_SKIP_DIRECTLY_LINKED_ITEM );

itemIds = boundWithParts.stream()
.map( part -> part.getString( "itemId" ) )
.collect( Collectors.toList() );

boolean boundWithsFound = itemIds.size()>0;
boolean boundWithsFound = !boundWithItemIds.isEmpty();
if (boundWithsFound) {
itemQuery = buildQueryByIds( itemIds );
itemQuery = buildQueryByIds( boundWithItemIds );
if (skipDirectlyLinkedItem)
{
itemQuery += " and holdingsRecordId <>" + holdingsRecordId;
Expand All @@ -169,7 +153,7 @@ private void joinAndRespondWithManyItems(RoutingContext routingContext,
}
try {
storage.getItemCollection(webContext).findByCql(itemQuery,
new PagingParameters(1000000,0), success ->
pagingParameters, success ->
respondWithManyItems(routingContext, webContext, success.getResult()),
FailureResponseConsumer.serverError(routingContext.response()));
} catch (UnsupportedEncodingException e) {
Expand Down Expand Up @@ -203,19 +187,27 @@ protected OkapiHttpClient createHttpClient(
exception.toString())));
}

static class BoundWithPartsCql {
private final String holdingsId;

public BoundWithPartsCql(String holdingsRecordId) {
this.holdingsId = holdingsRecordId;
/**
* Ensures a limit on the page size of no more than 10,000 items at a time,
* defaults to a limit of 200 items for a page.
* @param context The web context to extract requested paging from
* @return Adapted paging parameters.
*/
private PagingParameters getPagingParameters (WebContext context) {
final String maxPageSize = "10000";
String limit = context.getStringParameter("limit", "200");
String offset = context.getStringParameter("offset", "0");
if (!StringUtils.isNumeric(limit) || StringUtils.isEmpty(limit)) {
limit = "200";
} else if (Integer.parseInt(limit) > Integer.parseInt(maxPageSize)) {
log.error("A paging of {} items was requested but the /items-by-holdings-id API cuts off the page at {} items.", limit, maxPageSize);
limit = maxPageSize;
}

public CqlQuery byHoldingsRecordIdOrListOfItemIds(List<String> itemIds) {
return CqlQuery.exactMatchAny("id", itemIds)
.or(CqlQuery.exactMatch("holdingsRecordId", this.holdingsId));

if (!StringUtils.isNumeric(offset) || StringUtils.isEmpty(offset)) {
offset = "0";
}
PagingParameters enforcedPaging = new PagingParameters(Integer.parseInt(limit), Integer.parseInt(offset));
log.debug("Paging resolved to limit: {}, offset: {}", enforcedPaging.limit, enforcedPaging.offset);
return enforcedPaging;
}


}
14 changes: 11 additions & 3 deletions src/test/java/api/BoundWithTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public void canRetrieveBoundWithItemByHoldingsRecordId() throws InterruptedExcep

Response itemsResponse2 = okapiClient.get(ApiTestSuite.apiRoot()+
"/inventory/items-by-holdings-id?query=holdingsRecordId=="
+holdings1a.getJson().getString( "id" ))
+holdings1a.getJson().getString( "id" )+"&offset=0&limit=20000")
.toCompletableFuture().get(5, SECONDS);

assertThat("One and only one bound-with item is found: ", itemsResponse2.getJson().getInteger( "totalRecords" ), is(1));
Expand All @@ -215,7 +215,7 @@ public void canRetrieveBoundWithItemByHoldingsRecordId() throws InterruptedExcep

Response itemsResponse5 = okapiClient.get(ApiTestSuite.apiRoot()+
"/inventory/items-by-holdings-id?query=holdingsRecordId=="
+holdings3a.getJson().getString( "id" ))
+holdings3a.getJson().getString( "id" )+"&offset=string&limit=string")
.toCompletableFuture().get(5, SECONDS);
assertThat("One item is found for 'holdings3a' (non-bound-with) with relations criterion: ", itemsResponse5.getJson().getInteger( "totalRecords" ), is(1));

Expand All @@ -234,7 +234,7 @@ public void canRetrieveManyItemsThroughItemsByHoldingsId() throws InterruptedExc
}
Response itemsResponse = okapiClient.get(ApiTestSuite.apiRoot()+
"/inventory/items-by-holdings-id?query=holdingsRecordId=="
+holdings1.getJson().getString( "id" ))
+holdings1.getJson().getString( "id" )+"&offset=0&limit=1200")
.toCompletableFuture().get(5, SECONDS);
assertThat("1100 items found for 'holdings1': ", itemsResponse.getJson().getInteger( "totalRecords" ), is(1100));
assertThat("1100 items found for 'holdings1': ", itemsResponse.getJson().getJsonArray("items").size(), is(1100));
Expand Down Expand Up @@ -284,6 +284,14 @@ public void mustQueryBoundWithItemsByHoldingsRecordIdId()

assertThat("Response code 400 (bad request) expected when not querying by holdingsRecordId",
itemsResponse3.getStatusCode(), is(400));

Response itemsResponse4 = okapiClient.get(ApiTestSuite.apiRoot()+
"/inventory/items-by-holdings-id/?query=holdingsRecordId")
.toCompletableFuture().get(5, SECONDS);

assertThat("Response code 400 (bad request) expected when not querying by holdingsRecordId",
itemsResponse4.getStatusCode(), is(400));

}

@Test
Expand Down

0 comments on commit 022755f

Please sign in to comment.