CORDA-861 Pagination failure with aggregate groupBy query (#3135)

* Pagination relies on a recursive call to count total results, this sub-query should NOT perform pagination checks.

* Fix using defaulted parameter.

* Make internal method private.

node/src/main/kotlin/net/corda/node/services/vault/NodeVaultService.kt

@@ -397,13 +397,18 @@ class NodeVaultService(
 
     @Throws(VaultQueryException::class)
     override fun <T : ContractState> _queryBy(criteria: QueryCriteria, paging: PageSpecification, sorting: Sort, contractStateType: Class<out T>): Vault.Page<T> {
+        return _queryBy(criteria, paging, sorting, contractStateType, false)
+    }
+
+    @Throws(VaultQueryException::class)
+    private fun <T : ContractState> _queryBy(criteria: QueryCriteria, paging: PageSpecification, sorting: Sort, contractStateType: Class<out T>, skipPagingChecks: Boolean): Vault.Page<T> {
         log.info("Vault Query for contract type: $contractStateType, criteria: $criteria, pagination: $paging, sorting: $sorting")
         // calculate total results where a page specification has been defined
         var totalStates = -1L
-        if (!paging.isDefault) {
+        if (!skipPagingChecks && !paging.isDefault) {
             val count = builder { VaultSchemaV1.VaultStates::recordedTime.count() }
             val countCriteria = QueryCriteria.VaultCustomQueryCriteria(count, Vault.StateStatus.ALL)
-            val results = queryBy(contractStateType, criteria.and(countCriteria))
+            val results = _queryBy(criteria.and(countCriteria), PageSpecification(), Sort(emptyList()), contractStateType, true)  // only skip pagination checks for total results count query
             totalStates = results.otherResults.last() as Long
         }
 
@@ -422,7 +427,7 @@ class NodeVaultService(
         val query = session.createQuery(criteriaQuery)
 
         // pagination checks
-        if (!paging.isDefault) {
+        if (!skipPagingChecks && !paging.isDefault) {
             // pagination
             if (paging.pageNumber < DEFAULT_PAGE_NUM) throw VaultQueryException("Page specification: invalid page number ${paging.pageNumber} [page numbers start from $DEFAULT_PAGE_NUM]")
             if (paging.pageSize < 1) throw VaultQueryException("Page specification: invalid page size ${paging.pageSize} [must be a value between 1 and $MAX_PAGE_SIZE]")
@@ -435,7 +440,7 @@ class NodeVaultService(
         val results = query.resultList
 
         // final pagination check (fail-fast on too many results when no pagination specified)
-        if (paging.isDefault && results.size > DEFAULT_PAGE_SIZE)
+        if (!skipPagingChecks && paging.isDefault && results.size > DEFAULT_PAGE_SIZE)
             throw VaultQueryException("Please specify a `PageSpecification` as there are more results [${results.size}] than the default page size [$DEFAULT_PAGE_SIZE]")
 
         val statesAndRefs: MutableList<StateAndRef<T>> = mutableListOf()

node/src/test/kotlin/net/corda/node/services/vault/VaultQueryTests.kt

@@ -28,10 +28,7 @@ import net.corda.nodeapi.internal.persistence.DatabaseTransaction
 import net.corda.testing.core.*
 import net.corda.testing.internal.TEST_TX_TIME
 import net.corda.testing.internal.rigorousMock
-import net.corda.testing.internal.vault.DUMMY_LINEAR_CONTRACT_PROGRAM_ID
+import net.corda.testing.internal.vault.*
-import net.corda.testing.internal.vault.DummyLinearContract
-import net.corda.testing.internal.vault.DummyLinearStateSchemaV1
-import net.corda.testing.internal.vault.VaultFiller
 import net.corda.testing.node.MockServices
 import net.corda.testing.node.MockServices.Companion.makeTestDatabaseAndMockServices
 import net.corda.testing.node.makeTestIdentityService
@@ -1182,6 +1179,30 @@ abstract class VaultQueryTestsBase : VaultQueryParties {
         }
     }
 
+    // test paging with aggregate function and group by clause
+    @Test
+    fun `test paging with aggregate function and group by clause`() {
+        database.transaction {
+            (0..200).forEach {
+                vaultFiller.fillWithSomeTestLinearStates(1, linearNumber = it.toLong(), linearString = it.toString())
+            }
+            val max = builder { DummyLinearStateSchemaV1.PersistentDummyLinearState::linearTimestamp.max(
+                    groupByColumns = listOf(DummyLinearStateSchemaV1.PersistentDummyLinearState::linearNumber)
+                )
+            }
+            val maxCriteria = VaultCustomQueryCriteria(max)
+            val pageSpec = PageSpecification(DEFAULT_PAGE_NUM, MAX_PAGE_SIZE)
+
+            val results = vaultService.queryBy<DummyLinearContract.State>(maxCriteria, paging = pageSpec)
+            println("Total states available: ${results.totalStatesAvailable}")
+            results.otherResults.forEachIndexed { index, any ->
+                println("$index : $any")
+            }
+            assertThat(results.otherResults.size).isEqualTo(402)
+            assertThat(results.otherResults.last()).isEqualTo(200L)
+        }
+    }
+
     // sorting
     @Test
     fun `sorting - all states sorted by contract type, state status, consumed time`() {