From f84b372a9efd1ec48ac7ea47acd6bb77c5bc6e30 Mon Sep 17 00:00:00 2001 From: Yusuf Murodov Date: Mon, 21 Oct 2024 11:45:42 +0500 Subject: [PATCH] MSEARCH-855: Update consortium location endpoints --- NEWS.md | 2 +- .../SearchConsortiumController.java | 3 +- .../ConsortiumLocationRepository.java | 20 +++++-- .../consortium/ConsortiumLocationService.java | 6 ++- .../search-consortium-locations.yaml | 1 + .../ConsortiumSearchLocationsIT.java | 23 +++++++- .../ConsortiumLocationRepositoryTest.java | 53 ++++++++++++++++--- .../ConsortiumLocationServiceTest.java | 11 ++-- 8 files changed, 97 insertions(+), 22 deletions(-) diff --git a/NEWS.md b/NEWS.md index 06f848dfc..0723c52dc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -48,7 +48,7 @@ * Modify usage of shelving order for call number browse ([MSEARCH-831](https://folio-org.atlassian.net/browse/MSEARCH-831)) * Implement new re-index flow for instance records ([MSEARCH-793](https://folio-org.atlassian.net/issues/MSEARCH-793), [MSEARCH-794](https://folio-org.atlassian.net/issues/MSEARCH-794), [MSEARCH-796](https://folio-org.atlassian.net/issues/MSEARCH-796), [MSEARCH-797](https://folio-org.atlassian.net/issues/MSEARCH-797), [MSEARCH-798](https://folio-org.atlassian.net/issues/MSEARCH-798), [MSEARCH-799](https://folio-org.atlassian.net/issues/MSEARCH-799), [MSEARCH-800](https://folio-org.atlassian.net/issues/MSEARCH-800), [MSEARCH-801](https://folio-org.atlassian.net/issues/MSEARCH-801), [MSEARCH-802](https://folio-org.atlassian.net/issues/MSEARCH-802)) * Implement Linked Data HUB index and search API ([MSEARCH-844](https://folio-org.atlassian.net/browse/MSEARCH-844)) -* Extend consortium library, campus, institution API with id param ([MSEARCH-855](https://folio-org.atlassian.net/browse/MSEARCH-855)) +* Extend consortium library, campus, institution, location API with id param ([MSEARCH-855](https://folio-org.atlassian.net/browse/MSEARCH-855)) * Extend instance-records reindex endpoint with index settings ([MSEARCH-853](https://folio-org.atlassian.net/browse/MSEARCH-853)) ### Bug fixes diff --git a/src/main/java/org/folio/search/controller/SearchConsortiumController.java b/src/main/java/org/folio/search/controller/SearchConsortiumController.java index 6c624b0eb..296b3a331 100644 --- a/src/main/java/org/folio/search/controller/SearchConsortiumController.java +++ b/src/main/java/org/folio/search/controller/SearchConsortiumController.java @@ -92,11 +92,12 @@ public ResponseEntity getConsortiumItems(String tenant @Override public ResponseEntity getConsortiumLocations(String tenantHeader, String tenantId, + String id, Integer limit, Integer offset, String sortBy, SortOrder sortOrder) { - var result = locationService.fetchLocations(tenantHeader, tenantId, limit, offset, sortBy, sortOrder); + var result = locationService.fetchLocations(tenantHeader, tenantId, id, limit, offset, sortBy, sortOrder); return ResponseEntity.ok(new ConsortiumLocationCollection() diff --git a/src/main/java/org/folio/search/repository/ConsortiumLocationRepository.java b/src/main/java/org/folio/search/repository/ConsortiumLocationRepository.java index 583215526..98f9bacf1 100644 --- a/src/main/java/org/folio/search/repository/ConsortiumLocationRepository.java +++ b/src/main/java/org/folio/search/repository/ConsortiumLocationRepository.java @@ -1,6 +1,7 @@ package org.folio.search.repository; import static org.folio.search.model.types.ResourceType.LOCATION; +import static org.folio.search.utils.SearchUtils.ID_FIELD; import static org.folio.search.utils.SearchUtils.TENANT_ID_FIELD_NAME; import static org.folio.search.utils.SearchUtils.performExceptionalOperation; import static org.opensearch.search.sort.SortOrder.ASC; @@ -37,27 +38,38 @@ public class ConsortiumLocationRepository { public SearchResult fetchLocations(String tenantHeader, String tenantId, + String id, Integer limit, Integer offset, String sortBy, SortOrder sortOrder) { - var sourceBuilder = getSearchSourceBuilder(tenantId, limit, offset, sortBy, sortOrder); + var sourceBuilder = getSearchSourceBuilder(tenantId, id, limit, offset, sortBy, sortOrder); var response = search(sourceBuilder, tenantHeader); return documentConverter.convertToSearchResult(response, ConsortiumLocation.class); } @NotNull private static SearchSourceBuilder getSearchSourceBuilder(String tenantId, + String locationId, Integer limit, Integer offset, String sortBy, SortOrder sortOrder) { var sourceBuilder = new SearchSourceBuilder(); + var boolQuery = QueryBuilders.boolQuery(); + Optional.ofNullable(tenantId) - .ifPresent(id -> sourceBuilder - .query(QueryBuilders - .termQuery(TENANT_ID_FIELD_NAME, id))); + .ifPresent(id -> boolQuery + .filter(QueryBuilders.termQuery(TENANT_ID_FIELD_NAME, id))); + + Optional.ofNullable(locationId) + .ifPresent(id -> boolQuery + .filter(QueryBuilders.termQuery(ID_FIELD, id))); + + if (boolQuery.hasClauses()) { + sourceBuilder.query(boolQuery); + } return sourceBuilder .from(offset) diff --git a/src/main/java/org/folio/search/service/consortium/ConsortiumLocationService.java b/src/main/java/org/folio/search/service/consortium/ConsortiumLocationService.java index 0bdad9d7d..ed7fab076 100644 --- a/src/main/java/org/folio/search/service/consortium/ConsortiumLocationService.java +++ b/src/main/java/org/folio/search/service/consortium/ConsortiumLocationService.java @@ -24,19 +24,21 @@ public class ConsortiumLocationService { public SearchResult fetchLocations(String tenantHeader, String tenantId, + String id, Integer limit, Integer offset, String sortBy, SortOrder sortOrder) { - log.info("fetching consortium locations for tenant: {}, tenantId: {}, sortBy: {}", + log.info("fetching consortium locations for tenant: {}, tenantId: {}, id: {}, sortBy: {}", tenantHeader, tenantId, + id, sortBy); validatePaginationParameters(limit, offset); validateSortByValue(sortBy); return executor.execute( tenantHeader, - () -> repository.fetchLocations(tenantHeader, tenantId, limit, offset, sortBy, sortOrder)); + () -> repository.fetchLocations(tenantHeader, tenantId, id, limit, offset, sortBy, sortOrder)); } private void validateSortByValue(String sortBy) { diff --git a/src/main/resources/swagger.api/paths/search-consortium/search-consortium-locations.yaml b/src/main/resources/swagger.api/paths/search-consortium/search-consortium-locations.yaml index 5f6aee61a..4151553f4 100644 --- a/src/main/resources/swagger.api/paths/search-consortium/search-consortium-locations.yaml +++ b/src/main/resources/swagger.api/paths/search-consortium/search-consortium-locations.yaml @@ -6,6 +6,7 @@ get: - search-consortium parameters: - $ref: '../../parameters/tenant-id-query-param.yaml' + - $ref: '../../parameters/id-query-param.yaml' - $ref: '../../parameters/consortium-locations-limit-param.yaml' - $ref: '../../parameters/offset-param.yaml' - $ref: '../../parameters/sort-by-location-name-param.yaml' diff --git a/src/test/java/org/folio/search/controller/ConsortiumSearchLocationsIT.java b/src/test/java/org/folio/search/controller/ConsortiumSearchLocationsIT.java index b46d2baed..846ddcd2d 100644 --- a/src/test/java/org/folio/search/controller/ConsortiumSearchLocationsIT.java +++ b/src/test/java/org/folio/search/controller/ConsortiumSearchLocationsIT.java @@ -78,7 +78,7 @@ void doGetConsortiumLocations_returns200AndRecords() { } @Test - void doGetConsortiumLocations_returns200AndRecords_withAllQueryParams() { + void doGetConsortiumLocations_returns200AndRecords_withTenantAndSortQueryParams() { List> queryParams = List.of( pair("tenantId", "consortium"), pair("limit", "5"), @@ -98,6 +98,27 @@ void doGetConsortiumLocations_returns200AndRecords_withAllQueryParams() { assertThat(actual.getLocations().get(1).getName()).isEqualTo("DCB"); } + @Test + void doGetConsortiumLocations_returns200AndRecords_withAllQueryParams() { + List> queryParams = List.of( + pair("tenantId", "consortium"), + pair("id", "53cf956f-c1df-410b-8bea-27f712cca7c0"), + pair("limit", "5"), + pair("offset", "0"), + pair("sortBy", "name"), + pair("sortOrder", "asc") + ); + + var result = doGet(consortiumLocationsSearchPath(queryParams), CENTRAL_TENANT_ID); + var actual = parseResponse(result, ConsortiumLocationCollection.class); + + assertThat(actual.getLocations()).hasSize(1); + assertThat(actual.getTotalRecords()).isEqualTo(1); + assertThat(actual.getLocations().get(0).getTenantId()).isEqualTo(CENTRAL_TENANT_ID); + assertThat(actual.getLocations().get(0).getName()).isEqualTo("Annex"); + assertThat(actual.getLocations().get(0).getCode()).isEqualTo("KU/CC/DI/A"); + } + private static void saveLocationRecords() { getLocationsSampleAsMap().stream() .flatMap(location -> Stream.of( diff --git a/src/test/java/org/folio/search/repository/ConsortiumLocationRepositoryTest.java b/src/test/java/org/folio/search/repository/ConsortiumLocationRepositoryTest.java index 850a86c5c..2a685e248 100644 --- a/src/test/java/org/folio/search/repository/ConsortiumLocationRepositoryTest.java +++ b/src/test/java/org/folio/search/repository/ConsortiumLocationRepositoryTest.java @@ -2,8 +2,11 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.folio.search.model.types.ResourceType.LOCATION; +import static org.folio.search.utils.SearchUtils.ID_FIELD; +import static org.folio.search.utils.SearchUtils.TENANT_ID_FIELD_NAME; import static org.folio.search.utils.TestConstants.INDEX_NAME; import static org.folio.search.utils.TestConstants.MEMBER_TENANT_ID; +import static org.folio.search.utils.TestConstants.RESOURCE_ID; import static org.folio.search.utils.TestConstants.TENANT_ID; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; @@ -12,6 +15,7 @@ import static org.opensearch.client.RequestOptions.DEFAULT; import java.io.IOException; +import java.util.List; import org.folio.search.domain.dto.ConsortiumLocation; import org.folio.search.model.SearchResult; import org.folio.search.service.converter.ElasticsearchDocumentConverter; @@ -28,7 +32,8 @@ import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchResponse; import org.opensearch.client.RestHighLevelClient; -import org.opensearch.index.query.TermQueryBuilder; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.QueryBuilders; import org.opensearch.search.sort.FieldSortBuilder; import org.opensearch.search.sort.SortOrder; @@ -64,7 +69,7 @@ void fetchLocations_positive() throws IOException { when(client.search(requestCaptor.capture(), eq(DEFAULT))).thenReturn(searchResponse); when(documentConverter.convertToSearchResult(searchResponse, ConsortiumLocation.class)).thenReturn(searchResult); - var actual = repository.fetchLocations(TENANT_ID, null, limit, offset, sortBy, null); + var actual = repository.fetchLocations(TENANT_ID, null, null, limit, offset, sortBy, null); assertThat(actual).isEqualTo(searchResult); assertThat(requestCaptor.getValue()) @@ -84,6 +89,39 @@ void fetchLocations_positive() throws IOException { }); } + @Test + void fetchLocations_positive_withIdFilter() throws IOException { + var limit = 123; + var offset = 321; + var sortBy = "test"; + var searchResponse = mock(SearchResponse.class); + var searchResult = Mockito.>mock(); + + when(client.search(requestCaptor.capture(), eq(DEFAULT))).thenReturn(searchResponse); + when(documentConverter.convertToSearchResult(searchResponse, ConsortiumLocation.class)).thenReturn(searchResult); + + var actual = repository.fetchLocations(TENANT_ID, null, RESOURCE_ID, limit, offset, sortBy, null); + + assertThat(actual).isEqualTo(searchResult); + assertThat(requestCaptor.getValue()) + .matches(request -> request.indices().length == 1 && request.indices()[0].equals(INDEX_NAME)) + .satisfies(request -> { + var source = request.source(); + assertThat(source.size()).isEqualTo(limit); + assertThat(source.from()).isEqualTo(offset); + + assertThat(source.sorts()).hasSize(1); + assertThat(source.sorts().get(0)).isInstanceOf(FieldSortBuilder.class); + var sort = (FieldSortBuilder) source.sorts().get(0); + assertThat(sort.getFieldName()).isEqualTo(sortBy); + assertThat(sort.order()).isEqualTo(SortOrder.ASC); + + var query = (BoolQueryBuilder) source.query(); + assertThat(query.filter()) + .isEqualTo(List.of(QueryBuilders.termQuery(ID_FIELD, RESOURCE_ID))); + }); + } + @Test void fetchLocations_positive_withTenantFilter() throws IOException { var limit = 123; @@ -95,7 +133,7 @@ void fetchLocations_positive_withTenantFilter() throws IOException { when(client.search(requestCaptor.capture(), eq(DEFAULT))).thenReturn(searchResponse); when(documentConverter.convertToSearchResult(searchResponse, ConsortiumLocation.class)).thenReturn(searchResult); - var actual = repository.fetchLocations(TENANT_ID, MEMBER_TENANT_ID, limit, offset, sortBy, null); + var actual = repository.fetchLocations(TENANT_ID, MEMBER_TENANT_ID, null, limit, offset, sortBy, null); assertThat(actual).isEqualTo(searchResult); assertThat(requestCaptor.getValue()) @@ -111,10 +149,9 @@ void fetchLocations_positive_withTenantFilter() throws IOException { assertThat(sort.getFieldName()).isEqualTo(sortBy); assertThat(sort.order()).isEqualTo(SortOrder.ASC); - assertThat(source.query()).isInstanceOf(TermQueryBuilder.class); - var query = (TermQueryBuilder) source.query(); - assertThat(query.fieldName()).isEqualTo("tenantId"); - assertThat(query.value()).isEqualTo(MEMBER_TENANT_ID); + var query = (BoolQueryBuilder) source.query(); + assertThat(query.filter()) + .isEqualTo(List.of(QueryBuilders.termQuery(TENANT_ID_FIELD_NAME, MEMBER_TENANT_ID))); }); } @@ -129,7 +166,7 @@ void fetchLocations_positive_sortDesc() throws IOException { when(client.search(requestCaptor.capture(), eq(DEFAULT))).thenReturn(searchResponse); when(documentConverter.convertToSearchResult(searchResponse, ConsortiumLocation.class)).thenReturn(searchResult); - var actual = repository.fetchLocations(TENANT_ID, null, limit, offset, sortBy, + var actual = repository.fetchLocations(TENANT_ID, null, null, limit, offset, sortBy, org.folio.search.domain.dto.SortOrder.DESC); assertThat(actual).isEqualTo(searchResult); diff --git a/src/test/java/org/folio/search/service/consortium/ConsortiumLocationServiceTest.java b/src/test/java/org/folio/search/service/consortium/ConsortiumLocationServiceTest.java index 0085a12d0..c5c1ac29c 100644 --- a/src/test/java/org/folio/search/service/consortium/ConsortiumLocationServiceTest.java +++ b/src/test/java/org/folio/search/service/consortium/ConsortiumLocationServiceTest.java @@ -43,21 +43,22 @@ public class ConsortiumLocationServiceTest { void fetchLocations_ValidSortBy() { var tenantHeader = CONSORTIUM_TENANT; var tenantId = CONSORTIUM_TENANT; + var locationId = ID; var sortOrder = SortOrder.ASC; var sortBy = NAME; var limit = 10; var offset = 0; var searchResult = prepareSearchResult(); - when(repository.fetchLocations(tenantHeader, tenantId, limit, offset, sortBy, sortOrder)) + when(repository.fetchLocations(tenantHeader, tenantId, locationId, limit, offset, sortBy, sortOrder)) .thenReturn(searchResult); when(executor.execute(eq(tenantId), any(Supplier.class))) .thenAnswer(invocation -> ((Supplier) invocation.getArgument(1)).get()); - var actual = service.fetchLocations(tenantHeader, tenantId, limit, offset, sortBy, sortOrder); + var actual = service.fetchLocations(tenantHeader, tenantId, locationId, limit, offset, sortBy, sortOrder); assertThat(actual).isEqualTo(searchResult); - verify(repository).fetchLocations(tenantHeader, tenantId, limit, offset, sortBy, sortOrder); + verify(repository).fetchLocations(tenantHeader, tenantId, locationId, limit, offset, sortBy, sortOrder); verify(executor).execute(eq(tenantId), any(Supplier.class)); } @@ -68,7 +69,7 @@ void fetchLocations_InvalidSortBy() { var offset = 0; Assertions.assertThrows(RequestValidationException.class, () -> - service.fetchLocations(CONSORTIUM_TENANT, CONSORTIUM_TENANT, limit, offset, "invalid", sortOrder) + service.fetchLocations(CONSORTIUM_TENANT, CONSORTIUM_TENANT, ID, limit, offset, "invalid", sortOrder) ); } @@ -79,7 +80,7 @@ void fetchLocations_InvalidPaginationParameters() { var offset = 9900; Assertions.assertThrows(RequestValidationException.class, () -> - service.fetchLocations(CONSORTIUM_TENANT, CONSORTIUM_TENANT, limit, offset, NAME, sortOrder) + service.fetchLocations(CONSORTIUM_TENANT, CONSORTIUM_TENANT, ID, limit, offset, NAME, sortOrder) ); }