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.

3 files changed, 86 insertions, 9 deletions

diff --git a/django/contrib/sitemaps/views.py b/django/contrib/sitemaps/views.py
index 660c283c53..948c7bc5c4 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 399cd35ae6..38a79a03f2 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 df423c7636..fcc5964d7a 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'),