From 8f5533ab250df07ea84f98d39808806e282468a5 Mon Sep 17 00:00:00 2001 From: Julian Bez Date: Tue, 23 Jul 2013 16:25:21 +0200 Subject: [PATCH] Fixed #20793 -- Added Last-Modified header to sitemaps. --- django/contrib/sitemaps/__init__.py | 12 +++++++++- django/contrib/sitemaps/tests/test_http.py | 15 ++++++++++++ django/contrib/sitemaps/tests/urls/http.py | 28 ++++++++++++++++++++++ django/contrib/sitemaps/views.py | 12 ++++++++-- docs/ref/contrib/sitemaps.txt | 9 +++++++ docs/releases/1.7.txt | 6 +++++ 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/django/contrib/sitemaps/__init__.py b/django/contrib/sitemaps/__init__.py index 1acae8296cb..627813a33e5 100644 --- a/django/contrib/sitemaps/__init__.py +++ b/django/contrib/sitemaps/__init__.py @@ -86,17 +86,27 @@ class Sitemap(object): domain = site.domain urls = [] + latest_lastmod = None + all_items_lastmod = True # track if all items have a lastmod for item in self.paginator.page(page).object_list: loc = "%s://%s%s" % (protocol, domain, self.__get('location', item)) priority = self.__get('priority', item, None) + lastmod = self.__get('lastmod', item, None) + if all_items_lastmod: + all_items_lastmod = lastmod is not None + if (all_items_lastmod and + (latest_lastmod is None or lastmod > latest_lastmod)): + latest_lastmod = lastmod url_info = { 'item': item, 'location': loc, - 'lastmod': self.__get('lastmod', item, None), + 'lastmod': lastmod, 'changefreq': self.__get('changefreq', item, None), 'priority': str(priority if priority is not None else ''), } urls.append(url_info) + if all_items_lastmod: + self.latest_lastmod = latest_lastmod return urls class FlatPageSitemap(Sitemap): diff --git a/django/contrib/sitemaps/tests/test_http.py b/django/contrib/sitemaps/tests/test_http.py index f68f3458156..c870b442de0 100644 --- a/django/contrib/sitemaps/tests/test_http.py +++ b/django/contrib/sitemaps/tests/test_http.py @@ -77,6 +77,21 @@ class HTTPSitemapTests(SitemapTestsBase): """ % (self.base_url, date.today()) self.assertXMLEqual(response.content.decode('utf-8'), expected_content) + def test_sitemap_last_modified(self): + "Tests that Last-Modified header is set correctly" + response = self.client.get('/lastmod/sitemap.xml') + self.assertEqual(response['Last-Modified'], 'Wed, 13 Mar 2013 10:00:00 GMT') + + def test_sitemap_last_modified_missing(self): + "Tests that Last-Modified header is missing when sitemap has no lastmod" + response = self.client.get('/generic/sitemap.xml') + self.assertFalse(response.has_header('Last-Modified')) + + def test_sitemap_last_modified_mixed(self): + "Tests that Last-Modified header is omitted when lastmod not on all items" + response = self.client.get('/lastmod-mixed/sitemap.xml') + self.assertFalse(response.has_header('Last-Modified')) + @skipUnless(settings.USE_I18N, "Internationalization is not enabled") @override_settings(USE_L10N=True) def test_localized_priority(self): diff --git a/django/contrib/sitemaps/tests/urls/http.py b/django/contrib/sitemaps/tests/urls/http.py index a8b804fd4b8..6721d72b81d 100644 --- a/django/contrib/sitemaps/tests/urls/http.py +++ b/django/contrib/sitemaps/tests/urls/http.py @@ -15,10 +15,36 @@ class SimpleSitemap(Sitemap): def items(self): return [object()] + +class FixedLastmodSitemap(SimpleSitemap): + lastmod = datetime(2013, 3, 13, 10, 0, 0) + + +class FixedLastmodMixedSitemap(Sitemap): + changefreq = "never" + priority = 0.5 + location = '/location/' + loop = 0 + + def items(self): + o1 = TestModel() + o1.lastmod = datetime(2013, 3, 13, 10, 0, 0) + o2 = TestModel() + return [o1, o2] + + simple_sitemaps = { 'simple': SimpleSitemap, } +fixed_lastmod_sitemaps = { + 'fixed-lastmod': FixedLastmodSitemap, +} + +fixed_lastmod__mixed_sitemaps = { + 'fixed-lastmod-mixed': FixedLastmodMixedSitemap, +} + generic_sitemaps = { 'generic': GenericSitemap({'queryset': TestModel.objects.all()}), } @@ -36,6 +62,8 @@ urlpatterns = patterns('django.contrib.sitemaps.views', (r'^simple/sitemap\.xml$', 'sitemap', {'sitemaps': simple_sitemaps}), (r'^simple/custom-sitemap\.xml$', 'sitemap', {'sitemaps': simple_sitemaps, 'template_name': 'custom_sitemap.xml'}), + (r'^lastmod/sitemap\.xml$', 'sitemap', {'sitemaps': fixed_lastmod_sitemaps}), + (r'^lastmod-mixed/sitemap\.xml$', 'sitemap', {'sitemaps': fixed_lastmod__mixed_sitemaps}), (r'^generic/sitemap\.xml$', 'sitemap', {'sitemaps': generic_sitemaps}), (r'^flatpages/sitemap\.xml$', 'sitemap', {'sitemaps': flatpage_sitemaps}), url(r'^cached/index\.xml$', cache_page(1)(views.index), diff --git a/django/contrib/sitemaps/views.py b/django/contrib/sitemaps/views.py index 7a94fb355d1..14983acea91 100644 --- a/django/contrib/sitemaps/views.py +++ b/django/contrib/sitemaps/views.py @@ -1,3 +1,4 @@ +from calendar import timegm from functools import wraps from django.contrib.sites.models import get_current_site @@ -6,6 +7,7 @@ from django.core.paginator import EmptyPage, PageNotAnInteger from django.http import Http404 from django.template.response import TemplateResponse from django.utils import six +from django.utils.http import http_date def x_robots_tag(func): @wraps(func) @@ -64,5 +66,11 @@ def sitemap(request, sitemaps, section=None, raise Http404("Page %s empty" % page) except PageNotAnInteger: raise Http404("No page '%s'" % page) - return TemplateResponse(request, template_name, {'urlset': urls}, - content_type=content_type) + 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 + # ConditionalGetMiddleware is able to send 304 NOT MODIFIED + response['Last-Modified'] = http_date( + timegm(site.latest_lastmod.utctimetuple())) + return response diff --git a/docs/ref/contrib/sitemaps.txt b/docs/ref/contrib/sitemaps.txt index 4467ed3a6ef..7b53fc8c129 100644 --- a/docs/ref/contrib/sitemaps.txt +++ b/docs/ref/contrib/sitemaps.txt @@ -178,6 +178,15 @@ Sitemap class reference representing the last-modified date/time for *every* object returned by :attr:`~Sitemap.items()`. + .. versionadded:: 1.7 + + If all items in a sitemap have a :attr:`~Sitemap.lastmod`, the sitemap + generated by :func:`views.sitemap` will have a ``Last-Modified`` + header equal to the latest ``lastmod``. You can activate the + :class:`~django.middleware.http.ConditionalGetMiddleware` to make + Django respond appropriately to requests with an ``If-Modified-Since`` + header which will prevent sending the sitemap if it hasn't changed. + .. attribute:: Sitemap.changefreq **Optional.** Either a method or attribute. diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index f0efe4f5591..0951cd30586 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -95,6 +95,12 @@ Minor features * The :djadminopt:`--no-color` option for ``django-admin.py`` allows you to disable the colorization of management command output. +* The :mod:`sitemap framework` now makes use of + :attr:`~django.contrib.sitemaps.Sitemap.lastmod` to set a ``Last-Modified`` + header in the response. This makes it possible for the + :class:`~django.middleware.http.ConditionalGetMiddleware` to handle + conditional ``GET`` requests for sitemaps which set ``lastmod``. + Backwards incompatible changes in 1.7 =====================================