Merge lp:~danilo/launchpad/ss-storm-rewrite-methods into lp:launchpad

[Данило Шеган]

[r=gmb][ui=none][no-qa] Convert StructuralSubscription to native Storm class.

= Convert StructuralSubscription to Storm =

This branch completes the migration of StructuralSubscription to Storm. The first step in lp:~danilo/launchpad/bugs-subscriptions was to replace column definitions with Storm versions.

This branch continues on that and moves all .select() and .selectBy() calls on StructuralSubscription to .find calls, uses store.remove() instead of object.destroySelf(), provides a constructor similar to one provided by SQLBase and then finally switches the class to inherit from Storm instead of SQLBase.

Followed https://dev.launchpad.net/StormMigrationGuide (constructor switch was not documented there, so I'll be adding it shortly).

== Implementation details ==

There are not enough unit tests for the functionality I am changing, but the full test run over all the layers of implementation (doc tests, unit tests, API tests and browser tests) did catch a few bugs (or incompatible changes that I did). While I'd love to add new tests for all the methods in StructuralSubscriptionTargetMixin, with the existing test coverage and our plan (where we'd allow more than one StructuralSubscription per target and person), most of the code I'd write tests for would go away.

It also includes slight optimizations in two places where we go over all the subscriptions to find one for a particular subscriber. It now executes a query which returns just one result instead.

To achieve this, subscriber parameter is added to getSubscriptions(), but it's not exposed in the interface because we are only using it from inside the module. That looks dirty even to me, so I'd be happy to change it if a reviewer thinks I should do it.

I am ignoring doctest lint errors since it might make diff harder to read. I'd be happy to update it after the review is done.

== Tests ==

bin/test -cvvt bugsubscription.txt -t bugnotification-sending.txt -t structural-subscriptions.txt -t test_structuralsubscription

== Demo and Q/A ==

no-qa

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/doc/bugsubscription.txt
  lib/lp/registry/model/structuralsubscription.py
  lib/lp/registry/model/person.py

./lib/lp/bugs/doc/bugsubscription.txt
       1: narrative uses a moin header.
       6: narrative uses a moin header.
      25: narrative uses a moin header.
     438: source has bad indentation.
     442: source has bad indentation.
     459: source has bad indentation.
     474: source has bad indentation.
     487: source has bad indentation.
     494: want exceeds 78 characters.
     528: narrative uses a moin header.
     589: narrative uses a moin header.
     644: narrative uses a moin header.
     852: narrative uses a moin header.

[Graham Binns (gmb)]

review: Approve (code)

[Robert Collins (lifeless)]

We can't use the Storm base class directly, we need a launchpad
specific one with CachedProperty __invalidate__ hooked in.

=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt	2011-01-18 22:16:03 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt	2011-01-20 17:50:31 +0000
@@ -1,9 +1,11 @@
-= BugSubscription =
+BugSubscription
+===============
 
 Users can get email notifications of changes to bugs by subscribing to
 them.
 
-== Bug Subscriber APIs ==
+Bug Subscriber APIs
+-------------------
 
 First, let's login:
 
@@ -22,7 +24,8 @@
 This list returns only *direct* subscribers. Bugs can also have
 indirect subscribers.
 
-=== Direct vs. Indirect Subscriptions ===
+Direct vs. Indirect Subscriptions
+.................................
 
 A user is directly subscribed to a bug if they or someone else has
 subscribed them to the bug.
@@ -147,6 +150,7 @@
     >>> subscription_no_priv = linux_source.addBugSubscription(
     ...     mr_no_privs, mr_no_privs)
 
+    >>> transaction.commit()
     >>> print_displayname(
     ...     sub.subscriber for sub in linux_source.bug_subscriptions)
     No Privileges Person
@@ -434,12 +438,12 @@
 a bug, and why, IBug.getBugNotificationRecipients() assembles an
 INotificationRecipientSet instance for us:
 
-   >>> recipients = linux_source_bug.getBugNotificationRecipients()
+    >>> recipients = linux_source_bug.getBugNotificationRecipients()
 
 You can query for the addresses and reasons:
 
-   >>> addresses = recipients.getEmails()
-   >>> [(address, recipients.getReason(address)[1]) for address in addresses]
+    >>> addresses = recipients.getEmails()
+    >>> [(address, recipients.getReason(address)[1]) for address in addresses]
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
@@ -455,10 +459,10 @@
 getBugNotificationRecipients() even if the parameter level is larger than
 his structural subscription setting.
 
-   >>> recipients = linux_source_bug.getBugNotificationRecipients(
-   ...     level=BugNotificationLevel.COMMENTS)
-   >>> addresses = recipients.getEmails()
-   >>> [(address, recipients.getReason(address)[1]) for address in addresses]
+    >>> recipients = linux_source_bug.getBugNotificationRecipients(
+    ...     level=BugNotificationLevel.COMMENTS)
+    >>> addresses = recipients.getEmails()
+    >>> [(address, recipients.getReason(address)[1]) for address in addresses]
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
@@ -470,11 +474,11 @@
 longer included in the result of getBugNotificationRecipients() for
 the COMMENTS level...
 
-   >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
-   >>> recipients = linux_source_bug.getBugNotificationRecipients(
-   ...     level=BugNotificationLevel.COMMENTS)
-   >>> addresses = recipients.getEmails()
-   >>> [(address, recipients.getReason(address)[1]) for address in addresses]
+    >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
+    >>> recipients = linux_source_bug.getBugNotificationRecipients(
+    ...     level=BugNotificationLevel.COMMENTS)
+    >>> addresses = recipients.getEmails()
+    >>> [(address, recipients.getReason(address)[1]) for address in addresses]
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
@@ -483,11 +487,11 @@
 
 ...but remains included for the level LIFECYCLE.
 
-   >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
-   >>> recipients = linux_source_bug.getBugNotificationRecipients(
-   ...     level=BugNotificationLevel.LIFECYCLE)
-   >>> addresses = recipients.getEmails()
-   >>> [(address, recipients.getReason(address)[1]) for address in addresses]
+    >>> linux_source_bug.unsubscribe(mr_no_privs, mr_no_privs)
+    >>> recipients = linux_source_bug.getBugNotificationRecipients(
+    ...     level=BugNotificationLevel.LIFECYCLE)
+    >>> addresses = recipients.getEmails()
+    >>> [(address, recipients.getReason(address)[1]) for address in addresses]
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
      ('[e-mail]', u'Subscriber (linux-source-2.6.15 in ubuntu)'),
@@ -524,7 +528,8 @@
     >>> bug_five.isSubscribedToDupes(sample_person)
     True
 
-== Subscribing and Unsubscribing ==
+Subscribing and Unsubscribing
+-----------------------------
 
 To subscribe people to and unsubscribe people from a bug, use
 IBug.subscribe and IBug.unsubscribe:
@@ -585,7 +590,8 @@
     False
 
 
-=== Determining whether a user can unsubscribe someone ===
+Determining whether a user can unsubscribe someone
+..................................................
 
 As user can't unsubscribe just anyone from a bug. To check whether
 someone can be unusubscribed, the canBeUnsubscribedByUser() method on
@@ -640,14 +646,12 @@
     UserCannotUnsubscribePerson: ...
 
 
-== Automatic Subscriptions on Bug Creation ==
+Automatic Subscriptions on Bug Creation
+---------------------------------------
 
 When a new bug is opened, only the bug reporter is automatically, explicitly
 subscribed to the bug:
 
-XXX: Brad Bollenbach, 2005-11-25: These bits need real sample data. See
-https://launchpad.net/bugs/5484.
-
 Define a function that get subscriber email addresses back conveniently:
 
     >>> def getSubscribers(bug):
@@ -804,7 +808,7 @@
 So, if the Ubuntu team is added as a bug contact to evolution:
 
     >>> evolution.addBugSubscription(ubuntu_team, ubuntu_team)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
 
 The team will be implicitly subscribed to the previous bug we
 created. (Remember that Sample Person is also implicitly subscribed
@@ -848,7 +852,8 @@
     ['[e-mail]', '[e-mail]']
 
 
-== Subscribed by ==
+Subscribed by
+-------------
 
 Each `BugSubscription` records who created it, and provides a handy
 utility method for formatting this information. The methods
 
=== modified file 'lib/lp/bugs/doc/bugtask-search.txt'
--- lib/lp/bugs/doc/bugtask-search.txt	2010-10-25 15:03:38 +0000
+++ lib/lp/bugs/doc/bugtask-search.txt	2011-01-20 17:50:31 +0000
@@ -1230,7 +1230,7 @@
     ...     name='product-struct-subber')
     >>> firefox.addBugSubscription(product_struct_subber,
     ...     product_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> product_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=product_struct_subber)
     >>> found_bugtasks = bugtask_set.search(product_struct_sub_search)
@@ -1248,7 +1248,7 @@
     ...     name='series-struct-subber')
     >>> product_series.addBugSubscription(series_struct_subber,
     ...     series_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> series_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=series_struct_subber)
     >>> found_bugtasks = bugtask_set.search(series_struct_sub_search)
@@ -1269,7 +1269,7 @@
     ...     name='project-struct-subber')
     >>> project.addBugSubscription(project_struct_subber,
     ...     project_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> project_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=project_struct_subber)
     >>> found_bugtasks = bugtask_set.search(project_struct_sub_search)
@@ -1285,7 +1285,7 @@
     >>> product2.project = project
     >>> product2.addBugSubscription(project_struct_subber,
     ...     project_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> project_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=project_struct_subber)
     >>> found_bugtasks = bugtask_set.search(project_struct_sub_search)
@@ -1300,7 +1300,7 @@
     ...     name='milestone-struct-subber')
     >>> product_milestone.addBugSubscription(milestone_struct_subber,
     ...     milestone_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> milestone_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=milestone_struct_subber)
     >>> found_bugtasks = bugtask_set.search(milestone_struct_sub_search)
@@ -1315,7 +1315,7 @@
     ...     name='distro-struct-subber')
     >>> ubuntu.addBugSubscription(distro_struct_subber,
     ...     distro_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> distro_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=distro_struct_subber)
     >>> found_bugtasks = bugtask_set.search(distro_struct_sub_search)
@@ -1332,7 +1332,7 @@
     ...     name='distro-series-struct-subber')
     >>> hoary.addBugSubscription(distro_series_struct_subber,
     ...     distro_series_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> distro_series_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=distro_series_struct_subber)
     >>> found_bugtasks = bugtask_set.search(distro_series_struct_sub_search)
@@ -1349,7 +1349,7 @@
     ...     name='package-struct-subber')
     >>> ubuntu_firefox.addBugSubscription(package_struct_subber,
     ...     package_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> package_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=package_struct_subber)
     >>> found_bugtasks = bugtask_set.search(package_struct_sub_search)
