Refine ListSerializer instance matching: reduce scope, add fallback behavior, improve tests

Author: zainnadeem786

rest_framework/serializers.py

@@ -667,28 +667,32 @@ def run_child_validation(self, data):
         if not hasattr(self.child, 'instance'):
             return self.child.run_validation(data)
 
-        original_instance = self.child.instance
-        had_initial_data = hasattr(self.child, 'initial_data')
-        original_initial_data = getattr(self.child, 'initial_data', None)
+        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', None)
+        data_pk = data.get(lookup_field)
+        if data_pk is None:
+            data_pk = data.get('id')
+        if data_pk is None:
+            data_pk = data.get('pk')
+
+        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
         try:
-            if (
-                hasattr(self, '_list_serializer_instance_map') and
-                isinstance(data, Mapping)
-            ):
-                data_pk = data.get('id') or data.get('pk')
-                self.child.instance = (self._list_serializer_instance_map.get(str(data_pk))
-                                       if data_pk is not None else None)
-
-            self.child.initial_data = data
+            self.child.instance = child_instance
             return self.child.run_validation(data)
         finally:
             self.child.instance = original_instance
-            if had_initial_data:
-                self.child.initial_data = original_initial_data
-            else:
-                if hasattr(self.child, 'initial_data'):
-                    del self.child.initial_data
 
     def to_internal_value(self, data):
         """
@@ -731,20 +735,30 @@ def to_internal_value(self, data):
         errors = []
 
         # Build a primary key lookup for instance matching in many=True updates.
-        instance_map = {}
+        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', None)
+
                 for obj in self.instance:
-                    pk = getattr(obj, 'pk', getattr(obj, 'id', None))
+                    if lookup_field is not None:
+                        pk = getattr(obj, lookup_field, None)
+                    else:
+                        pk = getattr(obj, 'pk', None)
+                        if pk is None:
+                            pk = getattr(obj, 'id', None)
+
                     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
 
-        self._list_serializer_instance_map = instance_map
+        if instance_map is not None:
+            self._list_serializer_instance_map = instance_map
 
         try:
             for item in data:

tests/test_serializer_lists.py

@@ -203,6 +203,155 @@ def update(self, instance, validated_data):
         assert updated_instances == expected_output
 
 
+class TestListSerializerInstanceMatching:
+    def test_matching_with_id(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            id = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        instance = [
+            BasicObject(id=1),
+            BasicObject(id=2),
+        ]
+        input_data = [
+            {'id': 1},
+            {'id': 2},
+        ]
+
+        serializer = TestSerializer(instance, data=input_data, many=True)
+        assert serializer.is_valid()
+        assert seen_instances == instance
+
+    def test_matching_with_pk(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_matching_with_id_against_object_with_pk_only(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            id = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        instance = [BasicObject(pk=1)]
+        input_data = [{'id': 1}]
+
+        serializer = TestSerializer(instance, data=input_data, many=True)
+        assert serializer.is_valid()
+        assert seen_instances == instance
+
+    def test_mapping_instance_matching(self):
+        seen_instances = []
+
+        class TestSerializer(serializers.Serializer):
+            id = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        obj1 = BasicObject(id=1)
+        obj2 = BasicObject(id=2)
+        instance = {
+            '1': obj1,
+            '2': obj2,
+        }
+        input_data = [
+            {'id': 1},
+            {'id': 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):
+            id = serializers.IntegerField()
+
+            def validate(self, attrs):
+                seen_instances.append(self.instance)
+                return attrs
+
+        serializer = TestSerializer(instance=123, data=[{'id': 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]
+
+
 class TestNestedListSerializer:
     """
     Tests for using a ListSerializer as a field.