From 4187f96ec00c02893eb9f44bb04ae009a226311d Mon Sep 17 00:00:00 2001 From: Piotr Wargulak Date: Wed, 19 Jun 2024 16:15:21 +0200 Subject: [PATCH 1/2] OAM-223: Don't read all orderables to crated SockCardSummaries --- ...ckCardSummariesServiceIntegrationTest.java | 9 +++--- .../StockCardControllerIntegrationTest.java | 6 ++-- .../domain/event/StockEvent.java | 2 ++ .../service/StockCardSummariesService.java | 29 +++++++++++++++---- .../LotReferenceDataService.java | 16 ++++++++++ .../web/StockCardsController.java | 14 +++++++-- .../StockCardSummariesServiceTest.java | 15 ++++++++++ 7 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/integration-test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceIntegrationTest.java b/src/integration-test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceIntegrationTest.java index c24b02a2..66d1103d 100644 --- a/src/integration-test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceIntegrationTest.java +++ b/src/integration-test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceIntegrationTest.java @@ -57,6 +57,7 @@ import org.openlmis.stockmanagement.service.referencedata.OrderableReferenceDataService; import org.openlmis.stockmanagement.service.referencedata.ProgramReferenceDataService; import org.openlmis.stockmanagement.testutils.StockEventDataBuilder; +import org.slf4j.profiler.Profiler; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; @@ -115,11 +116,9 @@ public void shouldFindExistingStockCards() OrderableDto orderable3 = createOrderableDto(orderable3Id, ""); OrderableDto orderable4 = createOrderableDto(orderable4Id, ""); - when(orderableReferenceDataService - .findAll()) + when(orderableReferenceDataService.findByIds(any())) .thenReturn(asList(orderable1, orderable2, orderable3, orderable4)); - when(lotReferenceDataService.getAllLotsOf(any(UUID.class))) - .thenReturn(emptyList()); + when(lotReferenceDataService.findByIds(any())).thenReturn(emptyList()); when(facilityReferenceDataService.findOne(any(UUID.class))) .thenReturn(new FacilityDto()); when(programReferenceDataService.findOne(any(UUID.class))) @@ -169,7 +168,7 @@ public void shouldReturnPageOfStockCards() throws Exception { PageRequest pageRequest = PageRequest.of(0, 1); //when Page stockCards = stockCardSummariesService - .findStockCards(programId, facilityId, pageRequest); + .findStockCards(programId, facilityId, pageRequest, new Profiler("TEST")); //then assertThat(stockCards.getContent().size(), is(1)); diff --git a/src/integration-test/java/org/openlmis/stockmanagement/web/StockCardControllerIntegrationTest.java b/src/integration-test/java/org/openlmis/stockmanagement/web/StockCardControllerIntegrationTest.java index 77c5d99a..38cad4ac 100644 --- a/src/integration-test/java/org/openlmis/stockmanagement/web/StockCardControllerIntegrationTest.java +++ b/src/integration-test/java/org/openlmis/stockmanagement/web/StockCardControllerIntegrationTest.java @@ -17,6 +17,7 @@ import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.hasSize; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Matchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; @@ -42,6 +43,7 @@ import org.openlmis.stockmanagement.service.StockCardSummariesService; import org.openlmis.stockmanagement.testutils.StockCardDtoDataBuilder; import org.openlmis.stockmanagement.util.Message; +import org.slf4j.profiler.Profiler; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -146,7 +148,7 @@ public void shouldGetPagedStockCardSummariesWhenPermissionIsGranted() throws Exc PageRequest pageable = PageRequest.of(0, 20); when(stockCardSummariesService - .findStockCards(programId, facilityId, pageable)) + .findStockCards(eq(programId), eq(facilityId), eq(pageable), any(Profiler.class))) .thenReturn(new PageImpl<>(singletonList(StockCardDtoDataBuilder.createStockCardDto()))); ResultActions resultActions = mvc.perform( @@ -215,4 +217,4 @@ public void shouldReturn404WhenStockCardNotFoundWhileMakingInactive() throws Exc // then resultActions.andExpect(status().isNotFound()); } -} \ No newline at end of file +} diff --git a/src/main/java/org/openlmis/stockmanagement/domain/event/StockEvent.java b/src/main/java/org/openlmis/stockmanagement/domain/event/StockEvent.java index c0e1e71a..425632ec 100644 --- a/src/main/java/org/openlmis/stockmanagement/domain/event/StockEvent.java +++ b/src/main/java/org/openlmis/stockmanagement/domain/event/StockEvent.java @@ -27,6 +27,7 @@ import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import lombok.ToString; import org.openlmis.stockmanagement.domain.BaseEntity; @Entity @@ -54,6 +55,7 @@ public class StockEvent extends BaseEntity { private String documentNumber; + @ToString.Exclude @OneToMany(cascade = ALL, mappedBy = "stockEvent") private List lineItems; } diff --git a/src/main/java/org/openlmis/stockmanagement/service/StockCardSummariesService.java b/src/main/java/org/openlmis/stockmanagement/service/StockCardSummariesService.java index ef9c5739..8abdff3e 100644 --- a/src/main/java/org/openlmis/stockmanagement/service/StockCardSummariesService.java +++ b/src/main/java/org/openlmis/stockmanagement/service/StockCardSummariesService.java @@ -16,6 +16,7 @@ package org.openlmis.stockmanagement.service; import static java.util.Collections.emptyList; +import static java.util.function.Function.identity; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toSet; @@ -29,9 +30,11 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import java.util.stream.Stream; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; @@ -223,10 +226,13 @@ public List findStockCards(UUID programId, UUID facilityId) { * @param pageable page object. * @return page of stock cards. */ - public Page findStockCards(UUID programId, UUID facilityId, Pageable pageable) { + public Page findStockCards(UUID programId, UUID facilityId, Pageable pageable, + Profiler profiler) { + profiler.start("FIND_BY_PROGRAM_AND_FACILITY"); Page pageOfCards = stockCardRepository .findByProgramIdAndFacilityId(programId, facilityId, pageable); + profiler.start("CARDS_TO_DTO"); List cardDtos = cardsToDtos(pageOfCards.getContent()); return new PageImpl<>(cardDtos, pageable, pageOfCards.getTotalElements()); } @@ -256,10 +262,23 @@ public List createDummyStockCards(UUID programId, UUID facilityId) private List cardsToDtos(List cards) { LOGGER.info("Calling ref data to get all approved orderables"); - Map orderableLotsMap = createOrderableLotMap( - orderableReferenceDataService.findAll()); - - return loadOrderableLotUnitAndRemoveLineItems(createDtos(cards), orderableLotsMap); + final Set orderableLotsMapIds = cards.stream().map( + stockCard -> new OrderableLotIdentity(stockCard.getOrderableId(), stockCard.getLotId())) + .collect(Collectors.toSet()); + + final Map orderables = orderableReferenceDataService.findByIds( + orderableLotsMapIds.stream().map(OrderableLotIdentity::getOrderableId).collect(toSet())) + .stream().collect(toMap(OrderableDto::getId, identity())); + final Map lots = lotReferenceDataService.findByIds( + orderableLotsMapIds.stream().map(OrderableLotIdentity::getLotId).filter(Objects::nonNull) + .collect(toSet())).stream().collect(toMap(LotDto::getId, identity())); + + return createDtos(cards).stream().map(cardDto -> { + cardDto.setOrderable(orderables.get(cardDto.getOrderableId())); + cardDto.setLot(cardDto.getLotId() != null ? lots.get(cardDto.getLotId()) : null); + cardDto.setLineItems(null); + return cardDto; + }).collect(Collectors.toList()); } private List loadOrderableLotUnitAndRemoveLineItems( diff --git a/src/main/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataService.java b/src/main/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataService.java index b101f6c9..c5b3c8c6 100644 --- a/src/main/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataService.java +++ b/src/main/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataService.java @@ -16,11 +16,15 @@ package org.openlmis.stockmanagement.service.referencedata; import java.time.LocalDate; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.UUID; import org.openlmis.stockmanagement.dto.referencedata.LotDto; +import org.openlmis.stockmanagement.util.RequestParameters; import org.springframework.stereotype.Service; +import org.springframework.util.CollectionUtils; @Service public class LotReferenceDataService extends BaseReferenceDataService { @@ -71,4 +75,16 @@ private List getAllLotsMatching(UUID tradeItemId, LocalDate expirationDa return getPage(params).getContent(); } + + /** + * Find Lot by IDs. + * + * @param ids the ids, not null + * @return the list of lots, never null + */ + public List findByIds(Collection ids) { + return CollectionUtils.isEmpty(ids) + ? Collections.emptyList() + : getPage(RequestParameters.init().set("id", ids)).getContent(); + } } diff --git a/src/main/java/org/openlmis/stockmanagement/web/StockCardsController.java b/src/main/java/org/openlmis/stockmanagement/web/StockCardsController.java index 5e4931c6..3fe32dbb 100644 --- a/src/main/java/org/openlmis/stockmanagement/web/StockCardsController.java +++ b/src/main/java/org/openlmis/stockmanagement/web/StockCardsController.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.UUID; - import org.openlmis.stockmanagement.dto.StockCardDto; import org.openlmis.stockmanagement.service.PermissionService; import org.openlmis.stockmanagement.service.StockCardService; @@ -28,6 +27,7 @@ import org.openlmis.stockmanagement.util.UuidUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.slf4j.profiler.Profiler; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -100,9 +100,17 @@ public Page getStockCardSummaries( @RequestParam() UUID facility, Pageable pageable ) { - LOGGER.debug("Try to find stock card summaries"); + Profiler profiler = new Profiler("GET_STOCK_CARDS_SUMMARIES"); + profiler.setLogger(LOGGER); + profiler.start("CAN_VIEW_STOCK_CARD"); permissionService.canViewStockCard(program, facility); - return stockCardSummariesService.findStockCards(program, facility, pageable); + profiler.start("FIND_STOCK_CARDS"); + try { + return stockCardSummariesService + .findStockCards(program, facility, pageable, profiler.startNested("FIND_STOCK_CARDS")); + } finally { + profiler.stop().log(); + } } /** diff --git a/src/test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceTest.java b/src/test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceTest.java index e17f635b..e45d32a0 100644 --- a/src/test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceTest.java +++ b/src/test/java/org/openlmis/stockmanagement/service/StockCardSummariesServiceTest.java @@ -36,8 +36,10 @@ import com.google.common.collect.ImmutableSet; import java.time.LocalDate; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -480,6 +482,13 @@ public void shouldFindStockCardsForProgramAndFacilityIds() { UUID lotId2 = UUID.randomUUID(); UUID unitOfOrderable1 = UUID.randomUUID(); UUID unitOfOrderable2 = UUID.randomUUID(); + + OrderableDto orderable1 = createOrderableDto(orderableId1, "1"); + OrderableDto orderable2 = createOrderableDto(orderableId2, "2"); + + LotDto lot1 = LotDto.builder().id(lotId1).build(); + LotDto lot2 = LotDto.builder().id(lotId2).build(); + StockEvent event = new StockEventDataBuilder() .withFacility(facilityId) .withProgram(programId) @@ -504,6 +513,12 @@ public void shouldFindStockCardsForProgramAndFacilityIds() { when(cardRepository.findByProgramIdAndFacilityId(programId, facilityId)) .thenReturn(stockCards); + when(orderableReferenceDataService + .findByIds(new HashSet<>(Arrays.asList(orderableId1, orderableId2)))) + .thenReturn(Arrays.asList(orderable1, orderable2)); + when(lotReferenceDataService.findByIds(new HashSet<>(Arrays.asList(lotId1, lotId2)))) + .thenReturn(Arrays.asList(lot1, lot2)); + //when List stockCardsDtos = stockCardSummariesService.findStockCards(programId, facilityId); From 5ba053aac26ca3d721fd3c56fe1656e48befd8f0 Mon Sep 17 00:00:00 2001 From: Piotr Wargulak Date: Wed, 19 Jun 2024 17:03:55 +0200 Subject: [PATCH 2/2] OAM-223: Added LotReferenceDataService unit test --- .../LotReferenceDataServiceTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/test/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataServiceTest.java b/src/test/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataServiceTest.java index 5b9be24b..a5709dc0 100644 --- a/src/test/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataServiceTest.java +++ b/src/test/java/org/openlmis/stockmanagement/service/referencedata/LotReferenceDataServiceTest.java @@ -15,6 +15,7 @@ package org.openlmis.stockmanagement.service.referencedata; +import static java.util.Collections.singletonList; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertEquals; @@ -100,4 +101,25 @@ public void getAllLotsExpiringOnDateShouldReturnMatchingLots() { assertAuthHeader(entityCaptor.getValue()); assertNull(entityCaptor.getValue().getBody()); } + + @Test + public void findByIdsShouldReturnMatchingLots() { + LotDto lot = mockPageResponseEntityAndGetDto(); + + List response = service.findByIds(singletonList(lot.getId())); + + assertThat(response, hasSize(1)); + assertThat(response, hasItem(lot)); + + verify(restTemplate).exchange( + uriCaptor.capture(), eq(HttpMethod.GET), entityCaptor.capture(), + refEq(new DynamicPageTypeReference<>(LotDto.class))); + + URI uri = uriCaptor.getValue(); + assertEquals(serviceUrl + service.getUrl() + "?id=" + lot.getId().toString(), + uri.toString()); + + assertAuthHeader(entityCaptor.getValue()); + assertNull(entityCaptor.getValue().getBody()); + } }