Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support default value binding with allOfSchema #885

Merged
merged 5 commits into from
Dec 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
113 changes: 113 additions & 0 deletions openapi3filter/issue884_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package openapi3filter

import (
"net/http"
"net/url"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

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

func TestIssue884(t *testing.T) {
loader := openapi3.NewLoader()
ctx := loader.Context
spec := `
openapi: 3.0.0
info:
version: 1.0.0
title: Sample API
components:
schemas:
TaskSortEnum:
enum:
- createdAt
- -createdAt
- updatedAt
- -updatedAt
paths:
/tasks:
get:
operationId: ListTask
parameters:
- in: query
name: withDefault
schema:
allOf:
- $ref: '#/components/schemas/TaskSortEnum'
- default: -createdAt
- in: query
name: withoutDefault
schema:
allOf:
- $ref: '#/components/schemas/TaskSortEnum'
- in: query
name: withManyDefaults
schema:
allOf:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this definition should be schema validation error ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unable to find in the spec 脿 justification to "forbid multiple schema defaults". But if you find one then yes let's impl this validation pass.

Otherwise let's continue to not return a validation error here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is not required according to the specifications.

- default: -updatedAt
- $ref: '#/components/schemas/TaskSortEnum'
- default: -createdAt
responses:
'200':
description: Successful response
`[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
expectedQuery url.Values
}{
{
name: "no defaults are added to requests",
options: &Options{
SkipSettingDefaults: true,
},
expectedQuery: url.Values{},
},

{
name: "defaults are added to requests",
expectedQuery: url.Values{
"withDefault": []string{"-createdAt"},
"withManyDefaults": []string{"-updatedAt"}, // first default is win
fenollp marked this conversation as resolved.
Show resolved Hide resolved
},
},
}

for _, testcase := range tests {
t.Run(testcase.name, func(t *testing.T) {
httpReq, err := http.NewRequest(http.MethodGet, "/tasks", nil)
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: testcase.options,
}
err = ValidateRequest(ctx, requestValidationInput)
require.NoError(t, err)

q := httpReq.URL.Query()
assert.Equal(t, testcase.expectedQuery, q)
})
}
}
43 changes: 26 additions & 17 deletions openapi3filter/validate_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,24 +136,33 @@ func ValidateParameter(ctx context.Context, input *RequestValidationInput, param
}

// Set default value if needed
if !options.SkipSettingDefaults && value == nil && schema != nil && schema.Default != nil {
if !options.SkipSettingDefaults && value == nil && schema != nil {
value = schema.Default
req := input.Request
switch parameter.In {
case openapi3.ParameterInPath:
// Path parameters are required.
// Next check `parameter.Required && !found` will catch this.
case openapi3.ParameterInQuery:
q := req.URL.Query()
q.Add(parameter.Name, fmt.Sprintf("%v", value))
req.URL.RawQuery = q.Encode()
case openapi3.ParameterInHeader:
req.Header.Add(parameter.Name, fmt.Sprintf("%v", value))
case openapi3.ParameterInCookie:
req.AddCookie(&http.Cookie{
Name: parameter.Name,
Value: fmt.Sprintf("%v", value),
})
for _, subSchema := range schema.AllOf {
if subSchema.Value.Default != nil {
value = subSchema.Value.Default
break // This is not a validation of the schema itself, so use the first default value.
}
}

if value != nil {
req := input.Request
switch parameter.In {
case openapi3.ParameterInPath:
// Path parameters are required.
// Next check `parameter.Required && !found` will catch this.
case openapi3.ParameterInQuery:
q := req.URL.Query()
q.Add(parameter.Name, fmt.Sprintf("%v", value))
req.URL.RawQuery = q.Encode()
case openapi3.ParameterInHeader:
req.Header.Add(parameter.Name, fmt.Sprintf("%v", value))
case openapi3.ParameterInCookie:
req.AddCookie(&http.Cookie{
Name: parameter.Name,
Value: fmt.Sprintf("%v", value),
})
}
}
}

Expand Down