From 88a3a397ca6e756ee644d87fdac51ba81cc971c1 Mon Sep 17 00:00:00 2001 From: Andy Babic Date: Wed, 23 Oct 2024 15:41:53 +0100 Subject: [PATCH] Prevent redundant calls to Site.find_for_request() from Page.get_url_parts() (#12454) --- CHANGELOG.txt | 1 + docs/releases/6.4.md | 1 + wagtail/admin/tests/test_page_chooser.py | 2 +- wagtail/models/__init__.py | 26 +++++++++++++----------- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index c9ede74fc7..fc5dca954a 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -57,6 +57,7 @@ Changelog * Fix: Correctly place comment buttons next to date / datetime / time fields. (Srishti Jaiswal) * Fix: Add missing heading and breadcrumbs in Account view (Sage Abdullah) * Fix: Reduce confusing spacing below StreamField blocks help text (Rishabh Sharma) + * Fix: Prevent redundant calls to `Site.find_for_request()` from `Page.get_url_parts()` (Andy Babic) * Docs: Move the model reference page from reference/pages to the references section as it covers all Wagtail core models (Srishti Jaiswal) * Docs: Move the panels reference page from references/pages to the references section as panels are available for any model editing, merge panels API into this page (Srishti Jaiswal) * Docs: Move the tags documentation to standalone advanced topic, instead of being inside the reference/pages section (Srishti Jaiswal) diff --git a/docs/releases/6.4.md b/docs/releases/6.4.md index c59bbdb0f5..4bbd6e604a 100644 --- a/docs/releases/6.4.md +++ b/docs/releases/6.4.md @@ -64,6 +64,7 @@ depth: 1 * Add missing `FilterField("created_at")` to `AbstractDocument` to fix ordering by `created_at` after searching in the documents index view (Srishti Jaiswal) * Add missing heading and breadcrumbs in Account view (Sage Abdullah) * Reduce confusing spacing below StreamField blocks help text (Rishabh Sharma) + * Prevent redundant calls to `Site.find_for_request()` from `Page.get_url_parts()` (Andy Babic) ### Documentation diff --git a/wagtail/admin/tests/test_page_chooser.py b/wagtail/admin/tests/test_page_chooser.py index eaa52d5b51..c3a3cc1f80 100644 --- a/wagtail/admin/tests/test_page_chooser.py +++ b/wagtail/admin/tests/test_page_chooser.py @@ -959,7 +959,7 @@ class TestChooserExternalLinkWithNonRootServePath(TestChooserExternalLink): "external-link-chooser-link_text": "about", } ) - with self.assertNumQueries(11): + with self.assertNumQueries(10): response = self.post( { "external-link-chooser-url": f"http://localhost/{self.prefix}about/", diff --git a/wagtail/models/__init__.py b/wagtail/models/__init__.py index 58c9223339..093c326023 100644 --- a/wagtail/models/__init__.py +++ b/wagtail/models/__init__.py @@ -16,7 +16,6 @@ import logging import posixpath import uuid from io import StringIO -from typing import TYPE_CHECKING from urllib.parse import urlsplit from warnings import warn @@ -40,7 +39,7 @@ from django.db.models import Q, Value from django.db.models.expressions import OuterRef, Subquery from django.db.models.functions import Concat, Substr from django.dispatch import receiver -from django.http import Http404, HttpResponse, HttpResponseNotAllowed +from django.http import Http404, HttpRequest, HttpResponse, HttpResponseNotAllowed from django.http.request import validate_host from django.template.response import TemplateResponse from django.urls import NoReverseMatch, reverse @@ -134,9 +133,6 @@ from .sites import Site, SiteManager, SiteRootPath # noqa: F401 from .specific import SpecificMixin from .view_restrictions import BaseViewRestriction -if TYPE_CHECKING: - from django.http import HttpRequest - logger = logging.getLogger("wagtail") PAGE_TEMPLATE_VAR = "page" @@ -2252,15 +2248,21 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): if not possible_sites: return None + # Thanks to the ordering applied by Site.get_site_root_paths(), + # the first item is ideal in the vast majority of setups. site_id, root_path, root_url, language_code = possible_sites[0] - site = Site.find_for_request(request) - if site: - for site_id, root_path, root_url, language_code in possible_sites: - if site_id == site.pk: - break - else: - site_id, root_path, root_url, language_code = possible_sites[0] + unique_site_ids = {values[0] for values in possible_sites} + if len(unique_site_ids) > 1 and isinstance(request, HttpRequest): + # The page somehow belongs to more than one site (rare, but possible). + # If 'request' is indeed a HttpRequest, use it to identify the 'current' + # site and prefer an option matching that (where present). + site = Site.find_for_request(request) + if site: + for values in possible_sites: + if values[0] == site.pk: + site_id, root_path, root_url, language_code = values + break use_wagtail_i18n = getattr(settings, "WAGTAIL_I18N_ENABLED", False)