Skip to content

Commit

Permalink
OAM-223: Don't read all orderables to crated SockCardSummaries (#58)
Browse files Browse the repository at this point in the history
* OAM-223: Don't read all orderables to crated SockCardSummaries

* OAM-223: Added LotReferenceDataService unit test
  • Loading branch information
pwargulak committed Jun 19, 2024
1 parent 87e17c9 commit 6da4960
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -169,7 +168,7 @@ public void shouldReturnPageOfStockCards() throws Exception {
PageRequest pageRequest = PageRequest.of(0, 1);
//when
Page<StockCardDto> stockCards = stockCardSummariesService
.findStockCards(programId, facilityId, pageRequest);
.findStockCards(programId, facilityId, pageRequest, new Profiler("TEST"));

//then
assertThat(stockCards.getContent().size(), is(1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -215,4 +217,4 @@ public void shouldReturn404WhenStockCardNotFoundWhileMakingInactive() throws Exc
// then
resultActions.andExpect(status().isNotFound());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;
import lombok.ToString;
import org.openlmis.stockmanagement.domain.BaseEntity;

@Entity
Expand Down Expand Up @@ -54,6 +55,7 @@ public class StockEvent extends BaseEntity {

private String documentNumber;

@ToString.Exclude
@OneToMany(cascade = ALL, mappedBy = "stockEvent")
private List<StockEventLineItem> lineItems;
}
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.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;
Expand All @@ -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;
Expand Down Expand Up @@ -223,10 +226,13 @@ public List<StockCardDto> findStockCards(UUID programId, UUID facilityId) {
* @param pageable page object.
* @return page of stock cards.
*/
public Page<StockCardDto> findStockCards(UUID programId, UUID facilityId, Pageable pageable) {
public Page<StockCardDto> findStockCards(UUID programId, UUID facilityId, Pageable pageable,
Profiler profiler) {
profiler.start("FIND_BY_PROGRAM_AND_FACILITY");
Page<StockCard> pageOfCards = stockCardRepository
.findByProgramIdAndFacilityId(programId, facilityId, pageable);

profiler.start("CARDS_TO_DTO");
List<StockCardDto> cardDtos = cardsToDtos(pageOfCards.getContent());
return new PageImpl<>(cardDtos, pageable, pageOfCards.getTotalElements());
}
Expand Down Expand Up @@ -256,10 +262,23 @@ public List<StockCardDto> createDummyStockCards(UUID programId, UUID facilityId)

private List<StockCardDto> cardsToDtos(List<StockCard> cards) {
LOGGER.info("Calling ref data to get all approved orderables");
Map<OrderableLotIdentity, OrderableLot> orderableLotsMap = createOrderableLotMap(
orderableReferenceDataService.findAll());

return loadOrderableLotUnitAndRemoveLineItems(createDtos(cards), orderableLotsMap);
final Set<OrderableLotIdentity> orderableLotsMapIds = cards.stream().map(
stockCard -> new OrderableLotIdentity(stockCard.getOrderableId(), stockCard.getLotId()))
.collect(Collectors.toSet());

final Map<UUID, OrderableDto> orderables = orderableReferenceDataService.findByIds(
orderableLotsMapIds.stream().map(OrderableLotIdentity::getOrderableId).collect(toSet()))
.stream().collect(toMap(OrderableDto::getId, identity()));
final Map<UUID, LotDto> 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<StockCardDto> loadOrderableLotUnitAndRemoveLineItems(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<LotDto> {
Expand Down Expand Up @@ -71,4 +75,16 @@ private List<LotDto> 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<LotDto> findByIds(Collection<UUID> ids) {
return CollectionUtils.isEmpty(ids)
? Collections.emptyList()
: getPage(RequestParameters.init().set("id", ids)).getContent();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@

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;
import org.openlmis.stockmanagement.service.StockCardSummariesService;
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;
Expand Down Expand Up @@ -100,9 +100,17 @@ public Page<StockCardDto> 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();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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<StockCardDto> stockCardsDtos =
stockCardSummariesService.findStockCards(programId, facilityId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,4 +101,25 @@ public void getAllLotsExpiringOnDateShouldReturnMatchingLots() {
assertAuthHeader(entityCaptor.getValue());
assertNull(entityCaptor.getValue().getBody());
}

@Test
public void findByIdsShouldReturnMatchingLots() {
LotDto lot = mockPageResponseEntityAndGetDto();

List<LotDto> 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());
}
}

0 comments on commit 6da4960

Please sign in to comment.