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

Added OpenAPI Schema Generation. #6532

Merged
merged 1 commit into from
May 13, 2019
Merged

Conversation

carltongibson
Copy link
Collaborator

@carltongibson carltongibson commented Mar 26, 2019

Introduces OpenAPI schema generation for DRF v3.10.

Updates:

'DEFAULT_SCHEMA_CLASS': 'rest_framework.schemas.inspectors.OpenAPIAutoSchema'

SchemaView and in-built interactive docs will (likely) require this to be set back to rest_framework.schemas.AutoSchema to function correctly.

  • Use ./manage.py generateschema > ... to create OpenAPI schema
  • Install APIStar pip install apistar.
  • Run apistar validate --path=... --format=openapi to check schema.

At this stage:

  • Are there validation issues?
  • What important OpenAPI elements are we missing?

@Lucidiot

This comment has been minimized.

@carltongibson

This comment has been minimized.

@Lucidiot

This comment has been minimized.

@carltongibson

This comment has been minimized.

@Lucidiot

This comment has been minimized.

@carltongibson

This comment has been minimized.

@carltongibson

This comment has been minimized.

@n2ygk
Copy link
Contributor

n2ygk commented Mar 28, 2019

@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:

  • Looks like DJA's serializers will need to be updated:
  File "/Users/alan/src/django-training/env/lib/python3.6/site-packages/rest_framework_json_api/serializers.py", line 33, in __init__
    'ResourceIdentifierObjectsSerializer must be initialized with a model class.'
RuntimeError: ResourceIdentifierObjectsSerializer must be initialized with a model class.
  • DjangoFilterBackend also is missing something:
AttributeError: 'DjangoFilterBackend' object has no attribute 'get_schema_operation_parameters'

Any pointers would be greatly appreciated.

I've added this issue to track needed DJA enhancements to follow your DRF OAS enhancements.

@carltongibson
Copy link
Collaborator Author

carltongibson commented Mar 28, 2019

Hi @n2ygk. Super. Thanks. At this stage all real-world input is great, as it helps us get the details right first time...

'DjangoFilterBackend' object has no attribute 'get_schema_operation_parameters'

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 [], as per rest_framework/filters.py:44 here.) Better though, would be a PR on Django Filter adding proper OpenAPI operation parameter generation. It's on my list but if someone gets to it first 💃

RuntimeError: ResourceIdentifierObjectsSerializer must be initialized with a model class.

Your get_serializer() method will need to work without the model_class kwarg (perhaps by using the model_class attribute....) or you'll need to subclass OpenApiAutoSchema to call it how that's expected. (We might need a hook to make that possible/realistic...???)

@Lucidiot
Copy link
Contributor

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 (APIView.authentication_classes). A requirement is just a reference to a security scheme. Would implementing this part of the spec, which would allow to get API clients with built-in authentication mangement, be out of scope for the pull request, or for DRF altogether?

@carltongibson

This comment has been minimized.

@n2ygk
Copy link
Contributor

n2ygk commented Mar 29, 2019

@carltongibson @Lucidiot here's an example of a security scheme that I'm currently hand-editing that allows django basic auth or oauth.

paths:
  /courses/:
    get:
      description: Returns a collection of courses
      operationId: find courses
      security:
        - basicAuth: []
        - oauth-test: [auth-columbia, read]
        - oauth-test: [auth-none, read]
# ...
components:
  securitySchemes:
    basicAuth:
      type: http
      scheme: basic
    oauth-test:
      type: oauth2
      flows:
        authorizationCode:
          authorizationUrl: https://oauth-test.cc.columbia.edu/as/authorization.oauth2
          tokenUrl: https://oauth-test.cc.columbia.edu/as/token.oauth2
          scopes:
            "auth-columbia": Columbia UNI login
            "auth-none": No user login (trusted client)
            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-netphone-admin": Administrative access to netphone resources

This corresponds to this DJA code fragment:

from oauth2_provider.contrib.rest_framework import (OAuth2Authentication,
                                                    TokenMatchesOASRequirements)
from rest_condition import And, Or
from rest_framework.authentication import (BasicAuthentication,
                                           SessionAuthentication)
from rest_framework.permissions import DjangoModelPermissions, IsAuthenticated
from rest_framework_json_api.views import ModelViewSet, RelationshipView

