Adds specs for validating items for array elements

kshinn · Apr 19, 2015 · 45f58ee · 45f58ee
1 parent 732e946
commit 45f58ee
Show file tree

Hide file tree

Showing 4 changed files with 230 additions and 40 deletions.
diff --git a/README.md b/README.md
@@ -59,41 +59,37 @@ For a V1 I want to have this feature set completed:
 
 -	[x] An object model that serializes to swagger yaml or json
 -	[x] A tool to work with swagger:
-	-	[x] validate a swagger spec document
-	-	[x] validate against jsonschema
-	-	[ ] 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)
-		- [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)
-		- [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)
-		- [x] :boom: each operation should have only 1 parameter of type body (Error)
-		- [ ] :boom: each reference must point to a valid object (Error)
-		- [ ] :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] validate a swagger spec document:
+    -	[x] validate against jsonschema
+    -	[ ] 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)
+      - [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)
+      - [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)
+      - [x] :boom: each operation should have only 1 parameter of type body (Error)
+      - [ ] :boom: each reference must point to a valid object (Error)
+      - [ ] :boom: every default value that is specified must validate against the schema for that property (Error)
+      - [x] :boom: items property is required for all schemas/definitions of type `array` (Error)
 	-	[x] serve swagger UI for any swagger spec file
-	-	[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
+  - code generation
+    -	[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 client
+    -	[ ] generate tests based on swagger spec for server
 -	[x] Middlewares:
 	-	[x] serve spec
 	-	[x] routing
 	-	[x] validation
 	-	[x] additional validation through an interface
-	-	[ ] authorization
+	-	[x] authorization
 		-	[x] basic auth
 		-	[x] api key auth
-		-	[ ] oauth2
-			-	[ ] implicit
-			-	[ ] access code
-			-	[ ] password
-			-	[ ] application
 	-	[x] swagger docs UI
 -	[x] Typed JSON Schema implementation
 	-	[x] JSON Pointer that knows about structs
@@ -127,6 +123,12 @@ After the v1 implementation extra transports are on the roadmap
 	- [ ] optimized serializer for YAML
 - Middlewares:
 	- [ ] swagger editor
+  - [ ] authorization:
+		-	[ ] oauth2
+			-	[ ] implicit
+			-	[ ] access code
+			-	[ ] password
+			-	[ ] application
 -	Tools:
 	-	[ ] generate spec document based on the code
 	-	[ ] watch swagger spec file and regenerate when modified

diff --git a/internal/validate/spec.go b/internal/validate/spec.go
@@ -1,6 +1,7 @@
 package validate
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/casualjim/go-swagger/errors"
@@ -24,34 +25,111 @@ func NewSpecValidator(schema *spec.Schema, formats strfmt.Registry) *SpecValidat
 }
 
 // Validate validates the swagger spec
-func (s *SpecValidator) Validate(data interface{}) *Result {
+func (s *SpecValidator) Validate(data interface{}) (errs *Result, warnings *Result) {
 	var sd *spec.Document
 
 	switch v := data.(type) {
 	case *spec.Document:
 		sd = v
 	}
 	if sd == nil {
-		return sErr(errors.New(500, "spec validator can only validate spec.Document objects"))
+		errs = sErr(errors.New(500, "spec validator can only validate spec.Document objects"))
+		return
 	}
 	s.spec = sd
 
-	res := new(Result)
+	errs = new(Result)
+	warnings = 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.validateReferenced())
-	res.Merge(s.validateRequiredDefinitions())
-	res.Merge(s.validateParameters())
-	res.Merge(s.validateReferencesValid())
-	res.Merge(s.validateDefaultValueValidAgainstSchema())
-	return res
+	errs.Merge(schv.Validate(sd.Spec()))                        // error -
+	errs.Merge(s.validateItems())                               // error -
+	warnings.Merge(s.validateUniqueSecurityScopes())            // warning
+	warnings.Merge(s.validateUniqueScopesSecurityDefinitions()) // warning
+	warnings.Merge(s.validateReferenced())                      // warning
+	errs.Merge(s.validateRequiredDefinitions())                 // error
+	errs.Merge(s.validateParameters())                          // error -
+	errs.Merge(s.validateReferencesValid())                     // error
+	errs.Merge(s.validateDefaultValueValidAgainstSchema())      // error
+
+	return
 }
 
 func (s *SpecValidator) validateItems() *Result {
 	// validate parameter, items, schema and response objects for presence of item if type is array
+	res := new(Result)
+
+	// TODO: implement support for lookups of refs
+	for method, pi := range s.spec.Operations() {
+		for path, op := range pi {
+			for _, param := range s.spec.ParamsFor(method, path) {
+				if param.TypeName() == "array" && param.ItemsTypeName() == "" {
+					res.AddErrors(errors.New(422, "param %q for %q is a collection without an element type", param.Name, op.ID))
+					continue
+				}
+				if param.In != "body" {
+					if param.Items != nil {
+						items := param.Items
+						for items.TypeName() == "array" {
+							if items.ItemsTypeName() == "" {
+								res.AddErrors(errors.New(422, "param %q for %q is a collection without an element type", param.Name, op.ID))
+								break
+							}
+							items = items.Items
+						}
+					}
+				} else {
+					if err := s.validateSchemaItems(*param.Schema, fmt.Sprintf("body param %q", param.Name), op.ID); err != nil {
+						res.AddErrors(err)
+					}
+				}
+			}
+
+			var responses []spec.Response
+			if op.Responses != nil {
+				if op.Responses.Default != nil {
+					responses = append(responses, *op.Responses.Default)
+				}
+				for _, v := range op.Responses.StatusCodeResponses {
+					responses = append(responses, v)
+				}
+			}
+
+			for _, resp := range responses {
+				for hn, hv := range resp.Headers {
+					if hv.TypeName() == "array" && hv.ItemsTypeName() == "" {
+						res.AddErrors(errors.New(422, "header %q for %q is a collection without an element type", hn, op.ID))
+					}
+				}
+				if resp.Schema != nil {
+					if err := s.validateSchemaItems(*resp.Schema, "response body", op.ID); err != nil {
+						res.AddErrors(err)
+					}
+				}
+			}
+		}
+	}
+	return res
+}
+
+func (s *SpecValidator) validateSchemaItems(schema spec.Schema, prefix, opID string) error {
+	if !schema.Type.Contains("array") {
+		return nil
+	}
+
+	if schema.Items == nil || schema.Items.Len() == 0 {
+		return errors.New(422, "%s for %q is a collection without an element type", prefix, opID)
+	}
+
+	schemas := schema.Items.Schemas
+	if schema.Items.Schema != nil {
+		schemas = []spec.Schema{*schema.Items.Schema}
+	}
+	for _, sch := range schemas {
+		if err := s.validateSchemaItems(sch, prefix, opID); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

diff --git a/internal/validate/spec_test.go b/internal/validate/spec_test.go
@@ -9,7 +9,98 @@ import (
 )
 
 func TestValidateItems(t *testing.T) {
+	doc, api := petstore.NewAPI(t)
+	validator := NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
+	validator.spec = doc
+	res := validator.validateItems()
+	assert.Empty(t, res.Errors)
+
+	// in operation parameters
+	sw := doc.Spec()
+	sw.Paths.Paths["/pets"].Get.Parameters[0].Type = "array"
+	res = validator.validateItems()
+	assert.NotEmpty(t, res.Errors)
+
+	sw.Paths.Paths["/pets"].Get.Parameters[0].Items = spec.NewItems().Typed("string", "")
+	res = validator.validateItems()
+	assert.Empty(t, res.Errors)
+
+	sw.Paths.Paths["/pets"].Get.Parameters[0].Items = spec.NewItems().Typed("array", "")
+	res = validator.validateItems()
+	assert.NotEmpty(t, res.Errors)
+
+	sw.Paths.Paths["/pets"].Get.Parameters[0].Items.Items = spec.NewItems().Typed("string", "")
+	res = validator.validateItems()
+	assert.Empty(t, res.Errors)
+
+	// in global parameters
+	sw.Parameters = make(map[string]spec.Parameter)
+	sw.Parameters["other"] = *spec.SimpleArrayParam("other", "array", "csv")
+	res = validator.validateItems()
+	assert.Empty(t, res.Errors)
+
+	pp := spec.SimpleArrayParam("other", "array", "")
+	pp.Items = nil
+	sw.Parameters["other"] = *pp
+	res = validator.validateItems()
+	assert.NotEmpty(t, res.Errors)
+
+	// in shared path object parameters
+	doc, api = petstore.NewAPI(t)
+	validator = NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
+	validator.spec = doc
+	sw = doc.Spec()
 
+	pa := sw.Paths.Paths["/pets"]
+	pa.Parameters = []spec.Parameter{*spec.SimpleArrayParam("another", "array", "csv")}
+	sw.Paths.Paths["/pets"] = pa
+	res = validator.validateItems()
+	assert.Empty(t, res.Errors)
+
+	pa = sw.Paths.Paths["/pets"]
+	pp = spec.SimpleArrayParam("other", "array", "")
+	pp.Items = nil
+	pa.Parameters = []spec.Parameter{*pp}
+	sw.Paths.Paths["/pets"] = pa
+	res = validator.validateItems()
+	assert.NotEmpty(t, res.Errors)
+
+	// in body param schema
+	doc, api = petstore.NewAPI(t)
+	validator = NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
+	validator.spec = doc
+	sw = doc.Spec()
+	pa = sw.Paths.Paths["/pets"]
+	pa.Post.Parameters[0].Schema = spec.ArrayProperty(nil)
+	res = validator.validateItems()
+	assert.NotEmpty(t, res.Errors)
+
+	// in response headers
+	doc, api = petstore.NewAPI(t)
+	validator = NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
+	validator.spec = doc
+	sw = doc.Spec()
+	pa = sw.Paths.Paths["/pets"]
+	rp := pa.Post.Responses.StatusCodeResponses[200]
+	var hdr spec.Header
+	hdr.Type = "array"
+	rp.Headers = make(map[string]spec.Header)
+	rp.Headers["X-YADA"] = hdr
+	pa.Post.Responses.StatusCodeResponses[200] = rp
+	res = validator.validateItems()
+	assert.NotEmpty(t, res.Errors)
+
+	// in response schema
+	doc, api = petstore.NewAPI(t)
+	validator = NewSpecValidator(spec.MustLoadSwagger20Schema(), api.Formats())
+	validator.spec = doc
+	sw = doc.Spec()
+	pa = sw.Paths.Paths["/pets"]
+	rp = pa.Post.Responses.StatusCodeResponses[200]
+	rp.Schema = spec.ArrayProperty(nil)
+	pa.Post.Responses.StatusCodeResponses[200] = rp
+	res = validator.validateItems()
+	assert.NotEmpty(t, res.Errors)
 }
 
 func TestValidateUniqueSecurityScopes(t *testing.T) {
@@ -22,6 +113,7 @@ func TestValidateReferenced(t *testing.T) {
 }
 
 func TestValidateRequiredDefinitions(t *testing.T) {
+
 }
 
 func TestValidateParameters(t *testing.T) {

diff --git a/spec/swagger.go b/spec/swagger.go
@@ -247,6 +247,22 @@ type SchemaOrArray struct {
 	Schemas []Schema
 }
 
+// Len returns the number of schemas in this property
+func (s SchemaOrArray) Len() int {
+	if s.Schema != nil {
+		return 1
+	}
+	return len(s.Schemas)
+}
+
+// ContainsType returns true when one of the schemas is of the specified type
+func (s *SchemaOrArray) ContainsType(name string) bool {
+	if s.Schema != nil {
+		return s.Schema.Type != nil && s.Schema.Type.Contains(name)
+	}
+	return false
+}
+
 // MarshalJSON converts this schema object or array into JSON structure
 func (s SchemaOrArray) MarshalJSON() ([]byte, error) {
 	if len(s.Schemas) > 0 {
@@ -280,3 +296,5 @@ func (s *SchemaOrArray) UnmarshalJSON(data []byte) error {
 	*s = nw
 	return nil
 }
+
+// vim:set ft=go noet sts=2 sw=2 ts=2: