From 002a4f72c46458514d97f7e54d4fd9416d89899d Mon Sep 17 00:00:00 2001 From: Alexander Rudakov Date: Sat, 26 Dec 2015 21:01:25 +0300 Subject: [PATCH] Fixed #25989 -- Corrected sitemap's Last-Modified header to use the latest lastmod of all sitemaps. Previously, the lastmod of the last sitemap was always used. All sitemaps are required to have a lastmod. --- django/contrib/sitemaps/views.py | 24 ++++++++++++------- tests/sitemaps_tests/test_http.py | 32 +++++++++++++++++++++++++ tests/sitemaps_tests/urls/http.py | 39 +++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/django/contrib/sitemaps/views.py b/django/contrib/sitemaps/views.py index 660c283c536..948c7bc5c49 100644 --- a/django/contrib/sitemaps/views.py +++ b/django/contrib/sitemaps/views.py @@ -57,6 +57,8 @@ def sitemap(request, sitemaps, section=None, maps = sitemaps.values() page = request.GET.get("p", 1) + lastmod = None + all_sites_lastmod = True urls = [] for site in maps: try: @@ -64,20 +66,24 @@ def sitemap(request, sitemaps, section=None, site = site() urls.extend(site.get_urls(page=page, site=req_site, protocol=req_protocol)) + if all_sites_lastmod: + site_lastmod = getattr(site, 'latest_lastmod', None) + if site_lastmod is not None: + site_lastmod = ( + site_lastmod.utctimetuple() if isinstance(site_lastmod, datetime.datetime) + else site_lastmod.timetuple() + ) + lastmod = site_lastmod if lastmod is None else max(lastmod, site_lastmod) + else: + all_sites_lastmod = False except EmptyPage: raise Http404("Page %s empty" % page) except PageNotAnInteger: raise Http404("No page '%s'" % page) response = TemplateResponse(request, template_name, {'urlset': urls}, content_type=content_type) - if hasattr(site, 'latest_lastmod'): - # if latest_lastmod is defined for site, set header so as + if all_sites_lastmod and lastmod is not None: + # if lastmod is defined for all sites, set header so as # ConditionalGetMiddleware is able to send 304 NOT MODIFIED - lastmod = site.latest_lastmod - response['Last-Modified'] = http_date( - timegm( - lastmod.utctimetuple() if isinstance(lastmod, datetime.datetime) - else lastmod.timetuple() - ) - ) + response['Last-Modified'] = http_date(timegm(lastmod)) return response diff --git a/tests/sitemaps_tests/test_http.py b/tests/sitemaps_tests/test_http.py index 399cd35ae68..38a79a03f2c 100644 --- a/tests/sitemaps_tests/test_http.py +++ b/tests/sitemaps_tests/test_http.py @@ -110,6 +110,38 @@ class HTTPSitemapTests(SitemapTestsBase): response = self.client.get('/lastmod-mixed/sitemap.xml') self.assertFalse(response.has_header('Last-Modified')) + def test_sitemaps_lastmod_mixed_ascending_last_modified_missing(self): + """ + The Last-Modified header is omitted when lastmod isn't found in all + sitemaps. Test sitemaps are sorted by lastmod in ascending order. + """ + response = self.client.get('/lastmod-sitemaps/mixed-ascending.xml') + self.assertFalse(response.has_header('Last-Modified')) + + def test_sitemaps_lastmod_mixed_descending_last_modified_missing(self): + """ + The Last-Modified header is omitted when lastmod isn't found in all + sitemaps. Test sitemaps are sorted by lastmod in descending order. + """ + response = self.client.get('/lastmod-sitemaps/mixed-descending.xml') + self.assertFalse(response.has_header('Last-Modified')) + + def test_sitemaps_lastmod_ascending(self): + """ + The Last-Modified header is set to the most recent sitemap lastmod. + Test sitemaps are sorted by lastmod in ascending order. + """ + response = self.client.get('/lastmod-sitemaps/ascending.xml') + self.assertEqual(response['Last-Modified'], 'Sat, 20 Apr 2013 05:00:00 GMT') + + def test_sitemaps_lastmod_descending(self): + """ + The Last-Modified header is set to the most recent sitemap lastmod. + Test sitemaps are sorted by lastmod in descending order. + """ + response = self.client.get('/lastmod-sitemaps/descending.xml') + self.assertEqual(response['Last-Modified'], 'Sat, 20 Apr 2013 05:00:00 GMT') + @skipUnless(settings.USE_I18N, "Internationalization is not enabled") @override_settings(USE_L10N=True) def test_localized_priority(self): diff --git a/tests/sitemaps_tests/urls/http.py b/tests/sitemaps_tests/urls/http.py index df423c76366..fcc5964d7ac 100644 --- a/tests/sitemaps_tests/urls/http.py +++ b/tests/sitemaps_tests/urls/http.py @@ -1,3 +1,4 @@ +from collections import OrderedDict from datetime import date, datetime from django.conf.urls import url @@ -55,6 +56,10 @@ class FixedLastmodMixedSitemap(Sitemap): return [o1, o2] +class FixedNewerLastmodSitemap(SimpleSitemap): + lastmod = datetime(2013, 4, 20, 5, 0, 0) + + class DateSiteMap(SimpleSitemap): lastmod = date(2013, 3, 13) @@ -87,6 +92,28 @@ fixed_lastmod__mixed_sitemaps = { 'fixed-lastmod-mixed': FixedLastmodMixedSitemap, } +sitemaps_lastmod_mixed_ascending = OrderedDict([ + ('no-lastmod', EmptySitemap), + ('lastmod', FixedLastmodSitemap), +]) + +sitemaps_lastmod_mixed_descending = OrderedDict([ + ('lastmod', FixedLastmodSitemap), + ('no-lastmod', EmptySitemap), +]) + +sitemaps_lastmod_ascending = OrderedDict([ + ('date', DateSiteMap), + ('datetime', FixedLastmodSitemap), + ('datetime-newer', FixedNewerLastmodSitemap), +]) + +sitemaps_lastmod_descending = OrderedDict([ + ('datetime-newer', FixedNewerLastmodSitemap), + ('datetime', FixedLastmodSitemap), + ('date', DateSiteMap), +]) + generic_sitemaps = { 'generic': GenericSitemap({'queryset': TestModel.objects.all()}), } @@ -123,6 +150,18 @@ urlpatterns = [ url(r'^lastmod/tz-sitemap.xml$', views.sitemap, {'sitemaps': {'tz-sitemap': TimezoneSiteMap}}, name='django.contrib.sitemaps.views.sitemap'), + url(r'^lastmod-sitemaps/mixed-ascending.xml$', views.sitemap, + {'sitemaps': sitemaps_lastmod_mixed_ascending}, + name='django.contrib.sitemaps.views.sitemap'), + url(r'^lastmod-sitemaps/mixed-descending.xml$', views.sitemap, + {'sitemaps': sitemaps_lastmod_mixed_descending}, + name='django.contrib.sitemaps.views.sitemap'), + url(r'^lastmod-sitemaps/ascending.xml$', views.sitemap, + {'sitemaps': sitemaps_lastmod_ascending}, + name='django.contrib.sitemaps.views.sitemap'), + url(r'^lastmod-sitemaps/descending.xml$', views.sitemap, + {'sitemaps': sitemaps_lastmod_descending}, + name='django.contrib.sitemaps.views.sitemap'), url(r'^generic/sitemap\.xml$', views.sitemap, {'sitemaps': generic_sitemaps}, name='django.contrib.sitemaps.views.sitemap'),