Exclude read_only=True fields from unique_together validation & add docs. (#4192)

* Exclude read_only=True fields from unique_together validation
* Test to ensure that unique_together validators can be removed
* Do not add uniquness_extra_kwargs when validators are explicitly declared.
* Add docs on validation in complex cases

docs/api-guide/validators.md

@@ -156,7 +156,7 @@ If you want the date field to be entirely hidden from the user, then use `Hidden
 
 ---
 
-# Advanced 'default' argument usage
+# Advanced field defaults
 
 Validators that are applied across multiple fields in the serializer can sometimes require a field input that should not be provided by the API client, but that *is* available as input to the validator.
 
@@ -188,6 +188,71 @@ It takes a single argument, which is the default value or callable that should b
 
 ---
 
+# Limitations of validators
+
+There are some ambiguous cases where you'll need to instead handle validation
+explicitly, rather than relying on the default serializer classes that
+`ModelSerializer` generates.
+
+In these cases you may want to disable the automatically generated validators,
+by specifying an empty list for the serializer `Meta.validators` attribute.
+
+## Optional fields
+
+By default "unique together" validation enforces that all fields be
+`required=True`. In some cases, you might want to explicit apply
+`required=False` to one of the fields, in which case the desired behaviour
+of the validation is ambiguous.
+
+In this case you will typically need to exclude the validator from the
+serializer class, and instead write any validation logic explicitly, either
+in the `.validate()` method, or else in the view.
+
+For example:
+
+    class BillingRecordSerializer(serializers.ModelSerializer):
+        def validate(self, data):
+            # Apply custom validation either here, or in the view.
+
+        class Meta:
+            fields = ('client', 'date', 'amount')
+            extra_kwargs = {'client' {'required': 'False'}}
+            validators = []  # Remove a default "unique together" constraint.
+
+## Updating nested serializers
+
+When applying an update to an existing instance, uniqueness validators will
+exclude the current instance from the uniqueness check. The current instance
+is available in the context of the uniqueness check, because it exists as
+an attribute on the serializer, having initially been passed using
+`instance=...` when instantiating the serializer.
+
+In the case of update operations on *nested* serializers there's no way of
+applying this exclusion, because the instance is not available.
+
+Again, you'll probably want to explicitly remove the validator from the
+serializer class, and write the code the for the validation constraint
+explicitly, in a `.validate()` method, or in the view.
+
+## Debugging complex cases
+
+If you're not sure exactly what behavior a `ModelSerializer` class will
+generate it is usually a good idea to run `manage.py shell`, and print
+an instance of the serializer, so that you can inspect the fields and
+validators that it automatically generates for you.
+
+    >>> serializer = MyComplexModelSerializer()
+    >>> print(serializer)
+    class MyComplexModelSerializer:
+        my_fields = ...
+
+Also keep in mind that with complex cases it can often be better to explicitly
+define your serializer classes, rather than relying on the default
+`ModelSerializer` behavior. This involves a little more code, but ensures
+that the resulting behavior is more transparent.
+
+---
+
 # Writing custom validators
 
 You can use any of Django's existing validators, or write your own custom validators.

rest_framework/serializers.py

@@ -1243,6 +1243,11 @@ class ModelSerializer(Serializer):
 
         read_only_fields = getattr(self.Meta, 'read_only_fields', None)
         if read_only_fields is not None:
+            if not isinstance(read_only_fields, (list, tuple)):
+                raise TypeError(
+                    'The `read_only_fields` option must be a list or tuple. '
+                    'Got %s.' % type(read_only_fields).__name__
+                )
             for field_name in read_only_fields:
                 kwargs = extra_kwargs.get(field_name, {})
                 kwargs['read_only'] = True
@@ -1258,6 +1263,9 @@ class ModelSerializer(Serializer):
 
         ('dict of updated extra kwargs', 'mapping of hidden fields')
         """
+        if getattr(self.Meta, 'validators', None) is not None:
+            return (extra_kwargs, {})
+
         model = getattr(self.Meta, 'model')
         model_fields = self._get_model_fields(
             field_names, declared_fields, extra_kwargs
@@ -1308,7 +1316,7 @@ class ModelSerializer(Serializer):
                 else:
                     uniqueness_extra_kwargs[unique_constraint_name] = {'default': default}
             elif default is not empty:
-                # The corresponding field is not present in the,
+                # The corresponding field is not present in the
                 # serializer. We have a default to use for it, so
                 # add in a hidden field that populates it.
                 hidden_fields[unique_constraint_name] = HiddenField(default=default)
@@ -1390,6 +1398,7 @@ class ModelSerializer(Serializer):
         field_names = {
             field.source for field in self.fields.values()
             if (field.source != '*') and ('.' not in field.source)
+            and not field.read_only
         }
 
         # Note that we make sure to check `unique_together` both on the

tests/test_model_serializer.py

@@ -521,8 +521,6 @@ class TestRelationalFieldMappings(TestCase):
                 one_to_one = NestedSerializer(read_only=True):
                     url = HyperlinkedIdentityField(view_name='onetoonetargetmodel-detail')
                     name = CharField(max_length=100)
-                class Meta:
-                    validators = [<UniqueTogetherValidator(queryset=UniqueTogetherModel.objects.all(), fields=('foreign_key', 'one_to_one'))>]
         """)
         if six.PY2:
             # This case is also too awkward to resolve fully across both py2

tests/test_validators.py

@@ -239,6 +239,45 @@ class TestUniquenessTogetherValidation(TestCase):
         """)
         assert repr(serializer) == expected
 
+    def test_ignore_read_only_fields(self):
+        """
+        When serializer fields are read only, then uniqueness
+        validators should not be added for that field.
+        """
+        class ReadOnlyFieldSerializer(serializers.ModelSerializer):
+            class Meta:
+                model = UniquenessTogetherModel
+                fields = ('id', 'race_name', 'position')
+                read_only_fields = ('race_name',)
+
+        serializer = ReadOnlyFieldSerializer()
+        expected = dedent("""
+            ReadOnlyFieldSerializer():
+                id = IntegerField(label='ID', read_only=True)
+                race_name = CharField(read_only=True)
+                position = IntegerField(required=True)
+        """)
+        assert repr(serializer) == expected
+
+    def test_allow_explict_override(self):
+        """
+        Ensure validators can be explicitly removed..
+        """
+        class NoValidatorsSerializer(serializers.ModelSerializer):
+            class Meta:
+                model = UniquenessTogetherModel
+                fields = ('id', 'race_name', 'position')
+                validators = []
+
+        serializer = NoValidatorsSerializer()
+        expected = dedent("""
+            NoValidatorsSerializer():
+                id = IntegerField(label='ID', read_only=True)
+                race_name = CharField(max_length=100)
+                position = IntegerField()
+        """)
+        assert repr(serializer) == expected
+
     def test_ignore_validation_for_null_fields(self):
         # None values that are on fields which are part of the uniqueness
         # constraint cause the instance to ignore uniqueness validation.