From 82758f6248994efaf4b79f3e386c1abe5bd9be11 Mon Sep 17 00:00:00 2001 From: Maksat-Galymzhan Date: Wed, 10 Jul 2024 16:10:45 +0500 Subject: [PATCH 1/7] CIRC-2117: return empty if no instance found --- .../circulation/infrastructure/storage/SearchRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index a999c2158d..04bc2aa577 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -2,6 +2,7 @@ import static org.folio.circulation.support.StringUtil.urlEncode; import static org.folio.circulation.support.results.Result.failed; +import static org.folio.circulation.support.results.Result.succeeded; import static org.folio.circulation.support.results.ResultBinding.flatMapResult; import static org.folio.circulation.support.results.ResultBinding.mapResult; @@ -63,8 +64,7 @@ private Result> mapResponseToInstances(Response private CompletableFuture> updateItemDetails(SearchInstance searchInstance) { log.debug("updateItemDetails:: searchInstance {}", () -> searchInstance); if (searchInstance == null) { - return CompletableFuture.completedFuture(failed(new BadRequestFailure( - "Search result is empty"))); + return CompletableFuture.completedFuture(succeeded(null)); } Map> itemsByTenant = searchInstance.getItems() From 85417bbe732215131a86e46a5fd41ef543aeefda Mon Sep 17 00:00:00 2001 From: Maksat-Galymzhan Date: Wed, 10 Jul 2024 16:36:32 +0500 Subject: [PATCH 2/7] CIRC-2117: loggin --- .../circulation/infrastructure/storage/SearchRepository.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 04bc2aa577..2337fe2e8f 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -64,6 +64,7 @@ private Result> mapResponseToInstances(Response private CompletableFuture> updateItemDetails(SearchInstance searchInstance) { log.debug("updateItemDetails:: searchInstance {}", () -> searchInstance); if (searchInstance == null) { + log.info("updateItemDetails:: searchInstance is empty"); return CompletableFuture.completedFuture(succeeded(null)); } From 99af030ac523ce204f18ed447dbb2e6a04c24714 Mon Sep 17 00:00:00 2001 From: Maksat-Galymzhan Date: Wed, 10 Jul 2024 16:44:36 +0500 Subject: [PATCH 3/7] CIRC-2117: refactoring --- .../circulation/infrastructure/storage/SearchRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 2337fe2e8f..04e576382f 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -1,8 +1,8 @@ package org.folio.circulation.infrastructure.storage; import static org.folio.circulation.support.StringUtil.urlEncode; +import static org.folio.circulation.support.results.Result.emptyAsync; import static org.folio.circulation.support.results.Result.failed; -import static org.folio.circulation.support.results.Result.succeeded; import static org.folio.circulation.support.results.ResultBinding.flatMapResult; import static org.folio.circulation.support.results.ResultBinding.mapResult; @@ -65,7 +65,7 @@ private CompletableFuture> updateItemDetails(SearchInstan log.debug("updateItemDetails:: searchInstance {}", () -> searchInstance); if (searchInstance == null) { log.info("updateItemDetails:: searchInstance is empty"); - return CompletableFuture.completedFuture(succeeded(null)); + return emptyAsync(); } Map> itemsByTenant = searchInstance.getItems() From 944e756595b692bfba6177af6bda4492c57cf591 Mon Sep 17 00:00:00 2001 From: Maksat-Galymzhan Date: Thu, 11 Jul 2024 11:25:08 +0500 Subject: [PATCH 4/7] CIRC-2117: Test --- .../java/api/ItemsByInstanceResourceTest.java | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index 71d3ce18b6..b2c5872f10 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -1,19 +1,32 @@ package api; +import io.vertx.core.http.HttpClient; +import org.apache.http.HttpStatus; +import org.folio.circulation.infrastructure.storage.SearchRepository; +import org.folio.circulation.support.CollectionResourceClient; +import org.folio.circulation.support.http.client.Response; +import org.folio.circulation.support.http.server.WebContext; +import org.junit.jupiter.api.Assertions; + import static api.support.APITestContext.clearTempTenantId; import static api.support.APITestContext.setTempTenantId; import static api.support.http.InterfaceUrls.itemsByInstanceUrl; import static api.support.matchers.JsonObjectMatcher.hasJsonPath; +import static org.folio.circulation.support.StringUtil.urlEncode; +import static org.folio.circulation.support.results.Result.ofAsync; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.core.Is.is; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.util.List; +import java.util.Map; import java.util.UUID; +import java.util.concurrent.ExecutionException; -import org.folio.circulation.support.http.client.Response; import org.junit.jupiter.api.Test; import api.support.APITests; @@ -77,6 +90,26 @@ void canGetInstanceById() { hasJsonPath("tenantId", is(TENANT_ID_UNIVERSITY))))); } + @Test + void shouldPassAndReturnEmptyResultIfThereIsNoResult() throws ExecutionException, InterruptedException { + WebContext webContext = mock(WebContext.class); + HttpClient httpClient = mock(HttpClient.class); + CollectionResourceClient collectionResourceClient = mock(CollectionResourceClient.class); + SearchRepository searchRepository = new SearchRepository(webContext, httpClient, collectionResourceClient); + + var queryParams = List.of(String.format("(id==%s)", UUID.randomUUID())); + + when(collectionResourceClient.getManyWithQueryStringParameters( + Map.of("expandAll", "true", "query", urlEncode(queryParams.get(0))))) + .thenReturn(ofAsync(new Response(HttpStatus.SC_OK, null, "application/json"))); + + var response = searchRepository.getInstanceWithItems(queryParams); + var result = response.get(); + + Assertions.assertTrue(result.succeeded()); + Assertions.assertNull(result.value()); + } + private Response get(String query, int expectedStatusCode) { return restAssuredClient.get(itemsByInstanceUrl(query), expectedStatusCode, "items-by-instance-request"); From e1b076a3673d7f51bbcea4dc44413acb0120ac43 Mon Sep 17 00:00:00 2001 From: Maksat-Galymzhan Date: Mon, 15 Jul 2024 19:23:53 +0500 Subject: [PATCH 5/7] CIRC-2117: Added test --- .../storage/SearchRepository.java | 2 +- .../java/api/ItemsByInstanceResourceTest.java | 54 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java index 04e576382f..ed441ce461 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/SearchRepository.java @@ -63,7 +63,7 @@ private Result> mapResponseToInstances(Response private CompletableFuture> updateItemDetails(SearchInstance searchInstance) { log.debug("updateItemDetails:: searchInstance {}", () -> searchInstance); - if (searchInstance == null) { + if (searchInstance == null || searchInstance.getId() == null) { log.info("updateItemDetails:: searchInstance is empty"); return emptyAsync(); } diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index b2c5872f10..9d304821e0 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -1,33 +1,21 @@ package api; -import io.vertx.core.http.HttpClient; -import org.apache.http.HttpStatus; -import org.folio.circulation.infrastructure.storage.SearchRepository; -import org.folio.circulation.support.CollectionResourceClient; -import org.folio.circulation.support.http.client.Response; -import org.folio.circulation.support.http.server.WebContext; -import org.junit.jupiter.api.Assertions; - import static api.support.APITestContext.clearTempTenantId; import static api.support.APITestContext.setTempTenantId; import static api.support.http.InterfaceUrls.itemsByInstanceUrl; import static api.support.matchers.JsonObjectMatcher.hasJsonPath; -import static org.folio.circulation.support.StringUtil.urlEncode; -import static org.folio.circulation.support.results.Result.ofAsync; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.core.Is.is; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import java.util.List; -import java.util.Map; import java.util.UUID; -import java.util.concurrent.ExecutionException; +import org.folio.circulation.support.http.client.Response; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Assertions; import api.support.APITests; import api.support.builders.SearchInstanceBuilder; @@ -91,23 +79,35 @@ void canGetInstanceById() { } @Test - void shouldPassAndReturnEmptyResultIfThereIsNoResult() throws ExecutionException, InterruptedException { - WebContext webContext = mock(WebContext.class); - HttpClient httpClient = mock(HttpClient.class); - CollectionResourceClient collectionResourceClient = mock(CollectionResourceClient.class); - SearchRepository searchRepository = new SearchRepository(webContext, httpClient, collectionResourceClient); + void canGetEmptyResult() { + IndividualResource instance = instancesFixture.basedUponDunkirk(); + UUID instanceId = instance.getId(); - var queryParams = List.of(String.format("(id==%s)", UUID.randomUUID())); + // create item in tenant "college" + setTempTenantId(TENANT_ID_COLLEGE); + IndividualResource collegeLocation = locationsFixture.mainFloor(); + IndividualResource collegeHoldings = holdingsFixture.defaultWithHoldings(instanceId); + IndividualResource collegeItem = itemsFixture.createItemWithHoldingsAndLocation( + collegeHoldings.getId(), collegeLocation.getId()); + clearTempTenantId(); - when(collectionResourceClient.getManyWithQueryStringParameters( - Map.of("expandAll", "true", "query", urlEncode(queryParams.get(0))))) - .thenReturn(ofAsync(new Response(HttpStatus.SC_OK, null, "application/json"))); + // create item in tenant "university" + setTempTenantId(TENANT_ID_UNIVERSITY); + IndividualResource universityLocation = locationsFixture.thirdFloor(); + IndividualResource universityHoldings = holdingsFixture.defaultWithHoldings(instanceId); + IndividualResource universityItem = itemsFixture.createItemWithHoldingsAndLocation( + universityHoldings.getId(), universityLocation.getId()); + clearTempTenantId(); - var response = searchRepository.getInstanceWithItems(queryParams); - var result = response.get(); + // make sure neither item exists in current tenant + assertThat(itemsFixture.getById(collegeItem.getId()).getResponse().getStatusCode(), is(404)); + assertThat(itemsFixture.getById(universityItem.getId()).getResponse().getStatusCode(), is(404)); + + ResourceClient.forSearchClient().replace(instanceId, new JsonObject()); + Response response = get(String.format("query=(id==%s)", instanceId), 200); + JsonObject responseJson = response.getJson(); - Assertions.assertTrue(result.succeeded()); - Assertions.assertNull(result.value()); + Assertions.assertTrue(responseJson.isEmpty()); } private Response get(String query, int expectedStatusCode) { From 422ba2cfedf2e56f95a9912551ab26d4c692107c Mon Sep 17 00:00:00 2001 From: Maksat-Galymzhan Date: Wed, 17 Jul 2024 20:23:00 +0500 Subject: [PATCH 6/7] CIRC-2117: refactoring --- .../java/api/ItemsByInstanceResourceTest.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index 9d304821e0..f10064b5f1 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -4,6 +4,8 @@ import static api.support.APITestContext.setTempTenantId; import static api.support.http.InterfaceUrls.itemsByInstanceUrl; import static api.support.matchers.JsonObjectMatcher.hasJsonPath; +import static org.folio.HttpStatus.HTTP_NOT_FOUND; +import static org.folio.HttpStatus.HTTP_OK; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.hasItem; @@ -15,7 +17,6 @@ import org.folio.circulation.support.http.client.Response; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.Assertions; import api.support.APITests; import api.support.builders.SearchInstanceBuilder; @@ -80,8 +81,7 @@ void canGetInstanceById() { @Test void canGetEmptyResult() { - IndividualResource instance = instancesFixture.basedUponDunkirk(); - UUID instanceId = instance.getId(); + UUID instanceId = instancesFixture.basedUponDunkirk().getId(); // create item in tenant "college" setTempTenantId(TENANT_ID_COLLEGE); @@ -100,14 +100,16 @@ void canGetEmptyResult() { clearTempTenantId(); // make sure neither item exists in current tenant - assertThat(itemsFixture.getById(collegeItem.getId()).getResponse().getStatusCode(), is(404)); - assertThat(itemsFixture.getById(universityItem.getId()).getResponse().getStatusCode(), is(404)); + assertThat(itemsFixture.getById(collegeItem.getId()).getResponse().getStatusCode(), + is(HTTP_NOT_FOUND.toInt())); + assertThat(itemsFixture.getById(universityItem.getId()).getResponse().getStatusCode(), + is(HTTP_NOT_FOUND.toInt())); ResourceClient.forSearchClient().replace(instanceId, new JsonObject()); - Response response = get(String.format("query=(id==%s)", instanceId), 200); + Response response = get(String.format("query=(id==%s)", instanceId), HTTP_OK.toInt()); JsonObject responseJson = response.getJson(); - Assertions.assertTrue(responseJson.isEmpty()); + assertThat(responseJson.isEmpty(), is(true)); } private Response get(String query, int expectedStatusCode) { From da41339321a0ec840f6398d949e05a9c46dc1bc1 Mon Sep 17 00:00:00 2001 From: Maksat-Galymzhan Date: Fri, 19 Jul 2024 17:34:52 +0500 Subject: [PATCH 7/7] CIRC-2117: refactoring --- .../java/api/ItemsByInstanceResourceTest.java | 25 +------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/test/java/api/ItemsByInstanceResourceTest.java b/src/test/java/api/ItemsByInstanceResourceTest.java index f10064b5f1..08d072d432 100644 --- a/src/test/java/api/ItemsByInstanceResourceTest.java +++ b/src/test/java/api/ItemsByInstanceResourceTest.java @@ -4,7 +4,6 @@ import static api.support.APITestContext.setTempTenantId; import static api.support.http.InterfaceUrls.itemsByInstanceUrl; import static api.support.matchers.JsonObjectMatcher.hasJsonPath; -import static org.folio.HttpStatus.HTTP_NOT_FOUND; import static org.folio.HttpStatus.HTTP_OK; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; @@ -81,29 +80,7 @@ void canGetInstanceById() { @Test void canGetEmptyResult() { - UUID instanceId = instancesFixture.basedUponDunkirk().getId(); - - // create item in tenant "college" - setTempTenantId(TENANT_ID_COLLEGE); - IndividualResource collegeLocation = locationsFixture.mainFloor(); - IndividualResource collegeHoldings = holdingsFixture.defaultWithHoldings(instanceId); - IndividualResource collegeItem = itemsFixture.createItemWithHoldingsAndLocation( - collegeHoldings.getId(), collegeLocation.getId()); - clearTempTenantId(); - - // create item in tenant "university" - setTempTenantId(TENANT_ID_UNIVERSITY); - IndividualResource universityLocation = locationsFixture.thirdFloor(); - IndividualResource universityHoldings = holdingsFixture.defaultWithHoldings(instanceId); - IndividualResource universityItem = itemsFixture.createItemWithHoldingsAndLocation( - universityHoldings.getId(), universityLocation.getId()); - clearTempTenantId(); - - // make sure neither item exists in current tenant - assertThat(itemsFixture.getById(collegeItem.getId()).getResponse().getStatusCode(), - is(HTTP_NOT_FOUND.toInt())); - assertThat(itemsFixture.getById(universityItem.getId()).getResponse().getStatusCode(), - is(HTTP_NOT_FOUND.toInt())); + UUID instanceId = UUID.randomUUID(); ResourceClient.forSearchClient().replace(instanceId, new JsonObject()); Response response = get(String.format("query=(id==%s)", instanceId), HTTP_OK.toInt());