From 434e5794e310a0bd8c6a9fe45bf338c2c66a91e9 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Mon, 18 Nov 2024 18:50:15 +0500 Subject: [PATCH 01/11] [MODFIN-379] - Create API for fund updates logs --- descriptors/ModuleDescriptor-template.json | 23 ++- ramls/acq-models | 2 +- ramls/finance-data.raml | 37 +++++ .../folio/config/ServicesConfiguration.java | 6 + .../org/folio/rest/impl/FinanceDataApi.java | 37 +++++ .../folio/rest/util/ResourcePathResolver.java | 2 + .../financedata/FinanceDataService.java | 42 +++++ .../rest/impl/EntitiesCrudBasicsTest.java | 14 +- .../folio/rest/impl/FinanceDataApiTest.java | 155 ++++++++++++++++++ .../java/org/folio/rest/util/MockServer.java | 34 ++++ .../org/folio/rest/util/TestEntities.java | 3 + .../mockdata/financedata/fy-finance-data.json | 24 +++ 12 files changed, 373 insertions(+), 6 deletions(-) create mode 100644 ramls/finance-data.raml create mode 100644 src/main/java/org/folio/rest/impl/FinanceDataApi.java create mode 100644 src/main/java/org/folio/services/financedata/FinanceDataService.java create mode 100644 src/test/java/org/folio/rest/impl/FinanceDataApiTest.java create mode 100644 src/test/resources/mockdata/financedata/fy-finance-data.json diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 7e1bdf76..7fdb78cb 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -776,6 +776,17 @@ } ] }, + { + "id": "finance.finance-data", + "version": "1.0", + "handlers": [ + { + "methods": ["GET"], + "pathPattern": "/finance/finance-data", + "permissionsRequired": ["finance.finance-data.collection.get"] + } + ] + }, { "id": "_jsonSchemas", "version": "1.0", @@ -853,6 +864,10 @@ { "id": "finance-storage.fund-update-logs", "version":"1.0" + }, + { + "id": "finance-storage.finance-data", + "version":"1.0" } ], "optional": [ @@ -1338,6 +1353,11 @@ "finance.fund-update-logs.item.delete" ] }, + { + "permissionName": "finance.finance-data.collection.get", + "displayName": "Finances - get finance data collection", + "description": "Get finance data collection" + }, { "permissionName": "finance.all", "displayName": "Finance module - all permissions", @@ -1357,7 +1377,8 @@ "finance.calculate-exchange.item.get", "finance.expense-classes.all", "finance.acquisitions-units-assignments.all", - "finance.fund-update-logs.all" + "finance.fund-update-logs.all", + "finance.finance-data.collection.get" ], "visible": false }, diff --git a/ramls/acq-models b/ramls/acq-models index 680fe63a..1652e69e 160000 --- a/ramls/acq-models +++ b/ramls/acq-models @@ -1 +1 @@ -Subproject commit 680fe63a34fc7b25d9d59013a2758de76cf82b56 +Subproject commit 1652e69e9e98499b805b698e59efd7d2a51d98b5 diff --git a/ramls/finance-data.raml b/ramls/finance-data.raml new file mode 100644 index 00000000..6d807495 --- /dev/null +++ b/ramls/finance-data.raml @@ -0,0 +1,37 @@ +#%RAML 1.0 +title: Finance - finance data +version: v1 +protocols: [ HTTP, HTTPS ] +baseUri: https://github.com/folio-org/mod-finance + +documentation: + - title: Finance data APIs + content: This documents the API calls that can be made to manage finance data + +types: + errors: !include raml-util/schemas/errors.schema + fy-finance-data-collection: !include acq-models/mod-finance/schemas/fy_finance_data_collection.json + UUID: + type: string + 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 + searchable: !include raml-util/traits/searchable.raml + validate: !include raml-util/traits/validation.raml + +resourceTypes: + collection-get: !include raml-util/rtypes/collection-get.raml + +/finance/finance-data: + type: + collection-get: + exampleCollection: !include acq-models/mod-finance/examples/fy_finance_data_collection.sample + schemaCollection: fy-finance-data-collection + get: + description: Get finance data + is: [ + searchable: { description: "with valid searchable fields: for example fiscalYearId", example: "[\"fiscalYearId\", \"7a4c4d30-3b63-4102-8e2d-3ee5792d7d02\", \"=\"]" }, + pageable + ] + diff --git a/src/main/java/org/folio/config/ServicesConfiguration.java b/src/main/java/org/folio/config/ServicesConfiguration.java index 5edc9163..765aafb7 100644 --- a/src/main/java/org/folio/config/ServicesConfiguration.java +++ b/src/main/java/org/folio/config/ServicesConfiguration.java @@ -8,6 +8,7 @@ import org.folio.services.budget.RecalculateBudgetService; import org.folio.services.budget.CreateBudgetService; import org.folio.services.configuration.ConfigurationEntriesService; +import org.folio.services.financedata.FinanceDataService; import org.folio.services.fiscalyear.FiscalYearApiService; import org.folio.services.fiscalyear.FiscalYearService; import org.folio.services.fund.FundCodeExpenseClassesService; @@ -216,4 +217,9 @@ FundCodeExpenseClassesService fundCodeExpenseClassesService(BudgetService budget FundUpdateLogService fundUpdateLogService(RestClient restClient) { return new FundUpdateLogService(restClient); } + + @Bean + FinanceDataService financeDataService(RestClient restClient, AcqUnitsService acqUnitsService) { + return new FinanceDataService(restClient, acqUnitsService); + } } diff --git a/src/main/java/org/folio/rest/impl/FinanceDataApi.java b/src/main/java/org/folio/rest/impl/FinanceDataApi.java new file mode 100644 index 00000000..70b5c4ac --- /dev/null +++ b/src/main/java/org/folio/rest/impl/FinanceDataApi.java @@ -0,0 +1,37 @@ +package org.folio.rest.impl; + +import static io.vertx.core.Future.succeededFuture; + +import io.vertx.core.AsyncResult; +import io.vertx.core.Context; +import io.vertx.core.Handler; +import io.vertx.core.Vertx; +import java.util.Map; +import javax.ws.rs.core.Response; +import org.folio.rest.annotations.Validate; +import org.folio.rest.core.models.RequestContext; +import org.folio.rest.jaxrs.resource.FinanceFinanceData; +import org.folio.services.financedata.FinanceDataService; +import org.folio.spring.SpringContextUtil; +import org.springframework.beans.factory.annotation.Autowired; + +public class FinanceDataApi extends BaseApi implements FinanceFinanceData { + + @Autowired + private FinanceDataService financeDataService; + + public FinanceDataApi() { + SpringContextUtil.autowireDependencies(this, Vertx.currentContext()); + } + + @Override + @Validate + public void getFinanceFinanceData(String query, String totalRecords, int offset, int limit, + Map okapiHeaders, Handler> asyncResultHandler, + Context vertxContext) { + financeDataService.getFinanceDataWithAcqUnitsRestriction(query, offset, limit, + new RequestContext(vertxContext, okapiHeaders)) + .onSuccess(financeData -> asyncResultHandler.handle(succeededFuture(buildOkResponse(financeData)))) + .onFailure(fail -> handleErrorResponse(asyncResultHandler, fail)); + } +} diff --git a/src/main/java/org/folio/rest/util/ResourcePathResolver.java b/src/main/java/org/folio/rest/util/ResourcePathResolver.java index 3592624b..d3d00085 100644 --- a/src/main/java/org/folio/rest/util/ResourcePathResolver.java +++ b/src/main/java/org/folio/rest/util/ResourcePathResolver.java @@ -11,6 +11,7 @@ private ResourcePathResolver() { } public static final String BUDGETS_STORAGE = "budgets"; + public static final String FINANCE_DATA_STORAGE = "financeData"; public static final String FUNDS_STORAGE = "funds"; public static final String FUND_TYPES = "fundTypes"; public static final String FUND_UPDATE_LOGS = "fundUpdateLogs"; @@ -38,6 +39,7 @@ private ResourcePathResolver() { static { Map apis = new HashMap<>(); apis.put(BUDGETS_STORAGE, "/finance-storage/budgets"); + apis.put(FINANCE_DATA_STORAGE, "/finance-storage/finance-data"); apis.put(FUNDS_STORAGE, "/finance-storage/funds"); apis.put(FUND_TYPES, "/finance-storage/fund-types"); apis.put(FUND_UPDATE_LOGS, "/finance-storage/fund-update-logs"); diff --git a/src/main/java/org/folio/services/financedata/FinanceDataService.java b/src/main/java/org/folio/services/financedata/FinanceDataService.java new file mode 100644 index 00000000..b64bbc48 --- /dev/null +++ b/src/main/java/org/folio/services/financedata/FinanceDataService.java @@ -0,0 +1,42 @@ +package org.folio.services.financedata; + +import static org.folio.rest.util.HelperUtils.combineCqlExpressions; +import static org.folio.rest.util.ResourcePathResolver.FINANCE_DATA_STORAGE; +import static org.folio.rest.util.ResourcePathResolver.resourcesPath; + +import io.vertx.core.Future; +import org.apache.commons.lang3.StringUtils; +import org.folio.rest.core.RestClient; +import org.folio.rest.core.models.RequestContext; +import org.folio.rest.core.models.RequestEntry; +import org.folio.rest.jaxrs.model.FyFinanceDataCollection; +import org.folio.services.protection.AcqUnitsService; + +public class FinanceDataService { + private final RestClient restClient; + private final AcqUnitsService acqUnitsService; + public static final String ID = "id"; + + public FinanceDataService(RestClient restClient, AcqUnitsService acqUnitsService) { + this.restClient = restClient; + this.acqUnitsService = acqUnitsService; + } + + public Future getFinanceDataWithAcqUnitsRestriction(String query, int offset, int limit, + RequestContext requestContext) { + + return acqUnitsService.buildAcqUnitsCqlClause(requestContext) + .map(clause -> StringUtils.isEmpty(query) ? clause : combineCqlExpressions("and", clause, query)) + .map(effectiveQuery -> new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) + .withOffset(offset) + .withLimit(limit) + .withQuery(effectiveQuery) + ) + .compose(requestEntry -> restClient.get(requestEntry.buildEndpoint(), FyFinanceDataCollection.class, requestContext)); + } +} +// return restClient.get(new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) +// .withOffset(offset) +// .withLimit(limit) +// .withQuery(query) +// .buildEndpoint(), FyFinanceDataCollection.class, requestContext); diff --git a/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java b/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java index 5409013f..185b0831 100644 --- a/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java +++ b/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java @@ -20,6 +20,7 @@ import static org.folio.rest.util.TestConstants.TOTAL_RECORDS; import static org.folio.rest.util.TestConstants.VALID_UUID; import static org.folio.rest.util.TestEntities.BUDGET; +import static org.folio.rest.util.TestEntities.FINANCE_DATA; import static org.folio.rest.util.TestEntities.FISCAL_YEAR; import static org.folio.rest.util.TestEntities.FUND; import static org.folio.rest.util.TestEntities.GROUP; @@ -128,14 +129,16 @@ static Stream getTestEntitiesWithGetEndpointWithoutGroup() { */ static Stream getTestEntitiesWithGetByIdEndpoint() { return getTestEntitiesWithGetEndpoint() - .filter(e -> !e.equals(GROUP_FUND_FISCAL_YEAR)); + .filter(e -> !e.equals(GROUP_FUND_FISCAL_YEAR)) + .filter(e -> !e.equals(FINANCE_DATA)); } static Stream getTestEntitiesWithPostEndpoint() { return getTestEntities() .filter(e -> !e.equals(TRANSACTIONS)) .filter(e -> !e.equals(TRANSACTIONS_ALLOCATION)) - .filter(e -> !e.equals(TRANSACTIONS_TRANSFER)); + .filter(e -> !e.equals(TRANSACTIONS_TRANSFER)) + .filter(e -> !e.equals(FINANCE_DATA)); } /** @@ -145,7 +148,8 @@ static Stream getTestEntitiesWithPostEndpoint() { */ static Stream getTestEntitiesWithPutEndpoint() { return getTestEntitiesWithGetByIdEndpoint() - .filter(e -> !e.equals(TRANSACTIONS)); + .filter(e -> !e.equals(TRANSACTIONS)) + .filter(e -> !e.equals(FINANCE_DATA)); } /** @@ -154,7 +158,9 @@ static Stream getTestEntitiesWithPutEndpoint() { * @return stream of test entities */ static Stream getTestEntitiesWithDeleteEndpoint() { - return getTestEntitiesWithGetEndpoint().filter(e -> !e.equals(TRANSACTIONS)); + return getTestEntitiesWithGetEndpoint() + .filter(e -> !e.equals(TRANSACTIONS)) + .filter(e -> !e.equals(FINANCE_DATA)); } /** diff --git a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java new file mode 100644 index 00000000..be27fc15 --- /dev/null +++ b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java @@ -0,0 +1,155 @@ +package org.folio.rest.impl; + +import static javax.ws.rs.core.MediaType.APPLICATION_JSON; +import static javax.ws.rs.core.Response.Status.NOT_FOUND; +import static javax.ws.rs.core.Response.Status.OK; +import static org.folio.rest.util.MockServer.addMockEntry; +import static org.folio.rest.util.RestTestUtils.verifyGet; +import static org.folio.rest.util.TestConfig.autowireDependencies; +import static org.folio.rest.util.TestConfig.clearVertxContext; +import static org.folio.rest.util.TestConfig.initSpringContext; +import static org.folio.rest.util.TestConfig.isVerticleNotDeployed; +import static org.folio.rest.util.TestEntities.FINANCE_DATA; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.core.Is.is; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.when; + +import io.vertx.core.Context; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; +import org.folio.ApiTestSuite; +import org.folio.config.ApplicationConfig; +import org.folio.rest.core.models.RequestContext; +import org.folio.rest.jaxrs.model.FyFinanceData; +import org.folio.rest.jaxrs.model.FyFinanceDataCollection; +import org.folio.rest.jaxrs.resource.FinanceFinanceData; +import org.folio.rest.util.HelperUtils; +import org.folio.rest.util.RestTestUtils; +import org.folio.services.financedata.FinanceDataService; +import org.folio.services.protection.AcqUnitsService; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; + +class FinanceDataApiTest { + + @Autowired + private FinanceDataService financeDataService; + @Autowired + private AcqUnitsService acqUnitsService; + + private Context vertxContext; + private Map okapiHeaders; + private static boolean runningOnOwn; + + @BeforeAll + static void before() throws InterruptedException, ExecutionException, TimeoutException { + if (isVerticleNotDeployed()) { + ApiTestSuite.before(); + runningOnOwn = true; + } + initSpringContext(ApplicationConfig.class); + } + + @BeforeEach + void setUp() { + autowireDependencies(this); + vertxContext = mock(Context.class); + okapiHeaders = mock(Map.class); + } + + @AfterEach + void resetMocks() { + reset(financeDataService); + reset(acqUnitsService); + } + + @AfterAll + static void after() { + if (runningOnOwn) { + ApiTestSuite.after(); + } + clearVertxContext(); + } + + @Test + void testGetFinanceFinanceDataSuccess() { + var fiscalYearId = "123e4567-e89b-12d3-a456-426614174004"; + var getFinanceDataWithFiscalYearId = FINANCE_DATA.getEndpoint() + "?query=fiscalYearId==" + fiscalYearId + "&offset=0&limit=10"; + + addMockEntry(FINANCE_DATA.name(), FINANCE_DATA.getMockObject()); +// when(acqUnitsService.buildAcqUnitsCqlClause(any())) +// .thenReturn(io.vertx.core.Future.succeededFuture("")); + when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))); + var financeData = RestTestUtils.verifyGet(FINANCE_DATA.getEndpoint(), APPLICATION_JSON, OK.getStatusCode()) + .as(FyFinanceData.class); + + assertThat(fiscalYearId, equalTo(financeData.getFiscalYearId())); + } + +// @Test +// void getFinanceFinanceDataReturnsEmptyCollectionWhenNoData() { +// var fiscalYearId = "123e4567-e89b-12d3-a456-426614174004"; +// var getFinanceDataWithFiscalYearId = FINANCE_DATA.getEndpoint() + "?query=fiscalYearId==" + fiscalYearId + "&offset=0&limit=10"; +// +// var financeData = RestTestUtils.verifyGet(FINANCE_DATA.getEndpoint(), APPLICATION_JSON, OK.getStatusCode()) +// .as(FyFinanceDataCollection.class); +// +// assertThat(financeData.getTotalRecords(), is(0)); +// assertThat(financeData.getFyFinanceData(), is(empty())); +// } +// +// @Test +// void testGetFinanceFinanceDataFailure() { +// String query = "query"; +// int offset = 0; +// int limit = 10; +// Throwable throwable = new RuntimeException("Error"); +// +// when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))) +// .thenReturn(io.vertx.core.Future.failedFuture(throwable)); +// +// Handler> asyncResultHandler = asyncResult -> { +// assertThat(asyncResult.failed(), equalTo(true)); +// assertThat(asyncResult.cause(), equalTo(throwable)); +// }; +// +//// financeDataApi.getFinanceFinanceData(query, null, offset, limit, okapiHeaders, asyncResultHandler, vertxContext); +// +// verify(financeDataService).getFinanceDataWithAcqUnitsRestriction(query, offset, limit, new RequestContext(vertxContext, okapiHeaders)); +// } + + @Test + void testGetCollectionGffyNotFound() { + var collection = verifyGet( + HelperUtils.getEndpoint(FinanceFinanceData.class) + RestTestUtils.buildQueryParam("id==(" + UUID.randomUUID() + ")"), + APPLICATION_JSON, NOT_FOUND.getStatusCode()).as(FyFinanceDataCollection.class); + + assertThat(collection.getTotalRecords(), is(0)); + assertThat(collection.getTotalRecords(), is(0)); + } + + static class ContextConfiguration { + + @Bean + public FinanceDataService financeDataService() { + return mock(FinanceDataService.class); + } + + @Bean + public AcqUnitsService acqUnitsService() { + return mock(AcqUnitsService.class); + } + } +} diff --git a/src/test/java/org/folio/rest/util/MockServer.java b/src/test/java/org/folio/rest/util/MockServer.java index 23f9e84f..03acc42e 100644 --- a/src/test/java/org/folio/rest/util/MockServer.java +++ b/src/test/java/org/folio/rest/util/MockServer.java @@ -19,6 +19,7 @@ import static org.folio.rest.util.ResourcePathResolver.BUDGETS_STORAGE; import static org.folio.rest.util.ResourcePathResolver.CONFIGURATIONS; import static org.folio.rest.util.ResourcePathResolver.EXPENSE_CLASSES_STORAGE_URL; +import static org.folio.rest.util.ResourcePathResolver.FINANCE_DATA_STORAGE; import static org.folio.rest.util.ResourcePathResolver.FISCAL_YEARS_STORAGE; import static org.folio.rest.util.ResourcePathResolver.FUNDS_STORAGE; import static org.folio.rest.util.ResourcePathResolver.FUND_TYPES; @@ -72,6 +73,8 @@ import org.folio.rest.jaxrs.model.FundUpdateLog; import org.folio.rest.jaxrs.model.FundUpdateLogCollection; import org.folio.rest.jaxrs.model.FundsCollection; +import org.folio.rest.jaxrs.model.FyFinanceData; +import org.folio.rest.jaxrs.model.FyFinanceDataCollection; import org.folio.rest.jaxrs.model.Group; import org.folio.rest.jaxrs.model.GroupCollection; import org.folio.rest.jaxrs.model.GroupFundFiscalYear; @@ -193,6 +196,8 @@ private Router defineRoutes() { router.route(HttpMethod.GET, resourcesPath(BUDGETS_STORAGE)) .handler(ctx -> handleGetCollection(ctx, TestEntities.BUDGET)); + router.route(HttpMethod.GET, resourcesPath(FINANCE_DATA_STORAGE)) + .handler(ctx -> handleGetCollection(ctx, TestEntities.FINANCE_DATA)); router.route(HttpMethod.GET, resourcesPath(FUNDS_STORAGE)) .handler(ctx -> handleGetCollection(ctx, TestEntities.FUND)); router.route(HttpMethod.GET, resourcesPath(FISCAL_YEARS_STORAGE)) @@ -466,6 +471,34 @@ record = funds.get(0); return JsonObject.mapFrom(record); } + private JsonObject getFinanceDataByIds(List ids, boolean isCollection) { + Supplier> getFromFile = () -> { + try { + return new JsonObject(getMockData(TestEntities.FINANCE_DATA.getPathToFileWithData())).mapTo( + FyFinanceDataCollection.class).getFyFinanceData(); + } catch (IOException e) { + return Collections.emptyList(); + } + }; + + List records = getMockEntries(TestEntities.FINANCE_DATA.name(), FyFinanceData.class).orElseGet(getFromFile); + + if (!ids.isEmpty()) { + records.removeIf(item -> !ids.contains(item.getFiscalYearId())); + } + + Object record; + if (isCollection) { + record = new FyFinanceDataCollection().withFyFinanceData(records).withTotalRecords(records.size()); + } else if (!records.isEmpty()) { + record = records.get(0); + } else { + return null; + } + + return JsonObject.mapFrom(record); + } + private JsonObject getFiscalYearsByIds(List fiscalYearIds, boolean isCollection) { Supplier> getFromFile = () -> { try { @@ -675,6 +708,7 @@ private JsonObject getMockRecord(TestEntities testEntity, String id) { private JsonObject getEntries(TestEntities testEntity, List ids, boolean isCollection) { return switch (testEntity) { case BUDGET -> getBudgetsByIds(ids, isCollection); + case FINANCE_DATA -> getFinanceDataByIds(ids, isCollection); case FUND -> getFundsByIds(ids, isCollection); case FISCAL_YEAR -> getFiscalYearsByIds(ids, isCollection); case FUND_TYPE -> getFundTypesByIds(ids, isCollection); diff --git a/src/test/java/org/folio/rest/util/TestEntities.java b/src/test/java/org/folio/rest/util/TestEntities.java index 72600478..dc34300a 100644 --- a/src/test/java/org/folio/rest/util/TestEntities.java +++ b/src/test/java/org/folio/rest/util/TestEntities.java @@ -12,6 +12,7 @@ import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.jaxrs.model.FundType; import org.folio.rest.jaxrs.model.FundUpdateLog; +import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.Group; import org.folio.rest.jaxrs.model.GroupFundFiscalYear; import org.folio.rest.jaxrs.model.Ledger; @@ -24,6 +25,7 @@ import org.folio.rest.jaxrs.resource.Finance; import org.folio.rest.jaxrs.resource.FinanceBudgets; import org.folio.rest.jaxrs.resource.FinanceExpenseClasses; +import org.folio.rest.jaxrs.resource.FinanceFinanceData; import org.folio.rest.jaxrs.resource.FinanceFiscalYears; import org.folio.rest.jaxrs.resource.FinanceFundTypes; import org.folio.rest.jaxrs.resource.FinanceFundUpdateLogs; @@ -42,6 +44,7 @@ public enum TestEntities { BUDGET("budgets", getEndpoint(FinanceBudgets.class), Budget.class, "mockdata/budgets/budgets.json", "budgets[0]", "name", "Updated name", 1, "allocated"), + FINANCE_DATA("financeData", getEndpoint(FinanceFinanceData.class), FyFinanceData.class, "mockdata/financedata/fy-finance-data.json", "financeData[0]", "", "", 1), FUND("funds", getEndpoint(FinanceFunds.class), Fund.class, "mockdata/funds/funds.json", "funds[0]", "name", "History", 1), FUND_TYPE("fundTypes", getEndpoint(FinanceFundTypes.class), FundType.class, "mockdata/fund-types/types.json", "fundTypes[0]", "name", "New type name", 1), FUND_UPDATE_LOG("fundUpdateLogs", getEndpoint(FinanceFundUpdateLogs.class), FundUpdateLog.class, "mockdata/fund-update-logs/fund-update-logs.json", "fundUpdateLogs[0]", "jobName", "Yearly Update", 1), diff --git a/src/test/resources/mockdata/financedata/fy-finance-data.json b/src/test/resources/mockdata/financedata/fy-finance-data.json new file mode 100644 index 00000000..a9235d6e --- /dev/null +++ b/src/test/resources/mockdata/financedata/fy-finance-data.json @@ -0,0 +1,24 @@ +{ + "fyFinanceData": [ + { + "fiscalYearId": "123e4567-e89b-12d3-a456-426614174004", + "fiscalYearCode": "FY2023", + "fundId": "123e4567-e89b-12d3-a456-426614174000", + "fundCode": "FND001", + "fundName": "General Fund", + "fundDescription": "This fund is used for general purposes.", + "fundStatus": "Active", + "fundTags": { + "tagList": ["Education", "Research"] + }, + "budgetId": "123e4567-e89b-12d3-a456-426614174001", + "budgetName": "Annual Budget", + "budgetStatus": "Active", + "budgetInitialAllocation": 1000000, + "budgetCurrentAllocation": 950000, + "budgetAllowableExpenditure": 80, + "budgetAllowableEncumbrance": 90 + } + ], + "totalRecords": 1 +} From 3d1ae114814b91e1663cebeab66c42bcc6545940 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Mon, 18 Nov 2024 18:52:34 +0500 Subject: [PATCH 02/11] Use without acq Units restrictions --- .../financedata/FinanceDataService.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/folio/services/financedata/FinanceDataService.java b/src/main/java/org/folio/services/financedata/FinanceDataService.java index b64bbc48..bd3fb9b6 100644 --- a/src/main/java/org/folio/services/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/services/financedata/FinanceDataService.java @@ -24,19 +24,19 @@ public FinanceDataService(RestClient restClient, AcqUnitsService acqUnitsService public Future getFinanceDataWithAcqUnitsRestriction(String query, int offset, int limit, RequestContext requestContext) { - - return acqUnitsService.buildAcqUnitsCqlClause(requestContext) - .map(clause -> StringUtils.isEmpty(query) ? clause : combineCqlExpressions("and", clause, query)) - .map(effectiveQuery -> new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) - .withOffset(offset) - .withLimit(limit) - .withQuery(effectiveQuery) - ) - .compose(requestEntry -> restClient.get(requestEntry.buildEndpoint(), FyFinanceDataCollection.class, requestContext)); + return restClient.get(new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) + .withOffset(offset) + .withLimit(limit) + .withQuery(query) + .buildEndpoint(), FyFinanceDataCollection.class, requestContext); +// return acqUnitsService.buildAcqUnitsCqlClause(requestContext) +// .map(clause -> StringUtils.isEmpty(query) ? clause : combineCqlExpressions("and", clause, query)) +// .map(effectiveQuery -> new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) +// .withOffset(offset) +// .withLimit(limit) +// .withQuery(effectiveQuery) +// ) +// .compose(requestEntry -> restClient.get(requestEntry.buildEndpoint(), FyFinanceDataCollection.class, requestContext)); } } -// return restClient.get(new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) -// .withOffset(offset) -// .withLimit(limit) -// .withQuery(query) -// .buildEndpoint(), FyFinanceDataCollection.class, requestContext); + From 2ce1ef80ace672436ecc5ce71609bc0359273051 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Tue, 19 Nov 2024 19:07:18 +0500 Subject: [PATCH 03/11] [MODFIN-388] - Implement endpoint to return all finance data for FY through acqUnitRestriciton --- ramls/acq-models | 2 +- .../financedata/FinanceDataService.java | 20 ++- .../services/protection/AcqUnitConstants.java | 4 + .../services/protection/AcqUnitsService.java | 20 ++- .../rest/impl/EntitiesCrudBasicsTest.java | 19 +-- .../folio/rest/impl/FinanceDataApiTest.java | 117 +++++++----------- .../java/org/folio/rest/util/MockServer.java | 34 ----- .../org/folio/rest/util/TestEntities.java | 3 - .../financedata/FinanceDataServiceTest.java | 86 +++++++++++++ .../protection/AcqUnitsServiceTest.java | 61 ++++++++- 10 files changed, 228 insertions(+), 138 deletions(-) create mode 100644 src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java diff --git a/ramls/acq-models b/ramls/acq-models index 1652e69e..197cdd8d 160000 --- a/ramls/acq-models +++ b/ramls/acq-models @@ -1 +1 @@ -Subproject commit 1652e69e9e98499b805b698e59efd7d2a51d98b5 +Subproject commit 197cdd8d4265eb27383ebe6ba9211f0814ac69ff diff --git a/src/main/java/org/folio/services/financedata/FinanceDataService.java b/src/main/java/org/folio/services/financedata/FinanceDataService.java index bd3fb9b6..3924cfc3 100644 --- a/src/main/java/org/folio/services/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/services/financedata/FinanceDataService.java @@ -24,19 +24,17 @@ public FinanceDataService(RestClient restClient, AcqUnitsService acqUnitsService public Future getFinanceDataWithAcqUnitsRestriction(String query, int offset, int limit, RequestContext requestContext) { - return restClient.get(new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) + return acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(requestContext) + .map(clause -> StringUtils.isEmpty(query) ? clause : combineCqlExpressions("and", clause, query)) + .compose(effectiveQuery -> getFinanceData(effectiveQuery, offset, limit, requestContext)); + } + + private Future getFinanceData(String query, int offset, int limit, RequestContext requestContext) { + var requestEntry = new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) .withOffset(offset) .withLimit(limit) - .withQuery(query) - .buildEndpoint(), FyFinanceDataCollection.class, requestContext); -// return acqUnitsService.buildAcqUnitsCqlClause(requestContext) -// .map(clause -> StringUtils.isEmpty(query) ? clause : combineCqlExpressions("and", clause, query)) -// .map(effectiveQuery -> new RequestEntry(resourcesPath(FINANCE_DATA_STORAGE)) -// .withOffset(offset) -// .withLimit(limit) -// .withQuery(effectiveQuery) -// ) -// .compose(requestEntry -> restClient.get(requestEntry.buildEndpoint(), FyFinanceDataCollection.class, requestContext)); + .withQuery(query); + return restClient.get(requestEntry.buildEndpoint(), FyFinanceDataCollection.class, requestContext); } } diff --git a/src/main/java/org/folio/services/protection/AcqUnitConstants.java b/src/main/java/org/folio/services/protection/AcqUnitConstants.java index 688c3aca..6a34e1fc 100644 --- a/src/main/java/org/folio/services/protection/AcqUnitConstants.java +++ b/src/main/java/org/folio/services/protection/AcqUnitConstants.java @@ -2,7 +2,11 @@ public final class AcqUnitConstants { public static final String ACQUISITIONS_UNIT_IDS = "acqUnitIds"; + public static final String FD_FUND_ACQUISITIONS_UNIT_IDS = "fundAcqUnitIds"; // for finance data view table + public static final String FD_BUDGET_ACQUISITIONS_UNIT_IDS = "budgetAcqUnitIds"; // for finance data view table public static final String NO_ACQ_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not " + ACQUISITIONS_UNIT_IDS + " <> []"; + public static final String FD_NO_ACQ_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not " + + FD_BUDGET_ACQUISITIONS_UNIT_IDS + " <> []" + FD_FUND_ACQUISITIONS_UNIT_IDS + " <> []"; public static final String IS_DELETED_PROP = "isDeleted"; public static final String ALL_UNITS_CQL = IS_DELETED_PROP + "=*"; public static final String ACTIVE_UNITS_CQL = IS_DELETED_PROP + "==false"; diff --git a/src/main/java/org/folio/services/protection/AcqUnitsService.java b/src/main/java/org/folio/services/protection/AcqUnitsService.java index 76a6b4a1..35d8dbfa 100644 --- a/src/main/java/org/folio/services/protection/AcqUnitsService.java +++ b/src/main/java/org/folio/services/protection/AcqUnitsService.java @@ -7,6 +7,9 @@ import static org.folio.rest.util.ResourcePathResolver.resourcesPath; import static org.folio.services.protection.AcqUnitConstants.ACQUISITIONS_UNIT_IDS; import static org.folio.services.protection.AcqUnitConstants.ACTIVE_UNITS_CQL; +import static org.folio.services.protection.AcqUnitConstants.FD_BUDGET_ACQUISITIONS_UNIT_IDS; +import static org.folio.services.protection.AcqUnitConstants.FD_FUND_ACQUISITIONS_UNIT_IDS; +import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; import static org.folio.services.protection.AcqUnitConstants.IS_DELETED_PROP; import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; @@ -60,7 +63,22 @@ public Future buildAcqUnitsCqlClause(RequestContext requestContext) { if (ids.isEmpty()) { return NO_ACQ_UNIT_ASSIGNED_CQL; } - return String.format("%s or (%s)", convertIdsToCqlQuery(ids, ACQUISITIONS_UNIT_IDS, false), NO_ACQ_UNIT_ASSIGNED_CQL); + return String.format("%s or (%s)", + convertIdsToCqlQuery(ids, ACQUISITIONS_UNIT_IDS, false), + NO_ACQ_UNIT_ASSIGNED_CQL); + }); + } + + public Future buildAcqUnitsCqlClauseForFinanceData(RequestContext requestContext) { + return getAcqUnitIdsForSearch(requestContext) + .map(ids -> { + if (ids.isEmpty()) { + return NO_ACQ_UNIT_ASSIGNED_CQL; + } + return String.format("(%s and %s) or (%s)", + convertIdsToCqlQuery(ids, FD_FUND_ACQUISITIONS_UNIT_IDS, false), + convertIdsToCqlQuery(ids, FD_BUDGET_ACQUISITIONS_UNIT_IDS, false), + FD_NO_ACQ_UNIT_ASSIGNED_CQL); }); } diff --git a/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java b/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java index 185b0831..9e84afbf 100644 --- a/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java +++ b/src/test/java/org/folio/rest/impl/EntitiesCrudBasicsTest.java @@ -20,7 +20,6 @@ import static org.folio.rest.util.TestConstants.TOTAL_RECORDS; import static org.folio.rest.util.TestConstants.VALID_UUID; import static org.folio.rest.util.TestEntities.BUDGET; -import static org.folio.rest.util.TestEntities.FINANCE_DATA; import static org.folio.rest.util.TestEntities.FISCAL_YEAR; import static org.folio.rest.util.TestEntities.FUND; import static org.folio.rest.util.TestEntities.GROUP; @@ -42,7 +41,6 @@ import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsEqual.equalTo; -import java.io.IOException; import java.util.Arrays; import java.util.List; import java.util.Optional; @@ -129,16 +127,14 @@ static Stream getTestEntitiesWithGetEndpointWithoutGroup() { */ static Stream getTestEntitiesWithGetByIdEndpoint() { return getTestEntitiesWithGetEndpoint() - .filter(e -> !e.equals(GROUP_FUND_FISCAL_YEAR)) - .filter(e -> !e.equals(FINANCE_DATA)); + .filter(e -> !e.equals(GROUP_FUND_FISCAL_YEAR)); } static Stream getTestEntitiesWithPostEndpoint() { return getTestEntities() .filter(e -> !e.equals(TRANSACTIONS)) .filter(e -> !e.equals(TRANSACTIONS_ALLOCATION)) - .filter(e -> !e.equals(TRANSACTIONS_TRANSFER)) - .filter(e -> !e.equals(FINANCE_DATA)); + .filter(e -> !e.equals(TRANSACTIONS_TRANSFER)); } /** @@ -148,8 +144,7 @@ static Stream getTestEntitiesWithPostEndpoint() { */ static Stream getTestEntitiesWithPutEndpoint() { return getTestEntitiesWithGetByIdEndpoint() - .filter(e -> !e.equals(TRANSACTIONS)) - .filter(e -> !e.equals(FINANCE_DATA)); + .filter(e -> !e.equals(TRANSACTIONS)); } /** @@ -158,9 +153,7 @@ static Stream getTestEntitiesWithPutEndpoint() { * @return stream of test entities */ static Stream getTestEntitiesWithDeleteEndpoint() { - return getTestEntitiesWithGetEndpoint() - .filter(e -> !e.equals(TRANSACTIONS)) - .filter(e -> !e.equals(FINANCE_DATA)); + return getTestEntitiesWithGetEndpoint().filter(e -> !e.equals(TRANSACTIONS)); } /** @@ -271,7 +264,7 @@ void testGetRecordByIdNotFound(TestEntities testEntity) { @ParameterizedTest @MethodSource("getTestEntitiesWithPostEndpoint") - void testPostRecord(TestEntities testEntity) throws IOException { + void testPostRecord(TestEntities testEntity) { logger.info("=== Test create {} record ===", testEntity.name()); JsonObject record = testEntity.getMockObject(); @@ -299,7 +292,7 @@ record = JsonObject.mapFrom(t); @ParameterizedTest @MethodSource("getTestEntitiesWithPostEndpoint") - void testPostRecordServerError(TestEntities testEntity) throws IOException { + void testPostRecordServerError(TestEntities testEntity) { logger.info("=== Test create {} record - Internal Server Error ===", testEntity.name()); Headers headers = RestTestUtils.prepareHeaders(TestConfig.X_OKAPI_URL, ERROR_X_OKAPI_TENANT); diff --git a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java index be27fc15..ecfb5314 100644 --- a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java +++ b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java @@ -1,17 +1,19 @@ package org.folio.rest.impl; +import static io.vertx.core.Future.succeededFuture; import static javax.ws.rs.core.MediaType.APPLICATION_JSON; -import static javax.ws.rs.core.Response.Status.NOT_FOUND; +import static javax.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR; import static javax.ws.rs.core.Response.Status.OK; -import static org.folio.rest.util.MockServer.addMockEntry; +import static org.folio.rest.util.ErrorCodes.GENERIC_ERROR_CODE; import static org.folio.rest.util.RestTestUtils.verifyGet; import static org.folio.rest.util.TestConfig.autowireDependencies; import static org.folio.rest.util.TestConfig.clearVertxContext; import static org.folio.rest.util.TestConfig.initSpringContext; import static org.folio.rest.util.TestConfig.isVerticleNotDeployed; -import static org.folio.rest.util.TestEntities.FINANCE_DATA; +import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.Is.is; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; @@ -19,18 +21,17 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; -import io.vertx.core.Context; -import java.util.Map; -import java.util.UUID; +import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; + +import io.vertx.core.Future; +import io.vertx.ext.web.handler.HttpException; import org.folio.ApiTestSuite; -import org.folio.config.ApplicationConfig; import org.folio.rest.core.models.RequestContext; +import org.folio.rest.jaxrs.model.Errors; import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.FyFinanceDataCollection; -import org.folio.rest.jaxrs.resource.FinanceFinanceData; -import org.folio.rest.util.HelperUtils; import org.folio.rest.util.RestTestUtils; import org.folio.services.financedata.FinanceDataService; import org.folio.services.protection.AcqUnitsService; @@ -42,102 +43,71 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; -class FinanceDataApiTest { +public class FinanceDataApiTest { @Autowired - private FinanceDataService financeDataService; + public FinanceDataService financeDataService; @Autowired - private AcqUnitsService acqUnitsService; + public AcqUnitsService acqUnitsService; - private Context vertxContext; - private Map okapiHeaders; private static boolean runningOnOwn; + private static final String FINANCE_DATA_ENDPOINT = "/finance-storage/finance-data"; @BeforeAll - static void before() throws InterruptedException, ExecutionException, TimeoutException { + static void init() throws InterruptedException, ExecutionException, TimeoutException { if (isVerticleNotDeployed()) { ApiTestSuite.before(); runningOnOwn = true; } - initSpringContext(ApplicationConfig.class); + initSpringContext(FinanceDataApiTest.ContextConfiguration.class); } @BeforeEach - void setUp() { + void beforeEach() { autowireDependencies(this); - vertxContext = mock(Context.class); - okapiHeaders = mock(Map.class); - } - - @AfterEach - void resetMocks() { - reset(financeDataService); - reset(acqUnitsService); } @AfterAll - static void after() { + static void afterAll() { if (runningOnOwn) { ApiTestSuite.after(); } clearVertxContext(); } + @AfterEach + void resetMocks() { + reset(financeDataService); + reset(acqUnitsService); + } + @Test void testGetFinanceFinanceDataSuccess() { var fiscalYearId = "123e4567-e89b-12d3-a456-426614174004"; - var getFinanceDataWithFiscalYearId = FINANCE_DATA.getEndpoint() + "?query=fiscalYearId==" + fiscalYearId + "&offset=0&limit=10"; + var financeDataCollection = new FyFinanceDataCollection() + .withFyFinanceData(List.of(new FyFinanceData().withFiscalYearId(fiscalYearId))) + .withTotalRecords(1); - addMockEntry(FINANCE_DATA.name(), FINANCE_DATA.getMockObject()); -// when(acqUnitsService.buildAcqUnitsCqlClause(any())) -// .thenReturn(io.vertx.core.Future.succeededFuture("")); - when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))); - var financeData = RestTestUtils.verifyGet(FINANCE_DATA.getEndpoint(), APPLICATION_JSON, OK.getStatusCode()) - .as(FyFinanceData.class); + when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))) + .thenReturn(succeededFuture(financeDataCollection)); + when(acqUnitsService.buildAcqUnitsCqlClause(any())).thenReturn(succeededFuture(NO_ACQ_UNIT_ASSIGNED_CQL)); + var actualFinanceDataCollection = RestTestUtils.verifyGet(FINANCE_DATA_ENDPOINT, APPLICATION_JSON, OK.getStatusCode()) + .as(FyFinanceDataCollection.class); - assertThat(fiscalYearId, equalTo(financeData.getFiscalYearId())); + assertThat(fiscalYearId, equalTo(actualFinanceDataCollection.getFyFinanceData().get(0).getFiscalYearId())); } -// @Test -// void getFinanceFinanceDataReturnsEmptyCollectionWhenNoData() { -// var fiscalYearId = "123e4567-e89b-12d3-a456-426614174004"; -// var getFinanceDataWithFiscalYearId = FINANCE_DATA.getEndpoint() + "?query=fiscalYearId==" + fiscalYearId + "&offset=0&limit=10"; -// -// var financeData = RestTestUtils.verifyGet(FINANCE_DATA.getEndpoint(), APPLICATION_JSON, OK.getStatusCode()) -// .as(FyFinanceDataCollection.class); -// -// assertThat(financeData.getTotalRecords(), is(0)); -// assertThat(financeData.getFyFinanceData(), is(empty())); -// } -// -// @Test -// void testGetFinanceFinanceDataFailure() { -// String query = "query"; -// int offset = 0; -// int limit = 10; -// Throwable throwable = new RuntimeException("Error"); -// -// when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))) -// .thenReturn(io.vertx.core.Future.failedFuture(throwable)); -// -// Handler> asyncResultHandler = asyncResult -> { -// assertThat(asyncResult.failed(), equalTo(true)); -// assertThat(asyncResult.cause(), equalTo(throwable)); -// }; -// -//// financeDataApi.getFinanceFinanceData(query, null, offset, limit, okapiHeaders, asyncResultHandler, vertxContext); -// -// verify(financeDataService).getFinanceDataWithAcqUnitsRestriction(query, offset, limit, new RequestContext(vertxContext, okapiHeaders)); -// } - @Test - void testGetCollectionGffyNotFound() { - var collection = verifyGet( - HelperUtils.getEndpoint(FinanceFinanceData.class) + RestTestUtils.buildQueryParam("id==(" + UUID.randomUUID() + ")"), - APPLICATION_JSON, NOT_FOUND.getStatusCode()).as(FyFinanceDataCollection.class); + void testGetFinanceFinanceDataFailure() { + Future financeDataFuture = Future.failedFuture(new HttpException(500, INTERNAL_SERVER_ERROR.getReasonPhrase())); + + when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))) + .thenReturn(financeDataFuture); - assertThat(collection.getTotalRecords(), is(0)); - assertThat(collection.getTotalRecords(), is(0)); + var errors = verifyGet(FINANCE_DATA_ENDPOINT, APPLICATION_JSON, INTERNAL_SERVER_ERROR.getStatusCode()).as(Errors.class); + + assertThat(errors.getErrors(), hasSize(1)); + assertThat(errors.getErrors().get(0).getCode(), is(GENERIC_ERROR_CODE.getCode())); } static class ContextConfiguration { @@ -147,8 +117,7 @@ public FinanceDataService financeDataService() { return mock(FinanceDataService.class); } - @Bean - public AcqUnitsService acqUnitsService() { + @Bean AcqUnitsService acqUnitsService() { return mock(AcqUnitsService.class); } } diff --git a/src/test/java/org/folio/rest/util/MockServer.java b/src/test/java/org/folio/rest/util/MockServer.java index 03acc42e..23f9e84f 100644 --- a/src/test/java/org/folio/rest/util/MockServer.java +++ b/src/test/java/org/folio/rest/util/MockServer.java @@ -19,7 +19,6 @@ import static org.folio.rest.util.ResourcePathResolver.BUDGETS_STORAGE; import static org.folio.rest.util.ResourcePathResolver.CONFIGURATIONS; import static org.folio.rest.util.ResourcePathResolver.EXPENSE_CLASSES_STORAGE_URL; -import static org.folio.rest.util.ResourcePathResolver.FINANCE_DATA_STORAGE; import static org.folio.rest.util.ResourcePathResolver.FISCAL_YEARS_STORAGE; import static org.folio.rest.util.ResourcePathResolver.FUNDS_STORAGE; import static org.folio.rest.util.ResourcePathResolver.FUND_TYPES; @@ -73,8 +72,6 @@ import org.folio.rest.jaxrs.model.FundUpdateLog; import org.folio.rest.jaxrs.model.FundUpdateLogCollection; import org.folio.rest.jaxrs.model.FundsCollection; -import org.folio.rest.jaxrs.model.FyFinanceData; -import org.folio.rest.jaxrs.model.FyFinanceDataCollection; import org.folio.rest.jaxrs.model.Group; import org.folio.rest.jaxrs.model.GroupCollection; import org.folio.rest.jaxrs.model.GroupFundFiscalYear; @@ -196,8 +193,6 @@ private Router defineRoutes() { router.route(HttpMethod.GET, resourcesPath(BUDGETS_STORAGE)) .handler(ctx -> handleGetCollection(ctx, TestEntities.BUDGET)); - router.route(HttpMethod.GET, resourcesPath(FINANCE_DATA_STORAGE)) - .handler(ctx -> handleGetCollection(ctx, TestEntities.FINANCE_DATA)); router.route(HttpMethod.GET, resourcesPath(FUNDS_STORAGE)) .handler(ctx -> handleGetCollection(ctx, TestEntities.FUND)); router.route(HttpMethod.GET, resourcesPath(FISCAL_YEARS_STORAGE)) @@ -471,34 +466,6 @@ record = funds.get(0); return JsonObject.mapFrom(record); } - private JsonObject getFinanceDataByIds(List ids, boolean isCollection) { - Supplier> getFromFile = () -> { - try { - return new JsonObject(getMockData(TestEntities.FINANCE_DATA.getPathToFileWithData())).mapTo( - FyFinanceDataCollection.class).getFyFinanceData(); - } catch (IOException e) { - return Collections.emptyList(); - } - }; - - List records = getMockEntries(TestEntities.FINANCE_DATA.name(), FyFinanceData.class).orElseGet(getFromFile); - - if (!ids.isEmpty()) { - records.removeIf(item -> !ids.contains(item.getFiscalYearId())); - } - - Object record; - if (isCollection) { - record = new FyFinanceDataCollection().withFyFinanceData(records).withTotalRecords(records.size()); - } else if (!records.isEmpty()) { - record = records.get(0); - } else { - return null; - } - - return JsonObject.mapFrom(record); - } - private JsonObject getFiscalYearsByIds(List fiscalYearIds, boolean isCollection) { Supplier> getFromFile = () -> { try { @@ -708,7 +675,6 @@ private JsonObject getMockRecord(TestEntities testEntity, String id) { private JsonObject getEntries(TestEntities testEntity, List ids, boolean isCollection) { return switch (testEntity) { case BUDGET -> getBudgetsByIds(ids, isCollection); - case FINANCE_DATA -> getFinanceDataByIds(ids, isCollection); case FUND -> getFundsByIds(ids, isCollection); case FISCAL_YEAR -> getFiscalYearsByIds(ids, isCollection); case FUND_TYPE -> getFundTypesByIds(ids, isCollection); diff --git a/src/test/java/org/folio/rest/util/TestEntities.java b/src/test/java/org/folio/rest/util/TestEntities.java index dc34300a..72600478 100644 --- a/src/test/java/org/folio/rest/util/TestEntities.java +++ b/src/test/java/org/folio/rest/util/TestEntities.java @@ -12,7 +12,6 @@ import org.folio.rest.jaxrs.model.Fund; import org.folio.rest.jaxrs.model.FundType; import org.folio.rest.jaxrs.model.FundUpdateLog; -import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.Group; import org.folio.rest.jaxrs.model.GroupFundFiscalYear; import org.folio.rest.jaxrs.model.Ledger; @@ -25,7 +24,6 @@ import org.folio.rest.jaxrs.resource.Finance; import org.folio.rest.jaxrs.resource.FinanceBudgets; import org.folio.rest.jaxrs.resource.FinanceExpenseClasses; -import org.folio.rest.jaxrs.resource.FinanceFinanceData; import org.folio.rest.jaxrs.resource.FinanceFiscalYears; import org.folio.rest.jaxrs.resource.FinanceFundTypes; import org.folio.rest.jaxrs.resource.FinanceFundUpdateLogs; @@ -44,7 +42,6 @@ public enum TestEntities { BUDGET("budgets", getEndpoint(FinanceBudgets.class), Budget.class, "mockdata/budgets/budgets.json", "budgets[0]", "name", "Updated name", 1, "allocated"), - FINANCE_DATA("financeData", getEndpoint(FinanceFinanceData.class), FyFinanceData.class, "mockdata/financedata/fy-finance-data.json", "financeData[0]", "", "", 1), FUND("funds", getEndpoint(FinanceFunds.class), Fund.class, "mockdata/funds/funds.json", "funds[0]", "name", "History", 1), FUND_TYPE("fundTypes", getEndpoint(FinanceFundTypes.class), FundType.class, "mockdata/fund-types/types.json", "fundTypes[0]", "name", "New type name", 1), FUND_UPDATE_LOG("fundUpdateLogs", getEndpoint(FinanceFundUpdateLogs.class), FundUpdateLog.class, "mockdata/fund-update-logs/fund-update-logs.json", "fundUpdateLogs[0]", "jobName", "Yearly Update", 1), diff --git a/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java b/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java new file mode 100644 index 00000000..91374817 --- /dev/null +++ b/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java @@ -0,0 +1,86 @@ +package org.folio.services.financedata; + +import static io.vertx.core.Future.succeededFuture; +import static org.folio.rest.util.TestUtils.assertQueryContains; +import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.HashMap; +import java.util.Map; + +import org.folio.rest.core.RestClient; +import org.folio.rest.core.models.RequestContext; +import org.folio.rest.jaxrs.model.FyFinanceDataCollection; +import org.folio.services.protection.AcqUnitsService; +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.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import io.vertx.core.Context; +import io.vertx.core.Vertx; +import io.vertx.junit5.VertxExtension; +import io.vertx.junit5.VertxTestContext; + +@ExtendWith(VertxExtension.class) +public class FinanceDataServiceTest { + + @InjectMocks + private FinanceDataService financeDataService; + + @Mock + private RestClient restClient; + + @Mock + private AcqUnitsService acqUnitsService; + + private RequestContext requestContextMock; + private AutoCloseable closeable; + + @BeforeEach + void initMocks() { + closeable = MockitoAnnotations.openMocks(this); + Context context = Vertx.vertx().getOrCreateContext(); + Map okapiHeaders = new HashMap<>(); + okapiHeaders.put("x-okapi-url", "http://localhost:9130"); // Ensure this URL is correct + okapiHeaders.put("x-okapi-token", "token"); + okapiHeaders.put("x-okapi-tenant", "tenant"); + okapiHeaders.put("x-okapi-user-id", "userId"); + requestContextMock = new RequestContext(context, okapiHeaders); + } + + @AfterEach + void closeMocks() throws Exception { + closeable.close(); + } + + @Test + void shouldCallGetForRestClientWhenCalledGetFinanceDataWithAcqUnitsRestriction(VertxTestContext vertxTestContext) { + String query = "fiscalYearId==db9c1ad6-026e-4b1a-9a99-032f41e7099b"; + String acqUnitIdsQuery = "fundAcqUnitIds=(1ee4b4e5-d621-4e43-8a76-0d904b0f491b) and budgetAcqUnitIds=(1ee4b4e5-d621-4e43-8a76-0d904b0f491b) or (" + NO_ACQ_UNIT_ASSIGNED_CQL + ")"; + String expectedQuery = "(" + acqUnitIdsQuery + ") and (" + query + ")"; + int offset = 0; + int limit = 10; + FyFinanceDataCollection fyFinanceDataCollection = new FyFinanceDataCollection(); + + when(acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(any())).thenReturn(succeededFuture(acqUnitIdsQuery)); + when(restClient.get(anyString(), eq(FyFinanceDataCollection.class), any())).thenReturn(succeededFuture(fyFinanceDataCollection)); + + var future = financeDataService.getFinanceDataWithAcqUnitsRestriction(query, offset, limit, requestContextMock); + vertxTestContext.assertComplete(future) + .onComplete(result -> { + assertTrue(result.succeeded()); + verify(restClient).get(assertQueryContains(expectedQuery), eq(FyFinanceDataCollection.class), eq(requestContextMock)); + vertxTestContext.completeNow(); + }); + } + +} diff --git a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java index 705a90df..10fe2e36 100644 --- a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java +++ b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java @@ -29,6 +29,7 @@ import org.folio.rest.acq.model.finance.AcquisitionsUnitMembershipCollection; import org.folio.rest.core.RestClient; import org.folio.rest.core.models.RequestContext; +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; @@ -52,9 +53,11 @@ public class AcqUnitsServiceTest { @Mock private RestClient restClient; + private AutoCloseable closeable; + @BeforeEach public void initMocks() { - MockitoAnnotations.openMocks(this); + closeable = MockitoAnnotations.openMocks(this); Context context = Vertx.vertx().getOrCreateContext(); Map okapiHeaders = new HashMap<>(); okapiHeaders.put(OKAPI_URL, "http://localhost:" + mockPort); @@ -64,6 +67,11 @@ public void initMocks() { requestContext = new RequestContext(context, okapiHeaders); } + @AfterEach + void clearContext() throws Exception { + closeable.close(); + } + @Test void testShouldUseRestClientWhenRetrieveAcqUnitByNotEmptyQuery(VertxTestContext vertxTestContext) { //Given @@ -164,4 +172,55 @@ void testShouldBuildCqlClauseWhenIdsIsNotEmpty(VertxTestContext vertxTestContext }); } + + @Test + void testShouldBuildCqlClauseForFinanceDataWhenIdsIsEmpty(VertxTestContext vertxTestContext) { + var units = new AcquisitionsUnitCollection() + .withAcquisitionsUnits(Collections.emptyList()).withTotalRecords(0); + var members = new AcquisitionsUnitMembershipCollection() + .withAcquisitionsUnitMemberships(Collections.emptyList()).withTotalRecords(0); + doReturn(succeededFuture(units)).when(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); + doReturn(succeededFuture(members)).when(acqUnitMembershipsService).getAcquisitionsUnitsMemberships(anyString(), anyInt(), anyInt(), eq(requestContext)); + + var future = acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(requestContext); + + vertxTestContext.assertComplete(future) + .onComplete(result -> { + assertTrue(result.succeeded()); + + var actClause = result.result(); + assertThat(actClause, equalTo(NO_ACQ_UNIT_ASSIGNED_CQL)); + verify(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); + verify(acqUnitMembershipsService).getAcquisitionsUnitsMemberships("userId==" + X_OKAPI_USER_ID.getValue(), 0, Integer.MAX_VALUE, requestContext); + + vertxTestContext.completeNow(); + }); + } + + @Test + void testShouldBuildCqlClauseForFinanceDataWhenIdsIsNotEmpty(VertxTestContext vertxTestContext) { + var unitId = UUID.randomUUID().toString(); + var units = new AcquisitionsUnitCollection() + .withAcquisitionsUnits(Collections.emptyList()).withTotalRecords(0); + var memberId = UUID.randomUUID().toString(); + var members = new AcquisitionsUnitMembershipCollection() + .withAcquisitionsUnitMemberships(List.of(new AcquisitionsUnitMembership().withAcquisitionsUnitId(unitId).withId(memberId))) + .withTotalRecords(1); + doReturn(succeededFuture(units)).when(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); + doReturn(succeededFuture(members)).when(acqUnitMembershipsService).getAcquisitionsUnitsMemberships(anyString(), anyInt(), anyInt(), eq(requestContext)); + + var future = acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(requestContext); + + vertxTestContext.assertComplete(future) + .onComplete(result -> { + assertTrue(result.succeeded()); + + var actClause = result.result(); + assertThat(actClause, equalTo("fundAcqUnitIds=(" + unitId + ") and budgetAcqUnitIds=(" + unitId + ") or (" + NO_ACQ_UNIT_ASSIGNED_CQL + ")")); + verify(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); + verify(acqUnitMembershipsService).getAcquisitionsUnitsMemberships("userId==" + X_OKAPI_USER_ID.getValue(), 0, Integer.MAX_VALUE, requestContext); + + vertxTestContext.completeNow(); + }); + } } From 8c920345ed279e93f13c953f2850d16134fed83c Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Tue, 19 Nov 2024 19:09:17 +0500 Subject: [PATCH 04/11] Added to api test suite --- src/test/java/org/folio/ApiTestSuite.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/java/org/folio/ApiTestSuite.java b/src/test/java/org/folio/ApiTestSuite.java index 30a36f30..33e72b6c 100644 --- a/src/test/java/org/folio/ApiTestSuite.java +++ b/src/test/java/org/folio/ApiTestSuite.java @@ -12,6 +12,7 @@ import org.folio.rest.impl.EncumbrancesTest; import org.folio.rest.impl.EntitiesCrudBasicsTest; import org.folio.rest.impl.ExchangeTest; +import org.folio.rest.impl.FinanceDataApiTest; import org.folio.rest.impl.FiscalYearTest; import org.folio.rest.impl.FundCodeExpenseClassesApiTest; import org.folio.rest.impl.FundsApiTest; @@ -32,6 +33,7 @@ import org.folio.services.budget.BudgetServiceTest; import org.folio.services.budget.CreateBudgetServiceTest; import org.folio.services.budget.RecalculateBudgetServiceTest; +import org.folio.services.financedata.FinanceDataServiceTest; import org.folio.services.fiscalyear.FiscalYearApiServiceTest; import org.folio.services.fiscalyear.FiscalYearServiceTest; import org.folio.services.fund.FundCodeExpenseClassesServiceTest; @@ -244,4 +246,9 @@ class GroupServiceNested extends GroupServiceTest {} @Nested class RecalculateBudgetServiceTestNested extends RecalculateBudgetServiceTest {} + @Nested + class FinanceDataApiTestNested extends FinanceDataApiTest {} + + @Nested + class FinanceDataServiceTestNested extends FinanceDataServiceTest {} } From 727d9bf10cd85e83eabd5067874d7ae1f0baadb6 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 12:51:57 +0500 Subject: [PATCH 05/11] Fixed acqUnitsRestriction and added tests --- .../financedata/FinanceDataService.java | 2 +- .../services/protection/AcqUnitConstants.java | 4 +- .../services/protection/AcqUnitsService.java | 2 +- .../folio/rest/impl/FinanceDataApiTest.java | 41 +++++++++++++------ .../financedata/FinanceDataServiceTest.java | 25 ++++++++++- .../protection/AcqUnitsServiceTest.java | 3 +- .../mockdata/financedata/fy-finance-data.json | 24 ----------- 7 files changed, 58 insertions(+), 43 deletions(-) delete mode 100644 src/test/resources/mockdata/financedata/fy-finance-data.json diff --git a/src/main/java/org/folio/services/financedata/FinanceDataService.java b/src/main/java/org/folio/services/financedata/FinanceDataService.java index 3924cfc3..bf58308d 100644 --- a/src/main/java/org/folio/services/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/services/financedata/FinanceDataService.java @@ -23,7 +23,7 @@ public FinanceDataService(RestClient restClient, AcqUnitsService acqUnitsService } public Future getFinanceDataWithAcqUnitsRestriction(String query, int offset, int limit, - RequestContext requestContext) { + RequestContext requestContext) { return acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(requestContext) .map(clause -> StringUtils.isEmpty(query) ? clause : combineCqlExpressions("and", clause, query)) .compose(effectiveQuery -> getFinanceData(effectiveQuery, offset, limit, requestContext)); diff --git a/src/main/java/org/folio/services/protection/AcqUnitConstants.java b/src/main/java/org/folio/services/protection/AcqUnitConstants.java index 6a34e1fc..02bb059e 100644 --- a/src/main/java/org/folio/services/protection/AcqUnitConstants.java +++ b/src/main/java/org/folio/services/protection/AcqUnitConstants.java @@ -5,8 +5,8 @@ public final class AcqUnitConstants { public static final String FD_FUND_ACQUISITIONS_UNIT_IDS = "fundAcqUnitIds"; // for finance data view table public static final String FD_BUDGET_ACQUISITIONS_UNIT_IDS = "budgetAcqUnitIds"; // for finance data view table public static final String NO_ACQ_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not " + ACQUISITIONS_UNIT_IDS + " <> []"; - public static final String FD_NO_ACQ_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not " + - FD_BUDGET_ACQUISITIONS_UNIT_IDS + " <> []" + FD_FUND_ACQUISITIONS_UNIT_IDS + " <> []"; + public static final String FD_NO_ACQ_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not (" + + FD_BUDGET_ACQUISITIONS_UNIT_IDS + " <> [] and " + FD_FUND_ACQUISITIONS_UNIT_IDS + " <> [])"; public static final String IS_DELETED_PROP = "isDeleted"; public static final String ALL_UNITS_CQL = IS_DELETED_PROP + "=*"; public static final String ACTIVE_UNITS_CQL = IS_DELETED_PROP + "==false"; diff --git a/src/main/java/org/folio/services/protection/AcqUnitsService.java b/src/main/java/org/folio/services/protection/AcqUnitsService.java index 35d8dbfa..0f4e859f 100644 --- a/src/main/java/org/folio/services/protection/AcqUnitsService.java +++ b/src/main/java/org/folio/services/protection/AcqUnitsService.java @@ -73,7 +73,7 @@ public Future buildAcqUnitsCqlClauseForFinanceData(RequestContext reques return getAcqUnitIdsForSearch(requestContext) .map(ids -> { if (ids.isEmpty()) { - return NO_ACQ_UNIT_ASSIGNED_CQL; + return FD_NO_ACQ_UNIT_ASSIGNED_CQL; } return String.format("(%s and %s) or (%s)", convertIdsToCqlQuery(ids, FD_FUND_ACQUISITIONS_UNIT_IDS, false), diff --git a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java index ecfb5314..1b68e0f5 100644 --- a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java +++ b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java @@ -1,27 +1,34 @@ package org.folio.rest.impl; +import static io.vertx.core.Future.failedFuture; import static io.vertx.core.Future.succeededFuture; import static javax.ws.rs.core.MediaType.APPLICATION_JSON; import static javax.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR; import static javax.ws.rs.core.Response.Status.OK; import static org.folio.rest.util.ErrorCodes.GENERIC_ERROR_CODE; import static org.folio.rest.util.RestTestUtils.verifyGet; +import static org.folio.rest.util.RestTestUtils.verifyGetWithParam; import static org.folio.rest.util.TestConfig.autowireDependencies; import static org.folio.rest.util.TestConfig.clearVertxContext; import static org.folio.rest.util.TestConfig.initSpringContext; import static org.folio.rest.util.TestConfig.isVerticleNotDeployed; -import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; +import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.Is.is; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeoutException; @@ -32,7 +39,6 @@ import org.folio.rest.jaxrs.model.Errors; import org.folio.rest.jaxrs.model.FyFinanceData; import org.folio.rest.jaxrs.model.FyFinanceDataCollection; -import org.folio.rest.util.RestTestUtils; import org.folio.services.financedata.FinanceDataService; import org.folio.services.protection.AcqUnitsService; import org.junit.jupiter.api.AfterAll; @@ -51,7 +57,7 @@ public class FinanceDataApiTest { public AcqUnitsService acqUnitsService; private static boolean runningOnOwn; - private static final String FINANCE_DATA_ENDPOINT = "/finance-storage/finance-data"; + private static final String FINANCE_DATA_ENDPOINT = "/finance/finance-data"; @BeforeAll static void init() throws InterruptedException, ExecutionException, TimeoutException { @@ -82,24 +88,34 @@ void resetMocks() { } @Test - void testGetFinanceFinanceDataSuccess() { + void positive_testGetFinanceFinanceDataSuccess() { var fiscalYearId = "123e4567-e89b-12d3-a456-426614174004"; var financeDataCollection = new FyFinanceDataCollection() .withFyFinanceData(List.of(new FyFinanceData().withFiscalYearId(fiscalYearId))) .withTotalRecords(1); + String query = "fiscalYearId==" + fiscalYearId; + int limit = 5; + int offset = 1; - when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))) + Map params = new HashMap<>(); + params.put("query", query); + params.put("limit", limit); + params.put("offset", offset); + + when(acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(any())).thenReturn(succeededFuture(FD_NO_ACQ_UNIT_ASSIGNED_CQL)); + when(financeDataService.getFinanceDataWithAcqUnitsRestriction(anyString(), anyInt(), anyInt(), any())) .thenReturn(succeededFuture(financeDataCollection)); - when(acqUnitsService.buildAcqUnitsCqlClause(any())).thenReturn(succeededFuture(NO_ACQ_UNIT_ASSIGNED_CQL)); - var actualFinanceDataCollection = RestTestUtils.verifyGet(FINANCE_DATA_ENDPOINT, APPLICATION_JSON, OK.getStatusCode()) + + var response = verifyGetWithParam(FINANCE_DATA_ENDPOINT, APPLICATION_JSON, OK.getStatusCode(), params) .as(FyFinanceDataCollection.class); - assertThat(fiscalYearId, equalTo(actualFinanceDataCollection.getFyFinanceData().get(0).getFiscalYearId())); + assertEquals(financeDataCollection, response); + verify(financeDataService).getFinanceDataWithAcqUnitsRestriction(eq(query), eq(offset), eq(limit), any(RequestContext.class)); } @Test - void testGetFinanceFinanceDataFailure() { - Future financeDataFuture = Future.failedFuture(new HttpException(500, INTERNAL_SERVER_ERROR.getReasonPhrase())); + void negative_testGetFinanceFinanceDataFailure() { + Future financeDataFuture = failedFuture(new HttpException(500, INTERNAL_SERVER_ERROR.getReasonPhrase())); when(financeDataService.getFinanceDataWithAcqUnitsRestriction(any(), anyInt(), anyInt(), any(RequestContext.class))) .thenReturn(financeDataFuture); @@ -112,8 +128,7 @@ void testGetFinanceFinanceDataFailure() { static class ContextConfiguration { - @Bean - public FinanceDataService financeDataService() { + @Bean public FinanceDataService financeDataService() { return mock(FinanceDataService.class); } diff --git a/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java b/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java index 91374817..c7932c9b 100644 --- a/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java +++ b/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java @@ -2,7 +2,9 @@ import static io.vertx.core.Future.succeededFuture; import static org.folio.rest.util.TestUtils.assertQueryContains; +import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -63,7 +65,7 @@ void closeMocks() throws Exception { } @Test - void shouldCallGetForRestClientWhenCalledGetFinanceDataWithAcqUnitsRestriction(VertxTestContext vertxTestContext) { + void positive_shouldGetFinanceDataWithAcqUnitsRestriction(VertxTestContext vertxTestContext) { String query = "fiscalYearId==db9c1ad6-026e-4b1a-9a99-032f41e7099b"; String acqUnitIdsQuery = "fundAcqUnitIds=(1ee4b4e5-d621-4e43-8a76-0d904b0f491b) and budgetAcqUnitIds=(1ee4b4e5-d621-4e43-8a76-0d904b0f491b) or (" + NO_ACQ_UNIT_ASSIGNED_CQL + ")"; String expectedQuery = "(" + acqUnitIdsQuery + ") and (" + query + ")"; @@ -83,4 +85,25 @@ void shouldCallGetForRestClientWhenCalledGetFinanceDataWithAcqUnitsRestriction(V }); } + @Test + void negative_shouldReturnEmptyCollectionWhenFinanceDataNotFound(VertxTestContext vertxTestContext) { + String query = "fiscalYearId==non-existent-id"; + String expectedQuery = "(" + FD_NO_ACQ_UNIT_ASSIGNED_CQL + ") and (" + query + ")"; + int offset = 0; + int limit = 10; + FyFinanceDataCollection emptyCollection = new FyFinanceDataCollection().withTotalRecords(0); + + when(acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(any())).thenReturn(succeededFuture(FD_NO_ACQ_UNIT_ASSIGNED_CQL)); + when(restClient.get(anyString(), eq(FyFinanceDataCollection.class), any())).thenReturn(succeededFuture(emptyCollection)); + + var future = financeDataService.getFinanceDataWithAcqUnitsRestriction(query, offset, limit, requestContextMock); + vertxTestContext.assertComplete(future) + .onComplete(result -> { + assertTrue(result.succeeded()); + assertEquals(0, (int) result.result().getTotalRecords()); + verify(restClient).get(assertQueryContains(expectedQuery), eq(FyFinanceDataCollection.class), eq(requestContextMock)); + vertxTestContext.completeNow(); + }); + } + } diff --git a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java index 10fe2e36..bc25baf1 100644 --- a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java +++ b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java @@ -6,6 +6,7 @@ import static org.folio.rest.util.TestConstants.X_OKAPI_TENANT; import static org.folio.rest.util.TestConstants.X_OKAPI_TOKEN; import static org.folio.rest.util.TestConstants.X_OKAPI_USER_ID; +import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -216,7 +217,7 @@ void testShouldBuildCqlClauseForFinanceDataWhenIdsIsNotEmpty(VertxTestContext ve assertTrue(result.succeeded()); var actClause = result.result(); - assertThat(actClause, equalTo("fundAcqUnitIds=(" + unitId + ") and budgetAcqUnitIds=(" + unitId + ") or (" + NO_ACQ_UNIT_ASSIGNED_CQL + ")")); + assertThat(actClause, equalTo("(fundAcqUnitIds=(" + unitId + ") and budgetAcqUnitIds=(" + unitId + ")) or (" + FD_NO_ACQ_UNIT_ASSIGNED_CQL + ")")); verify(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); verify(acqUnitMembershipsService).getAcquisitionsUnitsMemberships("userId==" + X_OKAPI_USER_ID.getValue(), 0, Integer.MAX_VALUE, requestContext); diff --git a/src/test/resources/mockdata/financedata/fy-finance-data.json b/src/test/resources/mockdata/financedata/fy-finance-data.json deleted file mode 100644 index a9235d6e..00000000 --- a/src/test/resources/mockdata/financedata/fy-finance-data.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "fyFinanceData": [ - { - "fiscalYearId": "123e4567-e89b-12d3-a456-426614174004", - "fiscalYearCode": "FY2023", - "fundId": "123e4567-e89b-12d3-a456-426614174000", - "fundCode": "FND001", - "fundName": "General Fund", - "fundDescription": "This fund is used for general purposes.", - "fundStatus": "Active", - "fundTags": { - "tagList": ["Education", "Research"] - }, - "budgetId": "123e4567-e89b-12d3-a456-426614174001", - "budgetName": "Annual Budget", - "budgetStatus": "Active", - "budgetInitialAllocation": 1000000, - "budgetCurrentAllocation": 950000, - "budgetAllowableExpenditure": 80, - "budgetAllowableEncumbrance": 90 - } - ], - "totalRecords": 1 -} From a6a805cde48a75ec381fe277d2ba66f01f5471b0 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 12:52:51 +0500 Subject: [PATCH 06/11] remove unused variable --- .../java/org/folio/services/financedata/FinanceDataService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/folio/services/financedata/FinanceDataService.java b/src/main/java/org/folio/services/financedata/FinanceDataService.java index bf58308d..8d68415f 100644 --- a/src/main/java/org/folio/services/financedata/FinanceDataService.java +++ b/src/main/java/org/folio/services/financedata/FinanceDataService.java @@ -15,7 +15,6 @@ public class FinanceDataService { private final RestClient restClient; private final AcqUnitsService acqUnitsService; - public static final String ID = "id"; public FinanceDataService(RestClient restClient, AcqUnitsService acqUnitsService) { this.restClient = restClient; From e58d75c42765a37ca4a7f1ecefa8327d80c39a1e Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 12:56:35 +0500 Subject: [PATCH 07/11] fix unit test --- .../java/org/folio/services/protection/AcqUnitsServiceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java index bc25baf1..2189d388 100644 --- a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java +++ b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java @@ -190,7 +190,7 @@ void testShouldBuildCqlClauseForFinanceDataWhenIdsIsEmpty(VertxTestContext vertx assertTrue(result.succeeded()); var actClause = result.result(); - assertThat(actClause, equalTo(NO_ACQ_UNIT_ASSIGNED_CQL)); + assertThat(actClause, equalTo(FD_NO_ACQ_UNIT_ASSIGNED_CQL)); verify(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); verify(acqUnitMembershipsService).getAcquisitionsUnitsMemberships("userId==" + X_OKAPI_USER_ID.getValue(), 0, Integer.MAX_VALUE, requestContext); From 1b47fc88f4e172c03523880cc3e05bb50a4d9f8d Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 14:23:29 +0500 Subject: [PATCH 08/11] change acq-models to master --- ramls/acq-models | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ramls/acq-models b/ramls/acq-models index 197cdd8d..c06778a4 160000 --- a/ramls/acq-models +++ b/ramls/acq-models @@ -1 +1 @@ -Subproject commit 197cdd8d4265eb27383ebe6ba9211f0814ac69ff +Subproject commit c06778a484802db30023f1e5388a6d2972606ae1 From 8374c579bb255918f6f8e390f64e20dd5be51298 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 17:53:04 +0500 Subject: [PATCH 09/11] Fix acqUnitRestriction cql --- .../services/protection/AcqUnitConstants.java | 4 ++-- .../services/protection/AcqUnitsService.java | 20 ++++++++++++++----- .../folio/rest/impl/FinanceDataApiTest.java | 6 ++++-- .../financedata/FinanceDataServiceTest.java | 6 +++--- .../protection/AcqUnitsServiceTest.java | 14 ++++++++++--- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/folio/services/protection/AcqUnitConstants.java b/src/main/java/org/folio/services/protection/AcqUnitConstants.java index 02bb059e..33ff47d2 100644 --- a/src/main/java/org/folio/services/protection/AcqUnitConstants.java +++ b/src/main/java/org/folio/services/protection/AcqUnitConstants.java @@ -5,8 +5,8 @@ public final class AcqUnitConstants { public static final String FD_FUND_ACQUISITIONS_UNIT_IDS = "fundAcqUnitIds"; // for finance data view table public static final String FD_BUDGET_ACQUISITIONS_UNIT_IDS = "budgetAcqUnitIds"; // for finance data view table public static final String NO_ACQ_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not " + ACQUISITIONS_UNIT_IDS + " <> []"; - public static final String FD_NO_ACQ_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not (" + - FD_BUDGET_ACQUISITIONS_UNIT_IDS + " <> [] and " + FD_FUND_ACQUISITIONS_UNIT_IDS + " <> [])"; + public static final String NO_FD_FUND_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not " + FD_FUND_ACQUISITIONS_UNIT_IDS + " <> []"; + public static final String NO_FD_BUDGET_UNIT_ASSIGNED_CQL = "cql.allRecords=1 not " + FD_BUDGET_ACQUISITIONS_UNIT_IDS + " <> []"; public static final String IS_DELETED_PROP = "isDeleted"; public static final String ALL_UNITS_CQL = IS_DELETED_PROP + "=*"; public static final String ACTIVE_UNITS_CQL = IS_DELETED_PROP + "==false"; diff --git a/src/main/java/org/folio/services/protection/AcqUnitsService.java b/src/main/java/org/folio/services/protection/AcqUnitsService.java index 0f4e859f..6066ae04 100644 --- a/src/main/java/org/folio/services/protection/AcqUnitsService.java +++ b/src/main/java/org/folio/services/protection/AcqUnitsService.java @@ -9,9 +9,10 @@ import static org.folio.services.protection.AcqUnitConstants.ACTIVE_UNITS_CQL; import static org.folio.services.protection.AcqUnitConstants.FD_BUDGET_ACQUISITIONS_UNIT_IDS; import static org.folio.services.protection.AcqUnitConstants.FD_FUND_ACQUISITIONS_UNIT_IDS; -import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; import static org.folio.services.protection.AcqUnitConstants.IS_DELETED_PROP; import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; +import static org.folio.services.protection.AcqUnitConstants.NO_FD_BUDGET_UNIT_ASSIGNED_CQL; +import static org.folio.services.protection.AcqUnitConstants.NO_FD_FUND_UNIT_ASSIGNED_CQL; import java.util.List; import java.util.stream.Collectors; @@ -73,12 +74,21 @@ public Future buildAcqUnitsCqlClauseForFinanceData(RequestContext reques return getAcqUnitIdsForSearch(requestContext) .map(ids -> { if (ids.isEmpty()) { - return FD_NO_ACQ_UNIT_ASSIGNED_CQL; + return String.format("(%s and %s)", NO_FD_FUND_UNIT_ASSIGNED_CQL, NO_FD_BUDGET_UNIT_ASSIGNED_CQL); } - return String.format("(%s and %s) or (%s)", + return String.format("(" + + "(%s and %s) or " + // Case 1: Both fund and budget have matching acqUnits + "(%s and %s) or " + // Case 2: Fund has matching acqUnit and budget is empty + "(%s and %s) or " + // Case 3: Fund is empty and budget has matching acqUnit + "(%s and %s))", convertIdsToCqlQuery(ids, FD_FUND_ACQUISITIONS_UNIT_IDS, false), convertIdsToCqlQuery(ids, FD_BUDGET_ACQUISITIONS_UNIT_IDS, false), - FD_NO_ACQ_UNIT_ASSIGNED_CQL); + convertIdsToCqlQuery(ids, FD_FUND_ACQUISITIONS_UNIT_IDS, false), + NO_FD_BUDGET_UNIT_ASSIGNED_CQL, + NO_FD_FUND_UNIT_ASSIGNED_CQL, + convertIdsToCqlQuery(ids, FD_BUDGET_ACQUISITIONS_UNIT_IDS, false), + NO_FD_FUND_UNIT_ASSIGNED_CQL, + NO_FD_BUDGET_UNIT_ASSIGNED_CQL); }); } @@ -120,6 +130,6 @@ private Future> getOpenForReadAcqUnitIds(RequestContext requestCont log.debug("{} acq units with 'protectRead==false' are found: {}", ids.size(), StreamEx.of(ids).joining(", ")); } return ids; - }); + }); } } diff --git a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java index 1b68e0f5..b2e986d2 100644 --- a/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java +++ b/src/test/java/org/folio/rest/impl/FinanceDataApiTest.java @@ -12,7 +12,8 @@ import static org.folio.rest.util.TestConfig.clearVertxContext; import static org.folio.rest.util.TestConfig.initSpringContext; import static org.folio.rest.util.TestConfig.isVerticleNotDeployed; -import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; +import static org.folio.services.protection.AcqUnitConstants.NO_FD_BUDGET_UNIT_ASSIGNED_CQL; +import static org.folio.services.protection.AcqUnitConstants.NO_FD_FUND_UNIT_ASSIGNED_CQL; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.Is.is; @@ -94,6 +95,7 @@ void positive_testGetFinanceFinanceDataSuccess() { .withFyFinanceData(List.of(new FyFinanceData().withFiscalYearId(fiscalYearId))) .withTotalRecords(1); String query = "fiscalYearId==" + fiscalYearId; + String notAcqUnitAssignedQuery = "(" + NO_FD_FUND_UNIT_ASSIGNED_CQL + " and " + NO_FD_BUDGET_UNIT_ASSIGNED_CQL + ")"; int limit = 5; int offset = 1; @@ -102,7 +104,7 @@ void positive_testGetFinanceFinanceDataSuccess() { params.put("limit", limit); params.put("offset", offset); - when(acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(any())).thenReturn(succeededFuture(FD_NO_ACQ_UNIT_ASSIGNED_CQL)); + when(acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(any())).thenReturn(succeededFuture(notAcqUnitAssignedQuery)); when(financeDataService.getFinanceDataWithAcqUnitsRestriction(anyString(), anyInt(), anyInt(), any())) .thenReturn(succeededFuture(financeDataCollection)); diff --git a/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java b/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java index c7932c9b..136dad1b 100644 --- a/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java +++ b/src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java @@ -2,7 +2,6 @@ import static io.vertx.core.Future.succeededFuture; import static org.folio.rest.util.TestUtils.assertQueryContains; -import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -88,12 +87,13 @@ void positive_shouldGetFinanceDataWithAcqUnitsRestriction(VertxTestContext vertx @Test void negative_shouldReturnEmptyCollectionWhenFinanceDataNotFound(VertxTestContext vertxTestContext) { String query = "fiscalYearId==non-existent-id"; - String expectedQuery = "(" + FD_NO_ACQ_UNIT_ASSIGNED_CQL + ") and (" + query + ")"; + String noFdUnitAssignedCql = "cql.allRecords=1 not fundAcqUnitIds <> [] and cql.allRecords=1 not budgetAcqUnitIds <> []"; + String expectedQuery = "(" + noFdUnitAssignedCql + ") and (" + query + ")"; int offset = 0; int limit = 10; FyFinanceDataCollection emptyCollection = new FyFinanceDataCollection().withTotalRecords(0); - when(acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(any())).thenReturn(succeededFuture(FD_NO_ACQ_UNIT_ASSIGNED_CQL)); + when(acqUnitsService.buildAcqUnitsCqlClauseForFinanceData(any())).thenReturn(succeededFuture(noFdUnitAssignedCql)); when(restClient.get(anyString(), eq(FyFinanceDataCollection.class), any())).thenReturn(succeededFuture(emptyCollection)); var future = financeDataService.getFinanceDataWithAcqUnitsRestriction(query, offset, limit, requestContextMock); diff --git a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java index 2189d388..f461b8b2 100644 --- a/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java +++ b/src/test/java/org/folio/services/protection/AcqUnitsServiceTest.java @@ -6,8 +6,9 @@ import static org.folio.rest.util.TestConstants.X_OKAPI_TENANT; import static org.folio.rest.util.TestConstants.X_OKAPI_TOKEN; import static org.folio.rest.util.TestConstants.X_OKAPI_USER_ID; -import static org.folio.services.protection.AcqUnitConstants.FD_NO_ACQ_UNIT_ASSIGNED_CQL; import static org.folio.services.protection.AcqUnitConstants.NO_ACQ_UNIT_ASSIGNED_CQL; +import static org.folio.services.protection.AcqUnitConstants.NO_FD_BUDGET_UNIT_ASSIGNED_CQL; +import static org.folio.services.protection.AcqUnitConstants.NO_FD_FUND_UNIT_ASSIGNED_CQL; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -180,6 +181,8 @@ void testShouldBuildCqlClauseForFinanceDataWhenIdsIsEmpty(VertxTestContext vertx .withAcquisitionsUnits(Collections.emptyList()).withTotalRecords(0); var members = new AcquisitionsUnitMembershipCollection() .withAcquisitionsUnitMemberships(Collections.emptyList()).withTotalRecords(0); + var expectedQuery = "(" + NO_FD_FUND_UNIT_ASSIGNED_CQL + " and " + NO_FD_BUDGET_UNIT_ASSIGNED_CQL + ")"; + doReturn(succeededFuture(units)).when(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); doReturn(succeededFuture(members)).when(acqUnitMembershipsService).getAcquisitionsUnitsMemberships(anyString(), anyInt(), anyInt(), eq(requestContext)); @@ -190,7 +193,7 @@ void testShouldBuildCqlClauseForFinanceDataWhenIdsIsEmpty(VertxTestContext vertx assertTrue(result.succeeded()); var actClause = result.result(); - assertThat(actClause, equalTo(FD_NO_ACQ_UNIT_ASSIGNED_CQL)); + assertThat(actClause, equalTo(expectedQuery)); verify(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); verify(acqUnitMembershipsService).getAcquisitionsUnitsMemberships("userId==" + X_OKAPI_USER_ID.getValue(), 0, Integer.MAX_VALUE, requestContext); @@ -207,6 +210,11 @@ void testShouldBuildCqlClauseForFinanceDataWhenIdsIsNotEmpty(VertxTestContext ve var members = new AcquisitionsUnitMembershipCollection() .withAcquisitionsUnitMemberships(List.of(new AcquisitionsUnitMembership().withAcquisitionsUnitId(unitId).withId(memberId))) .withTotalRecords(1); + var expectedQuery = "((fundAcqUnitIds=(" + unitId + ") and budgetAcqUnitIds=(" + unitId + ")) or " + + "(fundAcqUnitIds=(" + unitId + ") and cql.allRecords=1 not budgetAcqUnitIds <> []) or " + + "(cql.allRecords=1 not fundAcqUnitIds <> [] and budgetAcqUnitIds=(" + unitId + ")) or " + + "(cql.allRecords=1 not fundAcqUnitIds <> [] and cql.allRecords=1 not budgetAcqUnitIds <> []))"; + doReturn(succeededFuture(units)).when(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); doReturn(succeededFuture(members)).when(acqUnitMembershipsService).getAcquisitionsUnitsMemberships(anyString(), anyInt(), anyInt(), eq(requestContext)); @@ -217,7 +225,7 @@ void testShouldBuildCqlClauseForFinanceDataWhenIdsIsNotEmpty(VertxTestContext ve assertTrue(result.succeeded()); var actClause = result.result(); - assertThat(actClause, equalTo("(fundAcqUnitIds=(" + unitId + ") and budgetAcqUnitIds=(" + unitId + ")) or (" + FD_NO_ACQ_UNIT_ASSIGNED_CQL + ")")); + assertThat(actClause, equalTo(expectedQuery)); verify(restClient).get(anyString(), eq(AcquisitionsUnitCollection.class), eq(requestContext)); verify(acqUnitMembershipsService).getAcquisitionsUnitsMemberships("userId==" + X_OKAPI_USER_ID.getValue(), 0, Integer.MAX_VALUE, requestContext); From fa40917979a8bfed267389c00ab72254383caabb Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 17:57:53 +0500 Subject: [PATCH 10/11] Add required module permissions --- descriptors/ModuleDescriptor-template.json | 32 ++++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 7fdb78cb..0d5895d2 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -752,27 +752,42 @@ { "methods": ["GET"], "pathPattern": "/finance/fund-update-logs", - "permissionsRequired": ["finance.fund-update-logs.collection.get"] + "permissionsRequired": ["finance.fund-update-logs.collection.get"], + "modulePermissions": [ + "finance-storage.fund-update-logs.collection.get" + ] }, { "methods": ["GET"], "pathPattern": "/finance/fund-update-logs/{id}", - "permissionsRequired": ["finance.fund-update-logs.item.get"] + "permissionsRequired": ["finance.fund-update-logs.item.get"], + "modulePermissions": [ + "finance-storage.fund-update-logs.item.get" + ] }, { "methods": ["POST"], "pathPattern": "/finance/fund-update-logs", - "permissionsRequired": ["finance.fund-update-logs.item.post"] + "permissionsRequired": ["finance.fund-update-logs.item.post"], + "modulePermissions": [ + "finance-storage.fund-update-logs.item.post" + ] }, { "methods": ["PUT"], "pathPattern": "/finance/fund-update-logs/{id}", - "permissionsRequired": ["finance.fund-update-logs.item.put"] + "permissionsRequired": ["finance.fund-update-logs.item.put"], + "modulePermissions": [ + "finance-storage.fund-update-logs.item.put" + ] }, { "methods": ["DELETE"], "pathPattern": "/finance/fund-update-logs/{id}", - "permissionsRequired": ["finance.fund-update-logs.item.delete"] + "permissionsRequired": ["finance.fund-update-logs.item.delete"], + "modulePermissions": [ + "finance-storage.fund-update-logs.item.delete" + ] } ] }, @@ -783,7 +798,12 @@ { "methods": ["GET"], "pathPattern": "/finance/finance-data", - "permissionsRequired": ["finance.finance-data.collection.get"] + "permissionsRequired": ["finance.finance-data.collection.get"], + "modulePermissions": [ + "finance-storage.finance-data.collection.get", + "acquisitions-units-storage.units.collection.get", + "acquisitions-units-storage.memberships.collection.get" + ] } ] }, From e12eb1c7d9018f49360ef58e37cd8c8d660173a6 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 18:53:44 +0500 Subject: [PATCH 11/11] Add comment --- .../java/org/folio/services/protection/AcqUnitsService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/folio/services/protection/AcqUnitsService.java b/src/main/java/org/folio/services/protection/AcqUnitsService.java index 6066ae04..1588d1de 100644 --- a/src/main/java/org/folio/services/protection/AcqUnitsService.java +++ b/src/main/java/org/folio/services/protection/AcqUnitsService.java @@ -80,7 +80,7 @@ public Future buildAcqUnitsCqlClauseForFinanceData(RequestContext reques "(%s and %s) or " + // Case 1: Both fund and budget have matching acqUnits "(%s and %s) or " + // Case 2: Fund has matching acqUnit and budget is empty "(%s and %s) or " + // Case 3: Fund is empty and budget has matching acqUnit - "(%s and %s))", + "(%s and %s))", // Case 4: Both fund and budget are empty convertIdsToCqlQuery(ids, FD_FUND_ACQUISITIONS_UNIT_IDS, false), convertIdsToCqlQuery(ids, FD_BUDGET_ACQUISITIONS_UNIT_IDS, false), convertIdsToCqlQuery(ids, FD_FUND_ACQUISITIONS_UNIT_IDS, false),