axnsan12 / drf-yasg

Automated generation of real Swagger/OpenAPI 2.0 schemas from Django REST Framework code.
https://drf-yasg.readthedocs.io/en/stable/
Other
3.42k stars 438 forks source link

Add support for different read/write schemas #70

Open Place1 opened 6 years ago

Place1 commented 6 years ago

A pain point that often exists with automatic swagger specs with DRF is they reuse the same definition for retrieve/list/post endpoints. This leads to painful client side SDKs generated with swagger-codegen as most/all fields on the resulting model will be optional, requiring SDK consumers to litter their code with null checks (or ignore them at their peril).

A motivating example of this problem is the id field that most django models will have. This field is required on reads (the model will always have an id when retrieving/listing) but a read_only field for creates (not required for post requests). Another example would be a field that is derrives from a null=False model field (required at the DB level) but is not required at the API level (perhaps it's generated by the server's business logic).

Can (does) drf-yasg's inspection of serializers find these characteristics and if so, can it be shown in the resulting spec by defining multiple definitions for a serializer and referencing them appropriately in spec paths?

For context, our team generates SDKs for our APIs in many type safe languages and model definitions such as:

MyModel:
    type: object
    required:
        - name
    properties:
        id:
            type: number
        name:
            type: string

are a headache to use because the resulting model will label id as optional, meaning it must be checked before being passed to other SDK methods that expect a value. E.g.

interface MyModel {
    id?: number;
    name: string;
}

// ...

api.updateMyModel({ id: instance.id }) // compilation error because `.id` might be undefined
axnsan12 commented 6 years ago

Hello.

The swagger model<=> DRF serializer mapping is one-to-one. readOnly fields are detected from the serializer fields. OpenAPI 2 does not support writeOnly fields (OpenAPI 3 does), so those are not handled properly.

If this is not enough for your needs you will need to write separate "read" and "write" serializers.

Place1 commented 6 years ago

I understand the relationship between swagger 2 and rest serializers isn't 1 to 1; but i'm suggesting this might not be a limiting factor.

For example, we know from the view if we are reading vs. writing (i.e. GET or POST). This information could be used to generate multiple swagger definitions for a single serialzier that make use of different interpretations of fields.

Take the following serializer and model:

class Todo(models.Model):
    title = models.TextField(null=False)
    detail = models.TextField(null=True)
    owner = models.ForeignKey(User)
    created_at = models.DateTimeField(auto_now_add=True)

class TodoSerialzier(models.Model):
    owner = serializers.PrimaryKeyRelatedField(default=serializers.CurrentUserDefault(), read_only=True)
    created_at = serializers.DateTimeField(read_only=True)

    class Meta:
        model = Todo
        fields = '__all__'

If I were to create a swagger spec for a ModelViewSet using this serializer I would probably write the following

paths:
  /todos/:
    get:
      tags:
        - Todos
      summary: list Todos
      operationId: listTodos
      responses:
        200:
          description: a list of Todos
          schema:
            type: array
            items:
              $ref: '#/definitions/Todo'

    post:
      tags:
        - Todos
      summary: create a new Todo
      operationId: createTodo
      parameters:
        - name: Todo
          in: body
          description: the initial values for the Todo model
          schema:
            $ref: '#/definitions/postBodyDefinition'
      responses:
        201:
          description: the new Todo
          schema:
            $ref: '#/definitions/CreateTodo'

definitions:
  Todo:
    type: object
    required:
      - id
      - name
      - created_at
      - owner
    properties:
      id:
        type: number
      name:
        type: string
      detail:
        type: string
      created_at:
        type: string
      owner:
        type: number

  CreateTodo:
    type: object
    required:
      - name
      - detail
    properties:
      name:
        type: string
      detail:
        type: string

In reality, there is only 1 model and 1 serializer but the spec can include 2 different definitions. These definitions are generated by considering if the current view path is GET or POST (read or write) and then using different rules when creating the schema from the serializer. I.e. if the method is GET, then all write_only fields on the serializer should not be included, all model/serializer fields that are null=False should be a required field on the schema. Where as if the method is POST then different rules are used; write_only fields are included as required fields on the schema, serializer/model fields that have null=False are also required fields, ... etc.

