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

Enable jsonschema.validate() to validate responses. #1231

Closed
wants to merge 1 commit into from
Closed

Enable jsonschema.validate() to validate responses. #1231

wants to merge 1 commit into from

Conversation

ZDBioHazard
Copy link
Contributor

With this change, you can pass another schema dict to jsonschema.validate() as 'schema_response' to validate the response data against it.

A falcon.HTTPInternalServerError is raised on schema violations.

This is useful to make sure a bug in your code doesn't somehow accidentally cause your resource to return an out-of-spec response to users.

Let me know if you want me to change the name of the first parameter back. I think it's more specific as 'schema_request', but I understand it's possible some users may have explicitly keyworded it in their apps even though it was the only parameter.

I'm also open to splitting it into it's own 'validate_response' decorator.

Pass a schema dict to jsonschema.validate() as 'schema_response' and
the response data will be validated against it.

A falcon.HTTPInternalServerError is raised on schema violations.

This is useful to make sure a bug in your code doesn't cause
your resource to return an out-of-spec response to users.
@codecov
Copy link

codecov bot commented Mar 11, 2018

Codecov Report

Merging #1231 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1231   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          38      38           
  Lines        2484    2491    +7     
  Branches      364     366    +2     
======================================
+ Hits         2484    2491    +7
Impacted Files Coverage Δ
falcon/media/validators/jsonschema.py 100% <100%> (ø) ⬆️

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 800396f...e90a932. Read the comment docs.

Copy link
Member

@kgriffs kgriffs left a comment

Choose a reason for hiding this comment

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

Thanks, this looks like a great feature! I can include a note in the changelog about that first kwarg potentially causing a breaking change, but TBH it's a very low risk and this will go into a major new release version, so I don't think it will be a problem.


assert err.value.description == '\'message\' is a required property'
@skip_py26
Copy link
Member

Choose a reason for hiding this comment

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

Once you rebase on master, you can remove these @skip_py26 decorators, as we have removed 2.6 support.

@@ -8,7 +8,7 @@
pass


def validate(schema):
def validate(schema_request=None, schema_response=None):
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Would you mind swapping the word order in these arguments to follow the Modifier-Subject style, i.e.: request_schema and response_schema or req_schema and resp_schema?

@kgriffs
Copy link
Member

kgriffs commented Apr 23, 2018

I rebased and added a few additional patches on top, resubmitted as #1246

@kgriffs kgriffs closed this Apr 23, 2018
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.

2 participants