fix: ensureAllExpectedParamsInPathAreDefined now iterates over the co…

…rrect set of maps for all path params
daveshanley · Jan 30, 2024 · 6e6e034 · 6e6e034
1 parent 62d8842
commit 6e6e034
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 31 deletions.
diff --git a/functions/openapi/path_parameters.go b/functions/openapi/path_parameters.go
@@ -208,20 +208,6 @@ func (pp PathParameters) RunRule(nodes []*yaml.Node, context model.RuleFunctionC
 					continue
 				}
 				if isVerb(r) {
-					if len(topLevelParams) <= 0 {
-						if verbLevelParams[r] == nil {
-							for pe := range pathElements {
-								err := fmt.Sprintf("`%s` must define parameter `%s` as expected by path `%s`",
-									strings.ToUpper(r), pe, currentPath)
-								res := model.BuildFunctionResultString(err)
-								res.StartNode = startNode
-								res.EndNode = endNode
-								res.Path = fmt.Sprintf("$.paths['%s'].%s", currentPath, r)
-								res.Rule = context.Rule
-								results = append(results, res)
-							}
-						}
-					}
 					pp.ensureAllExpectedParamsInPathAreDefined(currentPath, allPathParams,
 						pathElements, &results, startNode, endNode, context, r)
 				}
@@ -291,25 +277,22 @@ func (pp PathParameters) ensureAllDefinedPathParamsAreUsedInPath(path string, al
 func (pp PathParameters) ensureAllExpectedParamsInPathAreDefined(path string, allPathParams map[string]map[string][]string,
 	pathElements map[string]bool, results *[]model.RuleFunctionResult, startNode, endNode *yaml.Node,
 	context model.RuleFunctionContext, verb string) {
-	var top map[string][]string
-
+	var topParams map[string][]string
+	var verbParams map[string][]string
 	if allPathParams != nil {
-		top = allPathParams["top"]
+		topParams = allPathParams["top"]
+		verbParams = allPathParams[verb]
 	}
-	for k, e := range allPathParams {
-		if k == "top" {
-			continue
-		}
-		for p := range pathElements {
-			if !pp.segmentExistsInPathParams(p, e, top) {
-				err := fmt.Sprintf("`%s` must define parameter `%s` as expected by path `%s`", strings.ToUpper(verb), p, path)
-				res := model.BuildFunctionResultString(err)
-				res.StartNode = startNode
-				res.EndNode = endNode
-				res.Path = fmt.Sprintf("$.paths['%s']", path)
-				res.Rule = context.Rule
-				*results = append(*results, res)
-			}
+	// For each expected path parameter, check the top and verb-level defined parameters
+	for p := range pathElements {
+		if !pp.segmentExistsInPathParams(p, verbParams, topParams) {
+			err := fmt.Sprintf("`%s` must define parameter `%s` as expected by path `%s`", strings.ToUpper(verb), p, path)
+			res := model.BuildFunctionResultString(err)
+			res.StartNode = startNode
+			res.EndNode = endNode
+			res.Path = fmt.Sprintf("$.paths['%s']", path)
+			res.Rule = context.Rule
+			*results = append(*results, res)
 		}
 	}
 }

diff --git a/functions/openapi/path_parameters_test.go b/functions/openapi/path_parameters_test.go
@@ -20,6 +20,72 @@ func TestPathParameters_RunRule(t *testing.T) {
 	assert.Len(t, res, 0)
 }
 
+func TestPathParameters_RunRule_AllParamsInTop(t *testing.T) {
+
+	yml := `paths:
+  /pizza/{type}/{topping}:
+    parameters:
+      - name: type
+        in: path
+    get:
+      operationId: get_pizza`
+
+	path := "$"
+
+	var rootNode yaml.Node
+	mErr := yaml.Unmarshal([]byte(yml), &rootNode)
+	assert.NoError(t, mErr)
+
+	nodes, _ := utils.FindNodes([]byte(yml), path)
+
+	rule := buildOpenApiTestRuleAction(path, "path_parameters", "", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	config := index.CreateOpenAPIIndexConfig()
+	ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)
+
+	def := PathParameters{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 1)
+	assert.Equal(t, "`GET` must define parameter `topping` as expected by path `/pizza/{type}/{topping}`", res[0].Message)
+}
+
+func TestPathParameters_RunRule_VerbsWithDifferentParams(t *testing.T) {
+
+	yml := `paths:
+  /pizza/{type}/{topping}:
+    parameters:
+      - name: type
+        in: path
+    get:
+      parameters:
+        - name: topping
+          in: path
+      operationId: get_pizza
+    post:
+      operationId: make_pizza
+`
+
+	path := "$"
+
+	var rootNode yaml.Node
+	mErr := yaml.Unmarshal([]byte(yml), &rootNode)
+	assert.NoError(t, mErr)
+
+	nodes, _ := utils.FindNodes([]byte(yml), path)
+
+	rule := buildOpenApiTestRuleAction(path, "path_parameters", "", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	config := index.CreateOpenAPIIndexConfig()
+	ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)
+
+	def := PathParameters{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 1)
+	assert.Equal(t, "`POST` must define parameter `topping` as expected by path `/pizza/{type}/{topping}`", res[0].Message)
+}
+
 func TestPathParameters_RunRule_DuplicatePathCheck(t *testing.T) {
 
 	yml := `paths: