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

On validation error: add cause and context. #129

Merged
merged 1 commit into from Jan 11, 2022
Merged

On validation error: add cause and context. #129

merged 1 commit into from Jan 11, 2022

Conversation

JulienPalard
Copy link
Contributor

I often miss the context provided by jsonschema library, so I'm trying to add it.

Given the following erroneous schema (it misses a description):

---
openapi: 3.0.0

info:
  title: test
  description: test
  version: 0.0.1

paths:
  "/":
    get:
      description: Get the API root
      responses:
        200:
          content:
            application/json:
              schema:
                type: string

without this PR, openapi-spec-validator displays:

{'content': {'application/json': {'schema': {'type': 'string'}}}} is not valid under any of the given schemas

Failed validating 'oneOf' in schema['properties']['paths']['patternProperties']['^\\/']['patternProperties']['^(get|put|post|delete|options|head|patch|trace)$'
]['properties']['responses']['patternProperties']['^[1-5](?:\\d{2}|XX)$']:
    {'oneOf': [{'$ref': '#/definitions/Response'},
               {'$ref': '#/definitions/Reference'}]}

On instance['paths']['/']['get']['responses']['200']:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}

Basically telling « The path./.get.responses.200 is invalid: it should be a Response or a Reference. » without telling me about the missing description.

With this PR it gives:

{'content': {'application/json': {'schema': {'type': 'string'}}}} is not valid under any of the given schemas

Failed validating 'oneOf' in schema['properties']['paths']['patternProperties']['^\\/']['patternProperties']['^(get|put|post|delete|options|head|patch|trace)$']['properties']['responses']['patternProperties']['^[1-5](?:\\d{2}|XX)$']:
    {'oneOf': [{'$ref': '#/definitions/Response'},
               {'$ref': '#/definitions/Reference'}]}

On instance['paths']['/']['get']['responses']['200']:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}


Due to those subschema errors:

'description' is a required property

Failed validating 'required' in schema[0]:
    {'additionalProperties': False,
     'patternProperties': {'^x-': {}},
     'properties': {'content': {'additionalProperties': {'$ref': '#/definitions/MediaType'},
                                'type': 'object'},
                    'description': {'type': 'string'},
                    'headers': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Header'},
                                                                   {'$ref': '#/definitions/Reference'}]},
                                'type': 'object'},
                    'links': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Link'},
                                                                 {'$ref': '#/definitions/Reference'}]},
                              'type': 'object'}},
     'required': ['description'],
     'type': 'object'}

On instance:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}


'$ref' is a required property

Failed validating 'required' in schema[1]:
    {'patternProperties': {'^\\$ref$': {'format': 'uri-reference',
                                        'type': 'string'}},
     'required': ['$ref'],
     'type': 'object'}

On instance:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}

This is probably a bit verbose, but I like a lot seeing 'description' is a required property.

@JulienPalard
Copy link
Contributor Author

Oh I just remembered jsonschema validator had a best_match function!

I'm using it now so it's less floody / more readable:

{'content': {'application/json': {'schema': {'type': 'string'}}}} is not valid under any of the given schemas

Failed validating 'oneOf' in schema['properties']['paths']['patternProperties']['^\\/']['patternProperties']['^(get|put|post|delete|options|head|patch|trace)$']['properties']['responses']['patternProperties']['^[1-5](?:\\d{2}|XX)$']:
    {'oneOf': [{'$ref': '#/definitions/Response'},
               {'$ref': '#/definitions/Reference'}]}

On instance['paths']['/']['get']['responses']['200']:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}


Due to this subschema error:

'description' is a required property

Failed validating 'required' in schema[0]:
    {'additionalProperties': False,
     'patternProperties': {'^x-': {}},
     'properties': {'content': {'additionalProperties': {'$ref': '#/definitions/MediaType'},
                                'type': 'object'},
                    'description': {'type': 'string'},
                    'headers': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Header'},
                                                                   {'$ref': '#/definitions/Reference'}]},
                                'type': 'object'},
                    'links': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Link'},
                                                                 {'$ref': '#/definitions/Reference'}]},
                              'type': 'object'}},
     'required': ['description'],
     'type': 'object'}

On instance:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}

@JulienPalard
Copy link
Contributor Author

I'm not happy with the --all flag which could be misinterpreted as giving all errors, not just the first error with all causes.

Any idea?

@paulclarkaranz
Copy link

This made error messages slightly more useful when trying to deal with upgrading past 0.3.0 and running into some minor adaptive changes required.

@p1c2u
Copy link
Collaborator

p1c2u commented Jan 11, 2022

@JulienPalard

I'm not happy with the --all flag which could be misinterpreted as giving all errors, not just the first error with all causes.

