You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by du...@apache.org on 2018/03/21 21:46:46 UTC
[incubator-openwhisk-cli] branch master updated: Fix Go regex for
path parameter parsing (#246)
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 <md...@us.ibm.com>
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
dubeejw@apache.org.