From 197ada7d52dc69c436d2b535b08ed0c50f672a78 Mon Sep 17 00:00:00 2001 From: Github Release Action Date: Wed, 13 Nov 2024 23:11:32 +0100 Subject: [PATCH 1/7] Remove `languages` field from `Page` --- cms/admin/pageadmin.py | 6 +- cms/api.py | 11 +- cms/cms_menus.py | 3 +- cms/management/commands/subcommands/copy.py | 3 - cms/models/pagemodel.py | 125 +++++++++----------- cms/templatetags/cms_admin.py | 2 +- cms/tests/test_toolbar.py | 5 +- cms/tests/test_views.py | 12 ++ cms/utils/page.py | 7 +- menus/utils.py | 15 +-- 10 files changed, 95 insertions(+), 94 deletions(-) diff --git a/cms/admin/pageadmin.py b/cms/admin/pageadmin.py index 31c60d990ac..3d99e3c9f45 100644 --- a/cms/admin/pageadmin.py +++ b/cms/admin/pageadmin.py @@ -1227,14 +1227,16 @@ def delete_view(self, request, object_id, extra_context=None): 'language': force_str(get_language_object(language)['name']) } messages.success(request, message) + if language in page.admin_content_cache: + del page.admin_content_cache[language] + if language in page.page_content_cache: + del page.page_content_cache[language] page_url.delete() page_content.delete() for p in saved_plugins: p.delete() - page.remove_language(language) - send_post_page_operation( request=request, operation=operations.DELETE_PAGE_TRANSLATION, diff --git a/cms/api.py b/cms/api.py index 207a3d7950f..317156c9111 100644 --- a/cms/api.py +++ b/cms/api.py @@ -313,12 +313,10 @@ def create_page_content(language, title, page, menu_title=None, slug=None, _thread_locals.user = created_by created_by = get_clean_username(created_by) - page.urls.create( - slug=slug, - path=path, + page.urls.update_or_create( page=page, - managed=not bool(overwrite_url), language=language, + defaults=dict(slug=slug, path=path, managed=not bool(overwrite_url)), ) # E.g., djangocms-versioning needs an User object to be passed when creating a versioned Object @@ -340,11 +338,8 @@ def create_page_content(language, title, page, menu_title=None, slug=None, xframe_options=xframe_options, ) page_content.rescan_placeholders() + page._clear_internal_cache() - page_languages = page.get_languages() - - if language not in page_languages: - page.update_languages(page_languages + [language]) return page_content diff --git a/cms/cms_menus.py b/cms/cms_menus.py index e8e756e550b..430fcdae8bc 100644 --- a/cms/cms_menus.py +++ b/cms/cms_menus.py @@ -333,7 +333,6 @@ def get_nodes(self, request) -> list[NavigationNode]: "soft_root", "in_navigation", "page__site_id", - "page__languages", "page__parent_id", "page__is_home", "page__login_required", @@ -352,7 +351,7 @@ def prefetch_urls(page_content: PageContent) -> PageContent: else: preview_url = None # No short-cut here prefetched_urls = PageUrl.objects.filter( - language__in=self.languages, + language__in=(page_content.language for page_content in page_contents), page_id__in=(page_content.page.pk for page_content in page_contents), ) # Fetch the PageUrl objects # Prepare for filling urls_cache diff --git a/cms/management/commands/subcommands/copy.py b/cms/management/commands/subcommands/copy.py index 869ad292ed8..573c66b6f49 100644 --- a/cms/management/commands/subcommands/copy.py +++ b/cms/management/commands/subcommands/copy.py @@ -89,9 +89,6 @@ def handle(self, *args, **options): new_title["page"] = page PageContent.objects.with_user(user).create(**new_title) - if to_lang not in page.get_languages(): - page.update_languages(page.get_languages() + [to_lang]) - if copy_content: # copy plugins using API if verbose: diff --git a/cms/models/pagemodel.py b/cms/models/pagemodel.py index 925eb165aa2..0289d26e1ac 100644 --- a/cms/models/pagemodel.py +++ b/cms/models/pagemodel.py @@ -16,12 +16,11 @@ from treebeard.mp_tree import MP_Node from cms import constants -from cms.exceptions import LanguageError from cms.models.managers import PageManager, PageUrlManager from cms.utils import i18n from cms.utils.compat.warnings import RemovedInDjangoCMS43Warning from cms.utils.conf import get_cms_setting -from cms.utils.i18n import get_current_language, get_fallback_languages +from cms.utils.i18n import get_current_language from cms.utils.page import get_clean_username from menus.menu_pool import menu_pool @@ -114,12 +113,6 @@ class Page(MP_Node): blank=True, null=True, ) - languages = models.CharField( - max_length=255, - editable=False, - blank=True, - null=True, - ) is_page_type = models.BooleanField( default=False, help_text=_("Mark this page as a page type"), @@ -143,6 +136,7 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.urls_cache = {} + #: Internal cache for page urls self.page_content_cache = {} #: Internal cache for page content objects visible publicly self.admin_content_cache = AdminCacheDict() @@ -438,7 +432,6 @@ def move_page(self, target_page, position='first-child'): languages = ( self .urls - .filter(language__in=self.get_languages()) .values_list('language', flat=True) ) @@ -524,7 +517,6 @@ def copy(self, site, parent_page=None, parent_node=None, language=None, ) placeholder.copy_plugins(new_placeholder, language=new_title.language) new_page.page_content_cache[new_title.language] = new_title - new_page.update_languages([trans.language for trans in translations]) if extensions: from cms.extensions import extension_pool @@ -692,30 +684,46 @@ def get_parent_page(self): ) return self.parent - def get_languages(self): - if self.languages: - return sorted(self.languages.split(',')) - else: - return [] + @property + def languages(self): + warnings.warn( + "Attribute `languages` is deprecated. Use `get_languages` instead.", + RemovedInDjangoCMS43Warning, + stacklevel=2 + ) + return ",".join(self.get_languages()) + + def get_languages(self, admin_manager=True): + """Returns available languages for the page. This is potentially costly.""" + if admin_manager: + if not self.admin_content_cache: + self.set_admin_content_cache() + return list(self.admin_content_cache.keys()) + if not self.page_content_cache: + self._get_page_content_cache(get_language(), fallback=False, force_reload=False) + return list(self.page_content_cache.keys()) def remove_language(self, language): - page_languages = self.get_languages() - - if language in page_languages: - page_languages.remove(language) - self.update_languages(page_languages) + warnings.warn( + "Method `remove_language` is deprecated and has no effect any more.", + RemovedInDjangoCMS43Warning, + stacklevel=2 + ) def update_languages(self, languages): - languages = ",".join(set(languages)) - # Update current instance - self.languages = languages - # Commit. It's important to not call save() - # we'd like to commit only the languages field and without - # any kind of signals. - self.update(languages=languages) + warnings.warn( + "Method `update_languages` is deprecated and has no effect any more.", + RemovedInDjangoCMS43Warning, + stacklevel=2 + ) def get_published_languages(self): - return self.get_languages() + warnings.warn( + "Method `get_published_languages` is deprecated. Use `get_languages(admin_manager=False)` instead.", + RemovedInDjangoCMS43Warning, + stacklevel=2, + ) + return self.get_languages(admin_manager=False) def set_translations_cache(self): warnings.warn( @@ -800,58 +808,42 @@ def get_page_content_obj_attribute(self, attrname, language=None, fallback=True, except AttributeError: return None - def get_path(self, language, fallback=True): + def get_url_obj(self, language, fallback=True): """Get the path of the page depending on the given language""" languages = [language] if fallback: languages.extend(self.get_fallbacks(language)) - page_languages = self.get_languages() - for _language in languages: - if _language in page_languages: + if _language in self.urls_cache: language = _language break - - if language not in self.urls_cache: - self.urls_cache.update({ - url.language: url for url in self.urls.all() if url.language in languages # TODO: overwrites multiple urls - }) - + else: + # `get_page_from_request` will fill the cache only for the current language + # Here, we fully fill it and try again + self.urls_cache ={ + url.language: url for url in self.urls.all() if url.language in languages + } for _language in languages: - self.urls_cache.setdefault(_language, None) - - try: - return self.urls_cache[language].path - except (AttributeError, KeyError): - return None + if _language in self.urls_cache: + language = _language + break - def get_slug(self, language, fallback=True): - languages = [language] + return self.urls_cache.get(language) - if fallback: - languages.extend(self.get_fallbacks(language)) - - page_languages = self.get_languages() - - for _language in languages: - if _language in page_languages: - language = _language - break - - if language not in self.urls_cache: - self.urls_cache.update({ - url.language: url for url in self.urls.filter(language__in=languages) - }) + def get_path(self, language, fallback=True): + url = self.get_url_obj(language, fallback) + if url: + return url.path + return None - for _language in languages: - self.urls_cache.setdefault(_language, None) + def get_slug(self, language, fallback=True): + url = self.get_url_obj(language, fallback) + if url: + return url.slug + return None - try: - return self.urls_cache[language].slug - except (AttributeError, KeyError): - return None def get_title(self, language=None, fallback=True, force_reload=False): """ @@ -1099,6 +1091,7 @@ class PageUrl(models.Model): class Meta: app_label = 'cms' default_permissions = [] + unique_together = ('language', 'page') def __str__(self): return f"{self.path or self.slug} ({self.language})" diff --git a/cms/templatetags/cms_admin.py b/cms/templatetags/cms_admin.py index 3eae6ad06e2..fbe289dc0bc 100644 --- a/cms/templatetags/cms_admin.py +++ b/cms/templatetags/cms_admin.py @@ -98,7 +98,7 @@ def get_page_display_name(cms_page): page_content = cms_page.get_admin_content(language, fallback="force") title = page_content.title or page_content.page_title or page_content.menu_title if not title: - title = cms_page.get_slug(language) + title = cms_page.get_slug(language) or _("Empty") return title if page_content.language == language else mark_safe(f"{title} ({page_content.language})") diff --git a/cms/tests/test_toolbar.py b/cms/tests/test_toolbar.py index 03152f10e95..fc855c8cfb4 100644 --- a/cms/tests/test_toolbar.py +++ b/cms/tests/test_toolbar.py @@ -533,6 +533,7 @@ def get_delete_url(pk): page = create_page("english-page", "nav_playground.html", "en") german_content = create_page_content("de", "german content", page) english_content = page.get_content_obj('en') + page_languages = page.get_languages() edit_url = get_object_edit_url(english_content) staff = self.get_staff() self.client.force_login(staff) @@ -541,8 +542,8 @@ def get_delete_url(pk): menus = response.context['cms_toolbar'].menus language_menu = menus['language-menu'] delete = language_menu.items[-2] - german_delete = delete.items[0] - english_delete = delete.items[1] + german_delete = delete.items[page_languages.index("de")] + english_delete = delete.items[page_languages.index("en")] copy = language_menu.items[-1] copy_german = copy.items[0] diff --git a/cms/tests/test_views.py b/cms/tests/test_views.py index 09fb10b1b93..aa4ccd0d759 100644 --- a/cms/tests/test_views.py +++ b/cms/tests/test_views.py @@ -366,6 +366,18 @@ def test_malicious_content_login_request(self): self.assertNotIn(response.url, "") + def test_queries(self): + create_page("home", "simple.html", "en") + cms_page = create_page("dreinhardt", "simple.html", "en") + url = cms_page.get_absolute_url() + with self.assertNumQueries(5): + # 1. get_page_from_request: checks PageUrl + # 2. get page contents: PageContent + # 3. Check permissions + # 4. Get placeholders + # 5. Get plugins + self.client.get(url) + @override_settings(ROOT_URLCONF='cms.test_utils.project.urls') class ContextTests(CMSTestCase): diff --git a/cms/utils/page.py b/cms/utils/page.py index 26838840ef0..4676c598f17 100644 --- a/cms/utils/page.py +++ b/cms/utils/page.py @@ -2,9 +2,10 @@ from django.urls import NoReverseMatch, reverse from django.utils.encoding import force_str +from django.utils.translation.trans_null import get_language from cms.constants import PAGE_USERNAME_MAX_LENGTH -from cms.utils import get_current_site +from cms.utils import get_current_site, get_language_from_request from cms.utils.conf import get_cms_setting SUFFIX_REGEX = re.compile(r'^(.*)-(\d+)$') @@ -105,10 +106,10 @@ def get_page_from_request(request, use_path=None, clean_path=None): page_urls = list(page_urls) # force queryset evaluation to save 1 query try: page = page_urls[0].page + if page_urls[0].language == get_language_from_request(request): + page.urls_cache = {url.language: url for url in page_urls} except IndexError: page = None - else: - page.urls_cache = {url.language: url for url in page_urls} return page diff --git a/menus/utils.py b/menus/utils.py index 2ae8e01e6e7..ca6902df35c 100644 --- a/menus/utils.py +++ b/menus/utils.py @@ -207,10 +207,9 @@ def get_page_path(self, lang): if not page: return '/%s/' % lang if settings.USE_I18N else '/' - page_languages = page.get_languages() - - if lang in page_languages: - return page.get_absolute_url(lang, fallback=False) + url = page.get_absolute_url(lang, fallback=False) + if url: + return url site = get_current_site() @@ -226,14 +225,16 @@ def get_page_path(self, lang): default_language = get_default_language_for_site(site.pk) - if not _valid_language and default_language in page_languages: + if not _valid_language: # The request language is not configured for the current site. # Fallback to the default language configured for the current site. - return page.get_absolute_url(default_language, fallback=False) + url = page.get_absolute_url(default_language, fallback=False) + if url: + return url if _valid_language: fallbacks = get_fallback_languages(lang, site_id=site.pk) or [] - fallbacks = [_lang for _lang in fallbacks if _lang in page_languages] + fallbacks = [_lang for _lang in fallbacks if _lang in page.get_languages()] else: fallbacks = [] From 6b15b65d08414a1a33db518fa19495f3d0095826 Mon Sep 17 00:00:00 2001 From: Github Release Action Date: Thu, 14 Nov 2024 07:50:56 +0100 Subject: [PATCH 2/7] add migration --- ...languages_alter_pageurl_unique_together.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py diff --git a/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py b/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py new file mode 100644 index 00000000000..4764c41aa32 --- /dev/null +++ b/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.16 on 2024-11-14 06:48 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("cms", "0038_alter_page_site"), + ] + + operations = [ + migrations.RemoveField( + model_name="page", + name="languages", + ), + migrations.AlterUniqueTogether( + name="pageurl", + unique_together={("language", "page")}, + ), + ] From a945a5870da1e87a3cdf78bcf138b817732deea1 Mon Sep 17 00:00:00 2001 From: Github Release Action Date: Thu, 14 Nov 2024 07:56:22 +0100 Subject: [PATCH 3/7] Remove unused imports --- cms/utils/page.py | 1 - cms/views.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cms/utils/page.py b/cms/utils/page.py index 4676c598f17..fb078cdf255 100644 --- a/cms/utils/page.py +++ b/cms/utils/page.py @@ -2,7 +2,6 @@ from django.urls import NoReverseMatch, reverse from django.utils.encoding import force_str -from django.utils.translation.trans_null import get_language from cms.constants import PAGE_USERNAME_MAX_LENGTH from cms.utils import get_current_site, get_language_from_request diff --git a/cms/views.py b/cms/views.py index 9b1b23cc0b6..3614682cd8a 100644 --- a/cms/views.py +++ b/cms/views.py @@ -33,7 +33,7 @@ _render_welcome_page, render_pagecontent, ) -from cms.toolbar.utils import get_object_preview_url, get_object_structure_url, get_toolbar_from_request +from cms.toolbar.utils import get_object_preview_url, get_toolbar_from_request from cms.utils import get_current_site from cms.utils.conf import get_cms_setting from cms.utils.helpers import is_editable_model From 491e8861449c9052908f69715db58ee6a1213bde Mon Sep 17 00:00:00 2001 From: Github Release Action Date: Mon, 18 Nov 2024 15:28:56 +0100 Subject: [PATCH 4/7] Remove duplicates before asserting uniqueness constraint --- ...ove_page_languages_alter_pageurl_unique_together.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py b/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py index 4764c41aa32..47f690d7a95 100644 --- a/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py +++ b/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py @@ -1,6 +1,15 @@ # Generated by Django 4.2.16 on 2024-11-14 06:48 from django.db import migrations +from django.db.models import Count + + +def remove_pageurl_duplicates(apps, schema_editor): + PageUrl = apps.get_model("cms", "PageUrl") + non_unique = PageUrl.objects.values("page_id", "language").annotate(total=Count("language")).filter(total__gt=1) + for item in non_unique: + for url in PageUrl.objects.filter(page_id=item["page_id"], language=item["language"])[1:]: + url.delete() class Migration(migrations.Migration): @@ -9,6 +18,7 @@ class Migration(migrations.Migration): ] operations = [ + migrations.RunPython(remove_pageurl_duplicates), migrations.RemoveField( model_name="page", name="languages", From be640fb7729e38ee36f551d834d70612f869f7c1 Mon Sep 17 00:00:00 2001 From: Github Release Action Date: Wed, 20 Nov 2024 15:38:21 +0100 Subject: [PATCH 5/7] Fix: sourcery complexity reduction recommendation --- ..._languages_alter_pageurl_unique_together.py | 2 +- cms/models/pagemodel.py | 18 ++++++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py b/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py index 47f690d7a95..207d7930897 100644 --- a/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py +++ b/cms/migrations/0039_remove_page_languages_alter_pageurl_unique_together.py @@ -8,7 +8,7 @@ def remove_pageurl_duplicates(apps, schema_editor): PageUrl = apps.get_model("cms", "PageUrl") non_unique = PageUrl.objects.values("page_id", "language").annotate(total=Count("language")).filter(total__gt=1) for item in non_unique: - for url in PageUrl.objects.filter(page_id=item["page_id"], language=item["language"])[1:]: + for url in PageUrl.objects.filter(page_id=item["page_id"], language=item["language"]).order_by("-pk")[1:]: url.delete() diff --git a/cms/models/pagemodel.py b/cms/models/pagemodel.py index 0289d26e1ac..de6a8634866 100644 --- a/cms/models/pagemodel.py +++ b/cms/models/pagemodel.py @@ -811,26 +811,20 @@ def get_page_content_obj_attribute(self, attrname, language=None, fallback=True, def get_url_obj(self, language, fallback=True): """Get the path of the page depending on the given language""" languages = [language] - if fallback: languages.extend(self.get_fallbacks(language)) - for _language in languages: - if _language in self.urls_cache: - language = _language - break - else: + if not language in self.urls_cache: # `get_page_from_request` will fill the cache only for the current language # Here, we fully fill it and try again - self.urls_cache ={ + self.urls_cache = { url.language: url for url in self.urls.all() if url.language in languages } - for _language in languages: - if _language in self.urls_cache: - language = _language - break - return self.urls_cache.get(language) + return next( + (self.urls_cache[lang] for lang in languages if lang in self.urls_cache), + None + ) def get_path(self, language, fallback=True): url = self.get_url_obj(language, fallback) From 76075e10cb2e62e3fb6d7112d970d5555ca396a9 Mon Sep 17 00:00:00 2001 From: Github Release Action Date: Wed, 20 Nov 2024 15:41:57 +0100 Subject: [PATCH 6/7] Fix linting --- cms/models/pagemodel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/models/pagemodel.py b/cms/models/pagemodel.py index de6a8634866..d4ac9f1986c 100644 --- a/cms/models/pagemodel.py +++ b/cms/models/pagemodel.py @@ -814,7 +814,7 @@ def get_url_obj(self, language, fallback=True): if fallback: languages.extend(self.get_fallbacks(language)) - if not language in self.urls_cache: + if language not in self.urls_cache: # `get_page_from_request` will fill the cache only for the current language # Here, we fully fill it and try again self.urls_cache = { From eb085760e0bb5d12a80e8f74a49ff37a457c8bab Mon Sep 17 00:00:00 2001 From: Github Release Action Date: Thu, 21 Nov 2024 16:39:22 +0100 Subject: [PATCH 7/7] Fix tests --- cms/tests/test_templatetags.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cms/tests/test_templatetags.py b/cms/tests/test_templatetags.py index 0f7ef16a696..3409dfb51ca 100644 --- a/cms/tests/test_templatetags.py +++ b/cms/tests/test_templatetags.py @@ -81,7 +81,7 @@ def test_get_preview_url(self): def test_get_admin_tree_title(self): - page = create_page("page_a", "nav_playground.html", "en") + page = create_page("page_a", "nav_playground.html", "en", slug="slug-test2") self.assertEqual(get_page_display_name(page), 'page_a') languages = { 1: [ @@ -108,7 +108,6 @@ def test_get_admin_tree_title(self): page.admin_content_cache = {'en': PageContent(menu_title="menu test2", language="en")} self.assertEqual('menu test2 (en)', force_str(get_page_display_name(page))) page.admin_content_cache = {'en': PageContent(language="en")} - page.urls_cache = {'en': PageUrl(slug='slug-test2')} self.assertEqual('slug-test2 (en)', force_str(get_page_display_name(page))) page.admin_content_cache = {'en': PageContent(language="en"), 'fr': EmptyPageContent('fr')} self.assertEqual('slug-test2 (en)', force_str(get_page_display_name(page)))