From 94c373a901bc12405ed9ba72042c81674eaf0508 Mon Sep 17 00:00:00 2001 From: Dmytro Krutii Date: Tue, 14 May 2024 16:46:45 +0300 Subject: [PATCH 1/4] MODINV-943 Add logs --- .../org/folio/inventory/resources/Items.java | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/folio/inventory/resources/Items.java b/src/main/java/org/folio/inventory/resources/Items.java index 977dcec8b..8137184a8 100644 --- a/src/main/java/org/folio/inventory/resources/Items.java +++ b/src/main/java/org/folio/inventory/resources/Items.java @@ -312,6 +312,7 @@ protected void respondWithManyItems( CollectionResourceClient locationsClient; try { + log.info("respondWithManyItems:: creating http clients"); OkapiHttpClient okapiClient = createHttpClient(routingContext, context); holdingsClient = createHoldingsClient(okapiClient, context); instancesClient = createInstancesClient(okapiClient, context); @@ -321,7 +322,7 @@ protected void respondWithManyItems( } catch (MalformedURLException e) { invalidOkapiUrlResponse(routingContext, context); - + log.info("respondWithManyItems:: error while creating http clients", e); return; } @@ -355,6 +356,8 @@ protected void respondWithManyItems( final List holdings = JsonArrayHelper.toList( holdingsResponse.getJson().getJsonArray("holdingsRecords")); + log.info("respondWithManyItems:: collecting instanceIds"); + List instanceIds = holdings.stream() .map(holding -> holding.getString(INSTANCE_ID_PROPERTY)) .filter(Objects::nonNull) @@ -377,15 +380,21 @@ protected void respondWithManyItems( instancesResponse.getBody())); } + log.info("respondWithManyItems:: instanceIds size: {}", instanceIds.size()); + final List instances = JsonArrayHelper.toList( instancesResponse.getJson().getJsonArray("instances")); + log.info("respondWithManyItems:: collecting materialTypeIds"); + List materialTypeIds = wrappedItems.records.stream() .map(Item::getMaterialTypeId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); + log.info("respondWithManyItems:: materialTypeIds size: {}", materialTypeIds.size()); + materialTypeIds.forEach(id -> { CompletableFuture newFuture = new CompletableFuture<>(); @@ -395,12 +404,17 @@ protected void respondWithManyItems( materialTypesClient.get(id, newFuture::complete); }); + log.info("respondWithManyItems:: collecting permanentLoanTypeIds"); + List permanentLoanTypeIds = wrappedItems.records.stream() .map(Item::getPermanentLoanTypeId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); + log.info("respondWithManyItems:: permanentLoanTypeIds size: {}", permanentLoanTypeIds.size()); + + log.info("respondWithManyItems:: collecting temporaryLoanTypeIds"); List temporaryLoanTypeIds = wrappedItems.records.stream() .map(Item::getTemporaryLoanTypeId) .filter(Objects::nonNull) @@ -419,18 +433,28 @@ protected void respondWithManyItems( loanTypesClient.get(id, newFuture::complete); }); + log.info("respondWithManyItems:: collecting effectiveLocationIds"); + List effectiveLocationIds = wrappedItems.records.stream() .map(Item::getEffectiveLocationId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); + log.info("respondWithManyItems:: effectiveLocationIds size: {}", effectiveLocationIds.size()); + + log.info("respondWithManyItems:: collecting permanentLocationIds"); + List permanentLocationIds = wrappedItems.records.stream() .map(Item::getPermanentLocationId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); + log.info("respondWithManyItems:: permanentLocationIds size: {}", permanentLocationIds.size()); + + log.info("respondWithManyItems: collecting temporaryLocationIds"); + List temporaryLocationIds = wrappedItems.records.stream() .map(Item::getTemporaryLocationId) .filter(Objects::nonNull) @@ -449,6 +473,8 @@ protected void respondWithManyItems( locationsClient.get(id, newFuture::complete); }); + log.info("respondWithManyItems:: temporaryLocationIds size: {}", temporaryLocationIds.size()); + CompletableFuture boundWithPartsFuture = getBoundWithPartsForMultipleItemsFuture(wrappedItems, routingContext); @@ -461,27 +487,45 @@ protected void respondWithManyItems( try { + log.info("respondWithManyItems:: trying to fetch materialTypes"); + Map foundMaterialTypes = allMaterialTypeFutures.stream() .map(CompletableFuture::join) + .peek(r -> log.info("Fetch material type -> response code '{}'", r.getStatusCode())) .filter(response -> response.getStatusCode() == 200) .map(Response::getJson) + .peek(r -> log.info("Successfully fetch material type with id '{}'", r.getString("id"))) .collect(Collectors.toMap(r -> r.getString("id"), r -> r)); + log.info("respondWithManyItems:: foundMaterialTypes size: {}", foundMaterialTypes.size()); + + log.info("respondWithManyItems:: trying to fetch loanTypes"); + Map foundLoanTypes = allLoanTypeFutures.stream() .map(CompletableFuture::join) + .peek(r -> log.info("Fetch loan type -> response code '{}'", r.getStatusCode())) .filter(response -> response.getStatusCode() == 200) .map(Response::getJson) + .peek(r -> log.info("Successfully fetch loan type with id '{}'", r.getString("id"))) .collect(Collectors.toMap(r -> r.getString("id"), r -> r)); + log.info("respondWithManyItems:: foundLoanTypes size: {}", foundLoanTypes.size()); + + log.info("respondWithManyItems:: trying to fetch locations"); + Map foundLocations = allLocationsFutures.stream() .map(CompletableFuture::join) + .peek(r -> log.info("Fetch location -> response code '{}'", r.getStatusCode())) .filter(response -> response.getStatusCode() == 200) .map(Response::getJson) + .peek(r -> log.info("Successfully fetch locations with id '{}'", r.getString("id"))) .collect(Collectors.toMap(r -> r.getString("id"), r -> r)); + log.info("respondWithManyItems:: foundLocations size: {}", foundLocations.size()); + setBoundWithFlagsOnItems(wrappedItems, boundWithPartsFuture); JsonResponse.success(routingContext.response(), @@ -1060,7 +1104,7 @@ private CompletableFuture getBoundWithPartsForItemFuture( private void setBoundWithFlagsOnItems(MultipleRecords wrappedItems, CompletableFuture boundWithPartsFuture) { - + log.info("setBoundWithFlagsOnItems:: trying to set withIsBoundWith"); Response response = boundWithPartsFuture.join(); if (response != null && response.hasBody() && response.getStatusCode()==200) { JsonArray boundWithParts = response.getJson().getJsonArray(BOUND_WITH_PARTS_COLLECTION); From a2b9e2c5d92a532d9474dff6bda3ac1e78990631 Mon Sep 17 00:00:00 2001 From: Dmytro Krutii Date: Wed, 29 May 2024 17:40:11 +0300 Subject: [PATCH 2/4] MODINV-943 Use POST instead GET for fetching Holdings and Instances --- NEWS.md | 1 + .../org/folio/inventory/resources/Items.java | 4 ++-- .../external/CollectionResourceClient.java | 19 +++++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 012eb24c4..7c3736d4e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,7 @@ * Remove null values from electronicAccess object before returning item and instance [MODINV-1006](https://folio-org.atlassian.net/browse/MODINV-1006) * 422 Unprocessable Content Error while updating Instances and Items with electronic access without URI field populated. [MODINV-1024](https://folio-org.atlassian.net/browse/MODINV-1024) * Error appears when edit via quickMARC MARC Instance shared from Member tenant [MODDATAIMP-1052](https://folio-org.atlassian.net/browse/MODDATAIMP-1052) +* Replace GET with POST request for fetching instances and holdings on /items endpoint to omit 414 error [MODINV-943](https://folio-org.atlassian.net/browse/MODINV-943) ## 20.2.0 2023-03-20 * Inventory cannot process Holdings with virtual fields ([MODINV-941](https://issues.folio.org/browse/MODINV-941)) diff --git a/src/main/java/org/folio/inventory/resources/Items.java b/src/main/java/org/folio/inventory/resources/Items.java index 8137184a8..bd4d92f0c 100644 --- a/src/main/java/org/folio/inventory/resources/Items.java +++ b/src/main/java/org/folio/inventory/resources/Items.java @@ -342,7 +342,7 @@ protected void respondWithManyItems( String holdingsQuery = multipleRecordsCqlQuery(holdingsIds); - holdingsClient.getMany(holdingsQuery, holdingsIds.size(), 0, + holdingsClient.retrieveMany(holdingsQuery, holdingsIds.size(), 0, holdingsFetched::complete); holdingsFetched.thenAccept(holdingsResponse -> { @@ -369,7 +369,7 @@ protected void respondWithManyItems( String instancesQuery = multipleRecordsCqlQuery(instanceIds); - instancesClient.getMany(instancesQuery, instanceIds.size(), 0, + instancesClient.retrieveMany(instancesQuery, instanceIds.size(), 0, instancesFetched::complete); instancesFetched.thenAccept(instancesResponse -> { diff --git a/src/main/java/org/folio/inventory/storage/external/CollectionResourceClient.java b/src/main/java/org/folio/inventory/storage/external/CollectionResourceClient.java index 39d748917..d57ad77ab 100644 --- a/src/main/java/org/folio/inventory/storage/external/CollectionResourceClient.java +++ b/src/main/java/org/folio/inventory/storage/external/CollectionResourceClient.java @@ -68,6 +68,25 @@ public void getMany( .thenAccept(responseHandler); } + /** + * Run the query using some limit and offset. + * + * @param cqlQuery the query without percent (url) encoding + */ + public void retrieveMany( + String cqlQuery, + Integer pageLimit, + Integer pageOffset, + Consumer responseHandler) { + var body = new JsonObject() + .put("query", cqlQuery) + .put("limit", pageLimit) + .put("offset", pageOffset); + var url = collectionRoot + "/retrieve"; + client.post(url, body) + .thenAccept(responseHandler); + } + /** * Runs the query while setting limit to maximum and offset to zero to get all records. * From 3a312a9a39ad47320b6f17542462566a039a4e01 Mon Sep 17 00:00:00 2001 From: Dmytro Krutii Date: Wed, 29 May 2024 17:42:23 +0300 Subject: [PATCH 3/4] Revert "MODINV-943 Add logs" This reverts commit 94c373a901bc12405ed9ba72042c81674eaf0508. --- .../org/folio/inventory/resources/Items.java | 48 +------------------ 1 file changed, 2 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/folio/inventory/resources/Items.java b/src/main/java/org/folio/inventory/resources/Items.java index bd4d92f0c..17c1ac4a4 100644 --- a/src/main/java/org/folio/inventory/resources/Items.java +++ b/src/main/java/org/folio/inventory/resources/Items.java @@ -312,7 +312,6 @@ protected void respondWithManyItems( CollectionResourceClient locationsClient; try { - log.info("respondWithManyItems:: creating http clients"); OkapiHttpClient okapiClient = createHttpClient(routingContext, context); holdingsClient = createHoldingsClient(okapiClient, context); instancesClient = createInstancesClient(okapiClient, context); @@ -322,7 +321,7 @@ protected void respondWithManyItems( } catch (MalformedURLException e) { invalidOkapiUrlResponse(routingContext, context); - log.info("respondWithManyItems:: error while creating http clients", e); + return; } @@ -356,8 +355,6 @@ protected void respondWithManyItems( final List holdings = JsonArrayHelper.toList( holdingsResponse.getJson().getJsonArray("holdingsRecords")); - log.info("respondWithManyItems:: collecting instanceIds"); - List instanceIds = holdings.stream() .map(holding -> holding.getString(INSTANCE_ID_PROPERTY)) .filter(Objects::nonNull) @@ -380,21 +377,15 @@ protected void respondWithManyItems( instancesResponse.getBody())); } - log.info("respondWithManyItems:: instanceIds size: {}", instanceIds.size()); - final List instances = JsonArrayHelper.toList( instancesResponse.getJson().getJsonArray("instances")); - log.info("respondWithManyItems:: collecting materialTypeIds"); - List materialTypeIds = wrappedItems.records.stream() .map(Item::getMaterialTypeId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); - log.info("respondWithManyItems:: materialTypeIds size: {}", materialTypeIds.size()); - materialTypeIds.forEach(id -> { CompletableFuture newFuture = new CompletableFuture<>(); @@ -404,17 +395,12 @@ protected void respondWithManyItems( materialTypesClient.get(id, newFuture::complete); }); - log.info("respondWithManyItems:: collecting permanentLoanTypeIds"); - List permanentLoanTypeIds = wrappedItems.records.stream() .map(Item::getPermanentLoanTypeId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); - log.info("respondWithManyItems:: permanentLoanTypeIds size: {}", permanentLoanTypeIds.size()); - - log.info("respondWithManyItems:: collecting temporaryLoanTypeIds"); List temporaryLoanTypeIds = wrappedItems.records.stream() .map(Item::getTemporaryLoanTypeId) .filter(Objects::nonNull) @@ -433,28 +419,18 @@ protected void respondWithManyItems( loanTypesClient.get(id, newFuture::complete); }); - log.info("respondWithManyItems:: collecting effectiveLocationIds"); - List effectiveLocationIds = wrappedItems.records.stream() .map(Item::getEffectiveLocationId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); - log.info("respondWithManyItems:: effectiveLocationIds size: {}", effectiveLocationIds.size()); - - log.info("respondWithManyItems:: collecting permanentLocationIds"); - List permanentLocationIds = wrappedItems.records.stream() .map(Item::getPermanentLocationId) .filter(Objects::nonNull) .distinct() .collect(Collectors.toList()); - log.info("respondWithManyItems:: permanentLocationIds size: {}", permanentLocationIds.size()); - - log.info("respondWithManyItems: collecting temporaryLocationIds"); - List temporaryLocationIds = wrappedItems.records.stream() .map(Item::getTemporaryLocationId) .filter(Objects::nonNull) @@ -473,8 +449,6 @@ protected void respondWithManyItems( locationsClient.get(id, newFuture::complete); }); - log.info("respondWithManyItems:: temporaryLocationIds size: {}", temporaryLocationIds.size()); - CompletableFuture boundWithPartsFuture = getBoundWithPartsForMultipleItemsFuture(wrappedItems, routingContext); @@ -487,45 +461,27 @@ protected void respondWithManyItems( try { - log.info("respondWithManyItems:: trying to fetch materialTypes"); - Map foundMaterialTypes = allMaterialTypeFutures.stream() .map(CompletableFuture::join) - .peek(r -> log.info("Fetch material type -> response code '{}'", r.getStatusCode())) .filter(response -> response.getStatusCode() == 200) .map(Response::getJson) - .peek(r -> log.info("Successfully fetch material type with id '{}'", r.getString("id"))) .collect(Collectors.toMap(r -> r.getString("id"), r -> r)); - log.info("respondWithManyItems:: foundMaterialTypes size: {}", foundMaterialTypes.size()); - - log.info("respondWithManyItems:: trying to fetch loanTypes"); - Map foundLoanTypes = allLoanTypeFutures.stream() .map(CompletableFuture::join) - .peek(r -> log.info("Fetch loan type -> response code '{}'", r.getStatusCode())) .filter(response -> response.getStatusCode() == 200) .map(Response::getJson) - .peek(r -> log.info("Successfully fetch loan type with id '{}'", r.getString("id"))) .collect(Collectors.toMap(r -> r.getString("id"), r -> r)); - log.info("respondWithManyItems:: foundLoanTypes size: {}", foundLoanTypes.size()); - - log.info("respondWithManyItems:: trying to fetch locations"); - Map foundLocations = allLocationsFutures.stream() .map(CompletableFuture::join) - .peek(r -> log.info("Fetch location -> response code '{}'", r.getStatusCode())) .filter(response -> response.getStatusCode() == 200) .map(Response::getJson) - .peek(r -> log.info("Successfully fetch locations with id '{}'", r.getString("id"))) .collect(Collectors.toMap(r -> r.getString("id"), r -> r)); - log.info("respondWithManyItems:: foundLocations size: {}", foundLocations.size()); - setBoundWithFlagsOnItems(wrappedItems, boundWithPartsFuture); JsonResponse.success(routingContext.response(), @@ -1104,7 +1060,7 @@ private CompletableFuture getBoundWithPartsForItemFuture( private void setBoundWithFlagsOnItems(MultipleRecords wrappedItems, CompletableFuture boundWithPartsFuture) { - log.info("setBoundWithFlagsOnItems:: trying to set withIsBoundWith"); + Response response = boundWithPartsFuture.join(); if (response != null && response.hasBody() && response.getStatusCode()==200) { JsonArray boundWithParts = response.getJson().getJsonArray(BOUND_WITH_PARTS_COLLECTION); From 74f7183165569e0221134a031a4ee01d2359a5ca Mon Sep 17 00:00:00 2001 From: Dmytro Krutii Date: Thu, 30 May 2024 15:04:03 +0300 Subject: [PATCH 4/4] MODINV-943 Fix Unit tests --- .../java/support/fakes/FakeStorageModule.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/test/java/support/fakes/FakeStorageModule.java b/src/test/java/support/fakes/FakeStorageModule.java index 33d543037..802c7ffcd 100644 --- a/src/test/java/support/fakes/FakeStorageModule.java +++ b/src/test/java/support/fakes/FakeStorageModule.java @@ -82,6 +82,7 @@ void register(Router router) { router.route(pathTree).handler(this::emulateFailureIfNeeded); router.route(pathTree).handler(this::checkTokenHeader); + router.post(rootPath + "/retrieve").handler(this::retrieveMany); router.post(rootPath).handler(this::checkRequiredProperties); router.post(rootPath).handler(this::checkUniqueProperties); router.post(rootPath + "/emulate-failure").handler(this::emulateFailure); @@ -266,6 +267,37 @@ private void getMany(RoutingContext routingContext) { JsonResponse.success(routingContext.response(), result); } + private void retrieveMany(RoutingContext routingContext) { + WebContext context = new WebContext(routingContext); + var requestBody = routingContext.getBodyAsJson(); + + var limit = requestBody.getInteger("limit"); + var offset = requestBody.getInteger("offset"); + var query = requestBody.getString("query");; + + System.out.printf("Handling %s%n", routingContext.request().uri()); + + Map resourcesForTenant = getResourcesForTenant(context); + + List filteredItems = new FakeCQLToJSONInterpreter(false) + .execute(resourcesForTenant.values(), query); + + List pagedItems = filteredItems.stream() + .skip(offset) + .limit(limit) + .collect(Collectors.toList()); + + JsonObject result = new JsonObject(); + + result.put(collectionPropertyName, new JsonArray(pagedItems)); + result.put("totalRecords", filteredItems.size()); + + System.out.printf("Found %s resources: %s%n", recordTypeName, + result.encodePrettily()); + + JsonResponse.success(routingContext.response(), result); + } + private void empty(RoutingContext routingContext) { WebContext context = new WebContext(routingContext);