From 778254f03efc49c7b1d2f8ebcc86804f584af1b6 Mon Sep 17 00:00:00 2001 From: Fabian Braun Date: Sun, 14 Jul 2024 14:08:12 +0200 Subject: [PATCH] feat: Optimize db evaluation (#416) * Cache `page_content` in toolbar * Avoid repeated db hits * Fix signature evaluation * Avoid double asignment * Add test for number of queries! * Fix ruff issue --- djangocms_versioning/cms_toolbars.py | 18 +++++++++++++++--- djangocms_versioning/helpers.py | 19 ++++++++++--------- djangocms_versioning/indicators.py | 21 +++++++++++---------- tests/test_indicators.py | 10 ++++++++++ 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/djangocms_versioning/cms_toolbars.py b/djangocms_versioning/cms_toolbars.py index 007cac53..f7da4b5c 100644 --- a/djangocms_versioning/cms_toolbars.py +++ b/djangocms_versioning/cms_toolbars.py @@ -1,5 +1,6 @@ from collections import OrderedDict from copy import copy +from typing import Optional from cms.cms_toolbars import ( ADD_PAGE_LANGUAGE_BREAK, @@ -288,18 +289,29 @@ class VersioningPageToolbar(PageToolbar): Overriding the original Page toolbar to ensure that draft and published pages can be accessed and to allow full control over the Page toolbar for versioned pages. """ - def get_page_content(self, language=None): + + def __init__(self, *args, **kwargs): + self.page_content: Optional[PageContent] = None + super().__init__(*args, **kwargs) + + def get_page_content(self, language: Optional[str] = None) -> PageContent: if not language: language = self.current_lang + if self.page_content and self.page_content.language == language: + # Already known - no need to query it again + return self.page_content toolbar_obj = self.toolbar.get_object() if toolbar_obj and toolbar_obj.language == language: + # Already in the toolbar, then use it! return self.toolbar.get_object() - return get_latest_admin_viewable_content(self.page, language=language) + else: + # Get it from the DB + return get_latest_admin_viewable_content(self.page, language=language) def populate(self): self.page = self.request.current_page - self.title = self.get_page_content() if self.page else None + self.page_content = self.get_page_content() if self.page else None self.permissions_activated = get_cms_setting("PERMISSION") self.override_language_menu() diff --git a/djangocms_versioning/helpers.py b/djangocms_versioning/helpers.py index 76636e14..19abd78d 100644 --- a/djangocms_versioning/helpers.py +++ b/djangocms_versioning/helpers.py @@ -304,7 +304,7 @@ def remove_published_where(queryset): def get_latest_admin_viewable_content( - grouper: type, + grouper: models.Model, include_unpublished_archived: bool = False, **extra_grouping_fields, ) -> models.Model: @@ -425,15 +425,16 @@ def send_email( def get_latest_draft_version(version): - """Get latest draft version of version object + """Get latest draft version of version object and caches it """ from djangocms_versioning.constants import DRAFT from djangocms_versioning.models import Version - drafts = ( - Version.objects - .filter_by_content_grouping_values(version.content) - .filter(state=DRAFT) - ) - - return drafts.first() + if not hasattr(version, "_latest_draft_version"): + drafts = ( + Version.objects + .filter_by_content_grouping_values(version.content) + .filter(state=DRAFT) + ) + version._latest_draft_version = drafts.first() + return version._latest_draft_version diff --git a/djangocms_versioning/indicators.py b/djangocms_versioning/indicators.py index 0b625d63..7424d97c 100644 --- a/djangocms_versioning/indicators.py +++ b/djangocms_versioning/indicators.py @@ -97,25 +97,26 @@ def content_indicator(content_obj): versions = Version.objects.filter_by_content_grouping_values( content_obj ).order_by("-pk") + version_states = dict(VERSION_STATES) signature = { - state: versions.filter(state=state) - for state, name in VERSION_STATES + version.state: version + for version in versions if version.state in version_states } - if signature[DRAFT] and not signature[PUBLISHED]: + if DRAFT in signature and PUBLISHED not in signature: content_obj._indicator_status = "draft" - content_obj._version = signature[DRAFT] - elif signature[DRAFT] and signature[PUBLISHED]: + content_obj._version = signature[DRAFT], + elif DRAFT in signature and PUBLISHED in signature: content_obj._indicator_status = "dirty" - content_obj._version = (signature[DRAFT][0], signature[PUBLISHED][0]) - elif signature[PUBLISHED]: + content_obj._version = (signature[DRAFT], signature[PUBLISHED]) + elif PUBLISHED in signature: content_obj._indicator_status = "published" - content_obj._version = signature[PUBLISHED] + content_obj._version = signature[PUBLISHED], elif versions[0].state == UNPUBLISHED: content_obj._indicator_status = "unpublished" - content_obj._version = signature[UNPUBLISHED] + content_obj._version = signature[UNPUBLISHED], elif versions[0].state == ARCHIVED: content_obj._indicator_status = "archived" - content_obj._version = signature[ARCHIVED] + content_obj._version = signature[ARCHIVED], else: # pragma: no cover content_obj._indicator_status = None content_obj._version = [None] diff --git a/tests/test_indicators.py b/tests/test_indicators.py index dabce266..551e3316 100644 --- a/tests/test_indicators.py +++ b/tests/test_indicators.py @@ -219,3 +219,13 @@ def test_mixin_factory(self): self.assertContains(response, "cms.pagetree.css"), # JS loadeD? self.assertContains(response, "indicators.js") + + def test_page_indicator_db_queries(self): + """Only one query should be executed to get the indicator""" + version = PageVersionFactory( + content__language="en", + ) + with self.assertNumQueries(1): + from djangocms_versioning.indicators import content_indicator + + content_indicator(version.content)