fix(search-instances): fix calculating totalNumber in response (#551)

Closes: MSEARCH-710
folio-org · Apr 2, 2024 · d492cef · d492cef
1 parent 8d3c476
commit d492cef
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 16 deletions.
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/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/...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/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 "