launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bugtaskflat-by-default-1 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bugtaskflat-by-default-1/+merge/105172

Port a few more things to BugTaskFlat, mostly so they respect the new sharing schema. But they also end up much faster as a side-effect.
-- 
https://code.launchpad.net/~wgrant/launchpad/bugtaskflat-by-default-1/+merge/105172
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bugtaskflat-by-default-1 into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-05-02 23:15:38 +0000
+++ lib/lp/bugs/model/bug.py	2012-05-09 08:41:19 +0000
@@ -2052,7 +2052,7 @@
         to be made to the queries which screen for privacy.  See
         Bug.searchAsUser and BugTask.get_bug_privacy_filter_with_decorator.
         """
-        from lp.bugs.model.bugtasksearch import get_bug_privacy_filter
+        from lp.bugs.interfaces.bugtask import BugTaskSearchParams
 
         if not self.private:
             # This is a public bug.
@@ -2064,11 +2064,8 @@
         if user.id in self._known_viewers:
             return True
 
-        filter = get_bug_privacy_filter(user)
-        store = Store.of(self)
-        store.flush()
-        if (not filter or
-            not store.find(Bug, Bug.id == self.id, filter).is_empty()):
+        params = BugTaskSearchParams(user=user, bug=self)
+        if not getUtility(IBugTaskSet).search(params).is_empty():
             self._known_viewers.add(user.id)
             return True
         return False

=== modified file 'lib/lp/bugs/model/bugbranch.py'
--- lib/lp/bugs/model/bugbranch.py	2012-02-08 22:52:37 +0000
+++ lib/lp/bugs/model/bugbranch.py	2012-05-09 08:41:19 +0000
@@ -70,18 +70,18 @@
     def getBranchesWithVisibleBugs(self, branches, user):
         """See `IBugBranchSet`."""
         # Avoid circular imports.
-        from lp.bugs.model.bug import Bug
+        from lp.bugs.model.bugtaskflat import BugTaskFlat
         from lp.bugs.model.bugtasksearch import get_bug_privacy_filter
-        
+
         branch_ids = [branch.id for branch in branches]
         if not branch_ids:
             return []
 
-        visible = get_bug_privacy_filter(user) or True
+        visible = get_bug_privacy_filter(user, use_flat=True) or True
         return IStore(BugBranch).find(
             BugBranch.branchID,
             BugBranch.branch_id.is_in(branch_ids),
-            Bug.id == BugBranch.bugID,
+            BugTaskFlat.bug_id == BugBranch.bugID,
             visible).config(distinct=True)
 
     def getBugBranchesForBugTasks(self, tasks):

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2012-04-26 07:58:38 +0000
+++ lib/lp/bugs/model/bugtask.py	2012-05-09 08:41:19 +0000
@@ -23,8 +23,14 @@
 
 from collections import defaultdict
 import datetime
-from itertools import chain
-from operator import attrgetter
+from itertools import (
+    chain,
+    repeat,
+    )
+from operator import (
+    attrgetter,
+    itemgetter,
+    )
 import re
 
 from lazr.lifecycle.event import (
@@ -40,7 +46,8 @@
     StringCol,
     )
 from storm.expr import (
-    And,
+    Cast,
+    Count,
     Join,
     Or,
     SQL,
@@ -1900,29 +1907,16 @@
     def getOpenBugTasksPerProduct(self, user, products):
         """See `IBugTaskSet`."""
         # Local import of Bug to avoid import loop.
-        from lp.bugs.model.bug import Bug
+        from lp.bugs.model.bugtaskflat import BugTaskFlat
         from lp.bugs.model.bugtasksearch import get_bug_privacy_filter
 
-        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-        origin = [
-            Bug,
-            Join(BugTask, BugTask.bug == Bug.id),
-            ]
-
-        product_ids = [product.id for product in products]
-        conditions = And(
-            BugTask._status.is_in(DB_UNRESOLVED_BUGTASK_STATUSES),
-            Bug.duplicateof == None,
-            BugTask.productID.is_in(product_ids))
-
-        privacy_filter = get_bug_privacy_filter(user)
-        if privacy_filter != '':
-            conditions = And(conditions, privacy_filter)
-        result = store.using(*origin).find(
-            (BugTask.productID, SQL('COUNT(*)')),
-            conditions)
-
-        result = result.group_by(BugTask.productID)
+        result = IStore(BugTaskFlat).find(
+            (BugTaskFlat.product_id, Count()),
+            BugTaskFlat.status.is_in(DB_UNRESOLVED_BUGTASK_STATUSES),
+            BugTaskFlat.duplicateof == None,
+            BugTaskFlat.product_id.is_in(product.id for product in products),
+            SQL(get_bug_privacy_filter(user, use_flat=True) or True)
+            ).group_by(BugTaskFlat.product_id)
         # The result will return a list of product ids and counts,
         # which will be converted into key-value pairs in the dictionary.
         return dict(result)
@@ -1943,6 +1937,7 @@
 
         See `IBugTask.getBugCountsForPackages` for more information.
         """
+        from lp.bugs.model.bugtaskflat import BugTaskFlat
         from lp.bugs.model.bugtasksearch import get_bug_privacy_filter
 
         packages = [
@@ -1951,70 +1946,42 @@
         package_name_ids = [
             package.sourcepackagename.id for package in packages]
 
-        open_bugs_cond = (
-            'BugTask.status IN %s' %
-            sqlvalues(DB_UNRESOLVED_BUGTASK_STATUSES))
-
-        sum_template = "SUM(CASE WHEN %s THEN 1 ELSE 0 END) AS %s"
-        sums = [
-            sum_template % (open_bugs_cond, 'open_bugs'),
-            sum_template % (
-                'BugTask.importance = %s' %
-                sqlvalues(BugTaskImportance.CRITICAL), 'open_critical_bugs'),
-            sum_template % (
-                'BugTask.assignee IS NULL', 'open_unassigned_bugs'),
-            sum_template % (
-                'BugTask.status = %s' %
-                sqlvalues(BugTaskStatus.INPROGRESS), 'open_inprogress_bugs'),
-            sum_template % (
-                'BugTask.importance = %s' %
-                sqlvalues(BugTaskImportance.HIGH), 'open_high_bugs'),
-            ]
-
-        conditions = [
-            'Bug.id = BugTask.bug',
-            open_bugs_cond,
-            'BugTask.sourcepackagename IN %s' % sqlvalues(package_name_ids),
-            'BugTask.distribution = %s' % sqlvalues(distribution),
-            'Bug.duplicateof is NULL',
-            ]
-        privacy_filter = get_bug_privacy_filter(user)
-        if privacy_filter:
-            conditions.append(privacy_filter)
-
-        query = """SELECT BugTask.distribution,
-                          BugTask.sourcepackagename,
-                          %(sums)s
-                   FROM BugTask, Bug
-                   WHERE %(conditions)s
-                   GROUP BY BugTask.distribution, BugTask.sourcepackagename"""
-        cur = cursor()
-        cur.execute(query % dict(
-            sums=', '.join(sums), conditions=' AND '.join(conditions)))
-        distribution_set = getUtility(IDistributionSet)
-        sourcepackagename_set = getUtility(ISourcePackageNameSet)
+        # The count of each package's open bugs matching each predicate
+        # will be returned in the dict under the given name.
+        sumexprs = [
+            ('open',
+             BugTaskFlat.status.is_in(DB_UNRESOLVED_BUGTASK_STATUSES)),
+            ('open_critical',
+             BugTaskFlat.importance == BugTaskImportance.CRITICAL),
+            ('open_unassigned', BugTaskFlat.assignee == None),
+            ('open_inprogress',
+             BugTaskFlat.status == BugTaskStatus.INPROGRESS),
+            ('open_high', BugTaskFlat.importance == BugTaskImportance.HIGH),
+            ]
+
+        result = IStore(BugTaskFlat).find(
+            (BugTaskFlat.distribution_id, BugTaskFlat.sourcepackagename_id)
+            + tuple(Sum(Cast(expr[1], 'integer')) for expr in sumexprs),
+            BugTaskFlat.status.is_in(DB_UNRESOLVED_BUGTASK_STATUSES),
+            BugTaskFlat.sourcepackagename_id.is_in(package_name_ids),
+            BugTaskFlat.distribution == distribution,
+            BugTaskFlat.duplicateof == None,
+            SQL(get_bug_privacy_filter(user, use_flat=True) or True)
+            ).group_by(
+                BugTaskFlat.distribution_id, BugTaskFlat.sourcepackagename_id)
+
+        # Map the returned counts back to their names and throw them in
+        # the dict.
         packages_with_bugs = set()
         counts = []
-        for (distro_id, spn_id, open_bugs,
-             open_critical_bugs, open_unassigned_bugs,
-             open_inprogress_bugs,
-             open_high_bugs) in shortlist(cur.fetchall()):
-            distribution = distribution_set.get(distro_id)
-            sourcepackagename = sourcepackagename_set.get(spn_id)
+        for row in result:
+            distribution = getUtility(IDistributionSet).get(row[0])
+            sourcepackagename = getUtility(ISourcePackageNameSet).get(row[1])
             source_package = distribution.getSourcePackage(sourcepackagename)
-            # XXX: Bjorn Tillenius 2006-12-15:
-            # Add a tuple instead of the distribution package
-            # directly, since DistributionSourcePackage doesn't define a
-            # __hash__ method.
             packages_with_bugs.add((distribution, sourcepackagename))
             package_counts = dict(
                 package=source_package,
-                open=open_bugs,
-                open_critical=open_critical_bugs,
-                open_unassigned=open_unassigned_bugs,
-                open_inprogress=open_inprogress_bugs,
-                open_high=open_high_bugs,
-                )
+                **zip(map(itemgetter(0), sumexprs), row[2:]))
             counts.append(package_counts)
 
         # Only packages with open bugs were included in the query. Let's
@@ -2026,8 +1993,7 @@
                 packages_with_bugs):
             package_counts = dict(
                 package=distribution.getSourcePackage(sourcepackagename),
-                open=0, open_critical=0, open_unassigned=0,
-                open_inprogress=0, open_high=0)
+                **zip(map(itemgetter(0), sumexprs), repeat(0)))
             counts.append(package_counts)
 
         return counts

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2012-05-04 05:43:28 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2012-05-09 08:41:19 +0000
@@ -1379,9 +1379,10 @@
 
 # Privacy restrictions
 
-def get_bug_privacy_filter(user, private_only=False):
+def get_bug_privacy_filter(user, private_only=False, use_flat=False):
     """An SQL filter for search results that adds privacy-awareness."""
-    return _get_bug_privacy_filter_with_decorator(user, private_only)[0]
+    return _get_bug_privacy_filter_with_decorator(
+        user, private_only, use_flat)[0]
 
 
 def _nocache_bug_decorator(obj):