Skip to content

Commit

Permalink
adds extra validations for operation parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
casualjim committed Apr 19, 2015
1 parent fcac9d2 commit 732e946
Show file tree
Hide file tree
Showing 8 changed files with 216 additions and 41 deletions.
26 changes: 26 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# top-most EditorConfig file
root = true

# Unix-style newlines with a newline ending every file
[*]
end_of_line = lf
insert_final_newline = true
indent_style = space
indent_size = 2
trim_trailing_whitespace = true

# Set default charset
[*.{js,py,go,scala,rb,java,html,css,less,sass,md}]
charset = utf-8

# Tab indentation (no size specified)
[*.go]
indent_style = tab

[*.md]
trim_trailing_whitespace = false

# Matches the exact files either package.json or .travis.yml
[{package.json,.travis.yml}]
indent_style = space
indent_size = 2
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ language: go

go:
- 1.4.2
- tip

before_install:
# linting
Expand Down
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ For a V1 I want to have this feature set completed:
- [ ] validate extra rules outlined [here](https://github.com/apigee-127/swagger-tools/blob/master/docs/Swagger_Validation.md)
- [ ] :boom: definition can't declare a property that's already defined by one of its ancestors (Error)
- [ ] :boom: definition's ancestor can't be a descendant of the same model (Error)
- [ ] :boom: each api path should be non-verbatim (account for path param names) unique per method (Error)
- [x] :boom: each api path should be non-verbatim (account for path param names) unique per method (Error)
- [ ] :warning: each security reference should contain only unique scopes (Warning)
- [ ] :warning: each security scope in a security definition should be unique (Warning)
- [ ] :boom: each path parameter should correspond to a parameter placeholder and vice versa (Error)
- [x] :boom: each path parameter should correspond to a parameter placeholder and vice versa (Error)
- [ ] :warning: each referencable definition must have references (Warning)
- [ ] :boom: each definition property listed in the required array must be defined in the properties of the model (Error)
- [x] :boom: each parameter should have a unique `name` and `type` combination (Error)
Expand All @@ -76,8 +76,8 @@ For a V1 I want to have this feature set completed:
- [ ] :boom: every default value that is specified must validate against the schema for that property (Error)
- [ ] :boom: items property is required for all schemas/definitions of type `array` (Error)
- [x] serve swagger UI for any swagger spec file
- [x] generate stub api based on swagger spec
- [ ] generate client from a swagger spec
- [x] generate api based on swagger spec
- [ ] generate go client from a swagger spec
- [ ] generate "sensible" random data based on swagger spec
- [ ] generate tests based on swagger spec for server
- [ ] generate tests based on swagger spec for client
Expand Down
11 changes: 9 additions & 2 deletions internal/testing/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,13 @@ const PetStore20 = `{
"in": "query",
"description": "The status to filter by",
"type": "string"
},
{
"name": "limit",
"in": "query",
"description": "The maximum number of results to return",
"type": "integer",
"format": "int64"
}
],
"summary": "Finds all pets in the system",
Expand Down Expand Up @@ -384,7 +391,7 @@ const PetStore20 = `{
"name"
]
}
]
]
},
"Tag": {
"id": "Tag",
Expand Down Expand Up @@ -660,7 +667,7 @@ const RootPetStore20 = `{
"name"
]
}
]
]
},
"Tag": {
"id": "Tag",
Expand Down
101 changes: 72 additions & 29 deletions internal/validate/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,19 @@ func (s *SpecValidator) Validate(data interface{}) *Result {
if sd == nil {
return sErr(errors.New(500, "spec validator can only validate spec.Document objects"))
}
s.spec = sd

res := new(Result)
schv := NewSchemaValidator(s.schema, nil, "", s.KnownFormats)
res.Merge(schv.Validate(sd.Spec())) // -
res.Merge(s.validateItems())
res.Merge(s.validateUniqueSecurityScopes())
res.Merge(s.validateUniqueScopesSecurityDefinitions())
res.Merge(s.validatePathParamPresence())
res.Merge(s.validateReferenced())
res.Merge(s.validateRequiredDefinitions())
res.Merge(s.validateParameters())
res.Merge(s.validateReferencesValid())
res.Merge(s.validateDefaultValueValidAgainstSchema())
return res
}

Expand All @@ -66,10 +67,37 @@ func (s *SpecValidator) validateUniqueScopesSecurityDefinitions() *Result {
return nil
}

func (s *SpecValidator) validatePathParamPresence() *Result {
func (s *SpecValidator) validatePathParamPresence(fromPath, fromOperation []string) *Result {
// Each defined operation path parameters must correspond to a named element in the API's path pattern.
// (For example, you cannot have a path parameter named id for the following path /pets/{petId} but you must have a path parameter named petId.)
return nil
res := new(Result)
for _, l := range fromPath {
var matched bool
for _, r := range fromOperation {
if l == "{"+r+"}" {
matched = true
break
}
}
if !matched {
res.Errors = append(res.Errors, errors.New(422, "path param %q has no parameter definition", l))
}
}

for _, p := range fromOperation {
var matched bool
for _, r := range fromPath {
if "{"+p+"}" == r {
matched = true
break
}
}
if !matched {
res.AddErrors(errors.New(422, "Path param %q is not present in the path", p))
}
}

return res
}

func (s *SpecValidator) validateReferenced() *Result {
Expand All @@ -87,35 +115,31 @@ func (s *SpecValidator) validateParameters() *Result {
// each operation should have only 1 parameter of type body
// each api path should be non-verbatim (account for path param names) unique per method
res := new(Result)
knownPaths := make(map[string]string)
for path, pi := range s.spec.Operations() {
segments, params := parsePath(path)
knowns := make([]string, 0, len(segments))
for _, s := range segments {
knowns = append(knowns, s)
}
for _, i := range params {
knowns[i] = "!"
}
knownPath := strings.Join(knowns, "/")
if orig, ok := knownPaths[knownPath]; ok {
res.AddErrors(errors.New(422, "path %s overlaps with %s", path, orig))
} else {
knownPaths[knownPath] = path
}
for method, pi := range s.spec.Operations() {
knownPaths := make(map[string]string)
for path, op := range pi {
segments, params := parsePath(path)
knowns := make([]string, 0, len(segments))
for _, s := range segments {
knowns = append(knowns, s)
}
var fromPath []string
for _, i := range params {
fromPath = append(fromPath, knowns[i])
knowns[i] = "!"
}
knownPath := strings.Join(knowns, "/")
if orig, ok := knownPaths[knownPath]; ok {
res.AddErrors(errors.New(422, "path %s overlaps with %s", path, orig))
} else {
knownPaths[knownPath] = path
}

for _, op := range pi {
ptypes := make(map[string]map[string]struct{})
var firstBodyParam string

var paramNames []string
for _, pr := range op.Parameters {
if pr.In == "body" {
if firstBodyParam != "" {
res.AddErrors(errors.New(422, "operation %q has more than 1 body param (accepted: %q, dropped: %q)", op.ID, firstBodyParam, pr.Name))
}
firstBodyParam = pr.Name
}

pnames, ok := ptypes[pr.In]
if !ok {
pnames = make(map[string]struct{})
Expand All @@ -128,13 +152,32 @@ func (s *SpecValidator) validateParameters() *Result {
}
pnames[pr.Name] = struct{}{}
}
for _, pr := range s.spec.ParamsFor(method, path) {
if pr.In == "body" {
if firstBodyParam != "" {
res.AddErrors(errors.New(422, "operation %q has more than 1 body param (accepted: %q, dropped: %q)", op.ID, firstBodyParam, pr.Name))
}
firstBodyParam = pr.Name
}

if pr.In == "path" {
paramNames = append(paramNames, pr.Name)
}
}
res.Merge(s.validatePathParamPresence(fromPath, paramNames))
}
}
return res
}

func parsePath(path string) ([]string, map[string]int) {
return nil, nil
func parsePath(path string) (segments []string, params []int) {
for i, p := range strings.Split(path, "/") {
segments = append(segments, p)
if len(p) > 0 && p[0] == '{' && p[len(p)-1] == '}' {
params = append(params, i)
}
}
return
}

func (s *SpecValidator) validateReferencesValid() *Result {
Expand Down
91 changes: 91 additions & 0 deletions internal/validate/spec_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package validate

import (
"testing"

"github.com/casualjim/go-swagger/internal/testing/petstore"
"github.com/casualjim/go-swagger/spec"
"github.com/stretchr/testify/assert"
)

func TestValidateItems(t *testing.T) {

}

func TestValidateUniqueSecurityScopes(t *testing.T) {
}

func TestValidateUniqueScopesSecurityDefinitions(t *testing.T) {
}

func TestValidateReferenced(t *testing.T) {
}

func TestValidateRequiredDefinitions(t *testing.T) {
}

func TestValidateParameters(t *testing.T) {
doc, api := petstore.NewAPI(t)
validator := NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
validator.spec = doc
res := validator.validateParameters()
assert.Empty(t, res.Errors)

sw := doc.Spec()
sw.Paths.Paths["/pets"].Get.Parameters = append(sw.Paths.Paths["/pets"].Get.Parameters, *spec.QueryParam("limit").Typed("string", ""))
res = validator.validateParameters()
assert.NotEmpty(t, res.Errors)

doc, api = petstore.NewAPI(t)
sw = doc.Spec()
sw.Paths.Paths["/pets"].Post.Parameters = append(sw.Paths.Paths["/pets"].Post.Parameters, *spec.BodyParam("fake", spec.RefProperty("#/definitions/Pet")))
validator = NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
validator.spec = doc
res = validator.validateParameters()
assert.NotEmpty(t, res.Errors)
assert.Len(t, res.Errors, 1)
assert.Contains(t, res.Errors[0].Error(), "has more than 1 body param")

doc, api = petstore.NewAPI(t)
sw = doc.Spec()
pp := sw.Paths.Paths["/pets/{id}"]
pp.Delete = nil
var nameParams []spec.Parameter
for _, p := range pp.Parameters {
if p.Name == "id" {
p.Name = "name"
nameParams = append(nameParams, p)
}
}
pp.Parameters = nameParams
sw.Paths.Paths["/pets/{name}"] = pp
doc.Reload()
validator = NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
validator.spec = doc
res = validator.validateParameters()
assert.NotEmpty(t, res.Errors)
assert.Len(t, res.Errors, 1)
assert.Contains(t, res.Errors[0].Error(), "overlaps with")

doc, api = petstore.NewAPI(t)
validator = NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
validator.spec = doc
sw = doc.Spec()
pp = sw.Paths.Paths["/pets/{id}"]
pp.Delete = nil
pp.Get.Parameters = nameParams
pp.Parameters = nil
sw.Paths.Paths["/pets/{id}"] = pp
doc.Reload()
res = validator.validateParameters()
assert.NotEmpty(t, res.Errors)
assert.Len(t, res.Errors, 2)
assert.Contains(t, res.Errors[1].Error(), "is not present in the path")
assert.Contains(t, res.Errors[0].Error(), "has no parameter definition")
}

func TestValidateReferencesValid(t *testing.T) {
}

func TestValidateDefaultValueAgainstSchema(t *testing.T) {
}
13 changes: 13 additions & 0 deletions spec/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,16 @@ func (d *Document) Host() string {
func (d *Document) Raw() json.RawMessage {
return d.raw
}

// Reload reanalyzes the spec
func (d *Document) Reload() *Document {
d.specAnalyzer = specAnalyzer{
spec: d.spec,
consumes: make(map[string]struct{}),
produces: make(map[string]struct{}),
authSchemes: make(map[string]struct{}),
operations: make(map[string]map[string]*Operation),
}
d.initialize()
return d
}
6 changes: 1 addition & 5 deletions validate/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@ import (
// - items property is required for all schemas/definitions of type `array`
func Spec(doc *spec.Document, formats strfmt.Registry) error {
// TODO: add more validations beyond just jsonschema
res := validate.NewSchemaValidator(doc.Schema(), nil, "", formats).Validate(doc.Spec())
if len(res.Errors) > 0 {
return errors.CompositeValidationError(res.Errors...)
}
return nil
return AgainstSchema(doc.Schema(), doc.Spec(), formats)
}

// AgainstSchema validates the specified data with the provided schema, when no schema
Expand Down

0 comments on commit 732e946

Please sign in to comment.