Fixed #14433 - replaced a thread-unsafe solution to #10235 introduced in [13980]

This patch also addresses sitemap code found in contrib/gis, which [13980]
did not.

Thanks to gabrielhurley for the initial patch.

Refs #10235, #14386

git-svn-id: http://code.djangoproject.com/svn/django/trunk@14141 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/gis/sitemaps/georss.py

@@ -36,12 +36,12 @@ class GeoRSSSitemap(Sitemap):
             else:
                 self.locations.append(section)
  
-    def get_urls(self, page=1):
+    def get_urls(self, page=1, site=None):
         """
         This method is overrridden so the appropriate `geo_format` attribute
         is placed on each URL element.
         """
-        urls = Sitemap.get_urls(self, page=page)
+        urls = Sitemap.get_urls(self, page=page, site=site)
         for url in urls: url['geo_format'] = 'georss'
         return urls
 

django/contrib/gis/sitemaps/kml.py

@@ -40,12 +40,12 @@ class KMLSitemap(Sitemap):
                 raise TypeError('KML Sources must be a model or a 3-tuple.')
         return kml_sources
 
-    def get_urls(self, page=1):
+    def get_urls(self, page=1, site=None):
         """
         This method is overrridden so the appropriate `geo_format` attribute
         is placed on each URL element.
         """
-        urls = Sitemap.get_urls(self, page=page)
+        urls = Sitemap.get_urls(self, page=page, site=site)
         for url in urls: url['geo_format'] = self.geo_format
         return urls
 

django/contrib/gis/sitemaps/views.py

@@ -46,12 +46,13 @@ def sitemap(request, sitemaps, section=None):
         maps = sitemaps.values()
 
     page = request.GET.get("p", 1)
+    current_site = get_current_site(request)
     for site in maps:
         try:
             if callable(site):
-                urls.extend(site().get_urls(page))
+                urls.extend(site().get_urls(page=page, site=current_site))
             else:
-                urls.extend(site.get_urls(page))
+                urls.extend(site.get_urls(page=page, site=current_site))
         except EmptyPage:
             raise Http404("Page %s empty" % page)
         except PageNotAnInteger:

django/contrib/sitemaps/__init__.py

@@ -1,5 +1,6 @@
-from django.contrib.sites.models import get_current_site
+from django.contrib.sites.models import Site, get_current_site
 from django.core import urlresolvers, paginator
+from django.core.exceptions import ImproperlyConfigured
 import urllib
 
 PING_URL = "http://www.google.com/webmasters/tools/ping"
@@ -60,11 +61,19 @@ class Sitemap(object):
         return self._paginator
     paginator = property(_get_paginator)
 
-    def get_urls(self, page=1):
-        current_site = get_current_site(self.request)
+    def get_urls(self, page=1, site=None):
+        if site is None:
+            if Site._meta.installed:
+                try:
+                    site = Site.objects.get_current()
+                except Site.DoesNotExist:
+                    pass
+            if site is None:
+                raise ImproperlyConfigured("In order to use Sitemaps you must either use the sites framework or pass in a Site or RequestSite object in your view code.")
+
         urls = []
         for item in self.paginator.page(page).object_list:
-            loc = "http://%s%s" % (current_site.domain, self.__get('location', item))
+            loc = "http://%s%s" % (site.domain, self.__get('location', item))
             priority = self.__get('priority', item, None)
             url_info = {
                 'location':   loc,
@@ -77,11 +86,8 @@ class Sitemap(object):
 
 class FlatPageSitemap(Sitemap):
     def items(self):
-        current_site = get_current_site(self.request)
-        if hasattr(current_site, "flatpage_set"):
-            return current_site.flatpage_set.filter(registration_required=False)
-        else:
-            return ()
+        current_site = Site.objects.get_current()
+        return current_site.flatpage_set.filter(registration_required=False)
 
 class GenericSitemap(Sitemap):
     priority = None

django/contrib/sitemaps/tests/basic.py

@@ -2,7 +2,9 @@ from datetime import date
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.contrib.flatpages.models import FlatPage
+from django.contrib.sitemaps import Sitemap
 from django.contrib.sites.models import Site
+from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase
 from django.utils.formats import localize
 from django.utils.translation import activate, deactivate
@@ -92,3 +94,22 @@ class SitemapTests(TestCase):
 <url><loc>http://testserver/location/</loc><lastmod>%s</lastmod><changefreq>never</changefreq><priority>0.5</priority></url>
 </urlset>
 """ % date.today().strftime('%Y-%m-%d'))
+
+    def test_sitemap_get_urls_no_site_1(self):
+        """
+        Check we get ImproperlyConfigured if we don't pass a site object to
+        Sitemap.get_urls and no Site objects exist
+        """
+        Site._meta.installed = True
+        Site.objects.all().delete()
+        self.assertRaises(ImproperlyConfigured, Sitemap().get_urls)
+
+    def test_sitemap_get_urls_no_site_2(self):
+        """
+        Check we get ImproperlyConfigured when we don't pass a site object to
+        Sitemap.get_urls if Site objects exists, but the sites framework is not
+        actually installed.
+        """
+        Site.objects.get_current()
+        Site._meta.installed = False
+        self.assertRaises(ImproperlyConfigured, Sitemap().get_urls)

django/contrib/sitemaps/views.py

@@ -32,13 +32,13 @@ def sitemap(request, sitemaps, section=None):
     else:
         maps = sitemaps.values()
     page = request.GET.get("p", 1)
+    current_site = get_current_site(request)
     for site in maps:
-        site.request = request
         try:
             if callable(site):
-                urls.extend(site().get_urls(page))
+                urls.extend(site().get_urls(page=page, site=current_site))
             else:
-                urls.extend(site.get_urls(page))
+                urls.extend(site.get_urls(page=page, site=current_site))
         except EmptyPage:
             raise Http404("Page %s empty" % page)
         except PageNotAnInteger: