You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by cs...@apache.org on 2018/02/24 05:13:35 UTC
[incubator-openwhisk] branch master updated: Make parameters with
defined values final (#3333)
This is an automated email from the ASF dual-hosted git repository.
csantanapr 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 1bb0280 Make parameters with defined values final (#3333)
1bb0280 is described below
commit 1bb02808b7a37554e797da06da70bda713e39fb0
Author: rodric rabbah <ro...@gmail.com>
AuthorDate: Sat Feb 24 00:13:33 2018 -0500
Make parameters with defined values final (#3333)
---
.../main/scala/whisk/core/entity/Parameter.scala | 15 ++--
.../main/scala/whisk/core/controller/Actions.scala | 84 +++++++++++++---------
.../scala/whisk/core/controller/WebActions.scala | 7 +-
.../core/controller/test/ActionsApiTests.scala | 17 +++++
4 files changed, 78 insertions(+), 45 deletions(-)
diff --git a/common/scala/src/main/scala/whisk/core/entity/Parameter.scala b/common/scala/src/main/scala/whisk/core/entity/Parameter.scala
index 0dd6518..18ddb6e 100644
--- a/common/scala/src/main/scala/whisk/core/entity/Parameter.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/Parameter.scala
@@ -41,10 +41,7 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
*/
def size = {
params
- .map {
- case (name, value) =>
- name.size + value.size
- }
+ .map { case (name, value) => name.size + value.size }
.foldLeft(0 B)(_ + _)
}
@@ -77,14 +74,14 @@ protected[core] class Parameters protected[entity] (private val params: Map[Para
params.keySet filter (params(_).isDefined) map (_.name)
}
- protected[core] def toJsArray =
+ protected[core] def toJsArray = {
JsArray(params map { p =>
JsObject("key" -> p._1.name.toJson, "value" -> p._2.value.toJson)
} toSeq: _*)
- protected[core] def toJsObject =
- JsObject(params map { p =>
- (p._1.name -> p._2.value.toJson)
- })
+ }
+
+ protected[core] def toJsObject = JsObject(params.map(p => (p._1.name -> p._2.value.toJson)))
+
override def toString = toJsArray.compactPrint
/**
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 1e26a66..1fb47c0 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Actions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Actions.scala
@@ -226,38 +226,17 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
onComplete(entitleReferencedEntitiesMetaData(user, Privilege.ACTIVATE, Some(action.exec))) {
case Success(_) =>
val actionWithMergedParams = env.map(action.inherit(_)) getOrElse action
- val waitForResponse = if (blocking) Some(waitOverride) else None
- 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)
- 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)
- }
- case Failure(t: RecordTooLargeException) =>
- logging.debug(this, s"[POST] action payload was too large")
- terminate(RequestEntityTooLarge)
- case Failure(RejectRequest(code, message)) =>
- logging.debug(this, s"[POST] action rejected with code $code: $message")
- terminate(code, message)
- case Failure(t: Throwable) =>
- logging.error(this, s"[POST] action activation failed: ${t.getMessage}")
- terminate(InternalServerError)
+
+ // incoming parameters may not override final parameters (i.e., parameters with already defined values)
+ // on an action once its parameters are resolved across package and binding
+ val allowInvoke = payload
+ .map(_.fields.keySet.forall(key => !actionWithMergedParams.immutableParameters.contains(key)))
+ .getOrElse(true)
+
+ if (allowInvoke) {
+ doInvoke(user, actionWithMergedParams, payload, blocking, waitOverride, result)
+ } else {
+ terminate(BadRequest, Messages.parametersNotAllowed)
}
case Failure(f) =>
@@ -268,6 +247,47 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
}
}
+ private def doInvoke(user: Identity,
+ actionWithMergedParams: WhiskActionMetaData,
+ payload: Option[JsObject],
+ blocking: Boolean,
+ waitOverride: FiniteDuration,
+ result: Boolean)(implicit transid: TransactionId): RequestContext => Future[RouteResult] = {
+ val waitForResponse = if (blocking) Some(waitOverride) else None
+ 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)
+ 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)
+ }
+ case Failure(t: RecordTooLargeException) =>
+ logging.debug(this, s"[POST] action payload was too large")
+ terminate(RequestEntityTooLarge)
+ case Failure(RejectRequest(code, message)) =>
+ logging.debug(this, s"[POST] action rejected with code $code: $message")
+ terminate(code, message)
+ case Failure(t: Throwable) =>
+ logging.error(this, s"[POST] action activation failed: ${t.getMessage}")
+ terminate(InternalServerError)
+ }
+ }
+
/**
* Deletes action.
*
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 0bb6b77..d81884f 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -91,7 +91,7 @@ private case class Context(propertyMap: WebApiDirectives,
// returns true iff the attached query and body parameters contain a property
// that conflicts with the given reserved parameters
- def overrides(reservedParams: Set[String]): Set[String] = {
+ def overrides(reservedParams: Set[String]): Boolean = {
val queryParams = queryAsMap.keySet
val bodyParams = body
.map {
@@ -100,7 +100,7 @@ private case class Context(propertyMap: WebApiDirectives,
}
.getOrElse(Set.empty)
- (queryParams ++ bodyParams) intersect reservedParams
+ (queryParams ++ bodyParams).forall(key => !reservedParams.contains(key))
}
// attach the body to the Context
@@ -612,8 +612,7 @@ trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostAc
// since these are system properties, the action should not define them, and if it does,
// they will be overwritten
if (isRawHttpAction || context
- .overrides(webApiDirectives.reservedProperties ++ action.immutableParameters)
- .isEmpty) {
+ .overrides(webApiDirectives.reservedProperties ++ action.immutableParameters)) {
val content = context.toActionArgument(onBehalfOf, isRawHttpAction)
invokeAction(actionOwnerIdentity, action, Some(JsObject(content)), maxWaitForWebActionResult, cause = None)
} else {
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 fb00184..380c5f0 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
@@ -832,6 +832,23 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
}
}
+ it should "not invoke an action when final parameters are redefined" in {
+ implicit val tid = transid()
+ val annotations = Parameters(WhiskActionMetaData.finalParamsAnnotationName, JsBoolean(true))
+ val parameters = Parameters("a", "A") ++ Parameters("empty", JsNull)
+ val action = WhiskAction(namespace, aname(), jsDefault("??"), parameters = parameters, annotations = annotations)
+ put(entityStore, action)
+ Seq((Parameters("a", "B"), BadRequest), (Parameters("empty", "C"), Accepted)).foreach {
+ case (p, code) =>
+ Post(s"$collectionPath/${action.name}", p.toJsObject) ~> Route.seal(routes(creds)) ~> check {
+ status should be(code)
+ if (code == BadRequest) {
+ responseAs[ErrorResponse].error shouldBe Messages.parametersNotAllowed
+ }
+ }
+ }
+ }
+
it should "invoke an action, blocking with default timeout" in {
implicit val tid = transid()
val action = WhiskAction(
--
To stop receiving notification emails like this one, please contact
csantanapr@apache.org.