Skip to content

fix(obtain_auth_token): Fix openapi schema generation #7211

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 1 commit into from
Mar 3, 2020

Conversation

gnuletik
Copy link
Contributor

@gnuletik gnuletik commented Mar 2, 2020

This PR fixes the AutoSchema generation for the obtain_auth_token view.

Here is the invalid (partial) OpenAPI schema currently generated.

  /api-token-auth/:
    post:
      operationId: CreateObtainAuthToken
      description: ''
      parameters: []
      requestBody:
        content:
          application/x-www-form-urlencoded:
            schema: &id029 {}
          multipart/form-data:
            schema: *id029
          application/json:
            schema: *id029
      responses:
        '200':
          content:
            application/json:
              schema: {}
          description: ''

As you can see the requestBody's schema and response's schema are empty.
This is because:

  • The ObtainAuthToken class inherit from rest_framework.views.APIView which do not have a get_serializer method (this is what is used by the rest_framework.schemas.openapi.AutoSchema class to get the serializer).
  • The AuthTokenSerializer did not had the token field.

With this PR, the generated schema is

  /api-token-auth/:
    post:
      operationId: CreateObtainAuthToken
      description: ''
      parameters: []
      requestBody:
        content:
          application/x-www-form-urlencoded:
            schema: &id029
              type: object
              properties:
                username:
                  type: string
                  writeOnly: true
                password:
                  type: string
                  writeOnly: true
                required:
                  - username
                  - password
          multipart/form-data:
            schema: *id029
          application/json:
            schema: *id029
      responses:
        '200':
          content:
            application/json:
              schema:
                type: object
                properties:
                  token:
                    type: string
                    readOnly: true
                required: []
          description: ''

NB: In the ObtainAuthToken class, the schema field (ManualSchema , for coreapi only) was override when the coreapi dependency was installed.
Instead of checking the installed dependency, I replaced the check with coreapi_schema.is_enabled.
This change enables us to run the test script (where coreapi is installed) with the AutoSchema, unless coreapi is explicitly configured.

Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

OK, fair, enough. Just one comment:



class ObtainAuthToken(APIView):
class ObtainAuthToken(GenericAPIView):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of this, can you just define get_serializer(), and then use that in post()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely! This is done :)

@gnuletik gnuletik force-pushed the openapi/authtoken_serializer branch 2 times, most recently from 9e56c8d to b39bde5 Compare March 2, 2020 17:45
fix(ObtainAuthToken): Fix openapi response type
fix(ObtainAuthToken): Use APIView and define get_serializer
fix test
@gnuletik gnuletik force-pushed the openapi/authtoken_serializer branch from b39bde5 to cb1f762 Compare March 2, 2020 19:35
@carltongibson carltongibson merged commit 609f708 into encode:master Mar 3, 2020
@carltongibson
Copy link
Collaborator

Thanks again @gnuletik! Great effort 🔥

@gnuletik
Copy link
Contributor Author

gnuletik commented Mar 3, 2020

It's a pleasure :)
Thanks for your time!

@gnuletik gnuletik deleted the openapi/authtoken_serializer branch March 3, 2020 14:35
@ayetorello
Copy link

This fix breaks my login view implementation:

class LoginView(viewsets.ViewSet):
    serializer_class = AuthTokenSerializer
    
    def create(self, request):
        return ObtainAuthToken().post(request)

AttributeError: 'ObtainAuthToken' object has no attribute 'request'

pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
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.

3 participants