Fix: select_related in remove_doclink() to avoid signal-triggered reload

Same pattern as the update_or_create() fix: target_doc_field_instance was
fetched without select_related, so its .document/.field weren't cached
when .save() fired the post_save signal -- auditlog's receiver touching
.document re-fetched it, once per (source, target) pair being unlinked
with no batching across calls. Also benefits the single-document PATCH
path in serialisers.py, which calls the same helper.

Broadened the removal test's query assertion now that both sides are fixed.

src/documents/bulk_edit.py

@@ -305,46 +305,74 @@ def modify_custom_fields(
         else [(field, None) for field in add_custom_fields]
     )
 
-    custom_fields = CustomField.objects.filter(
+    custom_fields_by_id: dict[int, CustomField] = {
-        id__in=[int(field) for field, _ in add_custom_fields],
+        cf.id: cf
-    ).distinct()
+        for cf in CustomField.objects.filter(
+            id__in=[int(field) for field, _ in add_custom_fields],
+        )
+    }
+    # Deferred, not `.only()`: signal receivers touch other Document fields,
+    # and `.only("pk")` would just turn that into a per-document reload.
+    # `content` is the one field both large and unused here. Skipped
+    # entirely for a remove-only call -- the removal pass below resolves
+    # its own documents.
+    docs_by_id: dict[int, Document] = (
+        {
+            doc.id: doc
+            for doc in Document.objects.filter(id__in=affected_docs).defer("content")
+        }
+        if add_custom_fields
+        else {}
+    )
     for field_id, value in add_custom_fields:
+        custom_field = custom_fields_by_id[field_id]
+        value_field = CustomFieldInstance.TYPE_TO_DATA_STORE_NAME_MAP[
+            custom_field.data_type
+        ]
         for doc_id in affected_docs:
-            defaults = {}
+            defaults = {value_field: value}
-            custom_field = custom_fields.get(id=field_id)
+            if (
-            if custom_field:
+                custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK
-                value_field = CustomFieldInstance.TYPE_TO_DATA_STORE_NAME_MAP[
+                and value
-                    custom_field.data_type
+                and doc_id in value
-                ]
+            ):
-                defaults[value_field] = value
+                # Prevent self-linking
-                if (
+                continue
-                    custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK
+            # Not update_or_create(): it fetches an existing row via plain
-                    and value
+            # `.get()` before calling .save(), so a signal receiver touching
-                    and doc_id in value
+            # `.field`/`.document` (e.g. auditlog) on that save re-fetches
-                ):
+            # per instance regardless of what's passed in as lookup kwargs.
-                    # Prevent self-linking
+            # Assigning the cached objects ourselves before .save() avoids
-                    continue
+            # that for both the create and update case.
-            CustomFieldInstance.objects.update_or_create(
+            try:
-                document_id=doc_id,
+                instance = CustomFieldInstance.objects.get(
-                field_id=field_id,
+                    document=docs_by_id[doc_id],
-                defaults=defaults,
+                    field=custom_field,
-            )
+                )
+            except CustomFieldInstance.DoesNotExist:
+                instance = CustomFieldInstance(
+                    document=docs_by_id[doc_id],
+                    field=custom_field,
+                )
+            instance.document = docs_by_id[doc_id]
+            instance.field = custom_field
+            for attr, val in defaults.items():
+                setattr(instance, attr, val)
+            instance.save()
             if custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK:
-                doc = Document.objects.get(id=doc_id)
+                reflect_doclinks(docs_by_id[doc_id], custom_field, value)
-                reflect_doclinks(doc, custom_field, value)
 
-    # For doc link fields that are being removed, remove symmetrical links
+    # For doc link fields being removed, remove symmetrical links.
+    # select_related here avoids resolving every affected document up front.
     for doclink_being_removed_instance in CustomFieldInstance.objects.filter(
         document_id__in=affected_docs,
         field__id__in=remove_custom_fields,
         field__data_type=CustomField.FieldDataType.DOCUMENTLINK,
         value_document_ids__isnull=False,
-    ):
+    ).select_related("field", "document"):
         for target_doc_id in doclink_being_removed_instance.value:
             remove_doclink(
-                document=Document.objects.get(
+                document=doclink_being_removed_instance.document,
-                    id=doclink_being_removed_instance.document.id,
-                ),
                 field=doclink_being_removed_instance.field,
                 target_doc_id=target_doc_id,
             )
@@ -1171,10 +1199,13 @@ def remove_doclink(
     """
     Removes a 'symmetrical' link to `document` from the target document's existing custom field instance
     """
-    target_doc_field_instance = CustomFieldInstance.objects.filter(
+    # select_related: a signal receiver (auditlog) touches .document/.field
-        document_id=target_doc_id,
+    # on save() below -- without this, that's a per-call reload query.
-        field=field,
+    target_doc_field_instance = (
-    ).first()
+        CustomFieldInstance.objects.filter(document_id=target_doc_id, field=field)
+        .select_related("document", "field")
+        .first()
+    )
     if (
         target_doc_field_instance is not None
         and document.id in target_doc_field_instance.value

src/documents/tests/test_bulk_edit.py

@@ -6,7 +6,9 @@ from unittest import mock
 import pikepdf
 from django.contrib.auth.models import Group
 from django.contrib.auth.models import User
+from django.db import connection
 from django.test import TestCase
+from django.test.utils import CaptureQueriesContext
 from guardian.shortcuts import assign_perm
 from guardian.shortcuts import get_groups_with_perms
 from guardian.shortcuts import get_users_with_perms
@@ -344,6 +346,202 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
         assert _cf_3 is not None
         self.assertNotIn(self.doc3.id, _cf_3.value)
 
+    def test_modify_custom_fields_batches_field_lookup(self) -> None:
+        """
+        GIVEN:
+            - Several documents are being bulk-edited to add several custom
+              fields at once
+        WHEN:
+            - modify_custom_fields runs
+        THEN:
+            - Each CustomField is resolved with one batched query total, not
+              once per (field, document) pair
+        """
+        docs = [
+            Document.objects.create(checksum=f"batch-{i}", title=f"batch-{i}")
+            for i in range(6)
+        ]
+        fields = [
+            CustomField.objects.create(
+                name=f"Batch Field {i}",
+                data_type=CustomField.FieldDataType.STRING,
+            )
+            for i in range(4)
+        ]
+
+        with CaptureQueriesContext(connection) as ctx:
+            bulk_edit.modify_custom_fields(
+                [doc.id for doc in docs],
+                add_custom_fields=[field.id for field in fields],
+                remove_custom_fields=[],
+            )
+
+        field_lookups = [
+            q
+            for q in ctx.captured_queries
+            if 'FROM "documents_customfield"' in q["sql"]
+        ]
+        self.assertEqual(
+            len(field_lookups),
+            1,
+            "Expected a single batched query to resolve the custom fields, "
+            f"got {len(field_lookups)}: {field_lookups}",
+        )
+
+        for doc in docs:
+            self.assertEqual(doc.custom_fields.count(), len(fields))
+
+    def test_modify_custom_fields_batches_document_lookup_for_documentlink(
+        self,
+    ) -> None:
+        """
+        GIVEN:
+            - Several documents are being bulk-edited to add a DOCUMENTLINK
+              custom field at once
+        WHEN:
+            - modify_custom_fields runs
+        THEN:
+            - The Document rows needed to reflect the symmetrical links are
+              resolved with one batched query total, not once per document
+        """
+        docs = [
+            Document.objects.create(checksum=f"link-{i}", title=f"link-{i}")
+            for i in range(6)
+        ]
+        target = Document.objects.create(checksum="link-target", title="link-target")
+        doclink_field = CustomField.objects.create(
+            name="Related",
+            data_type=CustomField.FieldDataType.DOCUMENTLINK,
+        )
+
+        with CaptureQueriesContext(connection) as ctx:
+            bulk_edit.modify_custom_fields(
+                [doc.id for doc in docs],
+                add_custom_fields={doclink_field.id: [target.id]},
+                remove_custom_fields=[],
+            )
+
+        single_document_lookups = [
+            q
+            for q in ctx.captured_queries
+            if 'FROM "documents_document"' in q["sql"]
+            and '"documents_document"."id" = ' in q["sql"]
+        ]
+        self.assertEqual(
+            len(single_document_lookups),
+            0,
+            "Expected document rows to come from a batched query, not "
+            f"per-document lookups, got: {single_document_lookups}",
+        )
+
+        for doc in docs:
+            self.assertEqual(
+                doc.custom_fields.get(field=doclink_field).value,
+                [target.id],
+            )
+
+    def test_modify_custom_fields_update_caches_document_and_field(self) -> None:
+        """
+        GIVEN:
+            - Several documents already have an instance of a custom field
+        WHEN:
+            - modify_custom_fields runs again for the same field, updating
+              the existing instances rather than creating new ones
+        THEN:
+            - No per-instance `.document`/`.field` reload query is issued
+              (e.g. by auditlog's post_save receiver touching them)
+        """
+        docs = [
+            Document.objects.create(checksum=f"update-{i}", title=f"update-{i}")
+            for i in range(6)
+        ]
+        field = CustomField.objects.create(
+            name="Update Field",
+            data_type=CustomField.FieldDataType.STRING,
+        )
+        bulk_edit.modify_custom_fields(
+            [doc.id for doc in docs],
+            add_custom_fields=[field.id],
+            remove_custom_fields=[],
+        )
+
+        with CaptureQueriesContext(connection) as ctx:
+            bulk_edit.modify_custom_fields(
+                [doc.id for doc in docs],
+                add_custom_fields={field.id: "updated value"},
+                remove_custom_fields=[],
+            )
+
+        single_row_reloads = [
+            q
+            for q in ctx.captured_queries
+            if ('FROM "documents_document"' in q["sql"] and '."id" = ' in q["sql"])
+            or ('FROM "documents_customfield"' in q["sql"] and '."id" = ' in q["sql"])
+        ]
+        self.assertEqual(
+            single_row_reloads,
+            [],
+            "Expected no per-instance document/field reload queries when "
+            f"updating existing custom field instances, got: {single_row_reloads}",
+        )
+
+        for doc in docs:
+            self.assertEqual(
+                doc.custom_fields.get(field=field).value,
+                "updated value",
+            )
+
+    def test_modify_custom_fields_removes_symmetrical_doclinks_batched(self) -> None:
+        """
+        GIVEN:
+            - Several source documents link to a shared target via a doc
+              link field
+        WHEN:
+            - The field is removed from all of them in one call
+        THEN:
+            - The symmetrical links are removed from the target
+            - No per-document lookup query is issued, on either side
+        """
+        target = Document.objects.create(checksum="rm-target", title="rm-target")
+        docs = [
+            Document.objects.create(checksum=f"rm-{i}", title=f"rm-{i}")
+            for i in range(6)
+        ]
+        field = CustomField.objects.create(
+            name="Related",
+            data_type=CustomField.FieldDataType.DOCUMENTLINK,
+        )
+        bulk_edit.modify_custom_fields(
+            [doc.id for doc in docs],
+            add_custom_fields={field.id: [target.id]},
+            remove_custom_fields=[],
+        )
+        self.assertEqual(
+            target.custom_fields.get(field=field).value,
+            [d.id for d in docs],
+        )
+
+        with CaptureQueriesContext(connection) as ctx:
+            bulk_edit.modify_custom_fields(
+                [doc.id for doc in docs],
+                add_custom_fields=[],
+                remove_custom_fields=[field.id],
+            )
+
+        single_document_lookups = [
+            q
+            for q in ctx.captured_queries
+            if 'FROM "documents_document"' in q["sql"]
+            and '"documents_document"."id" = ' in q["sql"]
+        ]
+        self.assertEqual(
+            single_document_lookups,
+            [],
+            "Expected batched document resolution, not per-document lookups, "
+            f"got: {single_document_lookups}",
+        )
+        self.assertEqual(target.custom_fields.get(field=field).value, [])
+
     def test_modify_custom_fields_doclink_self_link(self) -> None:
         """
         GIVEN: