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

feature: Add more discriminator error messages and return specific er… #394

Merged
merged 4 commits into from
Aug 16, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 40 additions & 26 deletions openapi3/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -824,10 +824,7 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val
if v == nil {
return foundUnresolvedRef(ref.Ref)
}
var oldfailfast bool
oldfailfast, settings.failfast = settings.failfast, true
err := v.visitJSON(settings, value)
settings.failfast = oldfailfast
if err == nil {
if settings.failfast {
return errSchema
Expand All @@ -841,33 +838,53 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val
}

if v := schema.OneOf; len(v) > 0 {
var discriminatorRef string
if schema.Discriminator != nil {
pn := schema.Discriminator.PropertyName
if valuemap, okcheck := value.(map[string]interface{}); okcheck {
discriminatorVal, okcheck := valuemap[pn]
if !okcheck {
return errors.New("input does not contain the discriminator property")
}

if discriminatorRef, okcheck = schema.Discriminator.Mapping[discriminatorVal.(string)]; len(schema.Discriminator.Mapping) > 0 && !okcheck {
return errors.New("input does not contain a valid discriminator value")
}
}
}

ok := 0
validationErrors := []error{}
for _, item := range v {
v := item.Value
if v == nil {
return foundUnresolvedRef(item.Ref)
}
var oldfailfast bool
oldfailfast, settings.failfast = settings.failfast, true

if discriminatorRef != "" && discriminatorRef != item.Ref {
continue
}

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++
}
if err != nil {
validationErrors = append(validationErrors, err)
continue
}

ok++
}

if ok != 1 {
if len(validationErrors) > 1 {
errorMessage := ""
for _, err := range validationErrors {
if errorMessage != "" {
errorMessage += " Or "
}
errorMessage += err.Error()
}
return errors.New("doesn't match schema due to: " + errorMessage)
}
if settings.failfast {
return errSchema
}
Expand All @@ -878,7 +895,10 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val
}
if ok > 1 {
e.Origin = ErrOneOfConflict
} else if len(validationErrors) == 1 {
e.Origin = validationErrors[0]
}
Comment on lines 896 to 900
Copy link
Contributor Author

@rtfpessoa rtfpessoa Aug 11, 2021

Choose a reason for hiding this comment

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

@fenollp to bring the details back we would have to change this. But this will also add extra details to other errors. And would probably never use the Schema error.

Suggested change
if ok > 1 {
e.Origin = ErrOneOfConflict
} else if len(validationErrors) == 1 {
e.Origin = validationErrors[0]
}
if ok > 1 {
return ErrOneOfConflict
} else if len(validationErrors) == 1 {
return validationErrors[0]
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well the loss of context isn't that huge, also this should all be replaced by a dedicated lib anyway so let's get on with this.


return e
}
}
Expand All @@ -890,10 +910,7 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val
if v == nil {
return foundUnresolvedRef(item.Ref)
}
var oldfailfast bool
oldfailfast, settings.failfast = settings.failfast, true
err := v.visitJSON(settings, value)
settings.failfast = oldfailfast
if err == nil {
ok = true
break
Expand All @@ -916,10 +933,7 @@ func (schema *Schema) visitSetOperations(settings *schemaValidationSettings, val
if v == nil {
return foundUnresolvedRef(item.Ref)
}
var oldfailfast bool
oldfailfast, settings.failfast = settings.failfast, false
err := v.visitJSON(settings, value)
settings.failfast = oldfailfast
if err != nil {
if settings.failfast {
return errSchema
Expand Down
118 changes: 118 additions & 0 deletions openapi3/schema_oneOf_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package openapi3

import (
"testing"

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

var oneofSpec = []byte(`components:
schemas:
Cat:
type: object
properties:
name:
type: string
scratches:
type: boolean
$type:
type: string
enum:
- cat
required:
- name
- scratches
- $type
Dog:
type: object
properties:
name:
type: string
barks:
type: boolean
$type:
type: string
enum:
- dog
required:
- name
- barks
- $type
Animal:
type: object
oneOf:
- $ref: "#/components/schemas/Cat"
- $ref: "#/components/schemas/Dog"
discriminator:
propertyName: $type
mapping:
cat: "#/components/schemas/Cat"
dog: "#/components/schemas/Dog"
`)

var oneofNoDiscriminatorSpec = []byte(`components:
schemas:
Cat:
type: object
properties:
name:
type: string
scratches:
type: boolean
required:
- name
- scratches
Dog:
type: object
properties:
name:
type: string
barks:
type: boolean
required:
- name
- barks
Animal:
type: object
oneOf:
- $ref: "#/components/schemas/Cat"
- $ref: "#/components/schemas/Dog"
`)

func TestVisitJSON_OneOf_MissingDiscriptorProperty(t *testing.T) {
s, err := NewLoader().LoadFromData(oneofSpec)
require.NoError(t, err)
err = s.Components.Schemas["Animal"].Value.VisitJSON(map[string]interface{}{
"name": "snoopy",
})
require.EqualError(t, err, "input does not contain the discriminator property")
}

func TestVisitJSON_OneOf_MissingDiscriptorValue(t *testing.T) {
s, err := NewLoader().LoadFromData(oneofSpec)
require.NoError(t, err)
err = s.Components.Schemas["Animal"].Value.VisitJSON(map[string]interface{}{
"name": "snoopy",
"$type": "snake",
})
require.EqualError(t, err, "input does not contain a valid discriminator value")
}

func TestVisitJSON_OneOf_MissingField(t *testing.T) {
s, err := NewLoader().LoadFromData(oneofSpec)
require.NoError(t, err)
err = s.Components.Schemas["Animal"].Value.VisitJSON(map[string]interface{}{
"name": "snoopy",
"$type": "dog",
})
require.EqualError(t, err, "Error at \"/barks\": property \"barks\" is missing\nSchema:\n {\n \"properties\": {\n \"$type\": {\n \"enum\": [\n \"dog\"\n ],\n \"type\": \"string\"\n },\n \"barks\": {\n \"type\": \"boolean\"\n },\n \"name\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"name\",\n \"barks\",\n \"$type\"\n ],\n \"type\": \"object\"\n }\n\nValue:\n {\n \"$type\": \"dog\",\n \"name\": \"snoopy\"\n }\n")
}

func TestVisitJSON_OneOf_NoDiscriptor_MissingField(t *testing.T) {
s, err := NewLoader().LoadFromData(oneofNoDiscriminatorSpec)
require.NoError(t, err)
err = s.Components.Schemas["Animal"].Value.VisitJSON(map[string]interface{}{
"name": "snoopy",
})
require.EqualError(t, err, "doesn't match schema due to: Error at \"/scratches\": property \"scratches\" is missing\nSchema:\n {\n \"properties\": {\n \"name\": {\n \"type\": \"string\"\n },\n \"scratches\": {\n \"type\": \"boolean\"\n }\n },\n \"required\": [\n \"name\",\n \"scratches\"\n ],\n \"type\": \"object\"\n }\n\nValue:\n {\n \"name\": \"snoopy\"\n }\n Or Error at \"/barks\": property \"barks\" is missing\nSchema:\n {\n \"properties\": {\n \"barks\": {\n \"type\": \"boolean\"\n },\n \"name\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"name\",\n \"barks\"\n ],\n \"type\": \"object\"\n }\n\nValue:\n {\n \"name\": \"snoopy\"\n }\n")
}
24 changes: 2 additions & 22 deletions routers/legacy/validate_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,26 +107,6 @@ 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"
// }
// request body has an error: doesn't match the schema: input matches more than one oneOf schemas

}