Hopefully this explains my idea more clearly. The goal is to generate endpoint specific swagger definitions even if they derive from the same serializer.

axnsan12 commented 6 years ago

Okay, I see your point.

I still don't think that would be reasonable default behaviour, but I could see its utility as a feature enabed by a setting or class attribute. I'd be open to merging a PR implementing this.

prafulbagai commented 6 years ago

@axnsan12 - Any update on this feature?

axnsan12 commented 6 years ago

As far as I know no one has started work on it.

sebdiem commented 5 years ago

If this is not enough for your needs you will need to write separate "read" and "write" serializers.

And can drf-yasg automatically generate the spec from these serializers ? I don't see how it would be possible. Or you mean that I'll have to set serializer_class to e.g the ReadSerializer and then document the WriteSerializer manually ?

sireliah commented 5 years ago

I encountered the same issue - write_only fields are still present in the generated schema.

As for the read and write serializers, they are not supported, so in my case I needed to decorize the view with:

    @swagger_auto_schema(
        responses={201: CreateSerializer},
    )
    def post(self, request, *args, **kwargs):

Of course it would be nice to have automatic discovery of read and write serializers, but I'm not sure how to achieve that since DRF doesn't recognize write_serializer_class or serializer_class_create as serializer classes. Those are just static class attributes.

sebdiem commented 5 years ago

This is what I ended up doing

https://github.com/Polyconseil/django-mds/blob/master/mds/apis/utils.py#L70-L81

With a customized schema generator

https://github.com/Polyconseil/django-mds/blob/master/mds/apis/utils.py#L136-L213