@@ -1363,7 +1363,7 @@
 
     >>> product_series.addBugSubscription(package_struct_subber,
     ...     package_struct_subber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> package_struct_sub_search = BugTaskSearchParams(
     ...     user=None, structural_subscriber=package_struct_subber)
     >>> found_bugtasks = bugtask_set.search(package_struct_sub_search)
 
=== modified file 'lib/lp/bugs/doc/initial-bug-contacts.txt'
--- lib/lp/bugs/doc/initial-bug-contacts.txt	2010-12-24 11:02:19 +0000
+++ lib/lp/bugs/doc/initial-bug-contacts.txt	2011-01-20 17:50:31 +0000
@@ -38,7 +38,7 @@
     >>> login("[e-mail]")
 
     >>> debian_firefox.addBugSubscription(sample_person, sample_person)
-    <StructuralSubscription ...>
+    <...StructuralSubscription object at ...>
     >>> [pbc.subscriber.name for pbc in debian_firefox.bug_subscriptions]
     [u'name12']
 
@@ -46,13 +46,13 @@
 already subscribe to that package will return the existing subscription.
 
     >>> debian_firefox.addBugSubscription(sample_person, sample_person)
-    <StructuralSubscription ...>
+    <...StructuralSubscription object at ...>
 
 Let's add an ITeam as one of the subscribers:
 
     >>> ubuntu_team = personset.getByName("ubuntu-team")
     >>> debian_firefox.addBugSubscription(ubuntu_team, ubuntu_team)
-    <StructuralSubscription ...>
+    <...StructuralSubscription object at ...>
 
     >>> sorted([sub.subscriber.name for sub in debian_firefox.bug_subscriptions])
     [u'name12', u'ubuntu-team']
@@ -125,9 +125,9 @@
 
     >>> daf = personset.getByName("daf")
     >>> ubuntu_pmount.addBugSubscription(daf, daf)
-    <StructuralSubscription ...>
+    <...StructuralSubscription object at ...>
     >>> ubuntu_pmount.addBugSubscription(sample_person, sample_person)
-    <StructuralSubscription ...>
+    <...StructuralSubscription object at ...>
 
     >>> old_state = Snapshot(
     ...     bug_one_in_ubuntu_firefox, providing=IDistroBugTask)
@@ -247,7 +247,7 @@
     >>> ubuntu_gnome.preferredemail is None
     True
     >>> ubuntu_pmount.addBugSubscription(ubuntu_gnome, ubuntu_gnome)
-    <StructuralSubscription ...>
+    <...StructuralSubscription object at ...>
 
     >>> old_state = Snapshot(
     ...     bug_one_in_ubuntu_firefox, providing=IDistroBugTask)
@@ -279,7 +279,7 @@
 
     >>> pitti = personset.getByName("martin-pitt")
     >>> ubuntu_firefox.addBugSubscription(pitti, pitti)
-    <StructuralSubscription ...>
+    <...StructuralSubscription object at ...>
 
 and then the bug gets reassigned to mozilla firefox:
 
 
=== modified file 'lib/lp/bugs/stories/patches-view/patches-view.txt'
--- lib/lp/bugs/stories/patches-view/patches-view.txt	2010-07-17 21:02:11 +0000
+++ lib/lp/bugs/stories/patches-view/patches-view.txt	2011-01-20 17:50:31 +0000
@@ -420,7 +420,7 @@
     ...     name="project-subscriber", displayname="Project Subscriber")
     >>> add_bug_sub = lambda *args: patchy_product.addBugSubscription(*args)
     >>> with_anybody(add_bug_sub)(project_subscriber, project_subscriber)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
     >>> from zope.security.proxy import removeSecurityProxy
     >>> subscriber_name = removeSecurityProxy(project_subscriber).name
     >>> anon_browser.open(
 
=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
--- lib/lp/bugs/tests/bugs-emailinterface.txt	2010-12-13 13:40:38 +0000
+++ lib/lp/bugs/tests/bugs-emailinterface.txt	2011-01-20 17:50:31 +0000
@@ -1679,7 +1679,7 @@
     >>> helge = getUtility(IPersonSet).getByName('kreutzm')
     >>> mozilla_package = ubuntu.getSourcePackage(mozilla_name)
     >>> mozilla_package.addBugSubscription(helge, helge)
-    <StructuralSubscription ...>
+    <...StructuralSubscription ...>
     >>> commit()
     >>> LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)
 
 
=== modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
--- lib/lp/registry/browser/tests/milestone-views.txt	2010-11-02 20:10:56 +0000
+++ lib/lp/registry/browser/tests/milestone-views.txt	2011-01-20 17:50:31 +0000
@@ -682,7 +682,7 @@
     >>> bugtask.milestone = milestone
     >>> subscription = milestone.addSubscription(owner, owner)
     >>> [subscription for subscription in owner.structural_subscriptions]
-    [<StructuralSubscription ...>]
+    [<...StructuralSubscription ...>]
 
     >>> view = create_initialized_view(milestone, '+delete')
     >>> [bugtask.milestone.name for bugtask in view.bugtasks]
 
=== modified file 'lib/lp/registry/doc/milestone.txt'
--- lib/lp/registry/doc/milestone.txt	2010-12-22 00:07:36 +0000
+++ lib/lp/registry/doc/milestone.txt	2011-01-20 17:50:31 +0000
@@ -348,10 +348,10 @@
     >>> cprov = getUtility(IPersonSet).getByName('cprov')
     >>> ddaa = getUtility(IPersonSet).getByName('ddaa')
     >>> milestone_one.addBugSubscription(cprov, cprov)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
 
     >>> milestone_two.addBugSubscription(ddaa, ddaa)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
 
 We change the milestone for the task from 1.0 to 2.0, and fire the
 change event.
 
