diff --git a/src/documents/filters.py b/src/documents/filters.py index 00a47552db..54cec180f6 100644 --- a/src/documents/filters.py +++ b/src/documents/filters.py @@ -15,7 +15,6 @@ import dateparser from django.conf import settings from django.contrib.contenttypes.models import ContentType from django.db.models import Case -from django.db.models import CharField from django.db.models import Count from django.db.models import Exists from django.db.models import IntegerField @@ -208,36 +207,30 @@ class EffectiveContentFilter(Filter): @extend_schema_field(serializers.BooleanField) class SharedByUser(Filter): def filter(self, qs, value): + if value is None: + return qs + ctype = ContentType.objects.get_for_model(self.model) UserObjectPermission = get_user_obj_perms_model() GroupObjectPermission = get_group_obj_perms_model() - # see https://github.com/paperless-ngx/paperless-ngx/issues/5392, we limit subqueries - # to 1 because Postgres doesn't like returning > 1 row, but all we care about is > 0 - return ( - qs.filter( - owner_id=value, - ) - .annotate( - num_shared_users=Count( - UserObjectPermission.objects.filter( - content_type=ctype, - object_pk=Cast(OuterRef("pk"), CharField()), - ).values("user_id")[:1], - ), - ) - .annotate( - num_shared_groups=Count( - GroupObjectPermission.objects.filter( - content_type=ctype, - object_pk=Cast(OuterRef("pk"), CharField()), - ).values("group_id")[:1], - ), - ) - .filter( - Q(num_shared_users__gt=0) | Q(num_shared_groups__gt=0), - ) - if value is not None - else qs + # Guardian stores object_pk as a string. Each grant table is read once, + # as an uncorrelated set of integer keys, because probing it per + # document by string key has no usable index (none leads with + # object_pk) and MariaDB re-scans the table for every owned document. + # DISTINCT keeps Postgres hashing each set once: without it a large + # group grant table exceeds work_mem and is re-scanned for every row. + shared_with_users = ( + UserObjectPermission.objects.filter(content_type=ctype) + .values(shared_pk=Cast("object_pk", IntegerField())) + .distinct() + ) + shared_with_groups = ( + GroupObjectPermission.objects.filter(content_type=ctype) + .values(shared_pk=Cast("object_pk", IntegerField())) + .distinct() + ) + return qs.filter(owner_id=value).filter( + Q(pk__in=shared_with_users) | Q(pk__in=shared_with_groups), ) diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py index eeb1ab59a9..7ee2023162 100644 --- a/src/documents/tests/test_api_documents.py +++ b/src/documents/tests/test_api_documents.py @@ -1480,6 +1480,45 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase): [u1_doc1.id], ) + def test_shared_by_filter_matches_user_and_group_grants_once(self) -> None: + """ + GIVEN: + - Documents owned by a user: one shared with a user, one shared with + a group, one shared with several users and a group, one unshared + - A shared document owned by someone else + WHEN: + - Documents are filtered by shared_by__id for the owner + THEN: + - Only the owner's shared documents are returned, each exactly once + """ + owner = UserFactory(username="owner") + other = UserFactory(username="other") + reader1 = UserFactory(username="reader1") + reader2 = UserFactory(username="reader2") + group = Group.objects.create(name="readers") + grant_all_global(owner) + + shared_with_user = DocumentFactory.create(owner=owner) + shared_with_group = DocumentFactory.create(owner=owner) + shared_widely = DocumentFactory.create(owner=owner) + DocumentFactory.create(owner=owner) + others_shared = DocumentFactory.create(owner=other) + grant_object(reader1, shared_with_user, "view_document") + grant_object(group, shared_with_group, "view_document") + for target in (reader1, reader2, group): + grant_object(target, shared_widely, "view_document") + grant_object(reader1, others_shared, "view_document") + + self.client.force_authenticate(owner) + response = self.client.get(f"/api/documents/?shared_by__id={owner.id}") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 3) + self.assertCountEqual( + [result["id"] for result in response.data["results"]], + [shared_with_user.id, shared_with_group.id, shared_widely.id], + ) + def test_document_owned_and_group_shared_not_duplicated_when_filtering_by_tags( self, ) -> None: