From 19444bab09617c521c8e196ca7401676dbab4588 Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 26 Feb 2024 09:19:36 -0500 Subject: [PATCH 01/12] moved to key node for media types Signed-off-by: quobix --- functions/openapi/examples_missing.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/functions/openapi/examples_missing.go b/functions/openapi/examples_missing.go index 617c96b7..d8f6caf4 100644 --- a/functions/openapi/examples_missing.go +++ b/functions/openapi/examples_missing.go @@ -82,7 +82,7 @@ func (em ExamplesMissing) RunRule(_ []*yaml.Node, context model.RuleFunctionCont results = append(results, buildResult(vacuumUtils.SuppliedOrDefault(context.Rule.Message, "parameter is missing `examples` or `example`"), p.GenerateJSONPath(), - p.Value.GoLow().RootNode, p)) + p.Value.GoLow().KeyNode, p)) } } } @@ -112,7 +112,7 @@ func (em ExamplesMissing) RunRule(_ []*yaml.Node, context model.RuleFunctionCont results = append(results, buildResult(vacuumUtils.SuppliedOrDefault(context.Rule.Message, "media type is missing `examples` or `example`"), mt.GenerateJSONPath(), - mt.Value.GoLow().RootNode, mt)) + mt.Value.GoLow().KeyNode, mt)) } } } From 775f18f12bef22ea7eac0e795be1057d19330e17 Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 26 Feb 2024 09:19:57 -0500 Subject: [PATCH 02/12] moved disgnostics into reusable function Signed-off-by: quobix --- language-server/server.go | 69 +++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/language-server/server.go b/language-server/server.go index f47a8958..ccbb9b0c 100644 --- a/language-server/server.go +++ b/language-server/server.go @@ -91,7 +91,6 @@ func NewServer(version string, lintRequest *utils.LintFileRequest) *ServerState handler.TextDocumentDidClose = func(context *glsp.Context, params *protocol.DidCloseTextDocumentParams) error { state.documentStore.Remove(params.TextDocument.URI) - return nil } @@ -108,7 +107,6 @@ func (s *ServerState) Run() error { func (s *ServerState) runDiagnostic(doc *Document, notify glsp.NotifyFunc, delay bool) { go func() { - var diagnostics []protocol.Diagnostic if s.lintRequest.BaseFlag == "" { s.lintRequest.BaseFlag = filepath.Dir(strings.TrimPrefix(doc.URI, "file://")) @@ -126,27 +124,7 @@ func (s *ServerState) runDiagnostic(doc *Document, notify glsp.NotifyFunc, delay SkipDocumentCheck: s.lintRequest.SkipCheckFlag, Logger: s.lintRequest.Logger, }) - - for _, vacuumResult := range result.Results { - severity := getDiagnosticSeverityFromRule(vacuumResult.Rule) - diagnosticErrorHref := fmt.Sprintf("%s/rules/%s/%s", model.WebsiteUrl, - strings.ToLower(vacuumResult.Rule.RuleCategory.Id), - strings.ReplaceAll(strings.ToLower(vacuumResult.Rule.Id), "$", "")) - diagnostics = append(diagnostics, protocol.Diagnostic{ - Range: protocol.Range{ - Start: protocol.Position{Line: protocol.UInteger(vacuumResult.StartNode.Line - 1), - Character: protocol.UInteger(vacuumResult.StartNode.Column - 1)}, - End: protocol.Position{Line: protocol.UInteger(vacuumResult.EndNode.Line - 1), - Character: protocol.UInteger(vacuumResult.EndNode.Column + len(vacuumResult.EndNode.Value) - 1)}, - }, - Severity: &severity, - Source: &serverName, - Code: &protocol.IntegerOrString{Value: vacuumResult.Rule.Id}, - CodeDescription: &protocol.CodeDescription{HRef: diagnosticErrorHref}, - Message: vacuumResult.Message, - }) - - } + diagnostics := ConvertResultsIntoDiagnostics(result) if len(diagnostics) > 0 { go notify(protocol.ServerTextDocumentPublishDiagnostics, protocol.PublishDiagnosticsParams{ URI: doc.URI, @@ -156,7 +134,50 @@ func (s *ServerState) runDiagnostic(doc *Document, notify glsp.NotifyFunc, delay }() } -func getDiagnosticSeverityFromRule(rule *model.Rule) protocol.DiagnosticSeverity { +func ConvertResultsIntoDiagnostics(result *motor.RuleSetExecutionResult) []protocol.Diagnostic { + var diagnostics []protocol.Diagnostic + for _, vacuumResult := range result.Results { + diagnostics = append(diagnostics, ConvertResultIntoDiagnostic(&vacuumResult)) + + } + return diagnostics +} + +func ConvertResultIntoDiagnostic(vacuumResult *model.RuleFunctionResult) protocol.Diagnostic { + severity := GetDiagnosticSeverityFromRule(vacuumResult.Rule) + diagnosticErrorHref := fmt.Sprintf("%s/rules/%s/%s", model.WebsiteUrl, + strings.ToLower(vacuumResult.Rule.RuleCategory.Id), + strings.ReplaceAll(strings.ToLower(vacuumResult.Rule.Id), "$", "")) + + startLine := 1 + startChar := 1 + endLine := 1 + endChar := 1 + if vacuumResult.StartNode.Line > 0 { + startLine = vacuumResult.StartNode.Line - 1 + startChar = vacuumResult.StartNode.Column - 1 + } + if vacuumResult.EndNode.Line > 0 { + endLine = vacuumResult.EndNode.Line - 1 + endChar = vacuumResult.EndNode.Column - 1 + } + + return protocol.Diagnostic{ + Range: protocol.Range{ + Start: protocol.Position{Line: protocol.UInteger(startLine), + Character: protocol.UInteger(startChar)}, + End: protocol.Position{Line: protocol.UInteger(endLine), + Character: protocol.UInteger(endChar)}, + }, + Severity: &severity, + Source: &serverName, + Code: &protocol.IntegerOrString{Value: vacuumResult.Rule.Id}, + CodeDescription: &protocol.CodeDescription{HRef: diagnosticErrorHref}, + Message: vacuumResult.Message, + } +} + +func GetDiagnosticSeverityFromRule(rule *model.Rule) protocol.DiagnosticSeverity { switch rule.Severity { case model.SeverityError: return protocol.DiagnosticSeverityError From ca4b33d928fd20858430d23939b86c021b27cc3d Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 26 Feb 2024 09:20:38 -0500 Subject: [PATCH 03/12] updated `BuildEndNode` to accomodate quotes Signed-off-by: quobix --- utils/end_node.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/utils/end_node.go b/utils/end_node.go index 7546120f..507580ab 100644 --- a/utils/end_node.go +++ b/utils/end_node.go @@ -11,5 +11,10 @@ func BuildEndNode(node *yaml.Node) *yaml.Node { if node == nil { return nil } - return &yaml.Node{Line: node.Line, Column: node.Column + len(node.Value), Kind: node.Kind, Value: node.Value} + modifier := 0 + // quotes need to be accounted for + if node.Style == yaml.DoubleQuotedStyle || node.Style == yaml.SingleQuotedStyle { + modifier = 2 + } + return &yaml.Node{Line: node.Line, Column: node.Column + len(node.Value) + modifier, Kind: node.Kind, Value: node.Value} } From 463985c7ee957532d9e669ebbaa1a3d86f0b5a43 Mon Sep 17 00:00:00 2001 From: quobix Date: Thu, 29 Feb 2024 10:35:19 -0500 Subject: [PATCH 04/12] updated truthy to render correct line Signed-off-by: quobix --- functions/core/truthy.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/functions/core/truthy.go b/functions/core/truthy.go index bbccc3d0..1633d645 100644 --- a/functions/core/truthy.go +++ b/functions/core/truthy.go @@ -9,6 +9,7 @@ import ( vacuumUtils "github.com/daveshanley/vacuum/utils" "github.com/pb33f/libopenapi/utils" "gopkg.in/yaml.v3" + "sort" ) // Truthy is a rule that will determine if something is seen as 'true' (could be a 1 or "pizza", or actually 'true') @@ -59,17 +60,29 @@ func (t *Truthy) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) } if !utils.IsNodeMap(fieldNode) && !utils.IsNodeArray(fieldNodeValue) && !utils.IsNodeMap(fieldNodeValue) { - var endNode *yaml.Node - if len(node.Content) > 0 { - endNode = node.Content[len(node.Content)-1] - } else { - endNode = node + origin := context.Index.FindNodeOrigin(node) + if origin.Line > 1 { + nm := context.Index.GetNodeMap() + var keys []int + for k := range nm { + keys = append(keys, k) + } + + // Sort the keys slice. + sort.Ints(keys) + + np := nm[origin.Line-1][keys[0]] + + if np != nil { + node = np + } } + results = append(results, model.RuleFunctionResult{ Message: vacuumUtils.SuppliedOrDefault(message, fmt.Sprintf("%s: `%s` must be set", ruleMessage, context.RuleAction.Field)), StartNode: node, - EndNode: vacuumUtils.BuildEndNode(endNode), + EndNode: vacuumUtils.BuildEndNode(node), Path: pathValue, Rule: context.Rule, }) From acfdb5cbf1c77d9ba7c20dac0d6f143e4615ee13 Mon Sep 17 00:00:00 2001 From: quobix Date: Thu, 14 Mar 2024 09:44:21 -0400 Subject: [PATCH 05/12] missing examples now more sensitive to location accuracy refs skew results, this helps a lot. Signed-off-by: quobix --- functions/openapi/examples_missing.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/functions/openapi/examples_missing.go b/functions/openapi/examples_missing.go index d8f6caf4..2388d024 100644 --- a/functions/openapi/examples_missing.go +++ b/functions/openapi/examples_missing.go @@ -79,10 +79,16 @@ func (em ExamplesMissing) RunRule(_ []*yaml.Node, context model.RuleFunctionCont continue } if p.Value.Examples.Len() <= 0 && isExampleNodeNull([]*yaml.Node{p.Value.Example}) { + n := p.Value.GoLow().RootNode + if p.Value.GoLow().KeyNode != nil { + if p.Value.GoLow().KeyNode.Line == n.Line-1 { + n = p.Value.GoLow().KeyNode + } + } results = append(results, buildResult(vacuumUtils.SuppliedOrDefault(context.Rule.Message, "parameter is missing `examples` or `example`"), p.GenerateJSONPath(), - p.Value.GoLow().KeyNode, p)) + n, p)) } } } @@ -97,10 +103,16 @@ func (em ExamplesMissing) RunRule(_ []*yaml.Node, context model.RuleFunctionCont continue } if h.Value.Examples.Len() <= 0 && isExampleNodeNull([]*yaml.Node{h.Value.Example}) { + n := h.Value.GoLow().RootNode + if h.Value.GoLow().KeyNode != nil { + if h.Value.GoLow().KeyNode.Line == n.Line-1 { + n = h.Value.GoLow().KeyNode + } + } results = append(results, buildResult(vacuumUtils.SuppliedOrDefault(context.Rule.Message, "header is missing `examples` or `example`"), h.GenerateJSONPath(), - h.Value.GoLow().RootNode, h)) + n, h)) } } } @@ -109,10 +121,17 @@ func (em ExamplesMissing) RunRule(_ []*yaml.Node, context model.RuleFunctionCont for i := range context.DrDocument.MediaTypes { mt := context.DrDocument.MediaTypes[i] if mt.Value.Examples.Len() <= 0 && isExampleNodeNull([]*yaml.Node{mt.Value.Example}) { + + n := mt.Value.GoLow().RootNode + if mt.Value.GoLow().KeyNode != nil { + if mt.Value.GoLow().KeyNode.Line == n.Line-1 { + n = mt.Value.GoLow().KeyNode + } + } results = append(results, buildResult(vacuumUtils.SuppliedOrDefault(context.Rule.Message, "media type is missing `examples` or `example`"), mt.GenerateJSONPath(), - mt.Value.GoLow().KeyNode, mt)) + n, mt)) } } } From 1cdc73c69d93b1dbac00591a0a6156609998b57a Mon Sep 17 00:00:00 2001 From: quobix Date: Thu, 14 Mar 2024 09:44:40 -0400 Subject: [PATCH 06/12] cleaned up security rule Signed-off-by: quobix --- functions/openapi/operation_security_defined.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/functions/openapi/operation_security_defined.go b/functions/openapi/operation_security_defined.go index 7ba0a5ab..4cf3a64c 100644 --- a/functions/openapi/operation_security_defined.go +++ b/functions/openapi/operation_security_defined.go @@ -90,9 +90,9 @@ func (osd OperationSecurityDefined) checkSecurityNode(securityNode *yaml.Node, results = append(results, model.RuleFunctionResult{ Message: fmt.Sprintf("Security definition points a non-existent "+ - "securityScheme '%s'", name.Value), - StartNode: startNode, - EndNode: vacuumUtils.BuildEndNode(startNode), + "securityScheme `%s`", name.Value), + StartNode: name, + EndNode: vacuumUtils.BuildEndNode(name), Path: fmt.Sprintf("%s.security[%d]", basePath, i), Rule: context.Rule, }) From 3cadf45cae3fe31cfa940752dce5959a77b086fe Mon Sep 17 00:00:00 2001 From: quobix Date: Thu, 14 Mar 2024 09:45:08 -0400 Subject: [PATCH 07/12] cleaned unused-component Signed-off-by: quobix --- functions/openapi/unused_component.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/functions/openapi/unused_component.go b/functions/openapi/unused_component.go index 361e7dac..8a84607d 100644 --- a/functions/openapi/unused_component.go +++ b/functions/openapi/unused_component.go @@ -139,7 +139,7 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction // for every orphan, build a result. for key, ref := range notUsed { - _, path := utils.ConvertComponentIdIntoPath(ref.Definition) + _, path := utils.ConvertComponentIdIntoFriendlyPathSearch(key) // roll back node by one, so we have the actual start. //rolledBack := *ref.Node @@ -149,7 +149,9 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction node = ref.Node } if ref.KeyNode != nil { - node = ref.KeyNode + if ref.KeyNode.Line == ref.Node.Line-1 { + node = ref.KeyNode + } } results = append(results, model.RuleFunctionResult{ Message: fmt.Sprintf("`%s` is potentially unused or has been orphaned", key), From f06c53eccf70f86e332c4effcc3aa587402c7187 Mon Sep 17 00:00:00 2001 From: quobix Date: Sun, 31 Mar 2024 12:15:28 -0400 Subject: [PATCH 08/12] rebuilt operation_descriptions function the doctor is now in the house. Signed-off-by: quobix --- functions/openapi/operation_descriptions.go | 574 ++++++++++++++---- .../openapi/operation_descriptions_test.go | 281 +++++---- 2 files changed, 606 insertions(+), 249 deletions(-) diff --git a/functions/openapi/operation_descriptions.go b/functions/openapi/operation_descriptions.go index b6f37f40..eb92320e 100644 --- a/functions/openapi/operation_descriptions.go +++ b/functions/openapi/operation_descriptions.go @@ -6,9 +6,13 @@ package openapi import ( "fmt" "github.com/daveshanley/vacuum/model" - v3 "github.com/pb33f/libopenapi/datamodel/low/v3" + "github.com/daveshanley/vacuum/model/reports" + vacuumUtils "github.com/daveshanley/vacuum/utils" + "github.com/pb33f/doctor/model/high/base" + v3 "github.com/pb33f/doctor/model/high/v3" "github.com/pb33f/libopenapi/utils" "gopkg.in/yaml.v3" + "net/http" "strconv" "strings" ) @@ -25,10 +29,6 @@ func (od OperationDescription) GetSchema() model.RuleFunctionSchema { // RunRule will execute the OperationDescription rule, based on supplied context and a supplied []*yaml.Node slice. func (od OperationDescription) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) []model.RuleFunctionResult { - if len(nodes) <= 0 { - return nil - } - var results []model.RuleFunctionResult // check supplied type @@ -37,148 +37,458 @@ func (od OperationDescription) RunRule(nodes []*yaml.Node, context model.RuleFun minWordsString := props["minWords"] minWords, _ := strconv.Atoi(minWordsString) - if context.Index.GetPathsNode() == nil { + if context.DrDocument == nil { return results } - ops := context.Index.GetPathsNode().Content - var opPath, opMethod string - for i, op := range ops { - if i%2 == 0 { - opPath = op.Value - continue + paths := context.DrDocument.V3Document.Paths + + buildResult := func(message, path string, node *yaml.Node, component base.AcceptsRuleResults) model.RuleFunctionResult { + endNode := vacuumUtils.BuildEndNode(node) + result := model.RuleFunctionResult{ + Message: message, + StartNode: node, + EndNode: endNode, + Path: path, + Rule: context.Rule, + Range: reports.Range{ + Start: reports.RangeItem{ + Line: node.Line, + Char: node.Column, + }, + End: reports.RangeItem{ + Line: endNode.Line, + Char: endNode.Column, + }, + }, + } + component.AddRuleFunctionResult(base.ConvertRuleResult(&result)) + return result + } + + checkDescription := func(description string, method, location, missing, JSONPath string, node *yaml.Node, component base.AcceptsRuleResults) { + if description == "" { + results = append(results, + buildResult(vacuumUtils.SuppliedOrDefault(context.Rule.Message, + fmt.Sprintf("operation method `%s` %s is missing a `%s`", method, location, missing)), + JSONPath, node, component)) + } else { + words := strings.Split(description, " ") + if len(words) < minWords { + results = append(results, buildResult(vacuumUtils.SuppliedOrDefault(context.Rule.Message, + fmt.Sprintf("operation method `%s` %s has a `%s` that must be at least `%d` words long", + method, location, missing, minWords)), location, node, component)) + } + } + } + + checkOperation := func(desc, summary, path, method, location, reqLocation, jsonPath string, node *yaml.Node, + requestBody *v3.RequestBody, responses *v3.Responses, op base.AcceptsRuleResults) { + + checkDescription(desc, method, location, "description", jsonPath, node, op) + checkDescription(summary, method, location, "summary", jsonPath, node, op) + + // check request body + if requestBody != nil { + checkDescription(requestBody.Value.Description, method, reqLocation, + "description", requestBody.GenerateJSONPath(), requestBody.Value.GoLow().KeyNode, op) + } + + // check responses + if responses != nil { + for responsePairs := responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + code := responsePairs.Key() + response := responsePairs.Value() + checkDescription(response.Value.Description, method, fmt.Sprintf("response code `%s` `responseBody` at path `%s`", code, path), + "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + } } + } + + if paths != nil { + for pathPairs := paths.PathItems.First(); pathPairs != nil; pathPairs = pathPairs.Next() { + + path := pathPairs.Key() + operation := pathPairs.Value() - skip := false - for m, method := range op.Content { + atPath := fmt.Sprintf("at path `%s`", path) + atRequest := fmt.Sprintf("`requestBody` at path `%s`", path) - if m%2 == 0 { - opMethod = method.Value - if skip { - skip = false - } - continue + if operation.Get != nil { + checkOperation(operation.Get.Value.Description, operation.Get.Value.Summary, path, http.MethodGet, + atPath, atRequest, operation.Get.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Get.RequestBody, operation.Get.Responses, operation.Get) } - // skip non-operations - switch opMethod { - case - // No v2.*Label here, they're duplicates - v3.GetLabel, v3.PutLabel, v3.PostLabel, v3.DeleteLabel, v3.OptionsLabel, v3.HeadLabel, v3.PatchLabel, v3.TraceLabel: - // Ok, an operation - default: - skip = true - continue + + if operation.Post != nil { + checkOperation(operation.Post.Value.Description, operation.Post.Value.Summary, path, http.MethodPost, + atPath, atRequest, operation.Post.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Post.RequestBody, operation.Post.Responses, operation.Post) } - if skip { - skip = false - continue + + if operation.Put != nil { + checkOperation(operation.Put.Value.Description, operation.Put.Value.Summary, path, http.MethodPut, + atPath, atRequest, operation.Put.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Put.RequestBody, operation.Put.Responses, operation.Put) } - basePath := fmt.Sprintf("$.paths['%s'].%s", opPath, opMethod) - descKey, descNode := utils.FindKeyNodeTop("description", method.Content) - _, summNode := utils.FindKeyNodeTop("summary", method.Content) - requestBodyKey, requestBodyNode := utils.FindKeyNodeTop("requestBody", method.Content) - _, responsesNode := utils.FindKeyNode("responses", method.Content) - - if descNode == nil { - - // if there is no summary either, then report - if summNode == nil { - res := createDescriptionResult(fmt.Sprintf("operation `%s` at path `%s` is missing a description and a summary", - opMethod, opPath), basePath, method, method) - res.Rule = context.Rule - results = append(results, res) - } - - } else { - - // check if description is above a certain length of words - words := strings.Split(descNode.Value, " ") - if len(words) < minWords { - - res := createDescriptionResult(fmt.Sprintf("operation `%s` description at path `%s` must be "+ - "at least %d words long, (%d is not enough)", opMethod, opPath, minWords, len(words)), basePath, descKey, descKey) - res.Rule = context.Rule - results = append(results, res) - } + if operation.Delete != nil { + checkOperation(operation.Delete.Value.Description, operation.Delete.Value.Summary, path, http.MethodDelete, + atPath, atRequest, operation.Delete.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Delete.RequestBody, operation.Delete.Responses, operation.Delete) } - // check operation request body - if requestBodyNode != nil { - - descKey, descNode = utils.FindKeyNodeTop("description", requestBodyNode.Content) - _, summNode = utils.FindKeyNodeTop("summary", requestBodyNode.Content) - - if descNode == nil { - - // if there is no summary either, then report - if summNode == nil { - res := createDescriptionResult(fmt.Sprintf("field `requestBody` for operation `%s` at path `%s` "+ - "is missing a description and a summary", opMethod, opPath), - utils.BuildPath(basePath, []string{"requestBody"}), requestBodyKey, requestBodyKey) - res.Rule = context.Rule - results = append(results, res) - } - - } else { - - // check if request body description is above a certain length of words - words := strings.Split(descNode.Value, " ") - if len(words) < minWords { - - res := createDescriptionResult(fmt.Sprintf("field `requestBody` for operation `%s` description "+ - "at path `%s` must be at least %d words long, (%d is not enough)", opMethod, opPath, - minWords, len(words)), basePath, descKey, descKey) - res.Rule = context.Rule - results = append(results, res) - } - } + + if operation.Head != nil { + checkOperation(operation.Head.Value.Description, operation.Head.Value.Summary, path, http.MethodHead, + atPath, atRequest, operation.Head.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Head.RequestBody, operation.Head.Responses, operation.Head) } - // check operation responses - if responsesNode != nil { - - // run through each response. - var opCode string - var opCodeNode *yaml.Node - for z, response := range responsesNode.Content { - if z%2 == 0 { - opCode = response.Value - opCodeNode = response - continue - } - if strings.HasPrefix(opCode, "x-") { - continue - } - - descKey, descNode = utils.FindKeyNodeTop("description", response.Content) - _, summNode = utils.FindKeyNodeTop("summary", response.Content) - - if descNode == nil { - - // if there is no summary either, then report - if summNode == nil { - res := createDescriptionResult(fmt.Sprintf("operation `%s` response `%s` "+ - "at path `%s` is missing a description and a summary", opMethod, opCode, opPath), - utils.BuildPath(basePath, []string{"requestBody"}), opCodeNode, opCodeNode) - res.Rule = context.Rule - results = append(results, res) - } - } else { - - // check if response description is above a certain length of words - words := strings.Split(descNode.Value, " ") - if len(words) < minWords { - - res := createDescriptionResult(fmt.Sprintf("operation `%s` response `%s` "+ - "description at path `%s` must be at least %d words long, (%d is not enough)", opMethod, opCode, opPath, - minWords, len(words)), basePath, descKey, descKey) - res.Rule = context.Rule - results = append(results, res) - } - } - } + if operation.Patch != nil { + checkOperation(operation.Patch.Value.Description, operation.Patch.Value.Summary, path, http.MethodPatch, + atPath, atRequest, operation.Patch.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Patch.RequestBody, operation.Patch.Responses, operation.Patch) } + + if operation.Options != nil { + checkOperation(operation.Options.Value.Description, operation.Options.Value.Summary, path, http.MethodOptions, + atPath, atRequest, operation.Options.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Options.RequestBody, operation.Options.Responses, operation.Options) + } + + if operation.Trace != nil { + checkOperation(operation.Trace.Value.Description, operation.Trace.Value.Summary, path, http.MethodTrace, + atPath, atRequest, operation.Trace.GenerateJSONPath(), operation.Value.GoLow().KeyNode, + operation.Trace.RequestBody, operation.Trace.Responses, operation.Trace) + } + + // if operation.Put != nil { + // checkDescription(operation.Put.Value.Description, http.MethodPut, fmt.Sprintf("at path `%s`", path), + // "description", operation.Put.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Put) + // + // checkDescription(operation.Put.Value.Summary, http.MethodPut, fmt.Sprintf("at path `%s`", path), + // "summary", operation.Put.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Put) + // + // // check request body + // if operation.Put.RequestBody != nil { + // checkDescription(operation.Put.Value.RequestBody.Description, http.MethodPut, fmt.Sprintf("`requestBody` at path `%s`", path), + // "description", operation.Put.GenerateJSONPath(), + // operation.Put.RequestBody.Value.GoLow().KeyNode, operation.Put) + // } + // + // // check responses + // if operation.Put.Responses != nil { + // for responsePairs := operation.Put.Responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + // code := responsePairs.Key() + // response := responsePairs.Value() + // checkDescription(response.Value.Description, http.MethodPut, fmt.Sprintf("code `%s` `responseBody` at path `%s`", code, path), + // "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + // } + // + // } + // } + // + // if operation.Post != nil { + // checkDescription(operation.Post.Value.Description, http.MethodPost, fmt.Sprintf("at path `%s`", path), + // "description", operation.Post.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Post) + // + // checkDescription(operation.Post.Value.Summary, http.MethodPost, fmt.Sprintf("at path `%s`", path), + // "summary", operation.Post.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Post) + // + // // check request body + // if operation.Post.RequestBody != nil { + // checkDescription(operation.Post.Value.RequestBody.Description, http.MethodPost, fmt.Sprintf("`requestBody` at path `%s`", path), + // "description", operation.Post.GenerateJSONPath(), + // operation.Post.RequestBody.Value.GoLow().KeyNode, operation.Post) + // } + // + // // check responses + // if operation.Post.Responses != nil { + // for responsePairs := operation.Post.Responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + // code := responsePairs.Key() + // response := responsePairs.Value() + // checkDescription(response.Value.Description, http.MethodPost, fmt.Sprintf("code `%s` `responseBody` at path `%s`", code, path), + // "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + // } + // + // } + // } + // + // if operation.Delete != nil { + // checkDescription(operation.Delete.Value.Description, http.MethodDelete, fmt.Sprintf("at path `%s`", path), + // "description", operation.Delete.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Delete) + // + // checkDescription(operation.Delete.Value.Summary, http.MethodDelete, fmt.Sprintf("at path `%s`", path), + // "summary", operation.Delete.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Delete) + // + // // check request body + // if operation.Delete.RequestBody != nil { + // checkDescription(operation.Delete.Value.RequestBody.Description, http.MethodDelete, fmt.Sprintf("`requestBody` at path `%s`", path), + // "description", operation.Delete.GenerateJSONPath(), + // operation.Delete.RequestBody.Value.GoLow().KeyNode, operation.Delete) + // } + // + // // check responses + // if operation.Delete.Responses != nil { + // for responsePairs := operation.Delete.Responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + // code := responsePairs.Key() + // response := responsePairs.Value() + // checkDescription(response.Value.Description, http.MethodDelete, fmt.Sprintf("code `%s` `responseBody` at path `%s`", code, path), + // "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + // } + // + // } + // } + // + // if operation.Options != nil { + // checkDescription(operation.Options.Value.Description, http.MethodOptions, fmt.Sprintf("at path `%s`", path), + // "description", operation.Options.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Options) + // + // checkDescription(operation.Options.Value.Summary, http.MethodOptions, fmt.Sprintf("at path `%s`", path), + // "summary", operation.Options.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Options) + // + // // check request body + // if operation.Options.RequestBody != nil { + // checkDescription(operation.Options.Value.RequestBody.Description, http.MethodOptions, fmt.Sprintf("`requestBody` at path `%s`", path), + // "description", operation.Options.GenerateJSONPath(), + // operation.Options.RequestBody.Value.GoLow().KeyNode, operation.Options) + // } + // + // // check responses + // if operation.Options.Responses != nil { + // for responsePairs := operation.Options.Responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + // code := responsePairs.Key() + // response := responsePairs.Value() + // checkDescription(response.Value.Description, http.MethodOptions, fmt.Sprintf("code `%s` `responseBody` at path `%s`", code, path), + // "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + // } + // + // } + // } + // + // if operation.Head != nil { + // checkDescription(operation.Head.Value.Description, http.MethodHead, fmt.Sprintf("at path `%s`", path), + // "description", operation.Head.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Head) + // + // checkDescription(operation.Head.Value.Summary, http.MethodHead, fmt.Sprintf("at path `%s`", path), + // "summary", operation.Head.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Head) + // + // // check request body + // if operation.Head.RequestBody != nil { + // checkDescription(operation.Head.Value.RequestBody.Description, http.MethodHead, fmt.Sprintf("`requestBody` at path `%s`", path), + // "description", operation.Head.GenerateJSONPath(), + // operation.Head.RequestBody.Value.GoLow().KeyNode, operation.Head) + // } + // + // // check responses + // if operation.Head.Responses != nil { + // for responsePairs := operation.Head.Responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + // code := responsePairs.Key() + // response := responsePairs.Value() + // checkDescription(response.Value.Description, http.MethodHead, fmt.Sprintf("code `%s` `responseBody` at path `%s`", code, path), + // "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + // } + // + // } + // } + // + // if operation.Patch != nil { + // checkDescription(operation.Patch.Value.Description, http.MethodPatch, fmt.Sprintf("at path `%s`", path), + // "description", operation.Patch.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Patch) + // + // checkDescription(operation.Patch.Value.Summary, http.MethodPatch, fmt.Sprintf("at path `%s`", path), + // "summary", operation.Patch.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Patch) + // + // // check request body + // if operation.Patch.RequestBody != nil { + // checkDescription(operation.Patch.Value.RequestBody.Description, http.MethodPatch, fmt.Sprintf("`requestBody` at path `%s`", path), + // "description", operation.Patch.GenerateJSONPath(), + // operation.Patch.RequestBody.Value.GoLow().KeyNode, operation.Patch) + // } + // + // // check responses + // if operation.Patch.Responses != nil { + // for responsePairs := operation.Patch.Responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + // code := responsePairs.Key() + // response := responsePairs.Value() + // checkDescription(response.Value.Description, http.MethodPatch, fmt.Sprintf("code `%s` `responseBody` at path `%s`", code, path), + // "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + // } + // + // } + // } + // + // if operation.Trace != nil { + // checkDescription(operation.Trace.Value.Description, http.MethodTrace, fmt.Sprintf("at path `%s`", path), + // "description", operation.Trace.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Trace) + // + // checkDescription(operation.Trace.Value.Summary, http.MethodTrace, fmt.Sprintf("at path `%s`", path), + // "summary", operation.Trace.GenerateJSONPath(), operation.Value.GoLow().KeyNode, operation.Trace) + // + // // check request body + // if operation.Trace.RequestBody != nil { + // checkDescription(operation.Trace.Value.RequestBody.Description, http.MethodTrace, fmt.Sprintf("`requestBody` at path `%s`", path), + // "description", operation.Trace.GenerateJSONPath(), + // operation.Trace.RequestBody.Value.GoLow().KeyNode, operation.Trace) + // } + // + // // check responses + // if operation.Trace.Responses != nil { + // for responsePairs := operation.Trace.Responses.Codes.First(); responsePairs != nil; responsePairs = responsePairs.Next() { + // code := responsePairs.Key() + // response := responsePairs.Value() + // checkDescription(response.Value.Description, http.MethodTrace, fmt.Sprintf("code `%s` `responseBody` at path `%s`", code, path), + // "description", response.GenerateJSONPath(), response.Value.GoLow().KeyNode, response) + // } + // + // } + // } } } + + // + //if context.Index.GetPathsNode() == nil { + // return results + //} + //ops := context.Index.GetPathsNode().Content + // + //var opPath, opMethod string + //for i, op := range ops { + // if i%2 == 0 { + // opPath = op.Value + // continue + // } + // + // skip := false + // for m, method := range op.Content { + // + // if m%2 == 0 { + // opMethod = method.Value + // if skip { + // skip = false + // } + // continue + // } + // // skip non-operations + // switch opMethod { + // case + // // No v2.*Label here, they're duplicates + // v3.GetLabel, v3.PutLabel, v3.PostLabel, v3.DeleteLabel, v3.OptionsLabel, v3.HeadLabel, v3.PatchLabel, v3.TraceLabel: + // // Ok, an operation + // default: + // skip = true + // continue + // } + // if skip { + // skip = false + // continue + // } + // + // basePath := fmt.Sprintf("$.paths['%s'].%s", opPath, opMethod) + // descKey, descNode := utils.FindKeyNodeTop("description", method.Content) + // _, summNode := utils.FindKeyNodeTop("summary", method.Content) + // requestBodyKey, requestBodyNode := utils.FindKeyNodeTop("requestBody", method.Content) + // _, responsesNode := utils.FindKeyNode("responses", method.Content) + // + // if descNode == nil { + // + // // if there is no summary either, then report + // if summNode == nil { + // res := createDescriptionResult(fmt.Sprintf("operation `%s` at path `%s` is missing a description and a summary", + // opMethod, opPath), basePath, method, method) + // res.Rule = context.Rule + // results = append(results, res) + // } + // + // } else { + // + // // check if description is above a certain length of words + // words := strings.Split(descNode.Value, " ") + // if len(words) < minWords { + // + // res := createDescriptionResult(fmt.Sprintf("operation `%s` description at path `%s` must be "+ + // "at least %d words long, (%d is not enough)", opMethod, opPath, minWords, len(words)), basePath, descKey, descKey) + // res.Rule = context.Rule + // results = append(results, res) + // } + // } + // // check operation request body + // if requestBodyNode != nil { + // + // descKey, descNode = utils.FindKeyNodeTop("description", requestBodyNode.Content) + // _, summNode = utils.FindKeyNodeTop("summary", requestBodyNode.Content) + // + // if descNode == nil { + // + // // if there is no summary either, then report + // if summNode == nil { + // res := createDescriptionResult(fmt.Sprintf("field `requestBody` for operation `%s` at path `%s` "+ + // "is missing a description and a summary", opMethod, opPath), + // utils.BuildPath(basePath, []string{"requestBody"}), requestBodyKey, requestBodyKey) + // res.Rule = context.Rule + // results = append(results, res) + // } + // + // } else { + // + // // check if request body description is above a certain length of words + // words := strings.Split(descNode.Value, " ") + // if len(words) < minWords { + // + // res := createDescriptionResult(fmt.Sprintf("field `requestBody` for operation `%s` description "+ + // "at path `%s` must be at least %d words long, (%d is not enough)", opMethod, opPath, + // minWords, len(words)), basePath, descKey, descKey) + // res.Rule = context.Rule + // results = append(results, res) + // } + // } + // } + // + // // check operation responses + // if responsesNode != nil { + // + // // run through each response. + // var opCode string + // var opCodeNode *yaml.Node + // for z, response := range responsesNode.Content { + // if z%2 == 0 { + // opCode = response.Value + // opCodeNode = response + // continue + // } + // if strings.HasPrefix(opCode, "x-") { + // continue + // } + // + // descKey, descNode = utils.FindKeyNodeTop("description", response.Content) + // _, summNode = utils.FindKeyNodeTop("summary", response.Content) + // + // if descNode == nil { + // + // // if there is no summary either, then report + // if summNode == nil { + // res := createDescriptionResult(fmt.Sprintf("operation `%s` response `%s` "+ + // "at path `%s` is missing a description and a summary", opMethod, opCode, opPath), + // utils.BuildPath(basePath, []string{"requestBody"}), opCodeNode, opCodeNode) + // res.Rule = context.Rule + // results = append(results, res) + // } + // } else { + // + // // check if response description is above a certain length of words + // words := strings.Split(descNode.Value, " ") + // if len(words) < minWords { + // + // res := createDescriptionResult(fmt.Sprintf("operation `%s` response `%s` "+ + // "description at path `%s` must be at least %d words long, (%d is not enough)", opMethod, opCode, opPath, + // minWords, len(words)), basePath, descKey, descKey) + // res.Rule = context.Rule + // results = append(results, res) + // } + // } + // } + // } + // } + //} return results } diff --git a/functions/openapi/operation_descriptions_test.go b/functions/openapi/operation_descriptions_test.go index 8ea093c8..d10280e9 100644 --- a/functions/openapi/operation_descriptions_test.go +++ b/functions/openapi/operation_descriptions_test.go @@ -1,7 +1,10 @@ package openapi import ( + "fmt" "github.com/daveshanley/vacuum/model" + drModel "github.com/pb33f/doctor/model" + "github.com/pb33f/libopenapi" "github.com/pb33f/libopenapi/index" "github.com/pb33f/libopenapi/utils" "github.com/stretchr/testify/assert" @@ -22,7 +25,8 @@ func TestOperationDescription_RunRule(t *testing.T) { func TestOperationDescription_CheckDescriptionMissing(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: get: operationId: a @@ -32,85 +36,99 @@ func TestOperationDescription_CheckDescriptionMissing(t *testing.T) { description: this is a description that is great and 10 words long at least operationId: c` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) opts["minWords"] = "10" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) - assert.Len(t, res, 2) + assert.Len(t, res, 5) + assert.Equal(t, "operation method `GET` at path `/fish/paste` is missing a `description`", res[0].Message) + assert.Equal(t, "operation method `GET` at path `/fish/paste` is missing a `summary`", res[1].Message) + assert.Equal(t, "operation method `POST` at path `/fish/paste` is missing a `summary`", res[2].Message) + assert.Equal(t, "operation method `PUT` at path `/fish/paste` is missing a `description`", res[3].Message) + assert.Equal(t, "operation method `PUT` at path `/fish/paste` is missing a `summary`", res[4].Message) } func TestOperationDescription_CheckDescriptionTooShort(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: post: description: this is a thing that does nothing` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) opts["minWords"] = "10" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) - def := OperationDescription{} - res := def.RunRule(nodes, ctx) + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule - assert.Len(t, res, 1) - assert.NotNil(t, res[0].Path) + def := OperationDescription{} + res := def.RunRule(nil, ctx) + assert.Len(t, res, 2) + assert.Equal(t, "operation method `POST` at path `/fish/paste` has a `description` that must be at least `10` words long", res[0].Message) + assert.Equal(t, "operation method `POST` at path `/fish/paste` is missing a `summary`", res[1].Message) } func TestOperationDescription_SummaryButNoDescription(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: post: + description: something boring but valid summary: this is a thing that does nothing` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) opts["minWords"] = "1" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) assert.Len(t, res, 0) @@ -118,7 +136,8 @@ func TestOperationDescription_SummaryButNoDescription(t *testing.T) { func TestOperationDescription_CheckRequestBodyDescriptionExists(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: post: description: this is a thing @@ -128,34 +147,38 @@ func TestOperationDescription_CheckRequestBodyDescriptionExists(t *testing.T) { schema: type: string` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) - opts["minWords"] = "2" + opts["minWords"] = "1" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) - assert.Len(t, res, 1) + assert.Len(t, res, 2) assert.NotNil(t, res[0].Path) - assert.Equal(t, "field `requestBody` for operation `post` at path `/fish/paste` is missing a description and a summary", res[0].Message) + assert.Equal(t, "operation method `POST` at path `/fish/paste` is missing a `summary`", res[0].Message) + assert.Equal(t, "operation method `POST` `requestBody` at path `/fish/paste` is missing a `description`", res[1].Message) } func TestOperationDescription_CheckRequestBodyDescriptionMeetsLength(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: post: description: this is a thing yeah @@ -166,37 +189,42 @@ func TestOperationDescription_CheckRequestBodyDescriptionMeetsLength(t *testing. schema: type: string` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) opts["minWords"] = "5" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) - assert.Len(t, res, 1) + assert.Len(t, res, 2) assert.NotNil(t, res[0].Path) - assert.Equal(t, "field `requestBody` for operation `post` description at path `/fish/paste` must be at least 5 words "+ - "long, (4 is not enough)", res[0].Message) + assert.Equal(t, "operation method `POST` at path `/fish/paste` is missing a `summary`", res[0].Message) + assert.Equal(t, "operation method `POST` `requestBody` at path `/fish/paste` has a `description` that must be at least `5` words long", res[1].Message) } func TestOperationDescription_CheckResponsesDescriptionExist(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: post: description: this is a thing + summary: this is a summary requestBody: description: this is a thing responses: @@ -206,38 +234,42 @@ func TestOperationDescription_CheckResponsesDescriptionExist(t *testing.T) { schema: type: string` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) - opts["minWords"] = "2" + opts["minWords"] = "1" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) assert.Len(t, res, 1) assert.NotNil(t, res[0].Path) - assert.Equal(t, "operation `post` response `200` at path `/fish/paste` is missing a description and a summary", + assert.Equal(t, "operation method `POST` response code `200` `responseBody` at path `/fish/paste` is missing a `description`", res[0].Message) } func TestOperationDescription_CheckResponsesDescriptionLongEnough(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: post: description: this is a thing + summary: this is a summary requestBody: description: this is a thing responses: @@ -248,123 +280,138 @@ func TestOperationDescription_CheckResponsesDescriptionLongEnough(t *testing.T) schema: type: string` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) opts["minWords"] = "2" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) assert.Len(t, res, 1) assert.NotNil(t, res[0].Path) - assert.Equal(t, "operation `post` response `200` description at path `/fish/paste` must be at least 2 "+ - "words long, (1 is not enough)", res[0].Message) + assert.Equal(t, "operation method `POST` response code `200` `responseBody` at path `/fish/paste` has a "+ + "`description` that must be at least `2` words long", res[0].Message) } func TestOperationDescription_CheckParametersIgnored(t *testing.T) { - yml := `paths: + yml := `openapi: 3.0.1 +paths: /fish/paste: parameters: - in: query post: description: this is a description that is great and 10 words long at least + summary: hey hey hey operationId: c` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) - opts["minWords"] = "10" + opts["minWords"] = "2" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) assert.Len(t, res, 0) } func TestOperationDescription_CheckServersIgnored(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: servers: - url: https://api.example.com/v1 post: description: this is a description that is great and 10 words long at least + summary: cakes operationId: c` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) - opts["minWords"] = "10" + opts["minWords"] = "2" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) - assert.Len(t, res, 0) + assert.Len(t, res, 1) + assert.Equal(t, "operation method `POST` at path `/fish/paste` has a `summary` that must be at least `2` words long", res[0].Message) } func TestOperationDescription_CheckExtensionsIgnored(t *testing.T) { - yml := `paths: + yml := `openapi: 3.1.0 +paths: /fish/paste: x-parameters: - in: query post: + summary: hey hey description: this is a description that is great and 10 words long at least operationId: c` + document, err := libopenapi.NewDocument([]byte(yml)) + if err != nil { + panic(fmt.Sprintf("cannot create new document: %e", err)) + } + m, _ := document.BuildV3Model() path := "$" - var rootNode yaml.Node - mErr := yaml.Unmarshal([]byte(yml), &rootNode) - assert.NoError(t, mErr) - - nodes, _ := utils.FindNodes([]byte(yml), path) - + drDocument := drModel.NewDrDocument(m) opts := make(map[string]string) - opts["minWords"] = "10" + opts["minWords"] = "2" rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts) ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts) - config := index.CreateOpenAPIIndexConfig() - ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config) + + ctx.Document = document + ctx.DrDocument = drDocument + ctx.Rule = &rule def := OperationDescription{} - res := def.RunRule(nodes, ctx) + res := def.RunRule(nil, ctx) assert.Len(t, res, 0) From b3f2d88df1f134f53ca887b13ec93b3fb7296f77 Mon Sep 17 00:00:00 2001 From: quobix Date: Sun, 31 Mar 2024 12:16:27 -0400 Subject: [PATCH 09/12] prevented stats from erroring out if there are no inputs also, hard wired the stats to drop off the face of the eath with schema violations. Signed-off-by: quobix --- statistics/statistics.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/statistics/statistics.go b/statistics/statistics.go index 195efd6a..876a6d69 100644 --- a/statistics/statistics.go +++ b/statistics/statistics.go @@ -11,6 +11,11 @@ import ( // that reduces churn on building stats over and over for different interfaces. func CreateReportStatistics(index *index.SpecIndex, info *datamodel.SpecInfo, results *model.RuleResultSet) *reports.ReportStatistics { + // don't go looking for stats if we don't have the necessary data + if index == nil || info == nil || results == nil { + return nil + } + opPCount := index.GetOperationsParameterCount() cPCount := index.GetComponentParameterCount() @@ -51,6 +56,13 @@ func CreateReportStatistics(index *index.SpecIndex, info *datamodel.SpecInfo, re score = 25.0 } + // if there are any oas-schema rile violations, bottom out the score, an invalid schema is a big deal. + for _, result := range results.Results { + if result.Rule.Id == "oas3-schema" { + score = score - 90 + } + } + if score < 0 { score = 10 // the lowest score we want to present can't be 0, there has to be some hope! } From 211629aab9d5a83b8908116a9e1562cce4f596f8 Mon Sep 17 00:00:00 2001 From: quobix Date: Wed, 3 Apr 2024 12:06:42 -0400 Subject: [PATCH 10/12] Fixed NPE discovered in the platform Signed-off-by: quobix --- functions/openapi/component_descriptions.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/functions/openapi/component_descriptions.go b/functions/openapi/component_descriptions.go index 30725825..01b420ea 100644 --- a/functions/openapi/component_descriptions.go +++ b/functions/openapi/component_descriptions.go @@ -76,12 +76,14 @@ func (cd ComponentDescription) RunRule(_ []*yaml.Node, context model.RuleFunctio key := schemaPairs.Key() k, _ := low.FindItemInOrderedMapWithKey(key, components.Value.GoLow().Schemas.Value) - checkDescription(schemaValue.Schema.Value.Description, - key, - "schemas", - schemaValue.GenerateJSONPath(), - k.GetKeyNode(), - schemaValue) + if schemaValue.Schema != nil && schemaValue.Schema.Value != nil { + checkDescription(schemaValue.Schema.Value.Description, + key, + "schemas", + schemaValue.GenerateJSONPath(), + k.GetKeyNode(), + schemaValue) + } } } From 7dc7e347bcf1638ed669893ebc0069640e833f71 Mon Sep 17 00:00:00 2001 From: quobix Date: Wed, 3 Apr 2024 12:29:10 -0400 Subject: [PATCH 11/12] Updated truthy to operate correctly tests were failing Signed-off-by: quobix --- functions/core/truthy.go | 33 +++++++++++++++++---------------- motor/rule_applicator_test.go | 9 +++++---- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/functions/core/truthy.go b/functions/core/truthy.go index 1633d645..d5e3dfef 100644 --- a/functions/core/truthy.go +++ b/functions/core/truthy.go @@ -60,24 +60,25 @@ func (t *Truthy) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) } if !utils.IsNodeMap(fieldNode) && !utils.IsNodeArray(fieldNodeValue) && !utils.IsNodeMap(fieldNodeValue) { - origin := context.Index.FindNodeOrigin(node) - if origin.Line > 1 { - nm := context.Index.GetNodeMap() - var keys []int - for k := range nm { - keys = append(keys, k) - } - - // Sort the keys slice. - sort.Ints(keys) - - np := nm[origin.Line-1][keys[0]] - - if np != nil { - node = np + if context.Index != nil { + origin := context.Index.FindNodeOrigin(node) + if origin != nil && origin.Line > 1 { + nm := context.Index.GetNodeMap() + var keys []int + for k := range nm { + keys = append(keys, k) + } + + // Sort the keys slice. + sort.Ints(keys) + + np := nm[origin.Line-1][keys[0]] + + if np != nil { + node = np + } } } - results = append(results, model.RuleFunctionResult{ Message: vacuumUtils.SuppliedOrDefault(message, fmt.Sprintf("%s: `%s` must be set", ruleMessage, context.RuleAction.Field)), diff --git a/motor/rule_applicator_test.go b/motor/rule_applicator_test.go index 92cd2819..67f6b3d1 100644 --- a/motor/rule_applicator_test.go +++ b/motor/rule_applicator_test.go @@ -646,7 +646,8 @@ func TestApplyRules_Length_Description_BadConfig(t *testing.T) { } results := ApplyRulesToRuleSet(rse) assert.Len(t, results.Errors, 0) - assert.Len(t, results.Results, 0) + assert.Len(t, results.Results, 1) + assert.Equal(t, "'length' needs 'min' or 'max' (or both) properties being set to operate: minimum property number not met (1)", results.Results[0].Message) } @@ -2058,7 +2059,7 @@ custom: assert.Len(t, results.Results, 1) assert.Equal(t, "name 'hello' and id '1234' are not 'some_name' or 'some_id'", results.Results[0].Message) - assert.Equal(t, "my-custom-js-rule", results.Results[0].RuleId) + assert.Equal(t, "my-custom-js-rule", results.Results[0].Rule.Id) assert.Equal(t, 3, results.Results[0].Range.Start.Line) } @@ -2104,7 +2105,7 @@ notCustom: true" assert.Len(t, results.Results, 2) assert.Equal(t, "core me up: `custom` must be set", results.Results[0].Message) - assert.Equal(t, "my-custom-js-rule", results.Results[0].RuleId) + assert.Equal(t, "my-custom-js-rule", results.Results[0].Rule.Id) assert.Equal(t, "this is a message, added after truthy was called", results.Results[1].Message) assert.Equal(t, 2, results.Results[0].Range.Start.Line) } @@ -2202,7 +2203,7 @@ notCustom: true" assert.Len(t, results.Results, 1) assert.Equal(t, "someOption is set to someValue", results.Results[0].Message) - assert.Equal(t, "my-custom-js-rule", results.Results[0].RuleId) + assert.Equal(t, "my-custom-js-rule", results.Results[0].Rule.Id) assert.Equal(t, 2, results.Results[0].Range.Start.Line) } From 73dbcffcdb893ae851c4d37e9faa111453c22ae7 Mon Sep 17 00:00:00 2001 From: quobix Date: Wed, 3 Apr 2024 12:44:14 -0400 Subject: [PATCH 12/12] disable dedupe on rules. Signed-off-by: quobix --- motor/rule_applicator.go | 2 +- motor/rule_applicator_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/motor/rule_applicator.go b/motor/rule_applicator.go index 35be20e4..d05d6b45 100644 --- a/motor/rule_applicator.go +++ b/motor/rule_applicator.go @@ -630,7 +630,7 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult { if indexResolved != nil && rolodexResolved != nil { filesProcessed = rolodexResolved.RolodexTotalFiles() fileSize = rolodexResolved.RolodexFileSize() - ruleResults = *removeDuplicates(&ruleResults, execution, indexResolved) + //ruleResults = *removeDuplicates(&ruleResults, execution, indexResolved) } then = time.Since(now).Milliseconds() diff --git a/motor/rule_applicator_test.go b/motor/rule_applicator_test.go index 67f6b3d1..f5c80303 100644 --- a/motor/rule_applicator_test.go +++ b/motor/rule_applicator_test.go @@ -647,7 +647,8 @@ func TestApplyRules_Length_Description_BadConfig(t *testing.T) { results := ApplyRulesToRuleSet(rse) assert.Len(t, results.Errors, 0) assert.Len(t, results.Results, 1) - assert.Equal(t, "'length' needs 'min' or 'max' (or both) properties being set to operate: minimum property number not met (1)", results.Results[0].Message) + assert.Equal(t, "'length' needs 'min' or 'max' (or both) properties being set to operate: minimum property number not met (1)", + results.Results[0].Message) }