query to return total results count (#3126)

* added totalResultsCount. updated tests, swagger & changelog.

CHANGELOG.md

@@ -4,6 +4,13 @@
 
 * Added a configuration option under `docker.hash-lookup.enabled` to disable docker hash lookup.
  Disabling it will also disable call caching for jobs with floating docker tags.
+
+* **API**    
+    + Updated the `/query` response to include the total number of query results returned. See [here](http://cromwell.readthedocs.io/en/develop/api/RESTAPI/#workflowqueryresponse) for more information.
+
+## 30.1 Release Notes
+
+* A set of bug fixes following the migration of Cromwell to WOM (the Workflow Object Model) in version 30.
 
 ## 30 Release Notes
 
@@ -701,4 +708,4 @@ types](https://github.com/docker/distribution/blob/05b0ab0/docs/spec/manifest-v2
 multiple input files.
 
 * The `/query` endpoint now supports querying by `id`, and submitting
-parameters as a HTTP POST.
+parameters as a HTTP POST.

docs/api/RESTAPI.md

@@ -1,5 +1,5 @@
 <!--
-This file was generated by `sbt generateRestApiDocs` on Fri, 15 Dec 2017 10:46:35 -0500
+This file was generated by `sbt generateRestApiDocs` on Mon, 8 Jan 2018 12:09:07 -0500
 
 !!! DO NOT CHANGE THIS FILE DIRECTLY !!!
 
@@ -851,6 +851,7 @@ Response to a workflow query
 |Name|Schema|
 |---|---|
 |**results**  <br>*required*|< [WorkflowQueryResult](#workflowqueryresult) > array|
+|**totalResultsCount**  <br>*required*|integer|
 
 
 <a name="workflowqueryresult"></a>

engine/src/main/resources/swagger/cromwell.yaml

@@ -805,11 +805,14 @@ definitions:
     description: Response to a workflow query
     required:
       - results
+      - totalResultsCount
     properties:
       results:
         type: array
         items:
           $ref: '#/definitions/WorkflowQueryResult'
+      totalResultsCount:
+        type: integer
   WorkflowQueryResult:
     description: Result for an individual workflow returned by a workflow query
     required:

engine/src/main/scala/cromwell/webservice/WorkflowJsonSupport.scala

@@ -51,5 +51,5 @@ object WorkflowJsonSupport extends DefaultJsonProtocol {
   }
 
   implicit val workflowQueryResult = jsonFormat7(WorkflowQueryResult)
-  implicit val workflowQueryResponse = jsonFormat1(WorkflowQueryResponse)
+  implicit val workflowQueryResponse = jsonFormat2(WorkflowQueryResponse)
 }

engine/src/test/scala/cromwell/webservice/CromwellApiServiceSpec.scala

@@ -557,6 +557,16 @@ class CromwellApiServiceSpec extends AsyncFlatSpec with ScalatestRouteTest with
         }
     }
 
+    it should "include totalResultCount in workflow query response" in {
+      Post(s"/workflows/$version/query", HttpEntity(ContentTypes.`application/json`, """[{"status":"Succeeded"}]""")) ~>
+        akkaHttpService.workflowRoutes ~>
+        check {
+          val result = responseAs[JsObject]
+          status should be(StatusCodes.OK)
+          result.fields("totalResultsCount") should be(JsNumber("1"))
+        }
+    }
+
     behavior of "REST API /labels GET endpoint"
     it should "return labels for a workflow ID" in {
       Get(s"/workflows/$version/${CromwellApiServiceSpec.ExistingWorkflowId}/labels") ~>
@@ -652,7 +662,7 @@ object CromwellApiServiceSpec {
           parameters.contains(("additionalQueryResultFields", "parentWorkflowId")).option("pid")
         }
         val response = WorkflowQuerySuccess(WorkflowQueryResponse(List(WorkflowQueryResult(ExistingWorkflowId.toString,
-          None, Some(WorkflowSucceeded.toString), None, None, labels, parentWorkflowId))), None)
+          None, Some(WorkflowSucceeded.toString), None, None, labels, parentWorkflowId)), 1), None)
         sender ! response
       case ValidateWorkflowId(id) =>
         if (RecognizedWorkflowIds.contains(id)) sender ! MetadataService.RecognizedWorkflowId

services/src/main/scala/cromwell/services/metadata/MetadataService.scala

