fix: N+1 query on PageListView

This commit is contained in:
imperosol
2025-09-22 18:11:02 +02:00
parent 34b0dc3302
commit acb6c6ce9c
3 changed files with 63 additions and 15 deletions

View File

@@ -1197,6 +1197,18 @@ class NotLocked(LockError):
pass pass
class PageQuerySet(models.QuerySet):
def viewable_by(self, user: User) -> Self:
if user.is_anonymous:
return self.filter(view_groups=settings.SITH_GROUP_PUBLIC_ID)
if user.has_perm("core.view_page"):
return self.all()
groups_ids = [g.id for g in user.cached_groups]
if user.is_subscribed:
groups_ids.append(settings.SITH_GROUP_SUBSCRIBERS_ID)
return self.filter(view_groups__in=groups_ids)
# This function prevents generating migration upon settings change # This function prevents generating migration upon settings change
def get_default_owner_group(): def get_default_owner_group():
return settings.SITH_GROUP_ROOT_ID return settings.SITH_GROUP_ROOT_ID
@@ -1266,6 +1278,8 @@ class Page(models.Model):
_("lock_timeout"), null=True, blank=True, default=None _("lock_timeout"), null=True, blank=True, default=None
) )
objects = PageQuerySet.as_manager()
class Meta: class Meta:
unique_together = ("name", "parent") unique_together = ("name", "parent")
permissions = ( permissions = (

View File

@@ -1,11 +1,14 @@
import pytest import pytest
from django.conf import settings
from django.contrib.auth.models import Permission
from django.test import Client from django.test import Client
from django.urls import reverse from django.urls import reverse
from model_bakery import baker from model_bakery import baker
from pytest_django.asserts import assertRedirects from pytest_django.asserts import assertRedirects
from core.baker_recipes import board_user from core.baker_recipes import board_user, subscriber_user
from core.models import Page from core.models import AnonymousUser, Page, User
from sith.settings import SITH_GROUP_OLD_SUBSCRIBERS_ID, SITH_GROUP_SUBSCRIBERS_ID
@pytest.mark.django_db @pytest.mark.django_db
@@ -24,3 +27,32 @@ def test_edit_page(client: Client):
assertRedirects(res, reverse("core:page", kwargs={"page_name": page._full_name})) assertRedirects(res, reverse("core:page", kwargs={"page_name": page._full_name}))
revision = page.revisions.last() revision = page.revisions.last()
assert revision.content == "Hello World" assert revision.content == "Hello World"
@pytest.mark.django_db
def test_viewable_by():
# remove existing pages to prevent side effect
Page.objects.all().delete()
view_groups = [
[settings.SITH_GROUP_PUBLIC_ID],
[settings.SITH_GROUP_PUBLIC_ID, SITH_GROUP_SUBSCRIBERS_ID],
[SITH_GROUP_SUBSCRIBERS_ID],
[SITH_GROUP_SUBSCRIBERS_ID, SITH_GROUP_OLD_SUBSCRIBERS_ID],
[],
]
pages = baker.make(Page, _quantity=len(view_groups), _bulk_create=True)
for page, groups in zip(pages, view_groups, strict=True):
page.view_groups.set(groups)
viewable = Page.objects.viewable_by(AnonymousUser()).values_list("id", flat=True)
assert set(viewable) == {pages[0].id, pages[1].id}
subscriber = subscriber_user.make()
viewable = Page.objects.viewable_by(subscriber).values_list("id", flat=True)
assert set(viewable) == {p.id for p in pages[0:4]}
root_user = baker.make(
User, user_permissions=[Permission.objects.get(codename="view_page")]
)
viewable = Page.objects.viewable_by(root_user).values_list("id", flat=True)
assert set(viewable) == {p.id for p in pages}

View File

@@ -43,23 +43,25 @@ class CanEditPagePropMixin(CanEditPropMixin):
return res return res
class PageListView(CanViewMixin, ListView): class PageListView(ListView):
model = Page model = Page
template_name = "core/page_list.jinja" template_name = "core/page_list.jinja"
queryset = (
Page.objects.annotate( def get_queryset(self):
display_name=Coalesce( return (
Subquery( Page.objects.viewable_by(self.request.user)
PageRev.objects.filter(page=OuterRef("id")) .annotate(
.order_by("-date") display_name=Coalesce(
.values("title")[:1] Subquery(
), PageRev.objects.filter(page=OuterRef("id"))
F("name"), .order_by("-date")
.values("title")[:1]
),
F("name"),
)
) )
.select_related("parent")
) )
.prefetch_related("view_groups")
.select_related("parent")
)
class PageView(CanViewMixin, DetailView): class PageView(CanViewMixin, DetailView):