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

[zainnadeem786]

Summary

This PR fixes issue #8926 by updating ListSerializer to preserve and provide access to self.instance during validation when many=True. Previously, child serializers in bulk updates could not access their corresponding instance, causing AssertionErrors and inconsistent behavior. This update ensures that each item in a list serializer automatically matches its input data to the correct instance using id or pk.

Key Enhancements

1. Automated Instance Matching

   • ListSerializer.run_child_validation now attempts to match input data to items in self.instance.
   • Builds an instance map for O(1) lookup during validation.
   • Supports subclasses that may override instance assignments.

2. Validation Fixes

   • Avoided premature access to validated_data by returning run_validation results directly.
   • Manually restores instance and initial_data in deepcopied child serializers.
   • Partial updates (partial=True) correctly propagate from root serializer to list items.
   • Standardized error reporting in to_internal_value for positional list errors.

3. Test Suite Updates

   • Updated 37 tests in tests/test_serializer_lists.py to reflect consistent validation and instance matching behavior.
   • Added regression test test_many_true_instance_level_validation_uses_matched_instance to confirm that validate_<field> methods can now access self.instance during bulk updates.

Verification

• Ran pytest tests/test_serializer_lists.py ? all 37 tests passed.
• Confirmed that individual list items now correctly reference their associated instance during validation.
• Verified correct handling of allow_empty, min_length, max_length, and nested serializers.

Notes

• This PR does not change the public API of ListSerializer.
• It improves reliability and consistency for serializers using many=True, particularly for update operations.

Related Issues

• Fixes Invalid self.instance when validating the serializer using many=True #8926

[Copilot]

Pull request overview

This PR addresses issue #8926 by implementing automated instance matching in ListSerializer for bulk validation operations with many=True. The core enhancement allows child serializers to access their corresponding instance during validation by automatically matching input data to instances using id or pk fields.

Changes:

• Automated instance-to-data matching in ListSerializer.run_child_validation using a pk-based lookup map
• Enhanced error handling with consistent ErrorDetail wrapping for validation errors
• Updated test suite with new regression test and corrected assertions for validation behavior

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 15 comments.

File	Description
rest_framework/serializers.py	Core changes to ListSerializer adding automated instance matching, improved error handling, and validation flow updates
tests/test_serializer_lists.py	Updated existing tests for consistency and added regression test for issue #8926
.gitignore	Added venv/ to ignored paths

Comments suppressed due to low confidence (1)

rest_framework/serializers.py:729

• Several docstrings and inline comments were removed (e.g., for get_value, run_validation, to_internal_value, to_representation methods). While the code may be self-documenting to some extent, these comments provided useful context about the purpose and behavior of these methods. Consider keeping at least the docstrings for public methods to maintain API documentation quality, especially since this is a framework used by many developers.

    def get_value(self, dictionary):
        if html.is_html_input(dictionary):
            return html.parse_html_list(dictionary, prefix=self.field_name, default=empty)
        return dictionary.get(self.field_name, empty)

    def run_validation(self, data=empty):
        is_empty_value, data = self.validate_empty_values(data)
        if is_empty_value:
            return data

        value = self.to_internal_value(data)
        try:
            self.run_validators(value)
            value = self.validate(value)
            assert value is not None, '.validate() should return the validated data'
        except (ValidationError, DjangoValidationError) as exc:
            raise ValidationError(detail=as_serializer_error(exc))

        return value

    def run_child_validation(self, data):
        child = copy.deepcopy(self.child)
        if getattr(self, 'partial', False) or getattr(self.root, 'partial', False):
            child.partial = True

        # Field.__deepcopy__ re-instantiates the field, wiping any state.
        # If the subclass set an instance or initial_data on self.child,
        # we manually restore them to the deepcopied child.
        child_instance = getattr(self.child, 'instance', None)
        if child_instance is not None and child_instance is not self.instance:
            child.instance = child_instance
        elif hasattr(self, '_instance_map') and isinstance(data, dict):
            # Automated instance matching (#8926)
            data_pk = data.get('id') or data.get('pk')
            if data_pk is not None:
                child.instance = self._instance_map.get(str(data_pk))
            else:
                child.instance = None
        else:
            child.instance = None

        child_initial_data = getattr(self.child, 'initial_data', empty)
        if child_initial_data is not empty:
            child.initial_data = child_initial_data
        else:
            # Set initial_data for item-level validation if not already set.
            child.initial_data = data

        validated = child.run_validation(data)
        return validated

    def to_internal_value(self, data):
        if html.is_html_input(data):
            data = html.parse_html_list(data, default=[])

        if not isinstance(data, list):
            raise ValidationError({
                api_settings.NON_FIELD_ERRORS_KEY: [
                    self.error_messages['not_a_list'].format(input_type=type(data).__name__)
                ]
            })

        if not self.allow_empty and len(data) == 0:
            raise ValidationError({
                api_settings.NON_FIELD_ERRORS_KEY: [ErrorDetail(self.error_messages['empty'], code='empty')]
            })

        if self.max_length is not None and len(data) > self.max_length:
            raise ValidationError({
                api_settings.NON_FIELD_ERRORS_KEY: [ErrorDetail(self.error_messages['max_length'].format(max_length=self.max_length), code='max_length')]
            })

        if self.min_length is not None and len(data) < self.min_length:
            raise ValidationError({
                api_settings.NON_FIELD_ERRORS_KEY: [ErrorDetail(self.error_messages['min_length'].format(min_length=self.min_length), code='min_length')]
            })

        # Build a primary key mapping for instance updates (#8926)
        instance_map = {}
        if self.instance is not None:
            if isinstance(self.instance, Mapping):
                instance_map = {str(k): v for k, v in self.instance.items()}
            elif hasattr(self.instance, '__iter__'):
                for obj in self.instance:
                    pk = getattr(obj, 'pk', getattr(obj, 'id', None))
                    if pk is not None:
                        instance_map[str(pk)] = obj

        self._instance_map = instance_map

        try:
            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({})

            if any(errors):
                raise ValidationError(errors)

            return ret
        finally:
            delattr(self, '_instance_map')

    def to_representation(self, data):
        # Dealing with nested relationships, data can be a Manager,
        # so, first get a queryset from the Manager if needed.
        # We avoid .all() on QuerySets to preserve Issue #2704 behavior.
        iterable = data.all() if isinstance(data, models.manager.BaseManager) else data

        return [
            self.child.to_representation(item) for item in iterable
        ]

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[auvipy]

