Improved logic of fetching actions

Author: Bhavesh Kemburu

common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala

@@ -231,7 +231,7 @@ object Messages {
   val resourceProvisionError = "Failed to provision resources to run the action."
 
   def forbiddenGetActionBinding(entityDocId: String) =
-    s"GET not permitted for '$entityDocId'. Resource does not exist or an action in a shared package binding."
+    s"GET not permitted for '$entityDocId'. Resource does not exist or is an action in a shared package binding."
   def forbiddenGetAction(entityPath: String) =
     s"GET not permitted for '$entityPath' since it's an action in a shared package"
   def forbiddenGetPackageBinding(packageName: String) =

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala

@@ -106,9 +106,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
 
   /** Config flag for Execute Only for Actions in Shared Packages */
   protected def executeOnly =
-    Try({
-      loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
-    }).getOrElse(false)
+    loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
 
   /** Entity normalizer to JSON object. */
   import RestApiCommons.emptyEntityToJsObject
@@ -338,6 +336,56 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
     deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
   }
 
+  /**GET --save which retrieves all source code for an entity **/
+  private def getEntityWithSourceCode(entityName: FullyQualifiedEntityName, env: Option[Parameters])(
+    implicit transid: TransactionId) = {
+    getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction =>
+      val mergedAction = env map {
+        action inherit _
+      } getOrElse action
+      complete(OK, mergedAction)
+    })
+  }
+
+  /**Standard GET which just retrieves metadata**/
+  private def getEntityMetaData(entityName: FullyQualifiedEntityName, env: Option[Parameters])(
+    implicit transid: TransactionId) = {
+    getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
+      action: WhiskActionMetaData =>
+        val mergedAction = env map {
+          action inherit _
+        } getOrElse action
+        complete(OK, mergedAction)
+    })
+  }
+
+  /** Checks for package binding case. we don't want to allow get for a package binding in shared package */
+  private def fetchEntity(entityName: FullyQualifiedEntityName, env: Option[Parameters], code: Boolean)(
+    implicit transid: TransactionId) = {
+    if (entityName.path.defaultPackage) {
+      if (code) {
+        getEntityWithSourceCode(entityName, env)
+      } else {
+        getEntityMetaData(entityName, env)
+      }
+    } else {
+      getEntity(
+        WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
+        Some { pkg: WhiskPackage =>
+          val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
+          if (executeOnly && originalPackageLocation != entityName.namespace) {
+            terminate(Forbidden, forbiddenGetActionBinding(entityName.toDocId.asString))
+          } else {
+            if (code) {
+              getEntityWithSourceCode(entityName, env)
+            } else {
+              getEntityMetaData(entityName, env)
+            }
+          }
+        })
+    }
+  }
+
   /**
    * Gets action. The action name is prefixed with the namespace to create the primary index key.
    *
@@ -351,67 +399,10 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
     parameter('code ? true) { code =>
       //check if execute only is enabled, and if there is a discrepancy between the current user's namespace
       //and that of the entity we are trying to fetch
-
       if (executeOnly && user.namespace.name != entityName.namespace) {
         terminate(Forbidden, forbiddenGetAction(entityName.path.asString))
       } else {
-        code match {
-          case true =>
-            //Resolve Binding(Package) of the action
-            if (entityName.path.defaultPackage) {
-              getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some {
-                action: WhiskAction =>
-                  val mergedAction = env map {
-                    action inherit _
-                  } getOrElse action
-                  complete(OK, mergedAction)
-              })
-            } else {
-              getEntity(
-                WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
-                Some { pkg: WhiskPackage =>
-                  val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
-                  if (executeOnly && originalPackageLocation != entityName.namespace) {
-                    terminate(Forbidden, forbiddenGetActionBinding(entityName.toDocId.asString))
-                  } else {
-                    getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some {
-                      action: WhiskAction =>
-                        val mergedAction = env map {
-                          action inherit _
-                        } getOrElse action
-                        complete(OK, mergedAction)
-                    })
-                  }
-                })
-            }
-          case false =>
-            if (entityName.path.defaultPackage) {
-              getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
-                action: WhiskActionMetaData =>
-                  val mergedAction = env map {
-                    action inherit _
-                  } getOrElse action
-                  complete(OK, mergedAction)
-              })
-            } else {
-              getEntity(
-                WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
-                Some { pkg: WhiskPackage =>
-                  val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
-                  if (executeOnly && originalPackageLocation != entityName.namespace) {
-                    terminate(Forbidden, forbiddenGetActionBinding(entityName.toDocId.asString))
-                  } else {
-                    getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
-                      action: WhiskActionMetaData =>
-                        val mergedAction = env map {
-                          action inherit _
-                        } getOrElse action
-                        complete(OK, mergedAction)
-                    })
-                  }
-                })
-            }
-        }
+        fetchEntity(entityName, env, code)
       }
     }
   }

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala

@@ -18,7 +18,7 @@
 package org.apache.openwhisk.core.controller
 
 import scala.concurrent.Future
-import scala.util.{Failure, Success, Try}
+import scala.util.{Failure, Success}
 import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
 import akka.http.scaladsl.model.StatusCodes._
 import akka.http.scaladsl.server.{RequestContext, RouteResult}
@@ -45,9 +45,7 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
 
   /** Config flag for Execute Only for Shared Packages */
   protected def executeOnly =
-    Try({
-      loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
-    }).getOrElse(false)
+    loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
 
   /** Notification service for cache invalidation. */
   protected implicit val cacheChangeNotification: Some[CacheChangeNotification]
@@ -321,10 +319,7 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
         } else {
 
           /** Here's where I check package execute only case with package binding. */
-          val packagePath = wp.namespace.asString
-          val bindingPath = wp.binding.iterator.next().toString
-          val indexOfSlash = bindingPath.indexOf('/')
-          if (executeOnly && packagePath != bindingPath.take(indexOfSlash)) {
+          if (executeOnly && wp.namespace.asString != b.namespace.asString) {
             terminate(Forbidden, forbiddenGetPackageBinding(wp.name.asString))
           } else {
             getEntity(WhiskPackage.get(entityStore, docid), Some {