From acbeaf6ac6f5a5256145b20518e1737be2c85a8e Mon Sep 17 00:00:00 2001 From: OleksandrVidinieiev <56632770+OleksandrVidinieiev@users.noreply.github.com> Date: Fri, 9 Sep 2022 16:46:02 +0300 Subject: [PATCH] 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 3ad52c59d7dba283875dfad1205b2a7459d011b6) --- descriptors/ModuleDescriptor-template.json | 11 +- .../rest/client/CirculationStorageClient.java | 9 + .../folio/rest/client/InventoryClient.java | 21 + .../org/folio/rest/client/OkapiClient.java | 101 +++- .../folio/rest/client/UserGroupsClient.java | 18 - .../org/folio/rest/client/UsersClient.java | 20 +- .../rest/exception/http/HttpException.java | 2 +- .../exception/http/HttpGetByIdException.java | 24 - .../rest/exception/http/HttpGetException.java | 14 + .../rest/repository/AbstractRepository.java | 88 ++++ .../repository/FeeFineActionRepository.java | 25 +- .../LostItemFeePolicyRepository.java | 20 +- .../OverdueFinePolicyRepository.java | 22 +- .../service/report/AccountContextData.java | 44 ++ ...ancialTransactionsDetailReportContext.java | 224 +++++++++ ...ancialTransactionsDetailReportService.java | 302 +----------- .../service/report/utils/LookupHelper.java | 442 +++++++++++------- .../templates/db_scripts/schema.json | 4 + .../folio/rest/client/OkapiClientTest.java | 2 +- .../folio/rest/client/UsersClientTest.java | 4 +- .../rest/impl/FeeFineActionsAPITest.java | 8 +- .../FinancialTransactionDetailReportTest.java | 64 +-- .../java/org/folio/test/support/ApiTests.java | 19 + 23 files changed, 916 insertions(+), 572 deletions(-) delete mode 100644 src/main/java/org/folio/rest/client/UserGroupsClient.java delete mode 100644 src/main/java/org/folio/rest/exception/http/HttpGetByIdException.java create mode 100644 src/main/java/org/folio/rest/exception/http/HttpGetException.java create mode 100644 src/main/java/org/folio/rest/repository/AbstractRepository.java create mode 100644 src/main/java/org/folio/rest/service/report/AccountContextData.java create mode 100644 src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportContext.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index bbf94ed3..4fad50a0 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/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" ] } ] diff --git a/src/main/java/org/folio/rest/client/CirculationStorageClient.java b/src/main/java/org/folio/rest/client/CirculationStorageClient.java index 68b9812d..2d6dcc03 100644 --- a/src/main/java/org/folio/rest/client/CirculationStorageClient.java +++ b/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 getLoanById(String id) { return getById("/loan-storage/loans", id, Loan.class); } + public Future> getLoansByIds(Collection ids) { + return getByIds("/loan-storage/loans", ids, Loan.class, "loans"); + } + public Future getLoanPolicyById(String id) { return getById("/loan-policy-storage/loan-policies", id, LoanPolicy.class); } + + public Future> getLoanPoliciesByIds(Collection ids) { + return getByIds("/loan-policy-storage/loan-policies", ids, LoanPolicy.class, "loanPolicies"); + } } diff --git a/src/main/java/org/folio/rest/client/InventoryClient.java b/src/main/java/org/folio/rest/client/InventoryClient.java index 407439d7..25a7d7d5 100644 --- a/src/main/java/org/folio/rest/client/InventoryClient.java +++ b/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 getItemById(String id) { return getById("/item-storage/items", id, Item.class); } + public Future> getItemsByIds(Collection ids) { + return getByIds("/item-storage/items", ids, Item.class, "items"); + } + public Future getHoldingById(String id) { return getById("/holdings-storage/holdings", id, HoldingsRecord.class); } + public Future> getHoldingsByIds(Collection ids) { + return getByIds("/holdings-storage/holdings", ids, HoldingsRecord.class, "holdingsRecords"); + } + public Future getInstanceById(String id) { return getById("/instance-storage/instances", id, Instance.class); } + public Future> getInstancesByIds(Collection ids) { + return getByIds("/instance-storage/instances", ids, Instance.class, "instances"); + } + public Future getLocationById(String id) { return getById("/locations", id, Location.class); } + public Future> getLocationsByIds(Collection ids) { + return getByIds("/locations", ids, Location.class, "locations"); + } + public Future getInstitutionById(String id) { return getById("/location-units/institutions", id, Institution.class); } @@ -141,4 +158,8 @@ public Future getLibraryById(String id) { public Future getServicePointById(String id) { return getById("/service-points", id, ServicePoint.class); } + + public Future> getServicePointsByIds(Collection ids) { + return getByIds("/service-points", ids, ServicePoint.class, "servicepoints"); + } } diff --git a/src/main/java/org/folio/rest/client/OkapiClient.java b/src/main/java/org/folio/rest/client/OkapiClient.java index cfbc404b..1cbbe5d2 100644 --- a/src/main/java/org/folio/rest/client/OkapiClient.java +++ b/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 okapiHeaders) { + public OkapiClient(Vertx vertx, Map okapiHeaders) { this.webClient = WebClientProvider.getWebClient(vertx); okapiUrl = okapiHeaders.get(OKAPI_URL_HEADER); tenant = okapiHeaders.get(OKAPI_HEADER_TENANT); @@ -71,15 +84,19 @@ public Future getById(String resourcePath, String id, Class objectType final String url = resourcePath + "/" + id; Promise> 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 Optional validateGetByIdArguments(String path, String return Optional.ofNullable(errorMessage); } + + public Future> getByQuery(String resourcePath, String query, + Class 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 Future> getByIds(String path, Collection ids, Class objectType, + String collectionName) { + + Collection results = new ArrayList<>(); + + Set 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 Future> fetchBatch(String resourcePath, List batch, + Class 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()); + } } diff --git a/src/main/java/org/folio/rest/client/UserGroupsClient.java b/src/main/java/org/folio/rest/client/UserGroupsClient.java deleted file mode 100644 index bb031e18..00000000 --- a/src/main/java/org/folio/rest/client/UserGroupsClient.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.folio.rest.client; - -import java.util.Map; - -import org.folio.rest.jaxrs.model.UserGroup; - -import io.vertx.core.Future; -import io.vertx.core.Vertx; - -public class UserGroupsClient extends OkapiClient { - public UserGroupsClient(Vertx vertx, Map okapiHeaders) { - super(vertx, okapiHeaders); - } - - public Future fetchUserGroupById(String userGroupId) { - return getById("/groups", userGroupId, UserGroup.class); - } -} diff --git a/src/main/java/org/folio/rest/client/UsersClient.java b/src/main/java/org/folio/rest/client/UsersClient.java index 66d7bf84..9cd12997 100644 --- a/src/main/java/org/folio/rest/client/UsersClient.java +++ b/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 okapiHeaders) { @@ -13,4 +17,16 @@ public UsersClient(Vertx vertx, Map okapiHeaders) { public Future fetchUserById(String userId) { return getById("/users", userId, User.class); } + + public Future> fetchUsers(Collection ids) { + return getByIds("/users", ids, User.class, "users"); + } + + public Future fetchUserGroupById(String userGroupId) { + return getById("/groups", userGroupId, UserGroup.class); + } + + public Future> fetchUserGroupsByIds(Collection userGroupIds) { + return getByIds("/groups", userGroupIds, UserGroup.class, "usergroups"); + } } diff --git a/src/main/java/org/folio/rest/exception/http/HttpException.java b/src/main/java/org/folio/rest/exception/http/HttpException.java index 015b7336..6346a90c 100644 --- a/src/main/java/org/folio/rest/exception/http/HttpException.java +++ b/src/main/java/org/folio/rest/exception/http/HttpException.java @@ -23,6 +23,6 @@ public HttpException(HttpMethod httpMethod, String url, HttpResponse 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); } } diff --git a/src/main/java/org/folio/rest/exception/http/HttpGetByIdException.java b/src/main/java/org/folio/rest/exception/http/HttpGetByIdException.java deleted file mode 100644 index e2d45250..00000000 --- a/src/main/java/org/folio/rest/exception/http/HttpGetByIdException.java +++ /dev/null @@ -1,24 +0,0 @@ -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 HttpGetByIdException extends HttpException { - private final Class objectType; - private final String id; - - public HttpGetByIdException(String url, HttpResponse response, Class objectType, String id) { - super(HttpMethod.GET, url, response); - this.objectType = objectType; - this.id = id; - } - - @Override - public String getMessage() { - return String.format("Failed to fetch %s %s: [%d] %s", objectType.getSimpleName(), id, - getResponseStatus(), getResponseBody()); - } -} diff --git a/src/main/java/org/folio/rest/exception/http/HttpGetException.java b/src/main/java/org/folio/rest/exception/http/HttpGetException.java new file mode 100644 index 00000000..4e9fdedc --- /dev/null +++ b/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 response) { + super(HttpMethod.GET, url, response); + } +} diff --git a/src/main/java/org/folio/rest/repository/AbstractRepository.java b/src/main/java/org/folio/rest/repository/AbstractRepository.java new file mode 100644 index 00000000..cd3e52fe --- /dev/null +++ b/src/main/java/org/folio/rest/repository/AbstractRepository.java @@ -0,0 +1,88 @@ +package org.folio.rest.repository; + +import static io.vertx.core.Future.succeededFuture; +import static java.lang.String.format; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; + +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.rest.persist.PostgresClient; +import org.folio.rest.service.report.utils.LookupHelper; +import org.folio.rest.tools.utils.TenantTool; + +import io.vertx.core.Context; +import io.vertx.core.Future; +import io.vertx.core.json.JsonObject; +import io.vertx.sqlclient.Row; +import io.vertx.sqlclient.RowSet; + +public abstract class AbstractRepository { + private static final Logger log = LogManager.getLogger(AbstractRepository.class); + + protected final PostgresClient pgClient; + + AbstractRepository(Map headers, Context context) { + pgClient = PostgresClient.getInstance(context.owner(), TenantTool.tenantId(headers)); + } + + Future getById(String tableName, String id, Class objectType) { + return pgClient.getById(tableName, id, objectType); + } + + Future> getByQuery(String query, Class objectType) { + return pgClient.select(query) + .map(rowSet -> mapRowSet(rowSet, objectType)); + } + + Future> getByIds(String tableName, Collection ids, Class objectType) { + log.info("Fetching {} {} by ID", ids.size(), objectType.getSimpleName()); + return getByKeyValues(tableName, "id", ids, objectType); + } + + Future> getByKeyValues(String tableName, String key, Collection values, + Class objectType) { + + Set filteredValues = values.stream() + .filter(StringUtils::isNotBlank) + .collect(toSet()); + + if (filteredValues.isEmpty()) { + return succeededFuture(new ArrayList<>()); + } + + String joinedValues = filteredValues.stream() + .map(value -> StringUtils.wrap(value, "'")) + .collect(Collectors.joining(",")); + + String query = format( + "SELECT t.jsonb FROM %s.%s t WHERE left(lower(f_unaccent(jsonb->>'%s')), 600) IN (%s)", + getSchemaName(), tableName, key, joinedValues); + + return getByQuery(query, objectType); + } + + private static Collection mapRowSet(RowSet rowSet, Class recordType) { + return StreamSupport.stream(rowSet.spliterator(), false) + .map(row -> row.get(JsonObject.class, row.getColumnIndex("jsonb"))) + .map(json -> json.mapTo(recordType)) + .collect(toList()); + } + + String getSchemaName() { + return pgClient.getSchemaName(); + } + + String getTenantId() { + return pgClient.getTenantId(); + } + +} diff --git a/src/main/java/org/folio/rest/repository/FeeFineActionRepository.java b/src/main/java/org/folio/rest/repository/FeeFineActionRepository.java index f4782b77..76635754 100644 --- a/src/main/java/org/folio/rest/repository/FeeFineActionRepository.java +++ b/src/main/java/org/folio/rest/repository/FeeFineActionRepository.java @@ -14,6 +14,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import org.folio.rest.domain.Action; @@ -23,9 +24,7 @@ import org.folio.rest.persist.Criteria.Criterion; import org.folio.rest.persist.Criteria.GroupedCriterias; import org.folio.rest.persist.Criteria.Limit; -import org.folio.rest.persist.PostgresClient; import org.folio.rest.persist.interfaces.Results; -import org.folio.rest.tools.utils.TenantTool; import org.folio.rest.utils.FeeFineActionHelper; import io.vertx.core.Context; @@ -37,7 +36,7 @@ import io.vertx.sqlclient.RowSet; import io.vertx.sqlclient.Tuple; -public class FeeFineActionRepository { +public class FeeFineActionRepository extends AbstractRepository { private static final String ACTIONS_TABLE = "feefineactions"; private static final String ACCOUNTS_TABLE = "accounts"; public static final String ACTIONS_TABLE_ALIAS = "actions"; @@ -53,12 +52,8 @@ public class FeeFineActionRepository { public static final String ORDER_BY_OWNER_SOURCE_DATE_ASC = "accounts.jsonb->>'feeFineOwner', " + "actions.jsonb->>'source' ASC, actions.jsonb->>'dateAction' ASC"; - private final PostgresClient pgClient; - private final String tenantId; - public FeeFineActionRepository(Map headers, Context context) { - pgClient = PostgresClient.getInstance(context.owner(), TenantTool.tenantId(headers)); - tenantId = TenantTool.tenantId(headers); + super(headers, context); } public Future> get(Criterion criterion) { @@ -84,6 +79,14 @@ public Future> findActionsForAccount(String accountId) { .map(Results::getResults); } + public Future> findActionsForAccounts(Collection accounts) { + Set accountIds = accounts.stream() + .map(Account::getId) + .collect(Collectors.toSet()); + + return getByKeyValues(ACTIONS_TABLE, ACCOUNT_ID_FIELD, accountIds, Feefineaction.class); + } + public Future> findActionsOfTypesForAccount(String accountId, List types) { @@ -201,11 +204,11 @@ public Future> findFeeFineActionsAndAccounts( String query = format( "SELECT actions.jsonb, accounts.jsonb FROM %1$s.%2$s %3$s " + - "LEFT OUTER JOIN %1$s.%4$s %5$s ON %3$s.jsonb->>'accountId' = %5$s.jsonb->>'id' " + + "LEFT OUTER JOIN %1$s.%4$s %5$s ON left(lower(f_unaccent(%3$s.jsonb->>'accountId')), 600) = %5$s.jsonb->>'id' " + "WHERE " + join(" AND ", conditions) + " " + "ORDER BY %6$s " + "LIMIT $1", - PostgresClient.convertToPsqlStandard(tenantId), + getSchemaName(), ACTIONS_TABLE, ACTIONS_TABLE_ALIAS, ACCOUNTS_TABLE, ACCOUNTS_TABLE_ALIAS, orderBy); @@ -235,7 +238,7 @@ public Future> findSources(Action typeAction, String createdAt, int "FROM %1$s.%2$s %3$s " + "WHERE " + join(" AND ", conditions) + " " + "LIMIT $1", - PostgresClient.convertToPsqlStandard(tenantId), ACTIONS_TABLE, ACTIONS_TABLE_ALIAS); + getSchemaName(), ACTIONS_TABLE, ACTIONS_TABLE_ALIAS); Promise> promise = Promise.promise(); pgClient.select(query, params, promise); diff --git a/src/main/java/org/folio/rest/repository/LostItemFeePolicyRepository.java b/src/main/java/org/folio/rest/repository/LostItemFeePolicyRepository.java index a4313dea..ecc499b4 100644 --- a/src/main/java/org/folio/rest/repository/LostItemFeePolicyRepository.java +++ b/src/main/java/org/folio/rest/repository/LostItemFeePolicyRepository.java @@ -1,31 +1,25 @@ package org.folio.rest.repository; +import java.util.Collection; import java.util.Map; import org.folio.rest.jaxrs.model.LostItemFeePolicy; -import org.folio.rest.persist.PostgresClient; -import org.folio.rest.tools.utils.TenantTool; import io.vertx.core.Context; import io.vertx.core.Future; -import io.vertx.core.Promise; -public class LostItemFeePolicyRepository { +public class LostItemFeePolicyRepository extends AbstractRepository { private static final String TABLE_NAME = "lost_item_fee_policy"; - private final PostgresClient pgClient; - public LostItemFeePolicyRepository(Context context, Map headers) { - this(PostgresClient.getInstance(context.owner(), TenantTool.tenantId(headers))); + super(headers, context); } - public LostItemFeePolicyRepository(PostgresClient pgClient) { - this.pgClient = pgClient; + public Future getLostItemFeePolicyById(String id) { + return getById(TABLE_NAME, id, LostItemFeePolicy.class); } - public Future getLostItemFeePolicyById(String id) { - Promise promise = Promise.promise(); - pgClient.getById(TABLE_NAME, id, LostItemFeePolicy.class, promise); - return promise.future(); + public Future> getLostItemFeePoliciesByIds(Collection ids) { + return getByIds(TABLE_NAME, ids, LostItemFeePolicy.class); } } diff --git a/src/main/java/org/folio/rest/repository/OverdueFinePolicyRepository.java b/src/main/java/org/folio/rest/repository/OverdueFinePolicyRepository.java index 9030c95a..7047dba4 100644 --- a/src/main/java/org/folio/rest/repository/OverdueFinePolicyRepository.java +++ b/src/main/java/org/folio/rest/repository/OverdueFinePolicyRepository.java @@ -1,31 +1,25 @@ package org.folio.rest.repository; +import java.util.Collection; import java.util.Map; import org.folio.rest.jaxrs.model.OverdueFinePolicy; -import org.folio.rest.persist.PostgresClient; -import org.folio.rest.tools.utils.TenantTool; import io.vertx.core.Context; import io.vertx.core.Future; -import io.vertx.core.Promise; -public class OverdueFinePolicyRepository { +public class OverdueFinePolicyRepository extends AbstractRepository { private static final String TABLE_NAME = "overdue_fine_policy"; - private final PostgresClient pgClient; - - public OverdueFinePolicyRepository(PostgresClient pgClient) { - this.pgClient = pgClient; - } - public OverdueFinePolicyRepository(Context context, Map headers) { - this(PostgresClient.getInstance(context.owner(), TenantTool.tenantId(headers))); + super(headers, context); } public Future getOverdueFinePolicyById(String id) { - Promise promise = Promise.promise(); - pgClient.getById(TABLE_NAME, id, OverdueFinePolicy.class, promise); - return promise.future(); + return getById(TABLE_NAME, id, OverdueFinePolicy.class); + } + + public Future> getOverdueFinePoliciesByIds(Collection ids) { + return getByIds(TABLE_NAME, ids, OverdueFinePolicy.class); } } diff --git a/src/main/java/org/folio/rest/service/report/AccountContextData.java b/src/main/java/org/folio/rest/service/report/AccountContextData.java new file mode 100644 index 00000000..f966e9c5 --- /dev/null +++ b/src/main/java/org/folio/rest/service/report/AccountContextData.java @@ -0,0 +1,44 @@ +package org.folio.rest.service.report; + +import java.util.ArrayList; +import java.util.List; + +import org.folio.rest.jaxrs.model.Account; +import org.folio.rest.jaxrs.model.Feefineaction; +import org.folio.rest.jaxrs.model.HoldingsRecord; +import org.folio.rest.jaxrs.model.Instance; +import org.folio.rest.jaxrs.model.Loan; +import org.folio.rest.jaxrs.model.LoanPolicy; +import org.folio.rest.jaxrs.model.LostItemFeePolicy; +import org.folio.rest.jaxrs.model.OverdueFinePolicy; + +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.With; + +@AllArgsConstructor +@Getter +@With +public class AccountContextData { + final Account account; + final List actions; + final HoldingsRecord holdingsRecord; + final Instance instance; + final String effectiveLocation; + final Loan loan; + final LoanPolicy loanPolicy; + final OverdueFinePolicy overdueFinePolicy; + final LostItemFeePolicy lostItemFeePolicy; + + public AccountContextData() { + account = null; + actions = new ArrayList<>(); + holdingsRecord = null; + instance = null; + effectiveLocation = ""; + loan = null; + loanPolicy = null; + overdueFinePolicy = null; + lostItemFeePolicy = null; + } +} diff --git a/src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportContext.java b/src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportContext.java new file mode 100644 index 00000000..b6e606f0 --- /dev/null +++ b/src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportContext.java @@ -0,0 +1,224 @@ +package org.folio.rest.service.report; + +import static io.vertx.core.Future.succeededFuture; +import static org.folio.util.UuidUtil.isUuid; +import static org.joda.time.DateTimeZone.UTC; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.folio.rest.jaxrs.model.Account; +import org.folio.rest.jaxrs.model.Feefineaction; +import org.folio.rest.jaxrs.model.Instance; +import org.folio.rest.jaxrs.model.Item; +import org.folio.rest.jaxrs.model.Loan; +import org.folio.rest.jaxrs.model.LoanPolicy; +import org.folio.rest.jaxrs.model.Location; +import org.folio.rest.jaxrs.model.LostItemFeePolicy; +import org.folio.rest.jaxrs.model.OverdueFinePolicy; +import org.folio.rest.jaxrs.model.ServicePoint; +import org.folio.rest.jaxrs.model.User; +import org.folio.rest.jaxrs.model.UserGroup; +import org.folio.rest.service.report.context.HasItemInfo; +import org.folio.rest.service.report.context.HasLoanInfo; +import org.folio.rest.service.report.context.HasServicePointsInfo; +import org.folio.rest.service.report.context.HasUserInfo; +import org.joda.time.DateTimeZone; + +import io.vertx.core.Future; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.With; + +@With +@AllArgsConstructor +@Getter +public class FinancialTransactionsDetailReportContext + implements HasUserInfo, HasItemInfo, HasServicePointsInfo, + HasLoanInfo { + + final DateTimeZone timeZone; + final Map actionsToAccounts; + final Map accountContexts; + final Map users; + final Map userGroups; + final Map items; + final Map servicePoints; + + public FinancialTransactionsDetailReportContext() { + timeZone = UTC; + actionsToAccounts = new HashMap<>(); + accountContexts = new HashMap<>(); + users = new HashMap<>(); + userGroups = new HashMap<>(); + items = new HashMap<>(); + servicePoints = new HashMap<>(); + } + + AccountContextData getAccountContextById(String accountId) { + if (accountId == null) { + return null; + } + + AccountContextData existingContext = accountContexts.get(accountId); + if (existingContext != null) { + return existingContext; + } + + Account account = actionsToAccounts.values().stream() + .filter(a -> accountId.equals(a.getId())) + .findFirst() + .orElse(null); + + if (account != null) { + AccountContextData accountContext = new AccountContextData().withAccount(account); + accountContexts.put(accountId, accountContext); + return accountContext; + } + + return null; + } + + @Override + public Account getAccountById(String accountId) { + AccountContextData accountContextData = getAccountContextById( + accountId); + return accountContextData == null ? null : accountContextData.account; + } + + @Override + public Item getItemByAccountId(String accountId) { + Account account = getAccountById(accountId); + + if (account != null) { + String itemId = account.getItemId(); + if (itemId != null && items.containsKey(itemId)) { + return items.get(itemId); + } + } + + return null; + } + + @Override + public User getUserByAccountId(String accountId) { + Account account = getAccountById(accountId); + + if (account != null && isUuid(account.getUserId())) { + return users.get(account.getUserId()); + } + + return null; + } + + @Override + public UserGroup getUserGroupByAccountId(String accountId) { + User user = getUserByAccountId(accountId); + + if (user != null && isUuid(user.getPatronGroup())) { + return userGroups.get(user.getPatronGroup()); + } + + return null; + } + + @Override + public Future updateAccountContextWithInstance(String accountId, Instance instance) { + accountContexts.put(accountId, getAccountContextById(accountId).withInstance(instance)); + return succeededFuture(); + } + + @Override + public Future updateAccountContextWithEffectiveLocation(String accountId, + Location effectiveLocation) { + + accountContexts.put(accountId, + getAccountContextById(accountId).withEffectiveLocation(effectiveLocation.getName())); + return succeededFuture(); + } + + @Override + public Future updateAccountContextWithActions(String accountId, + List actions) { + + AccountContextData accountContextData = getAccountContextById(accountId); + if (accountContextData == null) { + return succeededFuture(); + } + + accountContexts.put(accountId, accountContextData.withActions(actions)); + return succeededFuture(); + } + + @Override + public boolean isAccountContextCreated(String accountId) { + return getAccountContextById(accountId) != null; + } + + @Override + public List getAccountFeeFineActions(String accountId) { + AccountContextData accountCtx = getAccountContextById(accountId); + if (accountCtx == null) { + return new ArrayList<>(); + } + + return accountCtx.getActions(); + } + + @Override + public Loan getLoanByAccountId(String accountId) { + AccountContextData accountContextData = getAccountContextById( + accountId); + if (accountContextData == null) { + return null; + } + + return accountContextData.loan; + } + + @Override + public void updateAccountContextWithLoan(String accountId, Loan loan) { + AccountContextData accountContext = getAccountContextById(accountId); + if (accountContext == null) { + return; + } + + accountContexts.put(accountId, accountContext.withLoan(loan)); + } + + @Override + public void updateAccountContextWithLoanPolicy(String accountId, LoanPolicy loanPolicy) { + AccountContextData accountContext = getAccountContextById(accountId); + if (accountContext == null) { + return; + } + + accountContexts.put(accountId, accountContext.withLoanPolicy(loanPolicy)); + } + + @Override + public void updateAccountContextWithOverdueFinePolicy(String accountId, + OverdueFinePolicy overdueFinePolicy) { + + AccountContextData accountContext = getAccountContextById(accountId); + if (accountContext == null) { + return; + } + + accountContexts.put(accountId, accountContext.withOverdueFinePolicy(overdueFinePolicy)); + } + + @Override + public void updateAccountContextWithLostItemFeePolicy(String accountId, + LostItemFeePolicy lostItemFeePolicy) { + + AccountContextData accountContext = getAccountContextById(accountId); + if (accountContext == null) { + return; + } + + accountContexts.put(accountId, accountContext.withLostItemFeePolicy(lostItemFeePolicy)); + } +} diff --git a/src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportService.java b/src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportService.java index 8e55c138..ebd0fd0a 100644 --- a/src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportService.java +++ b/src/main/java/org/folio/rest/service/report/FinancialTransactionsDetailReportService.java @@ -1,19 +1,16 @@ package org.folio.rest.service.report; -import static io.vertx.core.Future.succeededFuture; +import static java.lang.System.currentTimeMillis; import static org.folio.rest.domain.Action.CANCEL; import static org.folio.rest.domain.Action.PAY; import static org.folio.rest.domain.Action.REFUND; import static org.folio.rest.domain.Action.TRANSFER; import static org.folio.rest.domain.Action.WAIVE; -import static org.folio.rest.repository.FeeFineActionRepository.ORDER_BY_ACTION_DATE_ASC; import static org.folio.rest.service.report.utils.ReportStatsHelper.calculateTotals; import static org.folio.rest.utils.FeeFineActionHelper.getPatronInfoFromComment; import static org.folio.rest.utils.FeeFineActionHelper.getStaffInfoFromComment; import static org.folio.rest.utils.PatronHelper.buildFormattedName; import static org.folio.rest.utils.PatronHelper.getEmail; -import static org.folio.util.UuidUtil.isUuid; -import static org.joda.time.DateTimeZone.UTC; import java.util.ArrayList; import java.util.HashMap; @@ -24,48 +21,31 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.folio.rest.client.CirculationStorageClient; -import org.folio.rest.client.InventoryClient; import org.folio.rest.domain.Action; -import org.folio.rest.domain.MonetaryValue; import org.folio.rest.jaxrs.model.Account; import org.folio.rest.jaxrs.model.Contributor; import org.folio.rest.jaxrs.model.Feefineaction; import org.folio.rest.jaxrs.model.FinancialTransactionsDetailReport; import org.folio.rest.jaxrs.model.FinancialTransactionsDetailReportEntry; import org.folio.rest.jaxrs.model.FinancialTransactionsDetailReportStats; -import org.folio.rest.jaxrs.model.Instance; import org.folio.rest.jaxrs.model.Item; import org.folio.rest.jaxrs.model.Loan; import org.folio.rest.jaxrs.model.LoanPolicy; -import org.folio.rest.jaxrs.model.Location; import org.folio.rest.jaxrs.model.LostItemFeePolicy; import org.folio.rest.jaxrs.model.OverdueFinePolicy; import org.folio.rest.jaxrs.model.ServicePoint; import org.folio.rest.jaxrs.model.User; import org.folio.rest.jaxrs.model.UserGroup; -import org.folio.rest.repository.FeeFineActionRepository; -import org.folio.rest.repository.LostItemFeePolicyRepository; -import org.folio.rest.repository.OverdueFinePolicyRepository; -import org.folio.rest.service.report.context.HasItemInfo; -import org.folio.rest.service.report.context.HasLoanInfo; -import org.folio.rest.service.report.context.HasServicePointsInfo; -import org.folio.rest.service.report.context.HasUserInfo; import org.folio.rest.service.report.parameters.FinancialTransactionsDetailReportParameters; import org.folio.rest.service.report.utils.LookupHelper; -import org.joda.time.DateTimeZone; import io.vertx.core.Future; -import lombok.AllArgsConstructor; -import lombok.Getter; -import lombok.With; public class FinancialTransactionsDetailReportService extends DateBasedReportService { private static final Logger log = LogManager.getLogger(FinancialTransactionsDetailReportService.class); - private static final int REPORT_ROWS_LIMIT = 1_000_000; private static final String EMPTY_VALUE = "-"; private static final Map ACTION_NAMES = new HashMap<>(); @@ -83,23 +63,10 @@ public class FinancialTransactionsDetailReportService extends ACTION_NAMES.put("Staff info only", "Staff info only"); } - private final InventoryClient inventoryClient; - private final CirculationStorageClient circulationStorageClient; - private final FeeFineActionRepository feeFineActionRepository; - private final LostItemFeePolicyRepository lostItemFeePolicyRepository; - private final OverdueFinePolicyRepository overdueFinePolicyRepository; - private final LookupHelper lookupHelper; public FinancialTransactionsDetailReportService(Map headers, io.vertx.core.Context context) { super(headers, context); - - inventoryClient = new InventoryClient(context.owner(), headers); - circulationStorageClient = new CirculationStorageClient(context.owner(), headers); - feeFineActionRepository = new FeeFineActionRepository(headers, context); - lostItemFeePolicyRepository = new LostItemFeePolicyRepository(context, headers); - overdueFinePolicyRepository = new OverdueFinePolicyRepository(context, headers); - lookupHelper = new LookupHelper(headers, context); } @@ -107,8 +74,12 @@ public FinancialTransactionsDetailReportService(Map headers, io. public Future build( FinancialTransactionsDetailReportParameters params) { + long startTimeMillis = currentTimeMillis(); + return adjustDates(params) - .compose(v -> buildWithAdjustedDates(params)); + .compose(v -> buildWithAdjustedDates(params)) + .onSuccess(r -> log.info("Report built in {} ms", currentTimeMillis() - startTimeMillis)) + .onFailure(t -> log.error("Report construction failed", t)); } private Future buildWithAdjustedDates( @@ -125,40 +96,21 @@ private Future buildWithAdjustedDates( CANCEL.getFullResult(), "Staff info only"); - Context ctx = new Context(); - - return feeFineActionRepository.findFeeFineActionsAndAccounts(actionTypes, - params.getStartDate(), params.getEndDate(), List.of(params.getFeeFineOwner()), - params.getCreatedAt(), null, ORDER_BY_ACTION_DATE_ASC, REPORT_ROWS_LIMIT) - .map(ctx::withActionsToAccounts) - .compose(this::processAllFeeFineActions) + FinancialTransactionsDetailReportContext ctx = new FinancialTransactionsDetailReportContext(); + + return lookupHelper.findActionsAndAccounts(params, actionTypes, ctx) + .compose(lookupHelper::lookupActionsForAccounts) + .compose(lookupHelper::lookupServicePointsForFeeFineActions) + .compose(lookupHelper::lookupUsersForAccounts) + .compose(lookupHelper::lookupGroupsForUsers) + .compose(lookupHelper::lookupItemsForAccounts) + .compose(lookupHelper::lookupInstancesForItems) + .compose(lookupHelper::lookupLocationsForItems) + .compose(lookupHelper::lookupLoansForAccounts) .map(this::buildReport); } - private Future processAllFeeFineActions(Context ctx) { - return ctx.actionsToAccounts.keySet().stream() - .reduce(succeededFuture(ctx), - (f, a) -> f.compose(result -> processSingleFeeFineAction(ctx, a)), - (a, b) -> succeededFuture(ctx)); - } - - private Future processSingleFeeFineAction(Context ctx, Feefineaction feeFineAction) { - Account account = ctx.actionsToAccounts.get(feeFineAction); - String accountId = account.getId(); - - return lookupHelper.lookupFeeFineActionsForAccount(ctx, accountId) - .compose(r -> lookupHelper.lookupServicePointsForAllActionsInAccount(ctx, accountId)) - .compose(r -> lookupHelper.lookupUserForAccount(ctx, account)) - .compose(r -> lookupHelper.lookupUserGroupForUser(ctx, accountId)) - .compose(r -> lookupHelper.lookupItemForAccount(ctx, accountId)) - .compose(r -> lookupHelper.lookupInstanceForAccount(ctx, accountId)) - .compose(r -> lookupHelper.lookupLocationForAccount(ctx, accountId)) - .compose(r -> lookupHelper.lookupLoanForAccount(ctx, accountId)) - .compose(r -> lookupHelper.lookupOverdueFinePolicyForAccount(ctx, accountId)) - .compose(r -> lookupHelper.lookupLostItemFeePolicyForAccount(ctx, accountId)); - } - - private FinancialTransactionsDetailReport buildReport(Context ctx) { + private FinancialTransactionsDetailReport buildReport(FinancialTransactionsDetailReportContext ctx) { List entryList = ctx.actionsToAccounts.keySet().stream() .map(action -> buildReportEntry(ctx, action)) .collect(Collectors.toList()); @@ -168,7 +120,8 @@ private FinancialTransactionsDetailReport buildReport(Context ctx) { .withReportStats(buildFinancialTransactionsDetailReportStats(ctx)); } - private FinancialTransactionsDetailReportEntry buildReportEntry(Context ctx, + private FinancialTransactionsDetailReportEntry buildReportEntry( + FinancialTransactionsDetailReportContext ctx, Feefineaction feeFineAction) { FinancialTransactionsDetailReportEntry entry = new FinancialTransactionsDetailReportEntry(); @@ -295,7 +248,7 @@ private FinancialTransactionsDetailReportEntry buildReportEntry(Context ctx, } private FinancialTransactionsDetailReportStats buildFinancialTransactionsDetailReportStats( - Context ctx) { + FinancialTransactionsDetailReportContext ctx) { FinancialTransactionsDetailReportStats stats = new FinancialTransactionsDetailReportStats(); @@ -346,222 +299,9 @@ private FinancialTransactionsDetailReportStats buildFinancialTransactionsDetailR return stats; } - private String formatMonetaryValue(Double value) { - return new MonetaryValue(value, currency).toString(); - } - - @With - @AllArgsConstructor - @Getter - private static class Context implements HasUserInfo, HasItemInfo, HasServicePointsInfo, - HasLoanInfo { - - final DateTimeZone timeZone; - final Map actionsToAccounts; - final Map accountContexts; - final Map users; - final Map userGroups; - final Map items; - final Map servicePoints; - - public Context() { - timeZone = UTC; - actionsToAccounts = new HashMap<>(); - accountContexts = new HashMap<>(); - users = new HashMap<>(); - userGroups = new HashMap<>(); - items = new HashMap<>(); - servicePoints = new HashMap<>(); - } - - AccountContextData getAccountContextById(String accountId) { - if (accountId == null) { - return null; - } - - if (accountContexts.containsKey(accountId)) { - return accountContexts.get(accountId); - } - - Account account = actionsToAccounts.values().stream() - .filter(a -> accountId.equals(a.getId())) - .findFirst() - .orElse(null); - - if (account != null) { - AccountContextData accountContext = new AccountContextData().withAccount(account); - accountContexts.put(accountId, accountContext); - return accountContext; - } - - return null; - } - - @Override - public Account getAccountById(String accountId) { - AccountContextData accountContextData = getAccountContextById(accountId); - return accountContextData == null ? null : accountContextData.account; - } - - @Override - public Item getItemByAccountId(String accountId) { - Account account = getAccountById(accountId); - - if (account != null) { - String itemId = account.getItemId(); - if (itemId != null && items.containsKey(itemId)) { - return items.get(itemId); - } - } - - return null; - } - - @Override - public User getUserByAccountId(String accountId) { - Account account = getAccountById(accountId); - - if (account != null && isUuid(account.getUserId())) { - return users.get(account.getUserId()); - } - - return null; - } - - @Override - public UserGroup getUserGroupByAccountId(String accountId) { - User user = getUserByAccountId(accountId); - - if (user != null && isUuid(user.getPatronGroup())) { - return userGroups.get(user.getPatronGroup()); - } - - return null; - } - - @Override - public Future updateAccountContextWithInstance(String accountId, Instance instance) { - accountContexts.put(accountId, getAccountContextById(accountId).withInstance(instance)); - return succeededFuture(); - } - - @Override - public Future updateAccountContextWithEffectiveLocation(String accountId, - Location effectiveLocation) { - - accountContexts.put(accountId, - getAccountContextById(accountId).withEffectiveLocation(effectiveLocation.getName())); - return succeededFuture(); - } - - @Override - public Future updateAccountContextWithActions(String accountId, - List actions) { - - AccountContextData accountContextData = getAccountContextById(accountId); - if (accountContextData == null) { - return succeededFuture(); - } - - accountContexts.put(accountId, accountContextData.withActions(actions)); - return succeededFuture(); - } - - @Override - public boolean isAccountContextCreated(String accountId) { - return getAccountContextById(accountId) != null; - } - - @Override - public List getAccountFeeFineActions(String accountId) { - AccountContextData accountCtx = getAccountContextById(accountId); - if (accountCtx == null) { - return null; - } - - return accountCtx.getActions(); - } - - @Override - public Loan getLoanByAccountId(String accountId) { - AccountContextData accountContextData = getAccountContextById(accountId); - if (accountContextData == null) { - return null; - } - - return accountContextData.loan; - } - - @Override - public void updateAccountContextWithLoan(String accountId, Loan loan) { - if (!isAccountContextCreated(accountId)) { - return; - } - - accountContexts.put(accountId, getAccountContextById(accountId).withLoan(loan)); - } - - @Override - public void updateAccountContextWithLoanPolicy(String accountId, LoanPolicy loanPolicy) { - if (!isAccountContextCreated(accountId)) { - return; - } - - accountContexts.put(accountId, getAccountContextById(accountId).withLoanPolicy(loanPolicy)); - } - - @Override - public void updateAccountContextWithOverdueFinePolicy(String accountId, - OverdueFinePolicy overdueFinePolicy) { - - if (!isAccountContextCreated(accountId)) { - return; - } - - accountContexts.put(accountId, getAccountContextById(accountId) - .withOverdueFinePolicy(overdueFinePolicy)); - } - - @Override - public void updateAccountContextWithLostItemFeePolicy(String accountId, - LostItemFeePolicy lostItemFeePolicy) { - - if (!isAccountContextCreated(accountId)) { - return; - } - - accountContexts.put(accountId, getAccountContextById(accountId) - .withLostItemFeePolicy(lostItemFeePolicy)); - } - } - private String getPaymentMethod(Action action, Feefineaction feeFineAction) { return List.of(action.getPartialResult(), action.getFullResult()) .contains(feeFineAction.getTypeAction()) ? feeFineAction.getPaymentMethod() : ""; } - @AllArgsConstructor - @Getter - @With - private static class AccountContextData { - final Account account; - final List actions; - final Instance instance; - final String effectiveLocation; - final Loan loan; - final LoanPolicy loanPolicy; - final OverdueFinePolicy overdueFinePolicy; - final LostItemFeePolicy lostItemFeePolicy; - - public AccountContextData() { - account = null; - actions = new ArrayList<>(); - instance = null; - effectiveLocation = ""; - loan = null; - loanPolicy = null; - overdueFinePolicy = null; - lostItemFeePolicy = null; - } - } } diff --git a/src/main/java/org/folio/rest/service/report/utils/LookupHelper.java b/src/main/java/org/folio/rest/service/report/utils/LookupHelper.java index 31b561a7..c56efb96 100644 --- a/src/main/java/org/folio/rest/service/report/utils/LookupHelper.java +++ b/src/main/java/org/folio/rest/service/report/utils/LookupHelper.java @@ -1,38 +1,54 @@ package org.folio.rest.service.report.utils; import static io.vertx.core.Future.succeededFuture; +import static java.util.function.Function.identity; +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; import static org.folio.rest.domain.Action.PAY; import static org.folio.rest.domain.Action.REFUND; import static org.folio.rest.domain.Action.TRANSFER; +import static org.folio.rest.repository.FeeFineActionRepository.ORDER_BY_ACTION_DATE_ASC; import static org.folio.util.UuidUtil.isUuid; +import java.util.Collection; import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.rest.client.CirculationStorageClient; import org.folio.rest.client.InventoryClient; -import org.folio.rest.client.UserGroupsClient; import org.folio.rest.client.UsersClient; import org.folio.rest.jaxrs.model.Account; import org.folio.rest.jaxrs.model.Feefineaction; import org.folio.rest.jaxrs.model.HoldingsRecord; +import org.folio.rest.jaxrs.model.Instance; import org.folio.rest.jaxrs.model.Item; import org.folio.rest.jaxrs.model.Loan; +import org.folio.rest.jaxrs.model.LoanPolicy; +import org.folio.rest.jaxrs.model.Location; +import org.folio.rest.jaxrs.model.LostItemFeePolicy; +import org.folio.rest.jaxrs.model.OverdueFinePolicy; import org.folio.rest.jaxrs.model.ServicePoint; import org.folio.rest.jaxrs.model.User; +import org.folio.rest.jaxrs.model.UserGroup; import org.folio.rest.repository.FeeFineActionRepository; import org.folio.rest.repository.LostItemFeePolicyRepository; import org.folio.rest.repository.OverdueFinePolicyRepository; +import org.folio.rest.service.report.FinancialTransactionsDetailReportContext; import org.folio.rest.service.report.context.HasAccountInfo; import org.folio.rest.service.report.context.HasItemInfo; -import org.folio.rest.service.report.context.HasLoanInfo; -import org.folio.rest.service.report.context.HasServicePointsInfo; import org.folio.rest.service.report.context.HasUserInfo; +import org.folio.rest.service.report.parameters.FinancialTransactionsDetailReportParameters; +import org.folio.util.UuidUtil; import org.joda.time.DateTime; import io.vertx.core.Context; @@ -40,10 +56,10 @@ public class LookupHelper { private static final Logger log = LogManager.getLogger(LookupHelper.class); + private static final int REPORT_ROWS_LIMIT = 1_000_000; private final InventoryClient inventoryClient; private final UsersClient usersClient; - private final UserGroupsClient userGroupsClient; private final FeeFineActionRepository feeFineActionRepository; private final CirculationStorageClient circulationStorageClient; @@ -53,7 +69,6 @@ public class LookupHelper { public LookupHelper(Map headers, Context context) { inventoryClient = new InventoryClient(context.owner(), headers); usersClient = new UsersClient(context.owner(), headers); - userGroupsClient = new UserGroupsClient(context.owner(), headers); circulationStorageClient = new CirculationStorageClient(context.owner(), headers); feeFineActionRepository = new FeeFineActionRepository(headers, context); @@ -81,6 +96,22 @@ public Future lookupUserForAccount(T ctx, Account acc .otherwise(ctx); } + public Future lookupUsersForAccounts( + FinancialTransactionsDetailReportContext context) { + + Set userIds = context.getActionsToAccounts().values() + .stream() + .filter(Objects::nonNull) + .map(Account::getUserId) + .collect(toSet()); + + return usersClient.fetchUsers(userIds) + .map(users -> mapBy(users, User::getId)) + .map(context::withUsers) + .onFailure(t -> log.error("Failed to fetch users", t)) + .otherwise(context); + } + private Future addUserToContext(T ctx, User user, String accountId, String userId) { @@ -100,12 +131,28 @@ public Future lookupUserGroupForUser(T ctx, String ac return succeededFuture(ctx); } - return userGroupsClient.fetchUserGroupById(user.getPatronGroup()) + return usersClient.fetchUserGroupById(user.getPatronGroup()) .map(userGroup -> ctx.getUserGroups().put(userGroup.getId(), userGroup)) .map(ctx) .otherwise(ctx); } + public Future lookupGroupsForUsers( + FinancialTransactionsDetailReportContext context) { + + Set patronGroupIds = context.getUsers().values() + .stream() + .filter(Objects::nonNull) + .map(User::getPatronGroup) + .collect(toSet()); + + return usersClient.fetchUserGroupsByIds(patronGroupIds) + .map(groups -> mapBy(groups, UserGroup::getId)) + .map(context::withUserGroups) + .onFailure(t -> log.error("Failed to fetch user groups", t)) + .otherwise(context); + } + public Future lookupItemForAccount(T ctx, String accountId) { Account account = ctx.getAccountById(accountId); if (account == null) { @@ -128,6 +175,22 @@ public Future lookupItemForAccount(T ctx, String acco .otherwise(ctx); } + public Future lookupItemsForAccounts( + FinancialTransactionsDetailReportContext context) { + + Set itemIds = context.getActionsToAccounts().values() + .stream() + .filter(Objects::nonNull) + .map(Account::getItemId) + .collect(toSet()); + + return inventoryClient.getItemsByIds(itemIds) + .map(items -> mapBy(items, Item::getId)) + .map(context::withItems) + .onFailure(t -> log.error("Failed to fetch items", t)) + .otherwise(context); + } + private T addItemToContext(T ctx, Item item, String accountId, String itemId) { @@ -169,50 +232,102 @@ public Future lookupInstanceForAccount(T ctx, String }); } - public Future lookupLocationForAccount(T ctx, String accountId) { - Account account = ctx.getAccountById(accountId); - if (account == null) { - return succeededFuture(ctx); - } + public Future lookupInstancesForItems( + FinancialTransactionsDetailReportContext context) { - Item item = ctx.getItemByAccountId(accountId); - if (item == null) { - return succeededFuture(ctx); - } + Set holdingsRecordIds = context.getItems().values() + .stream() + .filter(Objects::nonNull) + .map(Item::getHoldingsRecordId) + .collect(toSet()); - String effectiveLocationId = item.getEffectiveLocationId(); - if (!isUuid(effectiveLocationId)) { - log.info("Effective location ID {} is not a valid UUID - account {}", effectiveLocationId, - accountId); - return succeededFuture(ctx); - } + return inventoryClient.getHoldingsByIds(holdingsRecordIds) + .onFailure(t -> log.error("Failed to fetch holdings", t)) + .compose(holdings -> fetchInstancesForHoldings(context, holdings)) + .otherwise(context); + } - return inventoryClient.getLocationById(effectiveLocationId) - .map(effectiveLocation -> - ctx.updateAccountContextWithEffectiveLocation(accountId, effectiveLocation)) - .map(ctx) - .otherwise(throwable -> { - log.error("Failed to find location for account {}, effectiveLocationId is {}", accountId, - effectiveLocationId); - return ctx; - }); + private Future fetchInstancesForHoldings( + FinancialTransactionsDetailReportContext context, Collection holdingsRecords) { + + Map holdingsIdToInstanceId = holdingsRecords.stream() + .collect(toMap(HoldingsRecord::getId, HoldingsRecord::getInstanceId)); + + return inventoryClient.getInstancesByIds(holdingsIdToInstanceId.values()) + .map(instances -> mapBy(instances, Instance::getId)) + .onSuccess(instances -> context.getActionsToAccounts() + .values() + .stream() + .filter(Objects::nonNull) + .forEach(account -> Optional.ofNullable(account.getItemId()) + .map(itemId -> context.getItems().get(itemId)) + .map(Item::getHoldingsRecordId) + .map(holdingsIdToInstanceId::get) + .map(instances::get) + .ifPresent(instance -> context.updateAccountContextWithInstance(account.getId(), instance)))) + .onFailure(t -> log.error("Failed to fetch instances", t)) + .map(context); } - public Future lookupFeeFineActionsForAccount(T ctx, - String accountId) { + public Future lookupLocationsForItems( + FinancialTransactionsDetailReportContext context) { + + Set locationIds = context.getItems().values() + .stream() + .filter(Objects::nonNull) + .map(Item::getEffectiveLocationId) + .collect(toSet()); + + return inventoryClient.getLocationsByIds(locationIds) + .onSuccess(locations -> { + Map locationsById = mapBy(locations, Location::getId); + context.getActionsToAccounts() + .values() + .stream() + .filter(Objects::nonNull) + .forEach(account -> Optional.ofNullable(account.getItemId()) + .map(itemId -> context.getItems().get(itemId)) + .map(Item::getEffectiveLocationId) + .map(locationsById::get) + .ifPresent(location -> context.updateAccountContextWithEffectiveLocation(account.getId(), location))); + }) + .map(context) + .onFailure(t -> log.error("Failed to fetch locations", t)) + .otherwise(context); + } - if (!ctx.isAccountContextCreated(accountId)) { - return succeededFuture(ctx); - } + public Future findActionsAndAccounts( + FinancialTransactionsDetailReportParameters params, List actionTypes, + FinancialTransactionsDetailReportContext ctx) { - return feeFineActionRepository.findActionsForAccount(accountId) - .map(this::sortFeeFineActionsByDate) - .map(actions -> ctx.updateAccountContextWithActions(accountId, actions)) - .map(ctx) - .otherwise(throwable -> { - log.error("Failed to find actions for account {}", accountId); - return ctx; - }); + log.info("Fetching actions and accounts"); + + return feeFineActionRepository.findFeeFineActionsAndAccounts(actionTypes, + params.getStartDate(), params.getEndDate(), List.of(params.getFeeFineOwner()), + params.getCreatedAt(), null, ORDER_BY_ACTION_DATE_ASC, REPORT_ROWS_LIMIT) + .map(ctx::withActionsToAccounts); + } + + public Future lookupActionsForAccounts( + FinancialTransactionsDetailReportContext context) { + + List accounts = context.getActionsToAccounts() + .values() + .stream() + .filter(Objects::nonNull) + .collect(toList()); + + log.info("Fetching actions for accounts"); + + return feeFineActionRepository.findActionsForAccounts(accounts) + .onSuccess(actions -> log.info("Fetched {} actions", actions.size())) + .onSuccess(rowSet -> rowSet.stream() + .sorted(actionDateComparator()) + .collect(groupingBy(Feefineaction::getAccountId)) + .forEach(context::updateAccountContextWithActions)) + .map(context) + .onFailure(t -> log.error("Failed to fetch actions for accounts", t)) + .otherwise(context); } public Future lookupRefundPayTransferFeeFineActionsForAccount(T ctx, @@ -233,139 +348,147 @@ public Future lookupRefundPayTransferFeeFineAction }); } - public Future - lookupServicePointsForAllActionsInAccount(T ctx, String accountId) { - - List actions = ctx.getAccountFeeFineActions(accountId); - if (actions == null || actions.isEmpty()) { - return succeededFuture(ctx); - } + public Future lookupServicePointsForFeeFineActions( + FinancialTransactionsDetailReportContext context) { - return actions.stream() + Set servicePointIds = context.getActionsToAccounts().keySet() + .stream() .map(Feefineaction::getCreatedAt) - .distinct() - .reduce(succeededFuture(ctx), - (f, a) -> f.compose(result -> lookupServicePointForFeeFineAction(ctx, a)), - (a, b) -> succeededFuture(ctx)); - } - - private Future lookupServicePointForFeeFineAction(T ctx, - String servicePointId) { - - if (servicePointId == null) { - return succeededFuture(ctx); - } + .collect(toSet()); - if (!isUuid(servicePointId)) { - log.info("Service point ID is not a valid UUID - {}", servicePointId); - return succeededFuture(ctx); - } else { - if (ctx.getServicePoints().containsKey(servicePointId)) { - return succeededFuture(ctx); - } else { - return inventoryClient.getServicePointById(servicePointId) - .map(sp -> addServicePointToContext(ctx, sp, sp.getId())) - .map(ctx) - .otherwise(ctx); - } - } + return inventoryClient.getServicePointsByIds(servicePointIds) + .map(servicePoints -> mapBy(servicePoints, ServicePoint::getId)) + .map(context::withServicePoints) + .onFailure(t -> log.error("Failed to fetch service points", t)) + .otherwise(context); } - private Future addServicePointToContext(T ctx, - ServicePoint servicePoint, String servicePointId) { - - if (servicePoint == null) { - log.error("Service point not found - service point {}", servicePointId); - } else { - ctx.getServicePoints().put(servicePoint.getId(), servicePoint); - } - - return succeededFuture(ctx); + public Future lookupLoansForAccounts( + FinancialTransactionsDetailReportContext context) { + + Set loanIds = context.getActionsToAccounts().values() + .stream() + .filter(Objects::nonNull) + .map(Account::getLoanId) + .collect(toSet()); + + return circulationStorageClient.getLoansByIds(loanIds) + .onFailure(t -> log.error("Failed to fetch loans", t)) + .onSuccess(loans -> { + Map loansById = mapBy(loans, Loan::getId); + context.getActionsToAccounts() + .values() + .stream() + .filter(Objects::nonNull) + .forEach(account -> Optional.ofNullable(account.getLoanId()) + .map(loansById::get) + .ifPresent(loan -> context.updateAccountContextWithLoan(account.getId(), loan))); + }) + .compose(loans -> lookupPoliciesForLoans(context, loans)) + .otherwise(context); } - public Future lookupLoanForAccount(T ctx, - String accountId) { + private Future lookupPoliciesForLoans( + FinancialTransactionsDetailReportContext context, Collection loans) { - Account account = ctx.getAccountById(accountId); - if (account == null) { - return succeededFuture(ctx); - } - - String loanId = account.getLoanId(); - if (!isUuid(loanId)) { - log.info("Loan ID {} is not a valid UUID - account {}", loanId, accountId); - return succeededFuture(ctx); - } - - return circulationStorageClient.getLoanById(loanId) - .onSuccess(loan -> ctx.updateAccountContextWithLoan(accountId, loan)) - .compose(loan -> circulationStorageClient.getLoanPolicyById(loan.getLoanPolicyId())) - .onSuccess(loanPolicy -> ctx.updateAccountContextWithLoanPolicy(accountId, loanPolicy)) - .map(ctx) - .otherwise(throwable -> { - log.error("Failed to find loan for account {}, loan is {}", accountId, - loanId); - return ctx; - }); + return succeededFuture(loans) + .compose(l -> lookupLoanPoliciesForLoans(context, loans)) + .compose(l -> lookupOverdueFinePoliciesForLoans(context, loans)) + .compose(l -> lookupLostItemFeePoliciesForLoans(context, loans)); } - public Future lookupOverdueFinePolicyForAccount( - T ctx, String accountId) { - - Loan loan = ctx.getLoanByAccountId(accountId); - if (loan == null) { - return succeededFuture(ctx); - } - - String overdueFinePolicyId = loan.getOverdueFinePolicyId(); - if (!isUuid(overdueFinePolicyId)) { - log.info("Overdue fine policy ID {} is not a valid UUID - account {}", overdueFinePolicyId, - accountId); - return succeededFuture(ctx); - } - - return overdueFinePolicyRepository.getOverdueFinePolicyById(overdueFinePolicyId) - .onSuccess(policy -> ctx.updateAccountContextWithOverdueFinePolicy(accountId, policy)) - .map(ctx) - .otherwise(throwable -> { - log.error("Failed to find overdue fine policy for account {}, overdue fine policy is {}", - accountId, overdueFinePolicyId); - return ctx; - }); + private Future lookupLoanPoliciesForLoans( + FinancialTransactionsDetailReportContext context, Collection loans) { + + Map loanIdToPolicyId = loans.stream() + .collect(toMap(Loan::getId, Loan::getLoanPolicyId)); + + Set policyIds = loanIdToPolicyId.values() + .stream() + .filter(Objects::nonNull) + .collect(toSet()); + + return circulationStorageClient.getLoanPoliciesByIds(policyIds) + .onSuccess(policies -> { + Map policiesById = mapBy(policies, LoanPolicy::getId); + context.getActionsToAccounts() + .values() + .stream() + .filter(Objects::nonNull) + .forEach(account -> Optional.ofNullable(account.getLoanId()) + .map(loanIdToPolicyId::get) + .map(policiesById::get) + .ifPresent(policy -> context.updateAccountContextWithLoanPolicy(account.getId(), policy))); + }) + .map(context) + .onFailure(t -> log.error("Failed to fetch loan policies", t)) + .otherwise(context); } - public Future lookupLostItemFeePolicyForAccount( - T ctx, String accountId) { - - Loan loan = ctx.getLoanByAccountId(accountId); - if (loan == null) { - return succeededFuture(ctx); - } - - String lostItemFeePolicyId = loan.getLostItemPolicyId(); - if (!isUuid(lostItemFeePolicyId)) { - log.info("Lost item fee policy ID {} is not a valid UUID - account {}", lostItemFeePolicyId, - accountId); - return succeededFuture(ctx); - } + private Future lookupOverdueFinePoliciesForLoans( + FinancialTransactionsDetailReportContext context, Collection loans) { + + Map loanIdToPolicyId = loans.stream() + .collect(toMap(Loan::getId, Loan::getOverdueFinePolicyId)); + + Set policyIds = loanIdToPolicyId.values() + .stream() + .filter(Objects::nonNull) + .collect(toSet()); + + return overdueFinePolicyRepository.getOverdueFinePoliciesByIds(policyIds) + .onSuccess(policies -> { + Map policiesById = mapBy(policies, OverdueFinePolicy::getId); + context.getActionsToAccounts() + .values() + .stream() + .filter(Objects::nonNull) + .forEach(account -> Optional.ofNullable(account.getLoanId()) + .map(loanIdToPolicyId::get) + .map(policiesById::get) + .ifPresent(policy -> context.updateAccountContextWithOverdueFinePolicy(account.getId(), + policy))); + }) + .map(context) + .onFailure(t -> log.error("Failed to fetch overdue fine policies", t)) + .otherwise(context); + } - return lostItemFeePolicyRepository.getLostItemFeePolicyById(lostItemFeePolicyId) - .onSuccess(policy -> ctx.updateAccountContextWithLostItemFeePolicy(accountId, policy)) - .map(ctx) - .otherwise(throwable -> { - log.error("Failed to find lost item fee policy for account {}, lost item fee policy is {}", - accountId, lostItemFeePolicyId); - return ctx; - }); + private Future lookupLostItemFeePoliciesForLoans( + FinancialTransactionsDetailReportContext context, Collection loans) { + + Map loanIdToPolicyId = loans.stream() + .collect(toMap(Loan::getId, Loan::getLostItemPolicyId)); + + Set policyIds = loanIdToPolicyId.values() + .stream() + .filter(Objects::nonNull) + .collect(toSet()); + + return lostItemFeePolicyRepository.getLostItemFeePoliciesByIds(policyIds) + .onSuccess(lostItemFeePolicies -> { + Map policiesById = mapBy(lostItemFeePolicies, LostItemFeePolicy::getId); + context.getActionsToAccounts() + .values() + .stream() + .filter(Objects::nonNull) + .forEach(account -> Optional.ofNullable(account.getLoanId()) + .map(loanIdToPolicyId::get) + .map(policiesById::get) + .ifPresent(policy -> context.updateAccountContextWithLostItemFeePolicy(account.getId(), policy))); + }) + .map(context) + .onFailure(t -> log.error("Failed to fetch lost item fee policies", t)) + .otherwise(context); } private List sortFeeFineActionsByDate(List feeFineActions) { return feeFineActions.stream() .sorted(actionDateComparator()) - .collect(Collectors.toList()); + .collect(toList()); } - private Comparator actionDateComparator() { + private static Comparator actionDateComparator() { return (left, right) -> { if (left == null || right == null) { return 0; @@ -382,4 +505,9 @@ private Comparator actionDateComparator() { } }; } + + private static Map mapBy(Collection collection, Function keyMapper) { + return collection.stream() + .collect(toMap(keyMapper, identity())); + } } \ No newline at end of file diff --git a/src/main/resources/templates/db_scripts/schema.json b/src/main/resources/templates/db_scripts/schema.json index a76e0308..6301edd2 100644 --- a/src/main/resources/templates/db_scripts/schema.json +++ b/src/main/resources/templates/db_scripts/schema.json @@ -44,6 +44,10 @@ { "fieldName": "userId", "tOps": "ADD" + }, + { + "fieldName": "ownerId", + "tOps": "ADD" } ] }, diff --git a/src/test/java/org/folio/rest/client/OkapiClientTest.java b/src/test/java/org/folio/rest/client/OkapiClientTest.java index 76bdf76f..0dca38e8 100644 --- a/src/test/java/org/folio/rest/client/OkapiClientTest.java +++ b/src/test/java/org/folio/rest/client/OkapiClientTest.java @@ -94,7 +94,7 @@ public void getByIdShouldFailWhenRemoteCallFailed(TestContext context) { okapiClient.getById(USERS_URL, USER_ID, User.class) .onSuccess(r -> context.fail("should have failed")) .onFailure(failure -> { - String expectedErrorMessage = "Failed to fetch User " + USER_ID + ": [404] User not found"; + String expectedErrorMessage = "GET /users/" + USER_ID + ": [404] User not found"; context.assertEquals(expectedErrorMessage, failure.getMessage()); async.complete(); }); diff --git a/src/test/java/org/folio/rest/client/UsersClientTest.java b/src/test/java/org/folio/rest/client/UsersClientTest.java index 1d9334fe..313d49d6 100644 --- a/src/test/java/org/folio/rest/client/UsersClientTest.java +++ b/src/test/java/org/folio/rest/client/UsersClientTest.java @@ -79,12 +79,12 @@ public void shouldFailWhenUserIsNotFound(VertxTestContext context) { String responseBody = "User not found"; mockUsersResponse(HttpStatus.SC_NOT_FOUND, responseBody); - String expectedFailureMessage = "Failed to fetch User " + USER_ID + ": [404] User not found"; + String expectedFailureMessage = "GET /users/" + USER_ID + ": [404] User not found"; usersClient.fetchUserById(USER_ID) .onSuccess(user -> context.failNow("Should have failed")) .onFailure(throwable -> { - assertThat(expectedFailureMessage, is(throwable.getMessage())); + assertThat(throwable.getMessage(), is(expectedFailureMessage)); context.completeNow(); }); } diff --git a/src/test/java/org/folio/rest/impl/FeeFineActionsAPITest.java b/src/test/java/org/folio/rest/impl/FeeFineActionsAPITest.java index 6d5bfbea..038c5a77 100644 --- a/src/test/java/org/folio/rest/impl/FeeFineActionsAPITest.java +++ b/src/test/java/org/folio/rest/impl/FeeFineActionsAPITest.java @@ -221,10 +221,10 @@ public void noticeIsSentWhenFailedToFetchNonEssentialData() { verifyPublishedLogRecordsCount(NOTICE_ERROR, 1); String expectedErrorMessage = "Following errors may result in missing token values: " + - "\"Failed to fetch User " + user.getId() + ": [404] Not found\", " + - "\"Failed to fetch Item " + item.getId() + ": [404] Not found\", " + - "\"Failed to fetch HoldingsRecord " + holdingsRecord.getId() + ": [404] Not found\", " + - "\"Failed to fetch Instance " + instance.getId() + ": [404] Not found\", " + + "\"GET /users/" + user.getId() + ": [404] Not found\", " + + "\"GET /item-storage/items/" + item.getId() + ": [404] Not found\", " + + "\"GET /holdings-storage/holdings/" + holdingsRecord.getId() + ": [404] Not found\", " + + "\"GET /instance-storage/instances/" + instance.getId() + ": [404] Not found\", " + "\"Invalid Location ID: null\""; assertThatNoticeErrorEventWasPublished(charge, expectedErrorMessage); diff --git a/src/test/java/org/folio/rest/impl/FinancialTransactionDetailReportTest.java b/src/test/java/org/folio/rest/impl/FinancialTransactionDetailReportTest.java index 2ce0a87e..c4d23537 100644 --- a/src/test/java/org/folio/rest/impl/FinancialTransactionDetailReportTest.java +++ b/src/test/java/org/folio/rest/impl/FinancialTransactionDetailReportTest.java @@ -19,6 +19,7 @@ import static org.folio.test.support.matcher.constant.ServicePath.ACCOUNTS_PATH; import static org.folio.test.support.matcher.constant.ServicePath.HOLDINGS_PATH; import static org.folio.test.support.matcher.constant.ServicePath.INSTANCES_PATH; +import static org.folio.test.support.matcher.constant.ServicePath.LOANS_PATH; import static org.folio.test.support.matcher.constant.ServicePath.USERS_GROUPS_PATH; import static org.folio.test.support.matcher.constant.ServicePath.USERS_PATH; import static org.hamcrest.CoreMatchers.equalTo; @@ -124,6 +125,9 @@ public class FinancialTransactionDetailReportTest extends FeeFineReportsAPITestB private static final String ZERO_MONETARY = "0.00"; private static final String ZERO_COUNT = "0"; +// public static final String QUERY_BY_IDS_REGEX = "\\?query=id==\\(.+\\)&limit=\\d+"; + public static final String QUERY_BY_IDS_REGEX = "\\\\?.*"; + private final ReportResourceClient reportClient = buildFinancialTransactionsDetailReportClient(); private Location location; @@ -139,15 +143,14 @@ public class FinancialTransactionDetailReportTest extends FeeFineReportsAPITestB private Loan loan1; private Loan loan2; - private StubMapping userStubMapping1; - private StubMapping userGroupStubMapping1; + private StubMapping userStubMapping; + private StubMapping userGroupStubMapping; private StubMapping itemStubMapping; private StubMapping loanStubMapping; private StubMapping holdingsStubMapping; private StubMapping instanceStubMapping; private StubMapping locationStubMapping; - private StubMapping servicePoint1StubMapping; - private StubMapping servicePoint2StubMapping; + private StubMapping servicePointStubMapping; private StubMapping loanPolicyStubMapping; @BeforeEach @@ -156,59 +159,55 @@ public void setUp() { createLocaleSettingsStub(); loanPolicy = buildLoanPolicy("Loan policy"); - loanPolicyStubMapping = createStub(ServicePath.LOAN_POLICIES_PATH, loanPolicy, - loanPolicy.getId()); + loanPolicyStubMapping = createStubForCollection(ServicePath.LOAN_POLICIES_PATH, + List.of(loanPolicy), "loanPolicies"); + overdueFinePolicy = buildOverdueFinePolicy("ofp-1" + randomId()); createOverdueFinePolicy(overdueFinePolicy); lostItemFeePolicy = buildLostItemFeePolicy("lfp-1" + randomId()); createLostItemFeePolicy(lostItemFeePolicy); ServicePoint servicePoint1 = buildServicePoint(CREATED_AT_ID_1, CREATED_AT_1); - servicePoint1StubMapping = createStub(ServicePath.SERVICE_POINTS_PATH, servicePoint1, - servicePoint1.getId()); ServicePoint servicePoint2 = buildServicePoint(CREATED_AT_ID_2, CREATED_AT_2); - servicePoint2StubMapping = createStub(ServicePath.SERVICE_POINTS_PATH, servicePoint2, - servicePoint2.getId()); + servicePointStubMapping = createStubForCollection(ServicePath.SERVICE_POINTS_PATH, + List.of(servicePoint1, servicePoint2), "servicepoints"); location = buildLocation("Location"); - locationStubMapping = createStub(ServicePath.LOCATIONS_PATH, location, location.getId()); + locationStubMapping = createStubForCollection(ServicePath.LOCATIONS_PATH, List.of(location), + "locations"); instance = buildInstance(); + instanceStubMapping = createStubForCollection(INSTANCES_PATH, List.of(instance), "instances"); + holdingsRecord = buildHoldingsRecord(instance); + holdingsStubMapping = createStubForCollection(HOLDINGS_PATH, List.of(holdingsRecord), "holdingsRecords"); + item1 = buildItem(holdingsRecord, location); item2 = buildItem(holdingsRecord, location).withBarcode("item2-barcode"); + itemStubMapping = createStubForCollection(ServicePath.ITEMS_PATH, List.of(item1, item2), "items"); + loan1 = buildLoan(LOAN_DATE_1, DUE_DATE_1, RETURN_DATE_1, item1.getId(), loanPolicy.getId(), overdueFinePolicy.getId(), lostItemFeePolicy.getId()); - loanStubMapping = createStub(ServicePath.LOANS_PATH, loan1, loan1.getId()); loan2 = buildLoan(LOAN_DATE_2, DUE_DATE_2, RETURN_DATE_2, item2.getId(), loanPolicy.getId(), overdueFinePolicy.getId(), lostItemFeePolicy.getId()); - loanStubMapping = createStub(ServicePath.LOANS_PATH, loan2, loan2.getId()); - - itemStubMapping = createStub(ServicePath.ITEMS_PATH, item1, item1.getId()); - createStub(ServicePath.ITEMS_PATH, item2, item2.getId()); - holdingsStubMapping = createStub(HOLDINGS_PATH, holdingsRecord, holdingsRecord.getId()); - instanceStubMapping = createStub(INSTANCES_PATH, instance, instance.getId()); + loanStubMapping = createStubForCollection(ServicePath.LOANS_PATH, List.of(loan1, loan2), "loans"); UserGroup userGroup1 = EntityBuilder.buildUserGroup().withGroup(USER_GROUP_1); - userGroupStubMapping1 = createStub(USERS_GROUPS_PATH, userGroup1, userGroup1.getId()); + UserGroup userGroup2 = EntityBuilder.buildUserGroup().withGroup(USER_GROUP_2); + userGroupStubMapping = createStubForCollection(USERS_GROUPS_PATH, + List.of(userGroup1, userGroup2), "usergroups"); user1 = EntityBuilder.buildUser() .withId(USER_ID_1) .withPatronGroup(userGroup1.getId()) .withBarcode(USER_BARCODE_1); - userStubMapping1 = createStub(USERS_PATH, user1, USER_ID_1); - - UserGroup userGroup2 = EntityBuilder.buildUserGroup().withGroup(USER_GROUP_2); - createStub(USERS_GROUPS_PATH, userGroup2, userGroup2.getId()); user2 = EntityBuilder.buildUser() .withId(USER_ID_2) .withPatronGroup(userGroup2.getId()) .withBarcode(USER_BARCODE_2); - createStub(USERS_PATH, user2, USER_ID_2); - createStub(USERS_PATH, EntityBuilder.buildUser().withId(SOURCE_ID_1), SOURCE_ID_1); - createStub(USERS_PATH, EntityBuilder.buildUser().withId(SOURCE_ID_2), SOURCE_ID_2); + userStubMapping = createStubForCollection(USERS_PATH, List.of(user1,user2), "users"); } @Test @@ -555,7 +554,8 @@ void canHandleLoanDateInAnyValidFormat(String loanDate) { Loan loan = buildLoan(loanDate, DUE_DATE_1, RETURN_DATE_1, item1.getId(), loanPolicy.getId(), overdueFinePolicy.getId(), lostItemFeePolicy.getId()); - createStub(ServicePath.LOANS_PATH, loan, loan.getId()); + removeStub(loanStubMapping); + loanStubMapping = createStubForCollection(LOANS_PATH, List.of(loan), "loans"); Account account = charge(USER_ID_1, 10.0, FEE_FINE_TYPE_1, item1.getId(), loan, OWNER_ID_1, OWNER_1, withTenantTz("2020-01-02 02:00:00"), CREATED_AT_ID_1, SOURCE_1); @@ -573,22 +573,21 @@ void canHandleLoanDateInAnyValidFormat(String loanDate) { @Test public void validResponseWhenStubsAreMissing() { validResponseWithStubsMissing(() -> { - removeStub(userStubMapping1); - removeStub(userGroupStubMapping1); + removeStub(userStubMapping); + removeStub(userGroupStubMapping); removeStub(itemStubMapping); removeStub(loanStubMapping); removeStub(holdingsStubMapping); removeStub(instanceStubMapping); removeStub(locationStubMapping); - removeStub(servicePoint1StubMapping); - removeStub(servicePoint2StubMapping); + removeStub(servicePointStubMapping); }); } @Test public void validResponseWhenUserGroupStubIsMissing() { validResponseWithStubsMissing(() -> { - removeStub(userGroupStubMapping1); + removeStub(userGroupStubMapping); }); } @@ -700,4 +699,5 @@ private String getFullName(User user) { return format("%s, %s %s", user.getPersonal().getLastName(), user.getPersonal().getFirstName(), user.getPersonal().getMiddleName()); } + } diff --git a/src/test/java/org/folio/test/support/ApiTests.java b/src/test/java/org/folio/test/support/ApiTests.java index b804da4c..f38126a1 100644 --- a/src/test/java/org/folio/test/support/ApiTests.java +++ b/src/test/java/org/folio/test/support/ApiTests.java @@ -6,6 +6,8 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching; import static io.vertx.core.json.JsonObject.mapFrom; import static java.lang.String.format; +import static java.util.stream.Collectors.collectingAndThen; +import static java.util.stream.Collectors.toList; import static javax.ws.rs.core.HttpHeaders.ACCEPT; import static javax.ws.rs.core.MediaType.APPLICATION_JSON; import static org.folio.rest.RestVerticle.OKAPI_HEADER_TENANT; @@ -20,12 +22,14 @@ import java.text.SimpleDateFormat; import java.util.Base64; +import java.util.Collection; import java.util.Collections; import java.util.Map; import java.util.TimeZone; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import javax.ws.rs.core.MediaType; @@ -60,6 +64,7 @@ import io.restassured.specification.RequestSpecification; import io.vertx.core.DeploymentOptions; import io.vertx.core.Vertx; +import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; public class ApiTests { @@ -271,6 +276,20 @@ private StubMapping createStubForPathMatching(String regex, return createStub(urlPathMatching(regex), responseBuilder); } + public StubMapping createStubForCollection(String url, Collection returnObjects, + String collectionName) { + + JsonArray results = returnObjects.stream() + .map(JsonObject::mapFrom) + .collect(collectingAndThen(toList(), JsonArray::new)); + + JsonObject response = new JsonObject() + .put(collectionName, results) + .put("totalRecords", returnObjects.stream()); + + return createStubForPathMatching(url, aResponse().withBody(response.encodePrettily())); + } + private StubMapping createStub(UrlPathPattern urlPathPattern, ResponseDefinitionBuilder responseBuilder) {