mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-10-06 00:00:32 +00:00
Performance: deprecate and remove usage of all in API results (#12309)
This commit is contained in:
@@ -375,6 +375,26 @@ class DelayedQuery:
|
||||
]
|
||||
return self._manual_hits_cache
|
||||
|
||||
def get_result_ids(self) -> list[int]:
|
||||
"""
|
||||
Return all matching document IDs for the current query and ordering.
|
||||
"""
|
||||
if self._manual_sort_requested():
|
||||
return [hit["id"] for hit in self._manual_hits()]
|
||||
|
||||
q, mask, suggested_correction = self._get_query()
|
||||
self.suggested_correction = suggested_correction
|
||||
sortedby, reverse = self._get_query_sortedby()
|
||||
results = self.searcher.search(
|
||||
q,
|
||||
mask=mask,
|
||||
filter=MappedDocIdSet(self.filter_queryset, self.searcher.ixreader),
|
||||
limit=None,
|
||||
sortedby=sortedby,
|
||||
reverse=reverse,
|
||||
)
|
||||
return [hit["id"] for hit in results]
|
||||
|
||||
def __getitem__(self, item):
|
||||
if item.start in self.saved_results:
|
||||
return self.saved_results[item.start]
|
||||
|
||||
@@ -2602,13 +2602,25 @@ class ShareLinkBundleSerializer(OwnedObjectSerializer):
|
||||
|
||||
class BulkEditObjectsSerializer(SerializerWithPerms, SetPermissionsMixin):
|
||||
objects = serializers.ListField(
|
||||
required=True,
|
||||
allow_empty=False,
|
||||
required=False,
|
||||
allow_empty=True,
|
||||
label="Objects",
|
||||
write_only=True,
|
||||
child=serializers.IntegerField(),
|
||||
)
|
||||
|
||||
all = serializers.BooleanField(
|
||||
default=False,
|
||||
required=False,
|
||||
write_only=True,
|
||||
)
|
||||
|
||||
filters = serializers.DictField(
|
||||
required=False,
|
||||
allow_empty=True,
|
||||
write_only=True,
|
||||
)
|
||||
|
||||
object_type = serializers.ChoiceField(
|
||||
choices=[
|
||||
"tags",
|
||||
@@ -2681,10 +2693,20 @@ class BulkEditObjectsSerializer(SerializerWithPerms, SetPermissionsMixin):
|
||||
|
||||
def validate(self, attrs):
|
||||
object_type = attrs["object_type"]
|
||||
objects = attrs["objects"]
|
||||
objects = attrs.get("objects")
|
||||
apply_to_all = attrs.get("all", False)
|
||||
operation = attrs.get("operation")
|
||||
|
||||
self._validate_objects(objects, object_type)
|
||||
if apply_to_all:
|
||||
attrs.setdefault("objects", [])
|
||||
else:
|
||||
if objects is None:
|
||||
raise serializers.ValidationError(
|
||||
"objects is required unless all is true.",
|
||||
)
|
||||
if len(objects) == 0:
|
||||
raise serializers.ValidationError("objects must not be empty")
|
||||
self._validate_objects(objects, object_type)
|
||||
|
||||
if operation == "set_permissions":
|
||||
permissions = attrs.get("permissions")
|
||||
|
||||
@@ -1119,21 +1119,19 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
|
||||
[u1_doc1.id],
|
||||
)
|
||||
|
||||
def test_pagination_all(self) -> None:
|
||||
def test_pagination_results(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A set of 50 documents
|
||||
WHEN:
|
||||
- API request for document filtering
|
||||
THEN:
|
||||
- Results are paginated (25 items) and response["all"] returns all ids (50 items)
|
||||
- Results are paginated (25 items) and count reflects all results (50 items)
|
||||
"""
|
||||
t = Tag.objects.create(name="tag")
|
||||
docs = []
|
||||
for i in range(50):
|
||||
d = Document.objects.create(checksum=i, content=f"test{i}")
|
||||
d.tags.add(t)
|
||||
docs.append(d)
|
||||
|
||||
response = self.client.get(
|
||||
f"/api/documents/?tags__id__in={t.id}",
|
||||
@@ -1141,7 +1139,32 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
results = response.data["results"]
|
||||
self.assertEqual(len(results), 25)
|
||||
self.assertEqual(len(response.data["all"]), 50)
|
||||
self.assertEqual(response.data["count"], 50)
|
||||
self.assertNotIn("all", response.data)
|
||||
|
||||
def test_pagination_all_for_api_version_9(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A set of documents matching a filter
|
||||
WHEN:
|
||||
- API request uses legacy version 9
|
||||
THEN:
|
||||
- Response includes "all" for backward compatibility
|
||||
"""
|
||||
t = Tag.objects.create(name="tag")
|
||||
docs = []
|
||||
for i in range(4):
|
||||
d = Document.objects.create(checksum=i, content=f"test{i}")
|
||||
d.tags.add(t)
|
||||
docs.append(d)
|
||||
|
||||
response = self.client.get(
|
||||
f"/api/documents/?tags__id__in={t.id}",
|
||||
headers={"Accept": "application/json; version=9"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertIn("all", response.data)
|
||||
self.assertCountEqual(response.data["all"], [d.id for d in docs])
|
||||
|
||||
def test_list_with_include_selection_data(self) -> None:
|
||||
|
||||
@@ -145,6 +145,22 @@ class TestApiObjects(DirectoriesMixin, APITestCase):
|
||||
response.data["last_correspondence"],
|
||||
)
|
||||
|
||||
def test_paginated_objects_include_all_only_for_legacy_version(self) -> None:
|
||||
response_v10 = self.client.get("/api/correspondents/")
|
||||
self.assertEqual(response_v10.status_code, status.HTTP_200_OK)
|
||||
self.assertNotIn("all", response_v10.data)
|
||||
|
||||
response_v9 = self.client.get(
|
||||
"/api/correspondents/",
|
||||
headers={"Accept": "application/json; version=9"},
|
||||
)
|
||||
self.assertEqual(response_v9.status_code, status.HTTP_200_OK)
|
||||
self.assertIn("all", response_v9.data)
|
||||
self.assertCountEqual(
|
||||
response_v9.data["all"],
|
||||
[self.c1.id, self.c2.id, self.c3.id],
|
||||
)
|
||||
|
||||
|
||||
class TestApiStoragePaths(DirectoriesMixin, APITestCase):
|
||||
ENDPOINT = "/api/storage_paths/"
|
||||
@@ -794,6 +810,62 @@ class TestBulkEditObjects(APITestCase):
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertEqual(StoragePath.objects.count(), 0)
|
||||
|
||||
def test_bulk_objects_delete_all_filtered(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- Existing objects that can be filtered by name
|
||||
WHEN:
|
||||
- bulk_edit_objects API endpoint is called with all=true and filters
|
||||
THEN:
|
||||
- Matching objects are deleted without passing explicit IDs
|
||||
"""
|
||||
Correspondent.objects.create(name="c2")
|
||||
|
||||
response = self.client.post(
|
||||
"/api/bulk_edit_objects/",
|
||||
json.dumps(
|
||||
{
|
||||
"all": True,
|
||||
"filters": {"name__icontains": "c"},
|
||||
"object_type": "correspondents",
|
||||
"operation": "delete",
|
||||
},
|
||||
),
|
||||
content_type="application/json",
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertEqual(Correspondent.objects.count(), 0)
|
||||
|
||||
def test_bulk_objects_delete_all_filtered_tags_includes_descendants(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- Root tag with descendants
|
||||
WHEN:
|
||||
- bulk_edit_objects API endpoint is called with all=true
|
||||
THEN:
|
||||
- Root tags and descendants are deleted
|
||||
"""
|
||||
parent = Tag.objects.create(name="parent")
|
||||
child = Tag.objects.create(name="child", tn_parent=parent)
|
||||
|
||||
response = self.client.post(
|
||||
"/api/bulk_edit_objects/",
|
||||
json.dumps(
|
||||
{
|
||||
"all": True,
|
||||
"filters": {"is_root": True},
|
||||
"object_type": "tags",
|
||||
"operation": "delete",
|
||||
},
|
||||
),
|
||||
content_type="application/json",
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertFalse(Tag.objects.filter(id=parent.id).exists())
|
||||
self.assertFalse(Tag.objects.filter(id=child.id).exists())
|
||||
|
||||
def test_bulk_edit_object_permissions_insufficient_global_perms(self) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
@@ -881,3 +953,40 @@ class TestBulkEditObjects(APITestCase):
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
|
||||
self.assertEqual(response.content, b"Insufficient permissions")
|
||||
|
||||
def test_bulk_edit_all_filtered_permissions_insufficient_object_perms(
|
||||
self,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- Filter-matching objects include one that the user cannot edit
|
||||
WHEN:
|
||||
- bulk_edit_objects API endpoint is called with all=true
|
||||
THEN:
|
||||
- Operation applies only to editable objects
|
||||
"""
|
||||
self.t2.owner = User.objects.get(username="temp_admin")
|
||||
self.t2.save()
|
||||
|
||||
self.user1.user_permissions.add(
|
||||
*Permission.objects.filter(codename="delete_tag"),
|
||||
)
|
||||
self.user1.save()
|
||||
self.client.force_authenticate(user=self.user1)
|
||||
|
||||
response = self.client.post(
|
||||
"/api/bulk_edit_objects/",
|
||||
json.dumps(
|
||||
{
|
||||
"all": True,
|
||||
"filters": {"name__icontains": "t"},
|
||||
"object_type": "tags",
|
||||
"operation": "delete",
|
||||
},
|
||||
),
|
||||
content_type="application/json",
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertTrue(Tag.objects.filter(id=self.t2.id).exists())
|
||||
self.assertFalse(Tag.objects.filter(id=self.t1.id).exists())
|
||||
|
||||
@@ -68,26 +68,48 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase):
|
||||
results = response.data["results"]
|
||||
self.assertEqual(response.data["count"], 3)
|
||||
self.assertEqual(len(results), 3)
|
||||
self.assertCountEqual(response.data["all"], [d1.id, d2.id, d3.id])
|
||||
|
||||
response = self.client.get("/api/documents/?query=september")
|
||||
results = response.data["results"]
|
||||
self.assertEqual(response.data["count"], 1)
|
||||
self.assertEqual(len(results), 1)
|
||||
self.assertCountEqual(response.data["all"], [d3.id])
|
||||
self.assertEqual(results[0]["original_file_name"], "someepdf.pdf")
|
||||
|
||||
response = self.client.get("/api/documents/?query=statement")
|
||||
results = response.data["results"]
|
||||
self.assertEqual(response.data["count"], 2)
|
||||
self.assertEqual(len(results), 2)
|
||||
self.assertCountEqual(response.data["all"], [d2.id, d3.id])
|
||||
|
||||
response = self.client.get("/api/documents/?query=sfegdfg")
|
||||
results = response.data["results"]
|
||||
self.assertEqual(response.data["count"], 0)
|
||||
self.assertEqual(len(results), 0)
|
||||
self.assertCountEqual(response.data["all"], [])
|
||||
|
||||
def test_search_returns_all_for_api_version_9(self) -> None:
|
||||
d1 = Document.objects.create(
|
||||
title="invoice",
|
||||
content="bank payment",
|
||||
checksum="A",
|
||||
pk=1,
|
||||
)
|
||||
d2 = Document.objects.create(
|
||||
title="bank statement",
|
||||
content="bank transfer",
|
||||
checksum="B",
|
||||
pk=2,
|
||||
)
|
||||
with AsyncWriter(index.open_index()) as writer:
|
||||
index.update_document(writer, d1)
|
||||
index.update_document(writer, d2)
|
||||
|
||||
response = self.client.get(
|
||||
"/api/documents/?query=bank",
|
||||
headers={"Accept": "application/json; version=9"},
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertIn("all", response.data)
|
||||
self.assertCountEqual(response.data["all"], [d1.id, d2.id])
|
||||
|
||||
def test_search_with_include_selection_data(self) -> None:
|
||||
correspondent = Correspondent.objects.create(name="c1")
|
||||
|
||||
+47
-10
@@ -556,7 +556,9 @@ class TagViewSet(PermissionsAwareDocumentCountMixin, ModelViewSet):
|
||||
page = self.paginate_queryset(queryset)
|
||||
serializer = self.get_serializer(page, many=True)
|
||||
response = self.get_paginated_response(serializer.data)
|
||||
if descendant_pks:
|
||||
response.data["display_count"] = len(children_source)
|
||||
api_version = int(request.version or settings.REST_FRAMEWORK["DEFAULT_VERSION"])
|
||||
if descendant_pks and api_version < 10:
|
||||
# Include children in the "all" field, if needed
|
||||
response.data["all"] = [tag.pk for tag in children_source]
|
||||
return response
|
||||
@@ -2105,7 +2107,7 @@ class UnifiedSearchViewSet(DocumentViewSet):
|
||||
),
|
||||
),
|
||||
):
|
||||
result_ids = response.data.get("all", [])
|
||||
result_ids = queryset.get_result_ids()
|
||||
response.data["selection_data"] = (
|
||||
self._get_selection_data_for_queryset(
|
||||
Document.objects.filter(pk__in=result_ids),
|
||||
@@ -3919,20 +3921,55 @@ class BulkEditObjectsView(PassUserMixin):
|
||||
user = self.request.user
|
||||
object_type = serializer.validated_data.get("object_type")
|
||||
object_ids = serializer.validated_data.get("objects")
|
||||
apply_to_all = serializer.validated_data.get("all")
|
||||
object_class = serializer.get_object_class(object_type)
|
||||
operation = serializer.validated_data.get("operation")
|
||||
model_name = object_class._meta.model_name
|
||||
perm_codename = (
|
||||
f"change_{model_name}"
|
||||
if operation == "set_permissions"
|
||||
else f"delete_{model_name}"
|
||||
)
|
||||
|
||||
objs = object_class.objects.select_related("owner").filter(pk__in=object_ids)
|
||||
if apply_to_all:
|
||||
# Support all to avoid sending large lists of ids for bulk operations, with optional filters
|
||||
filters = serializer.validated_data.get("filters") or {}
|
||||
filterset_class = {
|
||||
"tags": TagFilterSet,
|
||||
"correspondents": CorrespondentFilterSet,
|
||||
"document_types": DocumentTypeFilterSet,
|
||||
"storage_paths": StoragePathFilterSet,
|
||||
}[object_type]
|
||||
user_permitted_objects = get_objects_for_user_owner_aware(
|
||||
user,
|
||||
perm_codename,
|
||||
object_class,
|
||||
)
|
||||
objs = filterset_class(
|
||||
data=filters,
|
||||
queryset=user_permitted_objects,
|
||||
).qs
|
||||
if object_type == "tags":
|
||||
editable_ids = set(user_permitted_objects.values_list("pk", flat=True))
|
||||
all_ids = set(objs.values_list("pk", flat=True))
|
||||
for tag in objs:
|
||||
all_ids.update(
|
||||
descendant.pk
|
||||
for descendant in tag.get_descendants()
|
||||
if descendant.pk in editable_ids
|
||||
)
|
||||
objs = object_class.objects.filter(pk__in=all_ids)
|
||||
objs = objs.select_related("owner")
|
||||
object_ids = list(objs.values_list("pk", flat=True))
|
||||
else:
|
||||
objs = object_class.objects.select_related("owner").filter(
|
||||
pk__in=object_ids,
|
||||
)
|
||||
|
||||
if not user.is_superuser:
|
||||
model_name = object_class._meta.model_name
|
||||
perm = (
|
||||
f"documents.change_{model_name}"
|
||||
if operation == "set_permissions"
|
||||
else f"documents.delete_{model_name}"
|
||||
)
|
||||
perm = f"documents.{perm_codename}"
|
||||
has_perms = user.has_perm(perm) and all(
|
||||
(obj.owner == user or obj.owner is None) for obj in objs
|
||||
has_perms_owner_aware(user, perm_codename, obj) for obj in objs
|
||||
)
|
||||
|
||||
if not has_perms:
|
||||
|
||||
Reference in New Issue
Block a user