Skip to content

Commit

Permalink
avoid unnecessary COUNT queries when exporting (#1715)
Browse files Browse the repository at this point in the history
* avoid unnecessary COUNT queries when exporting

* avoid unnecessary COUNT queries: different approach

* fix comment typos
  • Loading branch information
PetrDlouhy committed Dec 15, 2023
1 parent b63ddd6 commit bf18044
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 3 deletions.
2 changes: 1 addition & 1 deletion docs/changelog.rst
Expand Up @@ -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)
------------------
Expand Down
19 changes: 18 additions & 1 deletion import_export/admin.py
Expand Up @@ -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):
Expand Down
73 changes: 72 additions & 1 deletion tests/core/tests/test_admin_integration.py
Expand Up @@ -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 _
Expand Down Expand Up @@ -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")
Expand All @@ -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)
Expand Down

0 comments on commit bf18044

Please sign in to comment.