Support OpenAPI 3.0.0 specs #621
Conversation
tests/decorators/test_parameter.py
Outdated
|
|
||
| func.assert_called_with(p1='123') | ||
|
|
||
|
|
||
| def test_query_sanitazion(query_sanitazion): | ||
| app_client = query_sanitazion.app.test_client() | ||
| l = LogCapture() | ||
| #l = LogCapture() |
There was a problem hiding this comment.
This tests is pretty nasty - I'm thinking of just removing it.
tests/api/test_parameters.py
Outdated
| @@ -280,11 +290,13 @@ def test_nullable_parameter(simple_app): | |||
| '/v1.0/nullable-parameters?time_start={}'.format(time_start)) | |||
| assert json.loads(resp.data.decode('utf-8', 'replace')) == time_start | |||
|
|
|||
| resp = app_client.post('/v1.0/nullable-parameters', data={"post_param": 'None'}) | |||
| assert json.loads(resp.data.decode('utf-8', 'replace')) == 'it was None' | |||
| # XXX this has not valid OAS3 equivalent as far as I can tell | |||
There was a problem hiding this comment.
I could use help figuring out the correct behavior here.
It seems like there should be something that iterates through form parameters and replaces "null" or "None" with None? Does this need to be recursive? Is it only for form encoded request bodies? Can form parameters have recursive definitions, or are they guaranteed to be flat?
|
Another architecture improvement that would simplify the oas3 work in #627 |
dtkav
force-pushed
the
oas3
branch
4 times, most recently
from
64d674d
to
d79ae00
Compare
dtkav
force-pushed
the
oas3
branch
2 times, most recently
from
975567a
to
8ce099e
Compare
dtkav
force-pushed
the
dev-2.0
branch
2 times, most recently
from
0621fb8
to
99ac95f
Compare
dtkav
changed the title
connexion/decorators/validation.py
Outdated
| spec_params = self.schema.get('properties', {}).keys() | ||
| return validate_parameter_list(request_params, spec_params) | ||
|
|
||
| def coerce_types(self, param_schema, value, parameter_name=None): |
There was a problem hiding this comment.
I think I can refactor this code out a bit.
connexion/apis/abstract.py
Outdated
| @@ -150,6 +157,13 @@ def _set_base_path(self, base_path): | |||
| # type: (AnyStr) -> None | |||
| if base_path is None: | |||
| self.base_path = canonical_base_path(self.specification.get('basePath', '')) | |||
| if self.spec_version >= (3, 0, 0): | |||
There was a problem hiding this comment.
You have already checked the spec_version. Why not delegate to a validate_spec method?
There was a problem hiding this comment.
Sorry, I'm not sure what you are suggesting here. Could you elaborate?
Technically this version check doesn't actually need to be here (the servers field won't exist in any valid swagger 2 specs), but it is more explicit. I'm trying not to just casually mix swagger2/openapi3 specific code. Most of that separation happens in the Swagger2Operation/OpenAPIOperation classes, but this is one of the few places in AbstractAPI that needs to be "version aware" in order to maintain the current API.
connexion/apis/abstract.py
Outdated
| try: | ||
| version_string = spec.get('openapi') or spec.get('swagger') | ||
| except AttributeError: | ||
| raise InvalidSpecification('Unable to get spec version') |
There was a problem hiding this comment.
Show the bad string
raise XXXException("Unable to get spec version from: %r" % spec)
There was a problem hiding this comment.
I disagree with this one. If we get this exception, then it means we couldn't find either of openapi / swagger keys in the spec. Printing the whole spec here may dump several pages of text to their terminal, and it won't be useful because it won't show them the absence of the required field.
I'll happily make this error more verbose though - let me know if that would address your concern.
|
I'm going to to ahead and merge this (yay!), but it will only be in dev-2.0, so I'm happy to continue to address suggestions and fixes there. We'll make sure that branch is rock solid cutting the next major release. FYI the Codacy problems are due to the use of "assert" in the new unit tests. I've added a separate PR to remove those codacy checks. |
Fixes #420
Changes proposed in this pull request:
Architecture: