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.

Sign up for GitHub

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

Jump to bottom

Use mapping to directly validate mapped schema #338

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Use mapping to directly validate mapped schema #338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
45d4919
Avoid failfast and return proper error when mapping is available
riccardomanfrin Apr 7, 2021
be8af26
Revert "Adds oneOf/discriminator/mapping management (#321)"
riccardomanfrin Apr 13, 2021
9aa5c5a
Fixup missing Test prefix
riccardomanfrin Apr 13, 2021
146d4b9
Adds implicit mapping case
riccardomanfrin Apr 13, 2021
b343c1d
Cleaner [I'll squash this later]
riccardomanfrin Apr 13, 2021
f937147
Compliance
riccardomanfrin Apr 23, 2021
7800000
Revert "Fixup missing Test prefix"
riccardomanfrin Apr 23, 2021
781e68f
Now test passes
riccardomanfrin Apr 26, 2021
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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).

}