Updated libopenapi to v0.6.0

The index is more powerful now, but some of the index signatures changes for parameters, that needed updating. No changes to any vacuum signatures or anything else, a non breaking change.
daveshanley · Feb 23, 2023 · fe5b437 · fe5b437
1 parent 34d89a6
commit fe5b437
Show file tree

Hide file tree

Showing 8 changed files with 225 additions and 155 deletions.
diff --git a/functions/openapi/formdata_consume_check.go b/functions/openapi/formdata_consume_check.go
@@ -35,7 +35,7 @@ func (fd FormDataConsumeCheck) RunRule(nodes []*yaml.Node, context model.RuleFun
 	opParams := context.Index.GetAllParametersFromOperations()
 
 	for path, methodMap := range paths {
-		var topParams map[string]*index.Reference
+		var topParams map[string][]*index.Reference
 
 		// check for top params
 		if opParams[path]["top"] != nil {
@@ -63,11 +63,10 @@ func (fd FormDataConsumeCheck) RunRule(nodes []*yaml.Node, context model.RuleFun
 	return results
 }
 
-func (fd FormDataConsumeCheck) paramCheck(paramMap map[string]*index.Reference, consumesNode *yaml.Node,
+func (fd FormDataConsumeCheck) paramCheck(paramMap map[string][]*index.Reference, consumesNode *yaml.Node,
 	results []model.RuleFunctionResult, path string, method string, context model.RuleFunctionContext, top bool) []model.RuleFunctionResult {
 
 	for paramName, paramNode := range paramMap {
-
 		if paramNode == nil {
 			results = append(results, model.RuleFunctionResult{
 				Message:   fmt.Sprintf("paramter value for '%s' is empty / missing", paramName),
@@ -79,48 +78,51 @@ func (fd FormDataConsumeCheck) paramCheck(paramMap map[string]*index.Reference,
 			continue
 		}
 
-		inNodeStart, inNode := utils.FindKeyNode("in", paramNode.Node.Content)
-		if inNode != nil && inNode.Value == "formData" {
+		for r := range paramNode {
+			inNodeStart, inNode := utils.FindKeyNode("in", paramNode[r].Node.Content)
+			if inNode != nil && inNode.Value == "formData" {
 
-			pathString := fmt.Sprintf("$.%s.%s.parameters", path, method)
-			if top {
-				pathString = fmt.Sprintf("$.%s.parameters", path)
-			}
+				pathString := fmt.Sprintf("$.%s.%s.parameters", path, method)
+				if top {
+					pathString = fmt.Sprintf("$.%s.parameters", path)
+				}
 
-			// using formData without a consumes sequence.
-			if consumesNode == nil {
-				results = append(results, model.RuleFunctionResult{
-					Message:   fmt.Sprintf("in:formData param '%s' used without 'consumes' defined", paramName),
-					StartNode: inNodeStart,
-					EndNode:   inNode,
-					Path:      pathString,
-					Rule:      context.Rule,
-				})
-			}
+				// using formData without a consumes sequence.
+				if consumesNode == nil {
+					results = append(results, model.RuleFunctionResult{
+						Message:   fmt.Sprintf("in:formData param '%s' used without 'consumes' defined", paramName),
+						StartNode: inNodeStart,
+						EndNode:   inNode,
+						Path:      pathString,
+						Rule:      context.Rule,
+					})
+				}
 
-			validConsumer := false
-			if consumesNode != nil {
-				for _, consumeNode := range consumesNode.Content {
-					switch consumeNode.Value {
-					case "application/x-www-form-urlencoded":
-						validConsumer = true
-					case "multipart/form-data":
-						validConsumer = true
+				validConsumer := false
+				if consumesNode != nil {
+					for _, consumeNode := range consumesNode.Content {
+						switch consumeNode.Value {
+						case "application/x-www-form-urlencoded":
+							validConsumer = true
+						case "multipart/form-data":
+							validConsumer = true
+						}
 					}
 				}
-			}
 
-			if !validConsumer {
-				results = append(results, model.RuleFunctionResult{
-					Message: fmt.Sprintf("in:formData param '%s' parameter must include 'application/x-www-form-urlencoded'"+
-						" or 'multipart/form-data' in their 'consumes' property", paramName),
-					StartNode: inNodeStart,
-					EndNode:   inNode,
-					Path:      pathString,
-					Rule:      context.Rule,
-				})
+				if !validConsumer {
+					results = append(results, model.RuleFunctionResult{
+						Message: fmt.Sprintf("in:formData param '%s' parameter must include 'application/x-www-form-urlencoded'"+
+							" or 'multipart/form-data' in their 'consumes' property", paramName),
+						StartNode: inNodeStart,
+						EndNode:   inNode,
+						Path:      pathString,
+						Rule:      context.Rule,
+					})
+				}
 			}
 		}
+
 	}
 	return results
 }
diff --git a/functions/openapi/operation_parameters.go b/functions/openapi/operation_parameters.go
@@ -6,6 +6,7 @@ package openapi
 import (
 	"fmt"
 	"github.com/daveshanley/vacuum/model"
+	"github.com/pb33f/libopenapi/index"
 	"github.com/pb33f/libopenapi/utils"
 	"gopkg.in/yaml.v3"
 	"strings"
@@ -32,12 +33,14 @@ func (op OperationParameters) RunRule(nodes []*yaml.Node, context model.RuleFunc
 	var results []model.RuleFunctionResult
 
 	// add any param indexing errors already found.
-	for _, paramIndexError := range context.Index.GetOperationParametersIndexErrors() {
+	errs := context.Index.GetOperationParametersIndexErrors()
+	for n, _ := range errs {
+		er := errs[n].(*index.IndexingError)
 		results = append(results, model.RuleFunctionResult{
-			Message:   paramIndexError.Error.Error(),
-			StartNode: paramIndexError.Node,
-			EndNode:   paramIndexError.Node,
-			Path:      paramIndexError.Path,
+			Message:   er.Error(),
+			StartNode: er.Node,
+			EndNode:   er.Node,
+			Path:      er.Path,
 			Rule:      context.Rule,
 		})
 	}
@@ -53,8 +56,9 @@ func (op OperationParameters) RunRule(nodes []*yaml.Node, context model.RuleFunc
 
 			resultPath := fmt.Sprintf("$.paths.%s.%s.parameters", path, currentVerb)
 
-			for key, param := range methodNode {
+			for key, params := range methodNode {
 
+				// TODO: come back and re-visit this code
 				if strings.Contains(key, "~1") {
 					results = append(results, model.RuleFunctionResult{
 						Message: fmt.Sprintf("There is a `~1` character in this `%s` operation at '%s",
@@ -67,66 +71,51 @@ func (op OperationParameters) RunRule(nodes []*yaml.Node, context model.RuleFunc
 					continue
 				}
 
-				// check for crazy
-
-				if param == nil {
+				for _, param := range params {
+					_, paramInNode := utils.FindKeyNode("in", param.Node.Content)
 					startNode := param.Node
 					endNode := utils.FindLastChildNode(startNode)
 
-					results = append(results, model.RuleFunctionResult{
-						Message: fmt.Sprintf("the `%s` operation at path `%s` contains an "+
-							"empty parameter", currentVerb, currentPath),
-						StartNode: startNode,
-						EndNode:   endNode,
-						Path:      resultPath,
-						Rule:      context.Rule,
-					})
-					continue
-				}
-				_, paramInNode := utils.FindKeyNode("in", param.Node.Content)
-
-				startNode := param.Node
-				endNode := utils.FindLastChildNode(startNode)
-
-				if paramInNode != nil {
-					if seenParamInLocations[paramInNode.Value] {
-						if paramInNode.Value == "body" {
-							results = append(results, model.RuleFunctionResult{
-								Message: fmt.Sprintf("the `%s` operation at path `%s` contains a "+
-									"duplicate param in:body definition", currentVerb, currentPath),
-								StartNode: startNode,
-								EndNode:   endNode,
-								Path:      resultPath,
-								Rule:      context.Rule,
-							})
-						}
-					} else {
-						if paramInNode.Value == "body" || paramInNode.Value == "formData" {
-							if seenParamInLocations["formData"] || seenParamInLocations["body"] {
+					if paramInNode != nil {
+						if seenParamInLocations[paramInNode.Value] {
+							if paramInNode.Value == "body" {
 								results = append(results, model.RuleFunctionResult{
-									Message: fmt.Sprintf("the `%s` operation at path `%s` "+
-										"contains parameters using both in:body and in:formData",
-										currentVerb, currentPath),
+									Message: fmt.Sprintf("the `%s` operation at path `%s` contains a "+
+										"duplicate param in:body definition", currentVerb, currentPath),
 									StartNode: startNode,
 									EndNode:   endNode,
 									Path:      resultPath,
 									Rule:      context.Rule,
 								})
 							}
+						} else {
+							if paramInNode.Value == "body" || paramInNode.Value == "formData" {
+								if seenParamInLocations["formData"] || seenParamInLocations["body"] {
+									results = append(results, model.RuleFunctionResult{
+										Message: fmt.Sprintf("the `%s` operation at path `%s` "+
+											"contains parameters using both in:body and in:formData",
+											currentVerb, currentPath),
+										StartNode: startNode,
+										EndNode:   endNode,
+										Path:      resultPath,
+										Rule:      context.Rule,
+									})
+								}
+							}
+							seenParamInLocations[paramInNode.Value] = true
 						}
-						seenParamInLocations[paramInNode.Value] = true
-					}
-				} else {
-					rfr := model.RuleFunctionResult{
-						Message: fmt.Sprintf("the `%s` operation at path `%s` contains a "+
-							"parameter with no `in` value", currentVerb, currentPath),
-						StartNode: startNode,
-						EndNode:   endNode,
-						Path:      resultPath,
-						Rule:      context.Rule,
-					}
-					results = append(results, rfr)
+					} else {
+						rfr := model.RuleFunctionResult{
+							Message: fmt.Sprintf("the `%s` operation at path `%s` contains a "+
+								"parameter with no `in` value", currentVerb, currentPath),
+							StartNode: startNode,
+							EndNode:   endNode,
+							Path:      resultPath,
+							Rule:      context.Rule,
+						}
+						results = append(results, rfr)
 
+					}
 				}
 			}
 		}

diff --git a/functions/openapi/operation_parameters_test.go b/functions/openapi/operation_parameters_test.go
@@ -86,7 +86,7 @@ func TestOperationParameters_RunRule_MissingName(t *testing.T) {
 	assert.Equal(t, "the 'get' operation parameter at path '/users/{id}', index 1 has no 'name' value", res[0].Message)
 }
 
-func TestOperationParameters_RunRule_DuplicateId(t *testing.T) {
+func TestOperationParameters_RunRule_DuplicateIdButDifferentInType(t *testing.T) {
 
 	yml := `paths:
   /users/{id}:
@@ -112,11 +112,40 @@ func TestOperationParameters_RunRule_DuplicateId(t *testing.T) {
 	def := OperationParameters{}
 	res := def.RunRule(nodes, ctx)
 
+	assert.Len(t, res, 0)
+}
+
+func TestOperationParameters_RunRule_DuplicateIdSameInType(t *testing.T) {
+
+	yml := `paths:
+  /users/{id}:
+    get:
+      parameters:
+        - in: path
+          name: id
+        - in: path
+          name: id`
+
+	path := "$.paths"
+
+	var rootNode yaml.Node
+	mErr := yaml.Unmarshal([]byte(yml), &rootNode)
+	assert.NoError(t, mErr)
+
+	nodes, _ := utils.FindNodes([]byte(yml), path)
+
+	rule := buildOpenApiTestRuleAction(path, "operation-parameters", "", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	ctx.Index = index.NewSpecIndex(&rootNode)
+
+	def := OperationParameters{}
+	res := def.RunRule(nodes, ctx)
+
 	assert.Len(t, res, 1)
-	assert.Equal(t, "the `get` operation parameter at path `/users/{id}`, index 1 has a duplicate name `id`", res[0].Message)
+	assert.Equal(t, "the `get` operation parameter at path `/users/{id}`, index 1 has a duplicate name `id` and `in` type", res[0].Message)
 }
 
-func TestOperationParameters_RunRule_DuplicateId_MultipleVerbs(t *testing.T) {
+func TestOperationParameters_RunRule_DuplicateId_MultipleVerbsDifferentInTypes(t *testing.T) {
 
 	yml := `paths:
   /users/{id}:
@@ -148,8 +177,43 @@ func TestOperationParameters_RunRule_DuplicateId_MultipleVerbs(t *testing.T) {
 	def := OperationParameters{}
 	res := def.RunRule(nodes, ctx)
 
+	assert.Len(t, res, 0)
+}
+
+func TestOperationParameters_RunRule_DuplicateId_MultipleVerbs(t *testing.T) {
+
+	yml := `paths:
+  /users/{id}:
+    get:
+      parameters:
+        - in: path
+          name: id
+        - in: path
+          name: id
+    post:
+      parameters:
+        - in: path
+          name: id
+        - in: query
+          name: winter`
+
+	path := "$.paths"
+
+	var rootNode yaml.Node
+	mErr := yaml.Unmarshal([]byte(yml), &rootNode)
+	assert.NoError(t, mErr)
+
+	nodes, _ := utils.FindNodes([]byte(yml), path)
+
+	rule := buildOpenApiTestRuleAction(path, "operation-parameters", "", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	ctx.Index = index.NewSpecIndex(&rootNode)
+
+	def := OperationParameters{}
+	res := def.RunRule(nodes, ctx)
+
 	assert.Len(t, res, 1)
-	assert.Equal(t, "the `get` operation parameter at path `/users/{id}`, index 1 has a duplicate name `id`", res[0].Message)
+	assert.Equal(t, "the `get` operation parameter at path `/users/{id}`, index 1 has a duplicate name `id` and `in` type", res[0].Message)
 }
 
 func TestOperationParameters_RunRule_DuplicateInBody(t *testing.T) {

diff --git a/functions/openapi/parameter_description.go b/functions/openapi/parameter_description.go
@@ -38,8 +38,8 @@ func (pd ParameterDescription) RunRule(nodes []*yaml.Node, context model.RuleFun
 	// look through top level params first.
 	for id, param := range params {
 		// only check if the param has an 'in' property.
-		_, in := utils.FindKeyNode("in", param.Node.Content)
-		_, desc := utils.FindKeyNode("description", param.Node.Content)
+		_, in := utils.FindKeyNodeTop("in", param.Node.Content)
+		_, desc := utils.FindKeyNodeTop("description", param.Node.Content)
 		lastNode := utils.FindLastChildNode(param.Node)
 
 		if in != nil {
@@ -59,22 +59,24 @@ func (pd ParameterDescription) RunRule(nodes []*yaml.Node, context model.RuleFun
 	// look through all parameters from operations.
 	for path, methodMap := range opParams {
 		for method, paramMap := range methodMap {
-			for pName, param := range paramMap {
-				if param != nil && param.Node != nil {
-					_, in := utils.FindKeyNode("in", param.Node.Content)
-					_, desc := utils.FindKeyNode("description", param.Node.Content)
-					lastNode := utils.FindLastChildNode(param.Node)
+			for pName, opParam := range paramMap {
+				for _, param := range opParam {
+					if param != nil && param.Node != nil {
+						_, in := utils.FindKeyNodeTop("in", param.Node.Content)
+						_, desc := utils.FindKeyNodeTop("description", param.Node.Content)
+						lastNode := utils.FindLastChildNode(param.Node)
 
-					if in != nil {
-						if desc == nil || desc.Value == "" {
-							pathString := fmt.Sprintf("$.paths.%s.%s.parameters", path, method)
-							results = append(results, model.RuleFunctionResult{
-								Message:   fmt.Sprintf(msg, pName),
-								StartNode: param.Node,
-								EndNode:   lastNode,
-								Path:      pathString,
-								Rule:      context.Rule,
-							})
+						if in != nil {
+							if desc == nil || desc.Value == "" {
+								pathString := fmt.Sprintf("$.paths.%s.%s.parameters", path, method)
+								results = append(results, model.RuleFunctionResult{
+									Message:   fmt.Sprintf(msg, pName),
+									StartNode: param.Node,
+									EndNode:   lastNode,
+									Path:      pathString,
+									Rule:      context.Rule,
+								})
+							}
 						}
 					}
 				}