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

AttributeError: 'QuerySet' object has no attribute 'pk' when doing bulk update #68

Open
LaundroMat opened this issue Dec 31, 2017 · 15 comments

Comments

@LaundroMat
Copy link

LaundroMat commented Dec 31, 2017

When trying to PATCH with a list of dicts, I'm getting this error:

ERROR: test_bulk_update (api.tests.test_api.test_api_listings_bulk.TestBulkOperationsOnListings)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "E:\Development\django_projects\api\api.git\api\tests\test_api\test_api_listings_bulk.py", line 92, in test_bulk_update
    response = self.client.patch("/listings/all/", updated_listings, format="json")
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\test.py", line 315, in patch
    path, data=data, format=format, content_type=content_type, **extra)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\test.py", line 220, in patch
    return self.generic('PATCH', path, data, content_type, **extra)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\test.py", line 237, in generic
    method, path, data, content_type, secure, **extra)
  File "e:\Development\django_projects\api\lib\site-packages\django\test\client.py", line 416, in generic
    return self.request(**r)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\test.py", line 288, in request
    return super(APIClient, self).request(**kwargs)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\test.py", line 240, in request
    request = super(APIRequestFactory, self).request(**kwargs)
  File "e:\Development\django_projects\api\lib\site-packages\django\test\client.py", line 501, in request
    six.reraise(*exc_info)
  File "e:\Development\django_projects\api\lib\site-packages\django\utils\six.py", line 686, in reraise
    raise value
  File "e:\Development\django_projects\api\lib\site-packages\django\core\handlers\exception.py", line 41, in inner
    response = get_response(request)
  File "e:\Development\django_projects\api\lib\site-packages\django\core\handlers\base.py", line 187, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "e:\Development\django_projects\api\lib\site-packages\django\core\handlers\base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "e:\Development\django_projects\api\lib\site-packages\django\views\decorators\csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\viewsets.py", line 90, in view
    return self.dispatch(request, *args, **kwargs)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\views.py", line 489, in dispatch
    response = self.handle_exception(exc)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\views.py", line 449, in handle_exception
    self.raise_uncaught_exception(exc)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\views.py", line 486, in dispatch
    response = handler(request, *args, **kwargs)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework_bulk\drf3\mixins.py", line 79, in partial_bulk_update
    return self.bulk_update(request, *args, **kwargs)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework_bulk\drf3\mixins.py", line 73, in bulk_update
    serializer.is_valid(raise_exception=True)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\serializers.py", line 718, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\serializers.py", line 596, in run_validation
    value = self.to_internal_value(data)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\serializers.py", line 635, in to_internal_value
    validated = self.child.run_validation(item)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\serializers.py", line 431, in run_validation
    value = self.to_internal_value(data)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework_bulk\drf3\serializers.py", line 16, in to_internal_value
    ret = super(BulkSerializerMixin, self).to_internal_value(data)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\serializers.py", line 461, in to_internal_value
    validated_value = field.run_validation(primitive_value)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\fields.py", line 776, in run_validation
    return super(CharField, self).run_validation(data)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\fields.py", line 524, in run_validation
    self.run_validators(value)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\fields.py", line 538, in run_validators
    validator(value)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\validators.py", line 81, in __call__
    queryset = self.exclude_current_instance(queryset)
  File "e:\Development\django_projects\api\lib\site-packages\rest_framework\validators.py", line 75, in exclude_current_instance
    return queryset.exclude(pk=self.instance.pk)
AttributeError: 'QuerySet' object has no attribute 'pk'

Relevant code:

class ListingSerializer(BulkSerializerMixin, serializers.ModelSerializer):
    slug = serializers.ReadOnlyField()
    images = ListingImageSerializer(many=True, read_only=True)

    class Meta:
        model = Listing
        list_serializer_class = BulkListSerializer
        fields = '__all__'

class BaseListingViewSet(generics.BulkModelViewSet):
    queryset = Listing.objects.all()
    serializer_class = ListingSerializer

POSTing data works fine, by the way, it's only PATCH that generates this error.

I'm using

  • Django==1.11.7
  • djangorestframework==3.7.3
  • djangorestframework-bulk==0.2.1
@calmez
Copy link

calmez commented Jan 30, 2018

Already a little dated, but since I stumbled upon the same problem when trying to implement bulk actions myself I thought that I could shed some light onto this.
The problem is that exclude_current_instance on BaseUniqueForValidator as well as UniqueValidator and UniqueTogetherValidator expects the instance on the validator (which is taken from the serializer context) is a single object (see set_context). All these validators actually have an equivalent implementation of this method where the exclude is called on the queryset to take out the instance from it.
This clashes with how the serializer is instantiated – with a queryset of multiple objects. For bulk updates there is of course not just one, but more instances that need consideration.
Adapting the ListSerializer's to_internal_value such that the child serializer only gets one instance at a time (there's only on child serializer) will fix the problem. Instead of calling run_validation directly on the child serializer you should prepare the child serializer to only handle one instance at a time as sketched in the following code:

self.child.instance = self.instance.get(id=item['id'])
self.child.initial_data = item
validated = self.child.run_validation(item)

@LaundroMat
Copy link
Author

LaundroMat commented Jan 30, 2018

Thanks for your reply! I'm still a bit stuck however...

I've tried creating an AdaptedBulkListSerializerMixin with rest_framework's ListSerializer's to_internal_value method adapted to include your code.

class AdaptedBulkListSerializerMixin(object):
    def to_internal_value(self, data):
        """
        List of dicts of native values <- List of dicts of primitive datatypes.
        """
        if html.is_html_input(data):
            data = html.parse_html_list(data)

        if not isinstance(data, list):
            message = self.error_messages['not_a_list'].format(
                input_type=type(data).__name__
            )
            raise ValidationError({
                api_settings.NON_FIELD_ERRORS_KEY: [message]
            }, code='not_a_list')

        if not self.allow_empty and len(data) == 0:
            if self.parent and self.partial:
                raise SkipField()

            message = self.error_messages['empty']
            raise ValidationError({
                api_settings.NON_FIELD_ERRORS_KEY: [message]
            }, code='empty')

        ret = []
        errors = []

        for item in data:
            try:
                # Code that was inserted
                self.child.instance = self.instance.get(id=item['id'])
                self.child.initial_data = item
                # Until here
                validated = self.child.run_validation(item)
            except ValidationError as exc:
                errors.append(exc.detail)
            else:
                ret.append(validated)
                errors.append({})

        if any(errors):
            raise ValidationError(errors)

        return ret

class AdaptedBulkListSerializer(AdaptedBulkListSerializerMixin, BulkListSerializer):
    pass

class ListingSerializer(BulkSerializerMixin, serializers.HyperlinkedModelSerializer):
    slug = serializers.ReadOnlyField()
    images = ListingImageSerializer(many=True, read_only=True)
    # pk = serializers.ReadOnlyField()
    # update_lookup_field = 'id'

    class Meta:
        model = Listing
        list_serializer_class = AdaptedBulkListSerializer
        fields = '__all__'

I'm now getting an error related to the fact that the serializer's instance is None.

Traceback (most recent call last):
  File "E:\Development\django_projects\api\lib\site-packages\django\core\handlers\exception.py", line 35, in inner
    response = get_response(request)
  File "E:\Development\django_projects\api\lib\site-packages\django\core\handlers\base.py", line 128, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "E:\Development\django_projects\api\lib\site-packages\django\core\handlers\base.py", line 126, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "E:\Development\django_projects\api\lib\site-packages\django\views\decorators\csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "E:\Development\django_projects\api\lib\site-packages\rest_framework\viewsets.py", line 90, in view
    return self.dispatch(request, *args, **kwargs)
  File "E:\Development\django_projects\api\lib\site-packages\rest_framework\views.py", line 489, in dispatch
    response = self.handle_exception(exc)
  File "E:\Development\django_projects\api\lib\site-packages\rest_framework\views.py", line 449, in handle_exception
    self.raise_uncaught_exception(exc)
  File "E:\Development\django_projects\api\lib\site-packages\rest_framework\views.py", line 486, in dispatch
    response = handler(request, *args, **kwargs)
  File "E:\Development\django_projects\api\lib\site-packages\rest_framework_bulk\drf3\mixins.py", line 33, in create
    serializer.is_valid(raise_exception=True)
  File "E:\Development\django_projects\api\lib\site-packages\rest_framework\serializers.py", line 718, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "E:\Development\django_projects\api\lib\site-packages\rest_framework\serializers.py", line 596, in run_validation
    value = self.to_internal_value(data)
  File "E:\Development\django_projects\api\api.git\api\serializers.py", line 53, in to_internal_value
    self.child.instance = self.instance.get(id=item['id'])
AttributeError: 'NoneType' object has no attribute 'get'

Forgive me if I'm overlooking something really stupid :)

@calmez
Copy link

calmez commented Jan 31, 2018

Yeah, that's something I came across this morning as well. The code did not account for creations where there are no existing instances (which you are probably trying to do there).
I fixed this by checking for the existence of self.instance like in the following code:

# prepare child serializer to only handle one instance
self.child.instance = self.instance.get(id=item['id']) if self.instance else None
self.child.initial_data = item
validated = self.child.run_validation(item)

@LaundroMat
Copy link
Author

Thanks! You should try and get this in a PR.

I had to add

id = serializers.ReadOnlyField()

to my model serializer because it's a HyperlinkedModelSerializer, but other than that, it works!

@calmez
Copy link

calmez commented Feb 1, 2018

Good to hear that it works for you. If I find a minute, I'll create that PR. But seeing that the repo owner is not too active around here it might not be worth it.

@mzmmohideen
Copy link

for me i was imported BulkSerializerMixin, BulkListSerializer

from,
from rest_framework_bulk import BulkSerializerMixin, BulkListSerializer

but actually it should be from,
from rest_framework_bulk.drf3.serializers import BulkSerializerMixin, BulkListSerializer

@robindierckx
Copy link

@LaundroMat Your code works like a charm with the edit of @calmez

@dfeldstarsky
Copy link

Heya - hitting this just now too, would absolutely love if this was fixed or docs were updated to mention that this is broken atm!

@dfeldstarsky
Copy link

Ah but this project seems abandoned. Sad!

codecure added a commit to skbkontur/django-rest-framework-bulk that referenced this issue Jul 9, 2019
Dexes added a commit to skbkontur/django-rest-framework-bulk that referenced this issue Jul 9, 2019
@jonbesga
Copy link

jonbesga commented Nov 14, 2019

Thank you very much for the AdaptedBulkListSerializer @calmez.

@calmez
Copy link

calmez commented Nov 18, 2019

@jonbesga You're very welcome. 😄 I guess this is one of these nice moments with FOSS that keeps us all sharing our work 👍 Loving it ♥️

@radokristof
Copy link

Thanks for the AdaptedSerializerMixin @calmez and @LaundroMat.

Late to the game but...
However for me I think this doesn't work as it should.

I can update multiple fields with this, that's right. If I try to add a create a new object, by passing it in the list (without id) I get an 'Internal server error 'id''.

If I try to delete them, like, sending an array with one object in it (whereas I had multiple objects in this list), the serializer returns correct data, however this is not saved correctly to the database, nothing is removed.

Or this should work it like that? And I can't delete or create in one method?

Thanks!

@LaundroMat
Copy link
Author

Sorry @radokristof, but I haven't been using DRF for a long time now and I don't have the time to pick it up again.
I hope you find a solution!

@radokristof
Copy link

@LaundroMat Thanks for your response!

I fairly new to Django but what I was able to figure out is that it works (with your code) the way it should work or I might be wrong.
Object creation seems to be the only case where this method fails, I can easily update or delete them.

@DardanIljazi
Copy link

I had this error, for the ones coming here from Google, my problem was this one:

The model I wanted to serialize had a field containing unique=True in it. The multiple data I sent to the Serializer (with many=True) were not unique (already in db), and instead of having an error telling me this, the "'QuerySet' object has no attribute 'pk'" was thrown which is really confusing. Anyway, after getting rid of the uniqueness, my code works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants