You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/08/22 18:42:28 UTC
[GitHub] rabbah closed pull request #3671: Activation id in header
rabbah closed pull request #3671: Activation id in header
URL: https://github.com/apache/incubator-openwhisk/pull/3671
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 12c8ec6ff5..bd92c581eb 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 cd9a72a08d..b1f34b3dc0 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 a4d2ecfa59..d3192a2aeb 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 b6d39b5f58..6f526575f2 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 fb6f4f6624..4b1401933b 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 925c47f6aa..7455f48b8c 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 415d224985..4165b2e4e4 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 c3b3e1c5bb..a0acf185c7 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"
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services