From fb3816486cf9fac6bcc23a82200f632a6f1b6ce8 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:14:26 -0700 Subject: [PATCH] Fix (beta): avoid DRF update calling `save` on all fields (#12992) --- src/documents/serialisers.py | 49 +++++++++++++++++++++-- src/documents/tests/test_file_handling.py | 41 +++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 82c18b703..1770fc91b 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -48,6 +48,7 @@ from rest_framework import serializers from rest_framework.exceptions import PermissionDenied from rest_framework.fields import SerializerMethodField from rest_framework.filters import OrderingFilter +from rest_framework.utils import model_meta if settings.AUDIT_LOG_ENABLED: from auditlog.context import set_actor @@ -121,6 +122,45 @@ class DynamicFieldsModelSerializer(serializers.ModelSerializer[Any]): self.fields.pop(field_name) +class DocumentUpdateFieldsModelSerializer(DynamicFieldsModelSerializer): + stale_update_excluded_fields = frozenset({"filename", "archive_filename"}) + + def _get_update_fields(self, validated_data) -> list[str]: + model_fields = { + field.name + for field in self.Meta.model._meta.concrete_fields + if field.name not in self.stale_update_excluded_fields + } + update_fields = [ + field_name for field_name in validated_data if field_name in model_fields + ] + if "modified" in model_fields and "modified" not in update_fields: + update_fields.append("modified") + return update_fields + + def update(self, instance, validated_data): + serializers.raise_errors_on_nested_writes("update", self, validated_data) + info = model_meta.get_field_info(instance) + + m2m_fields = [] + for attr, value in validated_data.items(): + if attr in info.relations and info.relations[attr].to_many: + m2m_fields.append((attr, value)) + else: + setattr(instance, attr, value) + + # File names are managed by post-save file handling. Saving only the + # serializer-updated fields prevents stale in-memory path values from + # overwriting a concurrent move. + instance.save(update_fields=self._get_update_fields(validated_data)) + + for attr, value in m2m_fields: + field = getattr(instance, attr) + field.set(value) + + return instance + + class MatchingModelSerializer(serializers.ModelSerializer[Any]): document_count = serializers.IntegerField(read_only=True) @@ -989,7 +1029,7 @@ class DocumentVersionInfoSerializer(serializers.Serializer[_DocumentVersionInfo] class DocumentSerializer( OwnedObjectSerializer, NestedUpdateMixin, - DynamicFieldsModelSerializer, + DocumentUpdateFieldsModelSerializer, ): correspondent = CorrespondentField(allow_null=True) tags = TagsField(many=True) @@ -1128,10 +1168,9 @@ class DocumentSerializer( return super().validate(attrs) def update(self, instance: Document, validated_data): - if "created_date" in validated_data and "created" not in validated_data: - instance.created = validated_data.get("created_date") - instance.save() if "created_date" in validated_data: + if "created" not in validated_data: + validated_data["created"] = validated_data["created_date"] logger.warning( "created_date is deprecated, use created instead", ) @@ -1201,11 +1240,13 @@ class DocumentSerializer( for tag in instance.tags.all() if tag not in inbox_tags_not_being_added ] + if settings.AUDIT_LOG_ENABLED: with set_actor(self.user): super().update(instance, validated_data) else: super().update(instance, validated_data) + # hard delete custom field instances that were soft deleted CustomFieldInstance.deleted_objects.filter(document=instance).delete() return instance diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py index 2ae04b063..399d4ba7d 100644 --- a/src/documents/tests/test_file_handling.py +++ b/src/documents/tests/test_file_handling.py @@ -24,6 +24,7 @@ from documents.models import CustomFieldInstance from documents.models import Document from documents.models import DocumentType from documents.models import StoragePath +from documents.serialisers import DocumentSerializer from documents.tasks import empty_trash from documents.tests.factories import DocumentFactory from documents.tests.utils import DirectoriesMixin @@ -251,6 +252,46 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): self.assertIsNotFile(settings.ORIGINALS_DIR / "old" / "document.pdf") self.assertIsNotFile(settings.ARCHIVE_DIR / "old" / "document.pdf") + @override_settings(FILENAME_FORMAT="{title}") + def test_serializer_stale_update_does_not_clobber_filename(self) -> None: + old_path = settings.ORIGINALS_DIR / "original.pdf" + old_path.touch() + doc = Document.objects.create( + title="original", + mime_type="application/pdf", + checksum=hashlib.sha256(b"").hexdigest(), + filename="original.pdf", + ) + + first_instance = Document.objects.get(pk=doc.pk) + stale_instance = Document.objects.get(pk=doc.pk) + + serializer = DocumentSerializer( + first_instance, + data={"title": "first"}, + partial=True, + ) + self.assertTrue(serializer.is_valid(), serializer.errors) + serializer.save() + + doc.refresh_from_db() + self.assertEqual(doc.filename, "first.pdf") + self.assertIsFile(settings.ORIGINALS_DIR / "first.pdf") + + serializer = DocumentSerializer( + stale_instance, + data={"title": "second"}, + partial=True, + ) + self.assertTrue(serializer.is_valid(), serializer.errors) + serializer.save() + + doc.refresh_from_db() + self.assertEqual(doc.filename, "second.pdf") + self.assertIsFile(settings.ORIGINALS_DIR / "second.pdf") + self.assertIsNotFile(settings.ORIGINALS_DIR / "first.pdf") + self.assertIsNotFile(old_path) + @override_settings(FILENAME_FORMAT="{correspondent}/{correspondent}") def test_document_delete(self) -> None: document = Document()