Skip to content

adds optional config param for OpenAPI version 3 #174

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

Merged
merged 8 commits into from
Jan 5, 2018

Conversation

Bangertm
Copy link
Collaborator

@Bangertm Bangertm commented Dec 5, 2017

This is an intial attempt to add basic support for OpenApi 3.
Based on the value of the openapi_version field, the APISpec.todict
places schemas either directly in "definitions" (v2) or in
"components/schemas" (v3)

References for nested Marshmallow schemas are also handled

Begins work on #165

See https://blog.readme.io/an-example-filled-guide-to-swagger-3-2/

@Bangertm
Copy link
Collaborator Author

Bangertm commented Dec 5, 2017

These changes plus updating the docstrings on our view functions to support OpenAPI 3 was able to generate valid OpenAPI 3 output.

I used this this tool to convert the Swagger 2 output from APISpec to OpenAPI 3 format. Then updated the docstrings and made changes to APISpec to generate similar output that I verified in Swagger Editor.

@lafrech
Copy link
Member

lafrech commented Dec 5, 2017

Great. Thanks for tackling this.

Shouldn't it be

       elif self.openapi_version == '3.0.0':
           ret['openapi'] = self.openapi_version
           ret['components'] = {'schemas': self._definitions, 'parameters': self._parameters}

?

@Bangertm
Copy link
Collaborator Author

Bangertm commented Dec 5, 2017

Ultimately yes, parameters should be put in the 'components' dictionary.

I was going to do that, but noticed there is some functionality here that generates refs to parameters when the parameters is specified as just a name in the docstring. See this commit.

It looks like to generate valid OpenAPI 3 refs we would have to pass in the version to Path.init. Didn't want to make that change until I understood a little better how things were working, which I think I do now.

Also I should note, the assumption that is being made with this commit is that the only definitions being added to the spec are schemas. That's apparently not strictly true in OpenAPI 2, but it is the way I use APISpec.

apispec/core.py Outdated
@@ -16,20 +16,30 @@
'options',
]

OPENAPI_VERSION = SWAGGER_VERSION = '2.0'
OPENAPI_VERSIONS = ('2.0', '3.0.0')
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if users could specify just the major or major.minor version , e.g. 2, 3, 3.0.

Maybe you could use distutils.version.LooseVersion to parse the version.

@sloria
Copy link
Member

sloria commented Dec 10, 2017

Is there a way we can validate against the 3.0 spec in a test? Can utils.validate_swagger be modified to support OpenAPI 3?

@sloria
Copy link
Member

sloria commented Dec 10, 2017

This is looking great! Just a few suggestions above. Also, please get this up to date with the dev branch.

Once those are done, I think this good to merge.

This is an intial attempt to add basic support for OpenApi 3.
Based on the value of the openapi_version field, the APISpec.todict
places schemas either directly in "definitions" (v2) or in
"components/schemas" (v3)

References for nested Marshmallow schemas are also handled

See https://blog.readme.io/an-example-filled-guide-to-swagger-3-2/
Now minor version numbers are used to determining the
placement of schemas and paramaters in the dictionary
@Bangertm Bangertm force-pushed the open-api-3 branch 2 times, most recently from 487d917 to 0241b24 Compare December 12, 2017 05:00
@Bangertm
Copy link
Collaborator Author

Using distutils.version.LooseVersion to validate the version input by the user. The check I put in is to make sure the version is greater than or equal to 2.0 and less than 4.0.0. That leaves it up the user input the version that they want in the output dictionary.

It looks like the swagger validation tool we were previously using does not support OpenAPI 3. I found a list of validation tools here, but haven't had a chance to take a look at any of them.

Switches to using npm package check_api, which is a wrapper around
several API specification validators including OpenAPI 2 and 3.
swagger-tools is a depencency of check_api
@Bangertm
Copy link
Collaborator Author

Bangertm commented Jan 3, 2018

Added a test for an OpenAPI 3 spec. To do this I swapped out the swagger-tools npm package for check_api, which supports both OpenAPI 2 and 3 (and uses swagger-tools as a dependency).

Also added some additional functionality to resolve Marshmallow Schemas from their new locations in the requestBody and response dictionaries. Right now the resolver is not raising an exception if it finds a schema in the wrong place based on the version, the output will just be invalid.

The one other place that schemas can be in OpenAPI 3 are in parameters query, path, etc. (just not body). But these schema dictionaries need to be formatted differently in 3 than they were in 2. See query and path parameters on this blog. At this point I haven't worked out a good way to make this change. So I'd like to defer that.

@sloria
Copy link
Member

sloria commented Jan 5, 2018

This looks great. Thanks for your hard work and patience!

@sloria sloria merged commit 55daf13 into marshmallow-code:dev Jan 5, 2018
@sloria
Copy link
Member

sloria commented Jan 5, 2018

This is released in 0.29.0.

I've also given you write access to the repo. No pressure to do any more work than you already have; just want to open the door to further collaboration. =)

@sloria
Copy link
Member

sloria commented Feb 2, 2019

@Bangertm Since you're a frequent contributor to apispec, I'd like to invite you to the marshmallow-code contributor Slack. If you're interested, send me an email at sloria1 AT gmail DOT com.

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.

3 participants