don't evaluate querysets until the request cycle (bug 625735)

apps/addons/decorators.py

@@ -6,9 +6,9 @@
 from addons.models import Addon
 
 
-def addon_view(f, qs=Addon.objects):
+def addon_view(f, qs=Addon.objects.all):
     def wrapper(request, addon_id, *args, **kw):
-        get = lambda **kw: get_object_or_404(qs, **kw)
+        get = lambda **kw: get_object_or_404(qs(), **kw)
         if addon_id.isdigit():
             addon = get(id=addon_id)
             # Don't get in an infinite loop if addon.slug.isdigit().
@@ -24,4 +24,10 @@ def wrapper(request, addon_id, *args, **kw):
 
 
 def addon_view_factory(qs):
+    # Don't evaluate qs or the locale will get stuck on whatever the server
+    # starts with. The addon_view() decorator will call qs with no arguments
+    # before doing anything, so lambdas are ok.
+    # GOOD: Addon.objects.valid
+    # GOOD: lambda: Addon.objects.valid().filter(type=1)
+    # BAD: Addon.objects.valid()
     return functools.partial(addon_view, qs=qs)

apps/addons/tests/test_decorators.py

@@ -45,26 +45,26 @@ def test_404_by_slug(self):
             self.view(self.request, self.addon.slug + 'xx')
 
     def test_alternate_qs_301_by_id(self):
-        qs = Addon.objects.filter(type=1)
+        qs = lambda: Addon.objects.filter(type=1)
         view = dec.addon_view_factory(qs=qs)(self.func)
         r = view(self.request, str(self.addon.id))
         eq_(r.status_code, 301)
         eq_(r['Location'], self.slug_path)
 
     def test_alternate_qs_200_by_slug(self):
-        qs = Addon.objects.filter(type=1)
+        qs = lambda: Addon.objects.filter(type=1)
         view = dec.addon_view_factory(qs=qs)(self.func)
         r = view(self.request, self.addon.slug)
         eq_(r, mock.sentinel.OK)
 
     def test_alternate_qs_404_by_id(self):
-        qs = Addon.objects.filter(type=2)
+        qs = lambda: Addon.objects.filter(type=2)
         view = dec.addon_view_factory(qs=qs)(self.func)
         with self.assertRaises(http.Http404):
             view(self.request, str(self.addon.id))
 
     def test_alternate_qs_404_by_slug(self):
-        qs = Addon.objects.filter(type=2)
+        qs = lambda: Addon.objects.filter(type=2)
         view = dec.addon_view_factory(qs=qs)(self.func)
         with self.assertRaises(http.Http404):
             view(self.request, self.addon.slug)

apps/addons/views.py

@@ -38,7 +38,7 @@
 from .decorators import addon_view_factory
 
 log = commonware.log.getLogger('z.addons')
-addon_view = addon_view_factory(qs=Addon.objects.valid())
+addon_view = addon_view_factory(qs=Addon.objects.valid)
 
 
 def author_addon_clicked(f):

apps/discovery/views.py

@@ -22,7 +22,7 @@
 from .forms import DiscoveryModuleForm
 from .modules import registry as module_registry
 
-addon_view = addon_view_factory(Addon.objects.valid())
+addon_view = addon_view_factory(Addon.objects.valid)
 
 
 def pane(request, version, platform):

apps/reviews/views.py

@@ -16,7 +16,7 @@
 from . import forms
 
 log = commonware.log.getLogger('z.reviews')
-addon_view = addon_view_factory(qs=Addon.objects.valid())
+addon_view = addon_view_factory(qs=Addon.objects.valid)
 
 
 def flag_context():

apps/stats/views.py

@@ -174,7 +174,7 @@ def check_stats_permission(request, addon, for_contributions=False):
     raise PermissionDenied
 
 
-@addon_view_factory(Addon.objects.valid())
+@addon_view_factory(Addon.objects.valid)
 def stats_report(request, addon, report):
     check_stats_permission(request, addon)
     stats_base_url = reverse('stats.overview', args=[addon.slug])

apps/versions/views.py

@@ -19,7 +19,7 @@
 # The version detail page redirects to the version within pagination, so we
 # need to enforce the number of versions per page.
 PER_PAGE = 30
-addon_view = addon_view_factory(Addon.objects.valid())
+addon_view = addon_view_factory(Addon.objects.valid)
 
 
 @addon_view
@@ -69,7 +69,8 @@ def download_file(request, file_id, type=None):
     return response
 
 
-@addon_view_factory(Addon.objects.filter(_current_version__isnull=False))
+guard = lambda: Addon.objects.filter(_current_version__isnull=False)
+@addon_view_factory(guard)
 def download_latest(request, addon, type='xpi', platform=None):
     platforms = [amo.PLATFORM_ALL.id]
     if platform is not None and int(platform) in amo.PLATFORMS: