launchpad-reviewers team mailing list archive

[Colin Watson <mp+426018@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:stormify-packagediff into launchpad:master.

Commit message:
Convert PackageDiff to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/426018
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-packagediff into launchpad:master.

diff --git a/lib/lp/soyuz/doc/package-diff.rst b/lib/lp/soyuz/doc/package-diff.rst
index 97bd8db..6f38543 100644
--- a/lib/lp/soyuz/doc/package-diff.rst
+++ b/lib/lp/soyuz/doc/package-diff.rst
@@ -74,7 +74,7 @@ Its main attributes are:
     pmount - 0.1-2
 
 The PackageDiff record is not yet 'performed', so 'status' is PENDING
-and both 'date_fullfiled' and 'diff_content' fields are empty.
+and both 'date_fulfilled' and 'diff_content' fields are empty.
 
     >>> print(package_diff.date_fulfilled)
     None
@@ -315,7 +315,7 @@ was uploaded.
     >>> [diff] = biscuit_eight_pub.sourcepackagerelease.package_diffs
 
 The PackageDiff record is not yet 'performed', so both,
-'date_fullfiled' and 'diff_content' fields, are empty and 'status' is
+'date_fulfilled' and 'diff_content' fields, are empty and 'status' is
 PENDING.
 
     >>> print(diff.status.name)
@@ -332,7 +332,7 @@ Performing the diff.
     >>> diff.performDiff()
 
 The record is immediatelly updated, now the record contains a
-'date_fullfilled', its status is COMPLETED and 'diff_content' points
+'date_fulfilled', its status is COMPLETED and 'diff_content' points
 to a LibraryFileAlias with a proper mimetype.
 
     >>> diff.date_fulfilled is not None
@@ -636,7 +636,7 @@ uploaded source.
     diff from 1.0-1 (in Ubuntu) to 1.0-2
 
 With a tainted DSC 'debdiff' cannot do much and fails, resulting in a
-FAILED request (empty 'diff_content' and 'date_fullfilled').
+FAILED request (empty 'diff_content' and 'date_fulfilled').
 
     >>> broken_diff.performDiff()
     >>> transaction.commit()
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index 0ff9b0b..2a6d750 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -1728,8 +1728,8 @@ class Archive(SQLBase):
                 clauses = (
                     SourcePackagePublishingHistory.archive == self.id,
                     SourcePackagePublishingHistory.sourcepackagereleaseID ==
-                        PackageDiff.to_sourceID,
-                    PackageDiff.diff_contentID == LibraryFileAlias.id,
+                        PackageDiff.to_source_id,
+                    PackageDiff.diff_content_id == LibraryFileAlias.id,
                     )
                 package_diff_file = do_query()
                 if package_diff_file is not None:
diff --git a/lib/lp/soyuz/model/packagediff.py b/lib/lp/soyuz/model/packagediff.py
index 90d2170..29286dc 100644
--- a/lib/lp/soyuz/model/packagediff.py
+++ b/lib/lp/soyuz/model/packagediff.py
@@ -15,23 +15,27 @@ import shutil
 import subprocess
 import tempfile
 
-from storm.expr import Desc
+import pytz
+from storm.locals import (
+    DateTime,
+    Desc,
+    Int,
+    Reference,
+    )
 from storm.store import EmptyResultSet
 from zope.component import getUtility
 from zope.interface import implementer
 
 from lp.services.config import config
 from lp.services.database.bulk import load
-from lp.services.database.constants import UTC_NOW
-from lp.services.database.datetimecol import UtcDateTimeCol
+from lp.services.database.constants import (
+    DEFAULT,
+    UTC_NOW,
+    )
 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 ForeignKey
+from lp.services.database.stormbase import StormBase
 from lp.services.librarian.interfaces import ILibraryFileAliasSet
 from lp.services.librarian.model import (
     LibraryFileAlias,
@@ -43,6 +47,7 @@ from lp.soyuz.interfaces.packagediff import (
     IPackageDiff,
     IPackageDiffSet,
     )
+from lp.soyuz.model.files import SourcePackageReleaseFile
 
 
 def limit_deb_diff(max_size):
@@ -118,32 +123,45 @@ def download_file(destination_path, libraryfile):
 
 
 @implementer(IPackageDiff)
-class PackageDiff(SQLBase):
+class PackageDiff(StormBase):
     """A Package Diff request."""
 
-    _defaultOrder = ['id']
+    __storm_table__ = "PackageDiff"
+    __storm_order__ = ["id"]
+
+    id = Int(primary=True)
 
-    date_requested = UtcDateTimeCol(notNull=False, default=UTC_NOW)
+    date_requested = DateTime(
+        allow_none=False, default=UTC_NOW, tzinfo=pytz.UTC)
 
-    requester = ForeignKey(
-        dbName='requester', foreignKey='Person', notNull=False)
+    requester_id = Int(name="requester", allow_none=True)
+    requester = Reference(requester_id, "Person.id")
 
-    from_source = ForeignKey(
-        dbName="from_source", foreignKey='SourcePackageRelease', notNull=True)
+    from_source_id = Int(name="from_source", allow_none=False)
+    from_source = Reference(from_source_id, "SourcePackageRelease.id")
 
-    to_source = ForeignKey(
-        dbName="to_source", foreignKey='SourcePackageRelease', notNull=True)
+    to_source_id = Int(name="to_source", allow_none=False)
+    to_source = Reference(to_source_id, "SourcePackageRelease.id")
 
-    date_fulfilled = UtcDateTimeCol(notNull=False, default=None)
+    date_fulfilled = DateTime(allow_none=True, default=None, tzinfo=pytz.UTC)
 
-    diff_content = ForeignKey(
-        dbName="diff_content", foreignKey='LibraryFileAlias',
-        notNull=False, default=None)
+    diff_content_id = Int(name="diff_content", allow_none=True, default=None)
+    diff_content = Reference(diff_content_id, "LibraryFileAlias.id")
 
     status = DBEnum(
         name='status', allow_none=False, enum=PackageDiffStatus,
         default=PackageDiffStatus.PENDING)
 
+    def __init__(self, from_source, to_source, requester=None,
+                 date_fulfilled=None, diff_content=None, status=DEFAULT):
+        super().__init__()
+        self.from_source = from_source
+        self.to_source = to_source
+        self.requester = requester
+        self.date_fulfilled = date_fulfilled
+        self.diff_content = diff_content
+        self.status = status
+
     @property
     def title(self):
         """See `IPackageDiff`."""
@@ -167,19 +185,17 @@ class PackageDiff(SQLBase):
     def _countDeletedLFAs(self):
         """How many files associated with either source package have been
         deleted from the librarian?"""
-        query = """
-            SELECT COUNT(lfa.id)
-            FROM
-                SourcePackageRelease spr, SourcePackageReleaseFile sprf,
-                LibraryFileAlias lfa
-            WHERE
-                spr.id IN %s
-                AND sprf.SourcePackageRelease = spr.id
-                AND sprf.libraryfile = lfa.id
-                AND lfa.content IS NULL
-            """ % sqlvalues((self.from_source.id, self.to_source.id))
-        result = IStore(LibraryFileAlias).execute(query).get_one()
-        return (0 if result is None else result[0])
+        # Circular import.
+        from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+
+        return IStore(LibraryFileAlias).find(
+            LibraryFileAlias.id,
+            SourcePackageRelease.id.is_in(
+                (self.from_source_id, self.to_source_id)),
+            SourcePackageReleaseFile.sourcepackagereleaseID ==
+                SourcePackageRelease.id,
+            SourcePackageReleaseFile.libraryfileID == LibraryFileAlias.id,
+            LibraryFileAlias.content == None).count()
 
     def performDiff(self):
         """See `IPackageDiff`.
@@ -282,11 +298,15 @@ class PackageDiffSet:
 
     def __iter__(self):
         """See `IPackageDiffSet`."""
-        return iter(PackageDiff.select(orderBy=['-id']))
+        return iter(
+            IStore(PackageDiff)
+            .find(PackageDiff)
+            .order_by(Desc(PackageDiff.id))
+        )
 
     def get(self, diff_id):
         """See `IPackageDiffSet`."""
-        return PackageDiff.get(diff_id)
+        return IStore(PackageDiff).get(PackageDiff, diff_id)
 
     def getDiffsToReleases(self, sprs, preload_for_display=False):
         """See `IPackageDiffSet`."""
@@ -297,17 +317,17 @@ class PackageDiffSet:
             return EmptyResultSet()
         spr_ids = [spr.id for spr in sprs]
         result = IStore(PackageDiff).find(
-            PackageDiff, PackageDiff.to_sourceID.is_in(spr_ids))
-        result.order_by(PackageDiff.to_sourceID,
+            PackageDiff, PackageDiff.to_source_id.is_in(spr_ids))
+        result.order_by(PackageDiff.to_source_id,
                         Desc(PackageDiff.date_requested))
 
         def preload_hook(rows):
-            lfas = load(LibraryFileAlias, (pd.diff_contentID for pd in rows))
+            lfas = load(LibraryFileAlias, (pd.diff_content_id for pd in rows))
             load(LibraryFileContent, (lfa.contentID for lfa in lfas))
             sprs = load(
                 SourcePackageRelease,
                 itertools.chain.from_iterable(
-                    (pd.from_sourceID, pd.to_sourceID) for pd in rows))
+                    (pd.from_source_id, pd.to_source_id) for pd in rows))
             archives = load(Archive, (spr.upload_archiveID for spr in sprs))
             load(Distribution, (a.distributionID for a in archives))
 
@@ -320,4 +340,4 @@ class PackageDiffSet:
         """See `IPackageDiffSet`."""
         return IStore(PackageDiff).find(
             PackageDiff,
-            from_sourceID=from_spr.id, to_sourceID=to_spr.id).first()
+            from_source=from_spr, to_source=to_spr).first()
diff --git a/lib/lp/soyuz/model/sourcepackagerelease.py b/lib/lp/soyuz/model/sourcepackagerelease.py
index c68ad80..78cbb7c 100644
--- a/lib/lp/soyuz/model/sourcepackagerelease.py
+++ b/lib/lp/soyuz/model/sourcepackagerelease.py
@@ -43,6 +43,7 @@ from lp.services.database.constants import UTC_NOW
 from lp.services.database.datetimecol import UtcDateTimeCol
 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 (
     cursor,
     SQLBase,
@@ -400,8 +401,9 @@ class SourcePackageRelease(SQLBase):
 
     def getDiffTo(self, to_sourcepackagerelease):
         """See ISourcePackageRelease."""
-        return PackageDiff.selectOneBy(
-            from_source=self, to_source=to_sourcepackagerelease)
+        return IStore(PackageDiff).find(
+            PackageDiff,
+            from_source=self, to_source=to_sourcepackagerelease).one()
 
     def requestDiffTo(self, requester, to_sourcepackagerelease):
         """See ISourcePackageRelease."""
@@ -416,6 +418,7 @@ class SourcePackageRelease(SQLBase):
         packagediff = PackageDiff(
             from_source=self, to_source=to_sourcepackagerelease,
             requester=requester)
+        IStore(packagediff).flush()
         getUtility(IPackageDiffJobSource).create(packagediff)
         return packagediff
 
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index af13596..a313f40 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -4621,11 +4621,13 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             diff_content = self.getUniqueBytes("packagediff")
         lfa = self.makeLibraryFileAlias(
             filename=diff_filename, content=diff_content)
-        return ProxyFactory(
+        package_diff = ProxyFactory(
             PackageDiff(
                 requester=requester, from_source=from_source,
                 to_source=to_source, date_fulfilled=date_fulfilled,
                 status=status, diff_content=lfa))
+        IStore(package_diff).flush()
+        return package_diff
 
     # Factory methods for OAuth tokens.
     def makeOAuthConsumer(self, key=None, secret=None):