Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Using DRF spectacular #705

Closed
rvanlaar opened this issue Apr 11, 2022 · 16 comments
Closed

Using DRF spectacular #705

rvanlaar opened this issue Apr 11, 2022 · 16 comments

Comments

@rvanlaar
Copy link

Hi,

Thank you for creating and maintaining drf-spectacular. It's an awesome tool.
It's great that it's typed and it uses type hints to figure out.

I recently started using it and am encountering a couple rough edges. I'm using redoc for displaying the API.

Viewsets

The default is to use the first path of the url as the tag. The first part of my endpoints is /api/.
I had trouble figuring out what the default was.
What I expected was that one ViewSet, i.e. get, list, put, patch, post and delete, would be grouped together.
The same applies to the summary that's used to display. That it would have standard text for each generic part of the viewsets.
The DRF router already assigns names to these.

ApiView decorated functions

These functions themselves are relatively straightforward most of the time.
It seems there is a disconnect between the ease of use as a programmer and how to generate the documentation for them.
I haven't found a good way to handle these cases and the documentation is spares on how to handle it.

A couple examples: Multiple views have logic with custom responses. That is, variants on: Response({"status": "invalid link" }, status=status.HTTP_400_BAD_REQUEST)
Specifying it with @schema results in: status: Encountered 2 components with identical names "status"
How to better handle these cases?

Another view returns important user data on login. To name a few: name, preferences, auth_groups, last_visted_page, company_name. I don't see good options for this view. extend_schema requires having the code twice, the same with creating a serializer. It would be alright to use a dataclass here, however, that needs to be wrapped inside a serializer to be picked up.
Is it possible to use a dataclass directly as the response?

Authorization

Is it possible to have permission classes show up in the schema?

Responses

How to show error responses for generic view sets? I.e. 400, BAD request, of 404 not found?

@tfranzel
Copy link
Owner

Hi @rvanlaar,

thanks alot.

The default is to use the first path of the url as the tag. The first part of my endpoints is /api/.
I had trouble figuring out what the default was.

the default is not fixed. spectacular will estimate the greatest common divider, remove it and then use the next URL item as the tag, e.g. if all endpoints are prefixed with /api/v1/ it is ignored and the next item is subsequently used. This usually break when you have one endpoint that has a different prefix and thus the common divider is gone. To manually set this have a look at the setting SCHEMA_PATH_PREFIX to customize to your needs.

Specifying it with @Schema results in: status: Encountered 2 components with identical names "status"
How to better handle these cases?

You need to make sure components have a unique name. Among other things you can influence the name with ref_name or @extend_schema_serializer(component_name=...)

Is it possible to have permission classes show up in the schema?

YES, this happens automatically if proper authentication_classes and permission_classes are set on the view. Supported libraries are listed in the README. For others you have to write a small extension.

Is it possible to use a dataclass directly as the response?

we have some support for dataclasses. mileage may vary.

How to show error responses for generic view sets? I.e. 400, BAD request, of 404 not found?

this is an open feature request. Currently you have to set those error cases by hand via decorator @extend_schema(responses={...}). sry.

@rvanlaar
Copy link
Author

rvanlaar commented Apr 22, 2022

Hi @tfranzel

Thanks for your thoughtful response.

The default is to use the first path of the url as the tag. The first part of my endpoints is /api/.
I had trouble figuring out what the default was.

the default is not fixed. spectacular will estimate the greatest common divider, remove it and then use the next URL item as the tag, e.g. if all endpoints are prefixed with /api/v1/ it is ignored and the next item is subsequently used. This usually break when you have one endpoint that has a different prefix and thus the common divider is gone. To manually set this have a look at the setting SCHEMA_PATH_PREFIX to customize to your needs.

We indeed have two endpoints outside the /api/v1/ path. The SCHEMA_PATH_PREFIX is awesome. Now I can remove 80% of the manual tags I placed.

We have some parts of the application that use a different prefix e.g. /api/v1/personal/ and /api/v1/student/ They now get grouped under one tag 'personal' and 'student'.

Specifying it with @Schema results in: status: Encountered 2 components with identical names "status"
How to better handle these cases?

You need to make sure components have a unique name. Among other things you can influence the name with ref_name or @extend_schema_serializer(component_name=...)

I don't quite understand. I created some example code: https://gist.github.com/rvanlaar/4a99eb531d94c81e407d721bf2962e0d

The return type is the same, namely a Response with a status message. I can create a StatusSerializer with a charField of status. It would create an extra unnecessary abstraction.
Is there a underlying reason the names need to be different?
note: I might use a dataclass now I've seen how easy that is.

Is it possible to have permission classes show up in the schema?

YES, this happens automatically if proper authentication_classes and permission_classes are set on the view. Supported libraries are listed in the README. For others you have to write a small extension.

Can you help me with this?
I did write one for a slightly custom session authentication. Which worked great.
We use permission groups from Django: e.g. teacher, content creator and specify permission_classes in the view.
i.e. in pseudo code:

class TeacherPermission(BasePermission):
    def has_permission(self, request, view):
        return request.user.is_teacher()

How can I turn this into a thing Spectacular understands?

Is it possible to use a dataclass directly as the response?

we have some support for dataclasses. mileage may vary.

Thanks for the link to the tests. This is great: @extend_schema(responses=DataclassSerializer(dataclass=Person)).
I like how it combines the simplicity of @api_view functions with stronger typing. Response({"string": asdict(person_instance)})

How to show error responses for generic view sets? I.e. 400, BAD request, of 404 not found?

this is an open feature request. Currently you have to set those error cases by hand via decorator @extend_schema(responses={...}). sry.

No worries. Having this documentation is already better than nothing.

@tfranzel
Copy link
Owner

I don't quite understand. I created some example code: https://gist.github.com/rvanlaar/4a99eb531d94c81e407d721bf2962e0d

I see no decorators here. The error message cannot happen like that. In any case, each serializer is baked into at least one component. Those live in the component section of the schema. For obvious reasons, collisions would be bad if they are referenced all over the schema. Each serializer needs a unique name (or unique name override) for this to work properly.

If you use the same inline_serializer() statement multiple times, yes that would be a false positive warning.

I did write one for a slightly custom session authentication. Which worked great.

great! you likely cannot do much more here.

How can I turn this into a thing Spectacular understands?

This is outside the scope of simple auth methods. We can only distinguish between "auth required" or not and give details about the auth method (header, cookie, etc). OpenAPI is also limiting here. To get fine grained scopes, you would need to use OAuth, which OpenAPI explicitly supports (including scopes).

@tfranzel
Copy link
Owner

closing this issue for now. feel free to comment if anything is missing or not working and we will follow-up.

@rvanlaar
Copy link
Author

Hello,

I've been converting more of our @api_views to use ClassBasedViews with serializers. It has been working out very well.
There's one more thing. Can you help me with the following?

Due to the templating tool used in the front-end we wrap list objects inside a dict.
i.e.: [{'name': 'Joe'},{'name', 'Max'}] becomes {'users': [{'name': 'Joe'},{'name', 'Max'}] }

This is done with for the JSONrenderer and a with 'root' field on the serializer, i.e:

    class Meta:
        model = User
        fields = ('name', )
        root = 'users'

This is only done on responses, not requests.

How can I go about achieving the wrapped objects in the API?

@tfranzel
Copy link
Owner

Hey Roland

This is done with for the JSONrenderer and a with 'root' field on the serializer, i.e:

for a second there is thought this was a DRF feature I didn't know about. root is your addition, right?
spectacular assumes that the Renderer phase to be "transparent", meaning that it does not change the structure.
Having asymmetric serializer will require you to use "COMPONENT_SPLIT_REQUEST": True in any case.

I can think of 2 solution.

  1. write a POSTPROCESSING_HOOK, that transforms those serializer (response) components.
  2. Introduces a new mixin or base class that signals that functionality. It does not have to do anything, just as a marker for the extensions when to kick in. Then write a OpenApiSerializerExtension that handles that case. you got the direction and can resolve the serializer default handing with auto_schema., which you can then modify to your liking.

return auto_schema._map_serializer(self.target_class, direction, bypass_extensions=True)

  1. is a bit more dirty but will get the job done. 2. sounds cleaner but might be a bit more involved. I would give 2. a quick try before reverting to 1.

@rvanlaar
Copy link
Author

Hi,

for a second there is thought this was a DRF feature I didn't know about. root is your addition, right?

Indeed, that's my own add-on. Here's our code for the renderer.

class JSONRootElementRenderer(JSONRenderer):
    """
    Renderer which serializes to JSON with a root element.

    Moustache needs a root element in list renderers.
    """

    def render(self, data, accepted_media_type=None, renderer_context=None):

        renderer_context = renderer_context or {}
        if view := renderer_context.get("view"):
            if get_serializer := getattr(view, "get_serializer", None):
                serializer = get_serializer()
                if meta := getattr(serializer, "Meta", None):
                    if root_element := getattr(meta, "root", None):
                        data = {root_element: data}
        return super().render(data, accepted_media_type, renderer_context)

Regarding 2, introducing a new mixin/base class. I tried that for one specific serializer to get the ball rolling. It didn't have the results I wanted:

class RootFix(OpenApiSerializerExtension):
    target_class = 'path.to.specific.serializer'
    def map_serializer(self, auto_schema: 'AutoSchema', direction: Direction):
        schema = auto_schema._map_basic_serializer(self.target, direction)
        root = self.target.Meta.root
        return {root: schema}

What's the structure that should be returned?

@tfranzel
Copy link
Owner

tfranzel commented Jul 28, 2022