=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2010-12-07 00:58:46 +0000
+++ lib/lp/registry/doc/person.txt	2011-01-20 17:50:31 +0000
@@ -1027,11 +1027,11 @@
     >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
     >>> pmount = ubuntu.getSourcePackage('pmount')
     >>> pmount.addBugSubscription(no_priv, no_priv)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
 
     >>> mozilla_firefox = ubuntu.getSourcePackage('mozilla-firefox')
     >>> mozilla_firefox.addBugSubscription(no_priv, no_priv)
-    <StructuralSubscription at ...>
+    <...StructuralSubscription object at ...>
 
     >>> [package.name for package in no_priv.getBugSubscriberPackages()]
     [u'mozilla-firefox', u'pmount']
 
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-01-18 04:40:41 +0000
+++ lib/lp/registry/model/person.py	2011-01-20 17:50:31 +0000
@@ -2713,8 +2713,10 @@
     @property
     def structural_subscriptions(self):
         """See `IPerson`."""
-        return StructuralSubscription.selectBy(
-            subscriber=self, orderBy=['-date_created'])
+        return IStore(self).find(
+            StructuralSubscription,
+            StructuralSubscription.subscriberID==self.id).order_by(
+                Desc(StructuralSubscription.date_created))
 
     def autoSubscribeToMailingList(self, mailinglist, requester=None):
         """See `IPerson`."""
 
=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py	2011-01-19 18:12:07 +0000
+++ lib/lp/registry/model/structuralsubscription.py	2011-01-20 17:50:31 +0000
@@ -15,6 +15,7 @@
     Reference,
     )
 
+from storm.base import Storm
 from storm.expr import (
     Alias,
     And,
@@ -39,10 +40,7 @@
 
 from canonical.database.constants import UTC_NOW
 from canonical.database.enumcol import DBEnum
-from canonical.database.sqlbase import (
-    quote,
-    SQLBase,
-    )
+from canonical.database.sqlbase import quote
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.lpstorm import IStore
 from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
@@ -80,13 +78,11 @@
 from lp.services.propertycache import cachedproperty
 
 
-class StructuralSubscription(SQLBase):
+class StructuralSubscription(Storm):
     """A subscription to a Launchpad structure."""
 
     implements(IStructuralSubscription)
 
-    _table = 'StructuralSubscription'
-
     __storm_table__ = 'StructuralSubscription'
 
     id = Int(primary=True)
@@ -135,6 +131,12 @@
         "date_last_updated", allow_none=False, default=UTC_NOW,
         tzinfo=pytz.UTC)
 
+    def __init__(self, subscriber, subscribed_by, **kwargs):
+        self.subscriber = subscriber
+        self.subscribed_by = subscribed_by
+        for arg, value in kwargs.iteritems():
+            setattr(self, arg, value)
+
     @property
     def target(self):
         """See `IStructuralSubscription`."""
@@ -427,13 +429,9 @@
                 '%s does not have permission to unsubscribe %s.' % (
                     unsubscribed_by.name, subscriber.name))
 
-        subscription_to_remove = None
-        for subscription in self.getSubscriptions(
-            min_bug_notification_level=BugNotificationLevel.METADATA):
-            # Only search for bug subscriptions
-            if subscription.subscriber == subscriber:
-                subscription_to_remove = subscription
-                break
+        subscription_to_remove = self.getSubscriptions(
+            min_bug_notification_level=BugNotificationLevel.METADATA,
+            subscriber=subscriber).one()
 
         if subscription_to_remove is None:
             raise DeleteSubscriptionError(
@@ -447,39 +445,44 @@
                 subscription_to_remove.bug_notification_level = (
                     BugNotificationLevel.NOTHING)
             else:
-                subscription_to_remove.destroySelf()
+                store = Store.of(subscription_to_remove)
+                store.remove(subscription_to_remove)
 
     def getSubscription(self, person):
         """See `IStructuralSubscriptionTarget`."""
-        all_subscriptions = self.getSubscriptions()
-        for subscription in all_subscriptions:
-            if subscription.subscriber == person:
-                return subscription
-        return None
+        # getSubscriptions returns all subscriptions regardless of
+        # the person for person==None, so we special-case that.
+        if person is None:
+            return None
+        all_subscriptions = self.getSubscriptions(subscriber=person)
+        return all_subscriptions.one()
 
     def getSubscriptions(self,
                          min_bug_notification_level=
                          BugNotificationLevel.NOTHING,
                          min_blueprint_notification_level=
-                         BlueprintNotificationLevel.NOTHING):
+                         BlueprintNotificationLevel.NOTHING,
+                         subscriber=None):
         """See `IStructuralSubscriptionTarget`."""
