Skip to content

Commit

Permalink
OLMIS-6614: Replaced multiple calls to reference data by one call to …
Browse files Browse the repository at this point in the history
…the `GET /api/orderableFulfills` endpoint
  • Loading branch information
Daniel Serkowski authored and mkwiatkowskisoldevelo committed Oct 9, 2019
1 parent 888e117 commit dde6c40
Show file tree
Hide file tree
Showing 10 changed files with 118 additions and 114 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Improvements:
* [OLMIS-6408](https://openlmis.atlassian.net/browse/OLMIS-6408): Added pageable validator.
* [OLMIS-6474](https://openlmis.atlassian.net/browse/OLMIS-6474): Performance improvements of `GET /api/orderableFulfills` endpoint.
* [OLMIS-6564](https://openlmis.atlassian.net/browse/OLMIS-6564): Changed wiremock dependency configuration to avoid issue with HTTP response compression.
* [OLMIS-6614](https://openlmis.atlassian.net/browse/OLMIS-6614): Replaced multiple calls to reference data by one call to the `GET /api/orderableFulfills` endpoint.

Bug fixes:
* [OLMIS-6582](https://openlmis.atlassian.net/browse/OLMIS-6582): Changed wiremock dependency configuration to fix printing Physical Inventory.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ public void setUp() {
.thenReturn(summaries);

when(stockCardSummariesV2DtoBuilder
.build(summaries.getPageOfApprovedProducts(),
summaries.getStockCardsForFulfillOrderables(),
.build(summaries.getStockCardsForFulfillOrderables(),
summaries.getOrderableFulfillMap(),
false))
.thenReturn(asList(stockCardSummary, stockCardSummary2));
Expand Down Expand Up @@ -194,8 +193,7 @@ public void shouldReturnNonEmptySummariesIfFlagIsSet() throws Exception {
.thenReturn(summaries);

when(stockCardSummariesV2DtoBuilder
.build(summaries.getPageOfApprovedProducts(),
summaries.getStockCardsForFulfillOrderables(),
.build(summaries.getStockCardsForFulfillOrderables(),
summaries.getOrderableFulfillMap(),
true))
.thenReturn(singletonList(stockCardSummary));
Expand Down Expand Up @@ -231,8 +229,7 @@ public void shouldRespectSendNonEmptyCardsFlagInSubsequentRequests() throws Exce
.thenReturn(summaries);

when(stockCardSummariesV2DtoBuilder
.build(summaries.getPageOfApprovedProducts(),
summaries.getStockCardsForFulfillOrderables(),
.build(summaries.getStockCardsForFulfillOrderables(),
summaries.getOrderableFulfillMap(),
true))
.thenReturn(singletonList(stockCardSummary));
Expand Down Expand Up @@ -261,8 +258,7 @@ public void shouldRespectSendNonEmptyCardsFlagInSubsequentRequests() throws Exce
params.setNonEmptyOnly(false);

when(stockCardSummariesV2DtoBuilder
.build(summaries.getPageOfApprovedProducts(),
summaries.getStockCardsForFulfillOrderables(),
.build(summaries.getStockCardsForFulfillOrderables(),
summaries.getOrderableFulfillMap(),
false))
.thenReturn(asList(stockCardSummary, stockCardSummary2));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,14 @@
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.openlmis.stockmanagement.domain.card.StockCard;
import org.openlmis.stockmanagement.dto.referencedata.OrderableDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderableFulfillDto;

@AllArgsConstructor
@NoArgsConstructor
@Getter
@Setter
public class StockCardSummaries {
private List<OrderableDto> pageOfApprovedProducts;
private List<StockCard> stockCardsForFulfillOrderables;
private Map<UUID, OrderableFulfillDto> orderableFulfillMap;
private LocalDate asOfDate;
private Long totalElements;
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@
import org.openlmis.stockmanagement.dto.referencedata.LotDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderableDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderableFulfillDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderablesAggregator;
import org.openlmis.stockmanagement.repository.CalculatedStockOnHandRepository;
import org.openlmis.stockmanagement.repository.StockCardRepository;
import org.openlmis.stockmanagement.service.referencedata.ApprovedProductReferenceDataService;
import org.openlmis.stockmanagement.service.referencedata.LotReferenceDataService;
import org.openlmis.stockmanagement.service.referencedata.OrderableFulfillReferenceDataService;
import org.openlmis.stockmanagement.service.referencedata.OrderableReferenceDataService;
Expand Down Expand Up @@ -79,9 +77,6 @@ public class StockCardSummariesService extends StockCardBaseService {
@Autowired
private LotReferenceDataService lotReferenceDataService;

@Autowired
private ApprovedProductReferenceDataService approvedProductReferenceDataService;

@Autowired
private StockCardRepository stockCardRepository;

Expand All @@ -90,7 +85,6 @@ public class StockCardSummariesService extends StockCardBaseService {

@Autowired
private PermissionService permissionService;


/**
* Get a map of stock cards assigned to orderable ids.
Expand All @@ -104,15 +98,21 @@ public class StockCardSummariesService extends StockCardBaseService {
*/
public Map<UUID, StockCardAggregate> getGroupedStockCards(UUID programId, UUID facilityId,
Set<UUID> orderableIds, LocalDate startDate, LocalDate endDate) {
Profiler profiler = new Profiler("GET_GROUPED_STOCK_CARDS");
profiler.setLogger(LOGGER);

profiler.start("FIND_STOCK_CARD_BY_PROGRAM_AND_FACILITY");
List<StockCard> stockCards = calculatedStockOnHandService
.getStockCardsWithStockOnHand(programId, facilityId);

profiler.start("FIND_ORDERABLE_FULFILL_BY_IDS");
Map<UUID, OrderableFulfillDto> orderableFulfillMap =
orderableFulfillService.findByIds(stockCards.stream()
.map(StockCard::getOrderableId)
.collect(toSet()));

return stockCards.stream()
profiler.start("ASSIGN_ORDERABLES_TO_STOCK_CARD");
Map<UUID, StockCardAggregate> stockCardAggregateMap = stockCards.stream()
.map(stockCard -> assignOrderableToStockCard(
stockCard, orderableFulfillMap, orderableIds, startDate, endDate))
.filter(pair -> null != pair.getLeft())
Expand All @@ -123,6 +123,8 @@ public Map<UUID, StockCardAggregate> getGroupedStockCards(UUID programId, UUID f
aggregate1.getStockCards().addAll(aggregate2.getStockCards());
return aggregate1;
}));
profiler.stop().log();
return stockCardAggregateMap;
}

/**
Expand All @@ -138,25 +140,19 @@ public StockCardSummaries findStockCards(StockCardSummariesV2SearchParams params
profiler.start("VALIDATE_VIEW_RIGHTS");
permissionService.canViewStockCard(params.getProgramId(), params.getFacilityId());

profiler.start("GET_APPROVED_PRODUCTS");
OrderablesAggregator approvedProducts = approvedProductReferenceDataService
.getApprovedProducts(params.getFacilityId(), params.getProgramId(),
params.getOrderableIds());

profiler.start("FIND_ORDERABLE_FULFILL_BY_ID");
Map<UUID, OrderableFulfillDto> orderableFulfillMap = orderableFulfillService.findByIds(
approvedProducts.getIdentifiers());
profiler.start("FIND_ORDERABLE_FULFILL_BY_FACILITY_AND_PROGRAM");
Map<UUID, OrderableFulfillDto> orderableFulfillMap = orderableFulfillService
.findByFacilityIdProgramId(params.getFacilityId(), params.getProgramId());

profiler.start("FIND_STOCK_CARD_BY_PROGRAM_AND_FACILITY");

List<StockCard> stockCards = calculatedStockOnHandService
.getStockCardsWithStockOnHand(params.getProgramId(), params.getFacilityId(),
params.getAsOfDate());

Page<OrderableDto> orderablesPage = approvedProducts.getOrderablesPage();
StockCardSummaries result = new StockCardSummaries(
orderablesPage.getContent(), stockCards, orderableFulfillMap,
params.getAsOfDate(), orderablesPage.getTotalElements());
stockCards, orderableFulfillMap,
params.getAsOfDate());
profiler.stop().log();
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,20 @@ public Map<UUID, OrderableFulfillDto> findByIds(Collection<UUID> ids) {

return getMap(null, parameters, UUID.class, OrderableFulfillDto.class);
}

/**
* Finds orderables by facilityId and programId.
*
* @param facilityId id of facility which is used during searching orderables
* @param programId id of program which is used during searching orderables
* @return a page of orderables
*/
public Map<UUID, OrderableFulfillDto> findByFacilityIdProgramId(UUID facilityId, UUID programId) {
RequestParameters parameters = RequestParameters
.init()
.set("facilityId", facilityId)
.set("programId", programId);

return getMap(null, parameters, UUID.class, OrderableFulfillDto.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import static org.springframework.http.HttpStatus.CREATED;
import static org.springframework.http.HttpStatus.NO_CONTENT;
import static org.springframework.http.HttpStatus.OK;
import static org.springframework.web.bind.annotation.RequestMethod.DELETE;
import static org.springframework.web.bind.annotation.RequestMethod.POST;

import java.util.List;
import java.util.UUID;
Expand All @@ -35,8 +33,10 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity;
import org.springframework.util.MultiValueMap;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
Expand Down Expand Up @@ -81,7 +81,7 @@ public List<ValidSourceDestinationDto> getValidDestinations(
*
* @return the assigned destination and program and facility type.
*/
@RequestMapping(value = "/validDestinations", method = POST)
@PostMapping(value = "/validDestinations")
public ResponseEntity<ValidSourceDestinationDto> assignDestination(
@RequestBody ValidDestinationAssignment assignment) {
LOGGER.debug("Try to assign destinations");
Expand Down Expand Up @@ -120,7 +120,7 @@ public List<ValidSourceDestinationDto> getValidSources(
*
* @return the assigned source and program and facility type.
*/
@RequestMapping(value = "/validSources", method = POST)
@PostMapping(value = "/validSources")
public ResponseEntity<ValidSourceDestinationDto> assignSource(
@RequestBody ValidSourceAssignment assignment) {
LOGGER.debug("Try to assign source");
Expand All @@ -140,7 +140,7 @@ public ResponseEntity<ValidSourceDestinationDto> assignSource(
*
* @param assignmentId source assignment ID
*/
@RequestMapping(value = "/validSources/{id}", method = DELETE)
@DeleteMapping(value = "/validSources/{id}")
@ResponseStatus(NO_CONTENT)
public void removeValidSourceAssignment(@PathVariable("id") UUID assignmentId) {
LOGGER.debug(format("Try to remove source assignment %s.", assignmentId));
Expand All @@ -153,7 +153,7 @@ public void removeValidSourceAssignment(@PathVariable("id") UUID assignmentId) {
*
* @param assignmentId destination assignment ID
*/
@RequestMapping(value = "/validDestinations/{id}", method = DELETE)
@DeleteMapping(value = "/validDestinations/{id}")
@ResponseStatus(NO_CONTENT)
public void removeValidDestinationAssignment(@PathVariable("id") UUID assignmentId) {
LOGGER.debug(format("Try to remove destination assignment %s.", assignmentId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ public Page<StockCardSummaryV2Dto> getStockCardSummaries(

profiler.start("TO_DTO");
List<StockCardSummaryV2Dto> dtos = stockCardSummariesV2DtoBuilder.build(
summaries.getPageOfApprovedProducts(),
summaries.getStockCardsForFulfillOrderables(),
summaries.getOrderableFulfillMap(),
params.isNonEmptyOnly());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.apache.commons.collections4.MapUtils;
import org.openlmis.stockmanagement.domain.card.StockCard;
import org.openlmis.stockmanagement.dto.ObjectReferenceDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderableDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderableFulfillDto;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
Expand All @@ -49,17 +48,17 @@ public class StockCardSummariesV2DtoBuilder {
/**
* Builds Stock Card Summary dtos from stock cards and orderables.
*
* @param approvedProducts list of {@link OrderableDto} that summaries will be based on
* @param stockCards list of {@link StockCard} found for orderables
* @param orderables map of orderable ids as keys and {@link OrderableFulfillDto}
* @param stockCards list of {@link StockCard} found for orderables
* @param orderableFulfills map of orderable ids as keys and {@link OrderableFulfillDto}
* @param nonEmptySummariesOnly flag which allows filtering summaries with an empty
* getCanFulfillForMe collection
* @return list of {@link StockCardSummaryV2Dto}
*/
public List<StockCardSummaryV2Dto> build(List<OrderableDto> approvedProducts,
List<StockCard> stockCards, Map<UUID, OrderableFulfillDto> orderables,
boolean nonEmptySummariesOnly) {
Stream<StockCardSummaryV2Dto> summariesStream = approvedProducts.stream()
.map(p -> build(stockCards, p.getId(),
MapUtils.isEmpty(orderables) ? null : orderables.get(p.getId())))
public List<StockCardSummaryV2Dto> build(List<StockCard> stockCards,
Map<UUID, OrderableFulfillDto> orderableFulfills, boolean nonEmptySummariesOnly) {
Stream<StockCardSummaryV2Dto> summariesStream = orderableFulfills.keySet().stream()
.map(id -> build(stockCards, id,
MapUtils.isEmpty(orderableFulfills) ? null : orderableFulfills.get(id)))
.sorted();

if (nonEmptySummariesOnly) {
Expand Down Expand Up @@ -99,7 +98,7 @@ private List<CanFulfillForMeEntryDto> buildFulfillsEntries(UUID orderableId,
}
}

private CanFulfillForMeEntryDto createCanFulfillForMeEntry(StockCard stockCard,
private CanFulfillForMeEntryDto createCanFulfillForMeEntry(StockCard stockCard,
UUID orderableId) {
return new CanFulfillForMeEntryDto(
createStockCardReference(stockCard.getId()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.when;

Expand All @@ -50,15 +49,12 @@
import org.openlmis.stockmanagement.domain.event.StockEvent;
import org.openlmis.stockmanagement.domain.identity.OrderableLotIdentity;
import org.openlmis.stockmanagement.dto.StockCardDto;
import org.openlmis.stockmanagement.dto.referencedata.ApprovedProductDto;
import org.openlmis.stockmanagement.dto.referencedata.LotDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderableDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderableFulfillDto;
import org.openlmis.stockmanagement.dto.referencedata.OrderablesAggregator;
import org.openlmis.stockmanagement.exception.PermissionMessageException;
import org.openlmis.stockmanagement.repository.CalculatedStockOnHandRepository;
import org.openlmis.stockmanagement.repository.StockCardRepository;
import org.openlmis.stockmanagement.service.referencedata.ApprovedProductReferenceDataService;
import org.openlmis.stockmanagement.service.referencedata.FacilityReferenceDataService;
import org.openlmis.stockmanagement.service.referencedata.LotReferenceDataService;
import org.openlmis.stockmanagement.service.referencedata.OrderableFulfillReferenceDataService;
Expand All @@ -75,9 +71,6 @@
@RunWith(MockitoJUnitRunner.class)
public class StockCardSummariesServiceTest {

@Mock
private ApprovedProductReferenceDataService approvedProductReferenceDataService;

@Mock
private OrderableFulfillReferenceDataService orderableFulfillReferenceDataService;

Expand Down Expand Up @@ -192,29 +185,19 @@ public void shouldFindStockCards() {
OrderableDto orderable2 = new OrderableDtoDataBuilder().build();
OrderableDto orderable3 = new OrderableDtoDataBuilder().build();

OrderablesAggregator orderablesAggregator = new OrderablesAggregator(asList(
new ApprovedProductDto(orderable),
new ApprovedProductDto(orderable2),
new ApprovedProductDto(orderable3)
));

StockCardSummariesV2SearchParams params = new StockCardSummariesV2SearchParamsDataBuilder()
.withOrderableIds(asList(orderable.getId(), orderable2.getId()))
.build();

when(approvedProductReferenceDataService
.getApprovedProducts(eq(params.getFacilityId()), eq(params.getProgramId()),
eq(params.getOrderableIds())))
.thenReturn(orderablesAggregator);

Map<UUID, OrderableFulfillDto> fulfillMap = new HashMap<>();
fulfillMap.put(orderable.getId(), new OrderableFulfillDtoDataBuilder()
.withCanFulfillForMe(asList(orderable2.getId(), orderable3.getId())).build());
fulfillMap.put(orderable2.getId(), new OrderableFulfillDtoDataBuilder()
.withCanFulfillForMe(asList(orderable.getId(), orderable3.getId())).build());
fulfillMap.put(orderable3.getId(), new OrderableFulfillDtoDataBuilder().build());

StockCardSummariesV2SearchParams params = new StockCardSummariesV2SearchParamsDataBuilder()
.withOrderableIds(asList(orderable.getId(), orderable2.getId()))
.build();

when(orderableFulfillReferenceDataService
.findByIds(asList(orderable.getId(), orderable2.getId(), orderable3.getId())))
.findByFacilityIdProgramId(params.getFacilityId(), params.getProgramId()))
.thenReturn(fulfillMap);

StockEvent event = new StockEventDataBuilder()
Expand All @@ -238,15 +221,15 @@ public void shouldFindStockCards() {
.getStockCardsWithStockOnHand(params.getProgramId(), params.getFacilityId(),
params.getAsOfDate()))
.thenReturn(stockCards);

when(cardRepository.findByProgramIdAndFacilityId(
params.getProgramId(),
params.getFacilityId()))
.thenReturn(stockCards);

StockCardSummaries result = stockCardSummariesService.findStockCards(params);

assertEquals(3, result.getPageOfApprovedProducts().size());
assertEquals(3, result.getOrderableFulfillMap().size());
}

@Test(expected = PermissionMessageException.class)
Expand Down
Loading

0 comments on commit dde6c40

Please sign in to comment.