from myapp.models import Course, CourseTerm, Instructor, Person
from myapp.serializers import (CourseSerializer, CourseTermSerializer,
                               InstructorSerializer, PersonSerializer)

#: For a given HTTP method, a list of valid alternative required scopes.
#: For instance, GET will be allowed if "auth-columbia read" OR "auth-none read" scopes are provided.
#: Note that even HEAD and OPTIONS require the client to be authorized with at least "read" scope.
REQUIRED_SCOPES_ALTS = {
    'GET': [['auth-columbia', 'read'], ['auth-none', 'read']],
    'HEAD': [['read']],
    'OPTIONS': [['read']],
    #...
}


class MyDjangoModelPermissions(DjangoModelPermissions):
    """
    Override `DjangoModelPermissions <https://docs.djangoproject.com/en/dev/topics/auth/#permissions>`_
    to require view permission as well: The default allows view by anybody.
    """
    # TODO: refactor to just add the GET key to super().perms_map
    #: the usual permissions map plus GET. Also, we omit PUT since we only use PATCH with {json:api}.
    perms_map = {
        'GET': ['%(app_label)s.view_%(model_name)s'],
        'OPTIONS': ['%(app_label)s.view_%(model_name)s'],
        'HEAD': ['%(app_label)s.view_%(model_name)s'],
        'POST': ['%(app_label)s.add_%(model_name)s'],
        # PUT not allowed by JSON:API; use PATCH
        # 'PUT': ['%(app_label)s.change_%(model_name)s'],
        'PATCH': ['%(app_label)s.change_%(model_name)s'],
        'DELETE': ['%(app_label)s.delete_%(model_name)s'],
    }


class AuthnAuthzMixIn(object):
    """
    Common Authn/Authz mixin for all View and ViewSet-derived classes:
    """
    #: In production Oauth2 is preferred; Allow Basic and Session for testing and browseable API.
    authentication_classes = (BasicAuthentication, SessionAuthentication, 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


class CourseBaseViewSet(AuthnAuthzMixIn, ModelViewSet):
    """
    Base ViewSet for all our ViewSets:
    - Adds Authn/Authz
    """
    pass

@aleksanb
Copy link

I did a quick test locally on our project which uses django_fields amongst other things. I added

    def get_schema_operation_parameters(self, view):
        return [] 

to DjangoFilterSetBackend as suggested, and hit a new issue with generating the schema:

Traceback (most recent call last):
  File "manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/django/core/management/base.py", line 316, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/django/core/management/base.py", line 353, in execute
    output = self.handle(*args, **options)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/management/commands/generateschema.py", line 24, in handle
    schema = generator.get_schema(request=None, public=True)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/schemas/generators.py", line 519, in get_schema
    paths = self.get_paths(None if public else request)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/schemas/generators.py", line 506, in get_paths
    operation = view.schema.get_operation(path, method)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/schemas/inspectors.py", line 536, in get_operation
    operation['responses'] = self._get_responses(path, method)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/schemas/inspectors.py", line 791, in _get_responses
    content = self._map_serializer(serializer)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/schemas/inspectors.py", line 728, in _map_serializer
    schema = self._map_field(field)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/schemas/inspectors.py", line 655, in _map_field
    'items': self._map_serializer(field.child)
  File "/Users/.local/share/virtualenvs/app-SIjAPd74/lib/python3.7/site-packages/rest_framework/schemas/inspectors.py", line 721, in _map_serializer
    for field in serializer.fields.values():
AttributeError: 'DateTimeRangeField' object has no attribute 'fields'

so it might have issues with from drf_extra_fields.fields import DateTimeRangeField? Do custom fields need new attributes?

@aleksanb
Copy link

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.

@carltongibson
Copy link
Collaborator Author

@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!

@n2ygk
Copy link
Contributor

n2ygk commented Apr 29, 2019

@carltongibson I've started looking at what it would take to extend rest_framework.schemas.openapi.AutoSchema for DJA and it feels like the way it's currently structured, perhaps the entire class would have to be replaced rather than extended. I'd appreciate your advice on how to proceed. (And, yes, I see your TODO for field is list serializer....)

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

@samh
Copy link

samh commented May 1, 2019

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 speccy): responses require a description.

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!

