launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Convert SpecificationBranch to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

diff --git a/lib/lp/blueprints/doc/specification-branch.txt b/lib/lp/blueprints/doc/specification-branch.txt
index 7de9af4..02ccb50 100644
--- a/lib/lp/blueprints/doc/specification-branch.txt
+++ b/lib/lp/blueprints/doc/specification-branch.txt
@@ -5,7 +5,10 @@ Specification Branch Links
     >>> from lp.blueprints.interfaces.specificationbranch import (
     ...     ISpecificationBranch)
     >>> from lp.blueprints.model.specification import SpecificationBranch
-    >>> verifyObject(ISpecificationBranch, SpecificationBranch.get(1))
+    >>> from lp.services.database.interfaces import IStore
+    >>> verifyObject(
+    ...     ISpecificationBranch,
+    ...     IStore(SpecificationBranch).get(SpecificationBranch, 1))
     True
 
 A specification can be linked to a number of branches.  For example,
diff --git a/lib/lp/blueprints/model/specification.py b/lib/lp/blueprints/model/specification.py
index 6234088..cffe95b 100644
--- a/lib/lp/blueprints/model/specification.py
+++ b/lib/lp/blueprints/model/specification.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 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
@@ -255,7 +255,7 @@ class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
     def linked_branches(self):
         return list(Store.of(self).find(
             SpecificationBranch,
-            SpecificationBranch.specificationID == self.id).order_by(
+            SpecificationBranch.specification_id == self.id).order_by(
                 SpecificationBranch.id))
 
     def _fetch_children_or_parents(self, join_cond, cond, user):
@@ -871,8 +871,10 @@ class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
 
     # branches
     def getBranchLink(self, branch):
-        return SpecificationBranch.selectOneBy(
-            specificationID=self.id, branchID=branch.id)
+        return Store.of(self).find(
+            SpecificationBranch,
+            SpecificationBranch.specification == self,
+            SpecificationBranch.branch == branch).one()
 
     def linkBranch(self, branch, registrant):
         branch_link = self.getBranchLink(branch)
diff --git a/lib/lp/blueprints/model/specificationbranch.py b/lib/lp/blueprints/model/specificationbranch.py
index 8347401..39bfee7 100644
--- a/lib/lp/blueprints/model/specificationbranch.py
+++ b/lib/lp/blueprints/model/specificationbranch.py
@@ -1,8 +1,10 @@
-# 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 specifications and branches."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 
 __all__ = [
@@ -10,9 +12,12 @@ __all__ = [
     "SpecificationBranchSet",
     ]
 
-from sqlobject import (
-    ForeignKey,
-    IN,
+import pytz
+from storm.locals import (
+    DateTime,
+    Int,
+    Reference,
+    Store,
     )
 from zope.interface import implementer
 
@@ -22,22 +27,37 @@ from lp.blueprints.interfaces.specificationbranch 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.sqlbase import SQLBase
+from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
 
 
 @implementer(ISpecificationBranch)
-class SpecificationBranch(SQLBase):
+class SpecificationBranch(StormBase):
     """See `ISpecificationBranch`."""
 
-    datecreated = UtcDateTimeCol(notNull=True, default=UTC_NOW)
-    specification = ForeignKey(dbName="specification",
-                               foreignKey="Specification", notNull=True)
-    branch = ForeignKey(dbName="branch", foreignKey="Branch", notNull=True)
+    __storm_table__ = 'SpecificationBranch'
+
+    id = Int(primary=True)
+
+    datecreated = DateTime(
+        name='datecreated', tzinfo=pytz.UTC, allow_none=False, default=UTC_NOW)
+    specification_id = Int(name='specification', allow_none=False)
+    specification = Reference(specification_id, 'Specification.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, specification, branch, registrant):
+        super(SpecificationBranch, self).__init__()
+        self.specification = specification
+        self.branch = branch
+        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(ISpecificationBranchSet)
@@ -50,7 +70,8 @@ class SpecificationBranchSet:
         if not branch_ids:
             return []
 
-        # When specification gain the ability to be private, this
+        # When specifications gain the ability to be private, this
         # method will need to be updated to enforce the privacy checks.
-        return SpecificationBranch.select(
-            IN(SpecificationBranch.q.branchID, branch_ids))
+        return IStore(SpecificationBranch).find(
+            SpecificationBranch,
+            SpecificationBranch.branch_id.is_in(branch_ids))
diff --git a/lib/lp/blueprints/model/specificationsearch.py b/lib/lp/blueprints/model/specificationsearch.py
index da1f3e1..a0aed9a 100644
--- a/lib/lp/blueprints/model/specificationsearch.py
+++ b/lib/lp/blueprints/model/specificationsearch.py
@@ -1,4 +1,4 @@
-# Copyright 2013-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Helper methods to search specifications."""
@@ -155,7 +155,7 @@ def search_specifications(context, base_clauses, user, sort=None,
                     work_items_by_spec[spec.id], key=lambda wi: wi.sequence)
         if need_branches:
             spec_branches = load_referencing(
-                SpecificationBranch, rows, ['specificationID'])
+                SpecificationBranch, rows, ['specification_id'])
             for sbranch in spec_branches:
                 spec_cache = get_property_cache(sbranch.specification)
                 spec_cache.linked_branches.append(sbranch)
diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py
index cd9d69f..c9e43db 100644
--- a/lib/lp/code/model/branch.py
+++ b/lib/lp/code/model/branch.py
@@ -468,8 +468,9 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
         """See `IBranch`."""
         return bug.unlinkBranch(self, user)
 
-    spec_links = SQLMultipleJoin(
-        'SpecificationBranch', joinColumn='branch', orderBy='id')
+    spec_links = ReferenceSet(
+        'id', 'SpecificationBranch.branch_id',
+        order_by='SpecificationBranch.id')
 
     def getSpecificationLinks(self, user):
         """See `IBranch`."""
@@ -477,10 +478,10 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
             SpecificationBranch,
             Join(
                 Specification,
-                SpecificationBranch.specificationID == Specification.id)]
+                SpecificationBranch.specification_id == Specification.id)]
         return Store.of(self).using(*tables).find(
             SpecificationBranch,
-            SpecificationBranch.branchID == self.id,
+            SpecificationBranch.branch_id == self.id,
             *get_specification_privacy_filter(user))
 
     def linkSpecification(self, spec, registrant):
diff --git a/lib/lp/code/model/tests/test_branch.py b/lib/lp/code/model/tests/test_branch.py
index 3a04a03..1978e88 100644
--- a/lib/lp/code/model/tests/test_branch.py
+++ b/lib/lp/code/model/tests/test_branch.py
@@ -1613,10 +1613,10 @@ class TestBranchDeletionConsequences(TestCase):
         spec2.linkBranch(self.branch, self.branch.owner)
         spec2_branch_id = self.branch.spec_links[1].id
         self.branch.destroySelf(break_references=True)
-        self.assertRaises(
-            SQLObjectNotFound, SpecificationBranch.get, spec1_branch_id)
-        self.assertRaises(
-            SQLObjectNotFound, SpecificationBranch.get, spec2_branch_id)
+        self.assertIsNone(IStore(SpecificationBranch).get(
+            SpecificationBranch, spec1_branch_id))
+        self.assertIsNone(IStore(SpecificationBranch).get(
+            SpecificationBranch, spec2_branch_id))
 
     def test_branchWithSeriesRequirements(self):
         """Deletion requirements for a series' branch are right."""
@@ -1750,8 +1750,8 @@ class TestBranchDeletionConsequences(TestCase):
         spec_link = spec.linkBranch(self.branch, self.branch.owner)
         spec_link_id = spec_link.id
         DeletionCallable(spec, 'blah', spec_link.destroySelf)()
-        self.assertRaises(SQLObjectNotFound, SpecificationBranch.get,
-                          spec_link_id)
+        self.assertIsNone(IStore(SpecificationBranch).get(
+            SpecificationBranch, spec_link_id))
 
     def test_DeleteCodeImport(self):
         """DeleteCodeImport.__call__ must delete the CodeImport."""