launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Convert SpecificationSubscription to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

diff --git a/lib/lp/blueprints/browser/sprint.py b/lib/lp/blueprints/browser/sprint.py
index da94d43..6ba5372 100644
--- a/lib/lp/blueprints/browser/sprint.py
+++ b/lib/lp/blueprints/browser/sprint.py
@@ -506,11 +506,11 @@ class SprintMeetingExportView(LaunchpadView):
         people = defaultdict(dict)
         # Attendees per specification.
         for subscription in load_referencing(SpecificationSubscription,
-                model_specs, ['specificationID']):
-            if subscription.personID not in attendee_set:
+                model_specs, ['specification_id']):
+            if subscription.person_id not in attendee_set:
                 continue
-            people[subscription.specificationID][
-                subscription.personID] = subscription.essential
+            people[subscription.specification_id][
+                subscription.person_id] = subscription.essential
 
         # Spec specials - drafter/assignee.  Don't need approver for
         # performance, as specifications() above eager-loaded the
diff --git a/lib/lp/blueprints/doc/sprint-meeting-export.txt b/lib/lp/blueprints/doc/sprint-meeting-export.txt
index b66423b..ea315da 100644
--- a/lib/lp/blueprints/doc/sprint-meeting-export.txt
+++ b/lib/lp/blueprints/doc/sprint-meeting-export.txt
@@ -64,7 +64,8 @@ and check the list of interested people:
 
     >>> essential=False
     >>> ext_spec.subscribe(sampleperson, sampleperson, essential)
-    <SpecificationSubscription at ...>
+    <lp.blueprints.model.specificationsubscription.SpecificationSubscription
+    object at ...>
 
     >>> view = getMultiAdapter((ubz, request), name='+temp-meeting-export')
     >>> view.initialize()
diff --git a/lib/lp/blueprints/model/specification.py b/lib/lp/blueprints/model/specification.py
index 7ac13e5..e0d4001 100644
--- a/lib/lp/blueprints/model/specification.py
+++ b/lib/lp/blueprints/model/specification.py
@@ -233,12 +233,13 @@ class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
     date_started = UtcDateTimeCol(notNull=False, default=None)
 
     # useful joins
-    _subscriptions = SQLMultipleJoin('SpecificationSubscription',
-        joinColumn='specification', orderBy='id')
-    subscribers = SQLRelatedJoin('Person',
-        joinColumn='specification', otherColumn='person',
-        intermediateTable='SpecificationSubscription',
-        orderBy=['display_name', 'name'])
+    _subscriptions = ReferenceSet(
+        'id', 'SpecificationSubscription.specification_id',
+        order_by='SpecificationSubscription.id')
+    subscribers = ReferenceSet(
+        'id', 'SpecificationSubscription.specification_id',
+        'SpecificationSubscription.person_id', 'Person.id',
+        order_by=('Person.display_name', 'Person.name'))
     sprint_links = ReferenceSet(
         '<primary key>', 'SprintSpecification.specification_id',
         order_by='SprintSpecification.id')
@@ -715,8 +716,8 @@ class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
     # subscriptions
     def subscription(self, person):
         """See ISpecification."""
-        return SpecificationSubscription.selectOneBy(
-                specification=self, person=person)
+        return IStore(SpecificationSubscription).find(
+            SpecificationSubscription, specification=self, person=person).one()
 
     def getSubscriptionByName(self, name):
         """See ISpecification."""
@@ -776,7 +777,7 @@ class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
                             unsubscribed_by.displayname,
                             person.displayname))
                 get_property_cache(self).subscriptions.remove(sub)
