Skip to content

Commit

Permalink
bugfix PATCH split serializer disparity #249
Browse files Browse the repository at this point in the history
  • Loading branch information
tfranzel committed Jan 10, 2021
1 parent a32027f commit 6febe8f
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 4 deletions.
1 change: 1 addition & 0 deletions drf_spectacular/contrib/rest_polymorphic.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def map_serializer(self, auto_schema, direction):

for sub_model in serializer.model_serializer_mapping:
sub_serializer = serializer._get_serializer_from_model_or_instance(sub_model)
sub_serializer.partial = serializer.partial
resource_type = serializer.to_resource_type(sub_model)
ref = auto_schema.resolve_serializer(sub_serializer, direction).ref
sub_components.append((resource_type, ref))
Expand Down
8 changes: 5 additions & 3 deletions drf_spectacular/openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,8 @@ def _map_basic_serializer(self, serializer, direction):

properties[field.field_name] = safe_ref(schema)

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

return build_object_type(
Expand Down Expand Up @@ -847,6 +847,8 @@ def _get_request_body(self):
schema = build_array_type(self._map_serializer_field(serializer.child, 'request'))
request_body_required = True
elif is_serializer(serializer):
if self.method == 'PATCH':
serializer.partial = True
component = self.resolve_serializer(serializer, 'request')
if not component.schema:
# serializer is empty so skip content enumeration
Expand Down Expand Up @@ -1002,7 +1004,7 @@ def _get_serializer_name(self, serializer, direction):
name = name[:-10]

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

if direction == 'request' and spectacular_settings.COMPONENT_SPLIT_REQUEST:
Expand Down
2 changes: 2 additions & 0 deletions drf_spectacular/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def _get_implicit_sub_components(self, auto_schema, direction):
sub_components = []
for sub_serializer in self.target.serializers:
sub_serializer = force_instance(sub_serializer)
sub_serializer.partial = self.target.partial
resolved_sub_serializer = auto_schema.resolve_serializer(sub_serializer, direction)

try:
Expand All @@ -49,6 +50,7 @@ def _get_explicit_sub_components(self, auto_schema, direction):
sub_components = []
for resource_type, sub_serializer in self.target.serializers.items():
sub_serializer = force_instance(sub_serializer)
sub_serializer.partial = self.target.partial
resolved_sub_serializer = auto_schema.resolve_serializer(sub_serializer, direction)
sub_components.append((resource_type, resolved_sub_serializer.ref))

Expand Down
2 changes: 2 additions & 0 deletions drf_spectacular/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ def __init__(
self.component_name = component_name
self.serializers = serializers
self.resource_type_field_name = resource_type_field_name
self.partial = False
self.read_only = False


class OpenApiSchemaBase:
Expand Down
18 changes: 17 additions & 1 deletion tests/test_polymorphic.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from rest_framework.response import Response

from drf_spectacular.openapi import AutoSchema
from drf_spectacular.utils import PolymorphicProxySerializer, extend_schema
from drf_spectacular.utils import OpenApiParameter, PolymorphicProxySerializer, extend_schema
from tests import assert_schema, generate_schema


Expand Down Expand Up @@ -53,6 +53,14 @@ class ImplicitPersonViewSet(viewsets.GenericViewSet):
def create(self, request, *args, **kwargs):
return Response({}) # pragma: no cover

@extend_schema(
request=implicit_poly_proxy,
responses=implicit_poly_proxy,
parameters=[OpenApiParameter('id', int, OpenApiParameter.PATH)],
)
def partial_update(self, request, *args, **kwargs):
return Response({}) # pragma: no cover

explicit_poly_proxy = PolymorphicProxySerializer(
component_name='MetaPerson',
serializers={
Expand All @@ -67,6 +75,14 @@ class ExplicitPersonViewSet(viewsets.GenericViewSet):
def create(self, request, *args, **kwargs):
return Response({}) # pragma: no cover

@extend_schema(
request=explicit_poly_proxy,
responses=explicit_poly_proxy,
parameters=[OpenApiParameter('id', int, OpenApiParameter.PATH)],
)
def partial_update(self, request, *args, **kwargs):
return Response({}) # pragma: no cover


@pytest.mark.parametrize('viewset', [ImplicitPersonViewSet, ExplicitPersonViewSet])
def test_polymorphic(no_warnings, viewset):
Expand Down
70 changes: 70 additions & 0 deletions tests/test_polymorphic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,40 @@ paths:
schema:
$ref: '#/components/schemas/MetaPerson'
description: ''
/persons/{id}/:
patch:
operationId: persons_partial_update
description: ''
parameters:
- in: path
name: id
schema:
type: integer
required: true
tags:
- persons
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/PatchedMetaPerson'
application/x-www-form-urlencoded:
schema:
$ref: '#/components/schemas/PatchedMetaPerson'
multipart/form-data:
schema:
$ref: '#/components/schemas/PatchedMetaPerson'
security:
- cookieAuth: []
- basicAuth: []
- {}
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/MetaPerson'
description: ''
components:
schemas:
LegalPerson:
Expand Down Expand Up @@ -78,6 +112,42 @@ components:
- id
- last_name
- type
PatchedLegalPerson:
type: object
properties:
id:
type: integer
readOnly: true
company_name:
type: string
maxLength: 30
type:
type: string
readOnly: true
PatchedMetaPerson:
oneOf:
- $ref: '#/components/schemas/PatchedLegalPerson'
- $ref: '#/components/schemas/PatchedNaturalPerson'
discriminator:
propertyName: type
mapping:
legal: '#/components/schemas/PatchedLegalPerson'
natural: '#/components/schemas/PatchedNaturalPerson'
PatchedNaturalPerson:
type: object
properties:
id:
type: integer
readOnly: true
first_name:
type: string
maxLength: 30
last_name:
type: string
maxLength: 30
type:
type: string
readOnly: true
securitySchemes:
basicAuth:
type: http
Expand Down
23 changes: 23 additions & 0 deletions tests/test_regressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1195,3 +1195,26 @@ def view_func(request, format=None):
pass # pragma: no cover

generate_schema('x', view_function=view_func)


def test_nested_ro_serializer_has_required_fields_on_patch(no_warnings):
# issue #249 raised a disparity problem between serializer name
# generation and the actual serializer construction on PATCH.
class XSerializer(serializers.Serializer):
field = serializers.CharField()

class YSerializer(serializers.Serializer):
x_field = XSerializer(read_only=True)

class YViewSet(viewsets.GenericViewSet):
serializer_class = YSerializer
queryset = SimpleModel.objects.all()

def partial_update(self, request, *args, **kwargs):
pass

schema = generate_schema('x', YViewSet)
assert len(schema['components']['schemas']) == 3
assert 'Y' in schema['components']['schemas']
assert 'PatchedY' in schema['components']['schemas']
assert 'required' in schema['components']['schemas']['X']

0 comments on commit 6febe8f

Please sign in to comment.