refact(validators) (#169)

* refact(validators)

Readability improvements:
* Introduced private constructors for all validators
* All private constructors can use a *SchemaValidatorOptions (generalize
  options passing pattern)
* All exported methods use private constructors (hide the implementation
  of options)
* Readability: reduced the level of indentation with early exit
  conditions than nested conditions
* Object validator: refactored Validate in smaller functions
* Removed dead code/commented out code

Minor efficiency improvements

* In Applies method: reflect replaced by type switch whenever possible
* Removed (mostly unusable) debugLog
* In object validator:
  * pattern properties now use the regexp cache readily available
  * split path may be performed only once
* In schema props: keep arrays of pointers []*SchemaValidator and avoid
  copying a new schema
* In default & example validators:
  * map[string]bool -> map[string]struct{}
  * clear, don't reallocate the visited map
* []validator slice -> [n]validator array

* introduced a benchmark follow-up document, based on the existing Kubernetes API
  fixture

Signed-off-by: Frederic BIDON <[email protected]>

* fixed typo

Signed-off-by: Frederic BIDON <[email protected]>

---------

Signed-off-by: Frederic BIDON <[email protected]>

Author: fredbi

BENCHMARK.md

@@ -0,0 +1,22 @@
+# Benchmark
+
+Validating the Kubernetes Swagger API
+
+v0.22.6: 60,000,000 allocs
+```
+goos: linux
+goarch: amd64
+pkg: github.com/go-openapi/validate
+cpu: AMD Ryzen 7 5800X 8-Core Processor
+Benchmark_KubernetesSpec/validating_kubernetes_API-16         	       1	8549863982 ns/op	7067424936 B/op	59583275 allocs/op
+```
+
+After refact PR: minor but noticable improvements: 25,000,000 allocs
+```
+go test -bench Spec
+goos: linux
+goarch: amd64
+pkg: github.com/go-openapi/validate
+cpu: AMD Ryzen 7 5800X 8-Core Processor
+Benchmark_KubernetesSpec/validating_kubernetes_API-16         	       1	4064535557 ns/op	3379715592 B/op	25320330 allocs/op
+```

benchmark_test.go

@@ -0,0 +1,30 @@
+package validate
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/go-openapi/loads"
+	"github.com/go-openapi/strfmt"
+	"github.com/stretchr/testify/require"
+)
+
+func Benchmark_KubernetesSpec(b *testing.B) {
+	fp := filepath.Join("fixtures", "go-swagger", "canary", "kubernetes", "swagger.json")
+	doc, err := loads.Spec(fp)
+	require.NoError(b, err)
+	require.NotNil(b, doc)
+
+	b.Run("validating kubernetes API", func(b *testing.B) {
+		b.ResetTimer()
+		b.ReportAllocs()
+
+		for i := 0; i < b.N; i++ {
+			validator := NewSpecValidator(doc.Schema(), strfmt.Default)
+			res, _ := validator.Validate(doc)
+			if res == nil || !res.IsValid() {
+				b.FailNow()
+			}
+		}
+	})
+}

default_validator.go

