MODFEE-264: Batching for Financial Transactions Detail Report (#244)

* MODFEE-264 Batching for Financial Transactions Detail Report

* MODFEE-264 Fix existing tests

* MODFEE-264 Minor refactoring

* MODFEE-264 Minor refactoring

* MODFEE-264 Remove validation of UUIDs

* MODFEE-264 Improve long query performance, more logs

* MODFEE-264 Fetch actions using index

* MODFEE-264 Fetch policies by distinct IDs

* MODFEE-264 Small performance optimization

* MODFEE-264 Fix code smell

(cherry picked from commit 3ad52c5)

descriptors/ModuleDescriptor-template.json

@@ -981,14 +981,23 @@
           "modulePermissions": [
             "configuration.entries.collection.get",
             "inventory-storage.service-points.item.get",
+            "inventory-storage.service-points.collection.get",
             "users.item.get",
+            "users.collection.get",
             "usergroups.item.get",
+            "usergroups.collection.get",
             "inventory-storage.items.item.get",
+            "inventory-storage.items.collection.get",
             "inventory-storage.holdings.item.get",
+            "inventory-storage.holdings.collection.get",
             "inventory-storage.instances.item.get",
+            "inventory-storage.instances.collection.get",
             "inventory-storage.locations.item.get",
+            "inventory-storage.locations.collection.get",
             "circulation-storage.loans.item.get",
-            "circulation-storage.loan-policies.item.get"
+            "circulation-storage.loans.collection.get",
+            "circulation-storage.loan-policies.item.get",
+            "circulation-storage.loan-policies.collection.get"
           ]
         }
       ]

src/main/java/org/folio/rest/client/CirculationStorageClient.java

@@ -1,5 +1,6 @@
 package org.folio.rest.client;
 
+import java.util.Collection;
 import java.util.Map;
 
 import org.folio.rest.jaxrs.model.Loan;
@@ -17,7 +18,15 @@ public Future<Loan> getLoanById(String id) {
     return getById("/loan-storage/loans", id, Loan.class);
   }
 
+  public Future<Collection<Loan>> getLoansByIds(Collection<String> ids) {
+    return getByIds("/loan-storage/loans", ids, Loan.class, "loans");
+  }
+
   public Future<LoanPolicy> getLoanPolicyById(String id) {
     return getById("/loan-policy-storage/loan-policies", id, LoanPolicy.class);
   }
+
+  public Future<Collection<LoanPolicy>> getLoanPoliciesByIds(Collection<String> ids) {
+    return getByIds("/loan-policy-storage/loan-policies", ids, LoanPolicy.class, "loanPolicies");
+  }
 }

src/main/java/org/folio/rest/client/InventoryClient.java

@@ -14,6 +14,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
@@ -114,18 +115,34 @@ public Future<Item> getItemById(String id) {
     return getById("/item-storage/items", id, Item.class);
   }
 
+  public Future<Collection<Item>> getItemsByIds(Collection<String> ids) {
+    return getByIds("/item-storage/items", ids, Item.class, "items");
+  }
+
   public Future<HoldingsRecord> getHoldingById(String id) {
     return getById("/holdings-storage/holdings", id, HoldingsRecord.class);
   }
 
+  public Future<Collection<HoldingsRecord>> getHoldingsByIds(Collection<String> ids) {
+    return getByIds("/holdings-storage/holdings", ids, HoldingsRecord.class, "holdingsRecords");
+  }
+
   public Future<Instance> getInstanceById(String id) {
     return getById("/instance-storage/instances", id, Instance.class);
   }
 
+  public Future<Collection<Instance>> getInstancesByIds(Collection<String> ids) {
+    return getByIds("/instance-storage/instances", ids, Instance.class, "instances");
+  }
+
   public Future<Location> getLocationById(String id) {
     return getById("/locations", id, Location.class);
   }
 
+  public Future<Collection<Location>> getLocationsByIds(Collection<String> ids) {
+    return getByIds("/locations", ids, Location.class, "locations");
+  }
+
   public Future<Institution> getInstitutionById(String id) {
     return getById("/location-units/institutions", id, Institution.class);
   }
