jsonschema validation

[patrickkwang]

Addresses #930.

Changes proposed in this pull request:

• use jsonschema for validation instead of openapi_spec_validator

[rafaelcaricio]

This PR relies on internet connection. Please change that.

[patrickkwang]

Many of the tests seem to fail because the OpenAPI specs used for testing are actually invalid.
For example: https://travis-ci.org/zalando/connexion/jobs/530330378#L787
If "in: path", then you must have "required: true":
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#parameterRequired
Perhaps openapi-spec-validator was not handling this correctly.

[dtkav]

Hey @patrickkwang - I'm excited for these changes - thanks for taking them on!

[hjacobs]

Could you make the tests pass?

[patrickkwang]

No, the tests will fail because many of them are based on the invalid simple schema: #936 (comment)

The tests are corrected in #1010, so this PR will be dependent on that one.

[dtkav]

It looks like the jsonschema version doesn't check the case where a parameter default is the wrong type, and this results in two failing tests.
This is something that we can check at runtime, but it is somewhat unfortunate. Overall, it's probably worth the simplicity of this change, but we'll have to remove those tests.
I'll see if folks upstream have any thoughts on this.

[patrickkwang]

The JSON Schema actually doesn't enforce that default values validate against the associated schema:
https://json-schema.org/understanding-json-schema/reference/generic.html#annotations
The OpenAPI specification is stricter:
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#schema-object
So jsonschema is doing what it ought to. Someone could fork it to enforce that extra OpenAPI requirement.

[dtkav]

Here's the relevant section from the spec.

Unlike JSON Schema, the value MUST conform to the defined type for the Schema Object defined at the same level. For example, if type is string, then default can be "foo" but cannot be 1.

This PR uses jsonschema to validate the openapi spec.
I think the jsonschema spec provided fails to validate that defaults are correct type properly.

I think this is a bug in their openapi spec validation as implemented with jsonschema.

(the language here is very confusing - ack!)

[dtkav]

Also of note, this doesn't completely alleviate the need for openapi-spec-validator yet. We are still using it for remote '$ref' resolution for yaml files.

[patrickkwang]

It turns out it's not too hard to extend jsonschema to enforce that all the default values validate against the associated (sub)schemas. The last two tests pass now! This instance_validator business feels janky, though; I intend to ask @Julian if there's a cleaner way to handle references when validating the defaults.

[dtkav]

nice! I tried to ask in the swagger irc if there's a way to do it in jsonschema, but didn't hear back.

[RobbeSneyders]

Hi @patrickkwang, I hope you're still interested in getting this merged!
The changes look good, I left one comment.
We also switched to the main branch, so would be great if you can retarget and rebase / merge.

[RobbeSneyders]

This method is now the same in both classes, so we should be able to lift this to the base Specification class.

[coveralls]

Coverage increased (+0.01%) to 97.034% when pulling 736ba53 on patrickkwang:master into 2229831 on zalando:main.

[RobbeSneyders]

Thanks! LGTM.

[vmarkovtsev]

How about del schema_string after loading openapi_schema, we waste quite some memory here...