Fix 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.

[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]

I think this should be aligned with the changes in this PR. @browniebroke please let me know if otherwise

[auvipy]

please fix the merge conflicts. also see if any relevant parts were already fixed in the main branch

[zainnadeem786]

Hi @auvipy and @browniebroke,

Resolved all merge conflicts with encode:main, cleaned up the leftover duplicate validation loop, and fixed the Flake8 formatting issues.
Summary of changes:

• Preserved instance_map lookup behavior for ListSerializer.to_internal_value() while maintaining upstream dictionary-based error reporting.
• Aligned test assertions in tests/test_serializer_lists.py with the updated error structure.
• Fixed indentation and Flake8 spacing checks.

All local tests (pytest) and pre-commit hooks are passing cleanly, and CI checks are now green. Please let me know if any further tweaks are needed. Thank you!

[Copilot]

🟡 Changes recommended

The exact id-based issue remains unfixed by default, and custom matching and manager instances are not handled correctly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

rest_framework/serializers.py:743

• ListSerializer.to_representation() already accepts related managers by normalizing them with .all(), but this validation path excludes BaseManager. Passing a related manager as the update instance therefore leaves the manager on the child and instance-aware validators still fail. Normalize managers here as well.

            elif isinstance(self.instance, (list, tuple, models.query.QuerySet)):

• Files reviewed: 3/4 changed files
• Comments generated: 4
• Review effort level: Balanced