From ef3879e186928b96f97ecf3e1f208d24225d2257 Mon Sep 17 00:00:00 2001 From: Vignesh-Kalyanasundaram Date: Mon, 18 Dec 2023 14:24:58 +0530 Subject: [PATCH] MODDCB-86 Handling invalid barcode issue in Lender role --- .../feign/InventoryItemStorageClient.java | 5 +-- .../ExceptionHandlingController.java | 4 ++- .../org/folio/dcb/service/ItemService.java | 13 +++---- .../dcb/service/impl/ItemServiceImpl.java | 23 ++++++------- .../dcb/service/impl/RequestServiceImpl.java | 2 +- .../swagger.api/schemas/InventoryItem.yaml | 3 ++ .../TransactionApiControllerTest.java | 34 +++++++++++++++++++ .../dcb/service/InventoryItemServiceTest.java | 22 ++++++------ .../folio/dcb/service/RequestServiceTest.java | 4 +-- .../java/org/folio/dcb/utils/EntityUtils.java | 1 + src/test/resources/mappings/inventory.json | 9 ++--- 11 files changed, 79 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/folio/dcb/client/feign/InventoryItemStorageClient.java b/src/main/java/org/folio/dcb/client/feign/InventoryItemStorageClient.java index f424b40b..b7387385 100644 --- a/src/main/java/org/folio/dcb/client/feign/InventoryItemStorageClient.java +++ b/src/main/java/org/folio/dcb/client/feign/InventoryItemStorageClient.java @@ -5,14 +5,11 @@ import org.folio.spring.model.ResultList; import org.springframework.cloud.openfeign.FeignClient; import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestParam; @FeignClient(name = "item-storage", configuration = FeignClientConfiguration.class) public interface InventoryItemStorageClient { - @GetMapping("/items/{itemId}") - InventoryItem findItem(@PathVariable("itemId") String itemId); @GetMapping("/items") - ResultList fetchItemByBarcode(@RequestParam("query") String query); + ResultList fetchItemByQuery(@RequestParam("query") String query); } diff --git a/src/main/java/org/folio/dcb/controller/ExceptionHandlingController.java b/src/main/java/org/folio/dcb/controller/ExceptionHandlingController.java index e85bb066..797c29ba 100644 --- a/src/main/java/org/folio/dcb/controller/ExceptionHandlingController.java +++ b/src/main/java/org/folio/dcb/controller/ExceptionHandlingController.java @@ -7,6 +7,7 @@ import org.folio.spring.exception.NotFoundException; import org.springframework.http.HttpStatus; import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ResponseStatus; @@ -68,7 +69,8 @@ public Errors handleBadGatewayException(FeignException.BadGateway ex) { HttpMessageNotReadableException.class, IllegalArgumentException.class, StatusException.class, - FeignException.BadRequest.class + FeignException.BadRequest.class, + MethodArgumentNotValidException.class }) public Errors handleValidationErrors(Exception ex) { logExceptionMessage(ex); diff --git a/src/main/java/org/folio/dcb/service/ItemService.java b/src/main/java/org/folio/dcb/service/ItemService.java index caacd4e4..416a3113 100644 --- a/src/main/java/org/folio/dcb/service/ItemService.java +++ b/src/main/java/org/folio/dcb/service/ItemService.java @@ -4,12 +4,6 @@ import org.folio.spring.model.ResultList; public interface ItemService { - /** - * Get item details of an inventory by itemId - * @param itemId - id of an item - * @return InventoryItem - */ - InventoryItem fetchItemDetailsById(String itemId); /** * Provides material type Id by material type name. @@ -24,4 +18,11 @@ public interface ItemService { * @return InventoryItem */ ResultList fetchItemByBarcode(String itemBarcode); + /** + * Get item details of an inventory by item id and Barcode + * @param itemBarcode - barcode of an item + * @param id - id of an item + * @return InventoryItem + */ + InventoryItem fetchItemByIdAndBarcode(String id, String itemBarcode); } diff --git a/src/main/java/org/folio/dcb/service/impl/ItemServiceImpl.java b/src/main/java/org/folio/dcb/service/impl/ItemServiceImpl.java index 949ab6c3..eaf2464f 100644 --- a/src/main/java/org/folio/dcb/service/impl/ItemServiceImpl.java +++ b/src/main/java/org/folio/dcb/service/impl/ItemServiceImpl.java @@ -1,6 +1,5 @@ package org.folio.dcb.service.impl; -import feign.FeignException; import lombok.AllArgsConstructor; import lombok.extern.log4j.Log4j2; import org.folio.dcb.client.feign.InventoryItemStorageClient; @@ -19,16 +18,6 @@ public class ItemServiceImpl implements ItemService { private final InventoryItemStorageClient inventoryItemStorageClient; private final MaterialTypeClient materialTypeClient; - @Override - public InventoryItem fetchItemDetailsById(String itemId) { - log.debug("fetchItemDetailsById:: Trying to fetch item details for itemId {}", itemId); - try { - return inventoryItemStorageClient.findItem(itemId); - } catch (FeignException.NotFound ex) { - throw new NotFoundException(String.format("Item not found for itemId %s ", itemId)); - } - } - @Override public String fetchItemMaterialTypeIdByMaterialTypeName(String materialTypeName) { log.debug("fetchItemMaterialTypeIdByMaterialTypeName:: Fetching ItemMaterialTypeId by MaterialTypeName={}", materialTypeName); @@ -43,7 +32,17 @@ public String fetchItemMaterialTypeIdByMaterialTypeName(String materialTypeName) @Override public ResultList fetchItemByBarcode(String itemBarcode) { log.debug("fetchItemByBarcode:: fetching item details for barcode {} ", itemBarcode); - return inventoryItemStorageClient.fetchItemByBarcode("barcode==" + itemBarcode); + return inventoryItemStorageClient.fetchItemByQuery("barcode==" + itemBarcode); + } + + @Override + public InventoryItem fetchItemByIdAndBarcode(String id, String barcode) { + log.debug("fetchItemByBarcode:: fetching item details for id {} , barcode {} ", id, barcode); + return inventoryItemStorageClient.fetchItemByQuery("barcode==" + barcode + " and id==" + id) + .getResult() + .stream() + .findFirst() + .orElseThrow(() -> new NotFoundException(String.format("Unable to find existing item with id %s and barcode %s.", id, barcode))); } } diff --git a/src/main/java/org/folio/dcb/service/impl/RequestServiceImpl.java b/src/main/java/org/folio/dcb/service/impl/RequestServiceImpl.java index 3643aed8..65c1f8a3 100644 --- a/src/main/java/org/folio/dcb/service/impl/RequestServiceImpl.java +++ b/src/main/java/org/folio/dcb/service/impl/RequestServiceImpl.java @@ -34,7 +34,7 @@ public class RequestServiceImpl implements RequestService { public CirculationRequest createPageItemRequest(User user, DcbItem item, String pickupServicePointId) { log.debug("createPageItemRequest:: creating a new page request for userBarcode {} , itemBarcode {}", user.getBarcode(), item.getBarcode()); - var inventoryItem = itemService.fetchItemDetailsById(item.getId()); + var inventoryItem = itemService.fetchItemByIdAndBarcode(item.getId(), item.getBarcode()); var inventoryHolding = holdingsService.fetchInventoryHoldingDetailsByHoldingId(inventoryItem.getHoldingsRecordId()); var circulationRequest = createCirculationRequest(PAGE, user, item, inventoryItem.getHoldingsRecordId(), inventoryHolding.getInstanceId(), pickupServicePointId); return circulationClient.createRequest(circulationRequest); diff --git a/src/main/resources/swagger.api/schemas/InventoryItem.yaml b/src/main/resources/swagger.api/schemas/InventoryItem.yaml index 52cf16b7..97d374ac 100644 --- a/src/main/resources/swagger.api/schemas/InventoryItem.yaml +++ b/src/main/resources/swagger.api/schemas/InventoryItem.yaml @@ -5,6 +5,9 @@ InventoryItem: id: description: Item id $ref: "uuid.yaml" + barcode: + description: Barcode of the item + type: string holdingsRecordId: description: ID of the holdings record the item is a member of $ref: "uuid.yaml" diff --git a/src/test/java/org/folio/dcb/controller/TransactionApiControllerTest.java b/src/test/java/org/folio/dcb/controller/TransactionApiControllerTest.java index d165b97e..c3e59927 100644 --- a/src/test/java/org/folio/dcb/controller/TransactionApiControllerTest.java +++ b/src/test/java/org/folio/dcb/controller/TransactionApiControllerTest.java @@ -87,6 +87,40 @@ void createLendingCirculationRequestTest() throws Exception { ); } + @Test + void createLendingCirculationRequestWithValidIdAndInvalidBarcode() throws Exception { + removeExistedTransactionFromDbIfSoExists(); + var dcbTransaction = createDcbTransactionByRole(LENDER); + dcbTransaction.getItem().setBarcode("DCB_ITEM1"); + + this.mockMvc.perform( + post("/transactions/" + DCB_TRANSACTION_ID) + .content(asJsonString(dcbTransaction)) + .headers(defaultHeaders()) + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)) + .andExpectAll(status().is4xxClientError(), + jsonPath("$.errors[0].code", is("NOT_FOUND_ERROR"))); + + } + + @Test + void createCirculationRequestWithInvalidUUID() throws Exception { + removeExistedTransactionFromDbIfSoExists(); + var dcbTransaction = createDcbTransactionByRole(LENDER); + //Setting a non UUID for itemId + dcbTransaction.getItem().setId("1234"); + + this.mockMvc.perform( + post("/transactions/" + DCB_TRANSACTION_ID) + .content(asJsonString(dcbTransaction)) + .headers(defaultHeaders()) + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON)) + .andExpectAll(status().is4xxClientError(), + jsonPath("$.errors[0].code", is("VALIDATION_ERROR"))); + } + @Test void createBorrowingPickupCirculationRequestTest() throws Exception { removeExistedTransactionFromDbIfSoExists(); diff --git a/src/test/java/org/folio/dcb/service/InventoryItemServiceTest.java b/src/test/java/org/folio/dcb/service/InventoryItemServiceTest.java index ada66d68..b1df04b2 100644 --- a/src/test/java/org/folio/dcb/service/InventoryItemServiceTest.java +++ b/src/test/java/org/folio/dcb/service/InventoryItemServiceTest.java @@ -1,6 +1,5 @@ package org.folio.dcb.service; -import feign.FeignException; import org.folio.dcb.client.feign.InventoryItemStorageClient; import org.folio.dcb.service.impl.ItemServiceImpl; import org.folio.spring.exception.NotFoundException; @@ -18,8 +17,6 @@ import static org.folio.dcb.utils.EntityUtils.createInventoryItem; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -31,27 +28,30 @@ class InventoryItemServiceTest { private InventoryItemStorageClient inventoryItemStorageClient; @Test - void fetchItemDetailsByIdTest() { + void fetchItemDetailsByIdAndBarcodeTest() { var itemId = UUID.randomUUID().toString(); + var barcode = "DCB_ITEM"; var inventoryItem = createInventoryItem(); - when(inventoryItemStorageClient.findItem(itemId)).thenReturn(inventoryItem); - var response = itemService.fetchItemDetailsById(itemId); - verify(inventoryItemStorageClient).findItem(itemId); + when(inventoryItemStorageClient.fetchItemByQuery("barcode==" + barcode + " and id==" + itemId)).thenReturn(ResultList.of(1, List.of(inventoryItem))); + + var response = itemService.fetchItemByIdAndBarcode(itemId, barcode); assertEquals(inventoryItem, response); } @Test - void fetchItemDetailsByInvalidIdTest() { + void fetchItemByIdAndInvalidBarcode() { var itemId = UUID.randomUUID().toString(); - doThrow(FeignException.NotFound.class).when(inventoryItemStorageClient).findItem(itemId); - assertThrows(NotFoundException.class, () -> itemService.fetchItemDetailsById(itemId)); + var barcode = "DCB_ITEM"; + when(inventoryItemStorageClient.fetchItemByQuery("barcode==" + barcode + " and id==" + itemId)).thenReturn(ResultList.of(0, List.of())); + + assertThrows(NotFoundException.class, () -> itemService.fetchItemByIdAndBarcode(itemId, barcode)); } @Test void fetchItemByBarcode() { var item = createDcbItem(); var inventoryItem = createInventoryItem(); - when(itemService.fetchItemByBarcode(item.getBarcode())).thenReturn(ResultList.of(1, List.of(inventoryItem))); + when(inventoryItemStorageClient.fetchItemByQuery("barcode==" + item.getBarcode())).thenReturn(ResultList.of(1, List.of(inventoryItem))); var response = itemService.fetchItemByBarcode(item.getBarcode()); assertEquals(1, response.getTotalRecords()); diff --git a/src/test/java/org/folio/dcb/service/RequestServiceTest.java b/src/test/java/org/folio/dcb/service/RequestServiceTest.java index e93a7b85..dc6511a8 100644 --- a/src/test/java/org/folio/dcb/service/RequestServiceTest.java +++ b/src/test/java/org/folio/dcb/service/RequestServiceTest.java @@ -33,10 +33,10 @@ class RequestServiceTest { @Test void createPageItemRequestTest() { - when(itemService.fetchItemDetailsById(any())).thenReturn(createInventoryItem()); + when(itemService.fetchItemByIdAndBarcode(any(), any())).thenReturn(createInventoryItem()); when(holdingsService.fetchInventoryHoldingDetailsByHoldingId(any())).thenReturn(createInventoryHolding()); requestService.createPageItemRequest(createUser(), createDcbItem(), createDcbPickup().getServicePointId()); - verify(itemService).fetchItemDetailsById(any()); + verify(itemService).fetchItemByIdAndBarcode(any(), any()); verify(holdingsService).fetchInventoryHoldingDetailsByHoldingId(any()); verify(circulationClient).createRequest(any()); } diff --git a/src/test/java/org/folio/dcb/utils/EntityUtils.java b/src/test/java/org/folio/dcb/utils/EntityUtils.java index a106ef48..e3f15754 100644 --- a/src/test/java/org/folio/dcb/utils/EntityUtils.java +++ b/src/test/java/org/folio/dcb/utils/EntityUtils.java @@ -195,6 +195,7 @@ public static InventoryItem createInventoryItem() { return InventoryItem.builder() .id(UUID.randomUUID().toString()) .holdingsRecordId(UUID.randomUUID().toString()) + .barcode("DCB_ITEM") .build(); } diff --git a/src/test/resources/mappings/inventory.json b/src/test/resources/mappings/inventory.json index ba92db6d..94cc99d8 100644 --- a/src/test/resources/mappings/inventory.json +++ b/src/test/resources/mappings/inventory.json @@ -3,11 +3,11 @@ { "request": { "method": "GET", - "url": "/item-storage/items/5b95877d-86c0-4cb7-a0cd-7660b348ae5a" + "url": "/item-storage/items?query=barcode%3D%3DDCB_ITEM%20and%20id%3D%3D5b95877d-86c0-4cb7-a0cd-7660b348ae5a" }, "response": { "status": 200, - "body": "{\"id\":\"5b95877d-86c0-4cb7-a0cd-7660b348ae5a\",\"holdingsRecordId\": \"fb7b70f1-b898-4924-a991-0e4b6312bb5f\",\"status\": {\n \"name\": \"Available\",\n \"date\": \"2023-09-21T01:49:50.620+00:00\"\n}}", + "body": "{\"items\": [{ \"id\":\"5b95877d-86c0-4cb7-a0cd-7660b348ae5a\",\"barcode\":\"DCB_ITEM\",\"holdingsRecordId\": \"fb7b70f1-b898-4924-a991-0e4b6312bb5f\",\"status\": {\n \"name\": \"Available\",\n \"date\": \"2023-09-21T01:49:50.620+00:00\"\n}}],\n\"totalRecords\": 1,\n \"resultInfo\": {\n \"totalRecords\": 1,\n \"facets\": [],\n \"diagnostics\": []\n }}", "headers": { "Content-Type": "application/json" } @@ -16,10 +16,11 @@ { "request": { "method": "GET", - "url": "/item-storage/items/5b95877d-86c0-4cb7-a0cd-7660b348ae5b" + "url": "/item-storage/items?query=barcode%3D%3DDCB_ITEM1%20and%20id%3D%3D5b95877d-86c0-4cb7-a0cd-7660b348ae5a" }, "response": { - "status": 404, + "status": 200, + "body": "{\n\"items\": [],\n\"totalRecords\": 0,\n \"resultInfo\": {\n \"totalRecords\": 0,\n \"facets\": [],\n \"diagnostics\": []\n }\n}", "headers": { "Content-Type": "application/json" }