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

Add support for different read/write schemas #70

Open
Place1 opened this issue Feb 24, 2018 · 12 comments
Open

Add support for different read/write schemas #70

Place1 opened this issue Feb 24, 2018 · 12 comments
Labels
feature New feature proposal help wanted Help wanted

Comments

@Place1
Copy link

Place1 commented Feb 24, 2018

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
Copy link
Owner

axnsan12 commented Feb 24, 2018

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.

@axnsan12 axnsan12 added the question Open question label Feb 24, 2018
@Place1
Copy link
Author

Place1 commented Feb 26, 2018

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
Copy link
Owner

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.

@axnsan12 axnsan12 added feature New feature proposal and removed question Open question labels Feb 26, 2018
@axnsan12 axnsan12 added this to the someday/maybe milestone Mar 9, 2018
@prafulbagai
Copy link

@axnsan12 - Any update on this feature?

@axnsan12
Copy link
Owner

axnsan12 commented May 8, 2018

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

@sebdiem
Copy link

sebdiem commented Jan 16, 2019

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
Copy link

sireliah commented Feb 4, 2019

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
Copy link

sebdiem commented Feb 4, 2019

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
Copy link

Didericis commented Apr 20, 2019

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
Copy link

pyinto commented Feb 28, 2020

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
Copy link

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
Copy link

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',
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature proposal help wanted Help wanted
Projects
None yet
Development

No branches or pull requests

10 participants