Merge ~cjwatson/launchpad:stormify-binarypackagebuild into launchpad:master

[Colin Watson]

Convert BinaryPackageBuild to Storm

[Ines Almeida (ines-almeida)]

LGTM 👍

review: Approve

diff --git a/lib/lp/archiveuploader/tests/test_buildduploads.py b/lib/lp/archiveuploader/tests/test_buildduploads.py
index 0425a43..c368db7 100644
--- a/lib/lp/archiveuploader/tests/test_buildduploads.py
+++ b/lib/lp/archiveuploader/tests/test_buildduploads.py
@@ -15,6 +15,7 @@ from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.database.constants import UTC_NOW
+from lp.services.database.interfaces import IStore
 from lp.soyuz.enums import PackagePublishingStatus, PackageUploadStatus
 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 from lp.soyuz.interfaces.publishing import IPublishingSet
@@ -77,14 +78,18 @@ class TestStagedBinaryUploadBase(TestUploadProcessorBase):
         self.build_uploadprocessor = self.getUploadProcessor(
             self.layer.txn, builds=True
         )
-        self.builds_before_upload = BinaryPackageBuild.select().count()
+        self.builds_before_upload = (
+            IStore(BinaryPackageBuild).find(BinaryPackageBuild).count()
+        )
         self.source_queue = None
         self._uploadSource()
         self.layer.txn.commit()
 
     def assertBuildsCreated(self, amount):
         """Assert that a given 'amount' of build records was created."""
-        builds_count = BinaryPackageBuild.select().count()
+        builds_count = (
+            IStore(BinaryPackageBuild).find(BinaryPackageBuild).count()
+        )
         self.assertEqual(self.builds_before_upload + amount, builds_count)
 
     def _prepareUpload(self, upload_dir):
diff --git a/lib/lp/soyuz/doc/distroarchseriesbinarypackage.rst b/lib/lp/soyuz/doc/distroarchseriesbinarypackage.rst
index 1a36167..2b51ad9 100644
--- a/lib/lp/soyuz/doc/distroarchseriesbinarypackage.rst
+++ b/lib/lp/soyuz/doc/distroarchseriesbinarypackage.rst
@@ -48,30 +48,34 @@ builds that are in sampledata!
 XXX: noodles 2008-11-05 bug=294585: The dependency on a database id
 needs to be removed.
 