@n2ygk
Copy link
Contributor

n2ygk commented May 1, 2019

@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.

@n2ygk
Copy link
Contributor

n2ygk commented May 5, 2019

@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:

schema = DefaultSchema()

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:
class DefaultSchema(ViewInspector):
"""Allows overriding AutoSchema using DEFAULT_SCHEMA_CLASS setting"""

Perhaps add a
schema_class or inspector_class similar to the others? That feels like the most consistent approach to me.

class APIView(View):
    # ...
    schema_class = api_settings.DEFAULT_SCHEMA_CLASS

What do you think?

Hmm, scrolling back, it looks like I've just revisited this: #6532 (comment)

@n2ygk
Copy link
Contributor

n2ygk commented May 5, 2019

  • What important OpenAPI elements are we missing?
  • security
  • schemas

security:paths.<path>.<method>.security and components.securitySchemes

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 authentication_classes and permission_classes:

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:

  • rest_framework.authentication.BaseAuthentication (views.APIView.authentication_classes)
  • rest_framework.permssions.BasePermission (views.APIView.permission_classes)

Then someone extending these classes could potentially implement that function. For example, TokenMatchesOASRequirements understands how to interpret required_alternate_scopes and could generate:

{
  "security": [
    {"oauth": ["auth-columbia","read"]},
    {"oauth":["auth-none","read"]}
  ]
}

and BasicAuthentication can generate:

{
  "security": [
     {"basicAuth":[]}
  ]
}

and successive super() calls of these mixins would merge the dicts.

Also components.securitySchemes is needed as in this example:

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 TokenMatchesOASRequirements could certainly also return the appropriate components.securitySchemes dict that gets merged.


schemas: components.schemas.<schema>

Perhaps less important, but for purposes of readability of the generated openapi.yaml, perhaps the path schemas can $ref: '#/components/schemas/<schema>' rather than expanding the schema in place. This looks nice in swagger-UI as the Models are available to peruse separately from the paths.
For example:

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;-)

@carltongibson

This comment has been minimized.

@carltongibson

This comment has been minimized.

Co-authored-by: Lucidiot <[email protected]>
Co-authored-by: dongfangtianyu <[email protected]>
@carltongibson carltongibson force-pushed the openapi-schema-generation branch 2 times, most recently from b8bd082 to b4ec102 Compare May 13, 2019 13:50
@carltongibson
Copy link
Collaborator Author

Testing on my sample project, this is looking OK in both Swagger UI and ReDoc, using the management command and live SchemaView.

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:

  • We can take separate PRs to add server, security and schema/components elements.
  • @samh title and description are already available as arguments to get_schema_view and command line flags to generateschema. ASAICS they are working as expected. What's missing is version. A PR adding similar would be very welcome.
  • Improving field introspection would also be good: e.g. maxLength should usually be available on a CharField for instance. Input very welcome there.
  • @n2ygk I'm still thinking about your use-case. Let me get the docs done and then we'll swing back to it.

@n2ygk
Copy link
Contributor

n2ygk commented Jun 27, 2019

  • @n2ygk I'm still thinking about your use-case. Let me get the docs done and then we'll swing back to it.

FYI: django-json-api/django-rest-framework-json-api#669 (WIP, waiting on 3.10 release) extends generateschema, etc. to DJA.

@carltongibson
Copy link
Collaborator Author

Hi @n2ygk. Super! Do you fancy opening a PR on #6668 adding your examples to the Third-party section?

WIP, waiting on 3.10 release

Perhaps after that instead. 🙂

@carltongibson carltongibson deleted the openapi-schema-generation branch July 2, 2019 07:11
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
Co-authored-by: Lucidiot <[email protected]>
Co-authored-by: dongfangtianyu <[email protected]>
@abhinavsingh
Copy link

Can someone spread light on what are we supposed to use. coreapi runs into weird errors, apistar seems to have problems with UTF characters e.g. Django returns a pattern key for the field when UrlField is used. This breaks apistar. It's being archived, so don't think a fix can be expected.

So what client work in today's date? Thank you

sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
Co-authored-by: Lucidiot <[email protected]>
Co-authored-by: dongfangtianyu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants