feat: Pre-populate version.content cache when getting version object (#446)

* feat: Pre-populate `version.content` cache when getting version for content

* Update djangocms_versioning/models.py

* Bump python to 3.12 for tests against latest Django

* Use reverse cache

* add page prefetch for view published button

* remove lru_cache from toolbar (memory leak)

* remove unused import

* Undo unnecessary changes for outdated django CMS versions

* Sort imports

* fix: Language menu created twice

* fix lint issues

* Update djangocms_versioning/cms_toolbars.py

Author: fsbraun

.github/workflows/test.yml

@@ -149,7 +149,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.11']
+        python-version: ['3.12']
         requirements-file: ['dj51_cms41.txt']
         cms-version: [
           'https://github.com/django-cms/django-cms/archive/develop-4.tar.gz'
@@ -183,7 +183,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: [ "3.11" ]
+        python-version: [ "3.12" ]
         cms-version: [
           'https://github.com/django-cms/django-cms/archive/develop-4.tar.gz'
         ]

djangocms_versioning/cms_config.py

@@ -1,5 +1,6 @@
 import collections
 
+from cms import __version__ as cms_version
 from cms.app_base import CMSAppConfig, CMSAppExtension
 from cms.extensions.models import BaseExtension
 from cms.models import PageContent, Placeholder
@@ -22,6 +23,7 @@
 from django.utils.encoding import force_str
 from django.utils.functional import cached_property
 from django.utils.translation import gettext_lazy as _
+from packaging.version import Version as PackageVersion
 
 from . import indicators
 from .admin import VersioningAdminMixin
@@ -393,6 +395,7 @@ class VersioningCMSConfig(CMSAppConfig):
             content_admin_mixin=VersioningCMSPageAdminMixin,
         )
     ]
-    cms_toolbar_mixin = CMSToolbarVersioningMixin
+    if PackageVersion(cms_version) < PackageVersion("4.2"):
+        cms_toolbar_mixin = CMSToolbarVersioningMixin
     PageContent.add_to_class("is_editable", is_editable)
     PageContent.add_to_class("content_indicator", indicators.content_indicator)

djangocms_versioning/cms_toolbars.py

@@ -6,6 +6,7 @@
 from cms.cms_toolbars import (
     ADD_PAGE_LANGUAGE_BREAK,
     LANGUAGE_MENU_IDENTIFIER,
+    BasicToolbar,
     PageToolbar,
     PlaceholderToolbar,
 )
@@ -242,7 +243,7 @@ def _get_published_page_version(self):
 
         return PageContent._original_manager.filter(
             page=self.page, language=language, versions__state=PUBLISHED
-        ).first()
+        ).select_related("page").first()
 
     def _add_view_published_button(self):
         """Helper method to add a publish button to the toolbar
@@ -320,7 +321,6 @@ def populate(self):
         self.page_content = self.get_page_content() if self.page else None
         self.permissions_activated = get_cms_setting("PERMISSION")
 
-        self.override_language_menu()
         self.change_admin_menu()
         self.add_page_menu()
         self.change_language_menu()
@@ -401,8 +401,9 @@ def change_language_menu(self):
                         url = add_url_parameters(translation_delete_url, language=code)
                         on_close = REFRESH_PAGE
                         if self.toolbar.get_object() == pagecontent and not disabled:
-                            other_content = next((self.page.get_admin_content(lang)for lang in self.page.get_languages()
-                                                  if lang != pagecontent.language and lang in languages), None)
+                            other_content = next(
+                                (self.page.get_admin_content(lang) for lang in self.page.get_languages()
+                                 if lang != pagecontent.language and lang in languages), None)
                             on_close = get_object_preview_url(other_content)
                         remove_plugins_menu.add_modal_item(name, url=url, disabled=disabled, on_close=on_close)
 
@@ -432,10 +433,31 @@ def change_language_menu(self):
                         )
 
 
+class VersioningBasicToolbar(BasicToolbar):
+    def add_language_menu(self):
+        """
+        Originally did override the default language menu for pages that are versioned.
+        Now creates the menu from scratch, since VersiongBasicToolbar prevents the
+        core from creating the too generic default language menu.
+        """
+        if not settings.USE_I18N or not self.request.current_page:
+            # Only add if no page is shown
+            super().add_language_menu()
+            return
+
+        language_menu = self.toolbar.get_or_create_menu(
+            LANGUAGE_MENU_IDENTIFIER, _("Language"), position=-1
+        )
+        for code, name in get_language_tuple(self.current_site.pk):
+            # Get the page content, it could be draft too!
+            page_content = self.page.get_admin_content(language=code)
+            if page_content:
+                url = get_object_preview_url(page_content, code)
+                language_menu.add_link_item(name, url=url, active=self.current_lang == code)
+
+
 def replace_toolbar(old, new):
-    """Replace `old` toolbar class with `new` class,
-    while keeping its position in toolbar_pool.
-    """
+    """Replace `old` toolbar class with `new` class, while keeping its position in toolbar_pool."""
     new_name = ".".join((new.__module__, new.__name__))
     old_name = ".".join((old.__module__, old.__name__))
     toolbar_pool.toolbars = OrderedDict(
@@ -446,3 +468,4 @@ def replace_toolbar(old, new):
 
 replace_toolbar(PageToolbar, VersioningPageToolbar)
 replace_toolbar(PlaceholderToolbar, VersioningToolbar)
+replace_toolbar(BasicToolbar, VersioningBasicToolbar)

djangocms_versioning/datastructures.py

@@ -176,8 +176,7 @@ def content_types(self):
 
 
 class PolymorphicVersionableItem(VersionableItem):
-    """VersionableItem for use by base polymorphic class
-    (for example filer.File).
+    """VersionableItem for use by base polymorphic class (for example filer.File).
     """
 
     def _get_content_types(self):

