-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Added OpenAPI Schema Generation. #6532
Conversation
9d91a27
to
c0a31ed
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@carltongibson I am very interested in using openapi 3.0 with DRF and specifically DJA. Swagger 2 is unable to represent JSON:API but OAS 3 is able. I've manually written a partial JSON:API 1.0 OAS 3.0.2 schema that seems to work using swagger-editor (still a WIP). I did just try using the latest commit in this PR (not sure that's the right one to be looking at?) against this example app and still hit a few errors:
Any pointers would be greatly appreciated. I've added this issue to track needed DJA enhancements to follow your DRF OAS enhancements. |
Hi @n2ygk. Super. Thanks. At this stage all real-world input is great, as it helps us get the details right first time...
Yeah, right. So this would be a perfect place for some input on Django-Filter. The minimum is to subclass the backend and add a stub (returning
Your |
Hi @carltongibson, I found another problem with the current generation that prevents any requests from an API client (no Server Object on the schema); I will make another PR for that later. Also, I had another question: OpenAPI handles authentication using security schemes (basically authentication classes) and security requirements ( |
This comment has been minimized.
This comment has been minimized.
@carltongibson @Lucidiot here's an example of a security scheme that I'm currently hand-editing that allows django basic auth or oauth.
This corresponds to this DJA code fragment:
|
I did a quick test locally on our project which uses django_fields amongst other things. I added
to DjangoFilterSetBackend as suggested, and hit a new issue with generating the schema:
so it might have issues with from |
Could be related to our homebrewn permission system which returns different types of serializers from get_serializer dependent on the request user and which object you're trying to access. |
@aleksanb. Super. Can I ask you to put this in a separate ticket and we'll handle it there? Thanks for reporting, and for giving it a run! |
e61af55
to
bb0db35
Compare
@carltongibson I've started looking at what it would take to extend See below for an illustration with a comparison of what generateschema creates (parameters, etc removed) vs. a hand-coded example of what is needed for jsonapi: What's currently generated: info:
title: ''
version: TODO
openapi: 3.0.2
paths:
//course_terms/:
get:
operationId: ListCourseTerms
responses:
'200':
content:
application/json:
schema:
properties:
audit_permitted_code:
type: integer
course:
nullable: true
type: string
effective_end_date:
nullable: true
type: date
effective_start_date:
nullable: true
type: date
exam_credit_flag:
type: boolean
instructors:
items:
type: string
type: array
last_mod_date:
readOnly: true
type: date
last_mod_user_name:
readOnly: true
type: string
term_identifier:
type: string
url:
readOnly: true
type: string
required:
- term_identifier What's needed (portions incomplete): info:
title: ''
version: TODO
openapi: 3.0.2
paths:
//course_terms/:
get:
operationId: ListCourseTerms
responses:
'200':
description: get response
content:
application/vnd.api+json:
schema:
type: object
required:
- data
properties:
data:
type: array
nullable: true
items:
type: object
required:
- type
- id
properties:
type:
type: string
id:
type: string
attributes:
type: object
required:
- term_identifier
properties:
audit_permitted_code:
type: integer
course:
nullable: true
type: string
effective_end_date:
nullable: true
type: date-time
effective_start_date:
nullable: true
type: date-time
exam_credit_flag:
type: boolean
instructors:
items:
type: string
type: array
last_mod_date:
readOnly: true
type: date-time
last_mod_user_name:
readOnly: true
type: string
term_identifier:
type: string
url:
readOnly: true
type: string
relationships:
type: object
properties:
links:
type: object
properties:
meta:
type: object
properties:
included:
type: array
uniqueItems: true
items:
type: object
links:
type: array
uniqueItems: true
items:
type: object
jsonapi:
type: object
properties:
version:
type: string
meta:
type: object |
Hi @carltongibson, I did some quick tests on your branch. Here's my test project: https://github.com/samh/drf_test_openapi_schema I found one validation issue (validating with https://editor.swagger.io/ or other validators such as Here are the elements I found to be missing:
Thanks for your work on this - I've been looking forward to this feature for a while! |
@carltongibson FYI, I've started trying to extend this for jsonapi here: columbia-it/django-jsonapi-training#1. Will provide feedback/PRs once I get this more baked. |
@carltongibson I was looking at making a view-specific schema generator (rather than just overriding the DEFAULT_SCHEMA_CLASS) since it depends on how the view class is defined to determine how the openapi schema should look for that specific path, just as the view class determines the serialization. For example, it would be a mistake to apply rest_framework_jsonapi formatting to a plain rest_framework-based view just as it would be wrong to apply rest_framework formatting to a rest_framework_jsonapi view. However it is perfectly legal to have both flavors (and others) as urlpatterns views. I thought perhaps I would override the rest_framework.views.APIView.schema attribute: django-rest-framework/rest_framework/views.py Line 119 in 0f819d8
but since that is an instance returned by schemas.DefaultSchema() rather than a class, it's hard to just declare it in my CBV without basically replicating this: django-rest-framework/rest_framework/schemas/inspectors.py Lines 496 to 497 in 0f819d8
Perhaps add a
What do you think? Hmm, scrolling back, it looks like I've just revisited this: #6532 (comment) |
security: It seems that one would need to allow extending a view's "schema_class" for each of these as needed. For instance, I have a view mixin that provides from oauth2_provider.contrib.rest_framework import (
OAuth2Authentication, TokenMatchesOASRequirements)
from rest_condition import And, Or
from rest_framework.authentication import BasicAuthentication
from rest_framework.permissions import DjangoModelPermissions, IsAuthenticated
# ...
REQUIRED_SCOPES_ALTS = {
'GET': [['auth-columbia', 'read'], ['auth-none', 'read']],
'HEAD': [['read']],
'OPTIONS': [['read']],
'POST': [
['auth-columbia', 'demo-admin', 'create'],
['auth-none', 'demo-admin', 'create'],
],
'PATCH': [
['auth-columbia', 'demo-admin', 'update'],
['auth-none', 'demo-admin', 'update'],
],
'DELETE': [
['auth-columbia', 'demo-admin', 'delete'],
['auth-none', 'demo-admin', 'delete'],
],
}
class AuthnAuthzMixIn(object):
"""
Common Authn/Authz mixin for all View and ViewSet-derived classes:
"""
authentication_classes = (BasicAuthentication, OAuth2Authentication, )
#: Either use Scope-based OAuth 2.0 token checking OR authenticated user w/Model Permissions.
permission_classes = [
Or(TokenMatchesOASRequirements,
And(IsAuthenticated, MyDjangoModelPermissions))
]
#: list of alternatives for required scopes
required_alternate_scopes = REQUIRED_SCOPES_ALTS which leads to this openapi schema snippet for a given view: paths:
/courses/:
get:
description: Returns a collection of courses
operationId: find courses
tags: [courses]
security:
- basicAuth: []
- oauth: [auth-columbia, read]
- oauth: [auth-none, read] Most of that is not something a generic schema generator could possibly infer, however if one where to add an optional schema function to:
Then someone extending these classes could potentially implement that function. For example, {
"security": [
{"oauth": ["auth-columbia","read"]},
{"oauth":["auth-none","read"]}
]
} and {
"security": [
{"basicAuth":[]}
]
} and successive super() calls of these mixins would merge the dicts. Also components:
securitySchemes:
basicAuth:
type: http
scheme: basic
oauth:
type: oauth2
description: OAuth2 service
flows:
authorizationCode:
authorizationUrl: https://oauth.cc.columbia.edu/as/authorization.oauth2
tokenUrl: https://oauth.cc.columbia.edu/as/token.oauth2
scopes:
auth-columbia: Columbia UNI login
auth-none: no user logged in
create: create
read: read
update: update
delete: delete
openid: disclose your identity
profile: your user profile
email: your email address
https://api.columbia.edu/scope/group: groups you are a member of
demo-admin: I am an admin Most of that stuff is not something the settings.OAUTH2_PROVIDER necessarily describes, but schemas: Perhaps less important, but for purposes of readability of the generated openapi.yaml, perhaps the path schemas can paths:
/courses/{id}:
parameters:
- name: id
in: path
description: ID of course to fetch
required: true
schema:
type: string
get:
description: Returns a single course
operationId: return course by id
responses:
'200':
description: course response
content:
application/vnd.api+json:
schema:
$ref: '#/components/schemas/Course'
# ...
components:
schemas:
Course:
description: Course attributes
type: object
properties:
course_name:
type: string
maxLength: 80
course_description:
type: string
subject_area_code:
type: string
maxLength: 10
course_identifier:
type: string
maxLength: 10
uniqueItems: true
pattern: "[A-Z]{5}[0-9]{5}"
required:
- subject_area_code
- course_name
- course_identifier Note that this example is not correctly JSONAPI-formatted; I tried to simplify it just to demonstrate the use of components.schemas. Sorry for the excessive length of this comment! It's a rainy Sunday;-) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0006ae4
to
2622b22
Compare
Co-authored-by: Lucidiot <[email protected]> Co-authored-by: dongfangtianyu <[email protected]>
b8bd082
to
b4ec102
Compare
Testing on my sample project, this is looking OK in both Swagger UI and ReDoc, using the management command and live As such I'm going to pull this in now, moving the docs change to a follow-up PR, which'll be next. Thank you everyone for input and assistance! They'll be a few follow-ups:
|
FYI: django-json-api/django-rest-framework-json-api#669 (WIP, waiting on 3.10 release) extends generateschema, etc. to DJA. |
Co-authored-by: Lucidiot <[email protected]> Co-authored-by: dongfangtianyu <[email protected]>
Can someone spread light on what are we supposed to use. So what client work in today's date? Thank you |
Co-authored-by: Lucidiot <[email protected]> Co-authored-by: dongfangtianyu <[email protected]>
Introduces OpenAPI schema generation for DRF v3.10.
Updates:
SchemaView and in-built interactive docs will (likely) require this to be set back to
rest_framework.schemas.AutoSchema
to function correctly../manage.py generateschema > ...
to create OpenAPI schemapip install apistar
.apistar validate --path=... --format=openapi
to check schema.At this stage: