From cb9dd7a356503566b79d949d2b5c4a5185aded1a Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Thu, 28 Nov 2024 15:10:36 +0500 Subject: [PATCH] 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); - } - } }