From badf7779f296ea4c4dfa078a02353b9736abe913 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?George-Cristian=20B=C3=AErzan?= <g.birzan@mdg.bg>
Date: Wed, 15 Jun 2016 14:19:45 +0300
Subject: [PATCH 1/2] #4198: - Raise exception if field is defined on the class
 and present in extra_kwargs or read_only_fields - Do not overwrite options
 specified in extra_kwargs in get_uniqueness_extra_kwargs

---
 rest_framework/serializers.py  | 12 ++++++---
 tests/test_model_serializer.py | 45 ++++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index 3fcc85c3b..7883bd80d 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -947,6 +947,11 @@ class ModelSerializer(Serializer):
         # Determine any extra field arguments and hidden fields that
         # should be included
         extra_kwargs = self.get_extra_kwargs()
+        for field in extra_kwargs:
+            assert field not in declared_fields, (
+                "Field {} is declared on the class and also present in "
+                "extra_kwargs or read_only_fields".format(field)
+            )
         extra_kwargs, hidden_fields = self.get_uniqueness_extra_kwargs(
             field_names, declared_fields, extra_kwargs
         )
@@ -1321,12 +1326,11 @@ class ModelSerializer(Serializer):
                 # add in a hidden field that populates it.
                 hidden_fields[unique_constraint_name] = HiddenField(default=default)
 
-        # Update `extra_kwargs` with any new options.
+        # Update `extra_kwargs` with any new options but don't overwrite old values.
         for key, value in uniqueness_extra_kwargs.items():
             if key in extra_kwargs:
-                extra_kwargs[key].update(value)
-            else:
-                extra_kwargs[key] = value
+                value.update(extra_kwargs[key])
+            extra_kwargs[key] = value
 
         return extra_kwargs, hidden_fields
 
diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py
index 096cbc8d6..b812a6791 100644
--- a/tests/test_model_serializer.py
+++ b/tests/test_model_serializer.py
@@ -933,8 +933,8 @@ class TestMetaInheritance(TestCase):
 
             class Meta:
                 model = OneFieldModel
-                read_only_fields = ('char_field', 'non_model_field')
-                fields = read_only_fields
+                read_only_fields = ('char_field',)
+                fields = read_only_fields + ('non_model_field', )
                 extra_kwargs = {}
 
         class ChildSerializer(TestSerializer):
@@ -955,3 +955,44 @@ class TestMetaInheritance(TestCase):
         self.assertEqual(unicode_repr(ChildSerializer()), child_expected)
         self.assertEqual(unicode_repr(TestSerializer()), test_expected)
         self.assertEqual(unicode_repr(ChildSerializer()), child_expected)
+
+
+class TestDeclaredFieldsConflict(TestCase):
+    def test_extra_kwargs_conflict(self):
+        class TestSerializer(serializers.ModelSerializer):
+            some_field = serializers.CharField()
+
+            class Meta:
+                model = OneFieldModel
+                extra_kwargs = {'some_field': {'read_only': True}}
+        with self.assertRaises(AssertionError):
+            TestSerializer().get_fields()
+
+    def test_read_only_fields_conflict(self):
+        class TestSerializer(serializers.ModelSerializer):
+            some_field = serializers.CharField()
+
+            class Meta:
+                model = OneFieldModel
+                read_only_fields = ('some_field', )
+        with self.assertRaises(AssertionError):
+            TestSerializer().get_fields()
+
+
+class TestUniquenessOverride(TestCase):
+    def test_required_not_overwritten(self):
+        class TestModel(models.Model):
+            field_1 = models.IntegerField(null=True)
+            field_2 = models.IntegerField()
+
+            class Meta:
+                unique_together = (('field_1', 'field_2'),)
+
+        class TestSerializer(serializers.ModelSerializer):
+            class Meta:
+                model = TestModel
+                extra_kwargs = {'field_1': {'required': False}}
+
+        fields = TestSerializer().fields
+        self.assertFalse(fields['field_1'].required)
+        self.assertTrue(fields['field_2'].required)

From 5dd336ec6ca80dbcabaa1be054ac5a7f2f6c2a7d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?George-Cristian=20B=C3=AErzan?= <g.birzan@mdg.bg>
Date: Wed, 15 Jun 2016 16:35:37 +0300
Subject: [PATCH 2/2] #4198: Update documentation with behaviour changes.

---
 docs/api-guide/serializers.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md
index 038a4d6b9..48d9723ec 100644
--- a/docs/api-guide/serializers.md
+++ b/docs/api-guide/serializers.md
@@ -532,7 +532,7 @@ This option should be a list or tuple of field names, and is declared as follows
             fields = ('id', 'account_name', 'users', 'created')
             read_only_fields = ('account_name',)
 
-Model fields which have `editable=False` set, and `AutoField` fields will be set to read-only by default, and do not need to be added to the `read_only_fields` option.
+Model fields which have `editable=False` set, and `AutoField` fields will be set to read-only by default, and do not need to be added to the `read_only_fields` option. Having a field listed in `read_only_fields` and declared on the class will raise an exception.
 
 ---
 
@@ -570,6 +570,8 @@ This option is a dictionary, mapping field names to a dictionary of keyword argu
             user.save()
             return user
 
+Having arguments for a field in `extra_kwargs` for a field declared on the class will raise an exception.
+
 ## Relational fields
 
 When serializing model instances, there are a number of different ways you might choose to represent relationships.  The default representation for `ModelSerializer` is to use the primary keys of the related instances.