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/08/20 18:26:41 UTC

[GitHub] dubee closed pull request #3945: Treat action code as attachments

dubee closed pull request #3945: Treat action code as attachments
URL: https://github.com/apache/incubator-openwhisk/pull/3945
 
 
   

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/ansible/files/runtimes.json b/ansible/files/runtimes.json
index 4efd81468e..13583d27a0 100644
--- a/ansible/files/runtimes.json
+++ b/ansible/files/runtimes.json
@@ -8,7 +8,11 @@
                     "name": "nodejsaction",
                     "tag": "latest"
                 },
-                "deprecated": true
+                "deprecated": true,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             },
             {
                 "kind": "nodejs:6",
@@ -19,6 +23,10 @@
                     "tag": "latest"
                 },
                 "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                },
                 "stemCells": [{
                     "count": 2,
                     "memory": "256 MB"
@@ -32,7 +40,11 @@
                     "name": "action-nodejs-v8",
                     "tag": "latest"
                 },
-                "deprecated": false
+                "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             }
         ],
         "python": [
@@ -43,7 +55,11 @@
                     "name": "python2action",
                     "tag": "latest"
                 },
-                "deprecated": false
+                "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             },
             {
                 "kind": "python:2",
@@ -53,7 +69,11 @@
                     "name": "python2action",
                     "tag": "latest"
                 },
-                "deprecated": false
+                "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             },
             {
                 "kind": "python:3",
@@ -62,7 +82,11 @@
                     "name": "python3action",
                     "tag": "latest"
                 },
-                "deprecated": false
+                "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             }
         ],
         "swift": [
@@ -73,7 +97,11 @@
                     "name": "swiftaction",
                     "tag": "latest"
                 },
-                "deprecated": true
+                "deprecated": true,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             },
             {
                 "kind": "swift:3",
@@ -82,7 +110,11 @@
                     "name": "swift3action",
                     "tag": "latest"
                 },
-                "deprecated": true
+                "deprecated": true,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             },
             {
                 "kind": "swift:3.1.1",
@@ -91,7 +123,11 @@
                     "name": "action-swift-v3.1.1",
                     "tag": "latest"
                 },
-                "deprecated": false
+                "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             },
             {
                 "kind": "swift:4.1",
@@ -101,7 +137,11 @@
                     "name": "action-swift-v4.1",
                     "tag": "latest"
                 },
-                "deprecated": false
+                "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                }
             }
         ],
         "java": [
@@ -115,8 +155,8 @@
                 },
                 "deprecated": false,
                 "attached": {
-                    "attachmentName": "jarfile",
-                    "attachmentType": "application/java-archive"
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
                 },
                 "requireMain": true
             }
@@ -130,6 +170,10 @@
                     "prefix": "openwhisk",
                     "name": "action-php-v7.1",
                     "tag": "latest"
+                },
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
                 }
             },
             {
@@ -140,6 +184,10 @@
                     "prefix": "openwhisk",
                     "name": "action-php-v7.2",
                     "tag": "latest"
+                },
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
                 }
             }
         ],
@@ -148,6 +196,10 @@
                 "kind": "ruby:2.5",
                 "default": true,
                 "deprecated": false,
+                "attached": {
+                    "attachmentName": "codefile",
+                    "attachmentType": "text/plain"
+                },
                 "image": {
                     "prefix": "openwhisk",
                     "name": "action-ruby-v2.5",
diff --git a/common/scala/src/main/scala/whisk/core/entity/Exec.scala b/common/scala/src/main/scala/whisk/core/entity/Exec.scala
index ca77a9d1e9..c98e57188d 100644
--- a/common/scala/src/main/scala/whisk/core/entity/Exec.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/Exec.scala
@@ -140,14 +140,14 @@ protected[core] case class CodeExecMetaDataAsString(manifest: RuntimeManifest,
 
 protected[core] case class CodeExecAsAttachment(manifest: RuntimeManifest,
                                                 override val code: Attachment[String],
-                                                override val entryPoint: Option[String])
+                                                override val entryPoint: Option[String],
+                                                override val binary: Boolean = false)
     extends CodeExec[Attachment[String]] {
   override val kind = manifest.kind
   override val image = manifest.image
   override val sentinelledLogs = manifest.sentinelledLogs.getOrElse(true)
   override val deprecated = manifest.deprecated.getOrElse(false)
   override val pull = false
-  override lazy val binary = true
   override def codeAsJson = code.toJson
 
   def inline(bytes: Array[Byte]): CodeExecAsAttachment = {
@@ -301,22 +301,24 @@ protected[core] object Exec extends ArgNormalizer[Exec] with DefaultJsonProtocol
           }
 
           manifest.attached
-            .map { a =>
-              val jar: Attachment[String] = {
-                // java actions once stored the attachment in "jar" instead of "code"
-                obj.fields.get("code").orElse(obj.fields.get("jar"))
-              } map {
-                attFmt[String].read(_)
-              } getOrElse {
+            .map { _ =>
+              // java actions once stored the attachment in "jar" instead of "code"
+              val code = obj.fields.get("code").orElse(obj.fields.get("jar")).getOrElse {
                 throw new DeserializationException(
-                  s"'code' must be a valid base64 string in 'exec' for '$kind' actions")
+                  s"'code' must be a string or attachment object defined in 'exec' for '$kind' actions")
               }
+              val binary: Boolean = code match {
+                case JsString(c) => isBinaryCode(c)
+                case _           => obj.fields.get("binary").map(_.convertTo[Boolean]).getOrElse(false)
+              }
+
               val main = optMainField.orElse {
                 if (manifest.requireMain.exists(identity)) {
                   throw new DeserializationException(s"'main' must be a string defined in 'exec' for '$kind' actions")
                 } else None
               }
-              CodeExecAsAttachment(manifest, jar, main)
+
+              CodeExecAsAttachment(manifest, attFmt[String].read(code), main, binary)
             }
             .getOrElse {
               val code: String = obj.fields.get("code") match {
diff --git a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
index 3617ae098f..fd4d5461a4 100644
--- a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
@@ -19,8 +19,11 @@ package whisk.core.entity
 
 import java.io.ByteArrayInputStream
 import java.io.ByteArrayOutputStream
+import java.nio.charset.StandardCharsets
 import java.util.Base64
 
+import akka.http.scaladsl.model.ContentTypes
+
 import scala.concurrent.ExecutionContext
 import scala.concurrent.Future
 import scala.util.{Failure, Success, Try}
@@ -333,23 +336,27 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
       require(doc != null, "doc undefined")
     } map { _ =>
       doc.exec match {
-        case exec @ CodeExecAsAttachment(_, Inline(code), _) =>
+        case exec @ CodeExecAsAttachment(_, Inline(code), _, binary) =>
           implicit val logger = db.logging
           implicit val ec = db.executionContext
 
-          val stream = new ByteArrayInputStream(Base64.getDecoder().decode(code))
-          val manifest = exec.manifest.attached.get
+          val (bytes, attachmentType) = if (binary) {
+            (Base64.getDecoder.decode(code), ContentTypes.`application/octet-stream`)
+          } else {
+            (code.getBytes(StandardCharsets.UTF_8), ContentTypes.`text/plain(UTF-8)`)
+          }
+          val stream = new ByteArrayInputStream(bytes)
           val oldAttachment = old
             .flatMap(_.exec match {
-              case CodeExecAsAttachment(_, a: Attached, _) => Some(a)
-              case _                                       => None
+              case CodeExecAsAttachment(_, a: Attached, _, _) => Some(a)
+              case _                                          => None
             })
 
           super.putAndAttach(
             db,
             doc,
             (d, a) => d.copy(exec = exec.attach(a)).revision[WhiskAction](d.rev),
-            manifest.attachmentType,
+            attachmentType,
             stream,
             oldAttachment,
             Some { a: WhiskAction =>
@@ -378,12 +385,12 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
 
     fa.flatMap { action =>
       action.exec match {
-        case exec @ CodeExecAsAttachment(_, attached: Attached, _) =>
+        case exec @ CodeExecAsAttachment(_, attached: Attached, _, binary) =>
           val boas = new ByteArrayOutputStream()
-          val b64s = Base64.getEncoder().wrap(boas)
+          val wrapped = if (binary) Base64.getEncoder().wrap(boas) else boas
 
-          getAttachment[A](db, action, attached, b64s, Some { a: WhiskAction =>
-            b64s.close()
+          getAttachment[A](db, action, attached, wrapped, Some { a: WhiskAction =>
+            wrapped.close()
             val newAction = a.copy(exec = exec.inline(boas.toByteArray))
             newAction.revision(a.rev)
             newAction
@@ -397,7 +404,7 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[
 
   def attachmentHandler(action: WhiskAction, attached: Attached): WhiskAction = {
     val eu = action.exec match {
-      case exec @ CodeExecAsAttachment(_, Attached(attachmentName, _, _, _), _) =>
+      case exec @ CodeExecAsAttachment(_, Attached(attachmentName, _, _, _), _, _) =>
         require(
           attachmentName == attached.attachmentName,
           s"Attachment name '${attached.attachmentName}' does not match the expected name '$attachmentName'")
diff --git a/tests/performance/gatling_tests/src/gatling/scala/ColdBlockingInvokeSimulation.scala b/tests/performance/gatling_tests/src/gatling/scala/ColdBlockingInvokeSimulation.scala
index d7ec74f912..da68e7a5eb 100644
--- a/tests/performance/gatling_tests/src/gatling/scala/ColdBlockingInvokeSimulation.scala
+++ b/tests/performance/gatling_tests/src/gatling/scala/ColdBlockingInvokeSimulation.scala
@@ -32,6 +32,7 @@ class ColdBlockingInvokeSimulation extends Simulation {
   val host = sys.env("OPENWHISK_HOST")
 
   val users: Int = sys.env("USERS").toInt
+  val codeSize: Int = sys.env.getOrElse("CODE_SIZE", "0").toInt
   val seconds: FiniteDuration = sys.env.getOrElse("SECONDS", "10").toInt.seconds
   val actionsPerUser: Int = sys.env.getOrElse("ACTIONS_PER_USER", "5").toInt
 
@@ -64,8 +65,7 @@ class ColdBlockingInvokeSimulation extends Simulation {
           openWhisk("Create action")
             .authenticate(uuid, key)
             .action(actionName)
-            .create(FileUtils
-              .readFileToString(Resource.body("nodeJSAction.js").get.file, StandardCharsets.UTF_8)))
+            .create(actionCode))
       }.rendezVous(users)
         // Execute all actions for the given amount of time.
         .during(seconds) {
@@ -81,6 +81,13 @@ class ColdBlockingInvokeSimulation extends Simulation {
         }
     }
 
+  private def actionCode = {
+    val code = FileUtils
+      .readFileToString(Resource.body("nodeJSAction.js").get.file, StandardCharsets.UTF_8)
+    //Pad the code with empty space to increase the stored code size
+    if (codeSize > 0) code + " " * codeSize else code
+  }
+
   setUp(test.inject(atOnceUsers(users)))
     .protocols(openWhiskProtocol)
     // One failure will make the build yellow
diff --git a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
index 9b54b0a7bb..2b3cbb341a 100644
--- a/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/ActionsApiTests.scala
@@ -790,82 +790,91 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
   }
 
   it should "put and then get an action with attachment from cache" in {
-    val action =
+    val javaAction =
       WhiskAction(
         namespace,
         aname(),
         javaDefault(nonInlinedCode(entityStore), Some("hello")),
         annotations = Parameters("exec", "java"))
-    val content = WhiskActionPut(
-      Some(action.exec),
-      Some(action.parameters),
-      Some(ActionLimitsOption(Some(action.limits.timeout), Some(action.limits.memory), Some(action.limits.logs))))
-    val name = action.name
-    val cacheKey = s"${CacheKey(action)}".replace("(", "\\(").replace(")", "\\)")
-    val expectedPutLog =
-      Seq(s"uploading attachment '[\\w-]+' of document 'id: ${action.namespace}/${action.name}", s"caching $cacheKey")
-        .mkString("(?s).*")
-    val notExpectedGetLog = Seq(
-      s"finding document: 'id: ${action.namespace}/${action.name}",
-      s"finding attachment '[\\w-/:]+' of document 'id: ${action.namespace}/${action.name}").mkString("(?s).*")
+    val nodeAction = WhiskAction(namespace, aname(), jsDefault(nonInlinedCode(entityStore)), Parameters("x", "b"))
+    val swiftAction = WhiskAction(namespace, aname(), swift3(nonInlinedCode(entityStore)), Parameters("x", "b"))
+    val actions = Seq((javaAction, JAVA_DEFAULT), (nodeAction, NODEJS6), (swiftAction, SWIFT3))
 
-    // first request invalidates any previous entries and caches new result
-    Put(s"$collectionPath/$name", content) ~> Route.seal(routes(creds)(transid())) ~> check {
-      status should be(OK)
-      val response = responseAs[WhiskAction]
-      response should be(
-        WhiskAction(
-          action.namespace,
-          action.name,
-          action.exec,
-          action.parameters,
-          action.limits,
-          action.version,
-          action.publish,
-          action.annotations ++ Parameters(WhiskAction.execFieldName, JAVA_DEFAULT)))
-    }
+    actions.foreach {
+      case (action, kind) =>
+        val content = WhiskActionPut(
+          Some(action.exec),
+          Some(action.parameters),
+          Some(ActionLimitsOption(Some(action.limits.timeout), Some(action.limits.memory), Some(action.limits.logs))))
+        val cacheKey = s"${CacheKey(action)}".replace("(", "\\(").replace(")", "\\)")
 
-    stream.toString should not include (s"invalidating ${CacheKey(action)} on delete")
-    stream.toString should include regex (expectedPutLog)
-    stream.reset()
+        val expectedPutLog =
+          Seq(
+            s"uploading attachment '[\\w-]+' of document 'id: ${action.namespace}/${action.name}",
+            s"caching $cacheKey")
+            .mkString("(?s).*")
+        val notExpectedGetLog = Seq(
+          s"finding document: 'id: ${action.namespace}/${action.name}",
+          s"finding attachment '[\\w-/:]+' of document 'id: ${action.namespace}/${action.name}").mkString("(?s).*")
 
-    // second request should fetch from cache
-    Get(s"$collectionPath/$name") ~> Route.seal(routes(creds)(transid())) ~> check {
-      status should be(OK)
-      val response = responseAs[WhiskAction]
-      response should be(
-        WhiskAction(
-          action.namespace,
-          action.name,
-          action.exec,
-          action.parameters,
-          action.limits,
-          action.version,
-          action.publish,
-          action.annotations ++ Parameters(WhiskAction.execFieldName, JAVA_DEFAULT)))
-    }
+        // first request invalidates any previous entries and caches new result
+        Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)(transid())) ~> check {
+          status should be(OK)
+          val response = responseAs[WhiskAction]
+          response should be(
+            WhiskAction(
+              action.namespace,
+              action.name,
+              action.exec,
+              action.parameters,
+              action.limits,
+              action.version,
+              action.publish,
+              action.annotations ++ Parameters(WhiskAction.execFieldName, kind)))
+        }
 
-    stream.toString should include(s"serving from cache: ${CacheKey(action)}")
-    stream.toString should not include regex(notExpectedGetLog)
-    stream.reset()
+        stream.toString should not include (s"invalidating ${CacheKey(action)} on delete")
+        stream.toString should include regex (expectedPutLog)
+        stream.reset()
 
-    // delete should invalidate cache
-    Delete(s"$collectionPath/$name") ~> Route.seal(routes(creds)(transid())) ~> check {
-      status should be(OK)
-      val response = responseAs[WhiskAction]
-      response should be(
-        WhiskAction(
-          action.namespace,
-          action.name,
-          action.exec,
-          action.parameters,
-          action.limits,
-          action.version,
-          action.publish,
-          action.annotations ++ Parameters(WhiskAction.execFieldName, JAVA_DEFAULT)))
+        // second request should fetch from cache
+        Get(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)(transid())) ~> check {
+          status should be(OK)
+          val response = responseAs[WhiskAction]
+          response should be(
+            WhiskAction(
+              action.namespace,
+              action.name,
+              action.exec,
+              action.parameters,
+              action.limits,
+              action.version,
+              action.publish,
+              action.annotations ++ Parameters(WhiskAction.execFieldName, kind)))
+        }
+        stream.toString should include(s"serving from cache: ${CacheKey(action)}")
+        stream.toString should not include regex(notExpectedGetLog)
+        stream.reset()
+
+        // delete should invalidate cache
+        Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)(transid())) ~> check {
+          status should be(OK)
+          val response = responseAs[WhiskAction]
+          response should be(
+            WhiskAction(
+              action.namespace,
+              action.name,
+              action.exec,
+              action.parameters,
+              action.limits,
+              action.version,
+              action.publish,
+              action.annotations ++ Parameters(WhiskAction.execFieldName, kind)))
+        }
+
+        stream.toString should include(s"invalidating ${CacheKey(action)}")
+        stream.reset()
     }
-    stream.toString should include(s"invalidating ${CacheKey(action)}")
-    stream.reset()
   }
 
   it should "put and then get an action with inlined attachment" in {
@@ -961,8 +970,9 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
       s"finding attachment '[\\w-/:]+' of document 'id: ${action.namespace}/${action.name}").mkString("(?s).*")
 
     action.exec match {
-      case exec @ CodeExecAsAttachment(_, _, _) =>
-        val stream = new ByteArrayInputStream(Base64.getDecoder().decode(code))
+      case exec @ CodeExecAsAttachment(_, _, _, binary) =>
+        val bytes = if (binary) Base64.getDecoder().decode(code) else code.getBytes("UTF-8")
+        val stream = new ByteArrayInputStream(bytes)
         val manifest = exec.manifest.attached.get
         val src = StreamConverters.fromInputStream(() => stream)
         putAndAttach[WhiskAction, WhiskEntity](
@@ -1012,8 +1022,8 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
         .mkString("(?s).*")
 
     action.exec match {
-      case exec @ CodeExecAsAttachment(_, _, _) =>
-        val stream = new ByteArrayInputStream(Base64.getDecoder().decode(code))
+      case exec @ CodeExecAsAttachment(_, _, _, _) =>
+        val stream = new ByteArrayInputStream(code.getBytes)
         val manifest = exec.manifest.attached.get
         val src = StreamConverters.fromInputStream(() => stream)
         putAndAttach[WhiskAction, WhiskEntity](
@@ -1063,6 +1073,73 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
     stream.reset()
   }
 
+  it should "ensure old and new action schemas are supported" in {
+    implicit val tid = transid()
+    val code = nonInlinedCode(entityStore)
+    val actionOldSchema = WhiskAction(namespace, aname(), js6Old(code))
+    val actionNewSchema = WhiskAction(namespace, aname(), jsDefault(code))
+    val content = WhiskActionPut(
+      Some(actionOldSchema.exec),
+      Some(actionOldSchema.parameters),
+      Some(
+        ActionLimitsOption(
+          Some(actionOldSchema.limits.timeout),
+          Some(actionOldSchema.limits.memory),
+          Some(actionOldSchema.limits.logs))))
+    val expectedPutLog =
+      Seq(s"uploading attachment '[\\w-/:]+' of document 'id: ${actionOldSchema.namespace}/${actionOldSchema.name}")
+        .mkString("(?s).*")
+
+    put(entityStore, actionOldSchema)
+
+    stream.toString should not include regex(expectedPutLog)
+    stream.reset()
+
+    Post(s"$collectionPath/${actionOldSchema.name}") ~> Route.seal(routes(creds)) ~> check {
+      status should be(Accepted)
+      val response = responseAs[JsObject]
+      response.fields("activationId") should not be None
+    }
+
+    Put(s"$collectionPath/${actionOldSchema.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check {
+      val response = responseAs[WhiskAction]
+      response should be(
+        WhiskAction(
+          actionOldSchema.namespace,
+          actionOldSchema.name,
+          actionNewSchema.exec,
+          actionOldSchema.parameters,
+          actionOldSchema.limits,
+          actionOldSchema.version.upPatch,
+          actionOldSchema.publish,
+          actionOldSchema.annotations ++ Parameters(WhiskAction.execFieldName, NODEJS6)))
+    }
+
+    stream.toString should include regex (expectedPutLog)
+    stream.reset()
+
+    Post(s"$collectionPath/${actionOldSchema.name}") ~> Route.seal(routes(creds)) ~> check {
+      status should be(Accepted)
+      val response = responseAs[JsObject]
+      response.fields("activationId") should not be None
+    }
+
+    Delete(s"$collectionPath/${actionOldSchema.name}") ~> Route.seal(routes(creds)) ~> check {
+      status should be(OK)
+      val response = responseAs[WhiskAction]
+      response should be(
+        WhiskAction(
+          actionOldSchema.namespace,
+          actionOldSchema.name,
+          actionNewSchema.exec,
+          actionOldSchema.parameters,
+          actionOldSchema.limits,
+          actionOldSchema.version.upPatch,
+          actionOldSchema.publish,
+          actionOldSchema.annotations ++ Parameters(WhiskAction.execFieldName, NODEJS6)))
+    }
+  }
+
   it should "reject put with conflict for pre-existing action" in {
     implicit val tid = transid()
     val action = WhiskAction(namespace, aname(), jsDefault("??"), Parameters("x", "b"))
diff --git a/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala b/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala
index dc8969f50b..82b693259a 100644
--- a/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala
+++ b/tests/src/test/scala/whisk/core/database/test/AttachmentCompatibilityTests.scala
@@ -30,24 +30,15 @@ import org.scalatest.concurrent.ScalaFutures
 import org.scalatest.junit.JUnitRunner
 import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach, FlatSpec, Matchers}
 import pureconfig.loadConfigOrThrow
-import spray.json.DefaultJsonProtocol
+import spray.json._
 import whisk.common.TransactionId
 import whisk.core.ConfigKeys
+import whisk.core.controller.test.WhiskAuthHelpers
 import whisk.core.database.memory.MemoryAttachmentStoreProvider
 import whisk.core.database.{CouchDbConfig, CouchDbRestClient, CouchDbStoreProvider, NoDocumentException}
 import whisk.core.entity.Attachments.Inline
 import whisk.core.entity.test.ExecHelpers
-import whisk.core.entity.{
-  CodeExecAsAttachment,
-  DocInfo,
-  EntityName,
-  EntityPath,
-  WhiskAction,
-  WhiskDocumentReader,
-  WhiskEntity,
-  WhiskEntityJsonFormat,
-  WhiskEntityStore
-}
+import whisk.core.entity._
 
 import scala.concurrent.Future
 import scala.reflect.classTag
@@ -68,6 +59,10 @@ class AttachmentCompatibilityTests
   //Bring in sync the timeout used by ScalaFutures and DBUtils
   implicit override val patienceConfig: PatienceConfig = PatienceConfig(timeout = dbOpTimeout)
   implicit val materializer = ActorMaterializer()
+
+  val creds = WhiskAuthHelpers.newIdentity()
+  val namespace = EntityPath(creds.subject.asString)
+  def aname() = MakeName.next("action_tests")
   val config = loadConfigOrThrow[CouchDbConfig](ConfigKeys.couchdb)
   val entityStore = WhiskEntityStore.datastore()
   val client =
@@ -117,10 +112,75 @@ class AttachmentCompatibilityTests
     doc2.exec shouldBe exec
   }
 
+  it should "read existing base64 encoded code string" in {
+    implicit val tid: TransactionId = transid()
+    val exec = """{
+               |  "kind": "nodejs:6",
+               |  "code": "SGVsbG8gT3BlbldoaXNr"
+               |}""".stripMargin.parseJson.asJsObject
+    val (id, action) = makeActionJson(namespace, aname(), exec)
+    val info = putDoc(id, action)
+
+    val action2 = WhiskAction.get(entityStore, info.id).futureValue
+    codeExec(action2).codeAsJson shouldBe JsString("SGVsbG8gT3BlbldoaXNr")
+  }
+
+  it should "read existing simple code string" in {
+    implicit val tid: TransactionId = transid()
+    val exec = """{
+                 |  "kind": "nodejs:6",
+                 |  "code": "while (true)"
+                 |}""".stripMargin.parseJson.asJsObject
+    val (id, action) = makeActionJson(namespace, aname(), exec)
+    val info = putDoc(id, action)
+
+    val action2 = WhiskAction.get(entityStore, info.id).futureValue
+    codeExec(action2).codeAsJson shouldBe JsString("while (true)")
+  }
+
+  private def codeExec(a: WhiskAction) = a.exec.asInstanceOf[CodeExec[_]]
+
+  private def makeActionJson(namespace: EntityPath, name: EntityName, exec: JsObject): (String, JsObject) = {
+    val id = namespace.addPath(name).asString
+    val base = s"""{
+                 |  "name": "${name.asString}",
+                 |  "_id": "$id",
+                 |  "publish": false,
+                 |  "annotations": [],
+                 |  "version": "0.0.1",
+                 |  "updated": 1533623651650,
+                 |  "entityType": "action",
+                 |  "parameters": [
+                 |    {
+                 |      "key": "x",
+                 |      "value": "b"
+                 |    }
+                 |  ],
+                 |  "limits": {
+                 |    "timeout": 60000,
+                 |    "memory": 256,
+                 |    "logs": 10
+                 |  },
+                 |  "namespace": "${namespace.asString}"
+                 |}""".stripMargin.parseJson.asJsObject
+    (id, JsObject(base.fields + ("exec" -> exec)))
+  }
+
+  private def putDoc(id: String, js: JsObject): DocInfo = {
+    val r = client.putDoc(id, js).futureValue
+    r match {
+      case Right(response) =>
+        val info = response.convertTo[DocInfo]
+        docsToDelete += ((entityStore, info))
+        info
+      case _ => fail()
+    }
+  }
+
   private def createAction(doc: WhiskAction) = {
     implicit val tid: TransactionId = transid()
     doc.exec match {
-      case exec @ CodeExecAsAttachment(_, Inline(code), _) =>
+      case exec @ CodeExecAsAttachment(_, Inline(code), _, _) =>
         val attached = exec.manifest.attached.get
 
         val newDoc = doc.copy(exec = exec.copy(code = attached))
@@ -137,6 +197,14 @@ class AttachmentCompatibilityTests
     }
   }
 
+  object MakeName {
+    @volatile var counter = 1
+    def next(prefix: String = "test")(): EntityName = {
+      counter = counter + 1
+      EntityName(s"${prefix}_name$counter")
+    }
+  }
+
   private def attach(doc: DocInfo,
                      name: String,
                      contentType: ContentType,
diff --git a/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala b/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala
index ec5a3e6c4d..a669296a40 100644
--- a/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala
+++ b/tests/src/test/scala/whisk/core/database/test/CacheConcurrencyTests.scala
@@ -121,7 +121,11 @@ class CacheConcurrencyTests extends FlatSpec with WskTestHelpers with WskActorSy
         para.tasksupport = new ForkJoinTaskSupport(new ForkJoinPool(nThreads))
         para.map { i =>
           if (i != 16) {
-            wsk.action.get(name)
+            val rr = wsk.action.get(name, expectedExitCode = DONTCARE_EXIT)
+            withClue(s"expecting get to either succeed or fail with not found: $rr") {
+              // some will succeed and some should fail with not found
+              rr.exitCode should (be(SUCCESS_EXIT) or be(NOT_FOUND))
+            }
           } else {
             wsk.action.create(name, None, parameters = Map("color" -> JsString("blue")), update = true)
           }
diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala
index 1438e79d85..ec7fbe6736 100644
--- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala
+++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala
@@ -22,14 +22,15 @@ import java.util.Base64
 
 import akka.http.scaladsl.model.{ContentTypes, Uri}
 import akka.stream.IOResult
-import scala.concurrent.duration.DurationInt
 import akka.stream.scaladsl.{Sink, StreamConverters}
 import akka.util.{ByteString, ByteStringBuilder}
 import whisk.common.TransactionId
 import whisk.core.database.{AttachmentSupport, CacheChangeNotification, NoDocumentException}
 import whisk.core.entity.Attachments.{Attached, Attachment, Inline}
 import whisk.core.entity.test.ExecHelpers
-import whisk.core.entity.{CodeExec, DocInfo, EntityName, ExecManifest, WhiskAction}
+import whisk.core.entity.{CodeExec, DocInfo, EntityName, WhiskAction}
+
+import scala.concurrent.duration.DurationInt
 
 trait ArtifactStoreAttachmentBehaviors extends ArtifactStoreBehaviorBase with ExecHelpers {
   behavior of s"${storeType}ArtifactStore attachments"
@@ -133,12 +134,7 @@ trait ArtifactStoreAttachmentBehaviors extends ArtifactStoreBehaviorBase with Ex
 
     docsToDelete += ((entityStore, i1))
 
-    attached(action2).attachmentType shouldBe ExecManifest.runtimesManifest
-      .resolveDefaultRuntime(JAVA_DEFAULT)
-      .get
-      .attached
-      .get
-      .attachmentType
+    attached(action2).attachmentType shouldBe ContentTypes.`application/octet-stream`
     attached(action2).length shouldBe Some(size)
     attached(action2).digest should not be empty
 
diff --git a/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala b/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala
index 33dd7e17de..fa8fd2ec77 100644
--- a/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala
+++ b/tests/src/test/scala/whisk/core/entity/test/ExecHelpers.scala
@@ -50,11 +50,18 @@ trait ExecHelpers extends Matchers with WskActorSystem with StreamLogging {
     ExecManifest.runtimesManifest.runtimes.flatMap(_.versions).find(_.kind == name).get.image
   }
 
-  protected def js(code: String, main: Option[String] = None) = {
+  protected def jsOld(code: String, main: Option[String] = None) = {
     CodeExecAsString(RuntimeManifest(NODEJS, imagename(NODEJS), deprecated = Some(true)), trim(code), main.map(_.trim))
   }
 
-  protected def js6(code: String, main: Option[String] = None) = {
+  protected def js(code: String, main: Option[String] = None) = {
+    val attachment = attFmt[String].read(code.trim.toJson)
+    val manifest = ExecManifest.runtimesManifest.resolveDefaultRuntime(NODEJS).get
+
+    CodeExecAsAttachment(manifest, attachment, main.map(_.trim), Exec.isBinaryCode(code))
+  }
+
+  protected def js6Old(code: String, main: Option[String] = None) = {
     CodeExecAsString(
       RuntimeManifest(
         NODEJS6,
@@ -65,12 +72,18 @@ trait ExecHelpers extends Matchers with WskActorSystem with StreamLogging {
       trim(code),
       main.map(_.trim))
   }
+  protected def js6(code: String, main: Option[String] = None) = {
+    val attachment = attFmt[String].read(code.trim.toJson)
+    val manifest = ExecManifest.runtimesManifest.resolveDefaultRuntime(NODEJS6).get
+
+    CodeExecAsAttachment(manifest, attachment, main.map(_.trim), Exec.isBinaryCode(code))
+  }
 
   protected def jsDefault(code: String, main: Option[String] = None) = {
     js6(code, main)
   }
 
-  protected def js6MetaData(main: Option[String] = None, binary: Boolean) = {
+  protected def js6MetaDataOld(main: Option[String] = None, binary: Boolean) = {
     CodeExecMetaDataAsString(
       RuntimeManifest(
         NODEJS6,
@@ -82,11 +95,17 @@ trait ExecHelpers extends Matchers with WskActorSystem with StreamLogging {
       main.map(_.trim))
   }
 
+  protected def js6MetaData(main: Option[String] = None, binary: Boolean) = {
+    val manifest = ExecManifest.runtimesManifest.resolveDefaultRuntime(NODEJS6).get
+
+    CodeExecMetaDataAsAttachment(manifest, binary, main.map(_.trim))
+  }
+
   protected def javaDefault(code: String, main: Option[String] = None) = {
     val attachment = attFmt[String].read(code.trim.toJson)
     val manifest = ExecManifest.runtimesManifest.resolveDefaultRuntime(JAVA_DEFAULT).get
 
-    CodeExecAsAttachment(manifest, attachment, main.map(_.trim))
+    CodeExecAsAttachment(manifest, attachment, main.map(_.trim), Exec.isBinaryCode(code))
   }
 
   protected def javaMetaData(main: Option[String] = None, binary: Boolean) = {
@@ -95,16 +114,22 @@ trait ExecHelpers extends Matchers with WskActorSystem with StreamLogging {
     CodeExecMetaDataAsAttachment(manifest, binary, main.map(_.trim))
   }
 
-  protected def swift(code: String, main: Option[String] = None) = {
+  protected def swiftOld(code: String, main: Option[String] = None) = {
     CodeExecAsString(RuntimeManifest(SWIFT, imagename(SWIFT), deprecated = Some(true)), trim(code), main.map(_.trim))
   }
 
+  protected def swift(code: String, main: Option[String] = None) = {
+    val attachment = attFmt[String].read(code.trim.toJson)
+    val manifest = ExecManifest.runtimesManifest.resolveDefaultRuntime(SWIFT).get
+
+    CodeExecAsAttachment(manifest, attachment, main.map(_.trim), Exec.isBinaryCode(code))
+  }
+
   protected def swift3(code: String, main: Option[String] = None) = {
-    val default = ExecManifest.runtimesManifest.resolveDefaultRuntime(SWIFT3).flatMap(_.default)
-    CodeExecAsString(
-      RuntimeManifest(SWIFT3, imagename(SWIFT3), default = default, deprecated = Some(false)),
-      trim(code),
-      main.map(_.trim))
+    val attachment = attFmt[String].read(code.trim.toJson)
+    val manifest = ExecManifest.runtimesManifest.resolveDefaultRuntime(SWIFT3).get
+
+    CodeExecAsAttachment(manifest, attachment, main.map(_.trim), Exec.isBinaryCode(code))
   }
 
   protected def sequence(components: Vector[FullyQualifiedEntityName]) = SequenceExec(components)
diff --git a/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala b/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala
new file mode 100644
index 0000000000..48a4b0d7d3
--- /dev/null
+++ b/tests/src/test/scala/whisk/core/entity/test/ExecTests.scala
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package whisk.core.entity.test
+
+import common.StreamLogging
+import spray.json._
+import org.junit.runner.RunWith
+import org.scalatest.junit.JUnitRunner
+import org.scalatest.{BeforeAndAfterAll, FlatSpec, Matchers}
+import whisk.core.WhiskConfig
+import whisk.core.entity.Attachments.{Attached, Inline}
+import whisk.core.entity.{CodeExecAsAttachment, CodeExecAsString, Exec, ExecManifest, WhiskAction}
+
+import scala.collection.mutable
+
+@RunWith(classOf[JUnitRunner])
+class ExecTests extends FlatSpec with Matchers with StreamLogging with BeforeAndAfterAll {
+  behavior of "exec deserialization"
+
+  val config = new WhiskConfig(ExecManifest.requiredProperties)
+  ExecManifest.initialize(config)
+
+  override protected def afterAll(): Unit = {
+    ExecManifest.initialize(config)
+    super.afterAll()
+  }
+
+  it should "read existing code string as attachment" in {
+    val json = """{
+                 |  "name": "action_tests_name2",
+                 |  "_id": "anon-Yzycx8QnIYDp3Tby0Fnj23KcMtH/action_tests_name2",
+                 |  "publish": false,
+                 |  "annotations": [],
+                 |  "version": "0.0.1",
+                 |  "updated": 1533623651650,
+                 |  "entityType": "action",
+                 |  "exec": {
+                 |    "kind": "nodejs:6",
+                 |    "code": "foo",
+                 |    "binary": false
+                 |  },
+                 |  "parameters": [
+                 |    {
+                 |      "key": "x",
+                 |      "value": "b"
+                 |    }
+                 |  ],
+                 |  "limits": {
+                 |    "timeout": 60000,
+                 |    "memory": 256,
+                 |    "logs": 10
+                 |  },
+                 |  "namespace": "anon-Yzycx8QnIYDp3Tby0Fnj23KcMtH"
+                 |}""".stripMargin.parseJson.asJsObject
+    val action = WhiskAction.serdes.read(json)
+    action.exec should matchPattern { case CodeExecAsAttachment(_, Inline("foo"), None, false) => }
+  }
+
+  it should "properly determine binary property" in {
+    val j1 = """{
+               |  "kind": "nodejs:6",
+               |  "code": "SGVsbG8gT3BlbldoaXNr",
+               |  "binary": false
+               |}""".stripMargin.parseJson.asJsObject
+    Exec.serdes.read(j1) should matchPattern {
+      case CodeExecAsAttachment(_, Inline("SGVsbG8gT3BlbldoaXNr"), None, true) =>
+    }
+
+    val j2 = """{
+               |  "kind": "nodejs:6",
+               |  "code": "while (true)",
+               |  "binary": false
+               |}""".stripMargin.parseJson.asJsObject
+    Exec.serdes.read(j2) should matchPattern {
+      case CodeExecAsAttachment(_, Inline("while (true)"), None, false) =>
+    }
+
+    //Defaults to binary
+    val j3 = """{
+               |  "kind": "nodejs:6",
+               |  "code": "while (true)"
+               |}""".stripMargin.parseJson.asJsObject
+    Exec.serdes.read(j3) should matchPattern {
+      case CodeExecAsAttachment(_, Inline("while (true)"), None, false) =>
+    }
+  }
+
+  it should "read code stored as attachment" in {
+    val json = """{
+                 |  "kind": "java",
+                 |  "code": {
+                 |    "attachmentName": "foo:bar",
+                 |    "attachmentType": "application/java-archive",
+                 |    "length": 32768,
+                 |    "digest": "sha256-foo"
+                 |  },
+                 |  "binary": true,
+                 |  "main": "hello"
+                 |}""".stripMargin.parseJson.asJsObject
+    Exec.serdes.read(json) should matchPattern {
+      case CodeExecAsAttachment(_, Attached("foo:bar", _, Some(32768), Some("sha256-foo")), Some("hello"), true) =>
+    }
+  }
+
+  it should "read code stored as jar property" in {
+    val j1 = """{
+               |  "kind": "nodejs:6",
+               |  "jar": "SGVsbG8gT3BlbldoaXNr",
+               |  "binary": false
+               |}""".stripMargin.parseJson.asJsObject
+    Exec.serdes.read(j1) should matchPattern {
+      case CodeExecAsAttachment(_, Inline("SGVsbG8gT3BlbldoaXNr"), None, true) =>
+    }
+  }
+
+  it should "read existing code string as string with old manifest" in {
+    val oldManifestJson =
+      """{
+        |  "runtimes": {
+        |    "nodejs": [
+        |      {
+        |        "kind": "nodejs:6",
+        |        "default": true,
+        |        "image": {
+        |          "prefix": "openwhisk",
+        |          "name": "nodejs6action",
+        |          "tag": "latest"
+        |        },
+        |        "deprecated": false,
+        |        "stemCells": [{
+        |          "count": 2,
+        |          "memory": "256 MB"
+        |        }]
+        |      }
+        |    ]
+        |  }
+        |}""".stripMargin.parseJson.compactPrint
+
+    val oldConfig =
+      new TestConfig(Map(WhiskConfig.runtimesManifest -> oldManifestJson), ExecManifest.requiredProperties)
+    ExecManifest.initialize(oldConfig)
+    val j1 = """{
+               |  "kind": "nodejs:6",
+               |  "code": "SGVsbG8gT3BlbldoaXNr",
+               |  "binary": false
+             |}""".stripMargin.parseJson.asJsObject
+
+    Exec.serdes.read(j1) should matchPattern {
+      case CodeExecAsString(_, "SGVsbG8gT3BlbldoaXNr", None) =>
+    }
+
+    //Reset config back
+    ExecManifest.initialize(config)
+  }
+
+  private class TestConfig(val props: Map[String, String], requiredProperties: Map[String, String])
+      extends WhiskConfig(requiredProperties) {
+    override protected def getProperties() = mutable.Map(props.toSeq: _*)
+  }
+}
diff --git a/tools/db/moveCodeToAttachment.py b/tools/db/moveCodeToAttachment.py
index be90eea99a..254362c9df 100755
--- a/tools/db/moveCodeToAttachment.py
+++ b/tools/db/moveCodeToAttachment.py
@@ -21,29 +21,8 @@
 import argparse
 import couchdb.client
 import time
-import base64
 from couchdb import ResourceNotFound
 
-def updateJavaAction(db, doc, id):
-    updated = False
-    attachment = db.get_attachment(doc, 'jarfile')
-
-    if attachment != None:
-        encodedAttachment = base64.b64encode(attachment.getvalue())
-        db.put_attachment(doc, encodedAttachment, 'codefile', 'text/plain')
-        doc = db.get(id)
-        doc['exec']['code'] = {
-            'attachmentName': 'codefile',
-            'attachmentType': 'text/plain'
-        }
-        if 'jar' in doc['exec']:
-            del doc['exec']['jar']
-        db.save(doc)
-        db.delete_attachment(doc, 'jarfile')
-        updated = True
-
-    return updated
-
 def updateNonJavaAction(db, doc, id):
     updated = False
     code = doc['exec']['code']
@@ -96,7 +75,7 @@ def main(args):
             if doc['exec']['kind'] != 'java':
                 updated = updateNonJavaAction(db, doc, id)
             else:
-                updated = updateJavaAction(db, doc, id)
+                updated = False
 
             if updated:
                 print('Updated action: "{0}"'.format(id))


 

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