Skip to content

Commit

Permalink
SELV3-770: Improve geozone filtering - filtering should always provid…
Browse files Browse the repository at this point in the history 
…e performance boost
  • Loading branch information
@pwargulak
pwargulak committed Nov 18, 2024
1 parent c93a5cc commit d9ec795
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 53 deletions.
36 changes: 36 additions & 0 deletions src/main/java/org/openlmis/stockmanagement/dto/referencedata/BasicFacilityDto.java
View file
Original file line number Diff line number Diff line change
@@ -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;
}
8 changes: 8 additions & 0 deletions ...n/java/org/openlmis/stockmanagement/repository/SourceDestinationAssignmentRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,8 +33,14 @@ List<T> findByProgramIdAndFacilityTypeId(
@Param("programId") UUID programId, @Param("facilityTypeId") UUID facilityTypeId,
Pageable pageable);

List<T> findByProgramIdAndFacilityTypeIdAndNodeReferenceIdIn(
@Param("programId") UUID programId, @Param("facilityTypeId") UUID facilityTypeId,
@Param("referenceIds") Collection<UUID> referenceIds, Pageable pageable);

T findByProgramIdAndFacilityTypeIdAndNodeId(
@Param("programId") UUID programId, @Param("facilityTypeId") UUID facilityTypeId,
@Param("nodeId") UUID nodeId);

Page<T> findByNodeReferenceIdIn(@Param("referenceIds") Collection<UUID> referenceIds,
Pageable pageable);
}
73 changes: 34 additions & 39 deletions src/main/java/org/openlmis/stockmanagement/service/SourceDestinationBaseService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -311,8 +314,19 @@ private List<ValidSourceDestinationDto> createAssignmentDto(
programFacilityTypeExistenceService.checkProgramAndFacilityTypeExist(programId, facilityTypeId);

profiler.start("FIND_ASSIGNMENTS_BY_PROGRAM_AND_FACILITY_TYPE");
List<T> assignments = repository
.findByProgramIdAndFacilityTypeId(programId, facilityTypeId, Pageable.unpaged());
final List<T> assignments;

if (geographicZoneId == null) {
assignments = repository
.findByProgramIdAndFacilityTypeId(programId, facilityTypeId, Pageable.unpaged());
} else {
final Set<UUID> 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<UUID> facilitiesIds = getFacilitiesIds(assignments);
Expand All @@ -324,13 +338,7 @@ private List<ValidSourceDestinationDto> createAssignmentDto(
List<SourceDestinationAssignment> 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<ValidSourceDestinationDto> result = geoAssigment.stream()
.map(assignment -> createAssignmentDto(assignment, facilitiesById))
Expand All @@ -357,38 +365,26 @@ private List<ValidSourceDestinationDto> createAssignmentDto(
findAssignmentsByGeographicZone(UUID geographicZoneId,
SourceDestinationAssignmentRepository<T> repository,
Profiler profiler, Pageable pageable) {
profiler.start("FIND_ASSIGNMENTS_BY_GEOGRAPHIC_ZONE");

List<? extends SourceDestinationAssignment> assignments = repository.findAll();

profiler.start("FIND_FACILITY_IDS");
List<UUID> facilitiesIds = getFacilitiesIds(assignments);

profiler.start("FIND_FACILITIES_BY_ID_MAP");
Map<UUID, FacilityDto> facilitiesById = facilityRefDataService.findByIds(facilitiesIds);

assignments = assignments.stream()
.filter(assignment -> checkIfAssignmentInGeographicZone(geographicZoneId,
assignment, facilitiesById))
profiler.start("GET_FACILITY_FOR_GEO_ZONE");
final Map<UUID, FacilityDto> geoZoneFacilities =
facilityRefDataService.getPage(RequestParameters.init().set("zoneId", geographicZoneId))
.get().distinct().collect(Collectors.toMap(FacilityDto::getId, Function.identity()));
final Set<UUID> geoZoneFacilityIds =
geoZoneFacilities.values().stream().map(FacilityDto::getId).collect(Collectors.toSet());

profiler.start("GET_ASSIGNMENTS_PAGE");
final Page<T> assignments =
repository.findByNodeReferenceIdIn(geoZoneFacilityIds, pageable);

profiler.start("BUILD_DTOS");
final List<ValidSourceDestinationDto> validAssignments = assignments.get().map(
assignment -> ValidSourceDestinationDto.createFrom(assignment,
geoZoneFacilities.get(assignment.getNode().getReferenceId()).getName()))
.collect(Collectors.toList());

List<ValidSourceDestinationDto> validDestinations = createAssignmentDto(assignments);

return pageable.isUnpaged()
? Pagination.getPage(validDestinations)
: Pagination.getPage(validDestinations, pageable);
}

private <T extends SourceDestinationAssignment> boolean
checkIfAssignmentInGeographicZone(UUID geographicZoneId, T assignment,
Map<UUID, FacilityDto> 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 <T extends SourceDestinationAssignment> List<UUID> getFacilitiesIds(List<T> assignments) {
Expand All @@ -397,5 +393,4 @@ private <T extends SourceDestinationAssignment> List<UUID> getFacilitiesIds(List
.map(assignment -> assignment.getNode().getReferenceId())
.collect(Collectors.toList());
}

}
26 changes: 12 additions & 14 deletions src/test/java/org/openlmis/stockmanagement/service/SourceDestinationBaseServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -326,7 +329,7 @@ public void shouldReturn400WhenDestinationNotFound() throws Exception {
Map<UUID, FacilityDto> facilityMap = new HashMap<>();
facilityMap.put(facilityId, facilityDto);

when(facilityReferenceDataService.findByIds(Collections.singletonList(facilityId)))
when(facilityReferenceDataService.findByIds(singletonList(facilityId)))
.thenReturn(facilityMap);

List<ValidDestinationAssignment> validDestinationAssignments = asList(
Expand Down Expand Up @@ -410,7 +413,7 @@ public void shouldReturnListOfAllSourcesDtosWhenFindingValidSourcesAssignmentWit
Map<UUID, FacilityDto> facilityMap = new HashMap<>();
facilityMap.put(facilityId, facilityDto);

when(facilityReferenceDataService.findByIds(Collections.singletonList(facilityId)))
when(facilityReferenceDataService.findByIds(singletonList(facilityId)))
.thenReturn(facilityMap);

List<ValidSourceAssignment> validSourceAssignments = asList(
Expand Down Expand Up @@ -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<ValidSourceAssignment> validSourceAssignments = asList(
createOrganizationSourceAssignment(mockedOrganizationNode(ORGANIZATION_NODE_NAME)),
createFacilitySourceAssignment(mockedFacilityNode(facility1Id, FACILITY_NODE_NAME)),
createFacilitySourceAssignment(mockedFacilityNode(facility2Id, otherFacilityNodeName))
);
List<ValidSourceAssignment> validSourceAssignments = singletonList(
createFacilitySourceAssignment(mockedFacilityNode(facility1Id, FACILITY_NODE_NAME)));

when(sourceRepository.findAll())
.thenReturn(validSourceAssignments);
when(sourceRepository.findByNodeReferenceIdIn(any(), any()))
.thenReturn(Pagination.getPage(validSourceAssignments));

//when
Page<ValidSourceDestinationDto> validSources =
Expand Down Expand Up @@ -777,4 +775,4 @@ private Node mockedOrganizationNode(String name) {
node.setReferenceId(organization.getId());
return node;
}
}
}

0 comments on commit d9ec795

Please sign in to comment.