openwhisk-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rab...@apache.org
Subject [incubator-openwhisk] branch master updated: Handle trigger activations with inactive rules (#3262)
Date Tue, 20 Feb 2018 15:36:59 GMT
This is an automated email from the ASF dual-hosted git repository.

rabbah 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 b20778f  Handle trigger activations with inactive rules (#3262)
b20778f is described below

commit b20778f37e052893818c3c39f0307783d7e5024a
Author: Mark Deuser <mdeuser@us.ibm.com>
AuthorDate: Tue Feb 20 10:36:55 2018 -0500

    Handle trigger activations with inactive rules (#3262)
    
    If trigger has no associated active rules; return a 204.  Also no trigger activation record
is created. Updated swagger to reflect new 204 response from POST trigger.
    
    When a trigger activation is created, log all inactive rules as well.
---
 .../src/main/scala/whisk/http/ErrorResponse.scala  |  5 ++
 .../src/main/resources/apiv1swagger.json           |  6 +++
 .../scala/whisk/core/controller/Triggers.scala     | 26 ++++++---
 .../test/scala/system/basic/WskBasicTests.scala    | 62 ++++++++++++++++++++++
 .../core/controller/test/TriggersApiTests.scala    |  9 ++++
 5 files changed, 102 insertions(+), 6 deletions(-)

diff --git a/common/scala/src/main/scala/whisk/http/ErrorResponse.scala b/common/scala/src/main/scala/whisk/http/ErrorResponse.scala
index f161618..e599a1f 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 efb553a..81636a5 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 873dcfd..8302d74 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 4c8e197..1fd93fd 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 59e9e7e..fdfcf4f 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()

-- 
To stop receiving notification emails like this one, please contact
rabbah@apache.org.

Mime
View raw message