@@ -25,48 +25,61 @@ import (
 // According to Swagger spec, default values MUST validate their schema.
 type defaultValidator struct {
 	SpecValidator  *SpecValidator
-	visitedSchemas map[string]bool
+	visitedSchemas map[string]struct{}
+	schemaOptions  *SchemaValidatorOptions
 }
 
 // resetVisited resets the internal state of visited schemas
 func (d *defaultValidator) resetVisited() {
-	d.visitedSchemas = map[string]bool{}
+	if d.visitedSchemas == nil {
+		d.visitedSchemas = make(map[string]struct{})
+
+		return
+	}
+
+	// TODO(go1.21): clear(ex.visitedSchemas)
+	for k := range d.visitedSchemas {
+		delete(d.visitedSchemas, k)
+	}
 }
 
-func isVisited(path string, visitedSchemas map[string]bool) bool {
-	found := visitedSchemas[path]
-	if !found {
-		// search for overlapping paths
-		frags := strings.Split(path, ".")
-		if len(frags) < 2 {
-			// shortcut exit on smaller paths
-			return found
+func isVisited(path string, visitedSchemas map[string]struct{}) bool {
+	_, found := visitedSchemas[path]
+	if found {
+		return true
+	}
+
+	// search for overlapping paths
+	frags := strings.Split(path, ".")
+	if len(frags) < 2 {
+		// shortcut exit on smaller paths
+		return found
+	}
+	last := len(frags) - 1
+	var currentFragStr, parent string
+	for i := range frags {
+		if i == 0 {
+			currentFragStr = frags[last]
+		} else {
+			currentFragStr = strings.Join([]string{frags[last-i], currentFragStr}, ".")
 		}
-		last := len(frags) - 1
-		var currentFragStr, parent string
-		for i := range frags {
-			if i == 0 {
-				currentFragStr = frags[last]
-			} else {
-				currentFragStr = strings.Join([]string{frags[last-i], currentFragStr}, ".")
-			}
-			if i < last {
-				parent = strings.Join(frags[0:last-i], ".")
-			} else {
-				parent = ""
-			}
-			if strings.HasSuffix(parent, currentFragStr) {
-				found = true
-				break
-			}
+		if i < last {
+			parent = strings.Join(frags[0:last-i], ".")
+		} else {
+			parent = ""
+		}
+		if strings.HasSuffix(parent, currentFragStr) {
+			found = true
+			break
 		}
 	}
+
 	return found
 }
 
 // beingVisited asserts a schema is being visited
 func (d *defaultValidator) beingVisited(path string) {
-	d.visitedSchemas[path] = true
+	d.visitedSchemas[path] = struct{}{}
 }
 
 // isVisited tells if a path has already been visited
@@ -75,8 +88,8 @@ func (d *defaultValidator) isVisited(path string) bool {
 }
 
 // Validate validates the default values declared in the swagger spec
-func (d *defaultValidator) Validate() (errs *Result) {
-	errs = new(Result)
+func (d *defaultValidator) Validate() *Result {
+	errs := new(Result)
 	if d == nil || d.SpecValidator == nil {
 		return errs
 	}
@@ -107,7 +120,7 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
 				// default values provided must validate against their inline definition (no explicit schema)
 				if param.Default != nil && param.Schema == nil {
 					// check param default value is valid
-					red := NewParamValidator(&param, s.KnownFormats).Validate(param.Default) //#nosec
+					red := newParamValidator(&param, s.KnownFormats, d.schemaOptions).Validate(param.Default) //#nosec
 					if red.HasErrorsOrWarnings() {
 						res.AddErrors(defaultValueDoesNotValidateMsg(param.Name, param.In))
 						res.Merge(red)
@@ -176,7 +189,7 @@ func (d *defaultValidator) validateDefaultInResponse(resp *spec.Response, respon
 			d.resetVisited()
 
 			if h.Default != nil {
-				red := NewHeaderValidator(nm, &h, s.KnownFormats).Validate(h.Default) //#nosec
+				red := newHeaderValidator(nm, &h, s.KnownFormats, d.schemaOptions).Validate(h.Default) //#nosec
 				if red.HasErrorsOrWarnings() {
 					res.AddErrors(defaultValueHeaderDoesNotValidateMsg(operationID, nm, responseName))
 					res.Merge(red)
@@ -223,7 +236,9 @@ func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path, in stri
 	s := d.SpecValidator
 
 	if schema.Default != nil {
-		res.Merge(NewSchemaValidator(schema, s.spec.Spec(), path+".default", s.KnownFormats, SwaggerSchema(true)).Validate(schema.Default))
+		res.Merge(
+			newSchemaValidator(schema, s.spec.Spec(), path+".default", s.KnownFormats, d.schemaOptions).Validate(schema.Default),
+		)
 	}
 	if schema.Items != nil {
 		if schema.Items.Schema != nil {
@@ -267,7 +282,9 @@ func (d *defaultValidator) validateDefaultValueItemsAgainstSchema(path, in strin
 	s := d.SpecValidator
 	if items != nil {
 		if items.Default != nil {
-			res.Merge(newItemsValidator(path, in, items, root, s.KnownFormats).Validate(0, items.Default))
+			res.Merge(
+				newItemsValidator(path, in, items, root, s.KnownFormats, d.schemaOptions).Validate(0, items.Default),
+			)
 		}
 		if items.Items != nil {
 			res.Merge(d.validateDefaultValueItemsAgainstSchema(path+"[0].default", in, root, items.Items))

doc_test.go

@@ -65,7 +65,7 @@ func ExampleSpec_second() {
 			fmt.Printf("This spec has some validation errors: %v\n", errs)
 		}
 	} else {
-		fmt.Println("Could not load this spec")
+		fmt.Println("Could not load this spec:", err)
 	}
 
 	// Output:

example_validator.go

@@ -23,17 +23,27 @@ import (
 // ExampleValidator validates example values defined in a spec
 type exampleValidator struct {
 	SpecValidator  *SpecValidator
-	visitedSchemas map[string]bool
+	visitedSchemas map[string]struct{}
+	schemaOptions  *SchemaValidatorOptions
 }
 
 // resetVisited resets the internal state of visited schemas
 func (ex *exampleValidator) resetVisited() {
-	ex.visitedSchemas = map[string]bool{}
+	if ex.visitedSchemas == nil {
+		ex.visitedSchemas = make(map[string]struct{})
+
+		return
+	}
+
+	// TODO(go1.21): clear(ex.visitedSchemas)
+	for k := range ex.visitedSchemas {
+		delete(ex.visitedSchemas, k)
+	}
 }
 
 // beingVisited asserts a schema is being visited
 func (ex *exampleValidator) beingVisited(path string) {
-	ex.visitedSchemas[path] = true
+	ex.visitedSchemas[path] = struct{}{}
 }
 
 // isVisited tells if a path has already been visited
@@ -48,8 +58,8 @@ func (ex *exampleValidator) isVisited(path string) bool {
 //   - schemas
 //   - individual property
 //   - responses
-func (ex *exampleValidator) Validate() (errs *Result) {
-	errs = new(Result)
+func (ex *exampleValidator) Validate() *Result {
+	errs := new(Result)
 	if ex == nil || ex.SpecValidator == nil {
 		return errs
 	}
@@ -82,7 +92,7 @@ func (ex *exampleValidator) validateExampleValueValidAgainstSchema() *Result {
 				// default values provided must validate against their inline definition (no explicit schema)
 				if param.Example != nil && param.Schema == nil {
 					// check param default value is valid
-					red := NewParamValidator(&param, s.KnownFormats).Validate(param.Example) //#nosec
+					red := newParamValidator(&param, s.KnownFormats, ex.schemaOptions).Validate(param.Example) //#nosec
 					if red.HasErrorsOrWarnings() {
 						res.AddWarnings(exampleValueDoesNotValidateMsg(param.Name, param.In))
 						res.MergeAsWarnings(red)
@@ -151,7 +161,7 @@ func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, respo
 			ex.resetVisited()
 
 			if h.Example != nil {
-				red := NewHeaderValidator(nm, &h, s.KnownFormats).Validate(h.Example) //#nosec
+				red := newHeaderValidator(nm, &h, s.KnownFormats, ex.schemaOptions).Validate(h.Example) //#nosec
 				if red.HasErrorsOrWarnings() {
 					res.AddWarnings(exampleValueHeaderDoesNotValidateMsg(operationID, nm, responseName))
 					res.MergeAsWarnings(red)
@@ -189,7 +199,9 @@ func (ex *exampleValidator) validateExampleInResponse(resp *spec.Response, respo
 	if response.Examples != nil {
 		if response.Schema != nil {
 			if example, ok := response.Examples["application/json"]; ok {
-				res.MergeAsWarnings(NewSchemaValidator(response.Schema, s.spec.Spec(), path+".examples", s.KnownFormats, SwaggerSchema(true)).Validate(example))
+				res.MergeAsWarnings(
+					newSchemaValidator(response.Schema, s.spec.Spec(), path+".examples", s.KnownFormats, s.schemaOptions).Validate(example),
+				)
 			} else {
 				// TODO: validate other media types too
 				res.AddWarnings(examplesMimeNotSupportedMsg(operationID, responseName))
@@ -211,7 +223,9 @@ func (ex *exampleValidator) validateExampleValueSchemaAgainstSchema(path, in str
 	res := new(Result)
 
 	if schema.Example != nil {
-		res.MergeAsWarnings(NewSchemaValidator(schema, s.spec.Spec(), path+".example", s.KnownFormats, SwaggerSchema(true)).Validate(schema.Example))
+		res.MergeAsWarnings(
+			newSchemaValidator(schema, s.spec.Spec(), path+".example", s.KnownFormats, ex.schemaOptions).Validate(schema.Example),
+		)
 	}
 	if schema.Items != nil {
 		if schema.Items.Schema != nil {
@@ -256,14 +270,17 @@ func (ex *exampleValidator) validateExampleValueItemsAgainstSchema(path, in stri
 	s := ex.SpecValidator
 	if items != nil {
 		if items.Example != nil {
-			res.MergeAsWarnings(newItemsValidator(path, in, items, root, s.KnownFormats).Validate(0, items.Example))
+			res.MergeAsWarnings(
+				newItemsValidator(path, in, items, root, s.KnownFormats, ex.schemaOptions).Validate(0, items.Example),
+			)
 		}
 		if items.Items != nil {
-			res.Merge(ex.validateExampleValueItemsAgainstSchema(path+"[0].example", in, root, items.Items))
+			res.Merge(ex.validateExampleValueItemsAgainstSchema(path+"[0].example", in, root, items.Items)) // TODO(fred): intern string
 		}
 		if _, err := compileRegexp(items.Pattern); err != nil {
 			res.AddErrors(invalidPatternInMsg(path, in, items.Pattern))
 		}
 	}
+
 	return res
 }

formats.go

@@ -22,48 +22,58 @@ import (
 )
 
 type formatValidator struct {
-	Format       string
 	Path         string
 	In           string
+	Format       string
 	KnownFormats strfmt.Registry
+	Options      *SchemaValidatorOptions
+}
+
+func newFormatValidator(path, in, format string, formats strfmt.Registry, opts *SchemaValidatorOptions) *formatValidator {
+	if opts == nil {
+		opts = new(SchemaValidatorOptions)
+	}
+
+	f := new(formatValidator)
+
+	f.Path = path
+	f.In = in
+	f.Format = format
+	f.KnownFormats = formats
+	f.Options = opts
+
+	return f
 }
 
 func (f *formatValidator) SetPath(path string) {
 	f.Path = path
 }
 
 func (f *formatValidator) Applies(source interface{}, kind reflect.Kind) bool {
-	doit := func() bool {
-		if source == nil {
-			return false
-		}
-		switch source := source.(type) {
-		case *spec.Items:
-			return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
-		case *spec.Parameter:
-			return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
-		case *spec.Schema:
-			return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
-		case *spec.Header:
-			return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
-		}
+	if source == nil || f.KnownFormats == nil {
+		return false
+	}
+
+	switch source := source.(type) {
+	case *spec.Items:
+		return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
+	case *spec.Parameter:
+		return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
+	case *spec.Schema:
+		return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
+	case *spec.Header:
+		return kind == reflect.String && f.KnownFormats.ContainsName(source.Format)
+	default:
 		return false
 	}
-	r := doit()
-	debugLog("format validator for %q applies %t for %T (kind: %v)\n", f.Path, r, source, kind)
-	return r
 }
 
 func (f *formatValidator) Validate(val interface{}) *Result {
 	result := new(Result)
-	debugLog("validating \"%v\" against format: %s", val, f.Format)
 
 	if err := FormatOf(f.Path, f.In, f.Format, val.(string), f.KnownFormats); err != nil {
 		result.AddErrors(err)
 	}
 
-	if result.HasErrors() {
-		return result
-	}
-	return nil
+	return result
 }