From 131db36965dfe3b8abf0b853b0d1544958c703c7 Mon Sep 17 00:00:00 2001 From: Irakli Merabishvili Date: Tue, 21 Jan 2025 17:05:47 +0400 Subject: [PATCH 1/7] MCBFF-38: Update hardcoded effective location during checkin --- descriptors/ModuleDescriptor-template.json | 6 +- .../service/InventoryService.java | 9 ++ .../circulationbff/service/SearchService.java | 2 + .../service/UserTenantsService.java | 1 + .../service/impl/CheckInServiceImpl.java | 58 ++++++- .../service/impl/InventoryServiceImpl.java | 30 ++++ .../service/impl/SearchServiceImpl.java | 12 ++ .../service/impl/UserTenantsServiceImpl.java | 12 ++ .../schemas/response/check-in-response.yaml | 134 ++++++++++++++++- .../circulationbff/api/CheckInApiTest.java | 141 +++++++++++++++++- .../service/SearchServiceTest.java | 17 +++ 11 files changed, 405 insertions(+), 17 deletions(-) create mode 100644 src/main/java/org/folio/circulationbff/service/InventoryService.java create mode 100644 src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index dd59db4..2deab9c 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -150,7 +150,11 @@ "circulation-bff.loans.check-in-by-barcode.execute" ], "modulePermissions": [ - "circulation.check-in-by-barcode.post" + "circulation.check-in-by-barcode.post", + "user-tenants.collection.get", + "search.instances.collection.get", + "inventory-storage.items.item.get", + "inventory-storage.service-points.item.get" ] } ] diff --git a/src/main/java/org/folio/circulationbff/service/InventoryService.java b/src/main/java/org/folio/circulationbff/service/InventoryService.java new file mode 100644 index 0000000..213c67d --- /dev/null +++ b/src/main/java/org/folio/circulationbff/service/InventoryService.java @@ -0,0 +1,9 @@ +package org.folio.circulationbff.service; + +import org.folio.circulationbff.domain.dto.Item; +import org.folio.circulationbff.domain.dto.ServicePoint; + +public interface InventoryService { + Item fetchItem(String id); + ServicePoint fetchServicePoint(String id); +} diff --git a/src/main/java/org/folio/circulationbff/service/SearchService.java b/src/main/java/org/folio/circulationbff/service/SearchService.java index 9604149..a398f85 100644 --- a/src/main/java/org/folio/circulationbff/service/SearchService.java +++ b/src/main/java/org/folio/circulationbff/service/SearchService.java @@ -3,7 +3,9 @@ import java.util.Collection; import org.folio.circulationbff.domain.dto.BffSearchInstance; +import org.folio.circulationbff.domain.dto.SearchInstance; public interface SearchService { + SearchInstance findInstanceByItemId(String itemId); Collection findInstances(String query); } diff --git a/src/main/java/org/folio/circulationbff/service/UserTenantsService.java b/src/main/java/org/folio/circulationbff/service/UserTenantsService.java index 13c70a9..2629fcb 100644 --- a/src/main/java/org/folio/circulationbff/service/UserTenantsService.java +++ b/src/main/java/org/folio/circulationbff/service/UserTenantsService.java @@ -1,6 +1,7 @@ package org.folio.circulationbff.service; public interface UserTenantsService { + String getCurrentTenant(); String getCentralTenant(); boolean isCentralTenant(); boolean isCentralTenant(String tenantId); diff --git a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java index d2a8254..1975204 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java @@ -1,13 +1,19 @@ package org.folio.circulationbff.service.impl; +import lombok.RequiredArgsConstructor; +import lombok.extern.log4j.Log4j2; import org.folio.circulationbff.client.feign.CheckInClient; import org.folio.circulationbff.domain.dto.CheckInRequest; import org.folio.circulationbff.domain.dto.CheckInResponse; +import org.folio.circulationbff.domain.dto.SearchItem; import org.folio.circulationbff.service.CheckInService; +import org.folio.circulationbff.service.InventoryService; +import org.folio.circulationbff.service.SearchService; +import org.folio.circulationbff.service.UserTenantsService; +import org.folio.spring.service.SystemUserScopedExecutionService; import org.springframework.stereotype.Service; -import lombok.RequiredArgsConstructor; -import lombok.extern.log4j.Log4j2; +import java.util.Objects; @Service @RequiredArgsConstructor @@ -15,12 +21,56 @@ public class CheckInServiceImpl implements CheckInService { private final CheckInClient checkInClient; + private final SearchService searchService; + private final UserTenantsService userTenantsService; + private final InventoryService inventoryService; + private final SystemUserScopedExecutionService executionService; @Override public CheckInResponse checkIn(CheckInRequest request) { log.info("checkIn: checking in item with barcode {} on service point {}", - request::getItemBarcode, request::getServicePointId); - return checkInClient.checkIn(request); + request.getItemBarcode(), request.getServicePointId()); + var response = checkInClient.checkIn(request); + var item = response.getItem(); + var servicePointName = getEffectiveLocationServicePoint(item.getId()); + if (servicePointName != null) { + var slipContextItem = response.getStaffSlipContext().getItem(); + slipContextItem.setEffectiveLocationPrimaryServicePointName(servicePointName); + slipContextItem.toServicePoint(servicePointName); + } + return response; + } + + private String getEffectiveLocationServicePoint(String itemId) { + log.info("getEffectiveLocationServicePoint: itemId {}", itemId); + var instance = searchService.findInstanceByItemId(itemId); + if (instance == null) { + log.info("getEffectiveLocationServicePoint: instance not found"); + return null; + } + var itemTenantId = instance.getItems() + .stream() + .filter(item -> item.getId().equals(itemId)) + .findFirst() + .map(SearchItem::getTenantId) + .orElse(null); + if (Objects.equals(itemTenantId, userTenantsService.getCurrentTenant())) { + log.info("getEffectiveLocationServicePoint: same tenant case {}", itemTenantId); + var item = inventoryService.fetchItem(itemId); + var servicePoint = inventoryService.fetchServicePoint(item.getEffectiveLocationId()); + return servicePoint.getName(); + } else { + log.info("getEffectiveLocationServicePoint: cross tenant case {}", itemTenantId); + var item = executionService.executeSystemUserScoped( + itemTenantId, + () -> inventoryService.fetchItem(itemId) + ); + var servicePoint = executionService.executeSystemUserScoped( + itemTenantId, + () -> inventoryService.fetchServicePoint(item.getEffectiveLocationId()) + ); + return servicePoint.getName(); + } } } diff --git a/src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java new file mode 100644 index 0000000..a5779a1 --- /dev/null +++ b/src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java @@ -0,0 +1,30 @@ +package org.folio.circulationbff.service.impl; + +import lombok.RequiredArgsConstructor; +import lombok.extern.log4j.Log4j2; +import org.folio.circulationbff.client.feign.ItemStorageClient; +import org.folio.circulationbff.client.feign.ServicePointClient; +import org.folio.circulationbff.domain.dto.Item; +import org.folio.circulationbff.domain.dto.ServicePoint; +import org.folio.circulationbff.service.InventoryService; +import org.springframework.stereotype.Service; + +@Service +@RequiredArgsConstructor +@Log4j2 +public class InventoryServiceImpl implements InventoryService { + private final ItemStorageClient itemClient; + private final ServicePointClient servicePointClient; + + @Override + public Item fetchItem(String id) { + log.info("fetchItem:: fetching item {}", id); + return itemClient.findItem(id); + } + + @Override + public ServicePoint fetchServicePoint(String id) { + log.info("fetchServicePoint:: fetching service point {}", id); + return servicePointClient.findServicePoint(id); + } +} diff --git a/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java index 98d6586..41e5f8c 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java @@ -14,6 +14,7 @@ import java.util.function.Function; import java.util.stream.Collectors; +import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.folio.circulationbff.client.feign.HoldingsStorageClient; import org.folio.circulationbff.client.feign.InstanceStorageClient; @@ -71,6 +72,17 @@ public class SearchServiceImpl implements SearchService { private final BulkFetchingService fetchingService; private final SearchInstanceMapper searchInstanceMapper; + @Override + public SearchInstance findInstanceByItemId(String itemId) { + log.info("findInstanceByItemId:: itemId {}", itemId); + String query = String.format("items.id==%s&expandAll=true", itemId); + SearchInstances searchResult = searchClient.findInstances(query, true); + if (CollectionUtils.isEmpty(searchResult.getInstances())) { + return null; + } + return searchResult.getInstances().get(0); + } + @Override public Collection findInstances(String query) { log.info("findInstances:: searching instances by query: {}", query); diff --git a/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java index d3aea4a..f817705 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java @@ -18,6 +18,18 @@ public class UserTenantsServiceImpl implements UserTenantsService { private final UserTenantsClient userTenantsClient; + @Override + public String getCurrentTenant() { + UserTenant firstUserTenant = getFirstUserTenant(); + if (firstUserTenant == null) { + log.info("getCurrentTenant:: failed to fetch user tenants"); + return null; + } + String currentTenantId = firstUserTenant.getTenantId(); + log.info("getCurrentTenant:: currentTenantId={}", currentTenantId); + return currentTenantId; + } + @Override public String getCentralTenant() { UserTenant firstUserTenant = getFirstUserTenant(); diff --git a/src/main/resources/swagger.api/schemas/response/check-in-response.yaml b/src/main/resources/swagger.api/schemas/response/check-in-response.yaml index cce42be..b11de04 100644 --- a/src/main/resources/swagger.api/schemas/response/check-in-response.yaml +++ b/src/main/resources/swagger.api/schemas/response/check-in-response.yaml @@ -1,8 +1,134 @@ type: object description: Check-in response properties: - toServicePoint: - description: Name of the service point the item should be routed to - type: string + item: + description: Item data + type: object + properties: + id: + description: Item ID + type: string + instanceId: + description: Related Instance Id + type: string + holdingsRecordId: + description: Related holding record id + type: string + title: + description: Related title + type: string + barcode: + description: Barcode of the item + type: string + enumeration: + description: Enumeration of the item + type: string + volume: + description: Volume of the item + type: string + chronology: + description: Chronology of the item + type: string + displaySummary: + description: Display summary of the item + type: string + copyNumber: + description: Copy number of the item + type: string + callNumber: + description: Call number of the item + type: string + additionalProperties: true + staffSlipContext: + description: Staff slips data + type: object + properties: + item: + description: Staff slips item data + type: object + properties: + title: + description: Title of the instance record + type: string + primaryContributor: + description: Primary contributor name from the instance record + type: string + allContributors: + description: List of contributor names from the instance record concatenated with semicolon + type: string + barcode: + description: Barcode of the item + type: string + status: + description: Status of the item + type: string + enumeration: + description: Enumeration of the item + type: string + volume: + description: Volume of the item + type: string + chronology: + description: Chronology of the item + type: string + yearCaption: + description: Year caption of the item + type: string + materialType: + description: Material type of the item + type: string + loanType: + description: Loan type of the item + type: string + copy: + description: Copy number of the item + type: string + numberOfPieces: + description: Number of item pieces + type: string + displaySummary: + description: Display summary of the item + type: string + descriptionOfPieces: + description: Description of item pieces + type: string + effectiveLocationSpecific: + description: Name of the effective location + type: string + effectiveLocationLibrary: + description: Library name of the effective location + type: string + effectiveLocationCampus: + description: Campus name of the effective location + type: string + effectiveLocationInstitution: + description: Institution name of the effective location + type: string + effectiveLocationDiscoveryDisplayName: + description: Discovery display name of the effective location + type: string + effectiveLocationPrimaryServicePointName: + description: Primary service point name of the effective location + type: string + callNumber: + description: Call number of the item + type: string + callNumberPrefix: + description: Prefix of the item's call number + type: string + callNumberSuffix: + description: Suffix of the item's call number + type: string + lastCheckedInDateTime: + description: Last checked in date of the item + type: string + format: date-time + toServicePoint: + description: Destination service point of the item + type: string + fromServicePoint: + description: Last checked in service point of the item + type: string + additionalProperties: true -additionalProperties: true \ No newline at end of file +additionalProperties: true diff --git a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java index 35141a8..43fad67 100644 --- a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java +++ b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java @@ -6,13 +6,22 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; import static java.util.UUID.randomUUID; import static org.apache.http.HttpStatus.SC_OK; +import static org.hamcrest.Matchers.equalTo; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.util.Date; +import java.util.List; import org.folio.circulationbff.domain.dto.CheckInRequest; +import org.folio.circulationbff.domain.dto.Item; +import org.folio.circulationbff.domain.dto.SearchInstance; +import org.folio.circulationbff.domain.dto.SearchInstances; +import org.folio.circulationbff.domain.dto.SearchItem; +import org.folio.circulationbff.domain.dto.UserTenant; +import org.folio.circulationbff.domain.dto.UserTenantCollection; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -30,25 +39,141 @@ class CheckInApiTest extends BaseIT { @Test @SneakyThrows void checkInSuccess() { - CheckInRequest request = new CheckInRequest() + // given + var request = new CheckInRequest() .itemBarcode("test_barcode") .checkInDate(new Date()) .servicePointId(randomUUID()); - String mockResponse = """ + givenCirculationCheckinSucceed(request); + + var checkinItem = new Item() + .id("itemId") + .copyNumber("copyNumber") + .effectiveLocationId( "effectiveLocationId"); + + givenSearchInstanceReturnsItem(TENANT_ID_CONSORTIUM, checkinItem); + + givenCurrentTenantIsConsortium(); + + wireMockServer.stubFor(WireMock.get(urlMatching("/item-storage/items/itemId")) + .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_CONSORTIUM)) + .willReturn(jsonResponse(checkinItem, SC_OK))); + + var servicePointResponse = """ { - "randomProperty": "randomValue", - "toServicePoint": "Test service point" + "name": "updated service point", + "holdShelfClosedLibraryDateManagement": "Keep_the_current_due_date" } """; + wireMockServer.stubFor(WireMock.get(urlMatching("/service-points/effectiveLocationId")) + .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_CONSORTIUM)) + .willReturn(jsonResponse(servicePointResponse, SC_OK))); - wireMockServer.stubFor(WireMock.post(urlMatching(CIRCULATION_CHECK_IN_URL)) - .withRequestBody(equalToJson(asJsonString(request))) - .willReturn(jsonResponse(mockResponse, SC_OK))); + // when-then + var updatedServicePoint = "updated service point"; + checkIn(request) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.staffSlipContext.item.toServicePoint", equalTo(updatedServicePoint))) + .andExpect(jsonPath("$.staffSlipContext.item.effectiveLocationPrimaryServicePointName", equalTo(updatedServicePoint))); + } + + @Test + @SneakyThrows + void checkInSuccessCrossTenant() { + // given + var request = new CheckInRequest() + .itemBarcode("test_barcode") + .checkInDate(new Date()) + .servicePointId(randomUUID()); + + givenCirculationCheckinSucceed(request); + + var checkinItem = new Item() + .id("itemId") + .copyNumber("copyNumber") + .effectiveLocationId( "effectiveLocationId"); + + givenSearchInstanceReturnsItem(TENANT_ID_COLLEGE, checkinItem); + + givenCurrentTenantIsConsortium(); + + wireMockServer.stubFor(WireMock.get(urlMatching("/item-storage/items/itemId")) + .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_COLLEGE)) + .willReturn(jsonResponse(checkinItem, SC_OK))); + + var servicePointResponse = """ + { + "name": "updated service point", + "holdShelfClosedLibraryDateManagement": "Keep_the_current_due_date" + } + """; + wireMockServer.stubFor(WireMock.get(urlMatching("/service-points/effectiveLocationId")) + .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_COLLEGE)) + .willReturn(jsonResponse(servicePointResponse, SC_OK))); + // when-then + var updatedServicePoint = "updated service point"; checkIn(request) .andExpect(status().isOk()) - .andExpect(content().json(mockResponse)); + .andExpect(jsonPath("$.staffSlipContext.item.toServicePoint", equalTo(updatedServicePoint))) + .andExpect(jsonPath("$.staffSlipContext.item.effectiveLocationPrimaryServicePointName", equalTo(updatedServicePoint))); + } + + @Test + @SneakyThrows + void checkInSuccessWhenInstanceNotFound() { + // given + var request = new CheckInRequest() + .itemBarcode("test_barcode") + .checkInDate(new Date()) + .servicePointId(randomUUID()); + + givenCirculationCheckinSucceed(request); + + var searchResponse = new SearchInstances().instances(List.of()); + wireMockServer.stubFor(WireMock.get(urlMatching("/search/instances.*")) + .willReturn(jsonResponse(searchResponse, SC_OK))); + + // when-then + checkIn(request) + .andExpect(status().isOk()); + } + + private void givenCirculationCheckinSucceed(CheckInRequest request) { + var checkinResponse = """ + { + "item": {"id": "itemId"}, + "staffSlipContext": { + "item": { + "toServicePoint": "random service point", + "effectiveLocationPrimaryServicePointName": "random service point" + } + } + } + """; + wireMockServer.stubFor(WireMock.post(urlMatching(CIRCULATION_CHECK_IN_URL)) + .withRequestBody(equalToJson(asJsonString(request))) + .willReturn(jsonResponse(checkinResponse, SC_OK))); + } + + private void givenSearchInstanceReturnsItem(String tenantId, Item item) { + var searchItem = new SearchItem() + .id(item.getId()) + .tenantId(tenantId); + var searchInstance = new SearchInstance() + .tenantId(tenantId) + .items(List.of(searchItem)); + var searchResponse = new SearchInstances().instances(List.of(searchInstance)); + wireMockServer.stubFor(WireMock.get(urlMatching("/search/instances.*")) + .willReturn(jsonResponse(searchResponse, SC_OK))); + } + + private void givenCurrentTenantIsConsortium() { + var tenantsResponse = new UserTenantCollection() + .userTenants(List.of(new UserTenant().tenantId(TENANT_ID_CONSORTIUM))); + wireMockServer.stubFor(WireMock.get(urlMatching("/user-tenants.*")) + .willReturn(jsonResponse(tenantsResponse, SC_OK))); } @ParameterizedTest diff --git a/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java b/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java index aa05bf1..b9d149c 100644 --- a/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java +++ b/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java @@ -6,6 +6,7 @@ import static org.hamcrest.Matchers.emptyIterable; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @@ -43,6 +44,22 @@ class SearchServiceTest { @InjectMocks private SearchServiceImpl searchService; + @Test + void searchInstanceByItemId() { + String itemId = UUID.randomUUID().toString(); + SearchInstance instance = new SearchInstance(); + + SearchInstances mockSearchResponse = new SearchInstances() + .instances(List.of(instance)) + .totalRecords(1); + String query = "items.id==" + itemId + "&expandAll=true"; + when(searchClient.findInstances(query, true)) + .thenReturn(mockSearchResponse); + + SearchInstance response = searchService.findInstanceByItemId(itemId); + assertEquals(response, instance); + } + @Test void searchFindsNoInstances() { String instanceId = UUID.randomUUID().toString(); From ae4a7873862249dd6e1bce78f6552ddb11354e0d Mon Sep 17 00:00:00 2001 From: Irakli Merabishvili Date: Fri, 24 Jan 2025 17:11:03 +0400 Subject: [PATCH 2/7] MCBFF-38: fix search query --- .../folio/circulationbff/service/impl/SearchServiceImpl.java | 2 +- .../org/folio/circulationbff/service/SearchServiceTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java index 41e5f8c..d3872ec 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/SearchServiceImpl.java @@ -75,7 +75,7 @@ public class SearchServiceImpl implements SearchService { @Override public SearchInstance findInstanceByItemId(String itemId) { log.info("findInstanceByItemId:: itemId {}", itemId); - String query = String.format("items.id==%s&expandAll=true", itemId); + String query = "items.id==" + itemId; SearchInstances searchResult = searchClient.findInstances(query, true); if (CollectionUtils.isEmpty(searchResult.getInstances())) { return null; diff --git a/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java b/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java index b9d149c..2d3f615 100644 --- a/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java +++ b/src/test/java/org/folio/circulationbff/service/SearchServiceTest.java @@ -52,7 +52,7 @@ void searchInstanceByItemId() { SearchInstances mockSearchResponse = new SearchInstances() .instances(List.of(instance)) .totalRecords(1); - String query = "items.id==" + itemId + "&expandAll=true"; + String query = "items.id==" + itemId; when(searchClient.findInstances(query, true)) .thenReturn(mockSearchResponse); From c70dcf43630054a530fc454e10260bf05a3be12d Mon Sep 17 00:00:00 2001 From: Irakli Merabishvili Date: Fri, 24 Jan 2025 18:46:34 +0400 Subject: [PATCH 3/7] MCBFF-38: removed empty lines, added permissions, fixed duplicate logging --- descriptors/ModuleDescriptor-template.json | 4 +- .../service/impl/CheckInServiceImpl.java | 2 +- .../service/impl/UserTenantsServiceImpl.java | 37 +++++-------------- .../circulationbff/api/CheckInApiTest.java | 20 ++-------- 4 files changed, 18 insertions(+), 45 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 2deab9c..1870a86 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -154,7 +154,9 @@ "user-tenants.collection.get", "search.instances.collection.get", "inventory-storage.items.item.get", - "inventory-storage.service-points.item.get" + "inventory-storage.items.collection.get", + "inventory-storage.service-points.item.get", + "inventory-storage.service-points.collection.get" ] } ] diff --git a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java index 1975204..43dac9f 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java @@ -45,7 +45,7 @@ private String getEffectiveLocationServicePoint(String itemId) { log.info("getEffectiveLocationServicePoint: itemId {}", itemId); var instance = searchService.findInstanceByItemId(itemId); if (instance == null) { - log.info("getEffectiveLocationServicePoint: instance not found"); + log.warn("getEffectiveLocationServicePoint: instance not found"); return null; } var itemTenantId = instance.getItems() diff --git a/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java index f817705..64472c7 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/UserTenantsServiceImpl.java @@ -1,7 +1,6 @@ package org.folio.circulationbff.service.impl; -import java.util.List; - +import org.apache.commons.collections4.CollectionUtils; import org.folio.circulationbff.client.feign.UserTenantsClient; import org.folio.circulationbff.domain.dto.UserTenant; import org.folio.circulationbff.domain.dto.UserTenantCollection; @@ -22,7 +21,6 @@ public class UserTenantsServiceImpl implements UserTenantsService { public String getCurrentTenant() { UserTenant firstUserTenant = getFirstUserTenant(); if (firstUserTenant == null) { - log.info("getCurrentTenant:: failed to fetch user tenants"); return null; } String currentTenantId = firstUserTenant.getTenantId(); @@ -34,7 +32,6 @@ public String getCurrentTenant() { public String getCentralTenant() { UserTenant firstUserTenant = getFirstUserTenant(); if (firstUserTenant == null) { - log.info("getCentralTenant:: failed to fetch user tenants"); return null; } String centralTenantId = firstUserTenant.getCentralTenantId(); @@ -46,7 +43,6 @@ public String getCentralTenant() { public boolean isCentralTenant() { UserTenant firstUserTenant = getFirstUserTenant(); if (firstUserTenant == null) { - log.info("isCentralTenant:: failed to fetch user tenants"); return false; } String centralTenantId = firstUserTenant.getCentralTenantId(); @@ -57,14 +53,6 @@ public boolean isCentralTenant() { return centralTenantId.equals(tenantId); } - private UserTenant getFirstUserTenant() { - UserTenant firstUserTenant = findFirstUserTenant(); - if (firstUserTenant == null) { - log.info("processUserGroupEvent: Failed to get user-tenants info"); - } - return firstUserTenant; - } - @Override public boolean isCentralTenant(String tenantId) { UserTenant firstUserTenant = getFirstUserTenant(); @@ -79,21 +67,16 @@ public boolean isCentralTenant(String tenantId) { return false; } - private UserTenant findFirstUserTenant() { - log.info("findFirstUserTenant:: finding first userTenant"); - UserTenant firstUserTenant = null; - UserTenantCollection userTenantCollection = userTenantsClient.getUserTenants(1); - log.info("findFirstUserTenant:: userTenantCollection: {}", () -> userTenantCollection); - if (userTenantCollection != null) { - log.info("findFirstUserTenant:: userTenantCollection: {}", () -> userTenantCollection); - List userTenants = userTenantCollection.getUserTenants(); - if (!userTenants.isEmpty()) { - firstUserTenant = userTenants.get(0); - log.info("findFirstUserTenant:: found userTenant: {}", firstUserTenant); - } + private UserTenant getFirstUserTenant() { + log.info("getFirstUserTenant:: finding first userTenant"); + UserTenantCollection userTenants = userTenantsClient.getUserTenants(1); + log.info("getFirstUserTenant:: userTenants: {}", () -> userTenants); + if (userTenants == null || CollectionUtils.isEmpty(userTenants.getUserTenants())) { + log.warn("getFirstUserTenant: failed to fetch user tenants"); + return null; } - log.info("findFirstUserTenant:: result: {}", firstUserTenant); + var firstUserTenant = userTenants.getUserTenants().get(0); + log.info("getFirstUserTenant:: result: {}", firstUserTenant); return firstUserTenant; } } - diff --git a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java index 43fad67..72c032b 100644 --- a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java +++ b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java @@ -44,18 +44,13 @@ void checkInSuccess() { .itemBarcode("test_barcode") .checkInDate(new Date()) .servicePointId(randomUUID()); - givenCirculationCheckinSucceed(request); - var checkinItem = new Item() .id("itemId") .copyNumber("copyNumber") - .effectiveLocationId( "effectiveLocationId"); - + .effectiveLocationId("effectiveLocationId"); givenSearchInstanceReturnsItem(TENANT_ID_CONSORTIUM, checkinItem); - givenCurrentTenantIsConsortium(); - wireMockServer.stubFor(WireMock.get(urlMatching("/item-storage/items/itemId")) .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_CONSORTIUM)) .willReturn(jsonResponse(checkinItem, SC_OK))); @@ -86,18 +81,13 @@ void checkInSuccessCrossTenant() { .itemBarcode("test_barcode") .checkInDate(new Date()) .servicePointId(randomUUID()); - givenCirculationCheckinSucceed(request); - var checkinItem = new Item() .id("itemId") .copyNumber("copyNumber") - .effectiveLocationId( "effectiveLocationId"); - + .effectiveLocationId("effectiveLocationId"); givenSearchInstanceReturnsItem(TENANT_ID_COLLEGE, checkinItem); - givenCurrentTenantIsConsortium(); - wireMockServer.stubFor(WireMock.get(urlMatching("/item-storage/items/itemId")) .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_COLLEGE)) .willReturn(jsonResponse(checkinItem, SC_OK))); @@ -128,12 +118,10 @@ void checkInSuccessWhenInstanceNotFound() { .itemBarcode("test_barcode") .checkInDate(new Date()) .servicePointId(randomUUID()); - givenCirculationCheckinSucceed(request); - - var searchResponse = new SearchInstances().instances(List.of()); + var searchInstances = new SearchInstances().instances(List.of()); wireMockServer.stubFor(WireMock.get(urlMatching("/search/instances.*")) - .willReturn(jsonResponse(searchResponse, SC_OK))); + .willReturn(jsonResponse(searchInstances, SC_OK))); // when-then checkIn(request) From 0e725cda2a088f54bc42983114340d18c060035b Mon Sep 17 00:00:00 2001 From: Irakli Merabishvili Date: Fri, 24 Jan 2025 19:20:27 +0400 Subject: [PATCH 4/7] MCBFF-38: given-when-then comments removed --- .../java/org/folio/circulationbff/api/CheckInApiTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java index 72c032b..f3af981 100644 --- a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java +++ b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java @@ -39,7 +39,6 @@ class CheckInApiTest extends BaseIT { @Test @SneakyThrows void checkInSuccess() { - // given var request = new CheckInRequest() .itemBarcode("test_barcode") .checkInDate(new Date()) @@ -65,7 +64,6 @@ void checkInSuccess() { .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_CONSORTIUM)) .willReturn(jsonResponse(servicePointResponse, SC_OK))); - // when-then var updatedServicePoint = "updated service point"; checkIn(request) .andExpect(status().isOk()) @@ -76,7 +74,6 @@ void checkInSuccess() { @Test @SneakyThrows void checkInSuccessCrossTenant() { - // given var request = new CheckInRequest() .itemBarcode("test_barcode") .checkInDate(new Date()) @@ -102,7 +99,6 @@ void checkInSuccessCrossTenant() { .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_COLLEGE)) .willReturn(jsonResponse(servicePointResponse, SC_OK))); - // when-then var updatedServicePoint = "updated service point"; checkIn(request) .andExpect(status().isOk()) @@ -113,7 +109,6 @@ void checkInSuccessCrossTenant() { @Test @SneakyThrows void checkInSuccessWhenInstanceNotFound() { - // given var request = new CheckInRequest() .itemBarcode("test_barcode") .checkInDate(new Date()) @@ -123,7 +118,6 @@ void checkInSuccessWhenInstanceNotFound() { wireMockServer.stubFor(WireMock.get(urlMatching("/search/instances.*")) .willReturn(jsonResponse(searchInstances, SC_OK))); - // when-then checkIn(request) .andExpect(status().isOk()); } From a967e5e9f307b5b41300fabc6a83681c0f6bb50d Mon Sep 17 00:00:00 2001 From: Irakli Merabishvili Date: Mon, 27 Jan 2025 18:37:14 +0400 Subject: [PATCH 5/7] MCBFF-38: fix fetching service point by location id --- descriptors/ModuleDescriptor-template.json | 2 ++ .../circulationbff/service/InventoryService.java | 2 ++ .../service/impl/CheckInServiceImpl.java | 14 ++++++++------ .../service/impl/InventoryServiceImpl.java | 9 +++++++++ .../folio/circulationbff/api/CheckInApiTest.java | 15 +++++++++++++-- 5 files changed, 34 insertions(+), 8 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 1870a86..a2d403c 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -155,6 +155,8 @@ "search.instances.collection.get", "inventory-storage.items.item.get", "inventory-storage.items.collection.get", + "inventory-storage.locations.item.get", + "inventory-storage.locations.collection.get", "inventory-storage.service-points.item.get", "inventory-storage.service-points.collection.get" ] diff --git a/src/main/java/org/folio/circulationbff/service/InventoryService.java b/src/main/java/org/folio/circulationbff/service/InventoryService.java index 213c67d..38b32b6 100644 --- a/src/main/java/org/folio/circulationbff/service/InventoryService.java +++ b/src/main/java/org/folio/circulationbff/service/InventoryService.java @@ -1,9 +1,11 @@ package org.folio.circulationbff.service; import org.folio.circulationbff.domain.dto.Item; +import org.folio.circulationbff.domain.dto.Location; import org.folio.circulationbff.domain.dto.ServicePoint; public interface InventoryService { Item fetchItem(String id); + Location fetchLocation(String id); ServicePoint fetchServicePoint(String id); } diff --git a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java index 43dac9f..16a1785 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java @@ -57,17 +57,19 @@ private String getEffectiveLocationServicePoint(String itemId) { if (Objects.equals(itemTenantId, userTenantsService.getCurrentTenant())) { log.info("getEffectiveLocationServicePoint: same tenant case {}", itemTenantId); var item = inventoryService.fetchItem(itemId); - var servicePoint = inventoryService.fetchServicePoint(item.getEffectiveLocationId()); + var location = inventoryService.fetchLocation(item.getEffectiveLocationId()); + var servicePoint = inventoryService.fetchServicePoint(location.getPrimaryServicePoint().toString()); return servicePoint.getName(); } else { log.info("getEffectiveLocationServicePoint: cross tenant case {}", itemTenantId); - var item = executionService.executeSystemUserScoped( - itemTenantId, + var item = executionService.executeSystemUserScoped(itemTenantId, () -> inventoryService.fetchItem(itemId) ); - var servicePoint = executionService.executeSystemUserScoped( - itemTenantId, - () -> inventoryService.fetchServicePoint(item.getEffectiveLocationId()) + var location = executionService.executeSystemUserScoped(itemTenantId, + () -> inventoryService.fetchLocation(item.getEffectiveLocationId()) + ); + var servicePoint = executionService.executeSystemUserScoped(itemTenantId, + () -> inventoryService.fetchServicePoint(location.getPrimaryServicePoint().toString()) ); return servicePoint.getName(); } diff --git a/src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java index a5779a1..3b812c2 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/InventoryServiceImpl.java @@ -3,8 +3,10 @@ import lombok.RequiredArgsConstructor; import lombok.extern.log4j.Log4j2; import org.folio.circulationbff.client.feign.ItemStorageClient; +import org.folio.circulationbff.client.feign.LocationClient; import org.folio.circulationbff.client.feign.ServicePointClient; import org.folio.circulationbff.domain.dto.Item; +import org.folio.circulationbff.domain.dto.Location; import org.folio.circulationbff.domain.dto.ServicePoint; import org.folio.circulationbff.service.InventoryService; import org.springframework.stereotype.Service; @@ -14,6 +16,7 @@ @Log4j2 public class InventoryServiceImpl implements InventoryService { private final ItemStorageClient itemClient; + private final LocationClient locationClient; private final ServicePointClient servicePointClient; @Override @@ -22,6 +25,12 @@ public Item fetchItem(String id) { return itemClient.findItem(id); } + @Override + public Location fetchLocation(String id) { + log.info("fetchLocation:: fetching location {}", id); + return locationClient.findLocation(id); + } + @Override public ServicePoint fetchServicePoint(String id) { log.info("fetchServicePoint:: fetching service point {}", id); diff --git a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java index f3af981..c81bb21 100644 --- a/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java +++ b/src/test/java/org/folio/circulationbff/api/CheckInApiTest.java @@ -17,6 +17,7 @@ import org.folio.circulationbff.domain.dto.CheckInRequest; import org.folio.circulationbff.domain.dto.Item; +import org.folio.circulationbff.domain.dto.Location; import org.folio.circulationbff.domain.dto.SearchInstance; import org.folio.circulationbff.domain.dto.SearchInstances; import org.folio.circulationbff.domain.dto.SearchItem; @@ -54,13 +55,18 @@ void checkInSuccess() { .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_CONSORTIUM)) .willReturn(jsonResponse(checkinItem, SC_OK))); + var primaryServicePoint = randomUUID(); + var location = new Location().primaryServicePoint(primaryServicePoint); + wireMockServer.stubFor(WireMock.get(urlMatching("/locations/effectiveLocationId")) + .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_CONSORTIUM)) + .willReturn(jsonResponse(location, SC_OK))); var servicePointResponse = """ { "name": "updated service point", "holdShelfClosedLibraryDateManagement": "Keep_the_current_due_date" } """; - wireMockServer.stubFor(WireMock.get(urlMatching("/service-points/effectiveLocationId")) + wireMockServer.stubFor(WireMock.get(urlMatching("/service-points/" + primaryServicePoint)) .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_CONSORTIUM)) .willReturn(jsonResponse(servicePointResponse, SC_OK))); @@ -89,13 +95,18 @@ void checkInSuccessCrossTenant() { .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_COLLEGE)) .willReturn(jsonResponse(checkinItem, SC_OK))); + var primaryServicePoint = randomUUID(); + var location = new Location().primaryServicePoint(primaryServicePoint); + wireMockServer.stubFor(WireMock.get(urlMatching("/locations/effectiveLocationId")) + .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_COLLEGE)) + .willReturn(jsonResponse(location, SC_OK))); var servicePointResponse = """ { "name": "updated service point", "holdShelfClosedLibraryDateManagement": "Keep_the_current_due_date" } """; - wireMockServer.stubFor(WireMock.get(urlMatching("/service-points/effectiveLocationId")) + wireMockServer.stubFor(WireMock.get(urlMatching("/service-points/" + primaryServicePoint)) .withHeader(HEADER_TENANT, WireMock.equalTo(TENANT_ID_COLLEGE)) .willReturn(jsonResponse(servicePointResponse, SC_OK))); From 574aeb3b7cd490f4460a1df71960a93e1930b749 Mon Sep 17 00:00:00 2001 From: imerabishvili <144257054+imerabishvili@users.noreply.github.com> Date: Mon, 27 Jan 2025 20:56:24 +0400 Subject: [PATCH 6/7] Update src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java Co-authored-by: Roman Barannyk <53909129+roman-barannyk@users.noreply.github.com> --- .../service/impl/CheckInServiceImpl.java | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java index 16a1785..51febb2 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java @@ -54,7 +54,45 @@ private String getEffectiveLocationServicePoint(String itemId) { .findFirst() .map(SearchItem::getTenantId) .orElse(null); - if (Objects.equals(itemTenantId, userTenantsService.getCurrentTenant())) { + var tenantId = instance.getItems() + .stream() + .filter(item -> item.getId().equals(itemId)) + .findFirst() + .map(SearchItem::getTenantId) + .orElse(null); + + if (Objects.equals(tenantId, userTenantsService.getCurrentTenant())) { + log.info("getEffectiveLocationServicePoint: same tenant case {}", tenantId); + return fetchServicePointName(itemId); + } else { + log.info("getEffectiveLocationServicePoint: cross tenant case {}", tenantId); + return executionService.executeSystemUserScoped(tenantId, () -> fetchServicePointName(itemId)); + } + } + + private String fetchServicePointName(String itemId) { + var item = inventoryService.fetchItem(itemId); + if (item == null) { + log.warn("fetchServicePointName:: item not found, itemId: {}", itemId); + return null; + } + var location = inventoryService.fetchLocation(item.getEffectiveLocationId()); + if (location == null) { + log.warn("fetchServicePointName:: location not found, locationId: {}", + item.getEffectiveLocationId()); + return null; + } + var servicePoint = inventoryService.fetchServicePoint(location.getPrimaryServicePoint().toString()); + if (servicePoint == null) { + log.warn("fetchServicePointName:: servicePoint not found, servicePointId: {}", + location.getPrimaryServicePoint()); + return null; + } + String servicePointName = servicePoint.getName(); + log.info("fetchServicePointName:: result: {}", servicePointName); + + return servicePointName; + } log.info("getEffectiveLocationServicePoint: same tenant case {}", itemTenantId); var item = inventoryService.fetchItem(itemId); var location = inventoryService.fetchLocation(item.getEffectiveLocationId()); From b6df73236a282ff2fb5df5fcd0a3465c537903f6 Mon Sep 17 00:00:00 2001 From: Irakli Merabishvili Date: Mon, 27 Jan 2025 21:03:21 +0400 Subject: [PATCH 7/7] MCBFF-38: fix build --- .../service/impl/CheckInServiceImpl.java | 27 +------------------ 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java index 51febb2..666ec9c 100644 --- a/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java +++ b/src/main/java/org/folio/circulationbff/service/impl/CheckInServiceImpl.java @@ -48,12 +48,6 @@ private String getEffectiveLocationServicePoint(String itemId) { log.warn("getEffectiveLocationServicePoint: instance not found"); return null; } - var itemTenantId = instance.getItems() - .stream() - .filter(item -> item.getId().equals(itemId)) - .findFirst() - .map(SearchItem::getTenantId) - .orElse(null); var tenantId = instance.getItems() .stream() .filter(item -> item.getId().equals(itemId)) @@ -69,7 +63,7 @@ private String getEffectiveLocationServicePoint(String itemId) { return executionService.executeSystemUserScoped(tenantId, () -> fetchServicePointName(itemId)); } } - + private String fetchServicePointName(String itemId) { var item = inventoryService.fetchItem(itemId); if (item == null) { @@ -92,25 +86,6 @@ private String fetchServicePointName(String itemId) { log.info("fetchServicePointName:: result: {}", servicePointName); return servicePointName; - } - log.info("getEffectiveLocationServicePoint: same tenant case {}", itemTenantId); - var item = inventoryService.fetchItem(itemId); - var location = inventoryService.fetchLocation(item.getEffectiveLocationId()); - var servicePoint = inventoryService.fetchServicePoint(location.getPrimaryServicePoint().toString()); - return servicePoint.getName(); - } else { - log.info("getEffectiveLocationServicePoint: cross tenant case {}", itemTenantId); - var item = executionService.executeSystemUserScoped(itemTenantId, - () -> inventoryService.fetchItem(itemId) - ); - var location = executionService.executeSystemUserScoped(itemTenantId, - () -> inventoryService.fetchLocation(item.getEffectiveLocationId()) - ); - var servicePoint = executionService.executeSystemUserScoped(itemTenantId, - () -> inventoryService.fetchServicePoint(location.getPrimaryServicePoint().toString()) - ); - return servicePoint.getName(); - } } }