can you please cross check the suggestions?

[zainnadeem786]

Pushed follow-up fixes addressing the review feedback:

Standardized not_a_list error output using ErrorDetail.

Updated instance iterable checks to use explicit types (list, tuple, QuerySet).

Preserved ListSerializer.save() safety assertions, including is_valid checks, invalid-data handling, .data access, and non-None create/update guarantees.

Made _instance_map cleanup defensive.

Documented duplicate-key behavior in instance mapping (last-write-wins semantics).

Validation performed locally:

tests/test_serializer_lists.py

Issue #2704 regression test

Full test suite

All tests passing.

[zainnadeem786]

Thanks for the review. I’ve now restored the unintended docstring regressions in [serializers.py]

What was fixed
Restored as_serializer_error docstring to exactly match current upstream/main.
Restored raise_errors_on_nested_writes docstring to exactly match current upstream/main.

Scope

This change is docstring-only.
No functional ListSerializer logic was modified.
No unrelated sections from #9870 were changed.

Validation

Ran full test suite locally: all passing.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

[zainnadeem786]

Thanks @auvipy, I checked the latest Copilot comments.

Both points look relevant:

1. The id/pk lookup mismatch can happen if an object exposes both attributes with different values, so I’ll update the instance map to support both id and pk when no explicit lookup_field is configured.
2. The _list_serializer_instance_map preservation suggestion also makes sense for subclass safety, so I’ll preserve any existing value and restore it in the finally block.

I’ll push a small focused patch for these only.

[zainnadeem786]

Thanks @auvipy,

I reviewed the latest Copilot suggestions and cross-checked them against the current implementation of the instance-matching logic introduced in this PR.

Changes made

1. Align instance-map lookup behavior with run_child_validation()

The instance map was previously built using:

obj.pk -> obj.id

while run_child_validation() resolves incoming identifiers using:

id -> pk

For standard Django models this typically does not make a difference because id and pk resolve to the same value. However, for custom objects exposing both attributes with different values, the lookup order could become inconsistent and prevent the expected instance match from occurring.

To keep the matching behavior consistent throughout the validation flow, the instance-map creation logic now follows the same resolution order as run_child_validation() by preferring id first and falling back to pk.

2. Preserve existing _list_serializer_instance_map state

The current implementation temporarily assigns _list_serializer_instance_map during validation and removes it afterwards.

I updated the cleanup logic to preserve and restore any pre-existing value before applying the temporary mapping. This avoids unintentionally overwriting or removing serializer state in subclasses or custom implementations that may already use this attribute.

Test coverage

Added focused regression tests for both scenarios:

• verifies instance matching when an object exposes both id and pk with different values
• verifies that a pre-existing _list_serializer_instance_map is restored correctly after validation completes

Validation

Executed the serializer list test suite locally:

pytest tests/test_serializer_lists.py

Result:

50 passed

The changes are intentionally scoped only to the latest review feedback and do not modify any unrelated serializer behavior.

[zainnadeem786]

Hi @auvipy,

I've addressed the latest review feedback and pushed a small follow-up update.

The changes are limited to:

• aligning the instance lookup order for consistency during validation
• preserving and restoring any existing _list_serializer_instance_map state
• adding focused regression tests for both scenarios

Validation completed successfully:

pytest tests/test_serializer_lists.py

All 50 tests passed locally.

I'd appreciate it if you could take another look when you have time. Thank you for the review and guidance.

[zainnadeem786]

Thanks @auvipy.

I've addressed the remaining review comment regarding the not_a_list handling and restored the original implementation to keep the PR focused on the instance-matching behavior for #8926.

I also addressed the recent review feedback around instance-map handling and added focused regression coverage.

Validation completed successfully:

50 passed

[auvipy]

I will wait for other maintainers decision before going forward with this

[zainnadeem786]

Hi @browniebroke and @auvipy,

Once again, thank you for the review and feedback.

I've pushed a focused follow-up update addressing the latest comments:

• Fixed the missing space in the .save() assertion message.
• Simplified instance matching to use pk as the default lookup field.
• Removed the previous implicit id/pk fallback behavior.
• Added documentation for custom lookup_field usage and updated the multiple-update example.
• Updated the affected tests to cover the default pk lookup behavior, custom lookup fields, and instance-map restoration.

Validation completed successfully:

pytest tests/test_serializer_lists.py

Result: 47 passed

Please let me know if there are any remaining concerns or if further adjustments would be helpful.

Thanks again for taking the time to review this PR.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

[zainnadeem786]

@auvipy Thanks for pointing this out.

I verified that a related manager is not currently included in the instance-matching path, while to_representation() does normalize managers via .all().

The observation looks valid, but given the earlier feedback around keeping the scope focused, I wasn't sure whether manager/queryset parity should be included in this PR or handled separately.

Would you prefer support for BaseManager to be added here, or would a follow-up change be more appropriate?

[auvipy]

@browniebroke need your input here