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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use mapping to directly validate mapped schema #338

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 31 additions & 13 deletions openapi3/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"math/big"
"regexp"
"strconv"
"strings"
"unicode/utf16"

"github.com/getkin/kin-openapi/jsoninfo"
Expand Down Expand Up @@ -837,6 +838,35 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val
}

if v := schema.OneOf; len(v) > 0 {

if schema.Discriminator != nil {
/* Find mapped object by ref */
if valuemap, okcheck := value.(map[string]interface{}); okcheck {
pn := schema.Discriminator.PropertyName
if discriminatorVal, okcheck := valuemap[pn]; okcheck {
if len(schema.Discriminator.Mapping) > 0 {
if mapref, okcheck := schema.Discriminator.Mapping[discriminatorVal.(string)]; okcheck {
fenollp marked this conversation as resolved.
Show resolved Hide resolved
for _, oneof := range v {
if oneof.Ref == mapref {
return oneof.Value.visitJSON(settings, value)
}
}
}
} else {
/* Assume implicit mapping on objectType as stated in Mapping Type Names section:
``It is implied, that the property to which discriminator refers, contains the
name of the target schema. In the example above, the objectType property should
contain either simpleObject, or complexObject string.''*/
for _, v := range schema.OneOf {
if strings.HasSuffix(v.Ref, discriminatorVal.(string)) {
return v.Value.visitJSON(settings, value)
}
}
}
}
}
}

ok := 0
for _, item := range v {
v := item.Value
Expand All @@ -848,19 +878,7 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val
err := v.visitJSON(settings, value)
settings.failfast = oldfailfast
if err == nil {
if schema.Discriminator != nil {
pn := schema.Discriminator.PropertyName
if valuemap, okcheck := value.(map[string]interface{}); okcheck {
if discriminatorVal, okcheck := valuemap[pn]; okcheck == true {
mapref, okcheck := schema.Discriminator.Mapping[discriminatorVal.(string)]
if okcheck && mapref == item.Ref {
ok++
}
}
}
} else {
ok++
}
ok++
}
}
if ok != 1 {
Expand Down
22 changes: 0 additions & 22 deletions openapi3filter/validate_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,26 +107,4 @@ func Example() {
fmt.Println(err)
}
// Output:
// request body has an error: doesn't match the schema: Doesn't match schema "oneOf"
// Schema:
// {
// "discriminator": {
// "propertyName": "pet_type"
// },
// "oneOf": [
// {
// "$ref": "#/components/schemas/Cat"
// },
// {
// "$ref": "#/components/schemas/Dog"
// }
// ]
// }
//
// Value:
// {
// "bark": true,
// "breed": "Dingo",
// "pet_type": "Cat"
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand the request data should be invalid per the provided schema. What am I missing?

The test used to not pass because there was no output, because no error is returned. Are you sure this should not output anything?

Copy link
Contributor Author

@riccardomanfrin riccardomanfrin Apr 27, 2021

Choose a reason for hiding this comment

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

This relates to your original question about not being sure regarding the presence of the "else" statement:

Quoting from https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

"It is implied, that the property to which discriminator refers, contains the name of the target schema. In the example above, the objectType property should contain either simpleObject, or complexObject string. If the property values do not match the schema names, you can map the values to the names. To do this, use the discriminator/mapping keyword:"

I'm interpreting this as saying that if you don't specify the mapping the default mapping is based on the names of the objects included in the oneOf list: simpleObject, complexObject in the spec doc, Dog or Cat in the Example() from validate_request_test.go.

Because of that "else", ValidateRequest() is not anymore returning an error in the Example() code, and therefore I had to modify the output to be empty, accordingly.

Let me know if you believe I'm getting it all wrong or it makes sense.

Side note: the fact that now a "barking cat" is legitimate in the Example() can be ruled out with other means (additionalProperties and required keyword props). This would possibly make the Example more meaningful (and getting it back to fail again).

}