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

is_patched_serializer fails with DictField #413

Closed
mblayman opened this issue Jun 1, 2021 · 12 comments
Closed

is_patched_serializer fails with DictField #413

mblayman opened this issue Jun 1, 2021 · 12 comments
Labels
enhancement New feature or request fix confirmation pending issue has been fixed and confirmation from issue reporter is pending

Comments

@mblayman
Copy link

mblayman commented Jun 1, 2021

Describe the bug
My team has a list endpoint that returns a dictionary values that are generated by methods that aren't serializers. We'd like to document at least that this endpoint returns a list of objects. I tried adding the following extend_schema:

    @extend_schema(
        responses=serializers.ListSerializer(child=serializers.DictField()),
    )
    def get(self, request: Request, *args: Any, **kwargs: Any) -> Response:
        data = # complex code to produce list of dicts here.
        return Response(data, status=200)

drf-spectacular raised the following exception:

Traceback (most recent call last):
  File "manage.py", line 11, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.7/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.7/site-packages/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 323, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.7/site-packages/django/core/management/base.py", line 364, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/management/commands/spectacular.py", line 50, in handle
    schema = generator.get_schema(request=None, public=True)
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/generators.py", line 230, in get_schema
    paths=self.parse(request, public),
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/generators.py", line 207, in parse
    operation = view.schema.get_operation(path, path_regex, method, self.registry)
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/utils.py", line 222, in get_operation
    return super().get_operation(path, path_regex, method, registry)
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py", line 83, in get_operation
    operation['responses'] = self._get_response_bodies()
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py", line 975, in _get_response_bodies
    return {'200': self._get_response_for_code(response_serializers, '200')}
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py", line 1036, in _get_response_for_code
    paginated_name = f'Paginated{self._get_serializer_name(serializer, "response")}List'
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py", line 1119, in _get_serializer_name
    return self._get_serializer_name(serializer.child, direction)
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py", line 1126, in _get_serializer_name
    if is_patched_serializer(serializer, direction):
  File "/usr/local/lib/python3.7/site-packages/drf_spectacular/plumbing.py", line 101, in is_patched_serializer
    and serializer.partial
AttributeError: 'DictField' object has no attribute 'partial'

Expected behavior

Since this should be a valid serializer (test proving that is below), I'd expect the is_patched_serializer to handle a DictField.

    def test_list_serializer(self):
        from rest_framework.serializers import DictField, ListSerializer

        class MyListSerializer(ListSerializer):
            child = DictField()

        my_list = [{}, {'a': 'b'}]

        serializer = MyListSerializer(my_list)
        assert serializer.data == [{}, {'a': 'b'}]
@tfranzel
Copy link
Owner

tfranzel commented Jun 1, 2021

hi. i cannot reproduce this issue. also this does not look like the newest version. the following example has worked for me.

def test_list_dict_field(no_warnings):

    class XAPIView(APIView):
        @extend_schema(responses=serializers.ListSerializer(child=serializers.DictField()))
        def get(self, request):
            pass  # pragma: no cover

    schema = generate_schema('x', view=XAPIView)

@mblayman
Copy link
Author

mblayman commented Jun 2, 2021

Thanks for the example. That helped me write the reproduction test. The problem manifests itself for a ListAPIView. I was able to get a reproduction with:

    def test_list_dict_field(no_warnings):
        from django.urls import path
        from drf_spectacular.generators import SchemaGenerator
        from drf_spectacular.utils import extend_schema
        from drf_spectacular.validation import validate_schema
        from rest_framework.generics import ListAPIView
        from rest_framework.serializers import DictField, ListSerializer

        class XAPIView(ListAPIView):
            @extend_schema(responses=ListSerializer(child=DictField()))
            def get(self, request):
                pass  # pragma: no cover

        generator = SchemaGenerator(patterns=[path('v1/x', XAPIView.as_view())])
        schema = generator.get_schema(request=None, public=True)
        validate_schema(schema)

@tfranzel
Copy link
Owner

tfranzel commented Jun 9, 2021

hey @mblayman,

i still cannot reproduce the issue. at least on the current master. your snippet generates without a problem and with the expected output. i just pasted it in test_regressions.py and it ran like that.

are you using the most recent version there? although i cannot remember any fixes there in the past couple of releases.

@mblayman
Copy link
Author

mblayman commented Jun 9, 2021

We weren't when I reported this. I can try again on 0.17.0 (hopefully later today) and report if I can replicate the issue still.

@mblayman
Copy link
Author

mblayman commented Jun 9, 2021

I ran this same test that I have above with 0.17.0 and it still fails for me. Is there anything else I could provide to help replicate this?

    def test_list_dict_field(no_warnings):
        from django.urls import path
        from drf_spectacular.generators import SchemaGenerator
        from drf_spectacular.utils import extend_schema
        from drf_spectacular.validation import validate_schema
        from rest_framework.generics import ListAPIView
        from rest_framework.serializers import DictField, ListSerializer
    
        class XAPIView(ListAPIView):
            @extend_schema(responses=ListSerializer(child=DictField()))
            def get(self, request):
                pass  # pragma: no cover
    
        generator = SchemaGenerator(patterns=[path('v1/x', XAPIView.as_view())])
>       schema = generator.get_schema(request=None, public=True)

bhps/tests/test_views.py:1952: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/usr/local/lib/python3.7/site-packages/drf_spectacular/generators.py:257: in get_schema
    paths=self.parse(request, public),
/usr/local/lib/python3.7/site-packages/drf_spectacular/generators.py:232: in parse
    path, path_regex, path_prefix, method, self.registry
/usr/local/lib/python3.7/site-packages/drf_spectacular/utils.py:274: in get_operation
    return super().get_operation(path, path_regex, path_prefix, method, registry)
/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py:84: in get_operation
    operation['responses'] = self._get_response_bodies()
/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py:1001: in _get_response_bodies
    return {'200': self._get_response_for_code(response_serializers, '200')}
/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py:1075: in _get_response_for_code
    paginated_name = f'Paginated{self._get_serializer_name(serializer, "response")}List'
/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py:1155: in _get_serializer_name
    return self._get_serializer_name(serializer.child, direction)
/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py:1162: in _get_serializer_name
    if is_patched_serializer(serializer, direction):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

serializer = DictField(), direction = 'response'

    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')
        )
E       AttributeError: 'DictField' object has no attribute 'partial'

/usr/local/lib/python3.7/site-packages/drf_spectacular/plumbing.py:97: AttributeError

@tfranzel
Copy link
Owner

tfranzel commented Jun 9, 2021

/usr/local/lib/python3.7/site-packages/drf_spectacular/openapi.py:1075: in _get_response_for_code
paginated_name = f'Paginated{self._get_serializer_name(serializer, "response")}List'

the stacktrace contains a line that is only reached by using pagination. Since you have no pagination class in your example, i have to assume that you have a global default setting. with a added pagination class i can reproduce the error.

@mblayman
Copy link
Author

mblayman commented Jun 9, 2021

That's right. We're using the LimitOffsetPagination.

REST_FRAMEWORK = {
    'DEFAULT_PAGINATION_CLASS': 'rest_framework.pagination.LimitOffsetPagination',
    ...
}

@tfranzel
Copy link
Owner

tfranzel commented Jun 9, 2021

that was a small but important detail

@tfranzel
Copy link
Owner

so i analyzed this issue. it's definitely an edge case. in theory this is not supported.

theoretically the more correct version would have been serializers.ListField(child=serializers.CharField()), which we would have rejected with a warning. that is because putting fields in responses= is generally not supported. you circumvented that by using ListSerializer instead of ListField. those two are very close and DRF is intentionally fuzzy there.

on the other hand i can see the need for this. responses= takes a heap of things, just not fields and you cannot define a list of "things" with the basic types and OpenApiTypes.

since there is no other way expressing what you need and its a very reasonable ask, i think it's acceptable to be lenient here and "do the right thing" even though it would be technically incorrect.

tfranzel added a commit that referenced this issue Jun 11, 2021
strictly speaking this is not correct but it's the only
way to specify a unnamed list with extend_schema.
also the unpaginated version we do already support.
@tfranzel
Copy link
Owner

this should fix it. i also noticed that we support this for unpaginated endpoints. so having parity again is another benefit.

@tfranzel tfranzel added enhancement New feature or request fix confirmation pending issue has been fixed and confirmation from issue reporter is pending labels Jun 11, 2021
@mblayman
Copy link
Author

Thank you for digging into this and fixing it up. I appreciate your attention to detail.

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fix confirmation pending issue has been fixed and confirmation from issue reporter is pending
Projects
None yet
Development

No branches or pull requests

2 participants