+        from lp.registry.model.person import Person
         clauses = [
-            "StructuralSubscription.subscriber = Person.id",
-            "StructuralSubscription.bug_notification_level "
-            ">= %s" % quote(min_bug_notification_level),
-            "StructuralSubscription.blueprint_notification_level "
-            ">= %s" % quote(min_blueprint_notification_level),
+            StructuralSubscription.subscriberID==Person.id,
+            (StructuralSubscription.bug_notification_level >=
+             min_bug_notification_level),
+            (StructuralSubscription.blueprint_notification_level >=
+             min_blueprint_notification_level),
             ]
         for key, value in self._target_args.iteritems():
-            if value is None:
-                clauses.append(
-                    "StructuralSubscription.%s IS NULL" % (key,))
-            else:
-                clauses.append(
-                    "StructuralSubscription.%s = %s" % (key, quote(value)))
-        query = " AND ".join(clauses)
-        return StructuralSubscription.select(
-            query, orderBy='Person.displayname', clauseTables=['Person'])
+            clauses.append(
+                getattr(StructuralSubscription, key)==value)
+
+        if subscriber is not None:
+            clauses.append(
+                StructuralSubscription.subscriberID==subscriber.id)
+
+        store = Store.of(self.__helper.pillar)
+        return store.find(
+            StructuralSubscription, *clauses).order_by('Person.displayname')
 
     @property
     def bug_subscriptions(self):
 
=== modified file 'lib/lp/registry/templates/person-structural-subscriptions.pt'
--- lib/lp/registry/templates/person-structural-subscriptions.pt	2011-01-13 16:30:15 +0000
+++ lib/lp/registry/templates/person-structural-subscriptions.pt	2011-01-20 17:50:31 +0000
@@ -23,7 +23,7 @@
       <div class="yui-g">
         <div class="portlet" id="structural-subscriptions">
 
-          <ul tal:condition="structural_subscriptions">
+          <ul tal:condition="python:not structural_subscriptions.is_empty()">
             <li tal:repeat="subscription structural_subscriptions"
                 style="margin: 1em 0em; padding: 1em; border: 1px solid #ddd;">
               <span tal:replace="structure subscription/target/fmt:link" />
@@ -74,7 +74,7 @@
             </li>
           </ul>
 
-          <p tal:condition="not: structural_subscriptions">
+          <p tal:condition="python: structural_subscriptions.is_empty()">
             <tal:person content="context/fmt:displayname" /> does not
             have any structural subscriptions.
           </p>
 
=== modified file 'lib/lp/registry/tests/structural-subscription-target.txt'
--- lib/lp/registry/tests/structural-subscription-target.txt	2010-10-17 15:44:08 +0000
+++ lib/lp/registry/tests/structural-subscription-target.txt	2011-01-20 17:50:31 +0000
@@ -11,7 +11,7 @@
     >>> from canonical.launchpad.ftests import login
     >>> login("[e-mail]")
     >>> target.addBugSubscription(ubuntu_team, foobar)
-    <StructuralSubscription ...>
+    <...StructuralSubscription ...>
 
 Trying to remove a bug subscription when notification levels for other
 applications are set, doesn't remove the subscription. Instead the
@@ -41,7 +41,7 @@
 IStructuralSubscriptionTarget.getSubscription.
 
     >>> target.getSubscription(ubuntu_team)
-    <StructuralSubscription ...>
+    <...StructuralSubscription ...>
     >>> print target.getSubscription(no_priv)
     None