updated Site.find_for_request() to only hit the database once, and related unit tests

pull/2465/head
Matthew Downey 2015-11-27 11:41:51 +11:00 zatwierdzone przez Matt Westcott
rodzic 8b2a58ecae
commit 9bbf1ba9a0
4 zmienionych plików z 69 dodań i 28 usunięć

Wyświetl plik

@ -14,6 +14,7 @@ Changelog
* Highlight current day in date picker (Jonas Lergell)
* Eliminated the deprecated `register.assignment_tag` on Django 1.9 (Josh Schneier)
* Increased size of Save button on site settings (Liam Brenner)
* Optimised Site.find_for_request to only perform one database query (Matthew Downey)
* Fix: The currently selected day is now highlighted only in the correct month in date pickers (Jonas Lergell)
* Fix: Fixed crash when an image without a source file was resized with the "dynamic serve view"
* Fix: Registered settings admin menu items now show active correctly (Matthew Downey)

Wyświetl plik

@ -34,6 +34,7 @@ Minor features
* Highlight current day in date picker (Jonas Lergell)
* Eliminated the deprecated ``register.assignment_tag`` on Django 1.9 (Josh Schneier)
* Increased size of Save button on site settings (Liam Brenner)
* Optimised Site.find_for_request to only perform one database query (Matthew Downey)
Bug fixes
~~~~~~~~~

Wyświetl plik