you must return a valid OpenAPI object, but we got helpers for that. this snippet should do what you want.

    class RootMixin:
        pass

    class YourSerializer(RootMixin, serializers.ModelSerializer):
        class Meta:
            model = M1
            fields = '__all__'
            root = 'foo'

    from drf_spectacular.plumbing import build_object_type

    class RootFix(OpenApiSerializerExtension):
        target_class = 'path.to.RootMixin'
        match_subclasses = True  # !important for this

        def map_serializer(self, auto_schema: 'AutoSchema', direction: Direction):
            if direction == 'request':
                return auto_schema._map_serializer(self.target, direction, bypass_extensions=True)
            else:
                schema = auto_schema._map_serializer(self.target, direction, bypass_extensions=True)
                return build_object_type(
                    properties={self.target.Meta.root: schema}
                )

If you want the YourSerializer component to be reused for response (in case you use it also unwrapped) and not duplicated, you might need to introduce a separate name for the wrapped components ( write aRootFix.get_name for that). that is more complicated.

tfranzel added a commit that referenced this issue Jul 28, 2022
1. Allows get_name with full parameters without breaking legacy code.
2. enable calling ``auto_schema.resolve_serializer`` with
   ``bypass_extensions=True`` to allow offloading component creation
@tfranzel
Copy link
Owner

my striken comment made me think about an old open issue. Have a look at this PR. It contains the second solution I was talking about. It just depends on those 2 new features, while above solution works right now.

@rvanlaar
Copy link
Author

rvanlaar commented Aug 1, 2022

Thank you for quick the response, it's appreciated. 1

The proposed solution is getting me halfway there.

  • The list view is now: [ {"root_name": [objects]}, whereas it should be {"root_name": [objects]}
  • The PUT, POST and PATCH payloads are also wrapped in the root_name. I made sure to double check for the direction and your example.

As a heads up, I don't know when I'll have more time to work on this part of our application. I hope it's soon.

@tfranzel
Copy link
Owner

tfranzel commented Aug 1, 2022

The list view is now: [ {"root_name": [objects]}, whereas it should be {"root_name": [objects]}

i missed that one. There is a way to handle this atm, but its gets a bit more tedious. set a custom list_serializer_class on the serializer and write a extension for that dummy ListSerializer class. that extension can then handle the list wrapping (or not).

The PUT, POST and PATCH payloads are also wrapped in the root_name. I made sure to double check for the direction and your example.

yeah that makes sense but only the response part, right?

As a heads up, I don't know when I'll have more time to work on this part of our application. I hope it's soon.

sure, we can pick this up later. if this has no user interest, I just put it back on the pile.

@rvanlaar
Copy link
Author

rvanlaar commented Aug 4, 2022

I named it "ROOT" for this example: This is how it's rendered with redocly:
PUT statement

Could redocly be the problem here?

@tfranzel
Copy link
Owner

tfranzel commented Aug 4, 2022

not sure what is the problem here, could you laborate? not using redoc that often myself. generally I look at the schema and see if the schema is as expected before concerning myself with the UI

@rvanlaar
Copy link
Author

rvanlaar commented Aug 4, 2022

My understanding is as follows: Having a response that's different from the request results in two components.
Is that correct?

Example: Both POST and GET use the same component.

/api/v1/afspeellijst/:
    get:
      operationId: afspeellijst_list
      tags:
      - afspeellijst
      security:
      - cookieAuth: []
      - tokenAuth: []
      responses:
        '200':
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/AfspeelLijst'
          description: ''
    post:
      operationId: afspeellijst_create
      tags:
      - afspeellijst
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/AfspeelLijst'
          application/x-www-form-urlencoded:
            schema:
              $ref: '#/components/schemas/AfspeelLijst'
          multipart/form-data:
            schema:
              $ref: '#/components/schemas/AfspeelLijst'
      security:
      - cookieAuth: []
      - tokenAuth: []
      responses:
        '201':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/AfspeelLijst'
          description: ''
  schemas:
    AfspeelLijst:
      type: object
      properties:
        ROOT:
          type: object
          properties:
            id:
              type: integer
              readOnly: true
            ids:
              type: array
              items:
                type: integer
            bestanden:
              type: array
              items:
                $ref: '#/components/schemas/DatabankBestand'
              readOnly: true
            pagina:
              type: integer
              nullable: true
          required:
          - bestanden
          - id
          - ids

@tfranzel
Copy link
Owner

tfranzel commented Aug 4, 2022

My understanding is as follows: Having a response that's different from the request results in two components.
Is that correct?

yes but not quite. This all hinges on the setting "COMPONENT_SPLIT_REQUEST": True as I said. And when that is turned on, there are always two components even if they are the "same" (which they are not in most cases). This is to prevent "flickering" of components.

@rvanlaar
Copy link
Author

rvanlaar commented Aug 4, 2022

Thank you for your patience. Much appreciated. It works.

The rabbit hole continues though. I also use the serializer as a readonly subserializer. Here the response isn't wrapped.
I'll probably use a serializer without the root parameter, inherit from it and only set the root parameter on the child.

tfranzel added a commit that referenced this issue Aug 5, 2022
Extend OpenApiSerializerExtension interface. #392 #705
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants