launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:fix-py3-personsubscriptions-comprehension into launchpad:master.

Commit message:
Fix excessive queries in Bug:+portlet-subscription

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/396238

Fixing the order of the nested comprehension in PersonSubscriptions._getDirectAndDuplicateSubscriptions caused TestBugPortletSubscribers.test_bug_portlet_subscription_query_count to fail.  Use load_referencing instead to load the relevant BugTask rows in one go.  With a little care, this now makes fewer queries than the previous code.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-py3-personsubscriptions-comprehension into launchpad:master.

diff --git a/lib/lp/bugs/browser/tests/test_bug_views.py b/lib/lp/bugs/browser/tests/test_bug_views.py
index 9382821..a45a647 100644
--- a/lib/lp/bugs/browser/tests/test_bug_views.py
+++ b/lib/lp/bugs/browser/tests/test_bug_views.py
@@ -360,11 +360,14 @@ class TestBugPortletSubscribers(TestCaseWithFactory):
             removeSecurityProxy(dupe).subscribe(user, dupe.owner)
         Store.of(bug).invalidate()
         with person_logged_in(user):
-            with StormStatementRecorder() as recorder:
+            launchbag = getUtility(IOpenLaunchBag)
+            launchbag.add(bug)
+            launchbag.add(bug.default_bugtask)
+            with StormStatementRecorder(tracebacks_if=True) as recorder:
                 view = create_initialized_view(
                     bug, name='+portlet-subscription', principal=user)
                 view.render()
-        self.assertThat(recorder, HasQueryCount(Equals(21)))
+        self.assertThat(recorder, HasQueryCount(Equals(14)))
 
 
 class TestBugSecrecyViews(TestCaseWithFactory):
diff --git a/lib/lp/bugs/model/personsubscriptioninfo.py b/lib/lp/bugs/model/personsubscriptioninfo.py
index 279c3a0..e66eb78 100644
--- a/lib/lp/bugs/model/personsubscriptioninfo.py
+++ b/lib/lp/bugs/model/personsubscriptioninfo.py
@@ -26,11 +26,15 @@ from lp.bugs.model.bug import (
     generate_subscription_with,
     )
 from lp.bugs.model.bugsubscription import BugSubscription
+from lp.bugs.model.bugtask import BugTask
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.model.distribution import Distribution
 from lp.registry.model.person import Person
 from lp.registry.model.product import Product
-from lp.services.database.bulk import load_related
+from lp.services.database.bulk import (
+    load_referencing,
+    load_related,
+    )
 
 
 @implementer(IRealSubscriptionInfo)
@@ -183,7 +187,7 @@ class PersonSubscriptions(object):
         # Preload bug owners, then all pillars.
         list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
             [bug.ownerID for bug in bugs]))
-        all_tasks = [task for bug in bugs for task in bug.bugtasks]
+        all_tasks = load_referencing(BugTask, bugs, ['bug_id'])
         load_related(Product, all_tasks, ['product_id'])
         load_related(Distribution, all_tasks, ['distribution_id'])
         for bug in bugs: