csantanapr closed pull request #3333: Make parameters with defined values final 
URL: https://github.com/apache/incubator-openwhisk/pull/3333
 
 
   

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/common/scala/src/main/scala/whisk/core/entity/Parameter.scala 
b/common/scala/src/main/scala/whisk/core/entity/Parameter.scala
index 0dd6518584..18ddb6ea7d 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 1e26a6684c..1fb47c02ff 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 cd3c68ca37..a692530c89 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 fb001847ea..380c5f0b76 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(


 

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to