rabbah commented on a change in pull request #3187: reduce rule activation
records
URL:
https://github.com/apache/incubator-openwhisk/pull/3187#discussion_r165063674
##########
File path: core/controller/src/main/scala/whisk/core/controller/Triggers.scala
##########
@@ -390,46 +300,130 @@ trait WhiskTriggersApi extends WhiskCollectionAPI {
}
/**
- * Create JSON object containing the pertinent rule activation details
+ * Iterates through each active rule and invoke each mapped action.
+ */
+ private def activateRules(user: Identity,
+ args: JsObject,
+ rulesToActivate: Map[FullyQualifiedEntityName,
ReducedRule])(
+ implicit transid: TransactionId): Iterable[Future[JsObject]] = {
+ rulesToActivate.map {
+ case (ruleName, rule) =>
+ // Invoke the action. Retain action results for inclusion in the
trigger activation record
+ val actionActivationResult: Future[JsObject] = postActivation(user,
rule, args)
+ .flatMap { response =>
+ response.status match {
+ case OK | Accepted =>
+ Unmarshal(response.entity).to[JsObject].map {
activationResponse =>
+ val activationId: JsValue =
activationResponse.fields("activationId")
+ logging.debug(this, s"trigger-fired action '${rule.action}'
invoked with activation $activationId")
+ ruleResult(ActivationResponse.Success, ruleName,
rule.action, Some(activationId))
+ }
+
+ // all proper controller responses are JSON objects that
deserialize to an ErrorResponse instance
+ case code if (response.entity.contentType ==
ContentTypes.`application/json`) =>
+ Unmarshal(response.entity).to[ErrorResponse].map { e =>
+ val statusCode =
+ if (code != InternalServerError) {
+ logging
+ .debug(
+ this,
+ s"trigger-fired action '${rule.action}' failed to
invoke with ${e.error}, ${e.code}")
+ ActivationResponse.ApplicationError
+ } else {
+ logging
+ .error(
+ this,
+ s"trigger-fired action '${rule.action}' failed to
invoke with ${e.error}, ${e.code}")
+ ActivationResponse.WhiskError
+ }
+ ruleResult(statusCode, ruleName, rule.action, errorMsg =
Some(e.error))
+ }
+
+ case code =>
+ logging.error(this, s"trigger-fired action '${rule.action}'
failed to invoke with status code $code")
+ Unmarshal(response.entity).to[String].map { error =>
+ ruleResult(ActivationResponse.WhiskError, ruleName,
rule.action, errorMsg = Some(error))
+ }
+ }
+ }
+ .recover {
+ case t =>
+ logging.error(this, s"trigger-fired action '${rule.action}'
failed to invoke with $t")
+ ruleResult(
+ ActivationResponse.WhiskError,
+ ruleName,
+ rule.action,
+ errorMsg = Some(InternalServerError.defaultMessage))
+ }
+
+ actionActivationResult
+ }
+ }
+
+ /**
+ * Posts an action activation. Currently done by posting internally to the
controller.
+ * TODO: use a poper path that does not route through HTTP.
*
- * @param statusCode
- * @param ruleName
- * @param actionName
- * @param actionActivationId
- * @param msg
- * @return
+ * @param rule the name of the rule that is activated
+ * @param args the arguments to post to the action
+ * @return a future with the HTTP response from the action activation
+ */
+ private def postActivation(user: Identity, rule: ReducedRule, args:
JsObject): Future[HttpResponse] = {
+ // Build the url to invoke an action mapped to the rule
+ val actionUrl = baseControllerPath / rule.action.path.root.asString /
"actions"
+
+ val actionPath = {
+ rule.action.path.relativePath.map { pkg =>
+ (Path.SingleSlash + pkg.namespace) / rule.action.name.asString
+ } getOrElse {
+ Path.SingleSlash + rule.action.name.asString
+ }
+ }.toString
+
+ val request = HttpRequest(
+ method = POST,
+ uri = url.withPath(actionUrl + actionPath),
+ headers =
List(Authorization(BasicHttpCredentials(user.authkey.uuid.asString,
user.authkey.key.asString))),
+ entity = HttpEntity(MediaTypes.`application/json`, args.compactPrint))
+
+ Http().singleRequest(request)
+ }
+
+ /**
+ * Create JSON object containing the pertinent rule activation details.
+ * {
+ * "rule": "my-rule",
+ * "action": "my-action",
+ * "statusCode": 0,
+ * "status": "success",
+ * "activationId": "...", // either this
field, ...
+ * "error": "The requested resource does not exist." // ... or this
field will be present
+ * }
+ *
+ * @param statusCode one of ActivationResponse values
+ * @param ruleName the name of the rule that was activated
+ * @param actionName the name of the action activated by the rule
+ * @param actionActivationId the activation id, if there is one
+ * @param errorMsg the rror messages otherwise
+ * @return JsObject as formatted above
*/
private def ruleResult(statusCode: Int,
- ruleName: String,
- actionName: String,
- actionActivationId: Option[String] = None,
+ ruleName: FullyQualifiedEntityName,
+ actionName: FullyQualifiedEntityName,
+ actionActivationId: Option[JsValue] = None,
errorMsg: Option[String] = None): JsObject = {
- val objMap: Map[String, JsValue] = Map(
- "rule" -> JsString(ruleName),
- "action" -> JsString(actionName),
+ JsObject(
+ "rule" -> JsString(ruleName.asString),
+ "action" -> JsString(actionName.asString),
"statusCode" -> JsNumber(statusCode),
- "success" -> JsBoolean(statusCode == ActivationResponse.Success)) ++ {
- actionActivationId map { id =>
- Seq("activationId" -> JsString(id))
- } getOrElse Seq()
- } ++ {
- errorMsg map { err =>
- Seq("error" -> JsString(err))
- } getOrElse Seq()
- }
-
- // Final rule result looks like
- // {
- // "rule": "my-rule",
- // "action": "my-action",
- // "statusCode": 0,
- // "status": "success",
- // "activationId": "90c84e7b33c84ceb884e7b33c8ecebf6", // Optional
- // "error": "The requested resource does not exist." // Optional
- // }
- JsObject(objMap)
+ "success" -> JsBoolean(statusCode == ActivationResponse.Success),
+ "activationId" -> actionActivationId.toJson,
Review comment:
note: the unit tests (or integration tests) are not tight enough and don't
enforce a schema on the response; the activationId and error properties here
will emit as `null` if not present which changes the behavior relative to the
previous commit.
I suggest a unit test which checks the expected schema exactly; this can be
done for the intergration test as well; parse the array to json and check for
equality.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
