You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by st...@apache.org on 2020/08/24 01:36:48 UTC

[openwhisk] branch master updated: Do not delete previous annotation and support delete annotation via CLI (#4940)

This is an automated email from the ASF dual-hosted git repository.

style95 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openwhisk.git


The following commit(s) were added to refs/heads/master by this push:
     new 64ccb22  Do not delete previous annotation and support delete annotation via CLI (#4940)
64ccb22 is described below

commit 64ccb22ad022ebfac7bb3cf34a1b5627e8431cc4
Author: ningyougang <41...@qq.com>
AuthorDate: Mon Aug 24 09:36:25 2020 +0800

    Do not delete previous annotation and support delete annotation via CLI (#4940)
    
    * Do not delete previous annotation
    
    Currently, if passing another annotations, original previous annotation
    will be removed and the passed new annotations will be added.
    
    It may give users some confused that why my previous annotation gone.
    So it is better to not delete user's previous annotation when adding new
    annotation, but at the same time, need to provide a feature that
    support to delete annotation by user via ClI, e.g.
    wsk action update hello --del-annotation key1 --del-annotation key2
    
    CLI side needs to support as well
    
    * Add test cases
    
    * Fix some review comments
    
    Co-authored-by: ning.yougang <ni...@navercorp.com>
---
 .../apache/openwhisk/core/entity/WhiskAction.scala |  5 ++--
 .../src/main/resources/apiv1swagger.json           |  7 +++++
 .../apache/openwhisk/core/controller/Actions.scala |  9 +++++-
 tests/src/test/scala/common/WskCliOperations.scala |  5 ++++
 tests/src/test/scala/common/WskOperations.scala    |  1 +
 .../test/scala/common/rest/WskRestOperations.scala |  3 ++
 .../core/cli/test/WskRestBasicUsageTests.scala     | 32 ++++++++++++++++++++
 .../test/scala/system/basic/WskActionTests.scala   | 35 ++++++++++++++++++++++
 8 files changed, 94 insertions(+), 3 deletions(-)

diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
index 98154fe..6dd60a1 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala
@@ -56,7 +56,8 @@ case class WhiskActionPut(exec: Option[Exec] = None,
                           limits: Option[ActionLimitsOption] = None,
                           version: Option[SemVer] = None,
                           publish: Option[Boolean] = None,
-                          annotations: Option[Parameters] = None) {
+                          annotations: Option[Parameters] = None,
+                          delAnnotations: Option[Array[String]] = None) {
 
   protected[core] def replace(exec: Exec) = {
     WhiskActionPut(Some(exec), parameters, limits, version, publish, annotations)
@@ -643,5 +644,5 @@ object ActionLimitsOption extends DefaultJsonProtocol {
 }
 
 object WhiskActionPut extends DefaultJsonProtocol {
-  implicit val serdes = jsonFormat6(WhiskActionPut.apply)
+  implicit val serdes = jsonFormat7(WhiskActionPut.apply)
 }
diff --git a/core/controller/src/main/resources/apiv1swagger.json b/core/controller/src/main/resources/apiv1swagger.json
index 563d645..9befc7c 100644
--- a/core/controller/src/main/resources/apiv1swagger.json
+++ b/core/controller/src/main/resources/apiv1swagger.json
@@ -1910,6 +1910,13 @@
           },
           "description": "annotations on the item"
         },
+        "delAnnotations": {
+          "type": "array",
+          "items": {
+            "type": "string"
+          },
+          "description": "The list of annotations to be deleted from the item"
+        },
         "parameters": {
           "type": "array",
           "items": {
diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala
index 056aadf..4fc5312 100644
--- a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala
+++ b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala
@@ -537,6 +537,13 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
 
     val exec = content.exec getOrElse action.exec
 
+    val newAnnotations = content.delAnnotations
+      .map { annotationArray =>
+        annotationArray.foldRight(action.annotations)((a: String, b: Parameters) => b - a)
+      }
+      .map(_ ++ content.annotations)
+      .getOrElse(action.annotations ++ content.annotations)
+
     WhiskAction(
       action.namespace,
       action.name,
@@ -545,7 +552,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
       limits,
       content.version getOrElse action.version.upPatch,
       content.publish getOrElse action.publish,
-      WhiskActionsApi.amendAnnotations(content.annotations getOrElse action.annotations, exec, create = false))
+      WhiskActionsApi.amendAnnotations(newAnnotations, exec, create = false))
       .revision[WhiskAction](action.docinfo.rev)
   }
 
diff --git a/tests/src/test/scala/common/WskCliOperations.scala b/tests/src/test/scala/common/WskCliOperations.scala
index 3dea0cd..925e5d4 100644
--- a/tests/src/test/scala/common/WskCliOperations.scala
+++ b/tests/src/test/scala/common/WskCliOperations.scala
@@ -195,6 +195,7 @@ class CliActionOperations(override val wsk: RunCliCmd)
     docker: Option[String] = None,
     parameters: Map[String, JsValue] = Map.empty,
     annotations: Map[String, JsValue] = Map.empty,
+    delAnnotations: Array[String] = Array(),
     parameterFile: Option[String] = None,
     annotationFile: Option[String] = None,
     timeout: Option[Duration] = None,
@@ -230,6 +231,10 @@ class CliActionOperations(override val wsk: RunCliCmd)
         Seq("-a", p._1, p._2.compactPrint)
       }
     } ++ {
+      delAnnotations flatMap { p =>
+        Seq("--del-annotation", p)
+      }
+    } ++ {
       parameterFile map { pf =>
         Seq("-P", pf)
       } getOrElse Seq.empty
diff --git a/tests/src/test/scala/common/WskOperations.scala b/tests/src/test/scala/common/WskOperations.scala
index e7d22d5..111f305 100644
--- a/tests/src/test/scala/common/WskOperations.scala
+++ b/tests/src/test/scala/common/WskOperations.scala
@@ -234,6 +234,7 @@ trait ActionOperations extends DeleteFromCollectionOperations with ListOrGetFrom
              docker: Option[String] = None,
              parameters: Map[String, JsValue] = Map.empty,
              annotations: Map[String, JsValue] = Map.empty,
+             delAnnotations: Array[String] = Array(),
              parameterFile: Option[String] = None,
              annotationFile: Option[String] = None,
              timeout: Option[Duration] = None,
diff --git a/tests/src/test/scala/common/rest/WskRestOperations.scala b/tests/src/test/scala/common/rest/WskRestOperations.scala
index 3859a9d..2c652d9 100644
--- a/tests/src/test/scala/common/rest/WskRestOperations.scala
+++ b/tests/src/test/scala/common/rest/WskRestOperations.scala
@@ -264,6 +264,7 @@ class RestActionOperations(implicit val actorSystem: ActorSystem)
     docker: Option[String] = None,
     parameters: Map[String, JsValue] = Map.empty,
     annotations: Map[String, JsValue] = Map.empty,
+    delAnnotations: Array[String] = Array(),
     parameterFile: Option[String] = None,
     annotationFile: Option[String] = None,
     timeout: Option[Duration] = None,
@@ -364,6 +365,8 @@ class RestActionOperations(implicit val actorSystem: ActorSystem)
         content = content + ("annotations" -> annos.toJson)
       if (limits.nonEmpty)
         content = content + ("limits" -> limits.toJson)
+      if (delAnnotations.nonEmpty)
+        content = content + ("delAnnotations" -> delAnnotations.toJson)
       content
     }
 
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskRestBasicUsageTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskRestBasicUsageTests.scala
index 9b2e61a..fc9e54d 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskRestBasicUsageTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/cli/test/WskRestBasicUsageTests.scala
@@ -131,6 +131,38 @@ class WskRestBasicUsageTests extends TestHelpers with WskTestHelpers with WskAct
       }
   }
 
