Merge lp:~stevenk/launchpad/builder-history-better-query into lp:launchpad

[Steve Kowalik]

Rewrite the two (seperate) queries that are constructed by IBuildFarmJobSet.getBuildsForBuilder().

Rewrite the two (seperate) queries that are constructed by IBuildFarmJobSet.getBuildsForBuilder(). This, in conjunction with https://code.launchpad.net/~stevenk/launchpad/buildfarmjob-index/+merge/123675 pulls down the major query used on Builder:+history from 9 seconds to about 2 ms. For a further win, it reduces the line count of the function by a fair bit.

Also switch BuilderHistoryView to making use of StormRangeFactory.

[William Grant (wgrant)]

126 + TeamParticipation.teamID,
127 + where=(TeamParticipation.person == user),
128 + distinct=True))))

DISTINCT isn't necessary here. There's a unique constraint which implies it.

review: Approve (code)

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2012-01-02 11:21:14 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2012-09-11 06:20:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -9,18 +9,16 @@
     'BuildFarmJobOldDerived',
     ]
 
-
 import hashlib
 
 from lazr.delegates import delegates
 import pytz
 from storm.expr import (
-    And,
     Desc,
     LeftJoin,
+    Not,
     Or,
     Select,
-    Union,
     )
 from storm.locals import (
     Bool,
@@ -42,7 +40,6 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.errors import NotFoundError
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import (
     BuildFarmJobType,
     BuildStatus,
@@ -56,6 +53,7 @@
     ISpecificBuildFarmJobSource,
     )
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.enumcol import DBEnum
@@ -422,9 +420,9 @@
         from lp.buildmaster.model.packagebuild import PackageBuild
         from lp.soyuz.model.archive import Archive
 
-        extra_clauses = [BuildFarmJob.builder == builder_id]
+        clauses = [BuildFarmJob.builder == builder_id]
         if status is not None:
-            extra_clauses.append(BuildFarmJob.status == status)
+            clauses.append(BuildFarmJob.status == status)
 
         # We need to ensure that we don't include any private builds.
         # Currently only package builds can be private (via their
@@ -437,70 +435,26 @@
                 PackageBuild.build_farm_job == BuildFarmJob.id),
             ]
 
-        # STORM syntax has totally obfuscated this query and wasted
-        # THREE hours of my time converting perfectly good SQL syntax.  I'm
-        # really sorry if you're the poor sap who has to maintain this.
-
-        inner_privacy_query = (
-            Union(
-                Select(
-                    Archive.id,
-                    tables=(Archive,),
-                    where=(Archive._private == False)
-                    ),
-                Select(
-                    Archive.id,
-                    tables=(Archive,),
-                    where=And(
-                        Archive._private == True,
-                        Archive.ownerID.is_in(
-                            Select(
-                                TeamParticipation.teamID,
-                                where=(TeamParticipation.person == user),
-                                distinct=True
-                            )
-                        )
-                    )
-                )
-            )
-        )
-
         if user is None:
             # Anonymous requests don't get to see private builds at all.
-            extra_clauses.append(
-                Or(
-                    PackageBuild.id == None,
-                    PackageBuild.archive_id.is_in(
+            origin.append(
+                LeftJoin(Archive, Archive.id == PackageBuild.archive_id))
+            clauses.append(Or(PackageBuild.id == None, Not(Archive._private)))
+        elif not IPersonRoles(user).in_admin:
+            # Non-admin users see all public builds and the specific
+            # private builds to which they have access.
+            origin.append(
+                LeftJoin(Archive, Archive.id == PackageBuild.archive_id))
+            clauses.append(
+                Or(PackageBuild.id == None, Not(Archive._private),
+                    Archive.ownerID.is_in(
                         Select(
-                            Archive.id,
-                            tables=(Archive,),
-                            where=(Archive._private == False)
-                            )
-                        )
-                    )
-                )
-
-        elif user.inTeam(getUtility(ILaunchpadCelebrities).admin):
-            # Admins get to see everything.
-            pass
-        else:
-            # Everyone else sees all public builds and the
-            # specific private builds to which they have access.
-            extra_clauses.append(
-                Or(
-                    PackageBuild.id == None,
-                    PackageBuild.archive_id.is_in(inner_privacy_query)
-                    )
-                )
-
-        filtered_builds = IStore(BuildFarmJob).using(*origin).find(
-            BuildFarmJob, *extra_clauses)
-
-        filtered_builds.order_by(
-            Desc(BuildFarmJob.date_finished), BuildFarmJob.id)
-        filtered_builds.config(distinct=True)
-
-        return filtered_builds
+                            TeamParticipation.teamID,
+                            where=(TeamParticipation.person == user)))))
+
+        return IStore(BuildFarmJob).using(*origin).find(
+            BuildFarmJob, *clauses).order_by(
+                Desc(BuildFarmJob.date_finished), BuildFarmJob.id)
 
     def getByID(self, job_id):
         """See `IBuildfarmJobSet`."""

=== modified file 'lib/lp/soyuz/browser/builder.py'
--- lib/lp/soyuz/browser/builder.py	2012-05-23 10:28:40 +0000
+++ lib/lp/soyuz/browser/builder.py	2012-09-11 06:20:24 +0000
@@ -21,10 +21,7 @@
 import operator
 
 from lazr.restful.utils import smartquote
-from zope.app.form.browser import (
-    TextAreaWidget,
-    TextWidget,
-    )
+from zope.app.form.browser import TextWidget
 from zope.component import getUtility
 from zope.event import notify
 from zope.lifecycleevent import ObjectCreatedEvent
@@ -58,6 +55,7 @@
     StandardLaunchpadFacets,
     stepthrough,
     )
+from lp.services.webapp.batching import StormRangeFactory
 from lp.services.webapp.breadcrumb import Breadcrumb
 from lp.soyuz.browser.build import (
     BuildNavigationMixin,
@@ -296,6 +294,7 @@
 
     page_title = 'Build history'
     binary_only = False
+    range_factory = StormRangeFactory
 
     @property
     def label(self):