From ec835d0c31da2e31d69c719e30455239b3706e7d Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Tue, 26 Nov 2024 13:15:30 +0500 Subject: [PATCH 01/13] [MODFISTO-501] - Implement endpoint to save FY finance data in bulk --- descriptors/ModuleDescriptor-template.json | 21 ++++- ramls/finance-data.raml | 34 +++++++- src/main/java/org/folio/dao/fund/FundDAO.java | 1 + .../org/folio/dao/fund/FundPostgresDAO.java | 83 +++++++++++++++++++ .../org/folio/rest/impl/FinanceDataApi.java | 19 +++++ .../financedata/FinanceDataService.java | 10 +++ .../org/folio/service/fund/FundService.java | 1 + .../service/fund/StorageFundService.java | 5 ++ 8 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/folio/service/financedata/FinanceDataService.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index e2f8e4d3..2ddbbf19 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -470,6 +470,11 @@ "methods": ["GET"], "pathPattern": "/finance-storage/finance-data", "permissionsRequired": ["finance-storage.finance-data.collection.get"] + }, + { + "methods": ["POST"], + "pathPattern": "/finance-storage/finance-data", + "permissionsRequired": ["finance-storage.finance-data.item.post"] } ] }, @@ -1021,6 +1026,20 @@ "displayName": "all finance-data for fiscal year", "description": "Get collection of finance data for particular fiscal year" }, + { + "permissionName": "finance-storage.finance-data.item.post", + "displayName": "create finance-data for fiscal year", + "description": "Create finance data for particular fiscal year" + }, + { + "permissionName": "finance-storage.finance-data.all", + "displayName": "All finance-data perms", + "description": "All permissions for the finance data", + "subPermissions": [ + "finance-storage.finance-data.collection.get", + "finance-storage.finance-data.item.post" + ] + }, { "permissionName" : "finance.module.all", "displayName" : "All finance-module perms", @@ -1036,7 +1055,7 @@ "finance-storage.transactions.all", "finance-storage.fund-types.all", "finance-storage.fund-update-logs.all", - "finance-storage.finance-data.collection.get" + "finance-storage.finance-data.all" ] } ], diff --git a/ramls/finance-data.raml b/ramls/finance-data.raml index bd605d0c..df05a8f2 100644 --- a/ramls/finance-data.raml +++ b/ramls/finance-data.raml @@ -15,7 +15,7 @@ types: pattern: ^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[1-5][0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$ traits: - pageable: !include raml-util/traits/pageable.raml + pageable: !include raml-util/traits/pageable.raml searchable: !include raml-util/traits/searchable.raml validate: !include raml-util/traits/validation.raml @@ -33,3 +33,35 @@ resourceTypes: searchable: { description: "with valid searchable fields: for example fiscalYearId", example: "[\"fiscalYearId\", \"7a4c4d30-3b63-4102-8e2d-3ee5792d7d02\", \"=\"]" }, pageable ] + put: + description: Update finance, budget as a bulk + is: [ validate ] + body: + application/json: + type: fy-finance-data-collection + example: !include acq-models/mod-finance/examples/fy_finance_data_collection.sample + responses: + 204: + description: "Items successfully updated" + 404: + description: "One or more items not found" + body: + text/plain: + example: | + "One or more <> not found" + 400: + description: "Bad request, e.g. malformed request body or query parameter. Details of the error (e.g. name of the parameter or line/character number with malformed data) provided in the response." + body: + text/plain: + example: | + "unable to update <> -- malformed JSON at 13:4" + 409: + description: "Optimistic locking version conflict" + body: + text/plain: + example: "version conflict" + 500: + description: "Internal server error, e.g. due to misconfiguration" + body: + text/plain: + example: "internal server error, contact administrator" diff --git a/src/main/java/org/folio/dao/fund/FundDAO.java b/src/main/java/org/folio/dao/fund/FundDAO.java index 9be94b36..1274f0a9 100644 --- a/src/main/java/org/folio/dao/fund/FundDAO.java +++ b/src/main/java/org/folio/dao/fund/FundDAO.java @@ -10,4 +10,5 @@ public interface FundDAO { Future getFundById(String id, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); + Future updateFundById(String fundId, Fund fund, DBConn conn); } diff --git a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java index b12390d6..8fe09245 100644 --- a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java +++ b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java @@ -1,19 +1,32 @@ package org.folio.dao.fund; +import static io.vertx.core.Future.succeededFuture; +import static org.folio.rest.impl.BudgetAPI.BUDGET_TABLE; +import static org.folio.rest.impl.FiscalYearAPI.FISCAL_YEAR_TABLE; import static org.folio.rest.impl.FundAPI.FUND_TABLE; +import static org.folio.rest.persist.HelperUtils.getFullTableName; +import static org.folio.rest.util.ResponseUtils.handleFailure; +import static org.folio.rest.util.ResponseUtils.handleNoContentResponse; +import io.vertx.core.Promise; +import io.vertx.sqlclient.Tuple; import java.util.Collections; import java.util.List; +import java.util.UUID; import org.folio.rest.exception.HttpException; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.rest.jaxrs.model.Fund; +import org.folio.rest.jaxrs.resource.FinanceStorageFunds; import org.folio.rest.persist.Criteria.Criterion; import io.vertx.core.Future; import org.folio.rest.persist.CriterionBuilder; +import org.folio.rest.persist.DBClient; import org.folio.rest.persist.DBConn; +import org.folio.rest.persist.HelperUtils; +import org.folio.rest.persist.PgUtil; import org.folio.rest.persist.interfaces.Results; import javax.ws.rs.core.Response; @@ -49,6 +62,76 @@ public Future> getFundsByIds(List ids, DBConn conn) { return getFundsByCriterion(criterionBuilder.build(), conn); } + @Override + public Future updateFundById(String fundId, Fund fund, DBConn conn) { + logger.debug("Trying to update finance storage fund by id {}", id); + fund.setId(id); + DBClient client = new DBClient(vertxContext, okapiHeaders); + return vertxContext.runOnContext(event -> + isFundStatusChanged(fund, client) + .onComplete(result -> { + if (result.failed()) { + HttpException cause = (HttpException) result.cause(); + logger.error("Failed to update the finance storage fund with Id {}", fund.getId(), cause); + HelperUtils.replyWithErrorResponse(asyncResultHandler, cause); + } else if (result.result() == null) { + logger.warn("Finance storage fund with id {} not found", id); + asyncResultHandler.handle(succeededFuture( + FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.respond404WithTextPlain("Not found"))); + } else if (Boolean.TRUE.equals(result.result())) { + handleFundStatusUpdate(fund, client).onComplete(asyncResultHandler); + } else { + PgUtil.put(FUND_TABLE, fund, id, okapiHeaders, vertxContext, + FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.class, asyncResultHandler); + } + }) + ); + } + + private Future handleFundStatusUpdate(Fund fund, DBClient client) { + return client.withTrans(conn -> + updateRelatedCurrentFYBudgets(fund, conn) + .compose(v -> updateFund(fund, conn)) + ) + .transform(result -> handleNoContentResponse(result, fund.getId(), "Fund {} {} updated")); + } + + private Future isFundStatusChanged(Fund fund, DBClient client) { + Promise promise = Promise.promise(); + client.getPgClient().getById(FUND_TABLE, fund.getId(), Fund.class, event -> { + if (event.failed()) { + handleFailure(promise, event); + } else { + if (event.result() != null) { + promise.complete(event.result().getFundStatus() != fund.getFundStatus()); + } else { + promise.complete(null); + } + } + }); + return promise.future(); + } + + private Future updateFund(Fund fund, DBConn conn) { + return conn.update(FUND_TABLE, fund, fund.getId()) + .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) + .mapEmpty(); + } + + private Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { + String fullBudgetTableName = getFullTableName(conn.getTenantId(), BUDGET_TABLE); + String fullFYTableName = getFullTableName(conn.getTenantId(), FISCAL_YEAR_TABLE); + + String sql = "UPDATE " + fullBudgetTableName + " SET jsonb = jsonb_set(jsonb,'{budgetStatus}', $1) " + + "WHERE((fundId=$2) " + + "AND (budget.fiscalYearId IN " + + "(SELECT id FROM " + fullFYTableName + " WHERE current_date between (jsonb->>'periodStart')::timestamp " + + "AND (jsonb->>'periodEnd')::timestamp)));"; + + return conn.execute(sql, Tuple.of(fund.getFundStatus().value(), UUID.fromString(fund.getId()))) + .mapEmpty(); + } + private Future> getFundsByCriterion(Criterion criterion, DBConn conn) { logger.debug("Trying to get funds by criterion = {}", criterion); return conn.get(FUND_TABLE, Fund.class, criterion, false) diff --git a/src/main/java/org/folio/rest/impl/FinanceDataApi.java b/src/main/java/org/folio/rest/impl/FinanceDataApi.java index 44d4ec39..5473c0c4 100644 --- a/src/main/java/org/folio/rest/impl/FinanceDataApi.java +++ b/src/main/java/org/folio/rest/impl/FinanceDataApi.java @@ -11,11 +11,14 @@ import org.folio.rest.jaxrs.model.FyFinanceDataCollection; import org.folio.rest.jaxrs.resource.FinanceStorageFinanceData; import org.folio.rest.persist.PgUtil; +import org.folio.service.financedata.FinanceDataService; public class FinanceDataApi implements FinanceStorageFinanceData { private static final String FINANCE_DATA_VIEW = "finance_data_view"; + private FinanceDataService financeDataService; + @Override public void getFinanceStorageFinanceData(String query, String totalRecords, int offset, int limit, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { @@ -23,4 +26,20 @@ public void getFinanceStorageFinanceData(String query, String totalRecords, int okapiHeaders, vertxContext, GetFinanceStorageFinanceDataResponse.class, asyncResultHandler); } + @Override + public void putFinanceStorageFinanceData(FyFinanceDataCollection entity, Map okapiHeaders, + Handler> asyncResultHandler, Context vertxContext) { + financeDataService.update(entity, vertxContext, okapiHeaders) + .onComplete(result -> { + if (result.failed()) { + asyncResultHandler.handle(io.vertx.core.Future.succeededFuture( + PutFinanceStorageFinanceDataResponse.respond500WithTextPlain(result.cause().getMessage()))); + return; + } + asyncResultHandler.handle(io.vertx.core.Future.succeededFuture( + PutFinanceStorageFinanceDataResponse.respond200WithApplicationJson(result.result()))); + }); + } + + } diff --git a/src/main/java/org/folio/service/financedata/FinanceDataService.java b/src/main/java/org/folio/service/financedata/FinanceDataService.java new file mode 100644 index 00000000..84cdbbeb --- /dev/null +++ b/src/main/java/org/folio/service/financedata/FinanceDataService.java @@ -0,0 +1,10 @@ +package org.folio.service.financedata; + +public class FinanceDataService { + + + public void update() { + + } + +} diff --git a/src/main/java/org/folio/service/fund/FundService.java b/src/main/java/org/folio/service/fund/FundService.java index a13c6aba..7a4afc6b 100644 --- a/src/main/java/org/folio/service/fund/FundService.java +++ b/src/main/java/org/folio/service/fund/FundService.java @@ -9,4 +9,5 @@ public interface FundService { Future getFundById(String fundId, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); + Future updateFundById(String fundId, Fund fund, DBConn conn); } diff --git a/src/main/java/org/folio/service/fund/StorageFundService.java b/src/main/java/org/folio/service/fund/StorageFundService.java index a8bd387b..bb60d2cf 100644 --- a/src/main/java/org/folio/service/fund/StorageFundService.java +++ b/src/main/java/org/folio/service/fund/StorageFundService.java @@ -24,4 +24,9 @@ public Future getFundById(String fundId, DBConn conn) { public Future> getFundsByIds(List ids, DBConn conn) { return fundDAO.getFundsByIds(ids, conn); } + + @Override + public Future updateFundById(String fundId, Fund fund, DBConn conn) { + return fundDAO.updateFundById(fundId, fund, conn); + } } From c258ad83276f95bf210370d51146e7121578b3fc Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 11:14:10 +0500 Subject: [PATCH 02/13] Implement endpoint to save FY finance data in bulk --- .../folio/config/ServicesConfiguration.java | 6 + .../java/org/folio/dao/budget/BudgetDAO.java | 2 +- .../folio/dao/budget/BudgetPostgresDAO.java | 6 +- src/main/java/org/folio/dao/fund/FundDAO.java | 2 + .../org/folio/dao/fund/FundPostgresDAO.java | 74 ++------ .../org/folio/rest/impl/FinanceDataApi.java | 20 +- .../java/org/folio/rest/impl/FundAPI.java | 99 +++------- .../folio/service/budget/BudgetService.java | 7 + .../financedata/FinanceDataService.java | 93 ++++++++- .../org/folio/service/fund/FundService.java | 3 +- .../service/fund/StorageFundService.java | 19 +- .../folio/rest/impl/FinanceDataApiTest.java | 50 +++++ .../java/org/folio/rest/impl/TestBase.java | 8 + .../fianancedata/FinanceDataServiceTest.java | 177 ++++++++++++++++++ 14 files changed, 410 insertions(+), 156 deletions(-) create mode 100644 src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java diff --git a/src/main/java/org/folio/config/ServicesConfiguration.java b/src/main/java/org/folio/config/ServicesConfiguration.java index a6943119..5975ff43 100644 --- a/src/main/java/org/folio/config/ServicesConfiguration.java +++ b/src/main/java/org/folio/config/ServicesConfiguration.java @@ -21,6 +21,7 @@ import org.folio.service.budget.BudgetService; import org.folio.service.budget.RolloverBudgetExpenseClassTotalsService; import org.folio.service.email.EmailService; +import org.folio.service.financedata.FinanceDataService; import org.folio.service.fiscalyear.FiscalYearService; import org.folio.service.fund.FundService; import org.folio.service.fund.StorageFundService; @@ -164,4 +165,9 @@ RolloverBudgetExpenseClassTotalsService rolloverBudgetExpenseClassTotalsService( TemporaryEncumbranceService temporaryEncumbranceService) { return new RolloverBudgetExpenseClassTotalsService(budgetExpenseClassService, temporaryEncumbranceService); } + + @Bean + public FinanceDataService financeDataService(FundService fundService, BudgetService budgetService) { + return new FinanceDataService(fundService, budgetService); + } } diff --git a/src/main/java/org/folio/dao/budget/BudgetDAO.java b/src/main/java/org/folio/dao/budget/BudgetDAO.java index 6a0686d0..045cd7c6 100644 --- a/src/main/java/org/folio/dao/budget/BudgetDAO.java +++ b/src/main/java/org/folio/dao/budget/BudgetDAO.java @@ -17,7 +17,7 @@ public interface BudgetDAO { Future> getBudgetsBySql(String sql, Tuple params, DBConn conn); - Future> getBudgets(Criterion criterion, DBConn conn); + Future> getBudgetsByCriterion(Criterion criterion, DBConn conn); Future getBudgetById(String id, DBConn conn); diff --git a/src/main/java/org/folio/dao/budget/BudgetPostgresDAO.java b/src/main/java/org/folio/dao/budget/BudgetPostgresDAO.java index 8fd8dc93..f44f3a98 100644 --- a/src/main/java/org/folio/dao/budget/BudgetPostgresDAO.java +++ b/src/main/java/org/folio/dao/budget/BudgetPostgresDAO.java @@ -32,7 +32,7 @@ public Future updateBatchBudgets(List budgets, DBConn conn) { List ids = budgets.stream().map(Budget::getId).toList(); logger.debug("Trying update batch budgets, ids={}", ids); return conn.updateBatch(BUDGET_TABLE, budgets) - .onSuccess(rowSet -> logger.info("Updated {} batch budgets", budgets.size())) + .onSuccess(rowSet -> logger.info("updateBatchBudgets:: Updated {} batch budgets", budgets.size())) .onFailure(e -> logger.error("Update batch budgets by failed, ids={}", ids, e)) .mapEmpty(); } @@ -42,7 +42,7 @@ public Future updateBatchBudgetsBySql(String sql, DBConn conn) { logger.debug("Trying update batch budgets by query: {}", sql); return conn.execute(sql) .map(SqlResult::rowCount) - .onSuccess(rowCount -> logger.info("Updated {} batch budgets", rowCount)) + .onSuccess(rowCount -> logger.info("updateBatchBudgetsBySql:: Updated {} batch budgets", rowCount)) .onFailure(e -> logger.error("Update batch budgets by query: {} failed", sql, e)); } @@ -67,7 +67,7 @@ public Future> getBudgetsBySql(String sql, Tuple params, DBConn con * @param conn : db connection */ @Override - public Future> getBudgets(Criterion criterion, DBConn conn) { + public Future> getBudgetsByCriterion(Criterion criterion, DBConn conn) { logger.debug("Trying to get budgets by query: {}", criterion); return conn.get(BUDGET_TABLE, Budget.class, criterion, false) .map(results -> { diff --git a/src/main/java/org/folio/dao/fund/FundDAO.java b/src/main/java/org/folio/dao/fund/FundDAO.java index 1274f0a9..d8952920 100644 --- a/src/main/java/org/folio/dao/fund/FundDAO.java +++ b/src/main/java/org/folio/dao/fund/FundDAO.java @@ -11,4 +11,6 @@ public interface FundDAO { Future getFundById(String id, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); Future updateFundById(String fundId, Fund fund, DBConn conn); + Future isFundStatusChanged(Fund fund, DBConn conn); + Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn); } diff --git a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java index 8fe09245..49d43e53 100644 --- a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java +++ b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java @@ -1,14 +1,10 @@ package org.folio.dao.fund; -import static io.vertx.core.Future.succeededFuture; import static org.folio.rest.impl.BudgetAPI.BUDGET_TABLE; import static org.folio.rest.impl.FiscalYearAPI.FISCAL_YEAR_TABLE; import static org.folio.rest.impl.FundAPI.FUND_TABLE; import static org.folio.rest.persist.HelperUtils.getFullTableName; -import static org.folio.rest.util.ResponseUtils.handleFailure; -import static org.folio.rest.util.ResponseUtils.handleNoContentResponse; -import io.vertx.core.Promise; import io.vertx.sqlclient.Tuple; import java.util.Collections; import java.util.List; @@ -18,15 +14,11 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.rest.jaxrs.model.Fund; -import org.folio.rest.jaxrs.resource.FinanceStorageFunds; import org.folio.rest.persist.Criteria.Criterion; import io.vertx.core.Future; import org.folio.rest.persist.CriterionBuilder; -import org.folio.rest.persist.DBClient; import org.folio.rest.persist.DBConn; -import org.folio.rest.persist.HelperUtils; -import org.folio.rest.persist.PgUtil; import org.folio.rest.persist.interfaces.Results; import javax.ws.rs.core.Response; @@ -64,61 +56,19 @@ public Future> getFundsByIds(List ids, DBConn conn) { @Override public Future updateFundById(String fundId, Fund fund, DBConn conn) { - logger.debug("Trying to update finance storage fund by id {}", id); - fund.setId(id); - DBClient client = new DBClient(vertxContext, okapiHeaders); - return vertxContext.runOnContext(event -> - isFundStatusChanged(fund, client) - .onComplete(result -> { - if (result.failed()) { - HttpException cause = (HttpException) result.cause(); - logger.error("Failed to update the finance storage fund with Id {}", fund.getId(), cause); - HelperUtils.replyWithErrorResponse(asyncResultHandler, cause); - } else if (result.result() == null) { - logger.warn("Finance storage fund with id {} not found", id); - asyncResultHandler.handle(succeededFuture( - FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.respond404WithTextPlain("Not found"))); - } else if (Boolean.TRUE.equals(result.result())) { - handleFundStatusUpdate(fund, client).onComplete(asyncResultHandler); - } else { - PgUtil.put(FUND_TABLE, fund, id, okapiHeaders, vertxContext, - FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.class, asyncResultHandler); - } - }) - ); + logger.debug("Trying to update finance storage fund by id {}", fundId); + fund.setId(fundId); + return updateFund(fund, conn); } - private Future handleFundStatusUpdate(Fund fund, DBClient client) { - return client.withTrans(conn -> - updateRelatedCurrentFYBudgets(fund, conn) - .compose(v -> updateFund(fund, conn)) - ) - .transform(result -> handleNoContentResponse(result, fund.getId(), "Fund {} {} updated")); - } - - private Future isFundStatusChanged(Fund fund, DBClient client) { - Promise promise = Promise.promise(); - client.getPgClient().getById(FUND_TABLE, fund.getId(), Fund.class, event -> { - if (event.failed()) { - handleFailure(promise, event); - } else { - if (event.result() != null) { - promise.complete(event.result().getFundStatus() != fund.getFundStatus()); - } else { - promise.complete(null); - } - } - }); - return promise.future(); - } - - private Future updateFund(Fund fund, DBConn conn) { - return conn.update(FUND_TABLE, fund, fund.getId()) - .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) - .mapEmpty(); + @Override + public Future isFundStatusChanged(Fund fund, DBConn conn) { + return getFundById(fund.getId(), conn) + .map(existingFund -> existingFund.getFundStatus() != fund.getFundStatus()); } - private Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { + @Override + public Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { String fullBudgetTableName = getFullTableName(conn.getTenantId(), BUDGET_TABLE); String fullFYTableName = getFullTableName(conn.getTenantId(), FISCAL_YEAR_TABLE); @@ -132,6 +82,12 @@ private Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { .mapEmpty(); } + private Future updateFund(Fund fund, DBConn conn) { + return conn.update(FUND_TABLE, fund, fund.getId()) + .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) + .mapEmpty(); + } + private Future> getFundsByCriterion(Criterion criterion, DBConn conn) { logger.debug("Trying to get funds by criterion = {}", criterion); return conn.get(FUND_TABLE, Fund.class, criterion, false) diff --git a/src/main/java/org/folio/rest/impl/FinanceDataApi.java b/src/main/java/org/folio/rest/impl/FinanceDataApi.java index 5473c0c4..e3706941 100644 --- a/src/main/java/org/folio/rest/impl/FinanceDataApi.java +++ b/src/main/java/org/folio/rest/impl/FinanceDataApi.java @@ -1,16 +1,21 @@ package org.folio.rest.impl; +import static io.vertx.core.Future.succeededFuture; +import static org.folio.rest.jaxrs.resource.FinanceStorageFinanceData.PutFinanceStorageFinanceDataResponse.respond204; + import javax.ws.rs.core.Response; import java.util.Map; import io.vertx.core.AsyncResult; import io.vertx.core.Context; import io.vertx.core.Handler; +import org.folio.rest.core.model.RequestContext; import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.FyFinanceDataCollection; import org.folio.rest.jaxrs.resource.FinanceStorageFinanceData; import org.folio.rest.persist.PgUtil; +import org.folio.rest.util.ResponseUtils; import org.folio.service.financedata.FinanceDataService; public class FinanceDataApi implements FinanceStorageFinanceData { @@ -29,17 +34,8 @@ public void getFinanceStorageFinanceData(String query, String totalRecords, int @Override public void putFinanceStorageFinanceData(FyFinanceDataCollection entity, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { - financeDataService.update(entity, vertxContext, okapiHeaders) - .onComplete(result -> { - if (result.failed()) { - asyncResultHandler.handle(io.vertx.core.Future.succeededFuture( - PutFinanceStorageFinanceDataResponse.respond500WithTextPlain(result.cause().getMessage()))); - return; - } - asyncResultHandler.handle(io.vertx.core.Future.succeededFuture( - PutFinanceStorageFinanceDataResponse.respond200WithApplicationJson(result.result()))); - }); + financeDataService.update(entity, new RequestContext(vertxContext, okapiHeaders)) + .onSuccess(v -> asyncResultHandler.handle(succeededFuture(respond204()))) + .onFailure(ResponseUtils::handleFailure); } - - } diff --git a/src/main/java/org/folio/rest/impl/FundAPI.java b/src/main/java/org/folio/rest/impl/FundAPI.java index 1ec16596..9ff35361 100644 --- a/src/main/java/org/folio/rest/impl/FundAPI.java +++ b/src/main/java/org/folio/rest/impl/FundAPI.java @@ -1,34 +1,25 @@ package org.folio.rest.impl; import static io.vertx.core.Future.succeededFuture; -import static org.folio.rest.impl.BudgetAPI.BUDGET_TABLE; -import static org.folio.rest.impl.FiscalYearAPI.FISCAL_YEAR_TABLE; -import static org.folio.rest.persist.HelperUtils.getFullTableName; -import static org.folio.rest.util.ResponseUtils.handleFailure; -import static org.folio.rest.util.ResponseUtils.handleNoContentResponse; - -import java.util.Map; -import java.util.UUID; import javax.ws.rs.core.Response; +import java.util.Map; -import org.folio.rest.exception.HttpException; +import io.vertx.core.AsyncResult; +import io.vertx.core.Context; +import io.vertx.core.Handler; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.folio.rest.annotations.Validate; +import org.folio.rest.exception.HttpException; import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.jaxrs.model.FundCollection; import org.folio.rest.jaxrs.resource.FinanceStorageFunds; import org.folio.rest.persist.DBClient; -import org.folio.rest.persist.DBConn; import org.folio.rest.persist.HelperUtils; import org.folio.rest.persist.PgUtil; -import io.vertx.core.AsyncResult; -import io.vertx.core.Context; -import io.vertx.core.Future; -import io.vertx.core.Handler; -import io.vertx.core.Promise; -import org.apache.logging.log4j.Logger; -import io.vertx.sqlclient.Tuple; +import org.folio.service.fund.FundService; +import org.springframework.beans.factory.annotation.Autowired; public class FundAPI implements FinanceStorageFunds { @@ -36,6 +27,9 @@ public class FundAPI implements FinanceStorageFunds { public static final String FUND_TABLE = "fund"; + @Autowired + private FundService fundService; + @Override @Validate public void getFinanceStorageFunds(String query, String totalRecords, int offset, int limit, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { @@ -66,67 +60,18 @@ public void deleteFinanceStorageFundsById(String id, Map okapiHe public void putFinanceStorageFundsById(String id, Fund fund, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { logger.debug("Trying to update finance storage fund by id {}", id); fund.setId(id); - DBClient client = new DBClient(vertxContext, okapiHeaders); - vertxContext.runOnContext(event -> - isFundStatusChanged(fund, client) - .onComplete(result -> { - if (result.failed()) { - HttpException cause = (HttpException) result.cause(); - logger.error("Failed to update the finance storage fund with Id {}", fund.getId(), cause); - HelperUtils.replyWithErrorResponse(asyncResultHandler, cause); - } else if (result.result() == null) { - logger.warn("Finance storage fund with id {} not found", id); - asyncResultHandler.handle(succeededFuture(FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.respond404WithTextPlain("Not found"))); - } else if (Boolean.TRUE.equals(result.result())) { - handleFundStatusUpdate(fund, client).onComplete(asyncResultHandler); - } else { - PgUtil.put(FUND_TABLE, fund, id, okapiHeaders, vertxContext, FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.class, asyncResultHandler); - } - }) - ); - } - - private Future handleFundStatusUpdate(Fund fund, DBClient client) { - return client.withTrans(conn -> - updateRelatedCurrentFYBudgets(fund, conn) - .compose(v -> updateFund(fund, conn)) - ) - .transform(result -> handleNoContentResponse(result, fund.getId(), "Fund {} {} updated")); - } - - private Future isFundStatusChanged(Fund fund, DBClient client) { - Promise promise = Promise.promise(); - client.getPgClient().getById(FUND_TABLE, fund.getId(), Fund.class, event -> { - if (event.failed()) { - handleFailure(promise, event); - } else { - if (event.result() != null) { - promise.complete(event.result().getFundStatus() != fund.getFundStatus()); + var dbClient = new DBClient(vertxContext, okapiHeaders); + dbClient.withTrans(conn -> fundService.updateFund(fund, conn) + .onComplete(result -> { + if (result.failed()) { + HttpException cause = (HttpException) result.cause(); + logger.error("Failed to update the finance storage fund with Id {}", fund.getId(), cause); + HelperUtils.replyWithErrorResponse(asyncResultHandler, cause); } else { - promise.complete(null); + asyncResultHandler.handle(succeededFuture( + FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.respond204())); } - } - }); - return promise.future(); - } - - private Future updateFund(Fund fund, DBConn conn) { - return conn.update(FUND_TABLE, fund, fund.getId()) - .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) - .mapEmpty(); - } - - private Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { - String fullBudgetTableName = getFullTableName(conn.getTenantId(), BUDGET_TABLE); - String fullFYTableName = getFullTableName(conn.getTenantId(), FISCAL_YEAR_TABLE); - - String sql = "UPDATE "+ fullBudgetTableName +" SET jsonb = jsonb_set(jsonb,'{budgetStatus}', $1) " + - "WHERE((fundId=$2) " + - "AND (budget.fiscalYearId IN " + - "(SELECT id FROM " + fullFYTableName + " WHERE current_date between (jsonb->>'periodStart')::timestamp " + - "AND (jsonb->>'periodEnd')::timestamp)));"; - - return conn.execute(sql, Tuple.of(fund.getFundStatus().value(), UUID.fromString(fund.getId()))) - .mapEmpty(); + }) + ); } } diff --git a/src/main/java/org/folio/service/budget/BudgetService.java b/src/main/java/org/folio/service/budget/BudgetService.java index fb5da7bb..5e58c0d6 100644 --- a/src/main/java/org/folio/service/budget/BudgetService.java +++ b/src/main/java/org/folio/service/budget/BudgetService.java @@ -16,6 +16,7 @@ import org.folio.dao.budget.BudgetDAO; import org.folio.rest.jaxrs.model.Budget; import org.folio.rest.jaxrs.model.LedgerFiscalYearRollover; +import org.folio.rest.persist.CriterionBuilder; import org.folio.rest.persist.DBClient; import org.folio.rest.persist.DBConn; import org.folio.utils.CalculationUtils; @@ -64,6 +65,12 @@ public Future> getBudgets(String sql, Tuple params, DBConn conn) { return budgetDAO.getBudgetsBySql(sql, params, conn); } + public Future> getBudgetsByIds(List ids, DBConn conn) { + CriterionBuilder criterionBuilder = new CriterionBuilder("OR"); + ids.forEach(id -> criterionBuilder.with("id", id)); + return budgetDAO.getBudgetsByCriterion(criterionBuilder.build(), conn); + } + public void clearReadOnlyFields(Budget budgetFromNew) { budgetFromNew.setAllocated(null); budgetFromNew.setAvailable(null); diff --git a/src/main/java/org/folio/service/financedata/FinanceDataService.java b/src/main/java/org/folio/service/financedata/FinanceDataService.java index 84cdbbeb..632a0070 100644 --- a/src/main/java/org/folio/service/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/service/financedata/FinanceDataService.java @@ -1,10 +1,101 @@ package org.folio.service.financedata; +import static org.apache.commons.collections4.CollectionUtils.isNotEmpty; + +import java.util.List; + +import io.vertx.core.Future; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.okapi.common.GenericCompositeFuture; +import org.folio.rest.core.model.RequestContext; +import org.folio.rest.jaxrs.model.Budget; +import org.folio.rest.jaxrs.model.Fund; +import org.folio.rest.jaxrs.model.FyFinanceData; +import org.folio.rest.jaxrs.model.FyFinanceDataCollection; +import org.folio.rest.jaxrs.model.Tags; +import org.folio.rest.persist.DBConn; +import org.folio.service.budget.BudgetService; +import org.folio.service.fund.FundService; + public class FinanceDataService { + private static final Logger logger = LogManager.getLogger(); + + private final FundService fundService; + private final BudgetService budgetService; + public FinanceDataService(FundService fundService, BudgetService budgetService) { + this.fundService = fundService; + this.budgetService = budgetService; + } + + public Future update(FyFinanceDataCollection entity, RequestContext requestContext) { + var dbClient = requestContext.toDBClient(); + return dbClient + .withTrans(conn -> { + var updateFundFuture = processFundUpdate(entity, conn); + var updateBudgetFuture = processBudgetUpdate(entity, conn); + return GenericCompositeFuture.all(List.of(updateFundFuture, updateBudgetFuture)); + }) + .onSuccess(v -> logger.info("Successfully updated finance data")) + .onFailure(e -> logger.error("Failed to update finance data", e)) + .mapEmpty(); + } - public void update() { + private Future processFundUpdate(FyFinanceDataCollection entity, DBConn conn) { + List fundIds = entity.getFyFinanceData().stream() + .map(FyFinanceData::getFundId) + .toList(); + return fundService.getFundsByIds(fundIds, conn) + .compose(funds -> { + var futures = funds.stream() + .map(fund -> setNewValues(fund, entity)) + .map(fund -> fundService.updateFundWithMinChange(fund, conn)) + .toList(); + return GenericCompositeFuture.all(futures).mapEmpty(); + }); + } + private Future processBudgetUpdate(FyFinanceDataCollection entity, DBConn conn) { + List budgetIds = entity.getFyFinanceData().stream() + .map(FyFinanceData::getBudgetId) + .toList(); + return budgetService.getBudgetsByIds(budgetIds, conn) + .map(budgets -> setNewValues(budgets, entity)) + .compose(budgets -> budgetService.updateBatchBudgets(budgets, conn)); } + private Fund setNewValues(Fund fund, FyFinanceDataCollection entity) { + var fundFinanceData = entity.getFyFinanceData().stream() + .filter(data -> data.getFundId().equals(fund.getId())) + .findFirst() + .orElseThrow(); + + fund.setDescription(fundFinanceData.getFundDescription()); + if (fundFinanceData.getFundTags() != null && isNotEmpty(fundFinanceData.getFundTags().getTagList())) { + fund.setTags(new Tags().withTagList(fundFinanceData.getFundTags().getTagList())); + } + return fund; + } + + private List setNewValues(List budgets, FyFinanceDataCollection entity) { + return budgets.stream() + .map(budget -> setNewValues(budget, entity)) + .toList(); + } + + private Budget setNewValues(Budget budget, FyFinanceDataCollection entity) { + var budgetFinanceData = entity.getFyFinanceData().stream() + .filter(data -> data.getFundId().equals(budget.getId())) + .findFirst() + .orElseThrow(); + + return budget.withName(budgetFinanceData.getBudgetName()) + .withBudgetStatus(Budget.BudgetStatus.fromValue(budgetFinanceData.getBudgetStatus().value())) + .withInitialAllocation(budgetFinanceData.getBudgetInitialAllocation()) + .withAllocated(budgetFinanceData.getBudgetCurrentAllocation()) + .withAllowableEncumbrance(budgetFinanceData.getBudgetAllowableExpenditure()) + .withAllowableEncumbrance(budgetFinanceData.getBudgetAllowableEncumbrance()) + .withAcqUnitIds(budgetFinanceData.getBudgetAcqUnitIds()); + } } diff --git a/src/main/java/org/folio/service/fund/FundService.java b/src/main/java/org/folio/service/fund/FundService.java index 7a4afc6b..36fa28d3 100644 --- a/src/main/java/org/folio/service/fund/FundService.java +++ b/src/main/java/org/folio/service/fund/FundService.java @@ -9,5 +9,6 @@ public interface FundService { Future getFundById(String fundId, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); - Future updateFundById(String fundId, Fund fund, DBConn conn); + Future updateFund(Fund fund, DBConn conn); + Future updateFundWithMinChange(Fund fund, DBConn conn); } diff --git a/src/main/java/org/folio/service/fund/StorageFundService.java b/src/main/java/org/folio/service/fund/StorageFundService.java index bb60d2cf..ab8c9f66 100644 --- a/src/main/java/org/folio/service/fund/StorageFundService.java +++ b/src/main/java/org/folio/service/fund/StorageFundService.java @@ -1,6 +1,8 @@ package org.folio.service.fund; import io.vertx.core.Future; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.folio.dao.fund.FundDAO; import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.persist.DBConn; @@ -8,6 +10,7 @@ import java.util.List; public class StorageFundService implements FundService { + private static final Logger logger = LogManager.getLogger(); private final FundDAO fundDAO; @@ -26,7 +29,19 @@ public Future> getFundsByIds(List ids, DBConn conn) { } @Override - public Future updateFundById(String fundId, Fund fund, DBConn conn) { - return fundDAO.updateFundById(fundId, fund, conn); + public Future updateFund(Fund fund, DBConn conn) { + return fundDAO.isFundStatusChanged(fund, conn) + .compose(statusChanged -> { + if (Boolean.TRUE.equals(statusChanged)) { + return fundDAO.updateRelatedCurrentFYBudgets(fund, conn) + .compose(v -> fundDAO.updateFundById(fund.getId(), fund, conn)); + } + return updateFundWithMinChange(fund, conn); + }); + } + + @Override + public Future updateFundWithMinChange(Fund fund, DBConn conn) { + return fundDAO.updateFundById(fund.getId(), fund, conn); } } diff --git a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java index a57733a6..157f382d 100644 --- a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java +++ b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java @@ -10,6 +10,7 @@ import static org.folio.rest.utils.TestEntities.LEDGER; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.Mockito.mock; import java.util.List; import java.util.UUID; @@ -26,9 +27,13 @@ import org.folio.rest.jaxrs.model.TenantJob; import org.folio.rest.jaxrs.resource.FinanceStorageFinanceData; import org.folio.rest.persist.HelperUtils; +import org.folio.service.budget.BudgetService; +import org.folio.service.financedata.FinanceDataService; +import org.folio.service.fund.FundService; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.springframework.context.annotation.Bean; public class FinanceDataApiTest extends TestBase { @@ -38,6 +43,7 @@ public class FinanceDataApiTest extends TestBase { private static final String FINANCE_DATA_ENDPOINT = HelperUtils.getEndpoint(FinanceStorageFinanceData.class); private static TenantJob tenantJob; + @BeforeAll public static void before() { logger.info("Create a new tenant loading the sample data"); @@ -93,6 +99,32 @@ public void positive_testResponseOfGetWithParamsFiscalYearAndAcqUnitIds() { assertEquals(acqUnitId, actualFyFinanceData.getBudgetAcqUnitIds().get(0)); } + @Test + public void positive_testUpdateFinanceData() { + var fiscalYearId = UUID.randomUUID().toString(); + var acqUnitId = UUID.randomUUID().toString(); + createMockData(fiscalYearId, acqUnitId); + + var response = getData(FINANCE_DATA_ENDPOINT + "?query=(fiscalYearId==" + fiscalYearId + ")", TENANT_HEADER); + var body = response.getBody().as(FyFinanceDataCollection.class); + var fyFinanceData = body.getFyFinanceData().get(0); + + fyFinanceData.setFundName("Updated Fund Name"); + fyFinanceData.setBudgetName("Updated Budget Name"); + + var updatedCollection = new FyFinanceDataCollection().withFyFinanceData(List.of(fyFinanceData)).withTotalRecords(1); + + var updateResponse = putData(FINANCE_DATA_ENDPOINT, JsonObject.mapFrom(updatedCollection).encodePrettily(), TENANT_HEADER); + assertEquals(204, updateResponse.getStatusCode()); + + var updatedResponse = getData(FINANCE_DATA_ENDPOINT + "?query=(fiscalYearId==" + fiscalYearId + ")", TENANT_HEADER); + var updatedBody = updatedResponse.getBody().as(FyFinanceDataCollection.class); + var updatedFyFinanceData = updatedBody.getFyFinanceData().get(0); + + assertEquals("Updated Fund Name", updatedFyFinanceData.getFundName()); + assertEquals("Updated Budget Name", updatedFyFinanceData.getBudgetName()); + } + private void createMockData(String fiscalYearId, String acqUnitId) { var fundId = UUID.randomUUID().toString(); var ledgerId = UUID.randomUUID().toString(); @@ -116,4 +148,22 @@ private void createMockData(String fiscalYearId, String acqUnitId) { .withAcqUnitIds(List.of(acqUnitId)); createEntity(BUDGET.getEndpoint(), budget, TENANT_HEADER); } + + static class ContextConfiguration { + + @Bean + public FinanceDataService financeDataService(FundService fundService, BudgetService budgetService) { + return mock(FinanceDataService.class); + } + + @Bean + FundService fundService() { + return mock(FundService.class); + } + + @Bean + BudgetService budgetService() { + return mock(BudgetService.class); + } + } } diff --git a/src/test/java/org/folio/rest/impl/TestBase.java b/src/test/java/org/folio/rest/impl/TestBase.java index 02a1bd43..00f023b9 100644 --- a/src/test/java/org/folio/rest/impl/TestBase.java +++ b/src/test/java/org/folio/rest/impl/TestBase.java @@ -146,6 +146,14 @@ Response getDataById(String endpoint, String id, Header header) { .get(storageUrl(endpoint)); } + Response putData(String endpoint, String input, Header tenant) { + return given() + .header(tenant) + .contentType(ContentType.JSON) + .body(input) + .put(storageUrl(endpoint)); + } + Response putData(String endpoint, String id, String input, Header tenant) { return given() .pathParam("id", id) diff --git a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java new file mode 100644 index 00000000..3ae93549 --- /dev/null +++ b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java @@ -0,0 +1,177 @@ +package org.folio.service.fianancedata; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.UUID; +import java.util.function.Function; + +import io.vertx.core.Future; +import io.vertx.core.Vertx; +import io.vertx.junit5.VertxExtension; +import io.vertx.junit5.VertxTestContext; +import org.folio.rest.core.model.RequestContext; +import org.folio.rest.jaxrs.model.Budget; +import org.folio.rest.jaxrs.model.Fund; +import org.folio.rest.jaxrs.model.FyFinanceData; +import org.folio.rest.jaxrs.model.FyFinanceDataCollection; +import org.folio.rest.persist.DBClient; +import org.folio.rest.persist.DBConn; +import org.folio.service.budget.BudgetService; +import org.folio.service.financedata.FinanceDataService; +import org.folio.service.fund.FundService; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; + +@ExtendWith(VertxExtension.class) +public class FinanceDataServiceTest { + + @Mock + private FundService fundService; + @Mock + private BudgetService budgetService; + @Mock + private RequestContext requestContext; + @Mock + private DBClient dbClient; + @Mock + private DBConn dbConn; + + @InjectMocks + private FinanceDataService financeDataService2; + + private AutoCloseable mockitoMocks; + + @BeforeEach + void setUp(Vertx vertx) { + mockitoMocks = MockitoAnnotations.openMocks(this); + } + + @AfterEach + void tearDown() throws Exception { + mockitoMocks.close(); + } + + @Test + void shouldSuccessfullyUpdateFinanceData(VertxTestContext testContext) { + FinanceDataService financeDataService = Mockito.spy(financeDataService2); + // given + FyFinanceDataCollection collection = createTestFinanceDataCollection(); + Fund fund = new Fund() + .withId(collection.getFyFinanceData().get(0).getFundId()) + .withCode("OLD-CODE") + .withName("Old Name"); + Budget budget = new Budget() + .withId(collection.getFyFinanceData().get(0).getBudgetId()) + .withName("Old Budget Name"); + + when(requestContext.toDBClient()).thenReturn(dbClient); + doAnswer(invocation -> { + Function> function = invocation.getArgument(0); + return function.apply(dbConn); + }).when(dbClient).withTrans(any()); + + when(fundService.getFundsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(fund))); + when(fundService.updateFund(any(), any())).thenReturn(Future.succeededFuture()); + when(budgetService.getBudgetsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(budget))); + when(budgetService.updateBatchBudgets(any(), any())).thenReturn(Future.succeededFuture()); + + // when & then + testContext.assertComplete(financeDataService.update(collection, requestContext) + .onComplete(testContext.succeeding(result -> { + testContext.verify(() -> { + // Verify transaction handling + verify(dbClient).withTrans(any()); + + // Verify fund updates + ArgumentCaptor> fundIdsCaptor = ArgumentCaptor.forClass(List.class); + verify(fundService).getFundsByIds(fundIdsCaptor.capture(), eq(dbConn)); + assertEquals(collection.getFyFinanceData().get(0).getFundId(), fundIdsCaptor.getValue().get(0)); + + ArgumentCaptor fundCaptor = ArgumentCaptor.forClass(Fund.class); + verify(fundService).updateFund(fundCaptor.capture(), eq(dbConn)); + Fund updatedFund = fundCaptor.getValue(); + assertEquals("NEW-CODE", updatedFund.getCode()); + assertEquals("New Fund Name", updatedFund.getName()); + assertEquals(Fund.FundStatus.ACTIVE, updatedFund.getFundStatus()); + + // Verify budget updates + ArgumentCaptor> budgetIdsCaptor = ArgumentCaptor.forClass(List.class); + verify(budgetService).getBudgetsByIds(budgetIdsCaptor.capture(), eq(dbConn)); + assertEquals(collection.getFyFinanceData().get(0).getBudgetId(), budgetIdsCaptor.getValue().get(0)); + + ArgumentCaptor> budgetCaptor = ArgumentCaptor.forClass(List.class); + verify(budgetService).updateBatchBudgets(budgetCaptor.capture(), eq(dbConn)); + Budget updatedBudget = budgetCaptor.getValue().get(0); + assertEquals("New Budget Name", updatedBudget.getName()); + assertEquals(1000.0, updatedBudget.getInitialAllocation()); + assertEquals(900.0, updatedBudget.getAllocated()); + assertEquals(Budget.BudgetStatus.ACTIVE, updatedBudget.getBudgetStatus()); + }); + testContext.completeNow(); + }))); + } + + @Test + void shouldFailUpdateWhenFundServiceFails(VertxTestContext testContext) { + // given + FyFinanceDataCollection collection = createTestFinanceDataCollection(); + RuntimeException expectedError = new RuntimeException("Fund service error"); + + when(requestContext.toDBClient()).thenReturn(dbClient); + doAnswer(invocation -> { + Function> function = invocation.getArgument(0); + return function.apply(dbConn); + }).when(dbClient).withTrans(any()); + + when(fundService.getFundsByIds(any(), any())).thenReturn(Future.failedFuture(expectedError)); + + // when & then + financeDataService2.update(collection, requestContext) + .onComplete(testContext.failing(error -> { + testContext.verify(() -> { + assertEquals("Fund service error", error.getMessage()); + verify(budgetService, never()).getBudgetsByIds(any(), any()); + verify(budgetService, never()).updateBatchBudgets(any(), any()); + }); + testContext.completeNow(); + })); + } + + private FyFinanceDataCollection createTestFinanceDataCollection() { + String fundId = UUID.randomUUID().toString(); + String budgetId = UUID.randomUUID().toString(); + + FyFinanceData financeData = new FyFinanceData() + .withFundId(fundId) + .withBudgetId(budgetId) + .withFundCode("NEW-CODE") + .withFundName("New Fund Name") + .withFundStatus(FyFinanceData.FundStatus.ACTIVE) + .withFundDescription("New Description") + .withFundAcqUnitIds(List.of("unit1")) + .withBudgetName("New Budget Name") + .withBudgetStatus(FyFinanceData.BudgetStatus.ACTIVE) + .withBudgetInitialAllocation(1000.0) + .withBudgetCurrentAllocation(900.0) + .withBudgetAllowableExpenditure(800.0) + .withBudgetAllowableEncumbrance(700.0) + .withBudgetAcqUnitIds(List.of("unit1")); + + return new FyFinanceDataCollection() + .withFyFinanceData(List.of(financeData)); + } +} From cb9dd7a356503566b79d949d2b5c4a5185aded1a Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 15:10:36 +0500 Subject: [PATCH 03/13] Improved update as a bulk, and added unit tests --- descriptors/ModuleDescriptor-template.json | 16 ++-- src/main/java/org/folio/dao/fund/FundDAO.java | 3 +- .../org/folio/dao/fund/FundPostgresDAO.java | 24 +++--- .../org/folio/rest/impl/FinanceDataApi.java | 8 ++ .../financedata/FinanceDataService.java | 29 ++++---- .../org/folio/service/fund/FundService.java | 2 +- .../service/fund/StorageFundService.java | 8 +- .../db_scripts/all_finance_data_view.sql | 2 +- .../folio/rest/impl/FinanceDataApiTest.java | 73 +++++++++---------- 9 files changed, 88 insertions(+), 77 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 2ddbbf19..4a64b5bb 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -472,9 +472,9 @@ "permissionsRequired": ["finance-storage.finance-data.collection.get"] }, { - "methods": ["POST"], + "methods": ["PUT"], "pathPattern": "/finance-storage/finance-data", - "permissionsRequired": ["finance-storage.finance-data.item.post"] + "permissionsRequired": ["finance-storage.finance-data.collection.put"] } ] }, @@ -1023,13 +1023,13 @@ }, { "permissionName": "finance-storage.finance-data.collection.get", - "displayName": "all finance-data for fiscal year", - "description": "Get collection of finance data for particular fiscal year" + "displayName": "all finance-data", + "description": "Get collection of finance data" }, { - "permissionName": "finance-storage.finance-data.item.post", - "displayName": "create finance-data for fiscal year", - "description": "Create finance data for particular fiscal year" + "permissionName": "finance-storage.finance-data.collection.put", + "displayName": "Update finance-data as a bulk", + "description": "Update collection of finance data" }, { "permissionName": "finance-storage.finance-data.all", @@ -1037,7 +1037,7 @@ "description": "All permissions for the finance data", "subPermissions": [ "finance-storage.finance-data.collection.get", - "finance-storage.finance-data.item.post" + "finance-storage.finance-data.collection.put" ] }, { diff --git a/src/main/java/org/folio/dao/fund/FundDAO.java b/src/main/java/org/folio/dao/fund/FundDAO.java index d8952920..0ce101f1 100644 --- a/src/main/java/org/folio/dao/fund/FundDAO.java +++ b/src/main/java/org/folio/dao/fund/FundDAO.java @@ -10,7 +10,8 @@ public interface FundDAO { Future getFundById(String id, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); - Future updateFundById(String fundId, Fund fund, DBConn conn); + Future updateFund(Fund fund, DBConn conn); + Future updateFunds(List funds, DBConn conn); Future isFundStatusChanged(Fund fund, DBConn conn); Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn); } diff --git a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java index 49d43e53..652eb5ba 100644 --- a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java +++ b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java @@ -55,10 +55,20 @@ public Future> getFundsByIds(List ids, DBConn conn) { } @Override - public Future updateFundById(String fundId, Fund fund, DBConn conn) { - logger.debug("Trying to update finance storage fund by id {}", fundId); - fund.setId(fundId); - return updateFund(fund, conn); + public Future updateFund(Fund fund, DBConn conn) { + logger.debug("Trying to update finance storage fund by id {}", fund.getId()); + return conn.update(FUND_TABLE, fund, fund.getId()) + .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) + .mapEmpty(); + } + + @Override + public Future updateFunds(List funds, DBConn conn) { + List fundIds = funds.stream().map(Fund::getId).toList(); + logger.debug("Trying to update finance storage funds: '{}'", fundIds); + return conn.updateBatch(FUND_TABLE, funds) + .onSuccess(x -> logger.info("Funds '{}' was successfully updated", fundIds)) + .mapEmpty(); } @Override @@ -82,12 +92,6 @@ public Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { .mapEmpty(); } - private Future updateFund(Fund fund, DBConn conn) { - return conn.update(FUND_TABLE, fund, fund.getId()) - .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) - .mapEmpty(); - } - private Future> getFundsByCriterion(Criterion criterion, DBConn conn) { logger.debug("Trying to get funds by criterion = {}", criterion); return conn.get(FUND_TABLE, Fund.class, criterion, false) diff --git a/src/main/java/org/folio/rest/impl/FinanceDataApi.java b/src/main/java/org/folio/rest/impl/FinanceDataApi.java index e3706941..c91865c9 100644 --- a/src/main/java/org/folio/rest/impl/FinanceDataApi.java +++ b/src/main/java/org/folio/rest/impl/FinanceDataApi.java @@ -10,6 +10,7 @@ import io.vertx.core.AsyncResult; import io.vertx.core.Context; import io.vertx.core.Handler; +import io.vertx.core.Vertx; import org.folio.rest.core.model.RequestContext; import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.FyFinanceDataCollection; @@ -17,13 +18,20 @@ import org.folio.rest.persist.PgUtil; import org.folio.rest.util.ResponseUtils; import org.folio.service.financedata.FinanceDataService; +import org.folio.spring.SpringContextUtil; +import org.springframework.beans.factory.annotation.Autowired; public class FinanceDataApi implements FinanceStorageFinanceData { private static final String FINANCE_DATA_VIEW = "finance_data_view"; + @Autowired private FinanceDataService financeDataService; + public FinanceDataApi() { + SpringContextUtil.autowireDependencies(this, Vertx.currentContext()); + } + @Override public void getFinanceStorageFinanceData(String query, String totalRecords, int offset, int limit, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { diff --git a/src/main/java/org/folio/service/financedata/FinanceDataService.java b/src/main/java/org/folio/service/financedata/FinanceDataService.java index 632a0070..71a9dfd7 100644 --- a/src/main/java/org/folio/service/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/service/financedata/FinanceDataService.java @@ -47,13 +47,8 @@ private Future processFundUpdate(FyFinanceDataCollection entity, DBConn co .map(FyFinanceData::getFundId) .toList(); return fundService.getFundsByIds(fundIds, conn) - .compose(funds -> { - var futures = funds.stream() - .map(fund -> setNewValues(fund, entity)) - .map(fund -> fundService.updateFundWithMinChange(fund, conn)) - .toList(); - return GenericCompositeFuture.all(futures).mapEmpty(); - }); + .map(funds -> setNewValuesForFunds(funds, entity)) + .compose(funds -> fundService.updateFundsWithMinChange(funds, conn)); } private Future processBudgetUpdate(FyFinanceDataCollection entity, DBConn conn) { @@ -61,10 +56,16 @@ private Future processBudgetUpdate(FyFinanceDataCollection entity, DBConn .map(FyFinanceData::getBudgetId) .toList(); return budgetService.getBudgetsByIds(budgetIds, conn) - .map(budgets -> setNewValues(budgets, entity)) + .map(budgets -> setNewValuesForBudgets(budgets, entity)) .compose(budgets -> budgetService.updateBatchBudgets(budgets, conn)); } + private List setNewValuesForFunds(List funds, FyFinanceDataCollection entity) { + return funds.stream() + .map(fund -> setNewValues(fund, entity)) + .toList(); + } + private Fund setNewValues(Fund fund, FyFinanceDataCollection entity) { var fundFinanceData = entity.getFyFinanceData().stream() .filter(data -> data.getFundId().equals(fund.getId())) @@ -78,7 +79,7 @@ private Fund setNewValues(Fund fund, FyFinanceDataCollection entity) { return fund; } - private List setNewValues(List budgets, FyFinanceDataCollection entity) { + private List setNewValuesForBudgets(List budgets, FyFinanceDataCollection entity) { return budgets.stream() .map(budget -> setNewValues(budget, entity)) .toList(); @@ -86,16 +87,14 @@ private List setNewValues(List budgets, FyFinanceDataCollection private Budget setNewValues(Budget budget, FyFinanceDataCollection entity) { var budgetFinanceData = entity.getFyFinanceData().stream() - .filter(data -> data.getFundId().equals(budget.getId())) + .filter(data -> data.getBudgetId().equals(budget.getId())) .findFirst() .orElseThrow(); - return budget.withName(budgetFinanceData.getBudgetName()) - .withBudgetStatus(Budget.BudgetStatus.fromValue(budgetFinanceData.getBudgetStatus().value())) + return budget.withBudgetStatus(Budget.BudgetStatus.fromValue(budgetFinanceData.getBudgetStatus().value())) .withInitialAllocation(budgetFinanceData.getBudgetInitialAllocation()) .withAllocated(budgetFinanceData.getBudgetCurrentAllocation()) - .withAllowableEncumbrance(budgetFinanceData.getBudgetAllowableExpenditure()) - .withAllowableEncumbrance(budgetFinanceData.getBudgetAllowableEncumbrance()) - .withAcqUnitIds(budgetFinanceData.getBudgetAcqUnitIds()); + .withAllowableExpenditure(budgetFinanceData.getBudgetAllowableExpenditure()) + .withAllowableEncumbrance(budgetFinanceData.getBudgetAllowableEncumbrance()); } } diff --git a/src/main/java/org/folio/service/fund/FundService.java b/src/main/java/org/folio/service/fund/FundService.java index 36fa28d3..cbfdcc3a 100644 --- a/src/main/java/org/folio/service/fund/FundService.java +++ b/src/main/java/org/folio/service/fund/FundService.java @@ -10,5 +10,5 @@ public interface FundService { Future getFundById(String fundId, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); Future updateFund(Fund fund, DBConn conn); - Future updateFundWithMinChange(Fund fund, DBConn conn); + Future updateFundsWithMinChange(List fund, DBConn conn); } diff --git a/src/main/java/org/folio/service/fund/StorageFundService.java b/src/main/java/org/folio/service/fund/StorageFundService.java index ab8c9f66..af07e293 100644 --- a/src/main/java/org/folio/service/fund/StorageFundService.java +++ b/src/main/java/org/folio/service/fund/StorageFundService.java @@ -34,14 +34,14 @@ public Future updateFund(Fund fund, DBConn conn) { .compose(statusChanged -> { if (Boolean.TRUE.equals(statusChanged)) { return fundDAO.updateRelatedCurrentFYBudgets(fund, conn) - .compose(v -> fundDAO.updateFundById(fund.getId(), fund, conn)); + .compose(v -> fundDAO.updateFund(fund, conn)); } - return updateFundWithMinChange(fund, conn); + return fundDAO.updateFund(fund, conn); }); } @Override - public Future updateFundWithMinChange(Fund fund, DBConn conn) { - return fundDAO.updateFundById(fund.getId(), fund, conn); + public Future updateFundsWithMinChange(List funds, DBConn conn) { + return fundDAO.updateFunds(funds, conn); } } diff --git a/src/main/resources/templates/db_scripts/all_finance_data_view.sql b/src/main/resources/templates/db_scripts/all_finance_data_view.sql index 9c77b4c3..79454f98 100644 --- a/src/main/resources/templates/db_scripts/all_finance_data_view.sql +++ b/src/main/resources/templates/db_scripts/all_finance_data_view.sql @@ -9,7 +9,7 @@ SELECT 'fundName', fund.jsonb ->>'name', 'fundDescription', fund.jsonb ->>'description', 'fundStatus', fund.jsonb ->>'fundStatus', - 'fundTags', fund.jsonb ->'tags' -> 'tagList', + 'fundTags', jsonb_build_object('tagList', fund.jsonb -> 'tags' -> 'tagList'), 'fundAcqUnitIds', fund.jsonb ->'acqUnitIds', 'budgetId', budget.id, 'budgetName', budget.jsonb ->>'name', diff --git a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java index 157f382d..67b75ca9 100644 --- a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java +++ b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java @@ -10,7 +10,6 @@ import static org.folio.rest.utils.TestEntities.LEDGER; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.mockito.Mockito.mock; import java.util.List; import java.util.UUID; @@ -22,18 +21,16 @@ import org.folio.rest.jaxrs.model.Budget; import org.folio.rest.jaxrs.model.FiscalYear; import org.folio.rest.jaxrs.model.Fund; +import org.folio.rest.jaxrs.model.FundTags; +import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.FyFinanceDataCollection; import org.folio.rest.jaxrs.model.Ledger; import org.folio.rest.jaxrs.model.TenantJob; import org.folio.rest.jaxrs.resource.FinanceStorageFinanceData; import org.folio.rest.persist.HelperUtils; -import org.folio.service.budget.BudgetService; -import org.folio.service.financedata.FinanceDataService; -import org.folio.service.fund.FundService; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.springframework.context.annotation.Bean; public class FinanceDataApiTest extends TestBase { @@ -42,7 +39,7 @@ public class FinanceDataApiTest extends TestBase { private static final Logger logger = LogManager.getLogger(); private static final String FINANCE_DATA_ENDPOINT = HelperUtils.getEndpoint(FinanceStorageFinanceData.class); private static TenantJob tenantJob; - + private static final String FISCAL_YEAR_ID = UUID.randomUUID().toString(); @BeforeAll public static void before() { @@ -84,17 +81,16 @@ public void positive_testGetQuery() { @Test public void positive_testResponseOfGetWithParamsFiscalYearAndAcqUnitIds() { - var fiscalYearId = UUID.randomUUID().toString(); var acqUnitId = UUID.randomUUID().toString(); var fiscalYearAcqUnitEndpoint = String.format("%s?query=(fiscalYearId==%s and fundAcqUnitIds=%s and budgetAcqUnitIds=%s)", - FINANCE_DATA_ENDPOINT, fiscalYearId, acqUnitId, acqUnitId); - createMockData(fiscalYearId, acqUnitId); + FINANCE_DATA_ENDPOINT, FISCAL_YEAR_ID, acqUnitId, acqUnitId); + createMockData(FISCAL_YEAR_ID, acqUnitId, "FY2088", "first"); var response = getData(fiscalYearAcqUnitEndpoint, TENANT_HEADER); var body = response.getBody().as(FyFinanceDataCollection.class); var actualFyFinanceData = body.getFyFinanceData().get(0); - assertEquals(fiscalYearId, actualFyFinanceData.getFiscalYearId()); + assertEquals(FISCAL_YEAR_ID, actualFyFinanceData.getFiscalYearId()); assertEquals(acqUnitId, actualFyFinanceData.getFundAcqUnitIds().get(0)); assertEquals(acqUnitId, actualFyFinanceData.getBudgetAcqUnitIds().get(0)); } @@ -103,67 +99,70 @@ public void positive_testResponseOfGetWithParamsFiscalYearAndAcqUnitIds() { public void positive_testUpdateFinanceData() { var fiscalYearId = UUID.randomUUID().toString(); var acqUnitId = UUID.randomUUID().toString(); - createMockData(fiscalYearId, acqUnitId); + var expectedDescription = "UPDATED Description"; + var expectedTags = List.of("New tag"); + var expectedBudgetStatus = FyFinanceData.BudgetStatus.INACTIVE; + var expectedNumber = 200.0; + createMockData(fiscalYearId, acqUnitId, "FY2099", "second"); var response = getData(FINANCE_DATA_ENDPOINT + "?query=(fiscalYearId==" + fiscalYearId + ")", TENANT_HEADER); var body = response.getBody().as(FyFinanceDataCollection.class); var fyFinanceData = body.getFyFinanceData().get(0); - fyFinanceData.setFundName("Updated Fund Name"); - fyFinanceData.setBudgetName("Updated Budget Name"); + // Set required fields difference values than before + fyFinanceData.setFundDescription(expectedDescription); + fyFinanceData.setFundTags(new FundTags().withTagList(expectedTags)); + fyFinanceData.setBudgetStatus(expectedBudgetStatus); + fyFinanceData.setBudgetInitialAllocation(expectedNumber); + fyFinanceData.setBudgetAllowableExpenditure(expectedNumber); + fyFinanceData.setBudgetAllowableEncumbrance(expectedNumber); var updatedCollection = new FyFinanceDataCollection().withFyFinanceData(List.of(fyFinanceData)).withTotalRecords(1); + // Update finance data as a bulk var updateResponse = putData(FINANCE_DATA_ENDPOINT, JsonObject.mapFrom(updatedCollection).encodePrettily(), TENANT_HEADER); assertEquals(204, updateResponse.getStatusCode()); + // Get updated result var updatedResponse = getData(FINANCE_DATA_ENDPOINT + "?query=(fiscalYearId==" + fiscalYearId + ")", TENANT_HEADER); var updatedBody = updatedResponse.getBody().as(FyFinanceDataCollection.class); var updatedFyFinanceData = updatedBody.getFyFinanceData().get(0); - assertEquals("Updated Fund Name", updatedFyFinanceData.getFundName()); - assertEquals("Updated Budget Name", updatedFyFinanceData.getBudgetName()); + assertEquals(expectedDescription, updatedFyFinanceData.getFundDescription()); + assertEquals(expectedTags, updatedFyFinanceData.getFundTags().getTagList()); + assertEquals(expectedNumber, updatedFyFinanceData.getBudgetInitialAllocation()); + assertEquals(expectedNumber, updatedFyFinanceData.getBudgetAllowableEncumbrance()); + assertEquals(expectedNumber, updatedFyFinanceData.getBudgetAllowableExpenditure()); } - private void createMockData(String fiscalYearId, String acqUnitId) { + private void createMockData(String fiscalYearId, String acqUnitId, String code, String name) { var fundId = UUID.randomUUID().toString(); var ledgerId = UUID.randomUUID().toString(); var budgetId = UUID.randomUUID().toString(); var fiscalYear = new JsonObject(getFile(FISCAL_YEAR.getPathToSampleFile())).mapTo(FiscalYear.class) - .withId(fiscalYearId).withCode("FY2042"); + .withId(fiscalYearId).withCode(code); createEntity(FISCAL_YEAR.getEndpoint(), fiscalYear, TENANT_HEADER); var ledger = new JsonObject(getFile(LEDGER.getPathToSampleFile())).mapTo(Ledger.class).withId(ledgerId) - .withCode("first").withName("First Ledger").withFiscalYearOneId(fiscalYearId); + .withCode(code).withName(name).withFiscalYearOneId(fiscalYearId); createEntity(LEDGER.getEndpoint(), ledger, TENANT_HEADER); var fund = new JsonObject(getFile(FUND.getPathToSampleFile())).mapTo(Fund.class) - .withId(fundId).withCode("first").withName("first").withLedgerId(ledgerId) + .withId(fundId).withCode(code).withName(name).withLedgerId(ledgerId) .withFundTypeId(null).withAcqUnitIds(List.of(acqUnitId)); createEntity(FUND.getEndpoint(), fund, TENANT_HEADER); var budget = new JsonObject(getFile(BUDGET.getPathToSampleFile())).mapTo(Budget.class) - .withId(budgetId).withName("first").withFiscalYearId(fiscalYearId).withFundId(fundId) + .withId(budgetId).withName(name) + .withBudgetStatus(Budget.BudgetStatus.ACTIVE) + .withFiscalYearId(fiscalYearId) + .withFundId(fundId) + .withInitialAllocation(100.0) + .withAllowableExpenditure(101.0) + .withAllowableEncumbrance(102.0) .withAcqUnitIds(List.of(acqUnitId)); createEntity(BUDGET.getEndpoint(), budget, TENANT_HEADER); } - static class ContextConfiguration { - - @Bean - public FinanceDataService financeDataService(FundService fundService, BudgetService budgetService) { - return mock(FinanceDataService.class); - } - - @Bean - FundService fundService() { - return mock(FundService.class); - } - - @Bean - BudgetService budgetService() { - return mock(BudgetService.class); - } - } } From d89a2e1fb4a6b0acf050cecaecdeecb48d3cf97b Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 16:50:07 +0500 Subject: [PATCH 04/13] Added FinanceDataServiceTest and improvements --- src/main/java/org/folio/dao/fund/FundDAO.java | 4 +- .../org/folio/dao/fund/FundPostgresDAO.java | 34 ++--- .../java/org/folio/rest/impl/FundAPI.java | 18 ++- .../org/folio/service/fund/FundService.java | 3 +- .../service/fund/StorageFundService.java | 22 +-- src/test/java/org/folio/StorageTestSuite.java | 4 + .../fianancedata/FinanceDataServiceTest.java | 144 ++++++++++-------- 7 files changed, 128 insertions(+), 101 deletions(-) diff --git a/src/main/java/org/folio/dao/fund/FundDAO.java b/src/main/java/org/folio/dao/fund/FundDAO.java index 0ce101f1..fd1c7001 100644 --- a/src/main/java/org/folio/dao/fund/FundDAO.java +++ b/src/main/java/org/folio/dao/fund/FundDAO.java @@ -10,8 +10,8 @@ public interface FundDAO { Future getFundById(String id, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); - Future updateFund(Fund fund, DBConn conn); - Future updateFunds(List funds, DBConn conn); Future isFundStatusChanged(Fund fund, DBConn conn); Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn); + Future updateFund(Fund fund, DBConn conn); + Future updateFunds(List funds, DBConn conn); } diff --git a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java index 652eb5ba..6bfa48d4 100644 --- a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java +++ b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java @@ -54,23 +54,6 @@ public Future> getFundsByIds(List ids, DBConn conn) { return getFundsByCriterion(criterionBuilder.build(), conn); } - @Override - public Future updateFund(Fund fund, DBConn conn) { - logger.debug("Trying to update finance storage fund by id {}", fund.getId()); - return conn.update(FUND_TABLE, fund, fund.getId()) - .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) - .mapEmpty(); - } - - @Override - public Future updateFunds(List funds, DBConn conn) { - List fundIds = funds.stream().map(Fund::getId).toList(); - logger.debug("Trying to update finance storage funds: '{}'", fundIds); - return conn.updateBatch(FUND_TABLE, funds) - .onSuccess(x -> logger.info("Funds '{}' was successfully updated", fundIds)) - .mapEmpty(); - } - @Override public Future isFundStatusChanged(Fund fund, DBConn conn) { return getFundById(fund.getId(), conn) @@ -92,6 +75,23 @@ public Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { .mapEmpty(); } + @Override + public Future updateFund(Fund fund, DBConn conn) { + logger.debug("Trying to update finance storage fund by id {}", fund.getId()); + return conn.update(FUND_TABLE, fund, fund.getId()) + .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) + .mapEmpty(); + } + + @Override + public Future updateFunds(List funds, DBConn conn) { + List fundIds = funds.stream().map(Fund::getId).toList(); + logger.debug("Trying to update finance storage funds: '{}'", fundIds); + return conn.updateBatch(FUND_TABLE, funds) + .onSuccess(x -> logger.info("Funds '{}' was successfully updated", fundIds)) + .mapEmpty(); + } + private Future> getFundsByCriterion(Criterion criterion, DBConn conn) { logger.debug("Trying to get funds by criterion = {}", criterion); return conn.get(FUND_TABLE, Fund.class, criterion, false) diff --git a/src/main/java/org/folio/rest/impl/FundAPI.java b/src/main/java/org/folio/rest/impl/FundAPI.java index 9ff35361..a73f5c2e 100644 --- a/src/main/java/org/folio/rest/impl/FundAPI.java +++ b/src/main/java/org/folio/rest/impl/FundAPI.java @@ -1,6 +1,7 @@ package org.folio.rest.impl; import static io.vertx.core.Future.succeededFuture; +import static org.folio.rest.jaxrs.resource.FinanceStorageGroupFundFiscalYears.PutFinanceStorageGroupFundFiscalYearsByIdResponse.respond204; import javax.ws.rs.core.Response; import java.util.Map; @@ -8,17 +9,19 @@ import io.vertx.core.AsyncResult; import io.vertx.core.Context; import io.vertx.core.Handler; +import io.vertx.core.Vertx; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.rest.annotations.Validate; +import org.folio.rest.core.model.RequestContext; import org.folio.rest.exception.HttpException; import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.jaxrs.model.FundCollection; import org.folio.rest.jaxrs.resource.FinanceStorageFunds; -import org.folio.rest.persist.DBClient; import org.folio.rest.persist.HelperUtils; import org.folio.rest.persist.PgUtil; import org.folio.service.fund.FundService; +import org.folio.spring.SpringContextUtil; import org.springframework.beans.factory.annotation.Autowired; public class FundAPI implements FinanceStorageFunds { @@ -30,6 +33,10 @@ public class FundAPI implements FinanceStorageFunds { @Autowired private FundService fundService; + public FundAPI() { + SpringContextUtil.autowireDependencies(this, Vertx.currentContext()); + } + @Override @Validate public void getFinanceStorageFunds(String query, String totalRecords, int offset, int limit, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { @@ -60,18 +67,15 @@ public void deleteFinanceStorageFundsById(String id, Map okapiHe public void putFinanceStorageFundsById(String id, Fund fund, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { logger.debug("Trying to update finance storage fund by id {}", id); fund.setId(id); - var dbClient = new DBClient(vertxContext, okapiHeaders); - dbClient.withTrans(conn -> fundService.updateFund(fund, conn) + fundService.updateFund(fund, new RequestContext(vertxContext, okapiHeaders)) .onComplete(result -> { if (result.failed()) { HttpException cause = (HttpException) result.cause(); logger.error("Failed to update the finance storage fund with Id {}", fund.getId(), cause); HelperUtils.replyWithErrorResponse(asyncResultHandler, cause); } else { - asyncResultHandler.handle(succeededFuture( - FinanceStorageFunds.PutFinanceStorageFundsByIdResponse.respond204())); + asyncResultHandler.handle(succeededFuture(respond204())); } - }) - ); + }); } } diff --git a/src/main/java/org/folio/service/fund/FundService.java b/src/main/java/org/folio/service/fund/FundService.java index cbfdcc3a..f5103e22 100644 --- a/src/main/java/org/folio/service/fund/FundService.java +++ b/src/main/java/org/folio/service/fund/FundService.java @@ -1,6 +1,7 @@ package org.folio.service.fund; import io.vertx.core.Future; +import org.folio.rest.core.model.RequestContext; import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.persist.DBConn; @@ -9,6 +10,6 @@ public interface FundService { Future getFundById(String fundId, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); - Future updateFund(Fund fund, DBConn conn); + Future updateFund(Fund fund, RequestContext requestContext); Future updateFundsWithMinChange(List fund, DBConn conn); } diff --git a/src/main/java/org/folio/service/fund/StorageFundService.java b/src/main/java/org/folio/service/fund/StorageFundService.java index af07e293..81a57d34 100644 --- a/src/main/java/org/folio/service/fund/StorageFundService.java +++ b/src/main/java/org/folio/service/fund/StorageFundService.java @@ -4,6 +4,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.dao.fund.FundDAO; +import org.folio.rest.core.model.RequestContext; import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.persist.DBConn; @@ -29,15 +30,18 @@ public Future> getFundsByIds(List ids, DBConn conn) { } @Override - public Future updateFund(Fund fund, DBConn conn) { - return fundDAO.isFundStatusChanged(fund, conn) - .compose(statusChanged -> { - if (Boolean.TRUE.equals(statusChanged)) { - return fundDAO.updateRelatedCurrentFYBudgets(fund, conn) - .compose(v -> fundDAO.updateFund(fund, conn)); - } - return fundDAO.updateFund(fund, conn); - }); + public Future updateFund(Fund fund, RequestContext requestContext) { + var dbClient = requestContext.toDBClient(); + return dbClient.withTrans(conn -> + fundDAO.isFundStatusChanged(fund, conn) + .compose(statusChanged -> { + if (Boolean.TRUE.equals(statusChanged)) { + return fundDAO.updateRelatedCurrentFYBudgets(fund, conn) + .compose(v -> fundDAO.updateFund(fund, conn)); + } + return fundDAO.updateFund(fund, conn); + }) + ); } @Override diff --git a/src/test/java/org/folio/StorageTestSuite.java b/src/test/java/org/folio/StorageTestSuite.java index 6df6d059..75b54aa1 100644 --- a/src/test/java/org/folio/StorageTestSuite.java +++ b/src/test/java/org/folio/StorageTestSuite.java @@ -36,6 +36,7 @@ import org.folio.rest.tools.utils.NetworkUtils; import org.folio.rest.utils.DBClientTest; import org.folio.service.email.EmailServiceTest; +import org.folio.service.fianancedata.FinanceDataServiceTest; import org.folio.service.rollover.LedgerRolloverServiceTest; import org.folio.service.rollover.RolloverProgressServiceTest; import org.folio.service.rollover.RolloverValidationServiceTest; @@ -220,4 +221,7 @@ class PendingPaymentTestNested extends PendingPaymentTest {} @Nested class FinanceDataApiTestNested extends FinanceDataApiTest {} + + @Nested + class FinanceDataServiceTestNested extends FinanceDataServiceTest {} } diff --git a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java index 3ae93549..b752e81f 100644 --- a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java +++ b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java @@ -1,6 +1,7 @@ package org.folio.service.fianancedata; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; @@ -19,6 +20,7 @@ import org.folio.rest.core.model.RequestContext; import org.folio.rest.jaxrs.model.Budget; import org.folio.rest.jaxrs.model.Fund; +import org.folio.rest.jaxrs.model.FundTags; import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.FyFinanceDataCollection; import org.folio.rest.persist.DBClient; @@ -68,58 +70,20 @@ void tearDown() throws Exception { @Test void shouldSuccessfullyUpdateFinanceData(VertxTestContext testContext) { FinanceDataService financeDataService = Mockito.spy(financeDataService2); - // given - FyFinanceDataCollection collection = createTestFinanceDataCollection(); - Fund fund = new Fund() - .withId(collection.getFyFinanceData().get(0).getFundId()) - .withCode("OLD-CODE") - .withName("Old Name"); - Budget budget = new Budget() - .withId(collection.getFyFinanceData().get(0).getBudgetId()) - .withName("Old Budget Name"); + var collection = createTestFinanceDataCollection(); + var oldFund = new Fund().withId(collection.getFyFinanceData().get(0).getFundId()) + .withName("NAME").withCode("CODE").withFundStatus(Fund.FundStatus.ACTIVE) + .withDescription("Old des"); + var oldBudget = new Budget().withId(collection.getFyFinanceData().get(0).getBudgetId()) + .withName("NAME") + .withBudgetStatus(Budget.BudgetStatus.ACTIVE); + setupMocks(oldFund, oldBudget); - when(requestContext.toDBClient()).thenReturn(dbClient); - doAnswer(invocation -> { - Function> function = invocation.getArgument(0); - return function.apply(dbConn); - }).when(dbClient).withTrans(any()); - - when(fundService.getFundsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(fund))); - when(fundService.updateFund(any(), any())).thenReturn(Future.succeededFuture()); - when(budgetService.getBudgetsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(budget))); - when(budgetService.updateBatchBudgets(any(), any())).thenReturn(Future.succeededFuture()); - - // when & then testContext.assertComplete(financeDataService.update(collection, requestContext) .onComplete(testContext.succeeding(result -> { testContext.verify(() -> { - // Verify transaction handling - verify(dbClient).withTrans(any()); - - // Verify fund updates - ArgumentCaptor> fundIdsCaptor = ArgumentCaptor.forClass(List.class); - verify(fundService).getFundsByIds(fundIdsCaptor.capture(), eq(dbConn)); - assertEquals(collection.getFyFinanceData().get(0).getFundId(), fundIdsCaptor.getValue().get(0)); - - ArgumentCaptor fundCaptor = ArgumentCaptor.forClass(Fund.class); - verify(fundService).updateFund(fundCaptor.capture(), eq(dbConn)); - Fund updatedFund = fundCaptor.getValue(); - assertEquals("NEW-CODE", updatedFund.getCode()); - assertEquals("New Fund Name", updatedFund.getName()); - assertEquals(Fund.FundStatus.ACTIVE, updatedFund.getFundStatus()); - - // Verify budget updates - ArgumentCaptor> budgetIdsCaptor = ArgumentCaptor.forClass(List.class); - verify(budgetService).getBudgetsByIds(budgetIdsCaptor.capture(), eq(dbConn)); - assertEquals(collection.getFyFinanceData().get(0).getBudgetId(), budgetIdsCaptor.getValue().get(0)); - - ArgumentCaptor> budgetCaptor = ArgumentCaptor.forClass(List.class); - verify(budgetService).updateBatchBudgets(budgetCaptor.capture(), eq(dbConn)); - Budget updatedBudget = budgetCaptor.getValue().get(0); - assertEquals("New Budget Name", updatedBudget.getName()); - assertEquals(1000.0, updatedBudget.getInitialAllocation()); - assertEquals(900.0, updatedBudget.getAllocated()); - assertEquals(Budget.BudgetStatus.ACTIVE, updatedBudget.getBudgetStatus()); + verifyFundUpdates(collection); + verifyBudgetUpdates(collection); }); testContext.completeNow(); }))); @@ -127,30 +91,79 @@ void shouldSuccessfullyUpdateFinanceData(VertxTestContext testContext) { @Test void shouldFailUpdateWhenFundServiceFails(VertxTestContext testContext) { - // given - FyFinanceDataCollection collection = createTestFinanceDataCollection(); - RuntimeException expectedError = new RuntimeException("Fund service error"); - - when(requestContext.toDBClient()).thenReturn(dbClient); - doAnswer(invocation -> { - Function> function = invocation.getArgument(0); - return function.apply(dbConn); - }).when(dbClient).withTrans(any()); + var collection = createTestFinanceDataCollection(); + var expectedError = new RuntimeException("Fund service error"); - when(fundService.getFundsByIds(any(), any())).thenReturn(Future.failedFuture(expectedError)); + setupMocksForFailure(expectedError); - // when & then financeDataService2.update(collection, requestContext) .onComplete(testContext.failing(error -> { testContext.verify(() -> { assertEquals("Fund service error", error.getMessage()); - verify(budgetService, never()).getBudgetsByIds(any(), any()); verify(budgetService, never()).updateBatchBudgets(any(), any()); + verify(fundService, never()).updateFundsWithMinChange(any(), any()); }); testContext.completeNow(); })); } + private void setupMocks(Fund oldFund, Budget oldBudget) { + when(requestContext.toDBClient()).thenReturn(dbClient); + doAnswer(invocation -> { + Function> function = invocation.getArgument(0); + return function.apply(dbConn); + }).when(dbClient).withTrans(any()); + + when(fundService.getFundsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(oldFund))); + when(fundService.updateFundsWithMinChange(any(), any())).thenReturn(Future.succeededFuture()); + when(budgetService.getBudgetsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(oldBudget))); + when(budgetService.updateBatchBudgets(any(), any())).thenReturn(Future.succeededFuture()); + } + + private void setupMocksForFailure(RuntimeException expectedError) { + when(requestContext.toDBClient()).thenReturn(dbClient); + doAnswer(invocation -> { + Function> function = invocation.getArgument(0); + return function.apply(dbConn); + }).when(dbClient).withTrans(any()); + + when(fundService.getFundsByIds(any(), any())).thenReturn(Future.failedFuture(expectedError)); + when(budgetService.getBudgetsByIds(any(), any())).thenReturn(Future.succeededFuture()); + } + + private void verifyFundUpdates(FyFinanceDataCollection collection) { + ArgumentCaptor> fundIdsCaptor = ArgumentCaptor.forClass(List.class); + verify(fundService).getFundsByIds(fundIdsCaptor.capture(), eq(dbConn)); + assertEquals(collection.getFyFinanceData().get(0).getFundId(), fundIdsCaptor.getValue().get(0)); + + ArgumentCaptor> fundCaptor = ArgumentCaptor.forClass(List.class); + verify(fundService).updateFundsWithMinChange(fundCaptor.capture(), eq(dbConn)); + Fund updatedFund = fundCaptor.getValue().get(0); + + assertNotEquals("CODE CHANGED", updatedFund.getCode()); + assertNotEquals("NAME CHANGED", updatedFund.getName()); + + assertEquals(Fund.FundStatus.ACTIVE, updatedFund.getFundStatus()); + assertEquals("New Description", updatedFund.getDescription()); + } + + private void verifyBudgetUpdates(FyFinanceDataCollection collection) { + ArgumentCaptor> budgetIdsCaptor = ArgumentCaptor.forClass(List.class); + verify(budgetService).getBudgetsByIds(budgetIdsCaptor.capture(), eq(dbConn)); + assertEquals(collection.getFyFinanceData().get(0).getBudgetId(), budgetIdsCaptor.getValue().get(0)); + + ArgumentCaptor> budgetCaptor = ArgumentCaptor.forClass(List.class); + verify(budgetService).updateBatchBudgets(budgetCaptor.capture(), eq(dbConn)); + Budget updatedBudget = budgetCaptor.getValue().get(0); + + assertNotEquals("NAME CHANGED", updatedBudget.getName()); + + assertEquals(FyFinanceData.BudgetStatus.INACTIVE.value(), updatedBudget.getBudgetStatus().value()); + assertEquals(1000.0, updatedBudget.getInitialAllocation()); + assertEquals(900.0, updatedBudget.getAllocated()); + } + + private FyFinanceDataCollection createTestFinanceDataCollection() { String fundId = UUID.randomUUID().toString(); String budgetId = UUID.randomUUID().toString(); @@ -158,13 +171,14 @@ private FyFinanceDataCollection createTestFinanceDataCollection() { FyFinanceData financeData = new FyFinanceData() .withFundId(fundId) .withBudgetId(budgetId) - .withFundCode("NEW-CODE") - .withFundName("New Fund Name") - .withFundStatus(FyFinanceData.FundStatus.ACTIVE) + .withFundCode("CODE CHANGED") + .withFundName("NAME CHANGED") + .withFundStatus(FyFinanceData.FundStatus.INACTIVE) .withFundDescription("New Description") .withFundAcqUnitIds(List.of("unit1")) - .withBudgetName("New Budget Name") - .withBudgetStatus(FyFinanceData.BudgetStatus.ACTIVE) + .withFundTags(new FundTags().withTagList(List.of("Education"))) + .withBudgetName("NAME CHANGED") + .withBudgetStatus(FyFinanceData.BudgetStatus.INACTIVE) .withBudgetInitialAllocation(1000.0) .withBudgetCurrentAllocation(900.0) .withBudgetAllowableExpenditure(800.0) From 27fbd100216012eb06293ee6282a9d4c4f5b9ba4 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 17:03:19 +0500 Subject: [PATCH 05/13] minor update --- ramls/finance-data.raml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ramls/finance-data.raml b/ramls/finance-data.raml index df05a8f2..f3eeabe8 100644 --- a/ramls/finance-data.raml +++ b/ramls/finance-data.raml @@ -48,13 +48,13 @@ resourceTypes: body: text/plain: example: | - "One or more <> not found" + "One or more items not found" 400: description: "Bad request, e.g. malformed request body or query parameter. Details of the error (e.g. name of the parameter or line/character number with malformed data) provided in the response." body: text/plain: example: | - "unable to update <> -- malformed JSON at 13:4" + "unable to update items -- malformed JSON at 13:4" 409: description: "Optimistic locking version conflict" body: From 726bbebe124bf83cd5d2b9c9b340bd9d8c4d0d14 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 17:13:57 +0500 Subject: [PATCH 06/13] added null check and fixed sonar issue --- .../org/folio/service/financedata/FinanceDataService.java | 4 ++++ src/main/java/org/folio/service/fund/StorageFundService.java | 3 +++ src/test/java/org/folio/rest/impl/TestBase.java | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/folio/service/financedata/FinanceDataService.java b/src/main/java/org/folio/service/financedata/FinanceDataService.java index 71a9dfd7..6684938d 100644 --- a/src/main/java/org/folio/service/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/service/financedata/FinanceDataService.java @@ -5,6 +5,7 @@ import java.util.List; import io.vertx.core.Future; +import org.apache.commons.collections4.CollectionUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.okapi.common.GenericCompositeFuture; @@ -30,6 +31,9 @@ public FinanceDataService(FundService fundService, BudgetService budgetService) } public Future update(FyFinanceDataCollection entity, RequestContext requestContext) { + if (CollectionUtils.isEmpty(entity.getFyFinanceData())) { + return Future.succeededFuture(); + } var dbClient = requestContext.toDBClient(); return dbClient .withTrans(conn -> { diff --git a/src/main/java/org/folio/service/fund/StorageFundService.java b/src/main/java/org/folio/service/fund/StorageFundService.java index 81a57d34..843f9c45 100644 --- a/src/main/java/org/folio/service/fund/StorageFundService.java +++ b/src/main/java/org/folio/service/fund/StorageFundService.java @@ -31,11 +31,13 @@ public Future> getFundsByIds(List ids, DBConn conn) { @Override public Future updateFund(Fund fund, RequestContext requestContext) { + logger.debug("Trying to update fund '{}'", fund.getId()); var dbClient = requestContext.toDBClient(); return dbClient.withTrans(conn -> fundDAO.isFundStatusChanged(fund, conn) .compose(statusChanged -> { if (Boolean.TRUE.equals(statusChanged)) { + logger.info("updateFund:: Fund '{}' status has been changed to '{}'", fund.getId(), fund.getFundStatus()); return fundDAO.updateRelatedCurrentFYBudgets(fund, conn) .compose(v -> fundDAO.updateFund(fund, conn)); } @@ -46,6 +48,7 @@ public Future updateFund(Fund fund, RequestContext requestContext) { @Override public Future updateFundsWithMinChange(List funds, DBConn conn) { + logger.debug("updateFundsWithMinChange:: Trying to update '{}' fund(s) with minimal changes", funds.size()); return fundDAO.updateFunds(funds, conn); } } diff --git a/src/test/java/org/folio/rest/impl/TestBase.java b/src/test/java/org/folio/rest/impl/TestBase.java index 00f023b9..50b909b8 100644 --- a/src/test/java/org/folio/rest/impl/TestBase.java +++ b/src/test/java/org/folio/rest/impl/TestBase.java @@ -241,7 +241,7 @@ void testFetchingUpdatedEntity(String id, TestEntities subObject) { .path(subObject.getUpdatedFieldName()); // Get string value of updated field and compare - assertThat(String.valueOf(prop), equalTo(subObject.getUpdatedFieldValue())); + assertEquals(String.valueOf(prop), subObject.getUpdatedFieldValue()); } Response testEntitySuccessfullyFetched(String endpoint, String id) { From 7fbce496b8f525f5e4774982e3b867fd82a4e80a Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 17:45:17 +0500 Subject: [PATCH 07/13] Remove set of allocated field --- .../java/org/folio/service/financedata/FinanceDataService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/folio/service/financedata/FinanceDataService.java b/src/main/java/org/folio/service/financedata/FinanceDataService.java index 6684938d..82521c20 100644 --- a/src/main/java/org/folio/service/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/service/financedata/FinanceDataService.java @@ -97,7 +97,6 @@ private Budget setNewValues(Budget budget, FyFinanceDataCollection entity) { return budget.withBudgetStatus(Budget.BudgetStatus.fromValue(budgetFinanceData.getBudgetStatus().value())) .withInitialAllocation(budgetFinanceData.getBudgetInitialAllocation()) - .withAllocated(budgetFinanceData.getBudgetCurrentAllocation()) .withAllowableExpenditure(budgetFinanceData.getBudgetAllowableExpenditure()) .withAllowableEncumbrance(budgetFinanceData.getBudgetAllowableEncumbrance()); } From f569bd4865afd18baa5f6c7470bd72e4ceeb2542 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 17:50:40 +0500 Subject: [PATCH 08/13] Fix test --- .../org/folio/service/fianancedata/FinanceDataServiceTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java index b752e81f..0e733775 100644 --- a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java +++ b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java @@ -160,7 +160,8 @@ private void verifyBudgetUpdates(FyFinanceDataCollection collection) { assertEquals(FyFinanceData.BudgetStatus.INACTIVE.value(), updatedBudget.getBudgetStatus().value()); assertEquals(1000.0, updatedBudget.getInitialAllocation()); - assertEquals(900.0, updatedBudget.getAllocated()); + assertEquals(800.0, updatedBudget.getAllowableExpenditure()); + assertEquals(700.0, updatedBudget.getAllowableEncumbrance()); } From 7eb0c3cee648f3c635b5644b4608832719d526a3 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 17:53:05 +0500 Subject: [PATCH 09/13] Fix log --- src/main/java/org/folio/dao/fund/FundPostgresDAO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java index 6bfa48d4..d4616778 100644 --- a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java +++ b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java @@ -79,7 +79,7 @@ public Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { public Future updateFund(Fund fund, DBConn conn) { logger.debug("Trying to update finance storage fund by id {}", fund.getId()); return conn.update(FUND_TABLE, fund, fund.getId()) - .onSuccess(x -> logger.info("Fund record {} was successfully updated", fund)) + .onSuccess(x -> logger.info("Fund record '{}' was successfully updated", fund.getId())) .mapEmpty(); } From 113e6cc45655d1f636016a5a27aef41fe17e0e91 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Fri, 29 Nov 2024 14:00:35 +0500 Subject: [PATCH 10/13] Added ledgerId, ledgerCode, and groupId, groupCode --- ramls/acq-models | 2 +- .../db_scripts/all_finance_data_view.sql | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/ramls/acq-models b/ramls/acq-models index c06778a4..d6dac0a6 160000 --- a/ramls/acq-models +++ b/ramls/acq-models @@ -1 +1 @@ -Subproject commit c06778a484802db30023f1e5388a6d2972606ae1 +Subproject commit d6dac0a62d1dc2c95a74c6463976147965b1e9d2 diff --git a/src/main/resources/templates/db_scripts/all_finance_data_view.sql b/src/main/resources/templates/db_scripts/all_finance_data_view.sql index 79454f98..8fabd814 100644 --- a/src/main/resources/templates/db_scripts/all_finance_data_view.sql +++ b/src/main/resources/templates/db_scripts/all_finance_data_view.sql @@ -11,6 +11,8 @@ SELECT 'fundStatus', fund.jsonb ->>'fundStatus', 'fundTags', jsonb_build_object('tagList', fund.jsonb -> 'tags' -> 'tagList'), 'fundAcqUnitIds', fund.jsonb ->'acqUnitIds', + 'ledgerId', ledger.id, + 'ledgerCode', ledger.jsonb ->> 'code', 'budgetId', budget.id, 'budgetName', budget.jsonb ->>'name', 'budgetStatus', budget.jsonb ->>'budgetStatus', @@ -18,12 +20,18 @@ SELECT 'budgetCurrentAllocation', budget.jsonb ->>'allocated', 'budgetAllowableExpenditure', budget.jsonb ->>'allowableExpenditure', 'budgetAllowableEncumbrance', budget.jsonb ->>'allowableEncumbrance', - 'budgetAcqUnitIds', budget.jsonb ->'acqUnitIds' + 'budgetAcqUnitIds', budget.jsonb ->'acqUnitIds', + 'groupId', groups.id, + 'groupCode', groups.jsonb ->> 'code' ) as jsonb FROM ${myuniversity}_${mymodule}.fiscal_year LEFT OUTER JOIN ${myuniversity}_${mymodule}.ledger - ON ledger.fiscalyearoneid = fiscal_year.id + ON ledger.fiscalyearoneid = fiscal_year.id LEFT OUTER JOIN ${myuniversity}_${mymodule}.fund - ON fund.ledgerid = ledger.id + ON fund.ledgerid = ledger.id LEFT OUTER JOIN ${myuniversity}_${mymodule}.budget - ON fund.id = budget.fundid; + ON fund.id = budget.fundid +LEFT OUTER JOIN ${myuniversity}_${mymodule}.group_fund_fiscal_year + ON fund.id = (group_fund_fiscal_year.jsonb ->> 'fundId')::uuid +LEFT OUTER JOIN ${myuniversity}_${mymodule}.groups + ON (group_fund_fiscal_year.jsonb ->> 'groupId')::uuid = groups.id; From a12f4d05e18da07e72f480c0690e52f842290b32 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Fri, 29 Nov 2024 15:18:50 +0500 Subject: [PATCH 11/13] improved response handling and removed initial allocation field set --- ramls/acq-models | 2 +- .../java/org/folio/dao/fund/FundPostgresDAO.java | 15 ++++++++------- src/main/java/org/folio/rest/impl/FundAPI.java | 14 +++----------- .../service/financedata/FinanceDataService.java | 1 - .../org/folio/rest/impl/FinanceDataApiTest.java | 2 -- src/test/java/org/folio/rest/impl/TestBase.java | 1 - .../fianancedata/FinanceDataServiceTest.java | 2 +- 7 files changed, 13 insertions(+), 24 deletions(-) diff --git a/ramls/acq-models b/ramls/acq-models index d6dac0a6..0d8f736a 160000 --- a/ramls/acq-models +++ b/ramls/acq-models @@ -1 +1 @@ -Subproject commit d6dac0a62d1dc2c95a74c6463976147965b1e9d2 +Subproject commit 0d8f736a3f2b5401a5cfad20a95bd831843f77a6 diff --git a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java index d4616778..1a13360d 100644 --- a/src/main/java/org/folio/dao/fund/FundPostgresDAO.java +++ b/src/main/java/org/folio/dao/fund/FundPostgresDAO.java @@ -26,6 +26,12 @@ public class FundPostgresDAO implements FundDAO { private static final Logger logger = LogManager.getLogger(); private static final String FUND_NOT_FOUND = "Fund not found, id=%s"; + private static final String QUERY_UPDATE_CURRENT_FY_BUDGET = + "UPDATE %s SET jsonb = jsonb_set(jsonb,'{budgetStatus}', $1) " + + "WHERE((fundId=$2) " + + "AND (budget.fiscalYearId IN " + + "(SELECT id FROM %s WHERE current_date between (jsonb->>'periodStart')::timestamp " + + "AND (jsonb->>'periodEnd')::timestamp)));"; @Override public Future getFundById(String id, DBConn conn) { @@ -64,14 +70,9 @@ public Future isFundStatusChanged(Fund fund, DBConn conn) { public Future updateRelatedCurrentFYBudgets(Fund fund, DBConn conn) { String fullBudgetTableName = getFullTableName(conn.getTenantId(), BUDGET_TABLE); String fullFYTableName = getFullTableName(conn.getTenantId(), FISCAL_YEAR_TABLE); + String updateQuery = String.format(QUERY_UPDATE_CURRENT_FY_BUDGET, fullBudgetTableName, fullFYTableName); - String sql = "UPDATE " + fullBudgetTableName + " SET jsonb = jsonb_set(jsonb,'{budgetStatus}', $1) " + - "WHERE((fundId=$2) " + - "AND (budget.fiscalYearId IN " + - "(SELECT id FROM " + fullFYTableName + " WHERE current_date between (jsonb->>'periodStart')::timestamp " + - "AND (jsonb->>'periodEnd')::timestamp)));"; - - return conn.execute(sql, Tuple.of(fund.getFundStatus().value(), UUID.fromString(fund.getId()))) + return conn.execute(updateQuery, Tuple.of(fund.getFundStatus().value(), UUID.fromString(fund.getId()))) .mapEmpty(); } diff --git a/src/main/java/org/folio/rest/impl/FundAPI.java b/src/main/java/org/folio/rest/impl/FundAPI.java index a73f5c2e..56f4fa31 100644 --- a/src/main/java/org/folio/rest/impl/FundAPI.java +++ b/src/main/java/org/folio/rest/impl/FundAPI.java @@ -14,12 +14,11 @@ import org.apache.logging.log4j.Logger; import org.folio.rest.annotations.Validate; import org.folio.rest.core.model.RequestContext; -import org.folio.rest.exception.HttpException; import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.jaxrs.model.FundCollection; import org.folio.rest.jaxrs.resource.FinanceStorageFunds; -import org.folio.rest.persist.HelperUtils; import org.folio.rest.persist.PgUtil; +import org.folio.rest.util.ResponseUtils; import org.folio.service.fund.FundService; import org.folio.spring.SpringContextUtil; import org.springframework.beans.factory.annotation.Autowired; @@ -68,14 +67,7 @@ public void putFinanceStorageFundsById(String id, Fund fund, Map logger.debug("Trying to update finance storage fund by id {}", id); fund.setId(id); fundService.updateFund(fund, new RequestContext(vertxContext, okapiHeaders)) - .onComplete(result -> { - if (result.failed()) { - HttpException cause = (HttpException) result.cause(); - logger.error("Failed to update the finance storage fund with Id {}", fund.getId(), cause); - HelperUtils.replyWithErrorResponse(asyncResultHandler, cause); - } else { - asyncResultHandler.handle(succeededFuture(respond204())); - } - }); + .onSuccess(result -> asyncResultHandler.handle(succeededFuture(respond204()))) + .onFailure(ResponseUtils::handleFailure); } } diff --git a/src/main/java/org/folio/service/financedata/FinanceDataService.java b/src/main/java/org/folio/service/financedata/FinanceDataService.java index 82521c20..551ebdd7 100644 --- a/src/main/java/org/folio/service/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/service/financedata/FinanceDataService.java @@ -96,7 +96,6 @@ private Budget setNewValues(Budget budget, FyFinanceDataCollection entity) { .orElseThrow(); return budget.withBudgetStatus(Budget.BudgetStatus.fromValue(budgetFinanceData.getBudgetStatus().value())) - .withInitialAllocation(budgetFinanceData.getBudgetInitialAllocation()) .withAllowableExpenditure(budgetFinanceData.getBudgetAllowableExpenditure()) .withAllowableEncumbrance(budgetFinanceData.getBudgetAllowableEncumbrance()); } diff --git a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java index 67b75ca9..8745e889 100644 --- a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java +++ b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java @@ -113,7 +113,6 @@ public void positive_testUpdateFinanceData() { fyFinanceData.setFundDescription(expectedDescription); fyFinanceData.setFundTags(new FundTags().withTagList(expectedTags)); fyFinanceData.setBudgetStatus(expectedBudgetStatus); - fyFinanceData.setBudgetInitialAllocation(expectedNumber); fyFinanceData.setBudgetAllowableExpenditure(expectedNumber); fyFinanceData.setBudgetAllowableEncumbrance(expectedNumber); @@ -130,7 +129,6 @@ public void positive_testUpdateFinanceData() { assertEquals(expectedDescription, updatedFyFinanceData.getFundDescription()); assertEquals(expectedTags, updatedFyFinanceData.getFundTags().getTagList()); - assertEquals(expectedNumber, updatedFyFinanceData.getBudgetInitialAllocation()); assertEquals(expectedNumber, updatedFyFinanceData.getBudgetAllowableEncumbrance()); assertEquals(expectedNumber, updatedFyFinanceData.getBudgetAllowableExpenditure()); } diff --git a/src/test/java/org/folio/rest/impl/TestBase.java b/src/test/java/org/folio/rest/impl/TestBase.java index 50b909b8..f67ab4a4 100644 --- a/src/test/java/org/folio/rest/impl/TestBase.java +++ b/src/test/java/org/folio/rest/impl/TestBase.java @@ -7,7 +7,6 @@ import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.hasEntry; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; import java.io.InputStream; import java.util.Set; diff --git a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java index 0e733775..da8e2b6a 100644 --- a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java +++ b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java @@ -157,9 +157,9 @@ private void verifyBudgetUpdates(FyFinanceDataCollection collection) { Budget updatedBudget = budgetCaptor.getValue().get(0); assertNotEquals("NAME CHANGED", updatedBudget.getName()); + assertNotEquals(1000.0, updatedBudget.getInitialAllocation()); assertEquals(FyFinanceData.BudgetStatus.INACTIVE.value(), updatedBudget.getBudgetStatus().value()); - assertEquals(1000.0, updatedBudget.getInitialAllocation()); assertEquals(800.0, updatedBudget.getAllowableExpenditure()); assertEquals(700.0, updatedBudget.getAllowableEncumbrance()); } From 78f290a62cb0971bf9e3ad19bed08f813d7fc25c Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Fri, 29 Nov 2024 16:30:37 +0500 Subject: [PATCH 12/13] Fixed finance_data_view and improved method names --- .../org/folio/service/financedata/FinanceDataService.java | 2 +- src/main/java/org/folio/service/fund/FundService.java | 2 +- .../java/org/folio/service/fund/StorageFundService.java | 2 +- .../templates/db_scripts/all_finance_data_view.sql | 4 ++-- .../service/fianancedata/FinanceDataServiceTest.java | 8 ++++---- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/folio/service/financedata/FinanceDataService.java b/src/main/java/org/folio/service/financedata/FinanceDataService.java index 551ebdd7..66d11a2e 100644 --- a/src/main/java/org/folio/service/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/service/financedata/FinanceDataService.java @@ -52,7 +52,7 @@ private Future processFundUpdate(FyFinanceDataCollection entity, DBConn co .toList(); return fundService.getFundsByIds(fundIds, conn) .map(funds -> setNewValuesForFunds(funds, entity)) - .compose(funds -> fundService.updateFundsWithMinChange(funds, conn)); + .compose(funds -> fundService.updateFunds(funds, conn)); } private Future processBudgetUpdate(FyFinanceDataCollection entity, DBConn conn) { diff --git a/src/main/java/org/folio/service/fund/FundService.java b/src/main/java/org/folio/service/fund/FundService.java index f5103e22..9988c2f9 100644 --- a/src/main/java/org/folio/service/fund/FundService.java +++ b/src/main/java/org/folio/service/fund/FundService.java @@ -11,5 +11,5 @@ public interface FundService { Future getFundById(String fundId, DBConn conn); Future> getFundsByIds(List ids, DBConn conn); Future updateFund(Fund fund, RequestContext requestContext); - Future updateFundsWithMinChange(List fund, DBConn conn); + Future updateFunds(List fund, DBConn conn); } diff --git a/src/main/java/org/folio/service/fund/StorageFundService.java b/src/main/java/org/folio/service/fund/StorageFundService.java index 843f9c45..eeb757d3 100644 --- a/src/main/java/org/folio/service/fund/StorageFundService.java +++ b/src/main/java/org/folio/service/fund/StorageFundService.java @@ -47,7 +47,7 @@ public Future updateFund(Fund fund, RequestContext requestContext) { } @Override - public Future updateFundsWithMinChange(List funds, DBConn conn) { + public Future updateFunds(List funds, DBConn conn) { logger.debug("updateFundsWithMinChange:: Trying to update '{}' fund(s) with minimal changes", funds.size()); return fundDAO.updateFunds(funds, conn); } diff --git a/src/main/resources/templates/db_scripts/all_finance_data_view.sql b/src/main/resources/templates/db_scripts/all_finance_data_view.sql index 8fabd814..4c6492a9 100644 --- a/src/main/resources/templates/db_scripts/all_finance_data_view.sql +++ b/src/main/resources/templates/db_scripts/all_finance_data_view.sql @@ -32,6 +32,6 @@ LEFT OUTER JOIN ${myuniversity}_${mymodule}.fund LEFT OUTER JOIN ${myuniversity}_${mymodule}.budget ON fund.id = budget.fundid LEFT OUTER JOIN ${myuniversity}_${mymodule}.group_fund_fiscal_year - ON fund.id = (group_fund_fiscal_year.jsonb ->> 'fundId')::uuid + ON fund.id = group_fund_fiscal_year.fundid LEFT OUTER JOIN ${myuniversity}_${mymodule}.groups - ON (group_fund_fiscal_year.jsonb ->> 'groupId')::uuid = groups.id; + ON group_fund_fiscal_year.groupid = groups.id; diff --git a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java index da8e2b6a..1887ccef 100644 --- a/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java +++ b/src/test/java/org/folio/service/fianancedata/FinanceDataServiceTest.java @@ -58,7 +58,7 @@ public class FinanceDataServiceTest { private AutoCloseable mockitoMocks; @BeforeEach - void setUp(Vertx vertx) { + void setUp() { mockitoMocks = MockitoAnnotations.openMocks(this); } @@ -101,7 +101,7 @@ void shouldFailUpdateWhenFundServiceFails(VertxTestContext testContext) { testContext.verify(() -> { assertEquals("Fund service error", error.getMessage()); verify(budgetService, never()).updateBatchBudgets(any(), any()); - verify(fundService, never()).updateFundsWithMinChange(any(), any()); + verify(fundService, never()).updateFunds(any(), any()); }); testContext.completeNow(); })); @@ -115,7 +115,7 @@ private void setupMocks(Fund oldFund, Budget oldBudget) { }).when(dbClient).withTrans(any()); when(fundService.getFundsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(oldFund))); - when(fundService.updateFundsWithMinChange(any(), any())).thenReturn(Future.succeededFuture()); + when(fundService.updateFunds(any(), any())).thenReturn(Future.succeededFuture()); when(budgetService.getBudgetsByIds(any(), any())).thenReturn(Future.succeededFuture(List.of(oldBudget))); when(budgetService.updateBatchBudgets(any(), any())).thenReturn(Future.succeededFuture()); } @@ -137,7 +137,7 @@ private void verifyFundUpdates(FyFinanceDataCollection collection) { assertEquals(collection.getFyFinanceData().get(0).getFundId(), fundIdsCaptor.getValue().get(0)); ArgumentCaptor> fundCaptor = ArgumentCaptor.forClass(List.class); - verify(fundService).updateFundsWithMinChange(fundCaptor.capture(), eq(dbConn)); + verify(fundService).updateFunds(fundCaptor.capture(), eq(dbConn)); Fund updatedFund = fundCaptor.getValue().get(0); assertNotEquals("CODE CHANGED", updatedFund.getCode()); From c8cb33263b8e52d965d0d1fa235ededa8b26392c Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Fri, 29 Nov 2024 17:57:37 +0500 Subject: [PATCH 13/13] Fixed error response handling --- src/main/java/org/folio/rest/impl/FundAPI.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/folio/rest/impl/FundAPI.java b/src/main/java/org/folio/rest/impl/FundAPI.java index 56f4fa31..246fc51a 100644 --- a/src/main/java/org/folio/rest/impl/FundAPI.java +++ b/src/main/java/org/folio/rest/impl/FundAPI.java @@ -14,11 +14,12 @@ import org.apache.logging.log4j.Logger; import org.folio.rest.annotations.Validate; import org.folio.rest.core.model.RequestContext; +import org.folio.rest.exception.HttpException; import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.jaxrs.model.FundCollection; import org.folio.rest.jaxrs.resource.FinanceStorageFunds; +import org.folio.rest.persist.HelperUtils; import org.folio.rest.persist.PgUtil; -import org.folio.rest.util.ResponseUtils; import org.folio.service.fund.FundService; import org.folio.spring.SpringContextUtil; import org.springframework.beans.factory.annotation.Autowired; @@ -66,8 +67,14 @@ public void deleteFinanceStorageFundsById(String id, Map okapiHe public void putFinanceStorageFundsById(String id, Fund fund, Map okapiHeaders, Handler> asyncResultHandler, Context vertxContext) { logger.debug("Trying to update finance storage fund by id {}", id); fund.setId(id); - fundService.updateFund(fund, new RequestContext(vertxContext, okapiHeaders)) + vertxContext.runOnContext(event -> + fundService.updateFund(fund, new RequestContext(vertxContext, okapiHeaders)) .onSuccess(result -> asyncResultHandler.handle(succeededFuture(respond204()))) - .onFailure(ResponseUtils::handleFailure); + .onFailure(throwable -> { + HttpException cause = (HttpException) throwable; + logger.error("Failed to update the finance storage fund with Id {}", fund.getId(), cause); + HelperUtils.replyWithErrorResponse(asyncResultHandler, cause); + }) + ); } }