@@ -19,7 +19,7 @@ object MetadataService {
 
   final case class WorkflowQueryResult(id: String, name: Option[String], status: Option[String], start: Option[OffsetDateTime], end: Option[OffsetDateTime], labels: Option[Map[String, String]], parentWorkflowId: Option[String])
 
-  final case class WorkflowQueryResponse(results: Seq[WorkflowQueryResult])
+  final case class WorkflowQueryResponse(results: Seq[WorkflowQueryResult], totalResultsCount: Int)
 
   final case class QueryMetadata(page: Option[Int], pageSize: Option[Int], totalRecords: Option[Int])
 

services/src/main/scala/cromwell/services/metadata/impl/MetadataDatabaseAccess.scala

@@ -180,7 +180,7 @@ trait MetadataDatabaseAccess {
       queryParameters.startDate.map(_.toSystemTimestamp), queryParameters.endDate.map(_.toSystemTimestamp),
       queryParameters.page, queryParameters.pageSize)
 
-    val workflowSummaryCount = metadataDatabaseInterface.countWorkflowSummaries(
+    val workflowSummaryCount: Future[Int] = metadataDatabaseInterface.countWorkflowSummaries(
       queryParameters.statuses, queryParameters.names, queryParameters.ids.map(_.toString), queryParameters.labels.map(label => (label.key, label.value)),
       queryParameters.startDate.map(_.toSystemTimestamp), queryParameters.endDate.map(_.toSystemTimestamp))
 
@@ -236,7 +236,7 @@ trait MetadataDatabaseAccess {
       count <- workflowSummaryCount
       workflows <- workflowSummaries
       queryResults <- summariesToQueryResults(workflows)
-    } yield (WorkflowQueryResponse(queryResults.toSeq), queryMetadata(count))
+    } yield (WorkflowQueryResponse(queryResults.toSeq, count), queryMetadata(count))
 
   }
 }

services/src/test/scala/cromwell/services/metadata/impl/MetadataDatabaseAccessSpec.scala

@@ -23,6 +23,7 @@ object MetadataDatabaseAccessSpec {
 
   val Workflow1Name = "test1"
   val Workflow2Name = "test2"
+  val Workflow3Name = "test3"
 }
 
 class MetadataDatabaseAccessSpec extends FlatSpec with Matchers with ScalaFutures with BeforeAndAfterAll with Mockito {
@@ -93,7 +94,7 @@ class MetadataDatabaseAccessSpec extends FlatSpec with Matchers with ScalaFuture
         }
       } yield()).futureValue
     }
-    
+
     it should "sort metadata events by timestamp from older to newer" taggedAs DbmsTest in {
       def unorderedEvents(id: WorkflowId): Future[Vector[MetadataEvent]] = {
         val workflowKey = MetadataKey(id, jobKey = None, key = null)
@@ -111,7 +112,7 @@ class MetadataDatabaseAccessSpec extends FlatSpec with Matchers with ScalaFuture
 
         dataAccess.addMetadataEvents(events) map { _ => expectedEvents }
       }
-      
+
       (for {
         workflow1Id <- baseWorkflowMetadata(Workflow1Name)
         expected <- unorderedEvents(workflow1Id)
@@ -249,6 +250,28 @@ class MetadataDatabaseAccessSpec extends FlatSpec with Matchers with ScalaFuture
       } yield ()).futureValue(Timeout(scaled(Span(30, Seconds))), Interval(scaled(Span(500, Millis))))
     }
 
+    it should "return totalResultsCount when page and pagesize query params are specified" taggedAs DbmsTest in {
+      (for {
+        _ <- baseWorkflowMetadata(Workflow3Name)
+        _ <- baseWorkflowMetadata(Workflow3Name)
+        // refresh the metadata
+        _ <- dataAccess.refreshWorkflowMetadataSummaries() map { max =>
+          max should be > 0L
+        }
+        //get totalResultsCount when page and pagesize are specified
+        _ <- dataAccess.queryWorkflowSummaries(WorkflowQueryParameters(Seq(
+          // name being added to the query parameters so as to exclude workflows being populated by the other tests in this spec
+          WorkflowQueryKey.Name.name -> Workflow3Name,
+          WorkflowQueryKey.Page.name -> "1", WorkflowQueryKey.PageSize.name -> "1"))) map { case (resp, _) =>
+          resp.totalResultsCount match {
+            case 2 =>
+            case 1 => fail("totalResultsCount is suspiciously equal to the pageSize and not the expected total results count. Please fix!")
+            case other => fail(s"totalResultsCount is expected to be 2 but is actually $other. Something has gone horribly wrong!")
+          }
+        }
+      } yield()).futureValue
+    }
+
     it should "close the database" taggedAs DbmsTest in {
       dataAccess.metadataDatabaseInterface.close()
     }