Skip to content

Commit

Permalink
fix: optional read only and write only validations (#689)
Browse files Browse the repository at this point in the history
  • Loading branch information
orshlom committed Jan 31, 2023
1 parent 6e233af commit f2d73ad
Show file tree
Hide file tree
Showing 7 changed files with 308 additions and 9 deletions.
107 changes: 107 additions & 0 deletions openapi3/issue689_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package openapi3_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/getkin/kin-openapi/openapi3"
)

func TestIssue689(t *testing.T) {
t.Parallel()

tests := [...]struct {
name string
schema *openapi3.Schema
value map[string]interface{}
opts []openapi3.SchemaValidationOption
checkErr require.ErrorAssertionFunc
}{
// read-only
{
name: "read-only property succeeds when read-only validation is disabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", ReadOnly: true}}),
value: map[string]interface{}{"foo": true},
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsRequest(),
openapi3.DisableReadOnlyValidation()},
checkErr: require.NoError,
},
{
name: "non read-only property succeeds when read-only validation is disabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", ReadOnly: false}}),
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsRequest()},
value: map[string]interface{}{"foo": true},
checkErr: require.NoError,
},
{
name: "read-only property fails when read-only validation is enabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", ReadOnly: true}}),
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsRequest()},
value: map[string]interface{}{"foo": true},
checkErr: require.Error,
},
{
name: "non read-only property succeeds when read-only validation is enabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", ReadOnly: false}}),
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsRequest()},
value: map[string]interface{}{"foo": true},
checkErr: require.NoError,
},
// write-only
{
name: "write-only property succeeds when write-only validation is disabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", WriteOnly: true}}),
value: map[string]interface{}{"foo": true},
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsResponse(),
openapi3.DisableWriteOnlyValidation()},
checkErr: require.NoError,
},
{
name: "non write-only property succeeds when write-only validation is disabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", WriteOnly: false}}),
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsResponse()},
value: map[string]interface{}{"foo": true},
checkErr: require.NoError,
},
{
name: "write-only property fails when write-only validation is enabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", WriteOnly: true}}),
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsResponse()},
value: map[string]interface{}{"foo": true},
checkErr: require.Error,
},
{
name: "non write-only property succeeds when write-only validation is enabled",
schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{
"foo": {Type: "boolean", WriteOnly: false}}),
opts: []openapi3.SchemaValidationOption{
openapi3.VisitAsResponse()},
value: map[string]interface{}{"foo": true},
checkErr: require.NoError,
},
}

for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()
err := test.schema.VisitJSON(test.value, test.opts...)
test.checkErr(t, err)
})
}
}
4 changes: 2 additions & 2 deletions openapi3/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -1787,8 +1787,8 @@ func (schema *Schema) visitJSONObject(settings *schemaValidationSettings, value
sort.Strings(properties)
for _, propName := range properties {
propSchema := schema.Properties[propName]
reqRO := settings.asreq && propSchema.Value.ReadOnly
repWO := settings.asrep && propSchema.Value.WriteOnly
reqRO := settings.asreq && propSchema.Value.ReadOnly && !settings.readOnlyValidationDisabled
repWO := settings.asrep && propSchema.Value.WriteOnly && !settings.writeOnlyValidationDisabled

if value[propName] == nil {
if dlft := propSchema.Value.Default; dlft != nil && !reqRO && !repWO {
Expand Down
22 changes: 17 additions & 5 deletions openapi3/schema_validation_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import (
type SchemaValidationOption func(*schemaValidationSettings)

type schemaValidationSettings struct {
failfast bool
multiError bool
asreq, asrep bool // exclusive (XOR) fields
formatValidationEnabled bool
patternValidationDisabled bool
failfast bool
multiError bool
asreq, asrep bool // exclusive (XOR) fields
formatValidationEnabled bool
patternValidationDisabled bool
readOnlyValidationDisabled bool
writeOnlyValidationDisabled bool

onceSettingDefaults sync.Once
defaultsSet func()
Expand Down Expand Up @@ -47,6 +49,16 @@ func DisablePatternValidation() SchemaValidationOption {
return func(s *schemaValidationSettings) { s.patternValidationDisabled = true }
}

// DisableReadOnlyValidation setting makes Validate not return an error when validating properties marked as read-only
func DisableReadOnlyValidation() SchemaValidationOption {
return func(s *schemaValidationSettings) { s.readOnlyValidationDisabled = true }
}

// DisableWriteOnlyValidation setting makes Validate not return an error when validating properties marked as write-only
func DisableWriteOnlyValidation() SchemaValidationOption {
return func(s *schemaValidationSettings) { s.writeOnlyValidationDisabled = true }
}

// DefaultsSet executes the given callback (once) IFF schema validation set default values.
func DefaultsSet(f func()) SchemaValidationOption {
return func(s *schemaValidationSettings) { s.defaultsSet = f }
Expand Down
168 changes: 168 additions & 0 deletions openapi3filter/issue689_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
package openapi3filter

import (
"io"
"net/http"
"strings"
"testing"

"github.com/stretchr/testify/require"

"github.com/getkin/kin-openapi/openapi3"
"github.com/getkin/kin-openapi/routers/gorillamux"
)

func TestIssue689(t *testing.T) {
loader := openapi3.NewLoader()
ctx := loader.Context
spec := `
openapi: 3.0.0
info:
version: 1.0.0
title: Sample API
paths:
/items:
put:
requestBody:
content:
application/json:
schema:
properties:
testWithReadOnly:
readOnly: true
type: boolean
testNoReadOnly:
type: boolean
type: object
responses:
'200':
description: OK
get:
responses:
'200':
description: OK
content:
application/json:
schema:
properties:
testWithWriteOnly:
writeOnly: true
type: boolean
testNoWriteOnly:
type: boolean
`[1:]

doc, err := loader.LoadFromData([]byte(spec))
require.NoError(t, err)

err = doc.Validate(ctx)
require.NoError(t, err)

router, err := gorillamux.NewRouter(doc)
require.NoError(t, err)

tests := []struct {
name string
options *Options
body string
method string
checkErr require.ErrorAssertionFunc
}{
// read-only
{
name: "non read-only property is added to request when validation enabled",
body: `{"testNoReadOnly": true}`,
method: http.MethodPut,
checkErr: require.NoError,
},
{
name: "non read-only property is added to request when validation disabled",
body: `{"testNoReadOnly": true}`,
method: http.MethodPut,
options: &Options{
ExcludeReadOnlyValidations: true,
},
checkErr: require.NoError,
},
{
name: "read-only property is added to requests when validation enabled",
body: `{"testWithReadOnly": true}`,
method: http.MethodPut,
checkErr: require.Error,
},
{
name: "read-only property is added to requests when validation disabled",
body: `{"testWithReadOnly": true}`,
method: http.MethodPut,
options: &Options{
ExcludeReadOnlyValidations: true,
},
checkErr: require.NoError,
},
// write-only
{
name: "non write-only property is added to request when validation enabled",
body: `{"testNoWriteOnly": true}`,
method: http.MethodGet,
checkErr: require.NoError,
},
{
name: "non write-only property is added to request when validation disabled",
body: `{"testNoWriteOnly": true}`,
method: http.MethodGet,
options: &Options{
ExcludeWriteOnlyValidations: true,
},
checkErr: require.NoError,
},
{
name: "write-only property is added to requests when validation enabled",
body: `{"testWithWriteOnly": true}`,
method: http.MethodGet,
checkErr: require.Error,
},
{
name: "write-only property is added to requests when validation disabled",
body: `{"testWithWriteOnly": true}`,
method: http.MethodGet,
options: &Options{
ExcludeWriteOnlyValidations: true,
},
checkErr: require.NoError,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
httpReq, err := http.NewRequest(test.method, "/items", strings.NewReader(test.body))
require.NoError(t, err)
httpReq.Header.Set("Content-Type", "application/json")
require.NoError(t, err)

route, pathParams, err := router.FindRoute(httpReq)
require.NoError(t, err)

requestValidationInput := &RequestValidationInput{
Request: httpReq,
PathParams: pathParams,
Route: route,
Options: test.options,
}

if test.method == http.MethodGet {
responseValidationInput := &ResponseValidationInput{
RequestValidationInput: requestValidationInput,
Status: 200,
Header: httpReq.Header,
Body: io.NopCloser(strings.NewReader(test.body)),
Options: test.options,
}
err = ValidateResponse(ctx, responseValidationInput)

} else {
err = ValidateRequest(ctx, requestValidationInput)
}
test.checkErr(t, err)
})
}
}
6 changes: 6 additions & 0 deletions openapi3filter/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ type Options struct {
// Set ExcludeResponseBody so ValidateResponse skips response body validation
ExcludeResponseBody bool

// Set ExcludeReadOnlyValidations so ValidateRequest skips read-only validations
ExcludeReadOnlyValidations bool

// Set ExcludeWriteOnlyValidations so ValidateResponse skips write-only validations
ExcludeWriteOnlyValidations bool

// Set IncludeResponseStatus so ValidateResponse fails on response
// status not defined in OpenAPI spec
IncludeResponseStatus bool
Expand Down
5 changes: 4 additions & 1 deletion openapi3filter/validate_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func ValidateRequestBody(ctx context.Context, input *RequestValidationInput, req
}

defaultsSet := false
opts := make([]openapi3.SchemaValidationOption, 0, 3) // 3 potential opts here
opts := make([]openapi3.SchemaValidationOption, 0, 4) // 4 potential opts here
opts = append(opts, openapi3.VisitAsRequest())
if !options.SkipSettingDefaults {
opts = append(opts, openapi3.DefaultsSet(func() { defaultsSet = true }))
Expand All @@ -283,6 +283,9 @@ func ValidateRequestBody(ctx context.Context, input *RequestValidationInput, req
if options.customSchemaErrorFunc != nil {
opts = append(opts, openapi3.SetSchemaErrorMessageCustomizer(options.customSchemaErrorFunc))
}
if options.ExcludeReadOnlyValidations {
opts = append(opts, openapi3.DisableReadOnlyValidation())
}

// Validate JSON with the schema
if err := contentType.Schema.Value.VisitJSON(value, opts...); err != nil {
Expand Down
5 changes: 4 additions & 1 deletion openapi3filter/validate_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,16 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error
return &ResponseError{Input: input, Reason: "response has not been resolved"}
}

opts := make([]openapi3.SchemaValidationOption, 0, 2)
opts := make([]openapi3.SchemaValidationOption, 0, 3) // 3 potential options here
if options.MultiError {
opts = append(opts, openapi3.MultiErrors())
}
if options.customSchemaErrorFunc != nil {
opts = append(opts, openapi3.SetSchemaErrorMessageCustomizer(options.customSchemaErrorFunc))
}
if options.ExcludeWriteOnlyValidations {
opts = append(opts, openapi3.DisableWriteOnlyValidation())
}

headers := make([]string, 0, len(response.Headers))
for k := range response.Headers {
Expand Down

0 comments on commit f2d73ad

Please sign in to comment.