launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/dsp-questions-statement-death into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #876205 in Launchpad itself: "IQuestion.target generates excessive queries"
  https://bugs.launchpad.net/launchpad/+bug/876205

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/dsp-questions-statement-death/+merge/79519

This branch is a little badly named, but it stops IQuestion.target querying for SourcePackageName.name (every time!) and then passing that into IDistribution.getSourcePackage().
-- 
https://code.launchpad.net/~stevenk/launchpad/dsp-questions-statement-death/+merge/79519
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/dsp-questions-statement-death into lp:launchpad.

=== modified file 'lib/lp/answers/browser/tests/test_questiontarget.py'
--- lib/lp/answers/browser/tests/test_questiontarget.py	2011-08-16 06:17:12 +0000
+++ lib/lp/answers/browser/tests/test_questiontarget.py	2011-10-17 07:49:26 +0000
@@ -10,7 +10,9 @@
 from urllib import quote
 
 from BeautifulSoup import BeautifulSoup
+from storm.store import Store
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 from zope.traversing.browser import absoluteURL
 
 from lazr.restful.interfaces import (
@@ -34,6 +36,7 @@
     person_logged_in,
     TestCaseWithFactory,
     )
+from lp.testing.matchers import BrowsesWithQueryLimit
 from lp.testing.sampledata import ADMIN_EMAIL
 from lp.testing.views import (
     create_initialized_view,
@@ -64,6 +67,23 @@
         # This must not raise UnicodeEncodeError.
         self.assertIn(encoded_string, view.matching_faqs_url)
 
+    def test_query_count(self):
+        # SearchQuestionsView does not query for the target SPN every time.
+        owner = self.factory.makePerson()
+        distro = self.factory.makeDistribution()
+        removeSecurityProxy(distro).official_answers = True
+        dsp = self.factory.makeDistributionSourcePackage(
+            distribution=distro)
+        questions = []
+        for i in range(0, 5):
+            questions.append(self.factory.makeQuestion(
+                target=dsp, owner=owner))
+        # Empty the cache.
+        Store.of(questions[0]).invalidate()
+        browses_under_limit = BrowsesWithQueryLimit(
+            31, owner, view_name="+questions")
+        self.assertThat(dsp, browses_under_limit)
+
 
 class TestSearchQuestionsViewCanConfigureAnswers(TestCaseWithFactory):
 

=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py	2011-08-16 01:54:08 +0000
+++ lib/lp/answers/model/question.py	2011-10-17 07:49:26 +0000
@@ -1,8 +1,6 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0611,W0212
-
 """Question models."""
 
 __metaclass__ = type
@@ -225,8 +223,7 @@
         if self.product:
             return self.product
         elif self.sourcepackagename:
-            return self.distribution.getSourcePackage(
-                self.sourcepackagename.name)
+            return self.distribution.getSourcePackage(self.sourcepackagename)
         else:
             return self.distribution