Skip to content

Commit

Permalink
avoid unnecessary COUNT queries: different approach
Browse files Browse the repository at this point in the history
  • Loading branch information
PetrDlouhy committed Dec 15, 2023
1 parent b4a4971 commit 3678676
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 27 deletions.
41 changes: 17 additions & 24 deletions import_export/admin.py
@@ -1,6 +1,5 @@
import logging
import warnings
from contextlib import contextmanager

import django
from django import forms
Expand Down Expand Up @@ -70,24 +69,6 @@ def changelist_view(self, request, extra_context=None):
return super().changelist_view(request, extra_context)


class FakePaginator:
count = 0


def _get_paginator(request, queryset, per_page):
return FakePaginator()


@contextmanager
def temp_attr(obj, attr_name, new_value):
original_value = getattr(obj, attr_name)
setattr(obj, attr_name, new_value)
try:
yield
finally:
setattr(obj, attr_name, original_value)


class ImportMixin(BaseImportMixin, ImportExportMixinBase):
"""
Import mixin.
Expand Down Expand Up @@ -761,12 +742,24 @@ def get_export_queryset(self, request):
if django.VERSION >= (4, 0):
changelist_kwargs["search_help_text"] = self.search_help_text

# Temporarily set to False to avoid unnecessary COUNT queries.
with temp_attr(self, "show_full_result_count", False):
# Temporarily set to FakePaginator to avoid unnecessary COUNT queries.
with temp_attr(self, "get_paginator", _get_paginator):
cl = ChangeList(**changelist_kwargs)
class ExportChangeList(ChangeList):
def get_results(self, request):
"""
We override this method because we only call ChangeList.get_queryset()
so we don't need anything from this method.
The get_results() gets called during ChangeList.__init__()
and we do want to avoid unnecessary COUNT queries.
"""
pass

cl = ExportChangeList(**changelist_kwargs)

# get_queryset() is already called during initialization,
# it is enough to get it's 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
74 changes: 71 additions & 3 deletions 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,9 +683,8 @@ def test_export(self):
"file_format": "0",
}
date_str = datetime.now().strftime("%Y-%m-%d")
with self.assertNumQueries(
7
): # Should not contain COUNT queries from ModelAdmin.get_results()
# 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"))
Expand All @@ -697,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 diring 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 3678676

Please sign in to comment.