Improves sitemap to avoid passing site now that request is used.

pull/4686/head
Bertrand Bordage 2018-07-11 20:17:48 +02:00
rodzic 88d5a8cfee
commit a3fe8eb3b1
5 zmienionych plików z 41 dodań i 34 usunięć

Wyświetl plik

@ -20,7 +20,7 @@ Changelog
* Added faceted search using the `.facet()` method (Karl Hobley)
* Admin modal views no longer rely on Javascript `eval()`, for better CSP compliance (Matt Westcott)
* Update editor guide for embeds and documents in rich text (Kevin Howbrook)
* Improved performance of sitemap generation (Michael van Tellingen)
* Improved performance of sitemap generation (Michael van Tellingen, Bertrand Bordage)
* Added an internal API for autocomplete (Karl Hobley)
* Fix: Handle all exceptions from `Image.get_file_size` (Andrew Plummer)
* Fix: Fix display of breadcrumbs in ModelAdmin (LB (Ben Johnston))

Wyświetl plik

@ -29,7 +29,7 @@ Other features
* Added faceted search using the ``.facet()`` method (Karl Hobley)
* Admin modal views no longer rely on Javascript ``eval()``, for better CSP compliance (Matt Westcott)
* Update editor guide for embeds and documents in rich text (Kevin Howbrook)
* Improved performance of sitemap generation (Michael van Tellingen)
* Improved performance of sitemap generation (Michael van Tellingen, Bertrand Bordage)
* Added an internal API for autocomplete (Karl Hobley)
Bug fixes

Wyświetl plik

