Merge pull request #556 from folio-org/tmp-release-v3.2.1

Prepare next development iteration Quesnelia
folio-org · Apr 4, 2024 · b43a40b · b43a40b
2 parents e9ecaa6 + 9941949
commit b43a40b
Show file tree

Hide file tree

Showing 13 changed files with 109 additions and 24 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,3 +1,10 @@
+## v3.2.1 2024-04-04
+### Bug fixes
+* Fix browsing in one direction by shelving order ([MSEARCH-704](https://issues.folio.org/browse/MSEARCH-704))
+* Fix calculating totalNumber in search instances response ([MSEARCH-710](https://issues.folio.org/browse/MSEARCH-710))
+
+---
+
 ## v3.2.0 2024-03-20
 ### New APIs versions
 * Requires `classification-types v1.2`

diff --git a/pom.xml b/pom.xml
@@ -12,7 +12,7 @@
   <name>mod-search</name>
   <groupId>org.folio</groupId>
   <artifactId>mod-search</artifactId>
-  <version>3.2.1-SNAPSHOT</version>
+  <version>3.2.2-SNAPSHOT</version>
   <description>FOLIO search service</description>
   <packaging>jar</packaging>
 

diff --git a/src/main/java/org/folio/search/model/service/ConsortiumSearchContext.java b/src/main/java/org/folio/search/model/service/ConsortiumSearchContext.java
@@ -19,7 +19,7 @@ public class ConsortiumSearchContext {
 
   private static final Map<ResourceType, List<String>> ALLOWED_SORT_FIELDS = Map.of(
     ResourceType.HOLDINGS, List.of("id", "hrid", "tenantId", "instanceId",
-      "callNumberPrefix", "callNumber", "copyNumber", "permanentLocationId"),
+      "callNumberPrefix", "callNumber", "callNumberSuffix", "copyNumber", "permanentLocationId"),
     ResourceType.ITEM, List.of("id", "hrid", "tenantId", "instanceId", "holdingsRecordId", "barcode")
   );
 

diff --git a/src/main/java/org/folio/search/service/browse/CallNumberBrowseService.java b/src/main/java/org/folio/search/service/browse/CallNumberBrowseService.java
@@ -11,6 +11,7 @@
 import static org.folio.search.model.types.CallNumberTypeSource.FOLIO;
 import static org.folio.search.utils.CallNumberUtils.excludeIrrelevantResultItems;
 import static org.folio.search.utils.CollectionUtils.mergeSafelyToList;
+import static org.folio.search.utils.SearchUtils.SHELVING_ORDER_BROWSING_FIELD;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -57,7 +58,9 @@ protected BrowseResult<CallNumberBrowseItem> browseInOneDirection(BrowseRequest
     log.debug("browseInOneDirection:: by: [request: {}]", request);
 
     var callNumber = callNumberFromRequest(request);
-    var initialAnchor = effectiveShelvingOrderTermProcessor.getSearchTerm(callNumber, request.getRefinedCondition());
+    var initialAnchor = request.getQuery().contains(SHELVING_ORDER_BROWSING_FIELD)
+      ? context.getAnchor()
+      : effectiveShelvingOrderTermProcessor.getSearchTerm(callNumber, request.getRefinedCondition());
     if (!initialAnchor.equals(context.getAnchor())) {
       context = buildBrowseContext(context, initialAnchor);
     }

diff --git a/src/main/java/org/folio/search/service/consortium/ConsortiumInstanceRepository.java b/src/main/java/org/folio/search/service/consortium/ConsortiumInstanceRepository.java
@@ -86,13 +86,19 @@ public List<ConsortiumHolding> fetchHoldings(ConsortiumSearchQueryBuilder search
         .instanceId(rs.getString("instanceId"))
         .callNumberPrefix(rs.getString("callNumberPrefix"))
         .callNumber(rs.getString("callNumber"))
+        .callNumberSuffix(rs.getString("callNumberSuffix"))
         .copyNumber(rs.getString("copyNumber"))
         .permanentLocationId(rs.getString("permanentLocationId"))
         .discoverySuppress(rs.getBoolean("discoverySuppress")),
       searchQueryBuilder.getQueryArguments()
     );
   }
 
+  public Integer count(ConsortiumSearchQueryBuilder searchQueryBuilder) {
+    return jdbcTemplate.queryForObject(searchQueryBuilder.buildCountQuery(context),
+      Integer.class, searchQueryBuilder.getQueryArguments());
+  }
+
   public List<ConsortiumItem> fetchItems(ConsortiumSearchQueryBuilder searchQueryBuilder) {
     return jdbcTemplate.query(searchQueryBuilder.buildSelectQuery(context),
       (rs, rowNum) -> new ConsortiumItem()

diff --git a/src/main/java/org/folio/search/service/consortium/ConsortiumInstanceService.java b/src/main/java/org/folio/search/service/consortium/ConsortiumInstanceService.java
@@ -153,13 +153,15 @@ public List<ResourceEvent> fetchInstances(Iterable<String> instanceIds) {
   }
 
   public ConsortiumHoldingCollection fetchHoldings(ConsortiumSearchContext context) {
-    List<ConsortiumHolding> holdingList = repository.fetchHoldings(new ConsortiumSearchQueryBuilder(context));
-    return new ConsortiumHoldingCollection().holdings(holdingList).totalRecords(holdingList.size());
+    var searchQueryBuilder = new ConsortiumSearchQueryBuilder(context);
+    List<ConsortiumHolding> holdingList = repository.fetchHoldings(searchQueryBuilder);
+    return new ConsortiumHoldingCollection().holdings(holdingList).totalRecords(repository.count(searchQueryBuilder));
   }
 
   public ConsortiumItemCollection fetchItems(ConsortiumSearchContext context) {
-    List<ConsortiumItem> itemList = repository.fetchItems(new ConsortiumSearchQueryBuilder(context));
-    return new ConsortiumItemCollection().items(itemList).totalRecords(itemList.size());
+    var searchQueryBuilder = new ConsortiumSearchQueryBuilder(context);
+    List<ConsortiumItem> itemList = repository.fetchItems(searchQueryBuilder);
+    return new ConsortiumItemCollection().items(itemList).totalRecords(repository.count(searchQueryBuilder));
   }
 
   @SuppressWarnings("unchecked")

diff --git a/src/main/java/org/folio/search/service/consortium/ConsortiumSearchQueryBuilder.java b/src/main/java/org/folio/search/service/consortium/ConsortiumSearchQueryBuilder.java
@@ -29,7 +29,8 @@ public class ConsortiumSearchQueryBuilder {
   );
   private static final Map<ResourceType, List<String>> RESOURCE_FIELDS = Map.of(
     ResourceType.HOLDINGS,
-    List.of("id", "hrid", "callNumberPrefix", "callNumber", "copyNumber", "permanentLocationId", "discoverySuppress"),
+    List.of("id", "hrid", "callNumberPrefix", "callNumber", "callNumberSuffix",
+      "copyNumber", "permanentLocationId", "discoverySuppress"),
     ResourceType.ITEM,
     List.of("id", "hrid", "holdingsRecordId", "barcode")
   );
@@ -75,6 +76,16 @@ public String buildSelectQuery(FolioExecutionContext context) {
     return StringUtils.normalizeSpace(query);
   }
 
+  public String buildCountQuery(FolioExecutionContext context) {
+    var fullTableName = getFullTableName(context, CONSORTIUM_TABLES.get(resourceType));
+    var resourceCollection = RESOURCE_COLLECTION_NAME.get(resourceType);
+    String subQuery = "SELECT instance_id, tenant_id, json_array_elements(json -> '" + resourceCollection + "') "
+                      + "as " + resourceCollection + " FROM " + fullTableName + SPACE + getWhereClause(filters, null);
+    String query = "SELECT count(*) FROM (" + subQuery + ") i"
+                   + getWhereClause(jsonbFilters, "i." + resourceCollection);
+    return StringUtils.normalizeSpace(query);
+  }
+
   public Object[] getQueryArguments() {
     return Stream.concat(filters.stream(), jsonbFilters.stream())
       .map(Pair::getSecond)

diff --git a/src/main/resources/swagger.api/mod-search.yaml b/src/main/resources/swagger.api/mod-search.yaml
@@ -802,6 +802,9 @@ components:
         callNumber:
           description: Call number
           type: string
+        callNumberSuffix:
+          description: Call number suffix
+          type: string
         copyNumber:
           description: Copy number
           type: string
@@ -1024,6 +1027,7 @@ components:
           - instanceId
           - callNumberPrefix
           - callNumber
+          - callNumberSuffix
           - copyNumber
           - permanentLocationId
       required: false

diff --git a/src/test/java/org/folio/search/controller/BrowseTypedCallNumberIT.java b/src/test/java/org/folio/search/controller/BrowseTypedCallNumberIT.java
@@ -109,6 +109,23 @@ void browseByCallNumberDewey_invalidDeweyFormat() {
       )));
   }
 
+  @Test
+  void browseByShelfKeyDewey_backward() {
+    var request = get(instanceCallNumberBrowsePath())
+      .param("query", prepareQuery("itemEffectiveShelvingOrder < {value}", "\"A 3123.4\""))
+      .param("callNumberType", "dewey")
+      .param("limit", "5")
+      .param("expandAll", "true");
+    var actual = parseResponse(doGet(request), CallNumberBrowseResult.class);
+    assertThat(actual).isEqualTo(new CallNumberBrowseResult()
+      .totalRecords(6).prev(null).next("11 CE 3210 K 3297 541858").items(List.of(
+        cnBrowseItem(instance("instance #44"), "1CE 16 B6713 X 41993"),
+        cnBrowseItem(DEWEY, "1CE 16 B6724 41993", 2, null),
+        cnBrowseItem(instance("instance #04"), "1CE 16 D86 X 41998"),
+        cnBrowseItem(instance("instance #38"), "1CE 210 K297 41858")
+      )));
+  }
+
   @Test
   void browseByCallNumberNlm_browsingAroundWhenPrecedingRecordsCountIsSpecified() {
     var request = get(instanceCallNumberBrowsePath())

diff --git a/...ontroller/SearchHoldingsConsortiumIT.java → ...ontroller/ConsortiumSearchHoldingsIT.java b/...ontroller/SearchHoldingsConsortiumIT.java → ...ontroller/ConsortiumSearchHoldingsIT.java
@@ -25,7 +25,7 @@
 import org.junit.jupiter.api.Test;
 
 @IntegrationTest
-class SearchHoldingsConsortiumIT extends BaseConsortiumIntegrationTest {
+class ConsortiumSearchHoldingsIT extends BaseConsortiumIntegrationTest {
 
   @BeforeAll
   static void prepare() {
@@ -60,7 +60,7 @@ void doGetConsortiumHoldings_returns200AndRecords_withAllQueryParams() {
     var result = doGet(consortiumHoldingsSearchPath(queryParams), CENTRAL_TENANT_ID);
     var actual = parseResponse(result, ConsortiumHoldingCollection.class);
 
-    assertThat(actual.getTotalRecords()).isEqualTo(1);
+    assertThat(actual.getTotalRecords()).isEqualTo(3);
     assertThat(actual.getHoldings())
       .satisfiesExactly(input -> assertEquals("call number", input.getCallNumber()));
   }
@@ -101,6 +101,7 @@ private ConsortiumHolding[] getExpectedHoldings() {
         .instanceId(instance.getId())
         .callNumberPrefix(holding.getCallNumberPrefix())
         .callNumber(holding.getCallNumber())
+        .callNumberSuffix(holding.getCallNumberSuffix())
         .copyNumber(holding.getCopyNumber())
         .permanentLocationId(holding.getPermanentLocationId())
         .discoverySuppress(holding.getDiscoverySuppress() != null && holding.getDiscoverySuppress()))

diff --git a/...h/controller/SearchItemsConsortiumIT.java → ...h/controller/ConsortiumSearchItemsIT.java b/...h/controller/SearchItemsConsortiumIT.java → ...h/controller/ConsortiumSearchItemsIT.java
@@ -25,7 +25,7 @@
 import org.junit.jupiter.api.Test;
 
 @IntegrationTest
-class SearchItemsConsortiumIT extends BaseConsortiumIntegrationTest {
+class ConsortiumSearchItemsIT extends BaseConsortiumIntegrationTest {
 
   @BeforeAll
   static void prepare() {
@@ -64,7 +64,7 @@ void doGetConsortiumItems_returns200AndRecords_withAllQueryParams() {
     var result = doGet(consortiumItemsSearchPath(queryParams), CENTRAL_TENANT_ID);
     var actual = parseResponse(result, ConsortiumItemCollection.class);
 
-    assertThat(actual.getTotalRecords()).isEqualTo(1);
+    assertThat(actual.getTotalRecords()).isEqualTo(2);
     assertThat(actual.getItems())
       .satisfiesExactly(input -> assertEquals("10101", input.getBarcode()));
   }

diff --git a/src/test/java/org/folio/search/service/browse/CallNumberBrowseServiceTest.java b/src/test/java/org/folio/search/service/browse/CallNumberBrowseServiceTest.java
@@ -6,6 +6,7 @@
 import static org.apache.lucene.search.TotalHits.Relation.EQUAL_TO;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.folio.search.utils.SearchUtils.CALL_NUMBER_BROWSING_FIELD;
+import static org.folio.search.utils.SearchUtils.SHELVING_ORDER_BROWSING_FIELD;
 import static org.folio.search.utils.TestConstants.RESOURCE_NAME;
 import static org.folio.search.utils.TestConstants.TENANT_ID;
 import static org.folio.search.utils.TestUtils.cnBrowseItem;
@@ -15,6 +16,7 @@
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 import static org.opensearch.index.query.QueryBuilders.rangeQuery;
 
@@ -359,6 +361,27 @@ void browse_positive_backward() {
       cnBrowseItem(instance("A1"), "A1"), cnBrowseItem(instance("A2"), "A2"))));
   }
 
+  @Test
+  void browse_positive_backwardShelvingOrderTyped() {
+    var request = request("itemEffectiveShelvingOrder < B", SHELVING_ORDER_BROWSING_FIELD, false, "lc");
+    var query = rangeQuery(SHELVING_ORDER_BROWSING_FIELD).lt(ANCHOR);
+    var context = BrowseContext.builder().precedingQuery(query).precedingLimit(5).anchor(ANCHOR).build();
+    var browseItems = browseItems("A1", "A2");
+    browseItems.forEach(browseItem -> browseItem.getInstance().getItems().get(0).getEffectiveCallNumberComponents()
+      .setTypeId(CallNumberType.LC.getId()));
+    var browseResult = BrowseResult.of(2, browseItems).next("A2");
+
+    when(browseContextProvider.get(request)).thenReturn(context);
+    when(browseQueryProvider.get(request, context, false)).thenReturn(precedingQuery);
+    when(searchRepository.search(request, precedingQuery)).thenReturn(precedingResponse);
+    when(browseResultConverter.convert(precedingResponse, context, false)).thenReturn(browseResult);
+
+    var actual = callNumberBrowseService.browse(request);
+
+    assertThat(actual).isEqualTo(browseResult);
+    verifyNoInteractions(shelvingOrderProcessor);
+  }
+
   @Test
   void browse_positive_backwardWithPrevValue() {
     var request = request("callNumber < B", false);
@@ -517,24 +540,29 @@ private static BrowseContext contextAroundIncluding(String anchor, String search
   }
 
   private static BrowseRequest request(String query, boolean highlightMatch) {
-    return request(query, highlightMatch, null, 1, 2);
+    return request(query, CALL_NUMBER_BROWSING_FIELD, highlightMatch, null, 1, 2);
   }
 
   private static BrowseRequest request(String query, boolean highlightMatch, String callNumberType) {
-    return request(query, highlightMatch, callNumberType, 1, 2);
+    return request(query, CALL_NUMBER_BROWSING_FIELD, highlightMatch, callNumberType, 1, 2);
+  }
+
+  private static BrowseRequest request(String query, String browsingField, boolean highlightMatch,
+                                       String callNumberType) {
+    return request(query, browsingField, highlightMatch, callNumberType, 1, 2);
   }
 
   private static BrowseRequest request(String query, boolean highlightMatch, int precedingCount, int limit) {
-    return request(query, highlightMatch, null, precedingCount, limit);
+    return request(query, CALL_NUMBER_BROWSING_FIELD, highlightMatch, null, precedingCount, limit);
   }
 
-  private static BrowseRequest request(String query, boolean highlightMatch, String callNumberType, int precedingCount,
-                                       int limit) {
+  private static BrowseRequest request(String query, String browsingField, boolean highlightMatch,
+                                       String callNumberType, int precedingCount, int limit) {
     return BrowseRequest.builder().tenantId(TENANT_ID).resource(RESOURCE_NAME)
       .query(query)
       .highlightMatch(highlightMatch)
       .expandAll(false)
-      .targetField(CALL_NUMBER_BROWSING_FIELD)
+      .targetField(browsingField)
       .precedingRecordsCount(precedingCount)
       .limit(limit)
       .refinedCondition(callNumberType)

diff --git a/src/test/java/org/folio/search/service/consortium/ConsortiumSearchQueryBuilderTest.java b/src/test/java/org/folio/search/service/consortium/ConsortiumSearchQueryBuilderTest.java
@@ -50,7 +50,8 @@ void testBuildSelectQuery_forHoldingsResource_whenAllParametersDefined() {
     var actual = new ConsortiumSearchQueryBuilder(searchContext).buildSelectQuery(executionContext);
     assertEquals("SELECT i.instance_id as instanceId, i.tenant_id as tenantId, i.holdings ->> 'id' AS id, "
                  + "i.holdings ->> 'hrid' AS hrid, i.holdings ->> 'callNumberPrefix' AS callNumberPrefix, "
-                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'copyNumber' AS copyNumber, "
+                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'callNumberSuffix' AS callNumberSuffix, "
+                 + "i.holdings ->> 'copyNumber' AS copyNumber, "
                  + "i.holdings ->> 'permanentLocationId' AS permanentLocationId, "
                  + "i.holdings ->> 'discoverySuppress' AS discoverySuppress "
                  + "FROM (SELECT instance_id, tenant_id, json_array_elements(json -> 'holdings') as holdings "
@@ -67,7 +68,8 @@ void testBuildSelectQuery_forHoldingsResource_whenFiltersEmpty(String instanceId
     var actual = new ConsortiumSearchQueryBuilder(searchContext).buildSelectQuery(executionContext);
     assertEquals("SELECT i.instance_id as instanceId, i.tenant_id as tenantId, i.holdings ->> 'id' AS id, "
                  + "i.holdings ->> 'hrid' AS hrid, i.holdings ->> 'callNumberPrefix' AS callNumberPrefix, "
-                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'copyNumber' AS copyNumber, "
+                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'callNumberSuffix' AS callNumberSuffix, "
+                 + "i.holdings ->> 'copyNumber' AS copyNumber, "
                  + "i.holdings ->> 'permanentLocationId' AS permanentLocationId, "
                  + "i.holdings ->> 'discoverySuppress' AS discoverySuppress "
                  + "FROM (SELECT instance_id, tenant_id, json_array_elements(json -> 'holdings') as holdings "
@@ -83,7 +85,8 @@ void testBuildSelectQuery_forHoldingsResource_whenSortByEmpty(String sortBy) {
     var actual = new ConsortiumSearchQueryBuilder(searchContext).buildSelectQuery(executionContext);
     assertEquals("SELECT i.instance_id as instanceId, i.tenant_id as tenantId, i.holdings ->> 'id' AS id, "
                  + "i.holdings ->> 'hrid' AS hrid, i.holdings ->> 'callNumberPrefix' AS callNumberPrefix, "
-                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'copyNumber' AS copyNumber, "
+                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'callNumberSuffix' AS callNumberSuffix, "
+                 + "i.holdings ->> 'copyNumber' AS copyNumber, "
                  + "i.holdings ->> 'permanentLocationId' AS permanentLocationId, "
                  + "i.holdings ->> 'discoverySuppress' AS discoverySuppress "
                  + "FROM (SELECT instance_id, tenant_id, json_array_elements(json -> 'holdings') as holdings "
@@ -98,7 +101,8 @@ void testBuildSelectQuery_forHoldingsResource_whenSortOrderEmpty() {
     var actual = new ConsortiumSearchQueryBuilder(searchContext).buildSelectQuery(executionContext);
     assertEquals("SELECT i.instance_id as instanceId, i.tenant_id as tenantId, i.holdings ->> 'id' AS id, "
                  + "i.holdings ->> 'hrid' AS hrid, i.holdings ->> 'callNumberPrefix' AS callNumberPrefix, "
-                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'copyNumber' AS copyNumber, "
+                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'callNumberSuffix' AS callNumberSuffix, "
+                 + "i.holdings ->> 'copyNumber' AS copyNumber, "
                  + "i.holdings ->> 'permanentLocationId' AS permanentLocationId, "
                  + "i.holdings ->> 'discoverySuppress' AS discoverySuppress "
                  + "FROM (SELECT instance_id, tenant_id, json_array_elements(json -> 'holdings') as holdings "
@@ -113,7 +117,8 @@ void testBuildSelectQuery_forHoldingsResource_whenLimitEmpty() {
     var actual = new ConsortiumSearchQueryBuilder(searchContext).buildSelectQuery(executionContext);
     assertEquals("SELECT i.instance_id as instanceId, i.tenant_id as tenantId, i.holdings ->> 'id' AS id, "
                  + "i.holdings ->> 'hrid' AS hrid, i.holdings ->> 'callNumberPrefix' AS callNumberPrefix, "
-                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'copyNumber' AS copyNumber, "
+                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'callNumberSuffix' AS callNumberSuffix, "
+                 + "i.holdings ->> 'copyNumber' AS copyNumber, "
                  + "i.holdings ->> 'permanentLocationId' AS permanentLocationId, "
                  + "i.holdings ->> 'discoverySuppress' AS discoverySuppress "
                  + "FROM (SELECT instance_id, tenant_id, json_array_elements(json -> 'holdings') as holdings "
@@ -128,7 +133,8 @@ void testBuildSelectQuery_forHoldingsResource_whenOffsetEmpty() {
     var actual = new ConsortiumSearchQueryBuilder(searchContext).buildSelectQuery(executionContext);
     assertEquals("SELECT i.instance_id as instanceId, i.tenant_id as tenantId, i.holdings ->> 'id' AS id, "
                  + "i.holdings ->> 'hrid' AS hrid, i.holdings ->> 'callNumberPrefix' AS callNumberPrefix, "
-                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'copyNumber' AS copyNumber, "
+                 + "i.holdings ->> 'callNumber' AS callNumber, i.holdings ->> 'callNumberSuffix' AS callNumberSuffix, "
+                 + "i.holdings ->> 'copyNumber' AS copyNumber, "
                  + "i.holdings ->> 'permanentLocationId' AS permanentLocationId, "
                  + "i.holdings ->> 'discoverySuppress' AS discoverySuppress "
                  + "FROM (SELECT instance_id, tenant_id, json_array_elements(json -> 'holdings') as holdings "