Not ideal at all :( but it more or less works

Didericis commented 5 years ago

Also encountered this issue. I ended up creating an AutoSchema that creates a ReadOnly and WriteOnly version of the serializers it uses. Is similar to what @sebdiem did, but should work without needing to pass in extra context/by paying attention to the read_only and write_only attributes of serializer fields.

class ReadOnly():
    def get_fields(self):
        new_fields = OrderedDict()
        for fieldName, field in super().get_fields().items():
            if not field.write_only:
                new_fields[fieldName] = field
        return new_fields

class WriteOnly():
    def get_fields(self):
        new_fields = OrderedDict()
        for fieldName, field in super().get_fields().items():
            if not field.read_only:
                new_fields[fieldName] = field
        return new_fields

class BlankMeta:
    pass

class ReadWriteAutoSchema(SwaggerAutoSchema):
    def get_view_serializer(self):
        return self._convert_serializer(WriteOnly)

    def get_default_response_serializer(self):
        body_override = self._get_request_body_override()
        if body_override and body_override is not no_body:
            return body_override

        return self._convert_serializer(ReadOnly)

    def _convert_serializer(self, new_class):
        serializer = super().get_view_serializer()
        if not serializer:
            return serializer

        class CustomSerializer(new_class, serializer.__class__):
            class Meta(getattr(serializer.__class__, 'Meta', BlankMeta)):
                ref_name = new_class.__name__ + serializer.__class__.__name__

        new_serializer = CustomSerializer(data=serializer.data)
        return new_serializer
pyinto commented 4 years ago

Also encountered this issue. I ended up creating an AutoSchema that creates a ReadOnly and WriteOnly version of the serializers it uses. Is similar to what @sebdiem did, but should work without needing to pass in extra context/by paying attention to the read_only and write_only attributes of serializer fields.

class ReadOnly():
    def get_fields(self):
        new_fields = OrderedDict()
        for fieldName, field in super().get_fields().items():
            if not field.write_only:
                new_fields[fieldName] = field
        return new_fields

class WriteOnly():
    def get_fields(self):
        new_fields = OrderedDict()
        for fieldName, field in super().get_fields().items():
            if not field.read_only:
                new_fields[fieldName] = field
        return new_fields

class BlankMeta:
    pass

class ReadWriteAutoSchema(SwaggerAutoSchema):
    def get_view_serializer(self):
        return self._convert_serializer(WriteOnly)

    def get_default_response_serializer(self):
        body_override = self._get_request_body_override()
        if body_override and body_override is not no_body:
            return body_override

        return self._convert_serializer(ReadOnly)

    def _convert_serializer(self, new_class):
        serializer = super().get_view_serializer()
        if not serializer:
            return serializer

        class CustomSerializer(new_class, serializer.__class__):
            class Meta(getattr(serializer.__class__, 'Meta', BlankMeta)):
                ref_name = new_class.__name__ + serializer.__class__.__name__

        new_serializer = CustomSerializer(data=serializer.data)
        return new_serializer

Thanks a lot, it works, only one issue I've found, it doesn't work with custom schema generated using decorator @swagger_auto_schema (even if you pass auto_schema kwarg), this happens because it does not trigger SwaggerAutoSchema.get_view_serializer method.

wonderbeyond commented 4 years ago

I have tried fixing this issue by writing a view mixin:

class PartialUpdateModelMixin:
    """
    Update a model instance in patch mode.

    Why not rest_framework.mixins.UpdateModelMixin?
    Because it bundles "update" and "partial_update" together,
    and I only need the latter.
    """
    def get_serializer(self, *args, **kwargs):
        """
        Remove required option when the serializer is used for partial update.

        See also https://github.com/axnsan12/drf-yasg/issues/70
        and https://github.com/axnsan12/drf-yasg/issues/459
        """
        serializer = super().get_serializer(*args, **kwargs)
        if self.action == 'partial_update':
            _orig_get_fields = serializer.get_fields

            def get_fields_wrapper():
                fields = _orig_get_fields()
                for n, f in fields.items():
                    f.required = False
                return fields
            serializer.get_fields = get_fields_wrapper

        return serializer

    def partial_update(self, request, *args, **kwargs):
        partial = True
        instance = self.get_object()
        serializer = self.get_serializer(instance, data=request.data, partial=partial)
        serializer.is_valid(raise_exception=True)
        self.perform_update(serializer)

        if getattr(instance, '_prefetched_objects_cache', None):
            # If 'prefetch_related' has been applied to a queryset, we need to
            # forcibly invalidate the prefetch cache on the instance.
            instance._prefetched_objects_cache = {}

        return Response(serializer.data)

    def perform_update(self, serializer):
        serializer.save()

Then use it in your viewset:

class SomeModelViewSet(PartialUpdateModelMixin, GenericViewSet):
    pass

Hope it helps you!

oubeichen commented 4 years ago

Also encountered this issue. I ended up creating an AutoSchema that creates a ReadOnly and WriteOnly version of the serializers it uses. Is similar to what @sebdiem did, but should work without needing to pass in extra context/by paying attention to the read_only and write_only attributes of serializer fields.

class ReadOnly():
    def get_fields(self):
        new_fields = OrderedDict()
        for fieldName, field in super().get_fields().items():
            if not field.write_only:
                new_fields[fieldName] = field
        return new_fields

class WriteOnly():
    def get_fields(self):
        new_fields = OrderedDict()
        for fieldName, field in super().get_fields().items():
            if not field.read_only:
                new_fields[fieldName] = field
        return new_fields

class BlankMeta:
    pass

class ReadWriteAutoSchema(SwaggerAutoSchema):
    def get_view_serializer(self):
        return self._convert_serializer(WriteOnly)

    def get_default_response_serializer(self):
        body_override = self._get_request_body_override()
        if body_override and body_override is not no_body:
            return body_override

        return self._convert_serializer(ReadOnly)

    def _convert_serializer(self, new_class):
        serializer = super().get_view_serializer()
        if not serializer:
            return serializer

        class CustomSerializer(new_class, serializer.__class__):
            class Meta(getattr(serializer.__class__, 'Meta', BlankMeta)):
                ref_name = new_class.__name__ + serializer.__class__.__name__

        new_serializer = CustomSerializer(data=serializer.data)
        return new_serializer

Thanks a lot, it works, only one issue I've found, it doesn't work with custom schema generated using decorator @swagger_auto_schema (even if you pass auto_schema kwarg), this happens because it does not trigger SwaggerAutoSchema.get_view_serializer method.

add this to your settings.py

SWAGGER_SETTINGS = {
    'DEFAULT_AUTO_SCHEMA_CLASS': 'path.to.your.ReadWriteAutoSchema',
}