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

dubeejw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk-cli.git


The following commit(s) were added to refs/heads/master by this push:
     new 1c241dc  Fix Go regex for path parameter parsing (#246)
1c241dc is described below

commit 1c241dcb2b9ea2afe9dcece690d8e305634e034e
Author: Mark Deuser <[email protected]>
AuthorDate: Wed Mar 21 17:46:43 2018 -0400

    Fix Go regex for path parameter parsing (#246)
    
    * fix go regex
    update test to have a back-to-back path parameter
    
    * refactor to support creating unique parameter swagger entries
    
    * allow path parameters with any character except `/`
---
 commands/api.go                                    | 76 +++++++++++++---------
 commands/util.go                                   |  9 +++
 .../scala/whisk/core/cli/test/ApiGwCliTests.scala  | 69 ++++----------------
 3 files changed, 66 insertions(+), 88 deletions(-)

diff --git a/commands/api.go b/commands/api.go
index a79ab11..88322e5 100644
--- a/commands/api.go
+++ b/commands/api.go
@@ -43,7 +43,8 @@ const (
        formatOptionYaml = "yaml"
        formatOptionJson = "json"
 
-       pathParamRegex = `/\{([^/]+)}/|/\{([^/]+)}$`
+       pathParamRegex        = `\/\{([^\/]+)\}\/|\/\{([^\/]+)\}$|\{([^\/]+)}\/`
+       pathSegmentParamRegex = `^\/\{([^\/]+)\}\/$`
 )
 
 var apiCmd = &cobra.Command{
@@ -96,20 +97,34 @@ func isValidRelpath(relpath string) (error, bool) {
        return nil, true
 }
 
-func hasPathParameters(path string) (bool, error) {
-       matched, err := regexp.MatchString(pathParamRegex, path)
-       hasBracket := strings.ContainsRune(path, '{') || 
strings.ContainsRune(path, '}')
+func getPathParameterNames(path string) ([]string, error) {
+       var pathParameters []string
+
+       regexObj, err := regexp.Compile(pathSegmentParamRegex)
        if err != nil {
-               whisk.Debug(whisk.DbgInfo, "Unable to compile Regex '%s' to 
test against path '%s'\n", pathParamRegex, path)
-       }
-       if hasBracket && !matched {
-               errMsg := wski18n.T("Relative path '{{.path}}' does not include 
valid path parameters. Each parameter must be enclosed in curly braces '{}'.",
-                       map[string]interface{}{"path": path})
-               whiskErr := whisk.MakeWskError(errors.New(errMsg), 
whisk.EXIT_CODE_ERR_GENERAL,
-                       whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
-               return false, whiskErr
+               whisk.Debug(whisk.DbgError, "Failed to match path '%s' to 
regular expressions `%s`\n", path, pathSegmentParamRegex)
+       } else {
+               segments := strings.Split(path, "/")
+               for _, segment := range segments {
+                       segment = fmt.Sprintf("/%s/", segment)
+                       matchedItems := regexObj.FindAllStringSubmatch(segment, 
-1)
+                       for _, matchedParam := range matchedItems {
+                               for idx, paramName := range matchedParam {
+                                       whisk.Debug(whisk.DbgInfo, "Path 
parameter submatch '%v'; idx %v\n", paramName, idx)
+                                       if idx > 0 && len(paramName) > 0 {
+                                               pathParameters = 
append(pathParameters, paramName)
+                                       }
+                               }
+                       }
+               }
        }
-       return matched, nil
+
+       return pathParameters, err
+}
+
+func hasPathParameters(path string) (bool, error) {
+       pathParams, err := getPathParameterNames(path)
+       return len(pathParams) > 0, err
 }
 
 func isBasepathParameterized(basepath string) (error, bool) {
@@ -770,26 +785,27 @@ func getLargestApiNameSize(retApiArray 
*whisk.RetApiArray, apiPath string, apiVe
        return maxNameSize
 }
 
-func generatePathParameters(relativePath string) []whisk.ApiParameter {
+func generatePathParameters(relativePath string) ([]whisk.ApiParameter, error) 
{
        pathParams := []whisk.ApiParameter{}
-       regexObj, err := regexp.Compile(pathParamRegex)
-       if err != nil {
-               whisk.Debug(whisk.DbgError, "Failed to match path '%s' to 
regular expressions `%s`\n", relativePath, pathParamRegex)
-       }
-       matches := regexObj.FindAllString(relativePath, -1)
-       if matches != nil {
-               for _, paramName := range matches {
-                       //The next 3 lines clean up the paramName, as the 
matches are something like `/{param}`
-                       openIdx := strings.IndexRune(paramName, '{')
-                       closeIdx := strings.IndexRune(paramName, '}')
-                       paramName = string([]rune(paramName)[openIdx+1 : 
closeIdx])
-                       param := whisk.ApiParameter{Name: paramName, In: 
"path", Required: true, Type: "string",
-                               Description: wski18n.T("Default description for 
'{{.name}}'", map[string]interface{}{"name": paramName})}
+
+       pathParamNames, err := getPathParameterNames(relativePath)
+       if len(pathParamNames) > 0 && err == nil {
+               // Only create unique swagger entries
+               var uniqueParamNames []string
+               for _, name := range pathParamNames {
+                       if !contains(uniqueParamNames, name) {
+                               uniqueParamNames = append(uniqueParamNames, 
name)
+                       }
+               }
+               for _, uniqueName := range uniqueParamNames {
+                       whisk.Debug(whisk.DbgInfo, "Creating api parameter for 
'%s'\n", uniqueName)
+                       param := whisk.ApiParameter{Name: uniqueName, In: 
"path", Required: true, Type: "string",
+                               Description: wski18n.T("Default description for 
'{{.name}}'", map[string]interface{}{"name": uniqueName})}
                        pathParams = append(pathParams, param)
                }
        }
 
-       return pathParams
+       return pathParams, err
 }
 
 /*
@@ -907,10 +923,10 @@ func parseApi(cmd *cobra.Command, args []string) 
(*whisk.Api, *QualifiedName, er
        if !basepathArgIsApiName {
                api.Id = "API:" + api.Namespace + ":" + api.GatewayBasePath
        }
-       api.PathParameters = generatePathParameters(api.GatewayRelPath)
+       api.PathParameters, err = generatePathParameters(api.GatewayRelPath)
 
        whisk.Debug(whisk.DbgInfo, "Parsed api struct: %#v\n", api)
-       return api, qName, nil
+       return api, qName, err
 }
 
 func parseSwaggerApi() (*whisk.Api, error) {
diff --git a/commands/util.go b/commands/util.go
index 2fe6b5a..570a93e 100644
--- a/commands/util.go
+++ b/commands/util.go
@@ -1110,3 +1110,12 @@ func isApplicationError(err error) bool {
 
        return applicationError
 }
+
+func contains(arr []string, element string) bool {
+       for _, e := range arr {
+               if e == element {
+                       return true
+               }
+       }
+       return false
+}
diff --git a/tests/src/test/scala/whisk/core/cli/test/ApiGwCliTests.scala 
b/tests/src/test/scala/whisk/core/cli/test/ApiGwCliTests.scala
index 13ffc6a..3465815 100644
--- a/tests/src/test/scala/whisk/core/cli/test/ApiGwCliTests.scala
+++ b/tests/src/test/scala/whisk/core/cli/test/ApiGwCliTests.scala
@@ -37,60 +37,6 @@ class ApiGwCliTests extends ApiGwTests {
   override lazy val createCode = SUCCESS_EXIT
   behavior of "Cli Wsk api creation with path parameters no swagger"
 
-  it should "fail to create an API if the relative path contains invalid path 
parameters" in withAssetCleaner(wskprops) {(wp, assetHelper) =>
-    val actionName = "APIGWTEST_BAD_RELATIVE_PATH_ACTION"
-    val basePath = "/mybase/path"
-    val file = TestUtils.getTestActionFilename(s"echo-web-http.js")
-    assetHelper.withCleaner(wsk.action, actionName, confirmDelete = true) {
-      (action, _) =>
-        action.create(actionName, Some(file), web = Some("true"))
-    }
-    var relPath = "/bad/{path/value"
-    var rr = apiCreate(basepath = Some(basePath),
-      relpath = Some(relPath),
-      operation = Some("GET"),
-      action = Some(actionName),
-      expectedExitCode = ANY_ERROR_EXIT)
-    rr.stderr should include(
-      s"Relative path '${relPath}' does not include valid path parameters. 
Each parameter must be enclosed in curly braces '{}'.")
-
-    relPath = "/bad/path}/value"
-    rr = apiCreate(basepath = Some(basePath),
-      relpath = Some(relPath),
-      operation = Some("GET"),
-      action = Some(actionName),
-      expectedExitCode = ANY_ERROR_EXIT)
-    rr.stderr should include(
-      s"Relative path '${relPath}' does not include valid path parameters. 
Each parameter must be enclosed in curly braces '{}'.")
-
-    relPath = "/bad/{path/va}lue"
-    rr = apiCreate(basepath = Some(basePath),
-      relpath = Some(relPath),
-      operation = Some("GET"),
-      action = Some(actionName),
-      expectedExitCode = ANY_ERROR_EXIT)
-    rr.stderr should include(
-      s"Relative path '${relPath}' does not include valid path parameters. 
Each parameter must be enclosed in curly braces '{}'.")
-
-    relPath = "/ba}d/{path/value"
-    rr = apiCreate(basepath = Some(basePath),
-      relpath = Some(relPath),
-      operation = Some("GET"),
-      action = Some(actionName),
-      expectedExitCode = ANY_ERROR_EXIT)
-    rr.stderr should include(
-      s"Relative path '${relPath}' does not include valid path parameters. 
Each parameter must be enclosed in curly braces '{}'.")
-
-    relPath = "/ba}d/{p{at}h/value"
-    rr = apiCreate(basepath = Some(basePath),
-      relpath = Some(relPath),
-      operation = Some("GET"),
-      action = Some(actionName),
-      expectedExitCode = ANY_ERROR_EXIT)
-    rr.stderr should include(
-      s"Relative path '${relPath}' does not include valid path parameters. 
Each parameter must be enclosed in curly braces '{}'.")
-  }
-
   it should "fail to create an API if the base path contains path parameters" 
in withAssetCleaner(wskprops) {(wp, assetHelper) =>
     val actionName = "APIGWTEST_BAD_BASE_PATH_ACTION"
     val basePath = "/mybase/{path}"
@@ -193,10 +139,12 @@ class ApiGwCliTests extends ApiGwTests {
     wskprops) { (wp, assetHelper) =>
     val testName = "CLI_APIGWTEST_PATH_PARAMS2"
     val testBasePath = "/" + testName + "_bp"
-    val testRelPath = "/path/{with}/some/{path}"
+    val testRelPath = "/path/{with}/some/{double}/{extra}/{extra}/{path}"
     val testUrlName1 = "scooby"
     val testUrlName2 = "doo"
-    val testRelPathGet = s"/path/${testUrlName1}/some/$testUrlName2"
+    val testUrlName3 = "shaggy"
+    val testUrlName4 = "velma"
+    val testRelPathGet = 
s"/path/$testUrlName1/some/$testUrlName3/$testUrlName4/$testUrlName4/$testUrlName2"
     val testUrlOp = "get"
     val testApiName = testName + " API Name"
     val actionName = testName + "_action"
@@ -219,6 +167,7 @@ class ApiGwCliTests extends ApiGwTests {
       )
       verifyApiCreated(rr)
       val swaggerApiUrl = getSwaggerUrl(rr).replace("{with}", 
testUrlName1).replace("{path}", testUrlName2)
+          .replace("{double}", testUrlName3).replace("{extra}", testUrlName4)
 
       //Validate the api created contained parameters and they were correct
       rr = apiGet(basepathOrApiName = Some(testApiName))
@@ -227,9 +176,13 @@ class ApiGwCliTests extends ApiGwTests {
       rr.stdout should include regex 
(""""cors":\s*\{\s*\n\s*"enabled":\s*true""")
       rr.stdout should include regex 
(s"""target-url.*${actionName}.http${reqPath}""")
       val params = getParametersFromJson(rr, testRelPath)
-      params.size should be(2)
+
+      // should have 4, not 5 parameter definitions (i.e. don't define "extra" 
twice
+      params.size should be(4)
       validateParameter(params(0), "with", "path", true, "string", "Default 
description for 'with'")
-      validateParameter(params(1), "path", "path", true, "string", "Default 
description for 'path'")
+      validateParameter(params(1), "double", "path", true, "string", "Default 
description for 'double'")
+      validateParameter(params(2), "extra", "path", true, "string", "Default 
description for 'extra'")
+      validateParameter(params(3), "path", "path", true, "string", "Default 
description for 'path'")
 
       //Lets call the swagger url so we can make sure the response is valid 
and contains our path in the ow path
       val apiToInvoke = s"$swaggerApiUrl"

-- 
To stop receiving notification emails like this one, please contact
[email protected].

Reply via email to