You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by ra...@apache.org on 2018/08/22 18:42:31 UTC

[incubator-openwhisk] branch master updated: Add the activation id in the HTTP header response (#3671)

This is an automated email from the ASF dual-hosted git repository.

rabbah pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git


The following commit(s) were added to refs/heads/master by this push:
     new 75472f8  Add the activation id in the HTTP header response (#3671)
75472f8 is described below

commit 75472f88ff5c165ff13a59f65273fdddb8a8e45c
Author: Dominic Kim <st...@gmail.com>
AuthorDate: Thu Aug 23 03:42:26 2018 +0900

    Add the activation id in the HTTP header response (#3671)
---
 .../main/scala/whisk/core/controller/Actions.scala | 34 ++++++++-------
 .../scala/whisk/core/controller/Entities.scala     | 15 +++++--
 .../scala/whisk/core/controller/Triggers.scala     |  4 +-
 .../scala/whisk/core/controller/WebActions.scala   | 50 ++++++++++++----------
 .../core/controller/test/ActionsApiTests.scala     | 12 ++++++
 .../controller/test/ControllerTestCommon.scala     |  6 +--
 .../core/controller/test/TriggersApiTests.scala    |  4 +-
 .../core/controller/test/WebActionsApiTests.scala  | 18 +++++---
 8 files changed, 91 insertions(+), 52 deletions(-)

diff --git a/core/controller/src/main/scala/whisk/core/controller/Actions.scala b/core/controller/src/main/scala/whisk/core/controller/Actions.scala
index 12c8ec6..bd92c58 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Actions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Actions.scala
@@ -256,24 +256,28 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
     onComplete(invokeAction(user, actionWithMergedParams, payload, waitForResponse, cause = None)) {
       case Success(Left(activationId)) =>
         // non-blocking invoke or blocking invoke which got queued instead
-        complete(Accepted, activationId.toJsObject)
+        respondWithActivationIdHeader(activationId) {
+          complete(Accepted, activationId.toJsObject)
+        }
       case Success(Right(activation)) =>
         val response = if (result) activation.resultAsJson else activation.toExtendedJson
 
-        if (activation.response.isSuccess) {
-          complete(OK, response)
-        } else if (activation.response.isApplicationError) {
-          // actions that result is ApplicationError status are considered a 'success'
-          // and will have an 'error' property in the result - the HTTP status is OK
-          // and clients must check the response status if it exists
-          // NOTE: response status will not exist in the JSON object if ?result == true
-          // and instead clients must check if 'error' is in the JSON
-          // PRESERVING OLD BEHAVIOR and will address defect in separate change
-          complete(BadGateway, response)
-        } else if (activation.response.isContainerError) {
-          complete(BadGateway, response)
-        } else {
-          complete(InternalServerError, response)
+        respondWithActivationIdHeader(activation.activationId) {
+          if (activation.response.isSuccess) {
+            complete(OK, response)
+          } else if (activation.response.isApplicationError) {
+            // actions that result is ApplicationError status are considered a 'success'
+            // and will have an 'error' property in the result - the HTTP status is OK
+            // and clients must check the response status if it exists
+            // NOTE: response status will not exist in the JSON object if ?result == true
+            // and instead clients must check if 'error' is in the JSON
+            // PRESERVING OLD BEHAVIOR and will address defect in separate change
+            complete(BadGateway, response)
+          } else if (activation.response.isContainerError) {
+            complete(BadGateway, response)
+          } else {
+            complete(InternalServerError, response)
+          }
         }
       case Failure(t: RecordTooLargeException) =>
         logging.debug(this, s"[POST] action payload was too large")
diff --git a/core/controller/src/main/scala/whisk/core/controller/Entities.scala b/core/controller/src/main/scala/whisk/core/controller/Entities.scala
index cd9a72a..b1f34b3 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Entities.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Entities.scala
@@ -20,9 +20,9 @@ package whisk.core.controller
 import scala.concurrent.Future
 import scala.language.postfixOps
 import scala.util.Try
-
 import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
 import akka.http.scaladsl.model.StatusCodes.RequestEntityTooLarge
+import akka.http.scaladsl.model.headers.RawHeader
 import akka.http.scaladsl.server.Directive0
 import akka.http.scaladsl.server.Directives
 import akka.http.scaladsl.server.RequestContext
@@ -65,6 +65,15 @@ protected[controller] trait ValidateRequestSize extends Directives {
   protected val fieldDescriptionForSizeError = "Request"
 }
 
+protected trait CustomHeaders extends Directives {
+  val ActivationIdHeader = "x-openwhisk-activation-id"
+
+  /** Add activation ID in headers */
+  protected def respondWithActivationIdHeader(activationId: ActivationId): Directive0 = {
+    respondWithHeader(RawHeader(ActivationIdHeader, activationId.asString))
+  }
+}
+
 /** A trait implementing the basic operations on WhiskEntities in support of the various APIs. */
 trait WhiskCollectionAPI
     extends Directives
@@ -72,8 +81,8 @@ trait WhiskCollectionAPI
     with AuthorizedRouteProvider
     with ValidateRequestSize
     with ReadOps
-    with WriteOps {
-
+    with WriteOps
+    with CustomHeaders {
   /** The core collections require backend services to be injected in this trait. */
   services: WhiskServices =>
 
diff --git a/core/controller/src/main/scala/whisk/core/controller/Triggers.scala b/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
index a4d2ecf..d3192a2 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
@@ -169,7 +169,9 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
                 .map { activation =>
                   activationStore.store(activation, context)
                 }
-              complete(Accepted, triggerActivationId.toJsObject)
+              respondWithActivationIdHeader(triggerActivationId) {
+                complete(Accepted, triggerActivationId.toJsObject)
+              }
             } else {
               logging
                 .debug(
diff --git a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
index b6d39b5..6f52657 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -353,7 +353,7 @@ protected[core] object WhiskWebActionsApi extends Directives {
     headers.filter(_.lowercaseName != `Content-Type`.lowercaseName)
 }
 
-trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostActionActivation {
+trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostActionActivation with CustomHeaders {
   services: WhiskServices =>
 
   /** API path invocation path for posting activations directly through the host. */
@@ -643,37 +643,41 @@ trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostAc
                               responseType: MediaExtension)(implicit transid: TransactionId) = {
     onComplete(queuedActivation) {
       case Success(Right(activation)) =>
-        val result = activation.resultAsJson
+        respondWithActivationIdHeader(activation.activationId) {
+          val result = activation.resultAsJson
+
+          if (activation.response.isSuccess || activation.response.isApplicationError) {
+            val resultPath = if (activation.response.isSuccess) {
+              projectResultField
+            } else {
+              // the activation produced an error response: therefore ignore
+              // the requested projection and unwrap the error instead
+              // and attempt to handle it per the desired response type (extension)
+              List(ActivationResponse.ERROR_FIELD)
+            }
 
-        if (activation.response.isSuccess || activation.response.isApplicationError) {
-          val resultPath = if (activation.response.isSuccess) {
-            projectResultField
+            val result = getFieldPath(activation.resultAsJson, resultPath)
+            result match {
+              case Some(projection) =>
+                val marshaler = Future(responseType.transcoder(projection, transid, webApiDirectives))
+                onComplete(marshaler) {
+                  case Success(done) => done // all transcoders terminate the connection
+                  case Failure(t)    => terminate(InternalServerError)
+                }
+              case _ => terminate(NotFound, Messages.propertyNotFound)
+            }
           } else {
-            // the activation produced an error response: therefore ignore
-            // the requested projection and unwrap the error instead
-            // and attempt to handle it per the desired response type (extension)
-            List(ActivationResponse.ERROR_FIELD)
-          }
-
-          val result = getFieldPath(activation.resultAsJson, resultPath)
-          result match {
-            case Some(projection) =>
-              val marshaler = Future(responseType.transcoder(projection, transid, webApiDirectives))
-              onComplete(marshaler) {
-                case Success(done) => done // all transcoders terminate the connection
-                case Failure(t)    => terminate(InternalServerError)
-              }
-            case _ => terminate(NotFound, Messages.propertyNotFound)
+            terminate(BadRequest, Messages.errorProcessingRequest)
           }
-        } else {
-          terminate(BadRequest, Messages.errorProcessingRequest)
         }
 
       case Success(Left(activationId)) =>
         // blocking invoke which got queued instead
         // this should not happen, instead it should be a blocking invoke timeout
         logging.debug(this, "activation waiting period expired")
-        terminate(Accepted, Messages.responseNotReady)
+        respondWithActivationIdHeader(activationId) {
+          terminate(Accepted, Messages.responseNotReady)
+        }
 
       case Failure(t: RejectRequest) => terminate(t.code, t.message)
 
diff --git a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
index fb6f4f6..4b14019 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
@@ -39,6 +39,7 @@ import whisk.core.database.UserContext
 import java.io.ByteArrayInputStream
 import java.util.Base64
 
+import akka.http.scaladsl.model.headers.RawHeader
 import akka.stream.scaladsl._
 
 /**
@@ -1211,6 +1212,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
       status should be(Accepted)
       val response = responseAs[JsObject]
       response.fields("activationId") should not be None
+      headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
     }
 
     // it should "ignore &result when invoking nonblocking action"
@@ -1218,6 +1220,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
       status should be(Accepted)
       val response = responseAs[JsObject]
       response.fields("activationId") should not be None
+      headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
     }
   }
 
@@ -1229,6 +1232,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
       status should be(Accepted)
       val response = responseAs[JsObject]
       response.fields("activationId") should not be None
+      headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
     }
   }
 
@@ -1263,6 +1267,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
       status should be(Accepted)
       val response = responseAs[JsObject]
       response.fields("activationId") should not be None
+      headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
     }
   }
 
@@ -1294,6 +1299,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
         status should be(OK)
         val response = responseAs[JsObject]
         response should be(activation.resultAsJson)
+        headers should contain(RawHeader(ActivationIdHeader, activation.activationId.asString))
       }
     } finally {
       deleteActivation(ActivationId(activation.docid.asString), context)
@@ -1363,7 +1369,10 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
 
       // will not wait long enough should get accepted status
       Post(s"$collectionPath/${action.name}?blocking=true&timeout=100") ~> Route.seal(routes(creds)) ~> check {
+        val response = responseAs[JsObject]
         status shouldBe Accepted
+        headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
+
       }
 
       // repeat this time wait longer than active ack delay
@@ -1371,6 +1380,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
         status shouldBe OK
         val response = responseAs[JsObject]
         response shouldBe activation.withoutLogs.toExtendedJson
+        headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
       }
     } finally {
       loadBalancer.whiskActivationStub = None
@@ -1398,6 +1408,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
         status should be(InternalServerError)
         val response = responseAs[JsObject]
         response should be(activation.withoutLogs.toExtendedJson)
+        headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
       }
     } finally {
       deleteActivation(ActivationId(activation.docid.asString), context)
@@ -1412,6 +1423,7 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
       status should be(Accepted)
       val response = responseAs[JsObject]
       response.fields("activationId") should not be None
+      headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
     }
     stream.toString should include(s"[WhiskActionMetaData] [GET] serving from datastore: ${CacheKey(action)}")
     stream.reset()
diff --git a/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala b/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala
index 925c47f..7455f48 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ControllerTestCommon.scala
@@ -33,8 +33,7 @@ import whisk.common.TransactionId
 import whisk.core.WhiskConfig
 import whisk.core.connector.ActivationMessage
 import whisk.core.containerpool.logging.LogStoreProvider
-import whisk.core.controller.RestApiCommons
-import whisk.core.controller.WhiskServices
+import whisk.core.controller.{CustomHeaders, RestApiCommons, WhiskServices}
 import whisk.core.database.{ActivationStoreProvider, CacheChangeNotification, DocumentFactory}
 import whisk.core.database.test.DbUtils
 import whisk.core.entitlement._
@@ -53,7 +52,8 @@ protected trait ControllerTestCommon
     with DbUtils
     with ExecHelpers
     with WhiskServices
-    with StreamLogging {
+    with StreamLogging
+    with CustomHeaders {
 
   val activeAckTopicIndex = ControllerInstanceId("0")
 
diff --git a/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
index 415d224..4165b2e 100644
--- a/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
@@ -28,7 +28,7 @@ import org.scalatest.junit.JUnitRunner
 import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
 import akka.http.scaladsl.server.Route
 import akka.http.scaladsl.model.StatusCodes._
-
+import akka.http.scaladsl.model.headers.RawHeader
 import spray.json._
 import spray.json.DefaultJsonProtocol._
 
@@ -369,6 +369,7 @@ class TriggersApiTests extends ControllerTestCommon with WhiskTriggersApi {
       val JsString(id) = response.fields("activationId")
       val activationId = ActivationId.parse(id).get
       response.fields("activationId") should not be None
+      headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
 
       val activationDoc = DocId(WhiskEntity.qualifiedName(namespace, activationId))
       whisk.utils.retry({
@@ -399,6 +400,7 @@ class TriggersApiTests extends ControllerTestCommon with WhiskTriggersApi {
         println(s"trying to delete async activation doc: '${activationDoc}'")
         deleteActivation(ActivationId(activationDoc.asString), context)
         response.fields("activationId") should not be None
+        headers should contain(RawHeader(ActivationIdHeader, response.fields("activationId").convertTo[String]))
       }, 30, Some(1.second))
     }
   }
diff --git a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
index c3b3e1c..a0acf18 100644
--- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
@@ -793,7 +793,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
         status should be(NoContent)
         response.entity shouldBe HttpEntity.Empty
         withClue(headers) {
-          headers.length shouldBe 0
+          headers.length shouldBe 1
+          headers.exists(_.is(ActivationIdHeader)) should be(true)
         }
       }
 
@@ -922,7 +923,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
 
             m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
               status should be(statusCode)
-              headers shouldBe empty
+              headers.size shouldBe 1
+              headers.exists(_.is(ActivationIdHeader)) should be(true)
               response.entity shouldBe HttpEntity.Empty
             }
           }
@@ -939,7 +941,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
 
             m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
               status should be(statusCode)
-              headers shouldBe List(RawHeader("Set-Cookie", "a=b"))
+              headers should contain(RawHeader("Set-Cookie", "a=b"))
+              headers.exists(_.is(ActivationIdHeader)) should be(true)
               response.entity shouldBe HttpEntity.Empty
             }
           }
@@ -959,7 +962,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
 
             m(s"$testRoutePath/$path") ~> addHeader("Accept", "application/json") ~> Route.seal(routes(creds)) ~> check {
               status should be(statusCode)
-              headers shouldBe empty
+              headers.size shouldBe 1
+              headers.exists(_.is(ActivationIdHeader)) should be(true)
               response.entity shouldBe HttpEntity.Empty
             }
           }
@@ -976,7 +980,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
 
             m(s"$testRoutePath/$path") ~> addHeader("Accept", "application/json") ~> Route.seal(routes(creds)) ~> check {
               status should be(statusCode)
-              headers shouldBe List(RawHeader("Set-Cookie", "a=b"))
+              headers should contain(RawHeader("Set-Cookie", "a=b"))
+              headers.exists(_.is(ActivationIdHeader)) should be(true)
               response.entity shouldBe HttpEntity.Empty
             }
           }
@@ -1165,7 +1170,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
           m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
             status should be(OK)
             mediaType shouldBe MediaTypes.`application/json`
-            headers shouldBe empty
+            headers.size shouldBe 1
+            headers.exists(_.is(ActivationIdHeader)) should be(true)
             responseAs[String] shouldBe "hello world"
           }
         }