Skip to content

Commit

Permalink
refactor/bugfix PATCH & Serializer(partial=True) behaviour.
Browse files Browse the repository at this point in the history
  • Loading branch information
tfranzel committed Mar 1, 2021
1 parent 85246fc commit a4a5460
Show file tree
Hide file tree
Showing 5 changed files with 316 additions and 7 deletions.
18 changes: 11 additions & 7 deletions drf_spectacular/openapi.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy
import re
import typing

Expand Down Expand Up @@ -27,7 +28,8 @@
build_array_type, build_basic_type, build_choice_field, build_examples_list,
build_media_type_object, build_object_type, build_parameter_type, error, follow_field_source,
force_instance, get_doc, get_view_model, is_basic_type, is_field, is_list_serializer,
is_serializer, resolve_regex_path_parameter, resolve_type_hint, safe_ref, warn,
is_patched_serializer, is_serializer, resolve_regex_path_parameter, resolve_type_hint, safe_ref,
warn,
)
from drf_spectacular.settings import spectacular_settings
from drf_spectacular.types import OpenApiTypes, build_generic_type
Expand Down Expand Up @@ -741,9 +743,8 @@ def _map_basic_serializer(self, serializer, direction):

properties[field.field_name] = safe_ref(schema)

if self.method == 'PATCH' and spectacular_settings.COMPONENT_SPLIT_PATCH:
if direction == 'request' and serializer.partial and not serializer.read_only:
required = []
if is_patched_serializer(serializer, direction):
required = []

return build_object_type(
properties=properties,
Expand Down Expand Up @@ -922,6 +923,10 @@ def _get_request_for_media_type(self, serializer):
request_body_required = True
elif is_serializer(serializer):
if self.method == 'PATCH':
# we simulate what DRF is doing: the entry serializer is set to partial
# for PATCH requests. serializer instances received via extend_schema
# may be reused; prevent race conditions by modifying a copy.
serializer = copy.copy(serializer)
serializer.partial = True
component = self.resolve_serializer(serializer, 'request')
if not component.schema:
Expand Down Expand Up @@ -1106,9 +1111,8 @@ def _get_serializer_name(self, serializer, direction):
if name.endswith('Serializer'):
name = name[:-10]

if self.method == 'PATCH' and spectacular_settings.COMPONENT_SPLIT_PATCH:
if direction == 'request' and serializer.partial and not serializer.read_only:
name = 'Patched' + name
if is_patched_serializer(serializer, direction):
name = 'Patched' + name

if direction == 'request' and spectacular_settings.COMPONENT_SPLIT_REQUEST:
name = name + 'Request'
Expand Down
9 changes: 9 additions & 0 deletions drf_spectacular/plumbing.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ def is_basic_type(obj, allow_none=True):
return obj in OPENAPI_TYPE_MAPPING or obj in PYTHON_TYPE_MAPPING


def is_patched_serializer(serializer, direction):
return bool(
spectacular_settings.COMPONENT_SPLIT_PATCH
and serializer.partial
and not serializer.read_only
and not (spectacular_settings.COMPONENT_SPLIT_REQUEST and direction == 'response')
)


def get_lib_doc_excludes():
# do not import on package level due to potential import recursion when loading
# extensions as recommended: USER's settings.py -> USER EXTENSIONS -> extensions.py
Expand Down
53 changes: 53 additions & 0 deletions tests/test_split.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from unittest import mock

from django.db import models
from rest_framework import mixins, parsers, serializers, viewsets

from tests import assert_schema, generate_schema


class PNM1(models.Model):
field = models.IntegerField()


class PNM2(models.Model):
field_relation = models.ForeignKey(PNM1, on_delete=models.CASCADE)


class XSerializer(serializers.ModelSerializer):
class Meta:
model = PNM1
fields = '__all__'


class YSerializer(serializers.ModelSerializer):
field_relation = XSerializer()
field_relation_partial = XSerializer(source='field_relation', partial=True)

class Meta:
model = PNM2
fields = '__all__'


class XViewset(mixins.UpdateModelMixin, viewsets.GenericViewSet):
parser_classes = [parsers.JSONParser]
serializer_class = YSerializer
queryset = PNM2.objects.all()


@mock.patch('drf_spectacular.settings.spectacular_settings.COMPONENT_SPLIT_REQUEST', False)
def test_nested_partial_on_split_request_false(no_warnings):
# without split request, PatchedY and Y have the same properties (minus required).
# PATCH only modifies outermost serializer, nested serializers must stay unaffected.
assert_schema(
generate_schema('/x/', XViewset), 'tests/test_split_request_false.yml'
)


@mock.patch('drf_spectacular.settings.spectacular_settings.COMPONENT_SPLIT_REQUEST', True)
def test_nested_partial_on_split_request_true(no_warnings):
# with split request, behaves like above, however response schemas are always unpatched.
# nested request serializers are only affected by their manual partial flag and not due to PATCH.
assert_schema(
generate_schema('/x/', XViewset), 'tests/test_split_request_true.yml'
)
116 changes: 116 additions & 0 deletions tests/test_split_request_false.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
openapi: 3.0.3
info:
title: ''
version: 0.0.0
paths:
/x/{id}/:
put:
operationId: x_update
description: ''
parameters:
- in: path
name: id
schema:
type: integer
description: A unique integer value identifying this pn m2.
required: true
tags:
- x
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Y'
required: true
security:
- cookieAuth: []
- basicAuth: []
- {}
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/Y'
description: ''
patch:
operationId: x_partial_update
description: ''
parameters:
- in: path
name: id
schema:
type: integer
description: A unique integer value identifying this pn m2.
required: true
tags:
- x
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/PatchedY'
security:
- cookieAuth: []
- basicAuth: []
- {}
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/Y'
description: ''
components:
schemas:
PatchedX:
type: object
properties:
id:
type: integer
readOnly: true
field:
type: integer
PatchedY:
type: object
properties:
id:
type: integer
readOnly: true
field_relation:
$ref: '#/components/schemas/X'
field_relation_partial:
$ref: '#/components/schemas/PatchedX'
X:
type: object
properties:
id:
type: integer
readOnly: true
field:
type: integer
required:
- field
- id
Y:
type: object
properties:
id:
type: integer
readOnly: true
field_relation:
$ref: '#/components/schemas/X'
field_relation_partial:
$ref: '#/components/schemas/PatchedX'
required:
- field_relation
- field_relation_partial
- id
securitySchemes:
basicAuth:
type: http
scheme: basic
cookieAuth:
type: apiKey
in: cookie
name: Session
127 changes: 127 additions & 0 deletions tests/test_split_request_true.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
openapi: 3.0.3
info:
title: ''
version: 0.0.0
paths:
/x/{id}/:
put:
operationId: x_update
description: ''
parameters:
- in: path
name: id
schema:
type: integer
description: A unique integer value identifying this pn m2.
required: true
tags:
- x
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/YRequest'
required: true
security:
- cookieAuth: []
- basicAuth: []
- {}
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/Y'
description: ''
patch:
operationId: x_partial_update
description: ''
parameters:
- in: path
name: id
schema:
type: integer
description: A unique integer value identifying this pn m2.
required: true
tags:
- x
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/PatchedYRequest'
security:
- cookieAuth: []
- basicAuth: []
- {}
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/Y'
description: ''
components:
schemas:
PatchedXRequest:
type: object
properties:
field:
type: integer
PatchedYRequest:
type: object
properties:
field_relation:
$ref: '#/components/schemas/XRequest'
field_relation_partial:
$ref: '#/components/schemas/PatchedXRequest'
X:
type: object
properties:
id:
type: integer
readOnly: true
field:
type: integer
required:
- field
- id
XRequest:
type: object
properties:
field:
type: integer
required:
- field
Y:
type: object
properties:
id:
type: integer
readOnly: true
field_relation:
$ref: '#/components/schemas/X'
field_relation_partial:
$ref: '#/components/schemas/X'
required:
- field_relation
- field_relation_partial
- id
YRequest:
type: object
properties:
field_relation:
$ref: '#/components/schemas/XRequest'
field_relation_partial:
$ref: '#/components/schemas/PatchedXRequest'
required:
- field_relation
- field_relation_partial
securitySchemes:
basicAuth:
type: http
scheme: basic
cookieAuth:
type: apiKey
in: cookie
name: Session

0 comments on commit a4a5460

Please sign in to comment.