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 <rod...@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 csantan...@apache.org.