@@ -141,4 +158,8 @@ public Future<Library> getLibraryById(String id) {
   public Future<ServicePoint> getServicePointById(String id) {
     return getById("/service-points", id, ServicePoint.class);
   }
+
+  public Future<Collection<ServicePoint>> getServicePointsByIds(Collection<String> ids) {
+    return getByIds("/service-points", ids, ServicePoint.class, "servicepoints");
+  }
 }

src/main/java/org/folio/rest/client/OkapiClient.java

@@ -3,33 +3,46 @@
 import static io.vertx.core.Future.failedFuture;
 import static io.vertx.core.Future.succeededFuture;
 import static java.lang.String.format;
+import static java.lang.System.currentTimeMillis;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toSet;
 import static javax.ws.rs.core.HttpHeaders.ACCEPT;
 import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.folio.rest.RestVerticle.OKAPI_HEADER_TENANT;
 import static org.folio.rest.RestVerticle.OKAPI_HEADER_TOKEN;
+import static org.folio.util.StringUtil.urlEncode;
 import static org.folio.util.UuidUtil.isUuid;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import org.apache.commons.collections4.ListUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.folio.rest.exception.http.HttpGetException;
+
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 import io.vertx.core.Future;
 import io.vertx.core.Promise;
 import io.vertx.core.Vertx;
 import io.vertx.core.buffer.Buffer;
+import io.vertx.core.json.JsonObject;
 import io.vertx.ext.web.client.HttpRequest;
 import io.vertx.ext.web.client.HttpResponse;
 import io.vertx.ext.web.client.WebClient;
 
