From 727d9bf10cd85e83eabd5067874d7ae1f0baadb6 Mon Sep 17 00:00:00 2001 From: Azizbek Khushvakov Date: Wed, 20 Nov 2024 12:51:57 +0500 Subject: [PATCH] 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 -}