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/15 15:25:22 UTC

[GitHub] rabbah closed pull request #3254: Preserve rule status when rule is updated

rabbah closed pull request #3254: Preserve rule status when rule is updated
URL: https://github.com/apache/incubator-openwhisk/pull/3254
 
 
   

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/core/controller/src/main/scala/whisk/core/controller/Rules.scala b/core/controller/src/main/scala/whisk/core/controller/Rules.scala
index 3031222c7b..0e886165b1 100644
--- a/core/controller/src/main/scala/whisk/core/controller/Rules.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/Rules.scala
@@ -85,11 +85,31 @@ trait WhiskRulesApi extends WhiskCollectionAPI with ReferencedEntities {
         val request = content.resolve(entityName.namespace)
         onComplete(entitlementProvider.check(user, Privilege.READ, referencedEntities(request))) {
           case Success(_) =>
-            putEntity(WhiskRule, entityStore, entityName.toDocId, overwrite, update(request) _, () => {
-              create(request, entityName)
-            }, postProcess = Some { rule: WhiskRule =>
-              completeAsRuleResponse(rule, Status.ACTIVE)
-            })
+            putEntity(
+              WhiskRule,
+              entityStore,
+              entityName.toDocId,
+              overwrite,
+              update(request) _,
+              () => {
+                create(request, entityName)
+              },
+              postProcess = Some { rule: WhiskRule =>
+                if (overwrite == true) {
+                  val getRuleWithStatus = getTrigger(rule.trigger) map { trigger =>
+                    getStatus(trigger, FullyQualifiedEntityName(rule.namespace, rule.name))
+                  } map { status =>
+                    rule.withStatus(status)
+                  }
+
+                  onComplete(getRuleWithStatus) {
+                    case Success(r) => completeAsRuleResponse(rule, r.status)
+                    case Failure(t) => terminate(InternalServerError)
+                  }
+                } else {
+                  completeAsRuleResponse(rule, Status.ACTIVE)
+                }
+              })
           case Failure(f) =>
             handleEntitlementFailure(f)
         }
@@ -286,6 +306,7 @@ trait WhiskRulesApi extends WhiskCollectionAPI with ReferencedEntities {
     val oldTriggerName = rule.trigger
 
     getTrigger(oldTriggerName) flatMap { oldTriggerOpt =>
+      val status = getStatus(oldTriggerOpt, ruleName)
       val newTriggerEntity = content.trigger getOrElse rule.trigger
       val newTriggerName = newTriggerEntity
 
@@ -313,7 +334,7 @@ trait WhiskRulesApi extends WhiskCollectionAPI with ReferencedEntities {
             WhiskTrigger.put(entityStore, oldTrigger.removeRule(ruleName))
           }
 
-          val triggerLink = ReducedRule(actionName, Status.INACTIVE)
+          val triggerLink = ReducedRule(actionName, status)
           val update = WhiskTrigger.put(entityStore, newTrigger.addRule(ruleName, triggerLink))
           Future.sequence(Seq(deleteOldLink.getOrElse(Future.successful(true)), update)).map(_ => r)
       }
diff --git a/tests/src/test/scala/system/basic/WskRuleTests.scala b/tests/src/test/scala/system/basic/WskRuleTests.scala
index bcb9dea083..62890b7701 100644
--- a/tests/src/test/scala/system/basic/WskRuleTests.scala
+++ b/tests/src/test/scala/system/basic/WskRuleTests.scala
@@ -88,6 +88,35 @@ abstract class WskRuleTests extends TestHelpers with WskTestHelpers {
 
   behavior of "Whisk rules"
 
+  it should "preserve rule status when a rule is updated" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
+    val ruleName = withTimestamp("r1to1")
+    val triggerName = withTimestamp("t1to1")
+    val actionName = withTimestamp("a1 to 1")
+    val triggerName2 = withTimestamp("t2to1")
+    val active = Some("active".toJson)
+    val inactive = Some("inactive".toJson)
+    val statusPermutations =
+      Seq((triggerName, active), (triggerName, inactive), (triggerName2, active), (triggerName2, inactive))
+
+    ruleSetup(Seq((ruleName, triggerName, (actionName, actionName, defaultAction))), assetHelper)
+    assetHelper.withCleaner(wsk.trigger, triggerName2) { (trigger, name) =>
+      trigger.create(name)
+    }
+
+    statusPermutations.foreach {
+      case (trigger, status) =>
+        if (status == active) wsk.rule.enable(ruleName) else wsk.rule.disable(ruleName)
+        wsk.rule
+          .create(ruleName, trigger, actionName, update = true)
+          .stdout
+          .parseJson
+          .asJsObject
+          .fields
+          .get("status") shouldBe (status)
+        wsk.rule.get(ruleName).stdout.parseJson.asJsObject.fields.get("status") shouldBe (status)
+    }
+  }
+
   it should "invoke the action attached on trigger fire, creating an activation for each entity including the cause" in withAssetCleaner(
     wskprops) { (wp, assetHelper) =>
     val ruleName = withTimestamp("r1to1")
diff --git a/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala
index 6430a52884..78cea3c23a 100644
--- a/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/RulesApiTests.scala
@@ -563,7 +563,7 @@ class RulesApiTests extends ControllerTestCommon with WhiskRulesApi {
         WhiskRuleResponse(
           namespace,
           rule.name,
-          Status.ACTIVE,
+          Status.INACTIVE,
           trigger.fullyQualifiedName(false),
           action.fullyQualifiedName(false),
           version = SemVer().upPatch))
@@ -594,7 +594,7 @@ class RulesApiTests extends ControllerTestCommon with WhiskRulesApi {
         WhiskRuleResponse(
           namespace,
           rule.name,
-          Status.ACTIVE,
+          Status.INACTIVE,
           trigger.fullyQualifiedName(false),
           action.fullyQualifiedName(false),
           version = SemVer().upPatch))
@@ -625,7 +625,7 @@ class RulesApiTests extends ControllerTestCommon with WhiskRulesApi {
         WhiskRuleResponse(
           namespace,
           rule.name,
-          Status.ACTIVE,
+          Status.INACTIVE,
           trigger.fullyQualifiedName(false),
           action.fullyQualifiedName(false),
           version = SemVer().upPatch))
@@ -656,7 +656,7 @@ class RulesApiTests extends ControllerTestCommon with WhiskRulesApi {
         WhiskRuleResponse(
           namespace,
           rule.name,
-          Status.ACTIVE,
+          Status.INACTIVE,
           trigger.fullyQualifiedName(false),
           action.fullyQualifiedName(false),
           version = SemVer().upPatch))
@@ -684,7 +684,7 @@ class RulesApiTests extends ControllerTestCommon with WhiskRulesApi {
         WhiskRuleResponse(
           namespace,
           rule.name,
-          Status.ACTIVE,
+          Status.INACTIVE,
           trigger.fullyQualifiedName(false),
           action.fullyQualifiedName(false),
           version = SemVer().upPatch))


 

----------------------------------------------------------------
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