diff --git a/docs/changelog.rst b/docs/changelog.rst index cd66939ca..f0d500589 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,7 +5,7 @@ Changelog ------------------ - Nothing changed yet. - +- Remove unnecessary ChangeList queries to speed up export via Admin UI (#1715) 3.3.4 (2023-12-09) ------------------ diff --git a/import_export/admin.py b/import_export/admin.py index 735f91cde..3474c7a45 100644 --- a/import_export/admin.py +++ b/import_export/admin.py @@ -741,8 +741,25 @@ def get_export_queryset(self, request): changelist_kwargs["sortable_by"] = self.sortable_by if django.VERSION >= (4, 0): changelist_kwargs["search_help_text"] = self.search_help_text - cl = ChangeList(**changelist_kwargs) + class ExportChangeList(ChangeList): + def get_results(self, request): + """ + Overrides ChangeList.get_results() to bypass default operations like + pagination and result counting, which are not needed for export. This + prevents executing unnecessary COUNT queries during ChangeList + initialization. + """ + pass + + cl = ExportChangeList(**changelist_kwargs) + + # get_queryset() is already called during initialization, + # it is enough to get its results + if hasattr(cl, "queryset"): + return cl.queryset + + # Fallback in case the ChangeList doesn't have queryset attribute set return cl.get_queryset(request) def get_export_data(self, file_format, queryset, *args, **kwargs): diff --git a/tests/core/tests/test_admin_integration.py b/tests/core/tests/test_admin_integration.py index fcd39ba58..b45b42955 100644 --- a/tests/core/tests/test_admin_integration.py +++ b/tests/core/tests/test_admin_integration.py @@ -11,9 +11,12 @@ from core.admin import AuthorAdmin, BookAdmin, CustomBookAdmin, ImportMixin from core.models import Author, Book, Category, EBook, Parent from django.contrib.admin.models import DELETION, LogEntry +from django.contrib.admin.sites import AdminSite +from django.contrib.admin.views.main import ChangeList from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied from django.http import HttpRequest +from django.test import RequestFactory from django.test.testcases import TestCase, TransactionTestCase from django.test.utils import override_settings from django.utils.translation import gettext_lazy as _ @@ -680,7 +683,9 @@ def test_export(self): "file_format": "0", } date_str = datetime.now().strftime("%Y-%m-%d") - response = self.client.post("/admin/core/book/export/", data) + # Should not contain COUNT queries from ModelAdmin.get_results() + with self.assertNumQueries(5): + response = self.client.post("/admin/core/book/export/", data) self.assertEqual(response.status_code, 200) self.assertTrue(response.has_header("Content-Disposition")) self.assertEqual(response["Content-Type"], "text/csv") @@ -694,6 +699,72 @@ def test_export(self): response.content, ) + def test_get_export_queryset(self): + model_admin = BookAdmin(Book, AdminSite()) + + factory = RequestFactory() + request = factory.get("/admin/core/book/export/") + request.user = User.objects.create_user("admin1") + + call_number = 0 + + class MyChangeList(ChangeList): + def get_queryset(self, request): + nonlocal call_number + call_number += 1 + return super().get_queryset(request) + + model_admin.get_changelist = lambda request: MyChangeList + + with patch.object(model_admin, "get_paginator") as mock_get_paginator: + with self.assertNumQueries(4): + queryset = model_admin.get_export_queryset(request) + + mock_get_paginator.assert_not_called() + self.assertEqual(call_number, 1) + + self.assertEqual(queryset.count(), Book.objects.count()) + + def test_get_export_queryset_no_queryset_init(self): + """Test if user has own ChangeList which doesn't store queryset during init""" + model_admin = BookAdmin(Book, AdminSite()) + + factory = RequestFactory() + request = factory.get("/admin/core/book/export/") + request.user = User.objects.create_user("admin1") + + call_number = 0 + + class MyChangeList(ChangeList): + def __init__(self, *args, **kwargs): + self.filter_params = {} + self.model_admin = kwargs.pop("model_admin") + self.list_filter = kwargs.pop("list_filter") + self.model = kwargs.pop("model") + self.date_hierarchy = kwargs.pop("date_hierarchy") + self.root_queryset = self.model_admin.get_queryset(request) + self.list_select_related = kwargs.pop("list_select_related") + self.list_display = kwargs.pop("list_display") + self.lookup_opts = self.model._meta + self.params = {} + self.query = "" + + def get_queryset(self, request): + nonlocal call_number + call_number += 1 + return super().get_queryset(request) + + model_admin.get_changelist = lambda request: MyChangeList + + with patch.object(model_admin, "get_paginator") as mock_get_paginator: + with self.assertNumQueries(4): + queryset = model_admin.get_export_queryset(request) + + mock_get_paginator.assert_not_called() + self.assertEqual(call_number, 1) + + self.assertEqual(queryset.count(), Book.objects.count()) + def test_export_second_resource(self): response = self.client.get("/admin/core/book/export/") self.assertEqual(response.status_code, 200)