Unique constraint fails when trying to bulk update

[nikolaz111]

I'm trying to use the new bulk update that uses DRF 3. When I try to bulk update with a model that uses a unique_together constraint I get an error: 'QuerySet' object has no attribute 'pk'

Here is the code

#the model
class Answer(models.Model):
    user = models.ForeignKey(User, null=False, blank=False)
    parameter = models.ForeignKey(Parameter, null=False, blank=False)
    answer_number = models.FloatField(null=True, blank=True)
    answer_date = models.DateField(null=True, blank=True)
    answer_boolean = models.BooleanField(default=False)
    answer_currency = models.DecimalField(null=True, blank=True, max_digits=24, decimal_places=8)

    class Meta:
        unique_together = ("user", "parameter")

#the serializer
class AnswerSerializer(BulkSerializerMixin, serializers.ModelSerializer):
    answer = NoValidationField()

    class Meta:
        model = Answer
        fields = ['id', 'answer', 'parameter', 'user']
        list_serializer_class = BulkListSerializer

#the view
class AnswerList(ListBulkCreateUpdateAPIView):
    queryset = Answer.objects.all()
    serializer_class = AnswerSerializer

I tried to debug the problem and I think it is a validation problem. Plase help.

TY

[miki725]

What's your traceback? On Feb 17, 2015 9:52 PM, "nikolaz111" notifications@github.com wrote:

I'm trying to use the new bulk update that uses DRF 3. When I try to bulk update with a model that uses a unique_together constraint I get an error: 'QuerySet' object has no attribute 'pk'

Here is the code

the modelclass Answer(models.Model):

user = models.ForeignKey(User, null=False, blank=False)
parameter = models.ForeignKey(Parameter, null=False, blank=False)
answer_number = models.FloatField(null=True, blank=True)
answer_date = models.DateField(null=True, blank=True)
answer_boolean = models.BooleanField(default=False)
answer_currency = models.DecimalField(null=True, blank=True, max_digits=24, decimal_places=8)

class Meta:
    unique_together = ("user", "parameter")

the serializerclass AnswerSerializer(BulkSerializerMixin, serializers.ModelSerializer):

answer = NoValidationField()

class Meta:
    model = Answer
    fields = ['id', 'answer', 'parameter', 'user']
    list_serializer_class = BulkListSerializer

the viewclass AnswerList(ListBulkCreateUpdateAPIView):

queryset = Answer.objects.all()
serializer_class = AnswerSerializer

I tried to debug the problem and I think it is a validation problem. Plase help.

TY

— Reply to this email directly or view it on GitHub https://github.com/miki725/django-rest-framework-bulk/issues/30.

[nikolaz111]

Environment:

Request Method: PUT Request URL: http://localhost:8000/api/1.0/q/answers

Django Version: 1.7.4 Python Version: 3.4.0 Installed Applications: ('django.contrib.admin', 'django.contrib.auth', 'django.contrib.contenttypes', 'django.contrib.sessions', 'django.contrib.messages', 'django.contrib.staticfiles', 'rest_framework', 'rest_framework_bulk', 'corsheaders', 'questions', 'users', 'goals', 'octave') Installed Middleware: ('django.contrib.sessions.middleware.SessionMiddleware', 'corsheaders.middleware.CorsMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware')

Traceback: File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/django/core/handlers/base.py" in get_response

1. response = wrapped_callback(request, _callback_args, *_callback_kwargs) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/django/views/decorators/csrf.py" in wrapped_view
2. return view_func(_args, *_kwargs) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/django/views/generic/base.py" in view
3. return self.dispatch(request, _args, *_kwargs) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/views.py" in dispatch
4. response = self.handle_exception(exc) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/views.py" in dispatch
5. response = handler(request, _args, *_kwargs) File "/home/nicolas/alkanza/alkanza-django/app/questions/views.py" in put
6. return self.bulk_update(request, _args, *_kwargs) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework_bulk/drf3/mixins.py" in bulk_update
7. serializer.is_valid(raise_exception=True) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/serializers.py" in is_valid
8. self._validated_data = self.run_validation(self.initial_data) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/serializers.py" in run_validation
9. value = self.to_internal_value(data) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/serializers.py" in to_internal_value
10. validated = self.child.run_validation(item) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/serializers.py" in run_validation
11. self.run_validators(value) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/fields.py" in run_validators
12. validator(value) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/validators.py" in call
13. queryset = self.exclude_current_instance(attrs, queryset) File "/home/nicolas/alkanza/alkanza-django/app/venv34/lib/python3.4/site-packages/rest_framework/validators.py" in exclude_current_instance
14. return queryset.exclude(pk=self.instance.pk)

