Fix #8926: ListSerializer supports instance access during validation for many=True

.gitignore

@@ -13,6 +13,7 @@
 /env/
 MANIFEST
 coverage.*
+venv/
 .coverage
 .cache/
 

docs/api-guide/serializers.md

@@ -799,6 +799,8 @@ To support multiple updates you'll need to do so explicitly. When writing your m
 
 You will need to add an explicit `id` field to the instance serializer. The default implicitly-generated `id` field is marked as `read_only`. This causes it to be removed on updates. Once you declare it explicitly, it will be available in the list serializer's `update` method.
 
+During validation, `ListSerializer` matches each input item to an existing instance using `pk` by default. To use another identifier, such as `id` or `uuid`, set `lookup_field` on the child serializer's `Meta` class.
+
 Here's an example of how you might choose to implement multiple updates:
 
     class BookListSerializer(serializers.ListSerializer):
@@ -831,6 +833,7 @@ Here's an example of how you might choose to implement multiple updates:
 
         class Meta:
             list_serializer_class = BookListSerializer
+            lookup_field = 'id'
 
 ### Customizing ListSerializer initialization
 

rest_framework/serializers.py

@@ -664,7 +664,40 @@ def run_child_validation(self, data):
         self.child.initial_data = data
         return super().run_child_validation(data)
         """
-        return self.child.run_validation(data)
+        if not hasattr(self.child, 'instance'):
+            return self.child.run_validation(data)
+
+        if not (
+            hasattr(self, '_list_serializer_instance_map') and
+            isinstance(data, Mapping)
+        ):
+            return self.child.run_validation(data)
+
+        lookup_field = getattr(getattr(self.child, 'Meta', None), 'lookup_field', 'pk')
+        data_pk = data.get(lookup_field)
+
+        if data_pk is None:
+            return self.child.run_validation(data)
+
+        child_instance = self._list_serializer_instance_map.get(str(data_pk))
+        if child_instance is None:
+            return self.child.run_validation(data)
+
+        original_instance = self.child.instance
+        has_initial_data = hasattr(self.child, 'initial_data')
+        if has_initial_data:
+            original_initial_data = self.child.initial_data
+
+        try:
+            self.child.instance = child_instance
+            self.child.initial_data = data
+            return self.child.run_validation(data)
+        finally:
+            self.child.instance = original_instance
+            if has_initial_data:
+                self.child.initial_data = original_initial_data
+            elif hasattr(self.child, 'initial_data'):
+                delattr(self.child, 'initial_data')
 
     def to_internal_value(self, data):
         """
@@ -702,19 +735,50 @@ def to_internal_value(self, data):
         ret = []
         errors = []
 
-        for item in data:
-            try:
-                validated = self.run_child_validation(item)
-            except ValidationError as exc:
-                errors.append(exc.detail)
-            else:
-                ret.append(validated)
-                errors.append({})
+        # Build a primary key lookup for instance matching in many=True updates.
+        instance_map = None
+        if self.instance is not None:
+            if isinstance(self.instance, Mapping):
+                instance_map = {str(k): v for k, v in self.instance.items()}
+            elif isinstance(self.instance, (list, tuple, models.query.QuerySet)):
+                instance_map = {}
+                lookup_field = getattr(getattr(self.child, 'Meta', None), 'lookup_field', 'pk')
 
-        if any(errors):
-            raise ValidationError(errors)
+                for obj in self.instance:
+                    pk = getattr(obj, lookup_field, None)
 
-        return ret
+                    if pk is not None:
+                        key = str(pk)
+                        # If duplicate keys are present, keep the last value,
+                        # matching standard mapping assignment behavior.
+                        instance_map[key] = obj
+
+        has_instance_map = hasattr(self, '_list_serializer_instance_map')
+        if has_instance_map:
+            original_instance_map = self._list_serializer_instance_map
+
+        if instance_map is not None:
+            self._list_serializer_instance_map = instance_map
+
+        try:
+            for item in data:
+                try:
+                    validated = self.run_child_validation(item)
+                except ValidationError as exc:
+                    errors.append(exc.detail)
+                else:
+                    ret.append(validated)
+                    errors.append({})
+
+            if any(errors):
+                raise ValidationError(errors)
+
+            return ret
+        finally:
+            if instance_map is not None and has_instance_map:
+                self._list_serializer_instance_map = original_instance_map
+            elif instance_map is not None and hasattr(self, '_list_serializer_instance_map'):
+                delattr(self, '_list_serializer_instance_map')
 
     def to_representation(self, data):
         """
@@ -749,16 +813,27 @@ def save(self, **kwargs):
         """
         Save and return a list of object instances.
         """
+        assert hasattr(self, '_errors'), (
+            'You must call `.is_valid()` before calling `.save()`.'
+        )
+        assert not self.errors, (
+            'You cannot call `.save()` on a serializer with invalid data.'
+        )
+
         # Guard against incorrect use of `serializer.save(commit=False)`
         assert 'commit' not in kwargs, (
             "'commit' is not a valid keyword argument to the 'save()' method. "
             "If you need to access data before committing to the database then "
             "inspect 'serializer.validated_data' instead. "
             "You can also pass additional keyword arguments to 'save()' if you "
             "need to set extra attributes on the saved model instance. "
-            "For example: 'serializer.save(owner=request.user)'.'"
+            "For example: 'serializer.save(owner=request.user)'."
+        )
+        assert not hasattr(self, '_data'), (
+            "You cannot call `.save()` after accessing `serializer.data`. "
+            "If you need to access data before committing to the database then "
+            "inspect 'serializer.validated_data' instead. "
         )
-
         validated_data = [
             {**attrs, **kwargs} for attrs in self.validated_data
         ]

tests/test_serializer_lists.py

@@ -203,6 +203,182 @@ def update(self, instance, validated_data):
         assert updated_instances == expected_output
 
 
+class TestListSerializerInstanceMatching:
+    def test_matching_with_default_lookup_field(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            pk = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        instance = [
+            BasicObject(pk=1),
+            BasicObject(pk=2),
+        ]
+        input_data = [
+            {'pk': 1},
+            {'pk': 2},
+        ]
+
+        serializer = TestSerializer(instance, data=input_data, many=True)
+        assert serializer.is_valid()
+        assert seen_instances == instance
+
+    def test_field_validation_receives_item_initial_data(self):
+        seen_initial_data = []
+
+        class TestSerializer(serializers.Serializer):
+            pk = serializers.IntegerField()
+
+            def validate_pk(self, value):
+                seen_initial_data.append(self.initial_data)
+                return value
+
+        instance = [BasicObject(pk=1), BasicObject(pk=2)]
+        input_data = [{'pk': 1}, {'pk': 2}]
+
+        serializer = TestSerializer(instance, data=input_data, many=True)
+        assert serializer.is_valid()
+        assert seen_initial_data == input_data
+
+    def test_object_validation_receives_item_initial_data(self):
+        seen_initial_data = []
+
+        class TestSerializer(serializers.Serializer):
+            pk = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_initial_data.append(self.initial_data)
+                return attrs
+
+        instance = [BasicObject(pk=1), BasicObject(pk=2)]
+        input_data = [{'pk': 1}, {'pk': 2}]
+
+        serializer = TestSerializer(instance, data=input_data, many=True)
+        assert serializer.is_valid()
+        assert seen_initial_data == input_data
+
+    def test_child_initial_data_state_is_restored_after_validation(self):
+        class TestSerializer(serializers.Serializer):
+            pk = serializers.IntegerField()
+
+        instance = [BasicObject(pk=1)]
+        input_data = [{'pk': 1}]
+        serializer = TestSerializer(instance, data=input_data, many=True)
+        original_initial_data = serializer.child.initial_data
+
+        assert serializer.is_valid()
+        assert serializer.child.initial_data is original_initial_data
+
+        child = TestSerializer()
+        serializer = serializers.ListSerializer(
+            child=child, instance=instance, data=input_data
+        )
+
+        assert not hasattr(child, 'initial_data')
+        assert serializer.is_valid()
+        assert not hasattr(child, 'initial_data')
+
+    def test_mapping_instance_matching(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            pk = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        obj1 = BasicObject(pk=1)
+        obj2 = BasicObject(pk=2)
+        instance = {
+            '1': obj1,
+            '2': obj2,
+        }
+        input_data = [
+            {'pk': 1},
+            {'pk': 2},
+        ]
+
+        serializer = TestSerializer(instance, data=input_data, many=True)
+        assert serializer.is_valid()
+        assert seen_instances == [obj1, obj2]
+
+    def test_unsupported_instance_type_preserves_original_behavior(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            pk = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        serializer = TestSerializer(instance=123, data=[{'pk': 1}], many=True)
+        assert serializer.is_valid()
+        assert seen_instances == [123]
+
+    def test_missing_lookup_field_in_data_does_not_assign_instance(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            id = serializers.IntegerField(required=False)
+
+            class Meta:
+                lookup_field = 'uuid'
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        class TestListSerializer(serializers.ListSerializer):
+            child = TestSerializer()
+
+        serializer = TestListSerializer(
+            instance=[BasicObject(id=1, uuid='uuid-1')],
+            data=[{'id': 1}],
+        )
+        assert serializer.is_valid()
+        assert seen_instances == [None]
+
+    def test_matching_with_configurable_lookup_field(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            id = serializers.IntegerField(required=False)
+            uuid = serializers.CharField()
+
+            class Meta:
+                lookup_field = 'uuid'
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        obj1 = BasicObject(id=1, uuid='uuid-1')
+        obj2 = BasicObject(id=2, uuid='uuid-2')
+        input_data = [{'id': 1, 'uuid': 'uuid-2'}]
+
+        serializer = TestSerializer([obj1, obj2], data=input_data, many=True)
+        assert serializer.is_valid()
+        assert seen_instances == [obj2]
+
+    def test_existing_instance_map_is_restored_after_validation(self):
+        class TestSerializer(serializers.Serializer):
+            pk = serializers.IntegerField()
+
+        instance = [BasicObject(pk=1)]
+        original_instance_map = {'sentinel': BasicObject(pk=2)}
+        serializer = TestSerializer(instance, data=[{'pk': 1}], many=True)
+        serializer._list_serializer_instance_map = original_instance_map
+
+        assert serializer.is_valid()
+        assert serializer._list_serializer_instance_map is original_instance_map
+
+
 class TestNestedListSerializer:
     """
     Tests for using a ListSerializer as a field.
@@ -883,3 +1059,35 @@ def test(self):
         queryset = NullableOneToOneSource.objects.all()
         serializer = self.serializer(queryset, many=True)
         assert serializer.data
+
+
+def test_many_true_instance_level_validation_uses_matched_instance():
+    class Obj:
+        def __init__(self, id, valid):
+            self.id = id
+            self.valid = valid
+
+    class TestSerializer(serializers.Serializer):
+        id = serializers.IntegerField()
+        status = serializers.CharField()
+
+        class Meta:
+            lookup_field = 'id'
+
+        def validate_status(self, value):
+            if self.instance is None:
+                raise serializers.ValidationError("Instance not matched")
+            if not self.instance.valid:
+                raise serializers.ValidationError("Invalid instance")
+            return value
+
+    objs = [Obj(1, True), Obj(2, False)]
+    serializer = TestSerializer(
+        instance=objs,
+        data=[{"id": 1, "status": "ok"}, {"id": 2, "status": "fail"}],
+        many=True,
+        partial=True,
+    )
+
+    assert not serializer.is_valid()
+    assert serializer.errors == [{}, {'status': ['Invalid instance']}]