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

ListSerializer doesn't handle unique_together constraints #3970

Closed
5 of 6 tasks
hanssto opened this issue Feb 28, 2016 · 3 comments · Fixed by #4192
Closed
5 of 6 tasks

ListSerializer doesn't handle unique_together constraints #3970

hanssto opened this issue Feb 28, 2016 · 3 comments · Fixed by #4192

Comments

@hanssto
Copy link

hanssto commented Feb 28, 2016

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.
  • I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

As referenced in
#2575
miki725/django-rest-framework-bulk#30

ListSerializer attempts to apply unique together validation on the queryset passed to it, but some part of the code is passed the full queryset, where it attempts to look up attributes on what it expects to be individual instances.

Steps to reproduce

I've modified a test case found in the above links, and run them against master. I'm not sure what the file should be named, and if it follows your quality standards, so I have not submitted a PR with it.

from django.db import models
from django.test import TestCase

from rest_framework import serializers


class ManyUniqueTogetherModel(models.Model):
    foo = models.IntegerField()
    bar = models.IntegerField()
    class Meta:
        unique_together = ("foo", "bar")


class ManyUniqueTogetherSerializer(serializers.ModelSerializer):
    class Meta:
        model = ManyUniqueTogetherModel


class ManyUniqueTogetherTestCase(TestCase):

    def test_partial_false(self):
        obj = ManyUniqueTogetherModel.objects.create(foo=1, bar=2)
        serializer = ManyUniqueTogetherSerializer(
            instance=ManyUniqueTogetherModel.objects.all(),
            data=[{
                "id": obj.pk,
                "foo": 5,
                "bar": 6,
            }],
            many=True
        )
        self.assertTrue(serializer.is_valid())

    def test_partial_true(self):
        obj = ManyUniqueTogetherModel.objects.create(foo=1, bar=2)
        serializer = ManyUniqueTogetherSerializer(
            instance=ManyUniqueTogetherModel.objects.all(),
            data=[{
                "id": obj.pk,
                "foo": 5,
            }],
            partial=True,
            many=True
        )
        self.assertTrue(serializer.is_valid())

Expected behavior

The code path looks up properties on individual instances in the queryset, is_valid() returns either True or False depending on the actual data, the tests pass.

Actual behavior

====================================================================== FAILURES ======================================================================
___________________________________________________ ManyUniqueTogetherTestCase.test_partial_false ____________________________________________________
tests/test_unique_together.py:35: in test_partial_false
    self.assertTrue(serializer.is_valid())
rest_framework/serializers.py:213: in is_valid
    self._validated_data = self.run_validation(self.initial_data)
rest_framework/serializers.py:557: in run_validation
    value = self.to_internal_value(data)
rest_framework/serializers.py:593: in to_internal_value
    validated = self.child.run_validation(item)
rest_framework/serializers.py:409: in run_validation
    self.run_validators(value)
rest_framework/fields.py:499: in run_validators
    validator(value)
rest_framework/validators.py:142: in __call__
    queryset = self.exclude_current_instance(attrs, queryset)
rest_framework/validators.py:135: in exclude_current_instance
    return queryset.exclude(pk=self.instance.pk)
E   AttributeError: 'QuerySet' object has no attribute 'pk'
____________________________________________________ ManyUniqueTogetherTestCase.test_partial_true ____________________________________________________
tests/test_unique_together.py:50: in test_partial_true
    self.assertTrue(serializer.is_valid())
rest_framework/serializers.py:213: in is_valid
    self._validated_data = self.run_validation(self.initial_data)
rest_framework/serializers.py:557: in run_validation
    value = self.to_internal_value(data)
rest_framework/serializers.py:593: in to_internal_value
    validated = self.child.run_validation(item)
rest_framework/serializers.py:409: in run_validation
    self.run_validators(value)
rest_framework/fields.py:499: in run_validators
    validator(value)
rest_framework/validators.py:141: in __call__
    queryset = self.filter_queryset(attrs, queryset)
rest_framework/validators.py:120: in filter_queryset
    attrs[field_name] = getattr(self.instance, field_name)
E   AttributeError: 'QuerySet' object has no attribute 'bar'
@xordoquy
Copy link
Collaborator

xordoquy commented Feb 28, 2016

Note that I've been performing something similar but handled it outside of generic DRF.
Reason is, even if UniqueTogetherConstraint would work with QuerySet it would still fail in some cases such as:

data=[{
                "id": id1,
                "foo": 5,
                "bar": 6,
            }, {
                "id": id2,
                "foo": 5,
                "bar": 6,
            },],

I suspect there are some other specific use cases that would prove hard to solve with validators in their current forms.

@LouWii
Copy link

LouWii commented Mar 5, 2016

Could it be related to the issue I'm having ?

I got

author = models.ForeignKey(User)
created_at = models.DateTimeField(auto_now_add=True)

class Meta:
    get_latest_by = "created_at"
    ordering = ['-created_at']
    unique_together = [
        ["author", "created_at"]
    ]

in my Model. User is a cms.models.User object.

For some reason, Swagger is throwing an error when trying to list available API endpoints 'CreateOnlyDefault' object has no attribute 'is_update'

Traceback:
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/django/core/handlers/base.py" in get_response
  132.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/django/views/decorators/csrf.py" in wrapped_view
  58.         return view_func(*args, **kwargs)
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/django/views/generic/base.py" in view
  71.             return self.dispatch(request, *args, **kwargs)
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/rest_framework/views.py" in dispatch
  466.             response = self.handle_exception(exc)
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/rest_framework/views.py" in dispatch
  463.             response = handler(request, *args, **kwargs)
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/rest_framework_swagger/views.py" in get
  164.             'models': generator.get_models(apis),
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/rest_framework_swagger/docgenerator.py" in get_models
  139.             data = self._get_serializer_fields(serializer)
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/rest_framework_swagger/docgenerator.py" in _get_serializer_fields
  328.                 'defaultValue': get_default_value(field),
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/rest_framework_swagger/introspectors.py" in get_default_value
  54.         default_value = default_value()
File "/Users/louis/Documents/WebProjects/Python/SiteTestlib/python2.7/site-packages/rest_framework/fields.py" in __call__
  225.         if self.is_update:

I know Django REST Swagger is not part of that project, but the call is made from Django REST Framework if I understand the stack trace correctly.

@tomchristie
Copy link
Member

tomchristie commented Jun 13, 2016

Related: #2996.

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