Exception Type: AttributeError at /api/1.0/q/answers Exception Value: 'QuerySet' object has no attribute 'pk'

[miki725]

Don't see anything wrong. Will need to debug.

[nikolaz111]

I debugged it and saw that the problem was that instance is a QuerySet. Maybe is a DRF problem? Thanks for fast reply.

[kevin-brown]

The issue sounds like DRF is assuming you are working with a single object, but not a queryset. The issue though is that we are making it clear that it's multiple objects.

I'd have to look further into the traceback to determine the actual cause though.

[miki725]

Seems like this is an issue in DRF itself. It blows up here.

[miki725]

Just opened PR in DRF with a preliminary fix - https://github.com/tomchristie/django-rest-framework/pull/2575

Please let me know if that fixes your problem.

[nikolaz111]

Indeed it fixes my issue. Thank you very much.

[sazary]

Looking at PR #2575 in DRF itself, and then at PR #2720, I can see where the problem is, but I still don't know what the fix is, since @tomchristie said that he will not accept that pull request because that is not general enough.
I have "id" in my data so PR #2720 is OK for me, but I don't want to manually change the code and break it every time it updates. What can I do?

[mstachowiak]

Agreed this is a bug with DRF. While waiting for a DRF fix (I scanned through the DRF issue list and didn't see a matching entry for this bug), what if you did the following:

Instead of validating the list serializer in it's entirety, iterate over each item in the list and validate it individually. As each item is validated, the validated data can be captured in a running validated data list. Once every item has been validated, the list of validated data is set as the validated data on the original list serializer. The concept is outlined below. NOTE: To do this properly, you would not raise an exception immediately if an individual item fails validation as I have done, but rather capture the errors from each item that fails and return a complete error response for the whole list.

    def bulk_update(self, request, *args, **kwargs):
        partial = kwargs.pop('partial', False)
        # restrict the update to the filtered queryset
        serializer = self.get_serializer(
            self.filter_queryset(self.get_queryset()),
            data=request.data,
            many=True,
            partial=partial,
        )
        validated_data = []
        for item in request.data:
            item_serializer = self.get_serializer(
                data=item,
                partial=partial,
            )
            item_serializer.is_valid(raise_exception=True)
            validated_data.append(item_serializer.validated_data)
        serializer._validated_data = validated_data
        self.perform_bulk_update(serializer)
        return Response(serializer.data, status=status.HTTP_200_OK)

[adelaby]

Jumpin' in.

@mstachowiak this is a nice fix, I'm using it, actually and added error catching. There was another improvement to make for it to work, though: pass the updated instance to the item_serializer (otherwise it acts as if you want to create the object and not update it). Here's the version I suggest:

    def bulk_update(self, request, *args, **kwargs):
        partial = kwargs.pop('partial', False)
        # restrict the update to the filtered queryset
        serializer = self.get_serializer(
            self.filter_queryset(self.get_queryset()),
            data=request.data,
            many=True,
            partial=partial,
        )
        validated_data = []
        validation_errors = []
        for item in request.data:
            item_serializer = self.get_serializer(
                self.get_queryset().get(pk=item['id']),
                data=item,
                partial=partial,
            )
            item_serializer.is_valid(raise_exception=True)
            if item_serializer.errors:
                validation_errors.append(item_serializer.errors)
            validated_data.append(item_serializer.validated_data)
        if validation_errors:
            raise ValidationError(validation_errors)
        serializer._validated_data = validated_data
        self.perform_bulk_update(serializer)
        return Response(serializer.data, status=status.HTTP_200_OK)

Since it is on hold (and looks a little like a dead end...) on django-rest-framework side maybe the patch can be added directly to drf-bulk, what do you think @miki725 ?

[karthikvrao]

@adelaby thanks for extended code example. I would however like to suggest a couple of corrections:

• Raise exception in is_valid should be removed in order to collect all the errors and raise them together later.
• It would be better to use filtered queryset for item serializer like the list serializer and also use get_object_or_404 instead of get on the queryset (as done by DRF in get_object).

Corrected code:

def bulk_update(self, request, *args, **kwargs):
        partial = kwargs.pop('partial', False)
        # restrict the update to the filtered queryset
        serializer = self.get_serializer(
            self.filter_queryset(self.get_queryset()),
            data=request.data,
            many=True,
            partial=partial,
        )
        validated_data = []
        validation_errors = []
        for item in request.data:
            item_serializer = self.get_serializer(
                get_object_or_404(self.filter_queryset(self.get_queryset()), pk=item['id']),
                data=item,
                partial=partial,
            )
            if not item_serializer.is_valid():
                validation_errors.append(item_serializer.errors)
            validated_data.append(item_serializer.validated_data)
        if validation_errors:
            raise ValidationError(validation_errors)
        serializer._validated_data = validated_data
        self.perform_bulk_update(serializer)
        return Response(serializer.data, status=status.HTTP_200_OK)

[nikitautiu]

Considering that this bug is caused by the to_internal_value method of ListSerializer, as mentioned in this issue tomchristie/django-rest-framework#2720, couldn't we simply override the default behaviour of the method to properly set the child's instance to the one in the queryset whose update_lookup_field matches. Something like this maybe(?)

for item in data:
   try:
       if self.instance is not None:
           id_attr = getattr(self.child.Meta, 'update_lookup_field', 'id')
           # there should also be some preemptive checks
           # that the data actually has the id
           filters = {id_attr: item.get(id_attr, None)}
           self.child.instance = self.instance.filter(**filters).first()
       validated = self.child.run_validation(item)
   except ValidationError as exc:
       errors.append(exc.detail)
   else:
       ret.append(validated)
       errors.append({})

Correct me if I am wrong, I don't know what the stance is on rewriting DRF methods.

I understand their team's decision not to alter it, as the default behaviour of ListSerializers is not currently defined in DRF and, for all they care, one implementation could replace the entire collection like a DELETE/POST and another can behave like this one. However, in our case this behaviour is pretty well-defined.

Another less invasive, but more complex option might be to add a custom many_init classmethod on the BulkSerializerMixin that returns a custom ListSerializer implemented by us.

Any opinions?

[kevin-brown]

Another less invasive, but more complex option might be to add a custom many_init classmethod on the BulkSerializerMixin that returns a custom ListSerializer implemented by us.

We already do.

[nikitautiu]

@kevin-brown Sorry, I think I didn't explain it correctly. I actually meant many_init returning a custom serializer class, not directly inheriting from ListSerializer in which we actually implement the to_internal_value ourselves so we can correct the behaviour that is currently implemented by DRF. This means implementing the list serializer as a derivate of Serializer, but in retrospect it seems like a bit of an overkill.

[legshort]

'id' key error raises my case since individual serializer run .is_valid() instead of ListSerializer so that to_internal_value() does not add 'id' from request data at BulkSerializerMixin because "isinstance(self.root, BulkListSerializer)" fails.

I munually add root with BulkListSerializer in order to let BulkSerializerMixin.to_internal_value() add 'id' for 'PUT' and 'PATCH' mode.

for item in request.data:
    item_serializer = self.get_serializer(
        get_object_or_404(self.filter_queryset(self.get_queryset()), pk=item['id']),
        data=item,
        partial=partial,
    )
    # add here
    item_serializer.root = serializer
    if not item_serializer.is_valid():

[julienfabre]

Ran into this issue with the sample code provided in the README, while trying to bulk update a basic model (with only a unique constraint on the primary key). I got the error QuerySet' object has no attribute 'pk'.

Out of the box, the following seems to be broken (from the README):

# update multiple resources (requires all fields)
PUT
[{"field":"value","field2":"value2"}]   <- json list of objects in data

I'm running Django 1.10.2, Django Rest Framework 3.5.1 and Django Rest Framework-Bulk 0.2.1

[exploreshaifali]

I was also facing KeyError for id, adding root was not working for me. I solved in following manner:

for item in request.data:
    item_serializer = self.get_serializer(
     get_object_or_404(self.filter_queryset(self.get_queryset()), pk=item['id']),
     data=item,
     partial=True,
     )
    if not item_serializer.is_valid():
    validation_errors.append(item_serializer.errors)
    obj_data = item_serializer.validated_data
    # by default validated_data does not have `id`, so adding it in
    # validated_data of each iteam
    obj_data['id'] = item['id']
    validated_data.append(obj_data)

[tomchristie]

Closing as stale. Can consider reopening if it turns out this is still a current issue.