Skip to content

Commit

Permalink
Rule updates to address issue #137 and #138 (#139)
Browse files Browse the repository at this point in the history 
* Adding suggested copy changes to operation_description rules

@scop suggested some better phrasing for more accurate descriptions in #138

* Added tests to validate a fix for #137

path global parameters were being picked up as an operation (which is not correct), added a check for params to affected rules, and then added some tests to validate they are no-longer considered an operation.
`operation_descriptions` and `operation_tags` functions were updated.

Signed-off-by: Dave Shanley <dave@quobix.com>

Signed-off-by: Dave Shanley <dave@quobix.com>
  • Loading branch information
@daveshanley
daveshanley committed Oct 6, 2022
1 parent 3836117 commit 283b739
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 4 deletions.
10 changes: 8 additions & 2 deletions functions/openapi/operation_descriptions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package openapi
import (
"fmt"
"github.com/daveshanley/vacuum/model"
v3 "github.com/pb33f/libopenapi/datamodel/low/v3"
"github.com/pb33f/libopenapi/utils"
"gopkg.in/yaml.v3"
"strconv"
Expand Down Expand Up @@ -59,6 +60,11 @@ func (od OperationDescription) RunRule(nodes []*yaml.Node, context model.RuleFun
skip = true
continue
}
// do not process parameters, they are not operations.
if opMethod == v3.ParametersLabel {
skip = true
continue
}
if skip {
skip = false
continue
Expand Down Expand Up @@ -93,7 +99,7 @@ func (od OperationDescription) RunRule(nodes []*yaml.Node, context model.RuleFun
descKey, descNode = utils.FindKeyNode("description", requestBodyNode.Content)

if descNode == nil {
res := createDescriptionResult(fmt.Sprintf("Operation `requestBody` for method `%s` at path `%s` "+
res := createDescriptionResult(fmt.Sprintf("Field `requestBody` for operation `%s` at path `%s` "+
"is missing a description", opMethod, opPath),
utils.BuildPath(basePath, []string{"requestBody"}), requestBodyKey, requestBodyNode)
res.Rule = context.Rule
Expand All @@ -104,7 +110,7 @@ func (od OperationDescription) RunRule(nodes []*yaml.Node, context model.RuleFun
words := strings.Split(descNode.Value, " ")
if len(words) < minWords {

res := createDescriptionResult(fmt.Sprintf("Operation `requestBody` for method `%s` description "+
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, descNode)
res.Rule = context.Rule
Expand Down
94 changes: 92 additions & 2 deletions functions/openapi/operation_descriptions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestOperationDescription_CheckRequestBodyDescriptionExists(t *testing.T) {

assert.Len(t, res, 1)
assert.NotNil(t, res[0].Path)
assert.Equal(t, "Operation `requestBody` for method `post` at path `/fish/paste` is missing a description", res[0].Message)
assert.Equal(t, "Field `requestBody` for operation `post` at path `/fish/paste` is missing a description", res[0].Message)

}

Expand Down Expand Up @@ -152,7 +152,7 @@ func TestOperationDescription_CheckRequestBodyDescriptionMeetsLength(t *testing.

assert.Len(t, res, 1)
assert.NotNil(t, res[0].Path)
assert.Equal(t, "Operation `requestBody` for method `post` description at path `/fish/paste` must be at least 5 words "+
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)

}
Expand Down Expand Up @@ -237,3 +237,93 @@ func TestOperationDescription_CheckResponsesDescriptionLongEnough(t *testing.T)
"words long, (1 is not enough)", res[0].Message)

}

func TestOperationDescription_CheckParametersIgnored(t *testing.T) {

yml := `paths:
/fish/paste:
parameters:
- in: query
post:
description: this is a description that is great and 10 words long at least
operationId: c`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

opts := make(map[string]string)
opts["minWords"] = "10"

rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts)
ctx.Index = index.NewSpecIndex(&rootNode)

def := OperationDescription{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)

}

func TestOperationDescription_CheckExtensionsIgnored(t *testing.T) {

yml := `paths:
/fish/paste:
x-parameters:
- in: query
post:
description: this is a description that is great and 10 words long at least
operationId: c`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

opts := make(map[string]string)
opts["minWords"] = "10"

rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts)
ctx.Index = index.NewSpecIndex(&rootNode)

def := OperationDescription{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)

}

func TestOperationDescription_CheckForNoPaths(t *testing.T) {

yml := `openapi: 3.0.3`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

opts := make(map[string]string)
opts["minWords"] = "10"

rule := buildOpenApiTestRuleAction(path, "operation-description", "", opts)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), opts)
ctx.Index = index.NewSpecIndex(&rootNode)

def := OperationDescription{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)

}
6 changes: 6 additions & 0 deletions functions/openapi/operation_tags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package openapi
import (
"fmt"
"github.com/daveshanley/vacuum/model"
v3 "github.com/pb33f/libopenapi/datamodel/low/v3"
"github.com/pb33f/libopenapi/utils"
"gopkg.in/yaml.v3"
"strings"
Expand Down Expand Up @@ -59,6 +60,11 @@ func (ot OperationTags) RunRule(nodes []*yaml.Node, context model.RuleFunctionCo
skip = true
continue
}
// skip parameters, they are not an operation.
if currentVerb == v3.ParametersLabel {
skip = true
continue
}
if skip {
skip = false
continue
Expand Down
82 changes: 82 additions & 0 deletions functions/openapi/operation_tags_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,85 @@ func TestOperationTags_RunRule_EmptyTags(t *testing.T) {
assert.Equal(t, "$.paths./hello.get", res[0].Path)

}

func TestOperationTags_RunRule_IgnoreParameters(t *testing.T) {

yml := `paths:
/hello:
post:
tags:
- a
- b
parameters:
- in: query`

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

path := "$"

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "operation_tags", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
ctx.Index = index.NewSpecIndex(&rootNode)

def := OperationTags{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)
}

func TestOperationTags_RunRule_IgnoreExtensions(t *testing.T) {

yml := `paths:
/hello:
post:
tags:
- a
- b
x-parameters:
- in: query
parameters:
- in: path`

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

path := "$"

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "operation_tags", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
ctx.Index = index.NewSpecIndex(&rootNode)

def := OperationTags{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)
}

func TestOperationTags_RunRule_HandleNoPaths(t *testing.T) {

yml := `openapi: 3.0.3`

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

path := "$"

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "operation_tags", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
ctx.Index = index.NewSpecIndex(&rootNode)

def := OperationTags{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)
}

0 comments on commit 283b739

Please sign in to comment.