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/02/20 15:37:00 UTC
[GitHub] rabbah closed pull request #3262: Handle trigger activations with inactive rules
rabbah closed pull request #3262: Handle trigger activations with inactive rules
URL: https://github.com/apache/incubator-openwhisk/pull/3262
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/http/ErrorResponse.scala b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
index f161618fe1..e599a1f5fb 100644
--- a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
+++ b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
@@ -111,6 +111,11 @@ object Messages {
val notAllowedOnBinding = "Operation not permitted on package binding."
def packageNameIsReserved(name: String) = s"Package name '$name' is reserved."
+ /** Error messages for triggers */
+ def triggerWithInactiveRule(rule: String, action: String) = {
+ s"Rule '$rule' is inactive, action '$action' was not activated."
+ }
+
/** Error messages for sequence activations. */
def sequenceRetrieveActivationTimeout(id: ActivationId) =
s"Timeout reached when retrieving activation $id for sequence component."
diff --git a/core/controller/src/main/resources/apiv1swagger.json b/core/controller/src/main/resources/apiv1swagger.json
index efb553a629..81636a559f 100644
--- a/core/controller/src/main/resources/apiv1swagger.json
+++ b/core/controller/src/main/resources/apiv1swagger.json
@@ -940,6 +940,9 @@
"$ref": "#/definitions/ItemId"
}
},
+ "204": {
+ "$ref": "#/responses/NoActiveRules"
+ },
"401": {
"$ref": "#/responses/UnauthorizedRequest"
},
@@ -2109,6 +2112,9 @@
"schema": {
"$ref": "#/definitions/ErrorMessage"
}
+ },
+ "NoActiveRules": {
+ "description": "Trigger has no active rules"
}
}
}
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 873dcfdab3..8302d74db6 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Triggers.scala
@@ -26,7 +26,7 @@ import akka.actor.ActorSystem
import akka.http.scaladsl.Http
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
import akka.http.scaladsl.model.HttpMethods.POST
-import akka.http.scaladsl.model.StatusCodes.{Accepted, BadRequest, InternalServerError, OK, ServerError}
+import akka.http.scaladsl.model.StatusCodes.{Accepted, BadRequest, InternalServerError, NoContent, OK, ServerError}
import akka.http.scaladsl.model.Uri.Path
import akka.http.scaladsl.model.headers.{Authorization, BasicHttpCredentials}
import akka.http.scaladsl.model._
@@ -42,6 +42,7 @@ import whisk.core.entitlement.Collection
import whisk.core.entity._
import whisk.core.entity.types.{ActivationStore, EntityStore}
import whisk.http.ErrorResponse
+import whisk.http.Messages
/** A trait implementing the triggers API. */
trait WhiskTriggersApi extends WhiskCollectionAPI {
@@ -129,7 +130,7 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
if (activeRules.nonEmpty) {
val args: JsObject = trigger.parameters.merge(payload).getOrElse(JsObject())
- activateRules(user, args, activeRules)
+ activateRules(user, args, trigger.rules.getOrElse(Map.empty))
.map(results => triggerActivation.withLogs(ActivationLogs(results.map(_.toJson.compactPrint).toVector)))
.recover {
case e =>
@@ -146,9 +147,14 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
case Failure(t) =>
logging.error(this, s"[POST] storing trigger activation $triggerActivationId failed: ${t.getMessage}")
}
+ complete(Accepted, triggerActivationId.toJsObject)
+ } else {
+ logging
+ .debug(
+ this,
+ s"[POST] trigger without an active rule was activated; no trigger activation record created for $entityName")
+ complete(NoContent)
}
-
- complete(Accepted, triggerActivationId.toJsObject)
})
}
}
@@ -287,13 +293,21 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
}
/**
- * Iterates through each active rule and invoke each mapped action.
+ * Iterates through each rule and invoking each active rule's mapped action.
*/
private def activateRules(user: Identity,
args: JsObject,
rulesToActivate: Map[FullyQualifiedEntityName, ReducedRule])(
implicit transid: TransactionId): Future[Iterable[RuleActivationResult]] = {
val ruleResults = rulesToActivate.map {
+ case (ruleName, rule) if (rule.status != Status.ACTIVE) =>
+ Future.successful {
+ RuleActivationResult(
+ ActivationResponse.ApplicationError,
+ ruleName,
+ rule.action,
+ Left(Messages.triggerWithInactiveRule(ruleName.asString, rule.action.asString)))
+ }
case (ruleName, rule) =>
// Invoke the action. Retain action results for inclusion in the trigger activation record
postActivation(user, rule, args)
@@ -346,7 +360,7 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
/**
* Posts an action activation. Currently done by posting internally to the controller.
- * TODO: use a poper path that does not route through HTTP.
+ * TODO: use a proper path that does not route through HTTP.
*
* @param rule the name of the rule that is activated
* @param args the arguments to post to the action
diff --git a/tests/src/test/scala/system/basic/WskBasicTests.scala b/tests/src/test/scala/system/basic/WskBasicTests.scala
index 4c8e1972db..1fd93fd79b 100644
--- a/tests/src/test/scala/system/basic/WskBasicTests.scala
+++ b/tests/src/test/scala/system/basic/WskBasicTests.scala
@@ -39,6 +39,7 @@ import common.WskProps
import common.WskTestHelpers
import spray.json._
import spray.json.DefaultJsonProtocol._
+import whisk.http.Messages
@RunWith(classOf[JUnitRunner])
class WskBasicTests extends TestHelpers with WskTestHelpers {
@@ -754,6 +755,67 @@ class WskBasicTests extends TestHelpers with WskTestHelpers {
}
}
+ it should "create and fire a trigger having an active rule and an inactive rule" in withAssetCleaner(wskprops) {
+ (wp, assetHelper) =>
+ val ruleName1 = withTimestamp("r1toa1")
+ val ruleName2 = withTimestamp("r2toa2")
+ val triggerName = withTimestamp("t1tor1r2")
+ val actionName1 = withTimestamp("a1")
+ val actionName2 = withTimestamp("a2")
+ val ns = wsk.namespace.whois()
+
+ assetHelper.withCleaner(wsk.trigger, triggerName) { (trigger, _) =>
+ trigger.create(triggerName)
+ trigger.create(triggerName, update = true)
+ }
+
+ assetHelper.withCleaner(wsk.action, actionName1) { (action, name) =>
+ action.create(name, defaultAction)
+ }
+ assetHelper.withCleaner(wsk.action, actionName2) { (action, name) =>
+ action.create(name, defaultAction)
+ }
+
+ assetHelper.withCleaner(wsk.rule, ruleName1) { (rule, name) =>
+ rule.create(name, trigger = triggerName, action = actionName1)
+ }
+ assetHelper.withCleaner(wsk.rule, ruleName2) { (rule, name) =>
+ rule.create(name, trigger = triggerName, action = actionName2)
+ rule.disable(ruleName2)
+ }
+
+ val run = wsk.trigger.fire(triggerName)
+ withActivation(wsk.activation, run) { activation =>
+ activation.duration shouldBe 0L // shouldn't exist but CLI generates it
+ activation.end shouldBe Instant.EPOCH // shouldn't exist but CLI generates it
+ activation.logs shouldBe defined
+ activation.logs.get.size shouldBe 2
+
+ val logEntry1 = activation.logs.get(0).parseJson.asJsObject
+ val logEntry2 = activation.logs.get(1).parseJson.asJsObject
+ val logs = JsArray(logEntry1, logEntry2)
+ val ruleActivationId: String = if (logEntry1.getFields("activationId").size == 1) {
+ logEntry1.getFields("activationId")(0).convertTo[String]
+ } else {
+ logEntry2.getFields("activationId")(0).convertTo[String]
+ }
+ val expectedLogs = JsArray(
+ JsObject(
+ "statusCode" -> JsNumber(0),
+ "activationId" -> JsString(ruleActivationId),
+ "success" -> JsBoolean(true),
+ "rule" -> JsString(ns + "/" + ruleName1),
+ "action" -> JsString(ns + "/" + actionName1)),
+ JsObject(
+ "statusCode" -> JsNumber(1),
+ "success" -> JsBoolean(false),
+ "error" -> JsString(Messages.triggerWithInactiveRule(s"$ns/$ruleName2", s"$ns/$actionName2")),
+ "rule" -> JsString(ns + "/" + ruleName2),
+ "action" -> JsString(ns + "/" + actionName2)))
+ logs shouldBe expectedLogs
+ }
+ }
+
behavior of "Wsk Rule REST"
it should "create rule, get rule, update rule and list rule" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
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 59e9e7ea25..fdfcf4f71b 100644
--- a/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/TriggersApiTests.scala
@@ -366,6 +366,15 @@ class TriggersApiTests extends ControllerTestCommon with WhiskTriggersApi {
}
}
+ it should "not fire a trigger without a rule" in {
+ implicit val tid = transid()
+ val trigger = WhiskTrigger(namespace, aname())
+ put(entityStore, trigger)
+ Post(s"$collectionPath/${trigger.name}") ~> Route.seal(routes(creds)) ~> check {
+ status shouldBe NoContent
+ }
+ }
+
//// invalid resource
it should "reject invalid resource" in {
implicit val tid = transid()
----------------------------------------------------------------
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