@ -2,14 +2,14 @@ import warnings
from django.contrib.sitemaps import Sitemap as DjangoSitemap
from wagtail.core.models import Site
from wagtail.core.utils import accepts_kwarg
from wagtail.utils.deprecation import RemovedInWagtail24Warning
class Sitemap(DjangoSitemap):
def __init__(self, site=None, request=None):
self.site = site
def __init__(self, request=None):
self.request = request
def location(self, obj):
@ -20,9 +20,17 @@ class Sitemap(DjangoSitemap):
# (for backwards compatibility from before last_published_at was added)
return (obj.last_published_at or obj.latest_revision_created_at)
def get_wagtail_site(self):
site = getattr(self.request, 'site', None)
if site is None:
return Site.objects.select_related(
'root_page'
).get(is_default_site=True)
return site
def items(self):
return (
self.site
self.get_wagtail_site()
.root_page
.get_descendants(inclusive=True)
.live()

Wyświetl plik

@ -52,10 +52,15 @@ class TestSitemapGenerator(TestCase):
# Clear the cache to that runs are deterministic regarding the sql count
ContentType.objects.clear_cache()
def test_items(self):
request = RequestFactory().get('/sitemap.xml')
def get_request_and_django_site(self, url):
request = RequestFactory().get(url)
request.site = self.site
return request, get_current_site(request)
sitemap = Sitemap(self.site, request)
def test_items(self):
request, django_site = self.get_request_and_django_site('/sitemap.xml')
sitemap = Sitemap(request)
pages = sitemap.items()
self.assertIn(self.child_page.page_ptr.specific, pages)
@ -63,34 +68,31 @@ class TestSitemapGenerator(TestCase):
self.assertNotIn(self.protected_child_page.page_ptr.specific, pages)
def test_get_urls_without_request(self):
request = RequestFactory().get('/sitemap.xml')
request, django_site = self.get_request_and_django_site('/sitemap.xml')
req_protocol = request.scheme
req_site = get_current_site(request)
sitemap = Sitemap(self.site)
sitemap = Sitemap()
with self.assertNumQueries(18):
urls = [url['location'] for url in sitemap.get_urls(1, req_site, req_protocol)]
urls = [url['location'] for url in sitemap.get_urls(1, django_site, req_protocol)]
self.assertIn('http://localhost/', urls) # Homepage
self.assertIn('http://localhost/hello-world/', urls) # Child page
def test_get_urls_with_request_site_cache(self):
request = RequestFactory().get('/sitemap.xml')
request, django_site = self.get_request_and_django_site('/sitemap.xml')
req_protocol = request.scheme
req_site = get_current_site(request)
sitemap = Sitemap(self.site, request)
sitemap = Sitemap(request)
with self.assertNumQueries(16):
urls = [url['location'] for url in sitemap.get_urls(1, req_site, req_protocol)]
urls = [url['location'] for url in sitemap.get_urls(1, django_site, req_protocol)]
self.assertIn('http://localhost/', urls) # Homepage
self.assertIn('http://localhost/hello-world/', urls) # Child page
def test_get_urls_uses_specific(self):
request = RequestFactory().get('/sitemap.xml')
request, django_site = self.get_request_and_django_site('/sitemap.xml')
req_protocol = request.scheme
req_site = get_current_site(request)
# Add an event page which has an extra url in the sitemap
self.home_page.add_child(instance=EventIndex(
@ -99,19 +101,18 @@ class TestSitemapGenerator(TestCase):
live=True,
))
sitemap = Sitemap(self.site, request)
urls = [url['location'] for url in sitemap.get_urls(1, req_site, req_protocol)]
sitemap = Sitemap(request)
urls = [url['location'] for url in sitemap.get_urls(1, django_site, req_protocol)]
self.assertIn('http://localhost/events/', urls) # Main view
self.assertIn('http://localhost/events/past/', urls) # Sub view
def test_lastmod_uses_last_published_date(self):
request = RequestFactory().get('/sitemap.xml')
request, django_site = self.get_request_and_django_site('/sitemap.xml')
req_protocol = request.scheme
req_site = get_current_site(request)
sitemap = Sitemap(self.site, request)
urls = sitemap.get_urls(1, req_site, req_protocol)
sitemap = Sitemap(request)
urls = sitemap.get_urls(1, django_site, req_protocol)
child_page_lastmod = [
url['lastmod'] for url in urls
@ -131,12 +132,11 @@ class TestSitemapGenerator(TestCase):
self.home_page.last_published_at = datetime.datetime(2017, 3, 1, 12, 0, 0, tzinfo=pytz.utc)
self.home_page.save()
request = RequestFactory().get('/sitemap.xml')
request, django_site = self.get_request_and_django_site('/sitemap.xml')
req_protocol = request.scheme
req_site = get_current_site(request)
sitemap = Sitemap(self.site, request)
sitemap.get_urls(1, req_site, req_protocol)
sitemap = Sitemap(request)
sitemap.get_urls(1, django_site, req_protocol)
self.assertEqual(sitemap.latest_lastmod, datetime.datetime(2017, 3, 1, 12, 0, 0, tzinfo=pytz.utc))
@ -145,12 +145,11 @@ class TestSitemapGenerator(TestCase):
self.home_page.last_published_at = None
self.home_page.save()
request = RequestFactory().get('/sitemap.xml')
request, django_site = self.get_request_and_django_site('/sitemap.xml')
req_protocol = request.scheme
req_site = get_current_site(request)
sitemap = Sitemap(self.site, request)
sitemap.get_urls(1, req_site, req_protocol)
sitemap = Sitemap(request)
sitemap.get_urls(1, django_site, req_protocol)
self.assertFalse(hasattr(sitemap, 'latest_lastmod'))

Wyświetl plik

@ -12,7 +12,7 @@ def sitemap(request, sitemaps=None, **kwargs):
if sitemaps:
sitemaps = prepare_sitemaps(request, sitemaps)
else:
sitemaps = {'wagtail': Sitemap(request.site, request)}
sitemaps = {'wagtail': Sitemap(request)}
return sitemap_views.sitemap(request, sitemaps, **kwargs)
@ -21,7 +21,7 @@ def prepare_sitemaps(request, sitemaps):
initialised_sitemaps = {}
for name, sitemap_cls in sitemaps.items():
if issubclass(sitemap_cls, Sitemap):
initialised_sitemaps[name] = sitemap_cls(request.site, request)
initialised_sitemaps[name] = sitemap_cls(request)
else:
initialised_sitemaps[name] = sitemap_cls
return initialised_sitemaps