From d9ed48cf11664a5f0816e936eae536f5ca0a768e Mon Sep 17 00:00:00 2001 From: Tomek Date: Wed, 10 Jul 2024 11:13:07 +0200 Subject: [PATCH] OLMIS-7953 Copied solution with multiple program ids to make application logging performance better. (#61) * Cherry-pick (modified) from OAM-218 solution, commit 8b82790784a31999e4f13f2f794aac43e6c7d3e2. OAM-218: Updated programId param to repeatable (#60) * OAM-218: WIP * OAM-218: Updated programId param to repeatable * OAM-218: Removed unused import, provided constants * OAM-218: Removed unnecessary stubbings * OAM-218: Added extra tests * OAM-218: Provided changes after review * OAM-218: Removed 'Ignore' annotation * OAM-218: Added extra condition to test * OAM-218: Fixed params/method names * OAM-218: Added tests for coverage. --------- Co-authored-by: Piotr Wargulak Co-authored-by: tsznaj # Conflicts: # src/integration-test/java/org/openlmis/stockmanagement/web/ValidSourceDestinationControllerIntegrationTest.java # src/main/java/org/openlmis/stockmanagement/repository/SourceDestinationAssignmentRepository.java # src/main/java/org/openlmis/stockmanagement/service/SourceDestinationBaseService.java # src/main/java/org/openlmis/stockmanagement/service/StockCardSummariesService.java # src/main/java/org/openlmis/stockmanagement/service/ValidDestinationService.java # src/main/java/org/openlmis/stockmanagement/service/ValidSourceService.java # src/main/java/org/openlmis/stockmanagement/validators/SourceDestinationGeoLevelAffinityValidator.java # src/main/java/org/openlmis/stockmanagement/web/ValidSourceDestinationController.java # src/test/java/org/openlmis/stockmanagement/service/SourceDestinationBaseServiceTest.java # src/test/java/org/openlmis/stockmanagement/validators/SourceDestinationGeoLevelAffinityValidatorTest.java * OLMIS-7953: coverage test * OLMIS-7953: moved to impl subdirectories * OLMIS-7953: little changes to omit unnecessary coverage tests * OLMIS-7953: added coverage test --------- Co-authored-by: Szymon Radziszewski <117299684+sradziszewski@users.noreply.github.com> --- ...onAssignmentRepositoryIntegrationTest.java | 3 +- ...onAssignmentControllerIntegrationTest.java | 2 +- .../repository/StockCardRepository.java | 15 +- ...ValidReasonAssignmentRepositoryCustom.java | 2 +- .../ValidReasonAssignmentRepositoryImpl.java | 22 ++- .../web/ValidReasonAssignmentController.java | 2 +- .../ValidReasonAssignmentSearchParams.java | 22 ++- src/main/resources/api-definition.yaml | 1 + ...lidReasonAssignmentRepositoryImplTest.java | 141 ++++++++++++++++++ ...ValidReasonAssignmentSearchParamsTest.java | 19 ++- 10 files changed, 197 insertions(+), 32 deletions(-) create mode 100644 src/test/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImplTest.java diff --git a/src/integration-test/java/org/openlmis/stockmanagement/repository/ValidReasonAssignmentRepositoryIntegrationTest.java b/src/integration-test/java/org/openlmis/stockmanagement/repository/ValidReasonAssignmentRepositoryIntegrationTest.java index 34f43de5..ac6da577 100644 --- a/src/integration-test/java/org/openlmis/stockmanagement/repository/ValidReasonAssignmentRepositoryIntegrationTest.java +++ b/src/integration-test/java/org/openlmis/stockmanagement/repository/ValidReasonAssignmentRepositoryIntegrationTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertThat; import com.google.common.collect.Sets; +import java.util.Collections; import java.util.List; import java.util.UUID; import javax.persistence.EntityManager; @@ -93,7 +94,7 @@ public void shouldReturnValidReasonWithProgramAndFacilityTypeAndReasonAndReasonT repository.save(newAssignment); List validReasonAssignments = repository.search( - PROGRAM_ID, FACILITY_TYPE_ID, Sets.newHashSet( + Collections.singleton(PROGRAM_ID), FACILITY_TYPE_ID, Sets.newHashSet( validReasonAssignment.getReason().getReasonType(), stockCardLineItemReason.getReasonType()), stockCardLineItemReason.getId()); diff --git a/src/integration-test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentControllerIntegrationTest.java b/src/integration-test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentControllerIntegrationTest.java index c09ffdc2..04af7ab9 100644 --- a/src/integration-test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentControllerIntegrationTest.java +++ b/src/integration-test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentControllerIntegrationTest.java @@ -100,7 +100,7 @@ public void getValidReasonAssignments() { @Test public void getValidReasonAssignmentsByAllParameters() { - when(reasonAssignmentRepository.search(programId, facilityTypeId, + when(reasonAssignmentRepository.search(Collections.singleton(programId), facilityTypeId, Sets.newHashSet(ReasonType.CREDIT, ReasonType.DEBIT), reasonId)).thenReturn( Collections.singletonList(reasonAssignment)); diff --git a/src/main/java/org/openlmis/stockmanagement/repository/StockCardRepository.java b/src/main/java/org/openlmis/stockmanagement/repository/StockCardRepository.java index 968fc92e..8b388410 100644 --- a/src/main/java/org/openlmis/stockmanagement/repository/StockCardRepository.java +++ b/src/main/java/org/openlmis/stockmanagement/repository/StockCardRepository.java @@ -29,20 +29,23 @@ public interface StockCardRepository extends JpaRepository { + String PROGRAM_ID = "programId"; + String FACILITY_ID = "facilityId"; + StockCard findByProgramIdAndFacilityIdAndOrderableIdAndLotId( - @Param("programId") UUID programId, - @Param("facilityId") UUID facilityId, + @Param(PROGRAM_ID) UUID programId, + @Param(FACILITY_ID) UUID facilityId, @Param("orderableId") UUID orderableId, @Param("lotId") UUID lotId); Page findByProgramIdAndFacilityId( - @Param("programId") UUID programId, - @Param("facilityId") UUID facilityId, + @Param(PROGRAM_ID) UUID programId, + @Param(FACILITY_ID) UUID facilityId, Pageable pageable); List findByProgramIdAndFacilityId( - @Param("programId") UUID programId, - @Param("facilityId") UUID facilityId); + @Param(PROGRAM_ID) UUID programId, + @Param(FACILITY_ID) UUID facilityId); List findByOrderableIdInAndProgramIdAndFacilityId( Collection orderableIds, UUID programId, UUID facilityId); diff --git a/src/main/java/org/openlmis/stockmanagement/repository/custom/ValidReasonAssignmentRepositoryCustom.java b/src/main/java/org/openlmis/stockmanagement/repository/custom/ValidReasonAssignmentRepositoryCustom.java index 5dc2ac92..f96ccbbb 100644 --- a/src/main/java/org/openlmis/stockmanagement/repository/custom/ValidReasonAssignmentRepositoryCustom.java +++ b/src/main/java/org/openlmis/stockmanagement/repository/custom/ValidReasonAssignmentRepositoryCustom.java @@ -23,7 +23,7 @@ public interface ValidReasonAssignmentRepositoryCustom { - List search(UUID programId, UUID facilityTypeId, + List search(Collection programIds, UUID facilityTypeId, Collection reasonTypes, UUID reasonId); } diff --git a/src/main/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImpl.java b/src/main/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImpl.java index cc01a8ce..4dffb91e 100644 --- a/src/main/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImpl.java +++ b/src/main/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImpl.java @@ -33,26 +33,25 @@ public class ValidReasonAssignmentRepositoryImpl implements ValidReasonAssignmentRepositoryCustom { - private static final String PROGRAM_ID = "programId"; - private static final String FACILITY_TYPE_ID = "facilityTypeId"; + static final String PROGRAM_ID = "programId"; + static final String FACILITY_TYPE_ID = "facilityTypeId"; private static final String ID = "id"; private static final String REASON_TYPE = "reasonType"; private static final String REASON = "reason"; - @PersistenceContext private EntityManager entityManager; /** * This method is supposed to retrieve all Valid Reason Assignments with matched parameters. * - * @param programId Valid Reason Assignment program id + * @param programIds Valid Reason Assignment program ids * @param facilityTypeId Valid Reason Assignment facility type id - * @param reasonTypes Valid Reason Assignment stock card line item reason types - * @param reasonId Valid Reason Assignment stock card line item reason id + * @param reasonTypes Valid Reason Assignment stock card line item reason types + * @param reasonId Valid Reason Assignment stock card line item reason id * @return List of Valid Reason Assignments matching the parameters. */ - public List search(UUID programId, UUID facilityTypeId, - Collection reasonTypes, UUID reasonId) { + public List search(Collection programIds, UUID facilityTypeId, + Collection reasonTypes, UUID reasonId) { CriteriaBuilder builder = entityManager.getCriteriaBuilder(); CriteriaQuery query = builder.createQuery(ValidReasonAssignment.class); @@ -61,8 +60,8 @@ public List search(UUID programId, UUID facilityTypeId, Predicate predicate = builder.conjunction(); - if (null != programId) { - predicate = builder.and(predicate, builder.equal(root.get(PROGRAM_ID), programId)); + if (null != programIds) { + predicate = builder.and(predicate, root.get(PROGRAM_ID).in(programIds)); } if (null != facilityTypeId) { @@ -83,7 +82,6 @@ public List search(UUID programId, UUID facilityTypeId, query.where(predicate); - return entityManager.createQuery(query) - .getResultList(); + return entityManager.createQuery(query).getResultList(); } } diff --git a/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentController.java b/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentController.java index 49309ae1..88af8581 100644 --- a/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentController.java +++ b/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentController.java @@ -86,7 +86,7 @@ public List getValidReasons( ValidReasonAssignmentSearchParams params = new ValidReasonAssignmentSearchParams(queryParams); profiler.start("SEARCH_VALID_REASONS_IN_SERVICE"); - List reasons = reasonAssignmentRepository.search(params.getProgram(), + List reasons = reasonAssignmentRepository.search(params.getProgramIds(), params.getFacilityType(), params.getReasonType(), params.getReason()); diff --git a/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParams.java b/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParams.java index 6efb38d7..0fca2779 100644 --- a/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParams.java +++ b/src/main/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParams.java @@ -51,17 +51,25 @@ public ValidReasonAssignmentSearchParams(MultiValueMap queryMap) } /** - * Gets program. - * - * @return String value of program id or null if params doesn't contain "program" param. - * Empty string for null request param value. + * Gets collection of {@link UUID} for "program" key from params. */ - public UUID getProgram() { + public Collection getProgramIds() { if (!queryParams.containsKey(PROGRAM)) { return null; } - String program = queryParams.getFirst(PROGRAM); - return UuidUtil.fromString(program).orElse(null); + + Set programs = new HashSet<>(); + queryParams.asMultiValueMap().forEach((key, value) -> { + if (Objects.equals(key, PROGRAM)) { + value.forEach(id -> { + if (id != null && !id.isEmpty()) { + programs.add(UuidUtil.fromString(id).get()); + } + }); + } + }); + + return programs; } /** diff --git a/src/main/resources/api-definition.yaml b/src/main/resources/api-definition.yaml index c0d7d98a..ea260ec1 100644 --- a/src/main/resources/api-definition.yaml +++ b/src/main/resources/api-definition.yaml @@ -738,6 +738,7 @@ traits: description: Program id type: string required: false + repeat: true facilityType: description: Facility type id type: string diff --git a/src/test/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImplTest.java b/src/test/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImplTest.java new file mode 100644 index 00000000..0221cf5c --- /dev/null +++ b/src/test/java/org/openlmis/stockmanagement/repository/custom/impl/ValidReasonAssignmentRepositoryImplTest.java @@ -0,0 +1,141 @@ +/* + * This program is part of the OpenLMIS logistics management information system platform software. + * Copyright © 2017 VillageReach + * + * This program is free software: you can redistribute it and/or modify it under the terms + * of the GNU Affero General Public License as published by the Free Software Foundation, either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; + * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the GNU Affero General Public License for more details. You should have received a copy of + * the GNU Affero General Public License along with this program. If not, see + * http://www.gnu.org/licenses.  For additional information contact info@OpenLMIS.org. + */ + +package org.openlmis.stockmanagement.repository.custom.impl; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.openlmis.stockmanagement.repository.custom.impl.ValidReasonAssignmentRepositoryImpl.FACILITY_TYPE_ID; +import static org.openlmis.stockmanagement.repository.custom.impl.ValidReasonAssignmentRepositoryImpl.PROGRAM_ID; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.UUID; +import javax.persistence.EntityManager; +import javax.persistence.TypedQuery; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Path; +import javax.persistence.criteria.Predicate; +import javax.persistence.criteria.Root; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.openlmis.stockmanagement.domain.reason.ValidReasonAssignment; + +@RunWith(MockitoJUnitRunner.class) +public class ValidReasonAssignmentRepositoryImplTest { + + @InjectMocks + private ValidReasonAssignmentRepositoryImpl repository; + + @Mock + private EntityManager entityManager; + + @Test + public void shouldSearchForProgramIdsOnly() { + //given + UUID programId1 = UUID.randomUUID(); + UUID programId2 = UUID.randomUUID(); + Collection programIds = new ArrayList<>(); + programIds.add(programId1); + programIds.add(programId2); + + List validReasonAssignmentList = mock(List.class); + TypedQuery typedQuery = mock(TypedQuery.class); + when(typedQuery.getResultList()) + .thenReturn(validReasonAssignmentList); + + CriteriaQuery query = mock(CriteriaQuery.class); + Predicate conjunctionPredicate = mock(Predicate.class); + + Predicate inPredicate = mock(Predicate.class); + Path programIdPath = mock(Path.class); + when(programIdPath.in(programIds)).thenReturn(inPredicate); + + Root root = mock(Root.class); + when(root.get(PROGRAM_ID)).thenReturn(programIdPath); + Predicate wherePredicate = mock(Predicate.class); + + CriteriaBuilder criteriaBuilder = mock(CriteriaBuilder.class); + when(criteriaBuilder.createQuery(ValidReasonAssignment.class)) + .thenReturn(query); + when(criteriaBuilder.conjunction()).thenReturn(conjunctionPredicate); + when(criteriaBuilder.and(conjunctionPredicate, inPredicate)).thenReturn(wherePredicate); + + when(query.from(ValidReasonAssignment.class)).thenReturn(root); + + when(entityManager.getCriteriaBuilder()).thenReturn(criteriaBuilder); + when(entityManager.createQuery(query)) + .thenReturn(typedQuery); + + //when + List searchResults = + repository.search(programIds, null, null, null); + + //then + verify(entityManager).createQuery(query); + verify(query).where(wherePredicate); + assertThat(searchResults).isEqualTo(validReasonAssignmentList); + } + + @Test + public void shouldSearchForFacilityTypeIdOnly() { + //given + UUID facilityTypeId = UUID.randomUUID(); + + List validReasonAssignmentList = mock(List.class); + TypedQuery typedQuery = mock(TypedQuery.class); + when(typedQuery.getResultList()) + .thenReturn(validReasonAssignmentList); + + CriteriaQuery query = mock(CriteriaQuery.class); + Predicate conjunctionPredicate = mock(Predicate.class); + + Predicate equalPredicate = mock(Predicate.class); + Path facilityTypeIdPath = mock(Path.class); + + Root root = mock(Root.class); + when(root.get(FACILITY_TYPE_ID)).thenReturn(facilityTypeIdPath); + Predicate wherePredicate = mock(Predicate.class); + + CriteriaBuilder criteriaBuilder = mock(CriteriaBuilder.class); + when(criteriaBuilder.createQuery(ValidReasonAssignment.class)) + .thenReturn(query); + when(criteriaBuilder.conjunction()).thenReturn(conjunctionPredicate); + when(criteriaBuilder.equal(facilityTypeIdPath, facilityTypeId)).thenReturn(equalPredicate); + when(criteriaBuilder.and(conjunctionPredicate, equalPredicate)).thenReturn(wherePredicate); + + when(query.from(ValidReasonAssignment.class)).thenReturn(root); + + when(entityManager.getCriteriaBuilder()).thenReturn(criteriaBuilder); + when(entityManager.createQuery(query)) + .thenReturn(typedQuery); + + //when + List searchResults = + repository.search(null, facilityTypeId, null, null); + + //then + verify(entityManager).createQuery(query); + verify(query).where(wherePredicate); + assertThat(searchResults).isEqualTo(validReasonAssignmentList); + } +} diff --git a/src/test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParamsTest.java b/src/test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParamsTest.java index b52c3089..22fad7c5 100644 --- a/src/test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParamsTest.java +++ b/src/test/java/org/openlmis/stockmanagement/web/ValidReasonAssignmentSearchParamsTest.java @@ -16,13 +16,16 @@ package org.openlmis.stockmanagement.web; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.openlmis.stockmanagement.web.ValidReasonAssignmentSearchParams.FACILITY_TYPE; import static org.openlmis.stockmanagement.web.ValidReasonAssignmentSearchParams.PROGRAM; import static org.openlmis.stockmanagement.web.ValidReasonAssignmentSearchParams.REASON; import static org.openlmis.stockmanagement.web.ValidReasonAssignmentSearchParams.REASON_TYPE; import com.google.common.collect.Sets; +import java.util.Collection; import java.util.UUID; import org.junit.Test; import org.openlmis.stockmanagement.domain.reason.ReasonType; @@ -38,10 +41,17 @@ public class ValidReasonAssignmentSearchParamsTest { @Test public void shouldGetProgramIdValueFromParameters() { LinkedMultiValueMap queryMap = new LinkedMultiValueMap<>(); + final UUID typeId = UUID.randomUUID(); queryMap.add(PROGRAM, VALUE.toString()); + queryMap.add(PROGRAM, ""); + queryMap.add(FACILITY_TYPE, typeId.toString()); ValidReasonAssignmentSearchParams params = new ValidReasonAssignmentSearchParams(queryMap); - assertEquals(VALUE, params.getProgram()); + Collection programIds = params.getProgramIds(); + + assertTrue(programIds.contains(VALUE)); + assertFalse(programIds.contains(typeId)); + assertFalse(programIds.contains("")); } @Test @@ -49,7 +59,7 @@ public void shouldAssignNullIfProgramIdIsAbsentInParameters() { ValidReasonAssignmentSearchParams params = new ValidReasonAssignmentSearchParams(new LinkedMultiValueMap<>()); - assertNull(params.getProgram()); + assertNull(params.getProgramIds()); } @Test @@ -98,10 +108,13 @@ public void shouldGetReasonTypesFromParameters() { LinkedMultiValueMap queryMap = new LinkedMultiValueMap<>(); queryMap.add(REASON_TYPE, DEBIT); queryMap.add(REASON_TYPE, CREDIT); + queryMap.add(REASON, VALUE.toString()); ValidReasonAssignmentSearchParams params = new ValidReasonAssignmentSearchParams(queryMap); + Collection reasonTypes = params.getReasonType(); + assertEquals(Sets.newHashSet(ReasonType.fromString(CREDIT), ReasonType.fromString(DEBIT)), - params.getReasonType()); + reasonTypes); } @Test