From f745aae9d077256fa5c8f4e3e33df91fd30555b1 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 28 May 2019 16:11:58 +0200 Subject: [PATCH] change Site finding mechanism to be independent of deprecated SiteMiddleware --- wagtail/api/v2/endpoints.py | 12 ++++--- wagtail/api/v2/filters.py | 14 +++++--- wagtail/api/v2/serializers.py | 4 ++- wagtail/api/v2/utils.py | 5 +-- wagtail/contrib/redirects/middleware.py | 10 +++--- .../templatetags/wagtailroutablepage_tags.py | 4 ++- .../contrib/settings/context_processors.py | 8 +++-- wagtail/contrib/settings/jinja2tags.py | 2 +- .../templatetags/wagtailsettings_tags.py | 2 +- wagtail/contrib/settings/views.py | 3 +- wagtail/core/middleware.py | 9 +++++ wagtail/core/models.py | 36 ++++++++++++++----- wagtail/core/templatetags/wagtailcore_tags.py | 10 +++--- wagtail/core/views.py | 7 ++-- 14 files changed, 87 insertions(+), 39 deletions(-) diff --git a/wagtail/api/v2/endpoints.py b/wagtail/api/v2/endpoints.py index 3f02be47a5..24a54853fa 100644 --- a/wagtail/api/v2/endpoints.py +++ b/wagtail/api/v2/endpoints.py @@ -12,7 +12,7 @@ from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet from wagtail.api import APIField -from wagtail.core.models import Page +from wagtail.core.models import Page, Site from .filters import ( FieldsFilter, OrderingFilter, RestrictedChildOfFilter, RestrictedDescendantOfFilter, @@ -425,8 +425,9 @@ class PagesAPIEndpoint(BaseAPIEndpoint): queryset = queryset.public().live() # Filter by site - if request.site: - queryset = queryset.descendant_of(request.site.root_page, inclusive=True) + site = Site.find_for_request(request) + if site: + queryset = queryset.descendant_of(site.root_page, inclusive=True) else: # No sites configured queryset = queryset.none() @@ -438,12 +439,13 @@ class PagesAPIEndpoint(BaseAPIEndpoint): return base.specific def find_object(self, queryset, request): - if 'html_path' in request.GET and request.site is not None: + site = Site.find_for_request(request) + if 'html_path' in request.GET and site is not None: path = request.GET['html_path'] path_components = [component for component in path.split('/') if component] try: - page, _, _ = request.site.root_page.specific.route(request, path_components) + page, _, _ = site.root_page.specific.route(request, path_components) except Http404: return diff --git a/wagtail/api/v2/filters.py b/wagtail/api/v2/filters.py index 9083701fd1..317bcf3696 100644 --- a/wagtail/api/v2/filters.py +++ b/wagtail/api/v2/filters.py @@ -4,7 +4,7 @@ from rest_framework.filters import BaseFilterBackend from taggit.managers import TaggableManager from wagtail.core import hooks -from wagtail.core.models import Page, UserPagePermissionsProxy +from wagtail.core.models import Page, UserPagePermissionsProxy, Site from wagtail.search.backends import get_search_backend from wagtail.search.backends.base import FilterFieldError, OrderByFieldError @@ -167,10 +167,12 @@ class RestrictedChildOfFilter(ChildOfFilter): site to be specified. """ def get_root_page(self, request): - return request.site.root_page + site = Site.find_for_request(request) + return site.root_page def get_page_by_id(self, request, page_id): - site_pages = pages_for_site(request.site) + site = Site.find_for_request(request) + site_pages = pages_for_site(site) return site_pages.get(id=page_id) @@ -214,10 +216,12 @@ class RestrictedDescendantOfFilter(DescendantOfFilter): site to be specified. """ def get_root_page(self, request): - return request.site.root_page + site = Site.find_for_request(request) + return site.root_page def get_page_by_id(self, request, page_id): - site_pages = pages_for_site(request.site) + site = Site.find_for_request(request) + site_pages = pages_for_site(site) return site_pages.get(id=page_id) diff --git a/wagtail/api/v2/serializers.py b/wagtail/api/v2/serializers.py index e966dd82ee..6adb6bf009 100644 --- a/wagtail/api/v2/serializers.py +++ b/wagtail/api/v2/serializers.py @@ -7,6 +7,7 @@ from rest_framework.fields import Field, SkipField from taggit.managers import _TaggableManager from wagtail.core import fields as wagtailcore_fields +from wagtail.core.models import Site from .utils import get_object_detail_url, pages_for_site @@ -121,7 +122,8 @@ class PageParentField(relations.RelatedField): def get_attribute(self, instance): parent = instance.get_parent() - site_pages = pages_for_site(self.context['request'].site) + site = Site.find_for_request(self.context['request']) + site_pages = pages_for_site(site) if site_pages.filter(id=parent.id).exists(): return parent diff --git a/wagtail/api/v2/utils.py b/wagtail/api/v2/utils.py index 84bcee8a0a..25120023f6 100644 --- a/wagtail/api/v2/utils.py +++ b/wagtail/api/v2/utils.py @@ -2,7 +2,7 @@ from urllib.parse import urlparse from django.conf import settings -from wagtail.core.models import Page +from wagtail.core.models import Page, Site from wagtail.core.utils import resolve_model_string @@ -11,7 +11,8 @@ class BadRequestError(Exception): def get_base_url(request=None): - base_url = getattr(settings, 'WAGTAILAPI_BASE_URL', request.site.root_url if request and request.site else None) + site = Site.find_for_request(request) if request else None + base_url = getattr(settings, 'WAGTAILAPI_BASE_URL', site.root_url if site else None) if base_url: # We only want the scheme and netloc diff --git a/wagtail/contrib/redirects/middleware.py b/wagtail/contrib/redirects/middleware.py index c2eaba23a2..0b76d7a21e 100644 --- a/wagtail/contrib/redirects/middleware.py +++ b/wagtail/contrib/redirects/middleware.py @@ -5,17 +5,19 @@ from django.utils.deprecation import MiddlewareMixin from django.utils.encoding import uri_to_iri from wagtail.contrib.redirects import models +from wagtail.core.models import Site def _get_redirect(request, path): if '\0' in path: # reject URLs with null characters, which crash on Postgres (#4496) return None + site = Site.find_for_request(request) try: - return models.Redirect.get_for_site(request.site).get(old_path=path) + return models.Redirect.get_for_site(site).get(old_path=path) except models.Redirect.MultipleObjectsReturned: # We have a site-specific and a site-ambivalent redirect; prefer the specific one - return models.Redirect.objects.get(site=request.site, old_path=path) + return models.Redirect.objects.get(site=site, old_path=path) except models.Redirect.DoesNotExist: return None @@ -37,8 +39,8 @@ class RedirectMiddleware(MiddlewareMixin): # If a middleware before `SiteMiddleware` returned a response the # `site` attribute was never set, ref #2120 - if not hasattr(request, 'site'): - return response + #if not hasattr(request, 'site'): + # return response # Get the path path = models.Redirect.normalise_path(request.get_full_path()) diff --git a/wagtail/contrib/routable_page/templatetags/wagtailroutablepage_tags.py b/wagtail/contrib/routable_page/templatetags/wagtailroutablepage_tags.py index 08625b4297..ddf05892e0 100644 --- a/wagtail/contrib/routable_page/templatetags/wagtailroutablepage_tags.py +++ b/wagtail/contrib/routable_page/templatetags/wagtailroutablepage_tags.py @@ -1,4 +1,5 @@ from django import template +from wagtail.core.models import Site register = template.Library() @@ -18,7 +19,8 @@ def routablepageurl(context, page, url_name, *args, **kwargs): positional arguments and keyword arguments. """ request = context['request'] - base_url = page.relative_url(request.site, request) + site = Site.find_for_request(request) + base_url = page.relative_url(site, request) routed_url = page.reverse_subpage(url_name, args=args, kwargs=kwargs) if not base_url.endswith('/'): base_url += '/' diff --git a/wagtail/contrib/settings/context_processors.py b/wagtail/contrib/settings/context_processors.py index ba3fc507a3..75383a4c46 100644 --- a/wagtail/contrib/settings/context_processors.py +++ b/wagtail/contrib/settings/context_processors.py @@ -1,3 +1,5 @@ +from wagtail.core.models import Site + from .registry import registry @@ -49,10 +51,10 @@ class SettingModuleProxy(dict): def settings(request): - site = getattr(request, 'site', None) + site = Site.find_for_request(request) if site is None: - # Can't assume SiteMiddleware already executed - # (e.g. middleware rendering a template before that) + # find_for_request() can't determine the site + # old SiteMiddleware case # Unittest or email templates might also mock request # objects that don't have a request.site. return {} diff --git a/wagtail/contrib/settings/jinja2tags.py b/wagtail/contrib/settings/jinja2tags.py index 13d9fb1b7f..c7aebcb6f6 100644 --- a/wagtail/contrib/settings/jinja2tags.py +++ b/wagtail/contrib/settings/jinja2tags.py @@ -60,7 +60,7 @@ def get_setting(context, model_string, use_default_site=False): if use_default_site: site = Site.objects.get(is_default_site=True) elif 'request' in context: - site = context['request'].site + site = Site.find_for_request(context['request']) else: raise RuntimeError('No request found in context, and use_default_site ' 'flag not set') diff --git a/wagtail/contrib/settings/templatetags/wagtailsettings_tags.py b/wagtail/contrib/settings/templatetags/wagtailsettings_tags.py index a650b3bb49..360298a76f 100644 --- a/wagtail/contrib/settings/templatetags/wagtailsettings_tags.py +++ b/wagtail/contrib/settings/templatetags/wagtailsettings_tags.py @@ -12,7 +12,7 @@ def get_settings(context, use_default_site=False): if use_default_site: site = Site.objects.get(is_default_site=True) elif 'request' in context: - site = context['request'].site + site = Site.find_for_request(context['request']) else: raise RuntimeError('No request found in context, and use_default_site ' 'flag not set') diff --git a/wagtail/contrib/settings/views.py b/wagtail/contrib/settings/views.py index 88bc82b1fa..fdc1aa20cb 100644 --- a/wagtail/contrib/settings/views.py +++ b/wagtail/contrib/settings/views.py @@ -40,7 +40,8 @@ def get_setting_edit_handler(model): def edit_current_site(request, app_name, model_name): # Redirect the user to the edit page for the current site # (or the current request does not correspond to a site, the first site in the list) - site = request.site or Site.objects.first() + site_request = Site.find_for_request(request) + site = site_request or Site.objects.first() if not site: messages.error(request, _("This setting could not be opened because there is no site defined.")) return redirect('wagtailadmin_home') diff --git a/wagtail/core/middleware.py b/wagtail/core/middleware.py index 5fb3f885c6..9451ec3af8 100644 --- a/wagtail/core/middleware.py +++ b/wagtail/core/middleware.py @@ -2,6 +2,9 @@ from django.utils.deprecation import MiddlewareMixin from wagtail.core.models import Site +import warnings +from wagtail.utils.deprecation import RemovedInWagtail28Warning + class SiteMiddleware(MiddlewareMixin): def process_request(self, request): @@ -9,6 +12,12 @@ class SiteMiddleware(MiddlewareMixin): Set request.site to contain the Site object responsible for handling this request, according to hostname matching rules """ + warnings.warn( + 'wagtail SiteMiddleware and the use of request.site is deprecated ' + 'and will be removed in wagtail 2.8. Update your middleware settings.', + RemovedInWagtail28Warning, stacklevel=2 + ) + try: request.site = Site.find_for_request(request) except Site.DoesNotExist: diff --git a/wagtail/core/models.py b/wagtail/core/models.py index 95933c065c..6d20cc7bb4 100644 --- a/wagtail/core/models.py +++ b/wagtail/core/models.py @@ -19,7 +19,7 @@ from django.http import Http404 from django.template.response import TemplateResponse from django.urls import reverse from django.utils import timezone -from django.utils.functional import cached_property +from django.utils.functional import cached_property, lazy from django.utils.text import capfirst, slugify from django.utils.translation import ugettext_lazy as _ from modelcluster.models import ( @@ -106,11 +106,29 @@ class Site(models.Model): NB this means that high-numbered ports on an extant hostname may still be routed to a different hostname which is set as the default + + The site will be cached via request._wagtail_site """ + if request is None: + return None + + if not hasattr(request, '_wagtail_site'): + site = Site._find_for_request(request) + setattr(request, '_wagtail_site', site) + return request._wagtail_site + + @staticmethod + def _find_for_request(request): hostname = request.get_host().split(':')[0] port = request.get_port() - return get_site_for_hostname(hostname, port) + site = None + try: + site = get_site_for_hostname(hostname, port) + except Site.DoesNotExist: + pass + # copy old SiteMiddleware behavior + return site @property def root_url(self): @@ -771,9 +789,10 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): site_id, root_path, root_url = possible_sites[0] - if hasattr(request, 'site'): + site = Site.find_for_request(request) + if site: for site_id, root_path, root_url in possible_sites: - if site_id == request.site.pk: + if site_id == site.pk: break else: site_id, root_path, root_url = possible_sites[0] @@ -814,14 +833,15 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): Accepts an optional but recommended ``request`` keyword argument that, if provided, will be used to cache site-level URL information (thereby avoiding repeated database / cache - lookups) and, via the ``request.site`` attribute, determine whether a relative or full URL - is most appropriate. + lookups) and, via the ``Site.find_for_request()`` function, determine whether a relative + or full URL is most appropriate. """ # ``current_site`` is purposefully undocumented, as one can simply pass the request and get - # a relative URL based on ``request.site``. Nonetheless, support it here to avoid + # a relative URL based on ``Site.find_for_request()``. Nonetheless, support it here to avoid # copy/pasting the code to the ``relative_url`` method below. if current_site is None and request is not None: - current_site = getattr(request, 'site', None) + site = Site.find_for_request(request) + current_site = site url_parts = self.get_url_parts(request=request) if url_parts is None: diff --git a/wagtail/core/templatetags/wagtailcore_tags.py b/wagtail/core/templatetags/wagtailcore_tags.py index a50cf0668a..21e4a3fe4b 100644 --- a/wagtail/core/templatetags/wagtailcore_tags.py +++ b/wagtail/core/templatetags/wagtailcore_tags.py @@ -5,7 +5,7 @@ from django.utils.encoding import force_text from django.utils.safestring import mark_safe from wagtail import VERSION, __version__ -from wagtail.core.models import Page +from wagtail.core.models import Page, Site from wagtail.core.rich_text import RichText, expand_db_html from wagtail.utils.version import get_main_version @@ -26,9 +26,10 @@ def pageurl(context, page, fallback=None): raise ValueError("pageurl tag expected a Page object, got %r" % page) try: - current_site = context['request'].site + site = Site.find_for_request(context['request']) + current_site = site except (KeyError, AttributeError): - # request.site not available in the current context; fall back on page.url + # site not available in the current context; fall back on page.url return page.url # Pass page.relative_url the request object, which may contain a cached copy of @@ -49,7 +50,8 @@ def slugurl(context, slug): """ try: - current_site = context['request'].site + site = Site.find_for_request(context['request']) + current_site = site except (KeyError, AttributeError): # No site object found - allow the fallback below to take place. page = None diff --git a/wagtail/core/views.py b/wagtail/core/views.py index 879f8b2c1c..5438b51b8f 100644 --- a/wagtail/core/views.py +++ b/wagtail/core/views.py @@ -4,17 +4,18 @@ from django.urls import reverse from wagtail.core import hooks from wagtail.core.forms import PasswordViewRestrictionForm -from wagtail.core.models import Page, PageViewRestriction +from wagtail.core.models import Page, PageViewRestriction, Site def serve(request, path): # we need a valid Site object corresponding to this request (set in wagtail.core.middleware.SiteMiddleware) # in order to proceed - if not request.site: + site = Site.find_for_request(request) + if not site: raise Http404 path_components = [component for component in path.split('/') if component] - page, args, kwargs = request.site.root_page.specific.route(request, path_components) + page, args, kwargs = site.root_page.specific.route(request, path_components) for fn in hooks.get_hooks('before_serve_page'): result = fn(page, request, args, kwargs)