-                SpecificationSubscription.delete(sub.id)
+                IStore(SpecificationSubscription).remove(sub)
                 artifacts_to_delete = getUtility(
                     IAccessArtifactSource).find([self])
                 getUtility(IAccessArtifactGrantSource).revokeByArtifact(
@@ -929,7 +930,8 @@ class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
             raise CannotChangeInformationType("Forbidden by project policy.")
         self.information_type = information_type
         reconcile_access_for_artifact(self, information_type, [self.target])
-        if information_type in PRIVATE_INFORMATION_TYPES and self.subscribers:
+        if (information_type in PRIVATE_INFORMATION_TYPES and
+                not self.subscribers.is_empty()):
             # Grant the subscribers access if they do not have a
             # policy grant.
             service = getUtility(IService, 'sharing')
diff --git a/lib/lp/blueprints/model/specificationsubscription.py b/lib/lp/blueprints/model/specificationsubscription.py
index cc5bf66..17f70a1 100644
--- a/lib/lp/blueprints/model/specificationsubscription.py
+++ b/lib/lp/blueprints/model/specificationsubscription.py
@@ -5,9 +5,10 @@ __metaclass__ = type
 
 __all__ = ['SpecificationSubscription']
 
-from sqlobject import (
-    BoolCol,
-    ForeignKey,
+from storm.locals import (
+    Bool,
+    Int,
+    Reference,
     )
 from zope.component import getUtility
 from zope.interface import implementer
@@ -21,20 +22,26 @@ from lp.registry.interfaces.accesspolicy import (
     )
 from lp.registry.interfaces.person import validate_person
 from lp.registry.interfaces.role import IPersonRoles
-from lp.services.database.sqlbase import SQLBase
+from lp.services.database.stormbase import StormBase
 
 
 @implementer(ISpecificationSubscription)
-class SpecificationSubscription(SQLBase):
+class SpecificationSubscription(StormBase):
     """A subscription for person to a spec."""
 
-    _table = 'SpecificationSubscription'
-    specification = ForeignKey(dbName='specification',
-        foreignKey='Specification', notNull=True)
-    person = ForeignKey(
-        dbName='person', foreignKey='Person',
-        storm_validator=validate_person, notNull=True)
-    essential = BoolCol(notNull=True, default=False)
+    __storm_table__ = 'SpecificationSubscription'
+    id = Int(primary=True)
+    specification_id = Int(name='specification', allow_none=False)
+    specification = Reference(specification_id, 'Specification.id')
+    person_id = Int(name='person', validator=validate_person, allow_none=False)
+    person = Reference(person_id, 'Person.id')
+    essential = Bool(allow_none=False, default=False)
+
+    def __init__(self, specification, person, essential=False):
+        super(SpecificationSubscription, self).__init__()
+        self.specification = specification
+        self.person = person
+        self.essential = essential
 
     def canBeUnsubscribedByUser(self, user):
         """See `ISpecificationSubscription`."""
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index e1406ca..2c228e6 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -851,7 +851,8 @@ class Person(
         if SpecificationFilter.SUBSCRIBER in filter:
             role_clauses.append(
                 Specification.id.is_in(
-                    Select(SpecificationSubscription.specificationID,
+                    Select(
+                        SpecificationSubscription.specification_id,
                         [SpecificationSubscription.person == self])))
 
         clauses = [Or(*role_clauses)]
diff --git a/lib/lp/registry/model/sharingjob.py b/lib/lp/registry/model/sharingjob.py
index dbb29e5..04e2daa 100644
--- a/lib/lp/registry/model/sharingjob.py
+++ b/lib/lp/registry/model/sharingjob.py
@@ -428,7 +428,7 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
                         TeamParticipation.personID,
                         where=TeamParticipation.team == self.grantee)))
             specification_filters.append(
-                In(SpecificationSubscription.personID,
+                In(SpecificationSubscription.person_id,
                     Select(
                         TeamParticipation.personID,
                         where=TeamParticipation.team == self.grantee)))
@@ -472,13 +472,13 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
                     sub.person, self.requestor, ignore_permissions=True)
         if specification_filters:
             specification_filters.append(Not(*get_specification_privacy_filter(
-                SpecificationSubscription.personID)))
+                SpecificationSubscription.person_id)))
             tables = (
                 SpecificationSubscription,
                 Join(
                     Specification,
                     Specification.id ==
-                        SpecificationSubscription.specificationID))
+                        SpecificationSubscription.specification_id))
             specifications_subscriptions = IStore(
                 SpecificationSubscription).using(*tables).find(
                 SpecificationSubscription, *specification_filters).config(