djangocms_versioning/models.py

@@ -51,6 +51,7 @@ def get_for_content(self, content_object):
         version = self.get(
             object_id=content_object.pk, content_type__in=versionable.content_types
         )
+        version._state.fields_cache["content"] = content_object
         content_object._version_cache = version
         return version
 
@@ -243,7 +244,11 @@ def convert_to_proxy(self):
         """Returns a copy of current Version object, but as an instance
         of its correct proxy model"""
 
+        cache = self._state.fields_cache
+        del self._state.fields_cache  # Remove cache before creating deep copy
         new_obj = copy.deepcopy(self)
+        new_obj._state.fields_cache = cache  # Recover caches
+        self._state.fields_cache = cache  # Recover caches
         new_obj.__class__ = self.versionable.version_model_proxy
         return new_obj
 

djangocms_versioning/plugin_rendering.py

@@ -1,8 +1,8 @@
-from functools import lru_cache
 
 from cms import __version__ as cms_version
 from cms.plugin_rendering import ContentRenderer, StructureRenderer
 from cms.utils.placeholder import rescan_placeholders_for_obj
+from django.utils.functional import cached_property
 
 from . import versionables
 from .constants import DRAFT, PUBLISHED
@@ -77,12 +77,10 @@ def render_plugin(self, instance, page=None):
 
 
 class CMSToolbarVersioningMixin:
-    @property
-    @lru_cache(16)
+    @cached_property
     def content_renderer(self):
         return VersionContentRenderer(request=self.request)
 
-    @property
-    @lru_cache(16)
+    @cached_property
     def structure_renderer(self):
         return VersionStructureRenderer(request=self.request)

tests/test_integration_with_core.py

@@ -5,9 +5,10 @@
 from cms.toolbar.toolbar import CMSToolbar
 from cms.utils.urlutils import admin_reverse
 from django.template import Context
+from packaging.version import Version as PackageVersion
 
 from djangocms_versioning import constants
-from djangocms_versioning.plugin_rendering import VersionContentRenderer
+from djangocms_versioning.plugin_rendering import CMSToolbarVersioningMixin, VersionContentRenderer
 from djangocms_versioning.test_utils.factories import (
     PageFactory,
     PageVersionFactory,
@@ -18,12 +19,14 @@
 )
 
 
+@skipIf(PackageVersion(cms_version) >= PackageVersion("4.2"), "Toolbar integration not necessary for django CMS 4.2+")
 class CMSToolbarTestCase(CMSTestCase):
     def test_content_renderer(self):
         """Test that cms.toolbar.toolbar.CMSToolbar.content_renderer
         is replaced with a property returning VersionContentRenderer
         """
         request = self.get_request("/")
+        self.assertIn(CMSToolbarVersioningMixin, CMSToolbar.__mro__)
         self.assertEqual(
             CMSToolbar(request).content_renderer.__class__, VersionContentRenderer
         )
@@ -38,7 +41,6 @@ def test_cmstoolbar_mixin(self):
 
 
 class PageContentAdminTestCase(CMSTestCase):
-
     def test_get_admin_model_object(self):
         """
         PageContent normally won't be able to fetch objects in draft. Test if the RequestToolbarForm
@@ -70,7 +72,6 @@ def test_get_title_cache(self):
 
 
 class PageAdminCopyLanguageTestCase(CMSTestCase):
-
     def setUp(self):
         self.user = self.get_superuser()
         page = PageFactory()
@@ -280,7 +281,8 @@ def setUp(self):
         self.page.save()
 
 
-    @skipIf(cms_version < "4.1.4", "Bug only fixed in django CMS 4.1.4")
+    @skipIf(PackageVersion(cms_version) < PackageVersion("4.1.4"),
+            "Bug only fixed in django CMS 4.1.4")
     def test_get_admin_url_for_language(self):
         """Regression fixed that made unpublished and archived versions invisible to get_admin_url_for_language
         template tag. See: https://github.com/django-cms/django-cms/pull/7967"""

tests/test_toolbars.py

@@ -8,6 +8,7 @@
 from cms.toolbar.utils import get_object_edit_url, get_object_preview_url
 from cms.utils.urlutils import admin_reverse
 from django.contrib.auth.models import Permission
+from django.test import override_settings
 from django.utils.text import slugify
 from packaging.version import Version
 
@@ -669,6 +670,7 @@ def test_change_language_menu_page_toolbar_language_selector_version_link(self):
         self.assertEqual(de_item.url, de_preview_url)
         self.assertEqual(it_item.url, it_preview_url)
 
+    @override_settings(USE_I18N=False)
     def test_page_toolbar_wo_language_menu(self):
         from django.utils.translation import gettext as _
 
@@ -681,13 +683,13 @@ def test_page_toolbar_wo_language_menu(self):
             user=self.get_superuser(),
         )
         # Remove language menu from request's toolbar
-        del request.toolbar.menus[LANGUAGE_MENU_IDENTIFIER]
+        self.assertNotIn(LANGUAGE_MENU_IDENTIFIER, request.toolbar.menus)
 
-        # find VersioningPageToolbar
+        # find VersioningBasicToolbar
         for cls, toolbar in request.toolbar.toolbars.items():
-            if cls == "djangocms_versioning.cms_toolbars.VersioningPageToolbar":
+            if cls == "djangocms_versioning.cms_toolbars.VersioningBasicToolbar":
                 # and call override_language_menu
-                toolbar.override_language_menu()
+                toolbar.add_language_menu()
                 break
 
         language_menu = request.toolbar.get_menu(LANGUAGE_MENU_IDENTIFIER, _("Language"))