launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Convert BugBranch to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

diff --git a/lib/lp/bugs/model/bug.py b/lib/lp/bugs/model/bug.py
index be4bb07..7ea2576 100644
--- a/lib/lp/bugs/model/bug.py
+++ b/lib/lp/bugs/model/bug.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Launchpad bug-related database table classes."""
@@ -59,6 +59,7 @@ from storm.locals import (
     DateTime,
     Int,
     Reference,
+    ReferenceSet,
     )
 from storm.store import (
     EmptyResultSet,
@@ -360,8 +361,8 @@ class Bug(SQLBase, InformationTypeMixin):
     watches = SQLMultipleJoin(
         'BugWatch', joinColumn='bug', orderBy=['bugtracker', 'remotebug'])
     duplicates = SQLMultipleJoin('Bug', joinColumn='duplicateof', orderBy='id')
-    linked_bugbranches = SQLMultipleJoin(
-        'BugBranch', joinColumn='bug', orderBy='id')
+    linked_bugbranches = ReferenceSet(
+        'id', BugBranch.bug_id, order_by=BugBranch.id)
     date_last_message = UtcDateTimeCol(default=None)
     number_of_duplicates = IntCol(notNull=True, default=0)
     message_count = IntCol(notNull=True, default=0)
@@ -1368,7 +1369,8 @@ class Bug(SQLBase, InformationTypeMixin):
 
     def unlinkBranch(self, branch, user):
         """See `IBug`."""
-        bug_branch = BugBranch.selectOneBy(bug=self, branch=branch)
+        bug_branch = Store.of(self).find(
+            BugBranch, BugBranch.bug == self, BugBranch.branch == branch).one()
         if bug_branch is not None:
             self.addChange(BranchUnlinkedFromBug(UTC_NOW, user, branch, self))
             notify(ObjectUnlinkedEvent(branch, self, user=user))
@@ -1385,7 +1387,7 @@ class Bug(SQLBase, InformationTypeMixin):
             branch_ids = [branch.id for branch in linked_branches]
             results = Store.of(self).find(
                 BugBranch,
-                BugBranch.bug == self, In(BugBranch.branchID, branch_ids))
+                BugBranch.bug == self, In(BugBranch.branch_id, branch_ids))
             return results.order_by(BugBranch.id)
 
     def linkMergeProposal(self, merge_proposal, user, check_permissions=True):
diff --git a/lib/lp/bugs/model/bugbranch.py b/lib/lp/bugs/model/bugbranch.py
index 9a8728b..665f177 100644
--- a/lib/lp/bugs/model/bugbranch.py
+++ b/lib/lp/bugs/model/bugbranch.py
@@ -1,16 +1,23 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes for linking bugtasks and branches."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
-__all__ = ["BugBranch",
-           "BugBranchSet"]
+__all__ = [
+    "BugBranch",
+    "BugBranchSet",
+    ]
 
-from sqlobject import (
-    ForeignKey,
-    IntCol,
+import pytz
+from storm.locals import (
+    DateTime,
+    Int,
+    Reference,
+    Store,
     )
 from zope.interface import implementer
 
@@ -20,23 +27,37 @@ from lp.bugs.interfaces.bugbranch import (
     )
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.database.constants import UTC_NOW
-from lp.services.database.datetimecol import UtcDateTimeCol
 from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import SQLBase
+from lp.services.database.stormbase import StormBase
 
 
 @implementer(IBugBranch)
-class BugBranch(SQLBase):
+class BugBranch(StormBase):
     """See `IBugBranch`."""
 
-    datecreated = UtcDateTimeCol(notNull=True, default=UTC_NOW)
-    bug = ForeignKey(dbName="bug", foreignKey="Bug", notNull=True)
-    branch_id = IntCol(dbName="branch", notNull=True)
-    branch = ForeignKey(dbName="branch", foreignKey="Branch", notNull=True)
+    __storm_table__ = 'BugBranch'
+
+    id = Int(primary=True)
+
+    datecreated = DateTime(
+        name='datecreated', tzinfo=pytz.UTC, allow_none=False, default=UTC_NOW)
+    bug_id = Int(name='bug', allow_none=False)
+    bug = Reference(bug_id, 'Bug.id')
+    branch_id = Int(name="branch", allow_none=False)
+    branch = Reference(branch_id, 'Branch.id')
+
+    registrant_id = Int(
+        name='registrant', allow_none=False, validator=validate_public_person)
+    registrant = Reference(registrant_id, 'Person.id')
+
+    def __init__(self, branch, bug, registrant):
+        super(BugBranch, self).__init__()
+        self.branch = branch
+        self.bug = bug
+        self.registrant = registrant
 
-    registrant = ForeignKey(
-        dbName='registrant', foreignKey='Person',
-        storm_validator=validate_public_person, notNull=True)
+    def destroySelf(self):
+        Store.of(self).remove(self)
 
 
 @implementer(IBugBranchSet)
@@ -54,7 +75,7 @@ class BugBranchSet:
 
         visible = get_bug_privacy_filter(user)
         return IStore(BugBranch).find(
-            BugBranch.branchID,
+            BugBranch.branch_id,
             BugBranch.branch_id.is_in(branch_ids),
-            BugTaskFlat.bug_id == BugBranch.bugID,
+            BugTaskFlat.bug_id == BugBranch.bug_id,
             visible).config(distinct=True)
diff --git a/lib/lp/bugs/model/bugtask.py b/lib/lp/bugs/model/bugtask.py
index b682378..2c3b6ef 100644
--- a/lib/lp/bugs/model/bugtask.py
+++ b/lib/lp/bugs/model/bugtask.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Classes that implement IBugTask and its related interfaces."""
@@ -1392,7 +1392,7 @@ class BugTaskSet:
                 [(u'bug', unicode(bug_id)) for bug_id in bug_ids],
                 types=[u'specification']).keys())
         bug_ids_with_branches = set(IStore(BugBranch).find(
-                BugBranch.bugID, BugBranch.bugID.is_in(bug_ids)))
+                BugBranch.bug_id, BugBranch.bug_id.is_in(bug_ids)))
         # Badging looks up milestones too : eager load into the storm cache.
         milestoneset = getUtility(IMilestoneSet)
         # And trigger a load:
diff --git a/lib/lp/bugs/model/bugtasksearch.py b/lib/lp/bugs/model/bugtasksearch.py
index 18ea899..39c5cb8 100644
--- a/lib/lp/bugs/model/bugtasksearch.py
+++ b/lib/lp/bugs/model/bugtasksearch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -700,11 +700,11 @@ def _build_query(params):
         extra_clauses.append(hw_clause)
 
     def make_branch_clause(branches=None):
-        where = [BugBranch.bugID == BugTaskFlat.bug_id]
+        where = [BugBranch.bug_id == BugTaskFlat.bug_id]
         if branches is not None:
             where.append(
                 search_value_to_storm_where_condition(
-                    BugBranch.branchID, branches))
+                    BugBranch.branch_id, branches))
         return Exists(Select(1, tables=[BugBranch], where=And(*where)))
 
     def make_merge_proposal_clause(merge_proposals=None):
diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py
index cd0b4cc..cd9d69f 100644
--- a/lib/lp/code/model/branch.py
+++ b/lib/lp/code/model/branch.py
@@ -38,7 +38,10 @@ from storm.expr import (
     Select,
     SQL,
     )
-from storm.locals import AutoReload
+from storm.locals import (
+    AutoReload,
+    ReferenceSet,
+    )
 from storm.store import Store
 from zope.component import getUtility
 from zope.event import notify
@@ -441,12 +444,12 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
         'Person', joinColumn='branch', otherColumn='person',
         intermediateTable='BranchSubscription', orderBy='name')
 
-    bug_branches = SQLMultipleJoin(
-        'BugBranch', joinColumn='branch', orderBy='id')
+    bug_branches = ReferenceSet(
+        'id', 'BugBranch.branch_id', order_by='BugBranch.id')
 
-    linked_bugs = SQLRelatedJoin(
-        'Bug', joinColumn='branch', otherColumn='bug',
-        intermediateTable='BugBranch', orderBy='id')
+    linked_bugs = ReferenceSet(
+        'id', 'BugBranch.branch_id',
+        'BugBranch.bug_id', 'Bug.id', order_by='Bug.id')
 
     def getLinkedBugTasks(self, user, status_filter=None):
         """See `IBranch`."""
diff --git a/lib/lp/code/model/branchcollection.py b/lib/lp/code/model/branchcollection.py
index 24c23fa..79411cc 100644
--- a/lib/lp/code/model/branchcollection.py
+++ b/lib/lp/code/model/branchcollection.py
@@ -334,7 +334,7 @@ class GenericBranchCollection:
             # need for validity etc in the /branches API call.
             load_related(Person, rows,
                 ['ownerID', 'registrantID', 'reviewerID'])
-            load_referencing(BugBranch, rows, ['branchID'])
+            load_referencing(BugBranch, rows, ['branch_id'])
 
         def cache_permission(branch):
             if self._user:
@@ -549,11 +549,11 @@ class GenericBranchCollection:
             store = IStore(BugBranch)
             rs = store.using(
                 BugBranch,
-                Join(BugTask, BugTask.bugID == BugBranch.bugID),
+                Join(BugTask, BugTask.bugID == BugBranch.bug_id),
             ).find(
                 (BugTask, BugBranch),
-                BugBranch.bugID.is_in(bug_ids),
-                BugBranch.branchID.is_in(source_branch_ids)
+                BugBranch.bug_id.is_in(bug_ids),
+                BugBranch.branch_id.is_in(source_branch_ids)
             )
 
             # Build up a collection of bugtasks for each branch
@@ -731,7 +731,7 @@ class GenericBranchCollection:
         """See `IBranchCollection`."""
         bug_ids = [bug.id for bug in bugs]
         return self._filterBy(
-            [In(BugBranch.bugID, bug_ids)],
+            [In(BugBranch.bug_id, bug_ids)],
             table=BugBranch,
             join=Join(BugBranch, BugBranch.branch == Branch.id),
             symmetric=False)
diff --git a/lib/lp/code/model/tests/test_branch.py b/lib/lp/code/model/tests/test_branch.py
index 2bd0b62..3a04a03 100644
--- a/lib/lp/code/model/tests/test_branch.py
+++ b/lib/lp/code/model/tests/test_branch.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for Branches."""
@@ -1590,10 +1590,10 @@ class TestBranchDeletionConsequences(TestCase):
         """break_links allows deleting a branch with a bug."""
         bug1 = self.factory.makeBug()
         bug1.linkBranch(self.branch, self.branch.owner)
-        bug_branch1 = bug1.linked_bugbranches[0]
+        bug_branch1 = bug1.linked_bugbranches.first()
         bug_branch1_id = removeSecurityProxy(bug_branch1).id
         self.branch.destroySelf(break_references=True)
-        self.assertRaises(SQLObjectNotFound, BugBranch.get, bug_branch1_id)
+        self.assertIsNone(IStore(BugBranch).get(BugBranch, bug_branch1_id))
 
     def test_branchWithSpecRequirements(self):
         """Deletion requirements for a branch with a spec are right."""
@@ -2761,7 +2761,7 @@ class TestBranchBugLinks(TestCaseWithFactory):
 
         self.assertEqual(branch.linked_bugs.count(), 1)
 
-        linked_bug = branch.linked_bugs[0]
+        linked_bug = branch.linked_bugs.first()
 
         self.assertEqual(linked_bug.id, bug.id)