-import java.io.IOException;
-import java.util.Map;
-import java.util.Optional;
-
-import org.folio.rest.exception.http.HttpGetByIdException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 public class OkapiClient {
-  protected static final Logger log = LoggerFactory.getLogger(OkapiClient.class);
+  protected static final Logger log = LogManager.getLogger(OkapiClient.class);
+  private static final int ID_BATCH_SIZE = 88;
+
   private static final String OKAPI_URL_HEADER = "x-okapi-url";
   protected static final ObjectMapper objectMapper = new ObjectMapper();
 
@@ -38,7 +51,7 @@ public class OkapiClient {
   private final String tenant;
   private final String token;
 
-  OkapiClient(Vertx vertx, Map<String, String> okapiHeaders) {
+  public OkapiClient(Vertx vertx, Map<String, String> okapiHeaders) {
     this.webClient = WebClientProvider.getWebClient(vertx);
     okapiUrl = okapiHeaders.get(OKAPI_URL_HEADER);
     tenant = okapiHeaders.get(OKAPI_HEADER_TENANT);
@@ -71,15 +84,19 @@ public <T> Future<T> getById(String resourcePath, String id, Class<T> objectType
 
     final String url = resourcePath + "/" + id;
     Promise<HttpResponse<Buffer>> promise = Promise.promise();
+
+    long start = currentTimeMillis();
+
     okapiGetAbs(url).send(promise);
 
     return promise.future().compose(response -> {
       int responseStatus = response.statusCode();
+      log.debug("[{} {}ms] GET {}", responseStatus, currentTimeMillis() - start, resourcePath);
       if (responseStatus != 200) {
         final String errorMessage = format("Failed to get %s by ID %s. Response status code: %s",
           objectType.getSimpleName(), id, responseStatus);
         log.error(errorMessage);
-        return failedFuture(new HttpGetByIdException(url, response, objectType, id));
+        return failedFuture(new HttpGetException(url, response));
       }
       try {
         T object = objectMapper.readValue(response.bodyAsString(), objectType);
@@ -108,4 +125,66 @@ private static <T> Optional<String> validateGetByIdArguments(String path, String
 
     return Optional.ofNullable(errorMessage);
   }
+
+  public <T> Future<Collection<T>> getByQuery(String resourcePath, String query,
+    Class<T> objectType, String collectionName, int limit) {
+
+    long startTimeMillis = currentTimeMillis();
+    String path = String.format("%s?query=%s&limit=%d", resourcePath, urlEncode(query), limit);
+
+    return okapiGetAbs(path)
+      .send()
+      .compose(response -> {
+        int responseStatus = response.statusCode();
+        log.debug("[{}] [{}ms] GET {}", responseStatus, currentTimeMillis() - startTimeMillis,
+          resourcePath);
+        if (responseStatus != 200) {
+          HttpGetException exception = new HttpGetException(path, response);
+          log.error("GET by query failed", exception);
+          return failedFuture(exception);
+        }
+        return succeededFuture(
+          response.bodyAsJsonObject()
+            .getJsonArray(collectionName)
+            .stream()
+            .map(JsonObject::mapFrom)
+            .map(json -> json.mapTo(objectType))
+            .collect(toList())
+        );
+      });
+  }
+
+  public <T> Future<Collection<T>> getByIds(String path, Collection<String> ids, Class<T> objectType,
+    String collectionName) {
+
+    Collection<T> results = new ArrayList<>();
+
+    Set<String> filteredIds = ids.stream()
+      .filter(StringUtils::isNotBlank)
+      .collect(toSet());
+
+    if (ids.isEmpty()) {
+      return succeededFuture(results);
+    }
+
+    log.info("Fetching {} {} by ID", ids.size(), objectType.getSimpleName());
+    long startTime = currentTimeMillis();
+
+    return ListUtils.partition(new ArrayList<>(filteredIds), ID_BATCH_SIZE)
+      .stream()
+      .map(batch -> fetchBatch(path, batch, objectType, collectionName).onSuccess(results::addAll))
+      .reduce(succeededFuture(), (f1, f2) -> f1.compose(r -> f2))
+      .map(results)
+      .onSuccess(r -> log.debug("Fetched {} {} in {} ms", results.size(), objectType.getSimpleName(),
+        currentTimeMillis() - startTime));
+  }
+
+  private <T> Future<Collection<T>> fetchBatch(String resourcePath, List<String> batch,
+    Class<T> objectType, String collectionName) {
+
+    log.debug("Fetching batch of {} {}", batch.size(), objectType.getSimpleName());
+    String query = String.format("id==(%s)", String.join(" or ", batch));
+
+    return getByQuery(resourcePath, query, objectType, collectionName, batch.size());
+  }
 }

src/main/java/org/folio/rest/client/UsersClient.java

@@ -1,9 +1,13 @@
 package org.folio.rest.client;
 
-import io.vertx.core.Future;
-import io.vertx.core.Vertx;
+import java.util.Collection;
 import java.util.Map;
+
 import org.folio.rest.jaxrs.model.User;
+import org.folio.rest.jaxrs.model.UserGroup;
+
+import io.vertx.core.Future;
+import io.vertx.core.Vertx;
 
 public class UsersClient extends OkapiClient {
   public UsersClient(Vertx vertx, Map<String, String> okapiHeaders) {
@@ -13,4 +17,16 @@ public UsersClient(Vertx vertx, Map<String, String> okapiHeaders) {
   public Future<User> fetchUserById(String userId) {
     return getById("/users", userId, User.class);
   }
+
+  public Future<Collection<User>> fetchUsers(Collection<String> ids) {
+    return getByIds("/users", ids, User.class, "users");
+  }
+
+  public Future<UserGroup> fetchUserGroupById(String userGroupId) {
+    return getById("/groups", userGroupId, UserGroup.class);
+  }
+
+  public Future<Collection<UserGroup>> fetchUserGroupsByIds(Collection<String> userGroupIds) {
+    return getByIds("/groups", userGroupIds, UserGroup.class, "usergroups");
+  }
 }

src/main/java/org/folio/rest/exception/http/HttpException.java

@@ -23,6 +23,6 @@ public HttpException(HttpMethod httpMethod, String url, HttpResponse<Buffer> res
 
   @Override
   public String getMessage() {
-    return String.format("%s %s failed: [%d] %s", httpMethod, url, responseStatus, responseBody);
+    return String.format("%s %s: [%d] %s", httpMethod, url, responseStatus, responseBody);
   }
 }

src/main/java/org/folio/rest/exception/http/HttpGetException.java

@@ -0,0 +1,14 @@
+package org.folio.rest.exception.http;
+
+import io.vertx.core.buffer.Buffer;
+import io.vertx.core.http.HttpMethod;
+import io.vertx.ext.web.client.HttpResponse;
+import lombok.Getter;
+
+@Getter
+public class HttpGetException extends HttpException {
+
+  public HttpGetException(String url, HttpResponse<Buffer> response) {
+    super(HttpMethod.GET, url, response);
+  }
+}