-    >>> bpr = BinaryPackageBuild.get(8).createBinaryPackageRelease(
-    ...     binarypackagename=firefox_name.id,
-    ...     version="120.6-0",
-    ...     summary="Firefox loves lollies",
-    ...     description="Lolly-pop loving application",
-    ...     binpackageformat=binpackageformat,
-    ...     component=main_component.id,
-    ...     section=misc_section.id,
-    ...     priority=priority,
-    ...     shlibdeps=None,
-    ...     depends=None,
-    ...     recommends=None,
-    ...     suggests=None,
-    ...     conflicts=None,
-    ...     replaces=None,
-    ...     provides=None,
-    ...     pre_depends=None,
-    ...     enhances=None,
-    ...     breaks=None,
-    ...     built_using=None,
-    ...     essential=False,
-    ...     installedsize=0,
-    ...     architecturespecific=False,
-    ...     debug_package=None,
+    >>> bpr = (
+    ...     IStore(BinaryPackageBuild)
+    ...     .get(BinaryPackageBuild, 8)
+    ...     .createBinaryPackageRelease(
+    ...         binarypackagename=firefox_name.id,
+    ...         version="120.6-0",
+    ...         summary="Firefox loves lollies",
+    ...         description="Lolly-pop loving application",
+    ...         binpackageformat=binpackageformat,
+    ...         component=main_component.id,
+    ...         section=misc_section.id,
+    ...         priority=priority,
+    ...         shlibdeps=None,
+    ...         depends=None,
+    ...         recommends=None,
+    ...         suggests=None,
+    ...         conflicts=None,
+    ...         replaces=None,
+    ...         provides=None,
+    ...         pre_depends=None,
+    ...         enhances=None,
+    ...         breaks=None,
+    ...         built_using=None,
+    ...         essential=False,
+    ...         installedsize=0,
+    ...         architecturespecific=False,
+    ...         debug_package=None,
+    ...     )
     ... )
 
     >>> pe = BinaryPackagePublishingHistory(
@@ -93,30 +97,34 @@ needs to be removed.
 XXX: noodles 2008-11-06 bug=294585: The dependency on a database id
 needs to be removed.
 
-    >>> bpr = BinaryPackageBuild.get(9).createBinaryPackageRelease(
-    ...     binarypackagename=pmount_name.id,
-    ...     version="cr98.34",
-    ...     summary="Pmount bakes cakes",
-    ...     description="Phat cake-baker application",
-    ...     binpackageformat=binpackageformat,
-    ...     component=main_component.id,
-    ...     section=misc_section.id,
-    ...     priority=priority,
-    ...     shlibdeps=None,
-    ...     depends=None,
-    ...     recommends=None,
-    ...     suggests=None,
-    ...     conflicts=None,
-    ...     replaces=None,
-    ...     provides=None,
-    ...     pre_depends=None,
-    ...     enhances=None,
-    ...     breaks=None,
-    ...     built_using=None,
-    ...     essential=False,
-    ...     installedsize=0,
-    ...     architecturespecific=False,
-    ...     debug_package=None,
+    >>> bpr = (
+    ...     IStore(BinaryPackageBuild)
+    ...     .get(BinaryPackageBuild, 9)
+    ...     .createBinaryPackageRelease(
+    ...         binarypackagename=pmount_name.id,
+    ...         version="cr98.34",
+    ...         summary="Pmount bakes cakes",
+    ...         description="Phat cake-baker application",
+    ...         binpackageformat=binpackageformat,
+    ...         component=main_component.id,
+    ...         section=misc_section.id,
+    ...         priority=priority,
+    ...         shlibdeps=None,
+    ...         depends=None,
+    ...         recommends=None,
+    ...         suggests=None,
+    ...         conflicts=None,
+    ...         replaces=None,
+    ...         provides=None,
+    ...         pre_depends=None,
+    ...         enhances=None,
+    ...         breaks=None,
+    ...         built_using=None,
+    ...         essential=False,
+    ...         installedsize=0,
+    ...         architecturespecific=False,
+    ...         debug_package=None,
+    ...     )
     ... )
 
     >>> pe = BinaryPackagePublishingHistory(
diff --git a/lib/lp/soyuz/doc/gina-multiple-arch.rst b/lib/lp/soyuz/doc/gina-multiple-arch.rst
index c9bbea2..495b8eb 100644
--- a/lib/lp/soyuz/doc/gina-multiple-arch.rst
+++ b/lib/lp/soyuz/doc/gina-multiple-arch.rst
@@ -27,7 +27,9 @@ Get the current counts of stuff in the database:
     >>> orig_bpr_count = (
     ...     IStore(BinaryPackageRelease).find(BinaryPackageRelease).count()
     ... )
-    >>> orig_build_count = BinaryPackageBuild.select().count()
+    >>> orig_build_count = (
+    ...     IStore(BinaryPackageBuild).find(BinaryPackageBuild).count()
+    ... )
     >>> orig_sbpph_count = IStore(SBPPH).find(SBPPH).count()
 
 Create a distribution series and an arch series for dapper:
@@ -157,7 +159,10 @@ distroarchseries:
     ...     - orig_bpr_count
     ... )
     4
-    >>> BinaryPackageBuild.select().count() - orig_build_count
+    >>> (
+    ...     IStore(BinaryPackageBuild).find(BinaryPackageBuild).count()
+    ...     - orig_build_count
+    ... )
     2
     >>> IStore(SBPPH).find(SBPPH).count() - orig_sbpph_count
     4
diff --git a/lib/lp/soyuz/doc/gina.rst b/lib/lp/soyuz/doc/gina.rst
index db82fd9..39bcbdb 100644
--- a/lib/lp/soyuz/doc/gina.rst
+++ b/lib/lp/soyuz/doc/gina.rst
@@ -34,7 +34,9 @@ Get the current counts of stuff in the database:
     >>> orig_bpr_count = (
     ...     IStore(BinaryPackageRelease).find(BinaryPackageRelease).count()
     ... )
-    >>> orig_build_count = BinaryPackageBuild.select().count()
+    >>> orig_build_count = (
+    ...     IStore(BinaryPackageBuild).find(BinaryPackageBuild).count()
+    ... )
     >>> orig_sbpph_count = IStore(SBPPH).find(SBPPH).count()
     >>> orig_sspph_main_count = (
     ...     IStore(SSPPH)
@@ -431,7 +433,10 @@ work.
     ...     - orig_bpr_count
     ... )
     40
-    >>> BinaryPackageBuild.select().count() - orig_build_count
+    >>> (
+    ...     IStore(BinaryPackageBuild).find(BinaryPackageBuild).count()
+    ...     - orig_build_count
+    ... )
     13
     >>> IStore(SBPPH).find(SBPPH).count() - orig_sbpph_count
     46
@@ -681,7 +686,10 @@ changed, etc.
     ...     - orig_bpr_count
     ... )
     40
-    >>> BinaryPackageBuild.select().count() - orig_build_count
+    >>> (
+    ...     IStore(BinaryPackageBuild).find(BinaryPackageBuild).count()
+    ...     - orig_build_count
+    ... )
     13
 
 But the overrides do generate extra publishing entries:
diff --git a/lib/lp/soyuz/model/binarypackagebuild.py b/lib/lp/soyuz/model/binarypackagebuild.py
index b71a38d..9c9623c 100644
--- a/lib/lp/soyuz/model/binarypackagebuild.py
+++ b/lib/lp/soyuz/model/binarypackagebuild.py
@@ -45,11 +45,12 @@ from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.services.config import config
 from lp.services.database.bulk import load_related
+from lp.services.database.constants import DEFAULT
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import SQLBase, sqlvalues
-from lp.services.database.sqlobject import SQLObjectNotFound
+from lp.services.database.sqlbase import sqlvalues
+from lp.services.database.stormbase import StormBase
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.librarian.interfaces import ILibraryFileAlias
 from lp.services.librarian.model import LibraryFileAlias, LibraryFileContent
@@ -137,12 +138,14 @@ def storm_validate_external_dependencies(build, attr, value):
 
 
 @implementer(IBinaryPackageBuild)
-class BinaryPackageBuild(PackageBuildMixin, SQLBase):
-    _table = "BinaryPackageBuild"
-    _defaultOrder = "id"
+class BinaryPackageBuild(PackageBuildMixin, StormBase):
+    __storm_table__ = "BinaryPackageBuild"
+    __storm_order__ = "id"
 
     job_type = BuildFarmJobType.PACKAGEBUILD
 
+    id = Int(primary=True)
+
     build_farm_job_id = Int(name="build_farm_job")
     build_farm_job = Reference(build_farm_job_id, BuildFarmJob.id)
 
@@ -215,6 +218,46 @@ class BinaryPackageBuild(PackageBuildMixin, SQLBase):
     buildinfo_id = Int(name="buildinfo")
     buildinfo = Reference(buildinfo_id, "LibraryFileAlias.id")
 
+    def __init__(
+        self,
+        build_farm_job,
+        distro_arch_series,
+        source_package_release,
+        archive,
+        pocket,
+        arch_indep,
+        status,
+        processor=None,
+        virtualized=None,
+        date_created=DEFAULT,
+        builder=None,
+        buildinfo=None,
+    ):
+        # Compute these before creating the row, to avoid violating not-null
+        # constraints if these other rows aren't in the cache.
+        distribution = distro_arch_series.distroseries.distribution
+        distro_series = distro_arch_series.distroseries
+        is_distro_archive = archive.is_main
+        source_package_name = source_package_release.sourcepackagename
+
+        super().__init__()
+        self.build_farm_job = build_farm_job
+        self.distro_arch_series = distro_arch_series
+        self.source_package_release = source_package_release
+        self.archive = archive
+        self.pocket = pocket
+        self.arch_indep = arch_indep
+        self.status = status
+        self.processor = processor
+        self.virtualized = virtualized
+        self.date_created = date_created
+        self.builder = builder
+        self.buildinfo = buildinfo
+        self.distribution = distribution
+        self.distro_series = distro_series
+        self.is_distro_archive = is_distro_archive
+        self.source_package_name = source_package_name
+
     def getLatestSourcePublication(self):
         from lp.soyuz.model.publishing import SourcePackagePublishingHistory
 
@@ -748,7 +791,7 @@ class BinaryPackageBuild(PackageBuildMixin, SQLBase):
             assert self.buildinfo == buildinfo
 
     def verifySuccessfulUpload(self) -> bool:
-        return bool(self.binarypackages)
+        return not self.binarypackages.is_empty()
 
     def notify(self, extra_info=None):
         """See `IPackageBuild`.
@@ -855,7 +898,7 @@ class BinaryPackageBuildSet(SpecificBuildFarmJobSourceMixin):
         )
         processor = distro_arch_series.processor
         virtualized = is_build_virtualized(archive, processor)
-        return BinaryPackageBuild(
+        build = BinaryPackageBuild(
             build_farm_job=build_farm_job,
             distro_arch_series=distro_arch_series,
             source_package_release=source_package_release,
@@ -866,20 +909,18 @@ class BinaryPackageBuildSet(SpecificBuildFarmJobSourceMixin):
             processor=processor,
             virtualized=virtualized,
             builder=builder,
-            is_distro_archive=archive.is_main,
-            distribution=distro_arch_series.distroseries.distribution,
-            distro_series=distro_arch_series.distroseries,
-            source_package_name=source_package_release.sourcepackagename,
             buildinfo=buildinfo,
             date_created=date_created,
         )
+        IStore(build).flush()
+        return build
 
     def getByID(self, id):
         """See `IBinaryPackageBuildSet`."""
-        try:
-            return BinaryPackageBuild.get(id)
-        except SQLObjectNotFound as e:
-            raise NotFoundError(str(e))
+        build = IStore(BinaryPackageBuild).get(BinaryPackageBuild, id)
+        if build is None:
+            raise NotFoundError(id)
+        return build
 
     def getByBuildFarmJob(self, build_farm_job):
         """See `ISpecificBuildFarmJobSource`."""
diff --git a/lib/lp/soyuz/model/sourcepackagerelease.py b/lib/lp/soyuz/model/sourcepackagerelease.py
index 8c27732..ba1ec93 100644
--- a/lib/lp/soyuz/model/sourcepackagerelease.py
+++ b/lib/lp/soyuz/model/sourcepackagerelease.py
@@ -216,19 +216,24 @@ class SourcePackageRelease(SQLBase):
     @property
     def builds(self):
         """See `ISourcePackageRelease`."""
+        # Circular import.
+        from lp.soyuz.model.archive import Archive
+
         # Excluding PPA builds may seem like a strange thing to do, but,
         # since Archive.copyPackage can copy packages across archives, a
         # build may well have a different archive to the corresponding
         # sourcepackagerelease.
-        return BinaryPackageBuild.select(
-            """
-            source_package_release = %s AND
-            archive.id = binarypackagebuild.archive AND
-            archive.purpose IN %s
-            """
-            % sqlvalues(self.id, MAIN_ARCHIVE_PURPOSES),
-            orderBy=["-date_created", "id"],
-            clauseTables=["Archive"],
+        return (
+            IStore(BinaryPackageBuild)
+            .find(
+                BinaryPackageBuild,
+                BinaryPackageBuild.source_package_release == self,
+                BinaryPackageBuild.archive == Archive.id,
+                Archive.purpose.is_in(MAIN_ARCHIVE_PURPOSES),
+            )
+            .order_by(
+                Desc(BinaryPackageBuild.date_created), BinaryPackageBuild.id
+            )
         )
 
     @property
diff --git a/lib/lp/soyuz/scripts/gina/handlers.py b/lib/lp/soyuz/scripts/gina/handlers.py
index 5af9a8b..67368b9 100644
--- a/lib/lp/soyuz/scripts/gina/handlers.py
+++ b/lib/lp/soyuz/scripts/gina/handlers.py
@@ -40,7 +40,6 @@ from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import quote
 from lp.services.database.sqlobject import SQLObjectNotFound
 from lp.services.librarian.interfaces import ILibraryFileAliasSet
 from lp.services.scripts import log
@@ -928,11 +927,6 @@ class BinaryPackageHandler:
     def ensureBuild(self, binary, srcpkg, distroarchseries, archtag):
         """Ensure a build record."""
         distribution = distroarchseries.distroseries.distribution
-        clauseTables = [
-            "BinaryPackageBuild",
-            "DistroArchSeries",
-            "DistroSeries",
-        ]
 
         # XXX kiko 2006-02-03:
         # This method doesn't work for real bin-only NMUs that are
@@ -942,23 +936,23 @@ class BinaryPackageHandler:
         # once, and the two checks below will of course blow up when
         # doing it the second time.
 
-        query = (
-            "BinaryPackageBuild.source_package_release = %d AND "
-            "BinaryPackageBuild.distro_arch_series = "
-            "    DistroArchSeries.id AND "
-            "BinaryPackageBuild.archive = %d AND "
-            "DistroArchSeries.distroseries = DistroSeries.id AND "
-            "DistroSeries.distribution = %d"
-            % (srcpkg.id, distribution.main_archive.id, distribution.id)
-        )
+        clauses = [
+            BinaryPackageBuild.source_package_release == srcpkg,
+            BinaryPackageBuild.archive == distribution.main_archive,
+            BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
+            DistroArchSeries.distroseries == DistroSeries.id,
+            DistroSeries.distribution == distribution,
+        ]
 
         if archtag != "all":
-            query += "AND DistroArchSeries.architecturetag = %s" % quote(
-                archtag
-            )
+            clauses.append(DistroArchSeries.architecturetag == archtag)
 
         try:
-            build = BinaryPackageBuild.selectOne(query, clauseTables)
+            build = (
+                IStore(BinaryPackageBuild)
+                .find(BinaryPackageBuild, *clauses)
+                .one()
+            )
         except NotOneError:
             # XXX kiko 2005-10-27: Untested.
             raise MultipleBuildError(