From d9ec7950ea48c907d958bbc90f9eed0ca0b41dcb Mon Sep 17 00:00:00 2001 From: Piotr Wargulak Date: Mon, 18 Nov 2024 18:29:30 +0100 Subject: [PATCH] SELV3-770: Improve geozone filtering - filtering should always provide performance boost --- .../dto/referencedata/BasicFacilityDto.java | 36 +++++++++ ...SourceDestinationAssignmentRepository.java | 8 ++ .../service/SourceDestinationBaseService.java | 73 +++++++++---------- .../SourceDestinationBaseServiceTest.java | 26 +++---- 4 files changed, 90 insertions(+), 53 deletions(-) create mode 100644 src/main/java/org/openlmis/stockmanagement/dto/referencedata/BasicFacilityDto.java diff --git a/src/main/java/org/openlmis/stockmanagement/dto/referencedata/BasicFacilityDto.java b/src/main/java/org/openlmis/stockmanagement/dto/referencedata/BasicFacilityDto.java new file mode 100644 index 00000000..8c06901d --- /dev/null +++ b/src/main/java/org/openlmis/stockmanagement/dto/referencedata/BasicFacilityDto.java @@ -0,0 +1,36 @@ +/* + * 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.dto.referencedata; + +import java.util.UUID; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@Builder +@NoArgsConstructor +@AllArgsConstructor +public class BasicFacilityDto { + private UUID id; + private String code; + private String name; + private Boolean active; + private Boolean enabled; + private GeographicZoneDto geographicZone; + private FacilityTypeDto type; +} diff --git a/src/main/java/org/openlmis/stockmanagement/repository/SourceDestinationAssignmentRepository.java b/src/main/java/org/openlmis/stockmanagement/repository/SourceDestinationAssignmentRepository.java index 80ee8245..a3092cbb 100644 --- a/src/main/java/org/openlmis/stockmanagement/repository/SourceDestinationAssignmentRepository.java +++ b/src/main/java/org/openlmis/stockmanagement/repository/SourceDestinationAssignmentRepository.java @@ -15,9 +15,11 @@ package org.openlmis.stockmanagement.repository; +import java.util.Collection; import java.util.List; import java.util.UUID; import org.openlmis.stockmanagement.domain.sourcedestination.SourceDestinationAssignment; +import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.repository.NoRepositoryBean; @@ -31,8 +33,14 @@ List findByProgramIdAndFacilityTypeId( @Param("programId") UUID programId, @Param("facilityTypeId") UUID facilityTypeId, Pageable pageable); + List findByProgramIdAndFacilityTypeIdAndNodeReferenceIdIn( + @Param("programId") UUID programId, @Param("facilityTypeId") UUID facilityTypeId, + @Param("referenceIds") Collection referenceIds, Pageable pageable); + T findByProgramIdAndFacilityTypeIdAndNodeId( @Param("programId") UUID programId, @Param("facilityTypeId") UUID facilityTypeId, @Param("nodeId") UUID nodeId); + Page findByNodeReferenceIdIn(@Param("referenceIds") Collection referenceIds, + Pageable pageable); } diff --git a/src/main/java/org/openlmis/stockmanagement/service/SourceDestinationBaseService.java b/src/main/java/org/openlmis/stockmanagement/service/SourceDestinationBaseService.java index b729c8d2..3e34cc41 100644 --- a/src/main/java/org/openlmis/stockmanagement/service/SourceDestinationBaseService.java +++ b/src/main/java/org/openlmis/stockmanagement/service/SourceDestinationBaseService.java @@ -23,7 +23,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; +import java.util.function.Function; import java.util.stream.Collectors; import org.openlmis.stockmanagement.domain.sourcedestination.Node; import org.openlmis.stockmanagement.domain.sourcedestination.Organization; @@ -38,6 +40,7 @@ import org.openlmis.stockmanagement.service.referencedata.FacilityReferenceDataService; import org.openlmis.stockmanagement.service.referencedata.ProgramFacilityTypeExistenceService; import org.openlmis.stockmanagement.util.Message; +import org.openlmis.stockmanagement.util.RequestParameters; import org.openlmis.stockmanagement.web.Pagination; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -311,8 +314,19 @@ private List createAssignmentDto( programFacilityTypeExistenceService.checkProgramAndFacilityTypeExist(programId, facilityTypeId); profiler.start("FIND_ASSIGNMENTS_BY_PROGRAM_AND_FACILITY_TYPE"); - List assignments = repository - .findByProgramIdAndFacilityTypeId(programId, facilityTypeId, Pageable.unpaged()); + final List assignments; + + if (geographicZoneId == null) { + assignments = repository + .findByProgramIdAndFacilityTypeId(programId, facilityTypeId, Pageable.unpaged()); + } else { + final Set geoZoneFacilityIds = + facilityRefDataService.getPage(RequestParameters.init().set("zoneId", geographicZoneId)) + .get().map(FacilityDto::getId).collect(Collectors.toSet()); + assignments = repository + .findByProgramIdAndFacilityTypeIdAndNodeReferenceIdIn(programId, facilityTypeId, + geoZoneFacilityIds, Pageable.unpaged()); + } profiler.start("FIND_FACILITY_IDS"); List facilitiesIds = getFacilitiesIds(assignments); @@ -324,13 +338,7 @@ private List createAssignmentDto( List geoAssigment = assignments.stream() .filter(assignment -> !assignment.getNode().isRefDataFacility() || hasGeoAffinity(assignment, facility, facilitiesById)) - .filter(assignment -> { - if (geographicZoneId != null) { - return checkIfAssignmentInGeographicZone(geographicZoneId, assignment, facilitiesById); - } else { - return true; - } - }).collect(Collectors.toList()); + .collect(Collectors.toList()); List result = geoAssigment.stream() .map(assignment -> createAssignmentDto(assignment, facilitiesById)) @@ -357,38 +365,26 @@ private List createAssignmentDto( findAssignmentsByGeographicZone(UUID geographicZoneId, SourceDestinationAssignmentRepository repository, Profiler profiler, Pageable pageable) { - profiler.start("FIND_ASSIGNMENTS_BY_GEOGRAPHIC_ZONE"); - - List assignments = repository.findAll(); - - profiler.start("FIND_FACILITY_IDS"); - List facilitiesIds = getFacilitiesIds(assignments); - - profiler.start("FIND_FACILITIES_BY_ID_MAP"); - Map facilitiesById = facilityRefDataService.findByIds(facilitiesIds); - - assignments = assignments.stream() - .filter(assignment -> checkIfAssignmentInGeographicZone(geographicZoneId, - assignment, facilitiesById)) + profiler.start("GET_FACILITY_FOR_GEO_ZONE"); + final Map geoZoneFacilities = + facilityRefDataService.getPage(RequestParameters.init().set("zoneId", geographicZoneId)) + .get().distinct().collect(Collectors.toMap(FacilityDto::getId, Function.identity())); + final Set geoZoneFacilityIds = + geoZoneFacilities.values().stream().map(FacilityDto::getId).collect(Collectors.toSet()); + + profiler.start("GET_ASSIGNMENTS_PAGE"); + final Page assignments = + repository.findByNodeReferenceIdIn(geoZoneFacilityIds, pageable); + + profiler.start("BUILD_DTOS"); + final List validAssignments = assignments.get().map( + assignment -> ValidSourceDestinationDto.createFrom(assignment, + geoZoneFacilities.get(assignment.getNode().getReferenceId()).getName())) .collect(Collectors.toList()); - List validDestinations = createAssignmentDto(assignments); - return pageable.isUnpaged() - ? Pagination.getPage(validDestinations) - : Pagination.getPage(validDestinations, pageable); - } - - private boolean - checkIfAssignmentInGeographicZone(UUID geographicZoneId, T assignment, - Map facilitiesById) { - Node node = assignment.getNode(); - if (node.isRefDataFacility()) { - FacilityDto facilityDto = facilitiesById.get(node.getReferenceId()); - return facilityDto.getGeographicZone().getId().equals(geographicZoneId); - } else { - return false; - } + ? Pagination.getPage(validAssignments) + : Pagination.getPage(validAssignments, pageable, assignments.getTotalElements()); } private List getFacilitiesIds(List assignments) { @@ -397,5 +393,4 @@ private List getFacilitiesIds(List .map(assignment -> assignment.getNode().getReferenceId()) .collect(Collectors.toList()); } - } diff --git a/src/test/java/org/openlmis/stockmanagement/service/SourceDestinationBaseServiceTest.java b/src/test/java/org/openlmis/stockmanagement/service/SourceDestinationBaseServiceTest.java index 6d2ffccf..7a3f5d6d 100644 --- a/src/test/java/org/openlmis/stockmanagement/service/SourceDestinationBaseServiceTest.java +++ b/src/test/java/org/openlmis/stockmanagement/service/SourceDestinationBaseServiceTest.java @@ -16,6 +16,7 @@ package org.openlmis.stockmanagement.service; import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static java.util.UUID.randomUUID; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; @@ -34,6 +35,7 @@ import static org.openlmis.stockmanagement.testutils.ValidSourceDestinationDataBuilder.createOrganizationDestination; import static org.openlmis.stockmanagement.testutils.ValidSourceDestinationDataBuilder.createOrganizationSourceAssignment; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -63,6 +65,7 @@ import org.openlmis.stockmanagement.service.referencedata.ProgramFacilityTypeExistenceService; import org.openlmis.stockmanagement.testutils.GeographicLevelDtoDataBuilder; import org.openlmis.stockmanagement.testutils.GeographicZoneDtoDataBuilder; +import org.openlmis.stockmanagement.util.RequestParameters; import org.openlmis.stockmanagement.web.Pagination; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; @@ -326,7 +329,7 @@ public void shouldReturn400WhenDestinationNotFound() throws Exception { Map facilityMap = new HashMap<>(); facilityMap.put(facilityId, facilityDto); - when(facilityReferenceDataService.findByIds(Collections.singletonList(facilityId))) + when(facilityReferenceDataService.findByIds(singletonList(facilityId))) .thenReturn(facilityMap); List validDestinationAssignments = asList( @@ -410,7 +413,7 @@ public void shouldReturnListOfAllSourcesDtosWhenFindingValidSourcesAssignmentWit Map facilityMap = new HashMap<>(); facilityMap.put(facilityId, facilityDto); - when(facilityReferenceDataService.findByIds(Collections.singletonList(facilityId))) + when(facilityReferenceDataService.findByIds(singletonList(facilityId))) .thenReturn(facilityMap); List validSourceAssignments = asList( @@ -463,19 +466,14 @@ public void shouldReturnListOfSourcesDtosWhenFindingValidSourcesAssignmentByGeog facilityMap.put(facility1Id, facility1); facilityMap.put(facility2Id, facility2); - when(facilityReferenceDataService.findByIds(asList(facility1Id, facility2Id))) - .thenReturn(facilityMap); - when(facilityReferenceDataService.findByIds(Collections.singletonList(facility1Id))) - .thenReturn(Collections.singletonMap(facility1Id, facility1)); + when(facilityReferenceDataService.getPage(any(RequestParameters.class))) + .thenReturn(Pagination.getPage(new ArrayList<>(facilityMap.values()))); - List validSourceAssignments = asList( - createOrganizationSourceAssignment(mockedOrganizationNode(ORGANIZATION_NODE_NAME)), - createFacilitySourceAssignment(mockedFacilityNode(facility1Id, FACILITY_NODE_NAME)), - createFacilitySourceAssignment(mockedFacilityNode(facility2Id, otherFacilityNodeName)) - ); + List validSourceAssignments = singletonList( + createFacilitySourceAssignment(mockedFacilityNode(facility1Id, FACILITY_NODE_NAME))); - when(sourceRepository.findAll()) - .thenReturn(validSourceAssignments); + when(sourceRepository.findByNodeReferenceIdIn(any(), any())) + .thenReturn(Pagination.getPage(validSourceAssignments)); //when Page validSources = @@ -777,4 +775,4 @@ private Node mockedOrganizationNode(String name) { node.setReferenceId(organization.getId()); return node; } -} \ No newline at end of file +}