Merge lp:~stevenk/launchpad/stormier-getbuildsforbuilder into lp:launchpad

[Steve Kowalik]

Switch BinaryPackageBuildSet.getBuildsByBuilder() and friends to Storm.

Switch BinaryPackageBuildSet.getBuildsByBuilder() and friends to Storm. I took one look at the work to convert BinaryPackageBuildSet.getBuildsByArchIds() to Storm (since it is a callsite of handleOptionalParamsForBuildQueries()), and ran screaming. However, it already used Storm to make the query so that it could use DRS, and Storm doesn't seem to mind mixing straight clauses and quoted ones.

I have cleaned up the pylint garbage and trimmed a bit of whitespace.

[William Grant (wgrant)]

78 +def get_archive_privacy_filter(user):
79 + return [
80 + Not(Archive._private),
81 + Archive.ownerID.is_in(
82 + Select(
83 + TeamParticipation.teamID,
84 + where=(TeamParticipation.person == user)))]

The indentation here is wrong, and it might be better to just say Or() rather than returning a list.

174 + clauses.extend(
175 + [BinaryPackageBuild.package_build == PackageBuild.id,
176 + PackageBuild.build_farm_job == BuildFarmJob.id])

The opening square bracket should probably be on the previous line.

221 - AND SourcepackageName.name LIKE '%%%%' || %s || '%%%%'
222 - ''' % quote_like(name))
223 + clauses.append(SourcePackageName.name.like(name))

These aren't equivalent. You need to use quote_like and wrap it in % to get a substring match; see Distribution.searchSourcePackageCaches's LIKE stuff for an example of hacking around Storm. There's also another instance of this in the following block.

274 + if user is None:
275 + clauses.append(Not(Archive._private))
276 + elif not IPersonRoles(user).in_admin:
277 + clauses.append(*get_archive_privacy_filter(user))

Shouldn't this be in get_archive_privacy_filter?

279 + clauses.append(BuildFarmJob.builder_id == builder_id)

Can you put this in the initial clauses?

288 - queries = []
289 - clauseTables = []
290 + clauses = []
291 + origin = [PackageBuild]

Why the new table?

review: Approve (code)

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2012-09-11 06:18:18 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2012-09-26 04:19:22 +0000
@@ -16,9 +16,7 @@
 from storm.expr import (
     Desc,
     LeftJoin,
-    Not,
     Or,
-    Select,
     )
 from storm.locals import (
     Bool,
@@ -53,8 +51,6 @@
     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
 from lp.services.database.lpstorm import (
@@ -418,9 +414,12 @@
         """See `IBuildFarmJobSet`."""
         # Imported here to avoid circular imports.
         from lp.buildmaster.model.packagebuild import PackageBuild
-        from lp.soyuz.model.archive import Archive
+        from lp.soyuz.model.archive import (
+            Archive, get_archive_privacy_filter)
 
-        clauses = [BuildFarmJob.builder == builder_id]
+        clauses = [
+            BuildFarmJob.builder == builder_id,
+            Or(PackageBuild.id == None, get_archive_privacy_filter(user))]
         if status is not None:
             clauses.append(BuildFarmJob.status == status)
 
@@ -433,25 +432,9 @@
             LeftJoin(
                 PackageBuild,
                 PackageBuild.build_farm_job == BuildFarmJob.id),
+            LeftJoin(Archive, Archive.id == PackageBuild.archive_id),
             ]
 
-        if user is None:
-            # Anonymous requests don't get to see private builds at all.
-            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(
-                            TeamParticipation.teamID,
-                            where=(TeamParticipation.person == user)))))
-
         return IStore(BuildFarmJob).using(*origin).find(
             BuildFarmJob, *clauses).order_by(
                 Desc(BuildFarmJob.date_finished), BuildFarmJob.id)

=== modified file 'lib/lp/registry/interfaces/distroseries.py'
--- lib/lp/registry/interfaces/distroseries.py	2012-09-07 05:13:57 +0000
+++ lib/lp/registry/interfaces/distroseries.py	2012-09-26 04:19:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0211,E0213
-
 """Interfaces including and related to IDistroSeries."""
 
 __metaclass__ = type
@@ -380,9 +378,6 @@
                 on clients, which requires downloading Packages files for
                 multiple architectures.""")))
 
-    def priorReleasedSeries():
-        """Prior series *by date* from the same distribution."""
-
     main_archive = exported(
         Reference(
             Interface,  # Really IArchive, see below for circular import fix.
@@ -1071,24 +1066,8 @@
         released == None will do no filtering on status.
         """
 
-    def priorReleasedSeries(distribution, prior_to_date):
-        """Find distroseries for the supplied distro  released before a
-        certain date.
-
-        :param distribution: An `IDistribution` in which to search for its
-            series.
-        :param prior_to_date: A `datetime`
-
-        :return: `IResultSet` of `IDistroSeries` that were released before
-            prior_to_date, ordered in increasing order of age.
-        """
-
 
 @error_status(httplib.BAD_REQUEST)
 class DerivationError(Exception):
     """Raised when there is a problem deriving a distroseries."""
     _message_prefix = "Error deriving distro series"
-
-
-# Monkey patch for circular import avoidance done in
-# _schema_circular_imports.py

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2012-08-08 05:36:44 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2012-09-26 04:19:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0611,W0212
-
 """Classes to represent source packages in a distribution."""
 
 __metaclass__ = type
@@ -35,7 +33,6 @@
 from zope.interface import implements
 
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
-from lp.bugs.model.bug import BugSet
 from lp.bugs.model.bugtarget import BugTargetBase
 from lp.bugs.model.bugtask import BugTask
 from lp.bugs.model.structuralsubscription import (

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2012-09-11 19:14:41 +0000
+++ lib/lp/registry/model/distroseries.py	2012-09-26 04:19:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0611,W0212
-
 """Database classes for a distribution series."""
 
 __metaclass__ = type
@@ -689,15 +687,6 @@
             orderBy=["Language.englishname"])
         return result
 
-    def priorReleasedSeries(self):
-        """See `IDistroSeries`."""
-        datereleased = self.datereleased
-        # if this one is unreleased, use the last released one
-        if not datereleased:
-            datereleased = UTC_NOW
-        return getUtility(IDistroSeriesSet).priorReleasedSeries(
-            self.distribution, datereleased)
-
     @property
     def bug_reporting_guidelines(self):
         """See `IBugTarget`."""
@@ -1728,15 +1717,3 @@
         else:
 
             return DistroSeries.select(where_clause)
-
-    def priorReleasedSeries(self, distribution, prior_to_date):
-            """See `IDistroSeriesSet`."""
-            store = Store.of(distribution)
-            results = store.find(
-                DistroSeries,
-                DistroSeries.distributionID == distribution.id,
-                DistroSeries.datereleased < prior_to_date,
-                DistroSeries.datereleased != None
-            ).order_by(Desc(DistroSeries.datereleased))
-
-            return results

=== modified file 'lib/lp/registry/model/sourcepackage.py'
--- lib/lp/registry/model/sourcepackage.py	2012-08-08 05:36:44 +0000
+++ lib/lp/registry/model/sourcepackage.py	2012-09-26 04:19:22 +0000
@@ -1,7 +1,6 @@
-# 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).
 
-# pylint: disable-msg=E0611,W0212
 """Database classes that implement SourcePackage items."""
 
 __metaclass__ = type
@@ -156,10 +155,6 @@
         return recipients
 
     @property
-    def _store(self):
-        return Store.of(self.sourcepackagename)
-
-    @property
     def answer_contacts(self):
         """See `IQuestionTarget`."""
         answer_contacts = set()
@@ -222,18 +217,6 @@
         return '<%s %r %r %r>' % (self.__class__.__name__,
             self.distribution, self.distroseries, self.sourcepackagename)
 
-    def _get_ubuntu(self):
-        # XXX: kiko 2006-03-20: Ideally, it would be possible to just do
-        # ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        # and not need this method. However, importd currently depends
-        # on SourcePackage methods that require the ubuntu celebrity,
-        # and given it does not execute_zcml_for_scripts, we are forced
-        # here to do this hack instead of using components. Ideally,
-        # imports is rewritten to not use SourcePackage, or it
-        # initializes the component architecture correctly.
-        from lp.registry.model.distribution import Distribution
-        return Distribution.byName("ubuntu")
-
     def _getPublishingHistory(self, version=None, include_status=None,
                               exclude_status=None, order_by=None):
         """Build a query and return a list of SourcePackagePublishingHistory.
@@ -423,32 +406,17 @@
         """See `ISourcePackage`"""
         # First we look to see if there is packaging data for this
         # distroseries and sourcepackagename. If not, we look up through
-        # parent distroseries, and when we hit Ubuntu, we look backwards in
-        # time through Ubuntu series till we find packaging information or
-        # blow past the Warty Warthog.
+        # parent distroseries.
 
-        # see if there is a direct packaging
         result = self.direct_packaging
         if result is not None:
             return result
 
-        ubuntu = self._get_ubuntu()
-        # if we are an ubuntu sourcepackage, try the previous series of
-        # ubuntu
-        if self.distribution == ubuntu:
-            ubuntuseries = self.distroseries.priorReleasedSeries()
-            previous_ubuntu_series = ubuntuseries.first()
-            if previous_ubuntu_series is not None:
-                sp = SourcePackage(sourcepackagename=self.sourcepackagename,
-                                   distroseries=previous_ubuntu_series)
-                return sp.packaging
-        # if we have a parent distroseries, try that
+        # If we have a parent distroseries, try that.
         if self.distroseries.previous_series is not None:
             sp = SourcePackage(sourcepackagename=self.sourcepackagename,
                                distroseries=self.distroseries.previous_series)
             return sp.packaging
-        # capitulate
-        return None
 
     @property
     def published_by_pocket(self):
@@ -626,7 +594,7 @@
         # binary_only parameter as a source package can only have
         # binary builds.
 
-        clauseTables = ['SourcePackageRelease',
+        clauseTables = ['SourcePackageRelease', 'PackageBuild',
                         'SourcePackagePublishingHistory']
 
         condition_clauses = ["""
@@ -673,17 +641,17 @@
             clauseTables.append('BuildQueue')
             condition_clauses.append('BuildQueue.job = BuildPackageJob.job')
         elif build_state == BuildStatus.SUPERSEDED or build_state is None:
-            orderBy = ["-BuildFarmJob.date_created"]
+            orderBy = [Desc("BuildFarmJob.date_created")]
         else:
-            orderBy = ["-BuildFarmJob.date_finished"]
+            orderBy = [Desc("BuildFarmJob.date_finished")]
 
         # Fallback to ordering by -id as a tie-breaker.
-        orderBy.append("-id")
+        orderBy.append(Desc("id"))
 
         # End of duplication (see XXX cprov 2006-09-25 above).
 
-        return BinaryPackageBuild.select(' AND '.join(condition_clauses),
-                            clauseTables=clauseTables, orderBy=orderBy)
+        return IStore(BinaryPackageBuild).using(clauseTables).find(
+            BinaryPackageBuild, *condition_clauses).order_by(*orderBy)
 
     @property
     def latest_published_component(self):

=== modified file 'lib/lp/registry/tests/test_distroseries.py'
--- lib/lp/registry/tests/test_distroseries.py	2012-06-06 16:04:34 +0000
+++ lib/lp/registry/tests/test_distroseries.py	2012-09-26 04:19:22 +0000
@@ -9,7 +9,6 @@
     'CurrentSourceReleasesMixin',
     ]
 
-from datetime import timedelta
 from logging import getLogger
 
 import transaction
@@ -20,7 +19,6 @@
 from lp.registry.interfaces.distroseries import IDistroSeriesSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
-from lp.services.utils import utc_now
 from lp.soyuz.enums import (
     ArchivePurpose,
     PackagePublishingStatus,
@@ -281,36 +279,6 @@
         job = job_source.create(distroseries, [parent_distroseries.id])
         self.assertEqual(job, distroseries.getInitializationJob())
 
-    def test_priorReleasedSeries(self):
-        # Make sure that previousReleasedSeries returns all series with a
-        # release date less than the contextual series,
-        # ordered by descending date.
-        distro = self.factory.makeDistribution()
-        # Make an unreleased series.
-        self.factory.makeDistroSeries(distribution=distro)
-        ds1 = self.factory.makeDistroSeries(distribution=distro)
-        ds2 = self.factory.makeDistroSeries(distribution=distro)
-        ds3 = self.factory.makeDistroSeries(distribution=distro)
-        ds4 = self.factory.makeDistroSeries(distribution=distro)
-
-        now = utc_now()
-        older = now - timedelta(days=5)
-        oldest = now - timedelta(days=10)
-        newer = now + timedelta(days=15)
-        removeSecurityProxy(ds1).datereleased = oldest
-        removeSecurityProxy(ds2).datereleased = older
-        removeSecurityProxy(ds3).datereleased = now
-        removeSecurityProxy(ds4).datereleased = newer
-
-        # The data set up here is 5 distroseries. where one is unreleased,
-        # ds1 and ds2 are released and in the past and ds4 is released but
-        # in the future compared to ds3.
-
-        prior = ds3.priorReleasedSeries()
-        self.assertEqual(
-            [ds2, ds1],
-            list(prior))
-
     def test_getDifferenceComments_gets_DistroSeriesDifferenceComments(self):
         distroseries = self.factory.makeDistroSeries()
         dsd = self.factory.makeDistroSeriesDifference(

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-09-21 07:02:26 +0000
+++ lib/lp/soyuz/model/archive.py	2012-09-26 04:19:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0611,W0212
-
 """Database class for table Archive."""
 
 __metaclass__ = type
@@ -10,6 +8,7 @@
 __all__ = [
     'Archive',
     'ArchiveSet',
+    'get_archive_privacy_filter',
     'validate_ppa',
     ]
 
@@ -28,6 +27,7 @@
     And,
     Desc,
     Or,
+    Not,
     Select,
     SQL,
     Sum,
@@ -68,7 +68,10 @@
     validate_person,
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.interfaces.role import IHasOwner
+from lp.registry.interfaces.role import (
+    IHasOwner,
+    IPersonRoles,
+    )
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.registry.model.sourcepackagename import SourcePackageName
@@ -2574,3 +2577,18 @@
             )
 
         return results.order_by(SourcePackagePublishingHistory.id)
+
+
+def get_archive_privacy_filter(user):
+    if user is None:
+        privacy_filter = Not(Archive._private)
+    elif IPersonRoles(user).in_admin:
+        privacy_filter = True
+    else:
+        privacy_filter = Or(
+            Not(Archive._private),
+            Archive.ownerID.is_in(
+                Select(
+                    TeamParticipation.teamID,
+                    where=(TeamParticipation.person == user))))
+    return privacy_filter

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2012-07-05 09:13:42 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2012-09-26 04:19:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0611,W0212
-
 __metaclass__ = type
 __all__ = [
     'BinaryPackageBuild',
@@ -872,15 +870,9 @@
 
     def preloadBuildsData(self, builds):
         # Circular imports.
-        from lp.soyuz.model.distroarchseries import (
-            DistroArchSeries
-            )
-        from lp.registry.model.distroseries import (
-            DistroSeries
-            )
-        from lp.registry.model.distribution import (
-            Distribution
-            )
+        from lp.soyuz.model.distroarchseries import DistroArchSeries
+        from lp.registry.model.distroseries import DistroSeries
+        from lp.registry.model.distribution import Distribution
         from lp.soyuz.model.archive import Archive
         from lp.registry.model.person import Person
         self._prefetchBuildData(builds)
@@ -926,15 +918,15 @@
             BuildFarmJob.status == BuildStatus.NEEDSBUILD)
 
     def handleOptionalParamsForBuildQueries(
-        self, queries, tables, status=None, name=None, pocket=None,
+        self, clauses, origin, status=None, name=None, pocket=None,
         arch_tag=None):
         """Construct query clauses needed/shared by all getBuild..() methods.
 
         This method is not exposed via the public interface as it is only
         used to DRY-up trusted code.
 
-        :param queries: container to which to add any resulting query clauses.
-        :param tables: container to which to add joined tables.
+        :param clauses: container to which to add any resulting query clauses.
+        :param origin: container to which to add joined tables.
         :param status: optional build state for which to add a query clause if
             present.
         :param name: optional source package release name (or list of source
@@ -945,112 +937,96 @@
         :param arch_tag: optional architecture tag for which to add a
             query clause if present.
         """
+        # Circular. :(
+        from lp.registry.model.sourcepackagename import SourcePackageName
+        from lp.soyuz.model.distroarchseries import DistroArchSeries
+        from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 
         # Ensure the underlying buildfarmjob and package build tables
         # are included.
-        queries.append('BinaryPackageBuild.package_build = PackageBuild.id')
-        queries.append('PackageBuild.build_farm_job = BuildFarmJob.id')
-        tables.extend(['PackageBuild', 'BuildFarmJob'])
+        clauses.extend([
+            BinaryPackageBuild.package_build == PackageBuild.id,
+            PackageBuild.build_farm_job == BuildFarmJob.id])
+        origin.extend([BinaryPackageBuild, BuildFarmJob])
 
         # Add query clause that filters on build state if the latter is
         # provided.
         if status is not None:
-            queries.append('BuildFarmJob.status=%s' % sqlvalues(status))
+            clauses.append(BuildFarmJob.status == status)
 
         # Add query clause that filters on pocket if the latter is provided.
         if pocket:
             if not isinstance(pocket, (list, tuple)):
                 pocket = (pocket,)
-
-            queries.append('PackageBuild.pocket IN %s' % sqlvalues(pocket))
+            clauses.append(PackageBuild.pocket.is_in(pocket))
 
         # Add query clause that filters on architecture tag if provided.
         if arch_tag is not None:
-            queries.append('''
-                BinaryPackageBuild.distro_arch_series = DistroArchSeries.id
-                AND DistroArchSeries.architecturetag = %s
-            ''' % sqlvalues(arch_tag))
-            tables.extend(['DistroArchSeries'])
+            clauses.extend([
+                BinaryPackageBuild.distro_arch_series_id ==
+                    DistroArchSeries.id,
+                DistroArchSeries.architecturetag == arch_tag])
+            origin.append(DistroArchSeries)
 
         # Add query clause that filters on source package release name if the
         # latter is provided.
         if name is not None:
+            clauses.extend(
+                [BinaryPackageBuild.source_package_release_id ==
+                    SourcePackageRelease.id,
+                SourcePackageRelease.sourcepackagenameID ==
+                    SourcePackageName.id])
+            origin.extend([SourcePackageRelease, SourcePackageName])
             if not isinstance(name, (list, tuple)):
-                queries.append('''
-                    BinaryPackageBuild.source_package_release =
-                        SourcePackageRelease.id AND
-                    SourcePackageRelease.sourcepackagename =
-                        SourcePackageName.id
-                    AND SourcepackageName.name LIKE '%%%%' || %s || '%%%%'
-                ''' % quote_like(name))
+                clauses.append(
+                    "SourcepackageName.name LIKE '%%%%' || %s || '%%%%'" % (
+                        quote_like(name)))
             else:
-                queries.append('''
-                    BinaryPackageBuild.source_package_release =
-                        SourcePackageRelease.id AND
-                    SourcePackageRelease.sourcepackagename =
-                        SourcePackageName.id
-                    AND SourcepackageName.name IN %s
-                ''' % sqlvalues(name))
-            tables.extend(['SourcePackageRelease', 'SourcePackageName'])
+                clauses.append(SourcePackageName.name.is_in(name))
 
     def getBuildsForBuilder(self, builder_id, status=None, name=None,
                             arch_tag=None, user=None):
         """See `IBinaryPackageBuildSet`."""
-        queries = []
-        clauseTables = []
+        # Circular. :(
+        from lp.soyuz.model.archive import (
+            Archive, get_archive_privacy_filter)
+
+        clauses = [
+            PackageBuild.archive_id == Archive.id,
+            BuildFarmJob.builder_id == builder_id,
+            get_archive_privacy_filter(user)]
+        origin = [PackageBuild, Archive]
 
         self.handleOptionalParamsForBuildQueries(
-            queries, clauseTables, status, name, pocket=None,
-            arch_tag=arch_tag)
-
-        # This code MUST match the logic in the Build security adapter,
-        # otherwise users are likely to get 403 errors, or worse.
-        queries.append("Archive.id = PackageBuild.archive")
-        clauseTables.append('Archive')
-        if user is not None:
-            if not user.inTeam(getUtility(ILaunchpadCelebrities).admin):
-                queries.append("""
-                (Archive.private = FALSE
-                 OR %s IN (SELECT TeamParticipation.person
-                       FROM TeamParticipation
-                       WHERE TeamParticipation.person = %s
-                           AND TeamParticipation.team = Archive.owner)
-                )""" % sqlvalues(user, user))
-        else:
-            queries.append("Archive.private = FALSE")
-
-        queries.append("builder=%s" % builder_id)
-
-        return BinaryPackageBuild.select(
-            " AND ".join(queries), clauseTables=clauseTables,
-            orderBy=["-BuildFarmJob.date_finished", "id"])
+            clauses, origin, status, name, pocket=None, arch_tag=arch_tag)
+
+        return IStore(BinaryPackageBuild).using(*origin).find(
+            BinaryPackageBuild, *clauses).order_by(
+                Desc(BuildFarmJob.date_finished), BinaryPackageBuild.id)
 
     def getBuildsForArchive(self, archive, status=None, name=None,
                             pocket=None, arch_tag=None):
         """See `IBinaryPackageBuildSet`."""
-        queries = []
-        clauseTables = []
+        clauses = [PackageBuild.archive_id == archive.id]
+        origin = [PackageBuild]
 
         self.handleOptionalParamsForBuildQueries(
-            queries, clauseTables, status, name, pocket, arch_tag)
+            clauses, origin, status, name, pocket, arch_tag)
 
         # Ordering according status
         # * SUPERSEDED & All by -datecreated
         # * FULLYBUILT & FAILURES by -datebuilt
         # It should present the builds in a more natural order.
         if status == BuildStatus.SUPERSEDED or status is None:
-            orderBy = ["-BuildFarmJob.date_created"]
+            orderBy = [Desc(BuildFarmJob.date_created)]
         else:
-            orderBy = ["-BuildFarmJob.date_finished"]
+            orderBy = [Desc(BuildFarmJob.date_finished)]
         # All orders fallback to id if the primary order doesn't succeed
-        orderBy.append("id")
-
-        queries.append("archive=%s" % sqlvalues(archive))
-        clause = " AND ".join(queries)
+        orderBy.append(BinaryPackageBuild.id)
 
         return self._decorate_with_prejoins(
-            BinaryPackageBuild.select(
-                clause, clauseTables=clauseTables, orderBy=orderBy))
+            IStore(BinaryPackageBuild).using(*origin).find(
+                BinaryPackageBuild, *clauses).order_by(*orderBy))
 
     def getBuildsByArchIds(self, distribution, arch_ids, status=None,
                            name=None, pocket=None):
@@ -1059,7 +1035,7 @@
         if not arch_ids:
             return EmptyResultSet()
 
-        clauseTables = []
+        clauseTables = [PackageBuild]
 
         # format clause according single/multiple architecture(s) form
         if len(arch_ids) == 1:
@@ -1124,9 +1100,7 @@
             "PackageBuild.archive IN %s" %
             sqlvalues(list(distribution.all_distro_archive_ids)))
 
-        store = Store.of(distribution)
-        clauseTables = [BinaryPackageBuild] + clauseTables
-        result_set = store.using(*clauseTables).find(
+        result_set = Store.of(distribution).using(*clauseTables).find(
             (BinaryPackageBuild, order_by_table), *condition_clauses)
         result_set.order_by(*order_by)