From 644967de35ad982894b003dcd9ccd6441b76d258 Mon Sep 17 00:00:00 2001 From: Jeff Gentry Date: Fri, 7 Jul 2017 13:38:21 -0400 Subject: [PATCH] Only gzip metadata if requested. Closes #2419 (#2427) * Only gzip metadata if requested. Closes #2419 --- CHANGELOG.md | 1 + README.md | 10 ++++++++ .../cromwell/webservice/CromwellApiService.scala | 3 +-- .../webservice/CromwellApiServiceSpec.scala | 27 ++++++++++++++++------ 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b79db90d..d0ec11ffc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Breaking Changes * Request timeouts for HTTP requests on the REST API now return a 503 status code instead of 500. The response for a request timeout is no longer in JSON format. +* The metadata endpoint no longer returns gzipped responses by default. This now needs to be explicitly requested with an `Accept-Encoding: gzip` header * Command line usage has been extensively revised for Cromwell 29. Please see the [README](https://github.com/broadinstitute/cromwell#command-line-usage) for details. diff --git a/README.md b/README.md index 3b589e977..93da8659a 100644 --- a/README.md +++ b/README.md @@ -3639,6 +3639,16 @@ The `call` and `workflow` may optionally contain failures shaped like this: ] ``` +### Compressing the metadata response + +The response from the metadata endpoint can be quite large depending on the workflow. To help with this Cromwell supports gzip encoding the metadata prior to sending it back to the client. In order to enable this, make sure your client is sending the `Accept-Encoding: gzip` header. + +For instance, with cURL: + +``` +$ curl -H "Accept-Encoding: gzip" http://localhost:8000/api/workflows/v1/b3e45584-9450-4e73-9523-fc3ccf749848/metadata +``` + ## POST /api/workflows/:version/:id/abort cURL: diff --git a/engine/src/main/scala/cromwell/webservice/CromwellApiService.scala b/engine/src/main/scala/cromwell/webservice/CromwellApiService.scala index 2d29bb4c5..34830cd8e 100644 --- a/engine/src/main/scala/cromwell/webservice/CromwellApiService.scala +++ b/engine/src/main/scala/cromwell/webservice/CromwellApiService.scala @@ -22,7 +22,6 @@ import cromwell.services.metadata.MetadataService._ import cromwell.webservice.metadata.{MetadataBuilderActor, WorkflowQueryPagination} import cromwell.webservice.metadata.MetadataBuilderActor.{BuiltMetadataResponse, FailedMetadataResponse, MetadataBuilderActorResponse} import WorkflowJsonSupport._ -import akka.http.scaladsl.coding.{Deflate, Gzip, NoCoding} import akka.http.scaladsl.server.Route import cats.data.NonEmptyList import cats.data.Validated.{Invalid, Valid} @@ -87,7 +86,7 @@ trait CromwellApiService { } } } ~ - encodeResponseWith(Gzip, Deflate, NoCoding) { + encodeResponse { path("workflows" / Segment / Segment / "metadata") { (version, possibleWorkflowId) => parameters(('includeKey.*, 'excludeKey.*, 'expandSubWorkflows.as[Boolean].?)) { (includeKeys, excludeKeys, expandSubWorkflowsOption) => val includeKeysOption = NonEmptyList.fromList(includeKeys.toList) diff --git a/engine/src/test/scala/cromwell/webservice/CromwellApiServiceSpec.scala b/engine/src/test/scala/cromwell/webservice/CromwellApiServiceSpec.scala index 654a333aa..5537e64e0 100644 --- a/engine/src/test/scala/cromwell/webservice/CromwellApiServiceSpec.scala +++ b/engine/src/test/scala/cromwell/webservice/CromwellApiServiceSpec.scala @@ -12,6 +12,7 @@ import cromwell.engine.workflow.workflowstore.WorkflowStoreEngineActor.WorkflowA import cromwell.engine.workflow.workflowstore.WorkflowStoreSubmitActor.{WorkflowSubmittedToStore, WorkflowsBatchSubmittedToStore} import cromwell.services.metadata.MetadataService._ import akka.http.scaladsl.model._ +import akka.http.scaladsl.model.headers.{HttpEncodings, `Accept-Encoding`} import akka.http.scaladsl.testkit.{RouteTestTimeout, ScalatestRouteTest} import akka.http.scaladsl.unmarshalling.Unmarshal import akka.stream.ActorMaterializer @@ -20,7 +21,6 @@ import cromwell.util.SampleWdl.HelloWorld import org.scalatest.{AsyncFlatSpec, Matchers} import spray.json._ -import scala.concurrent.Await import scala.concurrent.duration._ class CromwellApiServiceSpec extends AsyncFlatSpec with ScalatestRouteTest with Matchers { @@ -321,8 +321,7 @@ class CromwellApiServiceSpec extends AsyncFlatSpec with ScalatestRouteTest with akkaHttpService.routes ~> check { status should be(StatusCodes.OK) - val decoder: Decoder = Gzip - val result = Await.result(Unmarshal(decoder.decodeMessage(response)).to[JsObject], 1.second) + val result = responseAs[JsObject] result.fields.keys should contain allOf("testKey1", "testKey2") result.fields.keys shouldNot contain("testKey3") result.fields("testKey1") should be(JsString("myValue1")) @@ -330,13 +329,28 @@ class CromwellApiServiceSpec extends AsyncFlatSpec with ScalatestRouteTest with } } + it should "return with gzip encoding when requested" in { + Get(s"/workflows/$version/${CromwellApiServiceSpec.ExistingWorkflowId}/metadata").addHeader(`Accept-Encoding`(HttpEncodings.gzip)) ~> + akkaHttpService.routes ~> + check { + response.headers.find(_.name == "Content-Encoding").get.value should be("gzip") + } + } + + it should "not return with gzip encoding when not requested" in { + Get(s"/workflows/$version/${CromwellApiServiceSpec.ExistingWorkflowId}/metadata") ~> + akkaHttpService.routes ~> + check { + response.headers.find(_.name == "Content-Encoding") shouldBe None + } + } + it should "return with included metadata from the metadata route" in { Get(s"/workflows/$version/${CromwellApiServiceSpec.ExistingWorkflowId}/metadata?includeKey=testKey1&includeKey=testKey2a") ~> akkaHttpService.routes ~> check { status should be(StatusCodes.OK) - val decoder: Decoder = Gzip - val result = Await.result(Unmarshal(decoder.decodeMessage(response)).to[JsObject], 1.second) + val result = responseAs[JsObject] result.fields.keys should contain allOf("testKey1a", "testKey1b", "testKey2a") result.fields.keys should contain noneOf("testKey2b", "testKey3") result.fields("testKey1a") should be(JsString("myValue1a")) @@ -350,8 +364,7 @@ class CromwellApiServiceSpec extends AsyncFlatSpec with ScalatestRouteTest with akkaHttpService.routes ~> check { status should be(StatusCodes.OK) - val decoder: Decoder = Gzip - val result = Await.result(Unmarshal(decoder.decodeMessage(response)).to[JsObject], 1.second) + val result = responseAs[JsObject] result.fields.keys should contain allOf("testKey1a", "testKey1b", "testKey2a") result.fields.keys should contain noneOf("testKey2b", "testKey3") result.fields("testKey1a") should be(JsString("myValue1a"))