Any idea?

Maybe --errors=all with default value best-match ?

@JulienPalard
Copy link
Contributor Author

Maybe --errors=all with default value best-match ?

Updated my PR with this, it would work for me.

@p1c2u
Copy link
Collaborator

p1c2u commented Jan 11, 2022

@JulienPalard thanks looks good, can you check failing test?

@JulienPalard
Copy link
Contributor Author

I'm having a look at the failing test.

@JulienPalard
Copy link
Contributor Author

JulienPalard commented Jan 11, 2022

I added a test and tried to make the message a bit readable (having all the subschema dumped in a serie was a bit hard to digest). It currently looks like:

$ python openapi_spec_validator tests/integration/data/v3.0/missing-description.yaml 
# Validation Error

{'content': {'application/json': {'schema': {'type': 'string'}}}} is not valid under any of the given schemas

Failed validating 'oneOf' in schema['properties']['paths']['patternProperties']['^\\/']['patternProperties']['^(get|put|post|delete|options|head|patch|trace)$']['properties']['responses']['patternProperties']['^[1-5](?:\\d{2}|XX)$']:
    {'oneOf': [{'$ref': '#/definitions/Response'},
               {'$ref': '#/definitions/Reference'}]}

On instance['paths']['/']['get']['responses']['200']:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}


# Probably due to this subschema error

## 'description' is a required property

Failed validating 'required' in schema[0]:
    {'additionalProperties': False,
     'patternProperties': {'^x-': {}},
     'properties': {'content': {'additionalProperties': {'$ref': '#/definitions/MediaType'},
                                'type': 'object'},
                    'description': {'type': 'string'},
                    'headers': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Header'},
                                                                   {'$ref': '#/definitions/Reference'}]},
                                'type': 'object'},
                    'links': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Link'},
                                                                 {'$ref': '#/definitions/Reference'}]},
                              'type': 'object'}},
     'required': ['description'],
     'type': 'object'}

On instance:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}

(1 more subschemas errors, use --errors=all to see them.)
$ python openapi_spec_validator tests/integration/data/v3.0/missing-description.yaml --errors=all
# Validation Error

{'content': {'application/json': {'schema': {'type': 'string'}}}} is not valid under any of the given schemas

Failed validating 'oneOf' in schema['properties']['paths']['patternProperties']['^\\/']['patternProperties']['^(get|put|post|delete|options|head|patch|trace)$']['properties']['responses']['patternProperties']['^[1-5](?:\\d{2}|XX)$']:
    {'oneOf': [{'$ref': '#/definitions/Response'},
               {'$ref': '#/definitions/Reference'}]}

On instance['paths']['/']['get']['responses']['200']:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}


# Due to one of those errors

## 'description' is a required property

Failed validating 'required' in schema[0]:
    {'additionalProperties': False,
     'patternProperties': {'^x-': {}},
     'properties': {'content': {'additionalProperties': {'$ref': '#/definitions/MediaType'},
                                'type': 'object'},
                    'description': {'type': 'string'},
                    'headers': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Header'},
                                                                   {'$ref': '#/definitions/Reference'}]},
                                'type': 'object'},
                    'links': {'additionalProperties': {'oneOf': [{'$ref': '#/definitions/Link'},
                                                                 {'$ref': '#/definitions/Reference'}]},
                              'type': 'object'}},
     'required': ['description'],
     'type': 'object'}

On instance:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}


## '$ref' is a required property

Failed validating 'required' in schema[1]:
    {'patternProperties': {'^\\$ref$': {'format': 'uri-reference',
                                        'type': 'string'}},
     'required': ['$ref'],
     'type': 'object'}

On instance:
    {'content': {'application/json': {'schema': {'type': 'string'}}}}

I find the "markdown-y" ## a good help to visually iterate the list of errors.

@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #129 (2dcf445) into master (afb6024) will decrease coverage by 0.32%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
- Coverage   98.46%   98.14%   -0.33%     
==========================================
  Files          19       19              
  Lines         520      538      +18     
==========================================
+ Hits          512      528      +16     
- Misses          8       10       +2     
Impacted Files Coverage Δ
openapi_spec_validator/__main__.py 94.23% <89.47%> (-2.83%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afb6024...2dcf445. Read the comment docs.

Copy link
Collaborator

@p1c2u p1c2u left a comment

Choose a reason for hiding this comment

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

LGTM

@p1c2u
Copy link
Collaborator

p1c2u commented Jan 11, 2022

@JulienPalard thank you for the contribution

@p1c2u p1c2u merged commit e23cb90 into python-openapi:master Jan 11, 2022
@JulienPalard JulienPalard deleted the show-context branch January 11, 2022 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants