From 361546baa2fae8b29ce8394f1df3b9db3f843d3b Mon Sep 17 00:00:00 2001
From: "jindra12.underdark" <jindra12.underdark@gmail.com>
Date: Tue, 18 Jul 2023 21:21:54 +0200
Subject: [PATCH] Optimize paginator usage

#210
---
 district/models.py | 32 +++++++++++++--------
 main/models.py     | 39 ++++++++++++++++++--------
 shared/models.py   | 69 ++++++++++++++++++++++++----------------------
 uniweb/models.py   |  8 ++++--
 4 files changed, 89 insertions(+), 59 deletions(-)

diff --git a/district/models.py b/district/models.py
index afb5594f..0aec9c91 100644
--- a/district/models.py
+++ b/district/models.py
@@ -541,10 +541,14 @@ class DistrictArticlesPage(
 
     def get_context(self, request):
         context = super().get_context(request)
-        context["articles"] = Paginator(
-            self.append_all_shared_articles(DistrictArticlePage.objects.child_of(self)),
-            self.max_items,
-        ).get_page(request.GET.get("page"))
+        context["articles"] = self.materialize_page_object_content(
+            Paginator(
+                self.append_all_shared_articles_query(
+                    DistrictArticlePage.objects.child_of(self)
+                ),
+                self.max_items,
+            ).get_page(request.GET.get("page"))
+        )
         return context
 
     @route(r"^tagy/$", name="tags")
@@ -591,26 +595,30 @@ class DistrictArticlesPage(
 
         try:
             tag = Tag.objects.filter(slug=request.GET["tag"])[0]
-            article_page_qs = self.append_all_shared_articles(
+            article_page_qs = self.append_all_shared_articles_query(
                 DistrictArticlePage.objects.filter(
                     page_ptr_id__in=page_ids, tags__slug=tag.slug
                 ),
-                filter=lambda shared: shared.filter(
+                custom_article_query=lambda shared: shared.filter(
                     page_ptr_id__in=page_ids, tags__slug=tag.slug
                 ),
             )
         except (KeyError, IndexError):
             tag = None
-            article_page_qs = self.append_all_shared_articles(
+            article_page_qs = self.append_all_shared_articles_query(
                 DistrictArticlePage.objects.filter(page_ptr_id__in=page_ids),
-                filter=lambda shared: shared.filter(page_ptr_id__in=page_ids),
+                custom_article_query=lambda shared: shared.filter(
+                    page_ptr_id__in=page_ids
+                ),
             )
 
         return {
-            "article_page_list": Paginator(
-                article_page_qs,
-                self.max_items,
-            ).get_page(request.GET.get("page")),
+            "article_page_list": self.materialize_page_object_content(
+                Paginator(
+                    article_page_qs,
+                    self.max_items,
+                ).get_page(request.GET.get("page"))
+            ),
             "tag": tag,
         }
 
diff --git a/main/models.py b/main/models.py
index 4f85221b..f4a97e6b 100644
--- a/main/models.py
+++ b/main/models.py
@@ -58,6 +58,7 @@ class MainHomePage(
     ExtendedMetadataHomePageMixin,
     MetadataPageMixin,
     InstagramMixin,
+    ArticlesMixin,
     Page,
 ):
     # header
@@ -153,6 +154,7 @@ class MainHomePage(
 
     content_panels = Page.content_panels + [
         FieldPanel("content"),
+        *ArticleMixin.content_panels,
         FieldPanel("footer_other_links"),
         FieldPanel("footer_person_list"),
     ]
@@ -245,15 +247,21 @@ class MainHomePage(
         return context
 
     def get_region_response(self, request):
+        context = {}
         if request.GET.get("region", None) == "VSK":
             sorted_article_qs = MainArticlePage.objects.filter(
                 region__isnull=False
             ).order_by("-date")
+            context = {"article_data_list": sorted_article_qs[:3]}
         else:
-            sorted_article_qs = MainArticlePage.objects.filter(
-                region=request.GET.get("region", None)
-            ).order_by("-date")
-        context = {"article_data_list": sorted_article_qs[:3]}
+            sorted_article_qs = self.append_all_shared_articles_query(
+                MainArticlePage.objects.filter(region=request.GET.get("region", None))
+            )
+            context = {
+                "article_data_list": self.materialize_shared_articles_query(
+                    sorted_article_qs[:3]
+                )
+            }
         data = {
             "html": render(
                 request, "main/includes/small_article_preview.html", context
@@ -494,12 +502,16 @@ class MainArticlesPage(
 
     def get_articles_response(self, request):
         article_paginator = Paginator(
-            MainArticlePage.objects.filter(article_type=ARTICLE_TYPES.PRESS_RELEASE)
-            .order_by("-date", "title")
-            .live(),
+            self.append_all_shared_articles_query(
+                MainArticlePage.objects.filter(
+                    article_type=ARTICLE_TYPES.PRESS_RELEASE
+                ),
+            ).order_by("union_date", "union_title"),
             10,
         )
-        article_page = article_paginator.get_page(request.GET.get("page", 1))
+        article_page = self.materialize_page_object_content(
+            article_paginator.get_page(request.GET.get("page", 1))
+        )
         context = {"article_data_list": article_page.object_list}
         html_content = render(
             request, "main/includes/person_article_preview.html", context
@@ -512,12 +524,17 @@ class MainArticlesPage(
 
     def get_all_articles_search_response(self, request):
         article_paginator = Paginator(
-            self.append_all_shared_articles(MainArticlePage.objects).search(
-                request.GET["q"]
+            self.append_all_shared_articles_query(
+                MainArticlePage.objects.search(
+                    request.GET["q"],
+                    custom_article_query=lambda query: query.search(request.GET["q"]),
+                )
             ),
             10,
         )
-        article_page = article_paginator.get_page(request.GET.get("page", 1))
+        article_page = self.materialize_page_object_content(
+            article_paginator.get_page(request.GET.get("page", 1))
+        )
         context = {"article_data_list": article_page.object_list}
         html_content = render(
             request, "main/includes/person_article_preview.html", context
diff --git a/shared/models.py b/shared/models.py
index 68dc2e30..7a29cb4d 100644
--- a/shared/models.py
+++ b/shared/models.py
@@ -3,11 +3,12 @@ from enum import Enum
 from functools import reduce
 
 from django.apps import apps
+from django.core.paginator import Page as PaginatorPage
 from django.db import models
 from django.db.models.expressions import F, Value
 from django.utils import timezone
 from modelcluster.fields import ParentalKey, ParentalManyToManyField
-from taggit.models import ItemBase, Tag, TagBase
+from taggit.models import ItemBase, TagBase
 from wagtail.admin.panels import FieldPanel, MultiFieldPanel, PublishingPanel
 from wagtail.fields import StreamField
 from wagtail.models import Page
@@ -308,7 +309,7 @@ class ArticlesMixin(models.Model):
         elif self._meta.app_label == "main":
             return SharedArticlesPageType.MAIN
 
-    def unique_page_query_materialized(self, results):
+    def evaluate_page_query(self, results):
         return list(
             reduce(
                 lambda unique, item: unique | {f"{item['union_page_ptr_id']}": item},
@@ -318,7 +319,9 @@ class ArticlesMixin(models.Model):
         )
 
     def append_all_shared_articles_query(
-        self, previous_query: models.QuerySet | None = None, filter=None
+        self,
+        previous_query: models.QuerySet | None = None,
+        custom_article_query=None,
     ):
         """
         To prevent circular deps, we get class models during runtime
@@ -385,7 +388,9 @@ class ArticlesMixin(models.Model):
         main_article_query: models.QuerySet = MainArticlePage.objects
 
         apply_additional_filter = (
-            lambda query: filter(query) if filter is not None else query
+            lambda query: custom_article_query(query)
+            if custom_article_query is not None
+            else query
         )
 
         create_query_by_slug = lambda query: apply_additional_filter(
@@ -401,16 +406,16 @@ class ArticlesMixin(models.Model):
         )
 
         district_by_slug = create_query_by_slug(district_article_query)
-        if filter is not None:
-            district_by_slug = filter(district_by_slug)
+        if custom_article_query is not None:
+            district_by_slug = custom_article_query(district_by_slug)
 
         uniweb_by_slug = create_query_by_slug(uniweb_article_query)
-        if filter is not None:
-            uniweb_by_slug = filter(uniweb_by_slug)
+        if custom_article_query is not None:
+            uniweb_by_slug = custom_article_query(uniweb_by_slug)
 
         main_by_slug = create_query_by_slug(main_article_query)
-        if filter is not None:
-            main_by_slug = filter(main_by_slug)
+        if custom_article_query is not None:
+            main_by_slug = custom_article_query(main_by_slug)
 
         shared_field = Value(
             self.page_ptr.id,
@@ -478,9 +483,7 @@ class ArticlesMixin(models.Model):
 
         return results.order_by("union_date")
 
-    def append_all_shared_articles(
-        self, previous_query: models.QuerySet | None = None, filter=None
-    ):
+    def materialize_shared_articles_query(self, results):
         """
         To prevent circular deps, we get class models during runtime
         """
@@ -494,16 +497,14 @@ class ArticlesMixin(models.Model):
         district_meta_fields = DistrictArticlePage._meta.fields
         uniweb_meta_fields = UniwebArticlePage._meta.fields
 
-        results = self.append_all_shared_articles_query(previous_query, filter)
-
-        evaluated = self.unique_page_query_materialized(
-            results
-        )  # We MUST eval here since we can't turn values() into concrete class instances in QuerySet after union
-
         assign_to_model = lambda unioned: lambda assignment, field: assignment | {
             field.column: unioned[f"union_{field.column}"]
         }
 
+        evaluated = self.evaluate_page_query(
+            results
+        )  # We MUST eval here since we can't turn values() into concrete class instances in QuerySet after union
+
         if page_type == SharedArticlesPageType.DISTRICT:
             return list(
                 map(
@@ -534,12 +535,23 @@ class ArticlesMixin(models.Model):
                 )
             )
 
+    def append_all_shared_articles(
+        self, previous_query: models.QuerySet | None = None, custom_article_query=None
+    ):
+        return self.materialize_shared_articles_query(
+            self.append_all_shared_articles_query(previous_query, custom_article_query)
+        )
+
     def get_article_page_by_slug(self, slug: str):
         articles = self.append_all_shared_articles(
-            filter=lambda query: query.filter(slug=slug)
+            custom_article_query=lambda query: query.filter(slug=slug)
         )
         return articles[0]
 
+    def materialize_page_object_content(self, page: PaginatorPage):
+        page.object_list = self.materialize_page_object_content(page.object_list)
+        return page
+
     def setup_article_page_context(self, request):
         slug = request.GET.get("sdilene", "")
         return self.get_article_page_by_slug(slug).serve(request)
@@ -547,7 +559,6 @@ class ArticlesMixin(models.Model):
     def search_tags_by_unioned_id_query(
         self,
         articles: list,
-        additional_query=None,
         tags_model_query=None,
     ):
         DistrictArticleTag = apps.get_model(app_label="district.DistrictArticleTag")
@@ -568,27 +579,19 @@ class ArticlesMixin(models.Model):
             content_object_id__in=get_ids_by_page_type(
                 SharedArticlesPageType.DISTRICT.value
             )
-        )
+        ).select_related("tag_id")
         uniweb_tags = UniwebArticleTag.objects.filter(
             content_object_id__in=get_ids_by_page_type(
                 SharedArticlesPageType.UNIWEB.value
             )
-        )
+        ).select_related("tag_id")
         main_tags = MainArticleTag.objects.filter(
             content_object_id__in=get_ids_by_page_type(
                 SharedArticlesPageType.MAIN.value
             )
-        )
-
-        if additional_query is not None:
-            district_tags = additional_query(district_tags)
-            uniweb_tags = additional_query(uniweb_tags)
-            main_tags = additional_query(main_tags)
-
-        union = district_tags.union(uniweb_tags).union(main_tags)
+        ).select_related("tag_id")
 
-        union = union.values_list("tag_id", flat=True)
-        tag_query = Tag.objects.filter(id__in=union)
+        tag_query = district_tags.union(uniweb_tags).union(main_tags)
 
         if tags_model_query is not None:
             tag_query = tags_model_query(tag_query)
diff --git a/uniweb/models.py b/uniweb/models.py
index a30db27c..b9f553b2 100644
--- a/uniweb/models.py
+++ b/uniweb/models.py
@@ -552,9 +552,9 @@ class UniwebArticlesIndexPage(
 
         own_children = UniwebArticlePage.objects.child_of(self)
 
-        articles = self.append_all_shared_articles(
+        articles = self.append_all_shared_articles_query(
             own_children,
-            filter=lambda articles: articles.filter(**tag_params)
+            custom_article_query=lambda articles: articles.filter(**tag_params)
             if tag is not None
             else articles,
         )
@@ -566,7 +566,9 @@ class UniwebArticlesIndexPage(
             else articles,
         )
 
-        context["articles"] = Paginator(articles, ARTICLES_PER_PAGE).get_page(num)
+        context["articles"] = self.materialize_page_object_content(
+            Paginator(articles, ARTICLES_PER_PAGE).get_page(num)
+        )
         context["tags"] = self.search_tags_by_unioned_id_query(articles_ids)
         context["active_tag"] = tag
         return context
-- 
GitLab