yezyilomo / django-restql

Turn your API made with Django REST Framework(DRF) into a GraphQL like API.
https://yezyilomo.github.io/django-restql
MIT License
616 stars 43 forks source link

exclude kwarg not working on 0.10.1 but is ok on 0.8.4 #178

Closed caesarleong closed 4 years ago

caesarleong commented 4 years ago
class IssueSerializer(DynamicFieldsMixin, serializers.ModelSerializer):
    task_detail = TaskDetailSerializer(
        read_only=True, exclude=['cashflow', 'cashflow_pending'])

    class Meta:
        model = Issue
        fields = '__all__'

I am upgrade from version 0.8.4, it works fine when version 0.8.4. But the same code is not working on version 0.10.1.

yezyilomo commented 4 years ago

Your case should be covered by this test https://github.com/yezyilomo/django-restql/blob/6081ab198ef4e7d2d2af3141360be4875f2df657/tests/testapp/serializers.py#L54-L59 This runs fine on v0.10.1, am having a hard time reproducing it, what are you getting? error or it doesn't exclude the field in a response?.

caesarleong commented 4 years ago

Hi yezyilomo,

this is the traceback i met: 2020-07-06 09:29:03,739 [ERROR][django.request:228] - Internal Server Error: /backend/issue_task_details/ Traceback (most recent call last): File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner response = get_response(request) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response response = self.process_exception_by_middleware(e, request) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response response = wrapped_callback(request, *callback_args, callback_kwargs) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view return view_func(*args, *kwargs) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/viewsets.py", line 114, in view return self.dispatch(request, args, kwargs) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/views.py", line 505, in dispatch response = self.handle_exception(exc) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/views.py", line 465, in handle_exception self.raise_uncaught_exception(exc) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/views.py", line 476, in raise_uncaught_exception raise exc File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/views.py", line 502, in dispatch response = handler(request, *args, **kwargs) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/mixins.py", line 40, in list page = self.paginate_queryset(queryset) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/generics.py", line 171, in paginate_queryset return self.paginator.paginate_queryset(queryset, self.request, view=self) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/rest_framework/pagination.py", line 218, in paginate_queryset return list(self.page) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/core/paginator.py", line 150, in len return len(self.object_list) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/query.py", line 256, in len self._fetch_all() File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/query.py", line 1242, in _fetch_all self._result_cache = list(self._iterable_class(self)) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/query.py", line 55, in iter results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 1127, in execute_sql sql, params = self.as_sql() File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 474, in as_sql extra_select, order_by, group_by = self.pre_sql_setup() File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 54, in pre_sql_setup self.setup_query() File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 45, in setup_query self.select, self.klass_info, self.annotation_col_map = self.get_select() File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 240, in get_select related_klass_infos = self.get_related_selections(select) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 851, in get_related_selections select, f.remote_field.model._meta, alias, cur_depth + 1, next, restricted) File "/home/caesar/Works/kingsman/venv_kingsman/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 940, in get_related_selections ', '.join(_get_field_choices()) or '(none)', django.core.exceptions.FieldError: Invalid field name(s) given in select_related: 'cashflow', 'cashflow_pending'. Choices are: pay_content_type, task, assignee, paytype, feedback_image, feedback_video, feedback_device, pending_content_type, content_type, commission_pending, commission_commit

This is my TaskDetailSerializer, the cashflow & cashflow_pending do exist.

class TaskDetailSerializer(DynamicFieldsMixin, serializers.ModelSerializer):
    assignee = serializers.CharField(
        source='assignee.username', allow_null=True)
    task = PayTaskSerializer(read_only=True, fields=['order_id'])
    feedback_device = serializers.CharField(
        source='device.identity', allow_null=True)
    feedback_image = FileSerializer(read_only=True, fields=['file'])
    feedback_video = FileSerializer(read_only=True, fields=['file'])
    cashflow = MerchantCashFlowSerializer(read_only=True)
    cashflow_pending = MerchantCashFlowPendingSerializer(read_only=True)
    commission_commit = CodeUserCashFlowSerializer(read_only=True)
    commission_pending = CodeUserCashFlowPendingSerializer(read_only=True)
    pay = GenericRelatedField({
        QRPay: QRPaySerializer(),
        TransferPay: TransferPaySerializer()
    })

    class Meta:
        model = TaskDetail
        fields = ['id', 'task', 'assignee', 'cashflow_pending', 'cashflow',
                  'commission_pending', 'commission_commit', 'paytype',
                  'amount', 'commission', 'actual', 'pay', 'feedback_device',
                  'feedback_image', 'feedback_video', 'status', 'action_type',
                  'fail_reason', 'comment', 'update_time']
yezyilomo commented 4 years ago

That's caused by giving wrong field name(s) to select_related(), probably in your view. Are you using EagerLoadingMixin in your view?, if not where exactly are you using select_related?.

caesarleong commented 4 years ago

I do use it through django_auto_prefetching. is that mean they cannot work together on 0.10.1?

yezyilomo commented 4 years ago

Okay, I think the issue is on that library, am also wondering why would it select cashflow_pending & cashflow when they have been excluded?(EagerLoadingMixin prevents this kind of problem). Also how have you declared cashflow_pending & cashflow fields in your model?.

I don't know how django_auto_prefetching works so am not sure if we have released something which cause this issue but all I know is prefetching is done at the view level so however it's implemented it should pass the right fields to select and prefetch to django ORM.

caesarleong commented 4 years ago

Okay, I see, I will try the EagerLoadingMixin to replace django_auto_prefetching, They seems doing the same thing to improve query performance. I will reply once testing finish. Thanks for your effort!!

caesarleong commented 4 years ago

The test performance compare between django-auto-prefetching and EagerLoadingMixin is quite close at most of the case. I will use EagerLoadingMixin to replace django-auto-prefetching. Thank you very much!