@ -14,7 +14,7 @@ from django.core.handlers.base import BaseHandler
from django.core.handlers.wsgi import WSGIRequest
from django.core.urlresolvers import reverse
from django.db import connection, models, transaction
from django.db.models import Q
from django.db.models import Q, Case, When, IntegerField
from django.db.models.signals import post_delete, post_save, pre_delete
from django.dispatch.dispatcher import receiver
from django.http import Http404
@ -42,6 +42,12 @@ logger = logging.getLogger('wagtail.core')
PAGE_TEMPLATE_VAR = 'page'
MATCH_HOSTNAME_PORT = 0
MATCH_HOSTNAME_DEFAULT = 1
MATCH_DEFAULT = 2
MATCH_HOSTNAME = 3
class SiteManager(models.Manager):
def get_by_natural_key(self, hostname, port):
return self.get(hostname=hostname, port=port)
@ -106,26 +112,49 @@ 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
"""
try:
hostname = request.get_host().split(':')[0]
try:
# find a Site matching this specific hostname
return Site.objects.get(hostname=hostname) # Site.DoesNotExist here goes to the final except clause
except Site.MultipleObjectsReturned:
# as there were more than one, try matching by port too
try:
port = request.get_port()
except AttributeError:
# Request.get_port is Django 1.9+
# KeyError here falls out below
port = request.META['SERVER_PORT']
return Site.objects.get(hostname=hostname, port=int(port))
# Site.DoesNotExist here goes to the final except clause
except (Site.DoesNotExist, KeyError):
# If no matching site exists, or request does not specify an HTTP_HOST (which
# will often be the case for the Django test client), look for a catch-all Site.
# If that fails, let the Site.DoesNotExist propagate back to the caller
return Site.objects.get(is_default_site=True)
except KeyError:
hostname = None
try:
port = request.get_port()
except (AttributeError, KeyError):
port = request.META.get('SERVER_PORT')
sites = list(Site.objects.annotate(match=Case(
# annotate the results by best choice descending
# put exact hostname+port match first
When(hostname=hostname, port=port, then=MATCH_HOSTNAME_PORT),
# then put hostname+default (better than just hostname or just default)
When(hostname=hostname, is_default_site=True, then=MATCH_HOSTNAME_DEFAULT),
# then match default with different hostname. there is only ever
# one default, so order it above (possibly multiple) hostname
# matches so we can use sites[0] below to access it
When(is_default_site=True, then=MATCH_DEFAULT),
# because of the filter below, if it's not default then its a hostname match
default=MATCH_HOSTNAME,
output_field=IntegerField(),
)).filter(Q(hostname=hostname) | Q(is_default_site=True)).order_by('match'))
if sites:
# if theres a unique match or hostname (with port or default) match
if len(sites) == 1 or sites[0].match in (MATCH_HOSTNAME_PORT, MATCH_HOSTNAME_DEFAULT):
return sites[0]
# if there is a default match with a different hostname, see if
# there are many hostname matches. if only 1 then use that instead
# otherwise we use the default
if sites[0].match == MATCH_DEFAULT:
return sites[len(sites) == 2]
raise Site.DoesNotExist()
@property
def root_url(self):

Wyświetl plik

@ -97,6 +97,7 @@ class TestSiteRouting(TestCase):
port='8765'
)
self.about_site = Site.objects.create(hostname='about.example.com', root_page=about_page)
self.alternate_port_default_site = Site.objects.create(hostname=self.default_site.hostname, port='8765', root_page=self.default_site.root_page)
self.unrecognised_port = '8000'
self.unrecognised_hostname = 'unknown.site.com'
@ -104,7 +105,8 @@ class TestSiteRouting(TestCase):
# requests without a Host: header should be directed to the default site
request = HttpRequest()
request.path = '/'
self.assertEqual(Site.find_for_request(request), self.default_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.default_site)
def test_valid_headers_route_to_specific_site(self):
# requests with a known Host: header should be directed to the specific site
@ -112,7 +114,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = self.events_site.hostname
request.META['SERVER_PORT'] = self.events_site.port
self.assertEqual(Site.find_for_request(request), self.events_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.events_site)
def test_ports_in_request_headers_are_respected(self):
# ports in the Host: header should be respected
@ -120,7 +123,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = self.alternate_port_events_site.hostname
request.META['SERVER_PORT'] = self.alternate_port_events_site.port
self.assertEqual(Site.find_for_request(request), self.alternate_port_events_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.alternate_port_events_site)
def test_unrecognised_host_header_routes_to_default_site(self):
# requests with an unrecognised Host: header should be directed to the default site
@ -128,7 +132,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = self.unrecognised_hostname
request.META['SERVER_PORT'] = '80'
self.assertEqual(Site.find_for_request(request), self.default_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.default_site)
def test_unrecognised_port_and_default_host_routes_to_default_site(self):
# requests to the default host on an unrecognised port should be directed to the default site
@ -136,7 +141,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = self.default_site.hostname
request.META['SERVER_PORT'] = self.unrecognised_port
self.assertEqual(Site.find_for_request(request), self.default_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.default_site)
def test_unrecognised_port_and_unrecognised_host_routes_to_default_site(self):
# requests with an unrecognised Host: header _and_ an unrecognised port
@ -145,7 +151,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = self.unrecognised_hostname
request.META['SERVER_PORT'] = self.unrecognised_port
self.assertEqual(Site.find_for_request(request), self.default_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.default_site)
def test_unrecognised_port_on_known_hostname_routes_there_if_no_ambiguity(self):
# requests on an unrecognised port should be directed to the site with
@ -154,7 +161,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = self.about_site.hostname
request.META['SERVER_PORT'] = self.unrecognised_port
self.assertEqual(Site.find_for_request(request), self.about_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.about_site)
def test_unrecognised_port_on_known_hostname_routes_to_default_site_if_ambiguity(self):
# requests on an unrecognised port should be directed to the default
@ -164,7 +172,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = self.events_site.hostname
request.META['SERVER_PORT'] = self.unrecognised_port
self.assertEqual(Site.find_for_request(request), self.default_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.default_site)
def test_port_in_http_host_header_is_ignored(self):
# port in the HTTP_HOST header is ignored
@ -172,7 +181,8 @@ class TestSiteRouting(TestCase):
request.path = '/'
request.META['HTTP_HOST'] = "%s:%s" % (self.events_site.hostname, self.events_site.port)
request.META['SERVER_PORT'] = self.alternate_port_events_site.port
self.assertEqual(Site.find_for_request(request), self.alternate_port_events_site)
with self.assertNumQueries(1):
self.assertEqual(Site.find_for_request(request), self.alternate_port_events_site)
class TestRouting(TestCase):