+  it should "delete the given annotations using delAnnotations" in withAssetCleaner(wskprops) { (wp, assetHelper) =>
+    val name = "hello"
+
+    assetHelper.withCleaner(wsk.action, name) { (action, _) =>
+      val annotations = Map("key1" -> "value1".toJson, "key2" -> "value2".toJson)
+      action.create(name, Some(TestUtils.getTestActionFilename("hello.js")), annotations = annotations)
+      val annotationString = wsk.parseJsonString(wsk.action.get(name).stdout).fields("annotations").toString
+
+      annotationString should include(""""key":"key1"""")
+      annotationString should include(""""value":"value1"""")
+      annotationString should include(""""key":"key2"""")
+      annotationString should include(""""value":"value2"""")
+
+      //Delete key1 only
+      val delAnnotations = Array("key1")
+
+      action.create(
+        name,
+        Some(TestUtils.getTestActionFilename("hello.js")),
+        delAnnotations = delAnnotations,
+        update = true)
+      val newAnnotationString = wsk.parseJsonString(wsk.action.get(name).stdout).fields("annotations").toString
+
+      newAnnotationString should not include (""""key":"key1"""")
+      newAnnotationString should not include (""""value":"value1"""")
+      newAnnotationString should include(""""key":"key2"""")
+      newAnnotationString should include(""""value":"value2"""")
+
+      action.create(name, Some(TestUtils.getTestActionFilename("hello.js")), update = true)
+    }
+  }
+
   it should "create, and get an action to verify file parameter and annotation parsing" in withAssetCleaner(wskprops) {
     (wp, assetHelper) =>
       val name = "actionAnnotAndParamParsing"
diff --git a/tests/src/test/scala/system/basic/WskActionTests.scala b/tests/src/test/scala/system/basic/WskActionTests.scala
index d0061cb..4b4d603 100644
--- a/tests/src/test/scala/system/basic/WskActionTests.scala
+++ b/tests/src/test/scala/system/basic/WskActionTests.scala
@@ -357,4 +357,39 @@ class WskActionTests extends TestHelpers with WskTestHelpers with JsHelpers with
     }
   }
 
+  it should "not delete existing annotations when updating action with new annotation" in withAssetCleaner(wskprops) {
+    (wp, assetHelper) =>
+      val name = "hello"
+
+      assetHelper.withCleaner(wsk.action, name) { (action, _) =>
+        val annotations = Map("key1" -> "value1".toJson, "key2" -> "value2".toJson)
+        action.create(name, Some(TestUtils.getTestActionFilename("hello.js")), annotations = annotations)
+        val annotationString = wsk.parseJsonString(wsk.action.get(name).stdout).fields("annotations").toString
+
+        annotationString should include(""""key":"key1"""")
+        annotationString should include(""""value":"value1"""")
+        annotationString should include(""""key":"key2"""")
+        annotationString should include(""""value":"value2"""")
+
+        val newAnnotations = Map("key3" -> "value3".toJson, "key4" -> "value4".toJson)
+        action.create(
+          name,
+          Some(TestUtils.getTestActionFilename("hello.js")),
+          annotations = newAnnotations,
+          update = true)
+        val newAnnotationString = wsk.parseJsonString(wsk.action.get(name).stdout).fields("annotations").toString
+
+        newAnnotationString should include(""""key":"key1"""")
+        newAnnotationString should include(""""value":"value1"""")
+        newAnnotationString should include(""""key":"key2"""")
+        newAnnotationString should include(""""value":"value2"""")
+        newAnnotationString should include(""""key":"key3"""")
+        newAnnotationString should include(""""value":"value3"""")
+        newAnnotationString should include(""""key":"key4"""")
+        newAnnotationString should include(""""value":"value4"""")
+
+        action.create(name, Some(TestUtils.getTestActionFilename("hello.js")), update = true)
+      }
+  }
+
 }