Merge lp:~gary/launchpad/bug723999-2d into lp:launchpad

[Gary Poster]

[r=benji][no-qa] Clean up structural subcription code to eliminate duplicated code and remove code middle-men that emerged after my work on bug 723999.

This is the final clean-up branch from my work on bug723999, building most recently on ~gary/launchpad/bug723999-2c.

This branch is all about removing three methods on the bugtaskset utility (lib/lp/bugs/model/bugtask.py) that were simply operating as middlemen to functions in the structuralsubscription module.

- _getStructuralSubscriptionTargets was just a helper function that had stayed around because it was tested. Those tests are now repointed to look at the underlying get_structural_subscription_targets function, and moved from test_bugtask to test_structuralsubscription.

- At the end of the 2c branch, getStructuralSubscribers was no longer called by anything in LP. It was only around because useful tests looked at it. Those tests are now repointed and moved, in the same way as described for _getStructuralSubscriptionTargets. This also meant that I was able to remove get_structural_subscribers_for_bugtasks from structuralsubscription.py, because that function was really only a shim against get_structural_subscribers for the method to use. I wrote some new tests for the method, which also prove that the function does what we want (TestGetStructuralSubscriptionsForPerson).

- getAllStructuralSubscriptions was now only used by a browser view. It essentially wanted the structural subscriptions for a bug, for a person. I eliminated this bugtaskset method and added a getStructuralSubscriptionsForPerson to IBug. The implementation again is just a middleman, but because this is called by browser code, we want to be able to control who calls what from a security perspective, so I thought it made sense to make the browser code call a method off of a class rather than a function. This is arguable, but it made sense to me at the time.

That's pretty much it.

Thank you!

Gary

[Benji York (benji)]

Looks good. This branch, like the others in this series, was cohesive
so that helped in reviewing it.

One small note: The import of StructuralSubscription in
lib/lp/bugs/model/bugtask.py could be on one line if you wanted.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2011-03-04 11:00:04 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2011-03-07 22:10:38 +0000
@@ -19,7 +19,6 @@
 from zope import formlib
 from zope.app.form import CustomWidgetFactory
 from zope.app.form.browser.itemswidgets import RadioWidget
-from zope.component import getUtility
 from zope.schema import Choice
 from zope.schema.vocabulary import (
     SimpleTerm,
@@ -41,7 +40,6 @@
 from lp.bugs.browser.bug import BugViewMixin
 from lp.bugs.enum import BugNotificationLevel, HIDDEN_BUG_NOTIFICATION_LEVELS
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
-from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.services import features
 from lp.services.propertycache import cachedproperty
 
@@ -559,5 +557,4 @@
 
     @property
     def structural_subscriptions(self):
-        return getUtility(IBugTaskSet).getAllStructuralSubscriptions(
-            self.context.bug.bugtasks, self.user)
+        return self.context.bug.getStructuralSubscriptionsForPerson(self.user)

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-03-07 17:54:44 +0000
+++ lib/lp/bugs/configure.zcml	2011-03-07 22:10:38 +0000
@@ -747,6 +747,7 @@
                     getSubscriptionsFromDuplicates
                     getSubscribersForPerson
                     getSubscriptionForPerson
+                    getStructuralSubscriptionsForPerson
                     getSubscriptionInfo
                     indexed_messages
                     _indexed_messages

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2011-03-07 22:10:32 +0000
+++ lib/lp/bugs/interfaces/bug.py	2011-03-07 22:10:38 +0000
@@ -533,6 +533,12 @@
         If no such `BugSubscription` exists, return None.
         """
 
+    def getStructuralSubscriptionsForPerson(person):
+        """Return the `StructuralSubscription`s for a `Person` to this `Bug`.
+
+        This disregards filters.
+        """
+
     def getSubscriptionInfo(level):
         """Return a `BugSubscriptionInfo` at the given `level`.
 

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2011-03-03 18:58:49 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2011-03-07 22:10:38 +0000
@@ -1597,18 +1597,6 @@
     def getOpenBugTasksPerProduct(user, products):
         """Return open bugtask count for multiple products."""
 
-    def getAllStructuralSubscriptions(bugtasks):
-        """Return all potential structural subscriptions for the bugtasks.
-
-        This method ignores subscription filters.
-        """
-
-    def getStructuralSubscribers(bugtasks, recipients=None, level=None):
-        """Return `IPerson`s subscribed to the given bug tasks.
-
-        This takes into account bug subscription filters.
-        """
-
     def getPrecachedNonConjoinedBugTasks(user, milestone):
         """List of non-conjoined bugtasks targeted to the milestone.
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-03-07 22:10:32 +0000
+++ lib/lp/bugs/model/bug.py	2011-03-07 22:10:38 +0000
@@ -982,6 +982,10 @@
             BugSubscription.person == person,
             BugSubscription.bug == self).one()
 
+    def getStructuralSubscriptionsForPerson(self, person):
+        """See `IBug`."""
+        return get_all_structural_subscriptions(self.bugtasks, person)
+
     def getAlsoNotifiedSubscribers(self, recipients=None, level=None):
         """See `IBug`.
 

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-03-07 07:27:56 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-03-07 22:10:38 +0000
@@ -131,12 +131,7 @@
     )
 from lp.bugs.model.bugnomination import BugNomination
 from lp.bugs.model.bugsubscription import BugSubscription
-from lp.bugs.model.structuralsubscription import (
-    get_all_structural_subscriptions,
-    get_structural_subscribers_for_bugtasks,
-    get_structural_subscription_targets,
-    StructuralSubscription,
-    )
+from lp.bugs.model.structuralsubscription import StructuralSubscription
 from lp.registry.interfaces.distribution import (
     IDistribution,
     IDistributionSet,
@@ -3107,20 +3102,3 @@
             counts.append(package_counts)
 
         return counts
-
-    def _getStructuralSubscriptionTargets(self, bugtasks):
-        """Return (bugtask, target) pairs for each target of the bugtasks.
-
-        Each bugtask may be responsible theoretically for 0 or more targets.
-        In practice, each generates one, two or three.
-        """
-        return get_structural_subscription_targets(bugtasks)
-
-    def getAllStructuralSubscriptions(self, bugtasks, recipient=None):
-        """See `IBugTaskSet`."""
-        return get_all_structural_subscriptions(bugtasks, recipient)
-
-    def getStructuralSubscribers(self, bugtasks, recipients=None, level=None):
-        """See `IBugTaskSet`."""
-        return get_structural_subscribers_for_bugtasks(
-            bugtasks, recipients, level)

=== modified file 'lib/lp/bugs/model/structuralsubscription.py'
--- lib/lp/bugs/model/structuralsubscription.py	2011-03-07 22:10:32 +0000
+++ lib/lp/bugs/model/structuralsubscription.py	2011-03-07 22:10:38 +0000
@@ -5,7 +5,6 @@
 __all__ = [
     'get_all_structural_subscriptions',
     'get_structural_subscribers',
-    'get_structural_subscribers_for_bugtasks',
     'get_structural_subscription_targets',
     'StructuralSubscription',
     'StructuralSubscriptionTargetMixin',
@@ -608,33 +607,6 @@
         return subscribers
 
 
-def get_structural_subscribers_for_bugtasks(bugtasks,
-                                            recipients=None,
-                                            level=None):
-    """Return subscribers for structural filters for the bugtasks at "level".
-
-    :param bugtasks: an iterable of bugtasks.  Should be a single bugtask, or
-                     all of the bugtasks from one bug.
-    :param recipients: a BugNotificationRecipients object or None.
-                       Populates if given.
-    :param level: a level from lp.bugs.enum.BugNotificationLevel.
-
-    Excludes structural subscriptions for people who are directly subscribed
-    to the bug."""
-    bugtasks = list(bugtasks)
-    if not bugtasks:
-        return EmptyResultSet()
-    if len(bugtasks) == 1:
-        target = bugtasks[0]
-    else:
-        target = bugtasks[0].bug
-        if target.bugtasks != bugtasks:
-            raise NotImplementedError(
-                'bugtasks must be one, or the full set of bugtasks from one '
-                'bug')
-    return get_structural_subscribers(target, recipients, level)
-
-
 class ArrayAgg(NamedFunc):
     "Aggregate values (within a GROUP BY) into an array."
     __slots__ = ()

=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py	2011-02-18 12:51:40 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2011-03-07 22:10:38 +0000
@@ -12,6 +12,7 @@
     person_logged_in,
     TestCaseWithFactory,
     )
+from lp.testing.factory import is_security_proxied_or_harmless
 
 
 class TestBug(TestCaseWithFactory):
@@ -247,3 +248,36 @@
         with person_logged_in(bug.owner):
             info = bug.getSubscriptionInfo(BugNotificationLevel.METADATA)
         self.assertEqual(BugNotificationLevel.METADATA, info.level)
+
+
+class TestGetStructuralSubscriptionsForPerson(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def getStructuralSubscriptionsForPerson(self, bug, recipient):
+        # Call bug.getStructuralSubscriptionsForPerson() and check that the
+        # result is security proxied.
+        result = bug.getStructuralSubscriptionsForPerson(recipient)
+        self.assertTrue(is_security_proxied_or_harmless(result))
+        return result
+
+    def setUp(self):
+        super(TestGetStructuralSubscriptionsForPerson, self).setUp()
+        self.subscriber = self.factory.makePerson()
+        login_person(self.subscriber)
+        self.product = self.factory.makeProduct()
+        self.milestone = self.factory.makeMilestone(product=self.product)
+        self.bug = self.factory.makeBug(
+            product=self.product, milestone=self.milestone)
+
+    def test_no_subscriptions(self):
+        subscriptions = self.getStructuralSubscriptionsForPerson(
+            self.bug, self.subscriber)
+        self.assertEqual([], list(subscriptions))
+
+    def test_one_subscription(self):
+        sub = self.product.addBugSubscription(
+            self.subscriber, self.subscriber)
+        subscriptions = self.getStructuralSubscriptionsForPerson(
+            self.bug, self.subscriber)
+        self.assertEqual([sub], list(subscriptions))

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-03-03 02:14:14 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-03-07 22:10:38 +0000
@@ -8,17 +8,9 @@
 import unittest
 
 from lazr.lifecycle.snapshot import Snapshot
-from storm.store import (
-    EmptyResultSet,
-    ResultSet,
-    )
-from testtools.matchers import (
-    Equals,
-    StartsWith,
-    )
+from testtools.matchers import Equals
 from zope.component import getUtility
 from zope.interface import providedBy
-from zope.security.proxy import removeSecurityProxy
 
 from canonical.database.sqlbase import flush_database_updates
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
@@ -32,7 +24,6 @@
     LaunchpadZopelessLayer,
     )
 from lp.app.enums import ServiceUsage
-from lp.bugs.enum import BugNotificationLevel
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.interfaces.bugtarget import IBugTarget
 from lp.bugs.interfaces.bugtask import (
@@ -45,7 +36,6 @@
     UNRESOLVED_BUGTASK_STATUSES,
     )
 from lp.bugs.interfaces.bugwatch import IBugWatchSet
-from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
 from lp.bugs.model.bugtask import build_tag_search_clause
 from lp.bugs.tests.bug import (
     create_old_bug,
@@ -72,10 +62,7 @@
     TestCase,
     TestCaseWithFactory,
     )
-from lp.testing.factory import (
-    is_security_proxied_or_harmless,
-    LaunchpadObjectFactory,
-    )
+from lp.testing.factory import LaunchpadObjectFactory
 from lp.testing.matchers import HasQueryCount
 
 
@@ -1346,239 +1333,6 @@
         self.assertNotIn(BugTaskStatus.UNKNOWN, UNRESOLVED_BUGTASK_STATUSES)
 
 
-class TestGetStructuralSubscriptionTargets(TestCaseWithFactory):
-    # This tests a private method because it has some subtleties that are
-    # nice to test in isolation.
-
-    layer = DatabaseFunctionalLayer
-
-    def getStructuralSubscriptionTargets(self, bugtasks):
-        unwrapped = removeSecurityProxy(getUtility(IBugTaskSet))
-        return unwrapped._getStructuralSubscriptionTargets(bugtasks)
-
-    def test_product_target(self):
-        product = self.factory.makeProduct()
-        bug = self.factory.makeBug(product=product)
-        bugtask = bug.bugtasks[0]
-        result = self.getStructuralSubscriptionTargets(bug.bugtasks)
-        self.assertEqual(list(result), [(bugtask, product)])
-
-    def test_milestone_target(self):
-        actor = self.factory.makePerson()
-        login_person(actor)
-        product = self.factory.makeProduct()
-        milestone = self.factory.makeMilestone(product=product)
-        bug = self.factory.makeBug(product=product, milestone=milestone)
-        bugtask = bug.bugtasks[0]
-        result = self.getStructuralSubscriptionTargets(bug.bugtasks)
-        self.assertEqual(set(result), set(
-            ((bugtask, product), (bugtask, milestone))))
-
-    def test_sourcepackage_target(self):
-        actor = self.factory.makePerson()
-        login_person(actor)
-        distroseries = self.factory.makeDistroSeries()
-        sourcepackage = self.factory.makeSourcePackage(
-            distroseries=distroseries)
-        product = self.factory.makeProduct()
-        bug = self.factory.makeBug(product=product)
-        bug.addTask(actor, sourcepackage)
-        product_bugtask = bug.bugtasks[0]
-        sourcepackage_bugtask = bug.bugtasks[1]
-        result = self.getStructuralSubscriptionTargets(bug.bugtasks)
-        self.assertEqual(set(result), set(
-            ((product_bugtask, product),
-             (sourcepackage_bugtask, distroseries))))
-
-    def test_distribution_source_package_target(self):
-        actor = self.factory.makePerson()
-        login_person(actor)
-        distribution = self.factory.makeDistribution()
-        dist_sourcepackage = self.factory.makeDistributionSourcePackage(
-            distribution=distribution)
-        product = self.factory.makeProduct()
-        bug = self.factory.makeBug(product=product)
-        bug.addTask(actor, dist_sourcepackage)
-        product_bugtask = bug.bugtasks[0]
-        dist_sourcepackage_bugtask = bug.bugtasks[1]
-        result = self.getStructuralSubscriptionTargets(bug.bugtasks)
-        self.assertEqual(set(result), set(
-            ((product_bugtask, product),
-             (dist_sourcepackage_bugtask, dist_sourcepackage),
-             (dist_sourcepackage_bugtask, distribution))))
-
-
-class TestGetAllStructuralSubscriptions(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
-
-    def getAllStructuralSubscriptions(self, bugtasks, recipient):
-        # Call IBugTaskSet.getAllStructuralSubscriptions() and check that the
-        # result is security proxied.
-        result = getUtility(IBugTaskSet).getAllStructuralSubscriptions(
-            bugtasks, recipient)
-        self.assertTrue(is_security_proxied_or_harmless(result))
-        return result
-
-    def setUp(self):
-        super(TestGetAllStructuralSubscriptions, self).setUp()
-        self.subscriber = self.factory.makePerson()
-        login_person(self.subscriber)
-        self.product = self.factory.makeProduct()
-        self.milestone = self.factory.makeMilestone(product=self.product)
-        self.bug = self.factory.makeBug(
-            product=self.product, milestone=self.milestone)
-
-    def test_no_subscriptions(self):
-        subscriptions = self.getAllStructuralSubscriptions(
-            self.bug.bugtasks, self.subscriber)
-        self.assertEqual([], list(subscriptions))
-
-    def test_one_subscription(self):
-        sub = self.product.addBugSubscription(
-            self.subscriber, self.subscriber)
-        subscriptions = self.getAllStructuralSubscriptions(
-            self.bug.bugtasks, self.subscriber)
-        self.assertEqual([sub], list(subscriptions))
-
-    def test_two_subscriptions(self):
-        sub1 = self.product.addBugSubscription(
-            self.subscriber, self.subscriber)
-        sub2 = self.milestone.addBugSubscription(
-            self.subscriber, self.subscriber)
-        subscriptions = self.getAllStructuralSubscriptions(
-            self.bug.bugtasks, self.subscriber)
-        self.assertEqual(set([sub1, sub2]), set(subscriptions))
-
-    def test_two_bugtasks_one_subscription(self):
-        sub = self.product.addBugSubscription(
-            self.subscriber, self.subscriber)
-        product2 = self.factory.makeProduct()
-        self.bug.addTask(self.subscriber, product2)
-        subscriptions = self.getAllStructuralSubscriptions(
-            self.bug.bugtasks, self.subscriber)
-        self.assertEqual([sub], list(subscriptions))
-
-    def test_two_bugtasks_two_subscriptions(self):
-        sub1 = self.product.addBugSubscription(
-            self.subscriber, self.subscriber)
-        product2 = self.factory.makeProduct()
-        self.bug.addTask(self.subscriber, product2)
-        sub2 = product2.addBugSubscription(
-            self.subscriber, self.subscriber)
-        subscriptions = self.getAllStructuralSubscriptions(
-            self.bug.bugtasks, self.subscriber)
-        self.assertEqual(set([sub1, sub2]), set(subscriptions))
-
-    def test_ignore_other_subscriptions(self):
-        sub1 = self.product.addBugSubscription(
-            self.subscriber, self.subscriber)
-        another_subscriber = self.factory.makePerson()
-        login_person(another_subscriber)
-        sub2 = self.product.addBugSubscription(
-            another_subscriber, another_subscriber)
-        subscriptions = self.getAllStructuralSubscriptions(
-            self.bug.bugtasks, self.subscriber)
-        self.assertEqual([sub1], list(subscriptions))
-        subscriptions = self.getAllStructuralSubscriptions(
-            self.bug.bugtasks, another_subscriber)
-        self.assertEqual([sub2], list(subscriptions))
-
-
-class TestGetStructuralSubscribers(TestCaseWithFactory):
-
-    layer = DatabaseFunctionalLayer
-
-    def make_product_with_bug(self):
-        product = self.factory.makeProduct()
-        bug = self.factory.makeBug(product=product)
-        return product, bug
-
-    def getStructuralSubscribers(self, bugtasks, *args, **kwargs):
-        # Call IBugTaskSet.getStructuralSubscribers() and check that the
-        # result is security proxied.
-        result = getUtility(IBugTaskSet).getStructuralSubscribers(
-            bugtasks, *args, **kwargs)
-        self.assertTrue(is_security_proxied_or_harmless(result))
-        return result
-
-    def test_getStructuralSubscribers_no_subscribers(self):
-        # If there are no subscribers for any of the bug's targets then no
-        # subscribers will be returned by getStructuralSubscribers().
-        product, bug = self.make_product_with_bug()
-        subscribers = self.getStructuralSubscribers(bug.bugtasks)
-        self.assertIsInstance(subscribers, (ResultSet, EmptyResultSet))
-        self.assertEqual([], list(subscribers))
-
-    def test_getStructuralSubscribers_single_target(self):
-        # Subscribers for any of the bug's targets are returned.
-        subscriber = self.factory.makePerson()
-        login_person(subscriber)
-        product, bug = self.make_product_with_bug()
-        product.addBugSubscription(subscriber, subscriber)
-        self.assertEqual(
-            [subscriber], list(
-                self.getStructuralSubscribers(bug.bugtasks)))
-
-    def test_getStructuralSubscribers_multiple_targets(self):
-        # Subscribers for any of the bug's targets are returned.
-        actor = self.factory.makePerson()
-        login_person(actor)
-
-        subscriber1 = self.factory.makePerson()
-        subscriber2 = self.factory.makePerson()
-
-        product1 = self.factory.makeProduct(owner=actor)
-        product1.addBugSubscription(subscriber1, subscriber1)
-        product2 = self.factory.makeProduct(owner=actor)
-        product2.addBugSubscription(subscriber2, subscriber2)
-
-        bug = self.factory.makeBug(product=product1)
-        bug.addTask(actor, product2)
-
-        subscribers = self.getStructuralSubscribers(bug.bugtasks)
-        self.assertIsInstance(subscribers, ResultSet)
-        self.assertEqual(set([subscriber1, subscriber2]), set(subscribers))
-
-    def test_getStructuralSubscribers_recipients(self):
-        # If provided, getStructuralSubscribers() calls the appropriate
-        # methods on a BugNotificationRecipients object.
-        subscriber = self.factory.makePerson()
-        login_person(subscriber)
-        product, bug = self.make_product_with_bug()
-        product.addBugSubscription(subscriber, subscriber)
-        recipients = BugNotificationRecipients()
-        subscribers = self.getStructuralSubscribers(
-            bug.bugtasks, recipients=recipients)
-        # The return value is a list only when populating recipients.
-        self.assertIsInstance(subscribers, list)
-        self.assertEqual([subscriber], recipients.getRecipients())
-        reason, header = recipients.getReason(subscriber)
-        self.assertThat(
-            reason, StartsWith(
-                u"You received this bug notification because "
-                u"you are subscribed to "))
-        self.assertThat(header, StartsWith(u"Subscriber "))
-
-    def test_getStructuralSubscribers_level(self):
-        # getStructuralSubscribers() respects the given level.
-        subscriber = self.factory.makePerson()
-        login_person(subscriber)
-        product, bug = self.make_product_with_bug()
-        subscription = product.addBugSubscription(subscriber, subscriber)
-        filter = subscription.bug_filters.one()
-        filter.bug_notification_level = BugNotificationLevel.METADATA
-        self.assertEqual(
-            [subscriber], list(
-                self.getStructuralSubscribers(
-                    bug.bugtasks, level=BugNotificationLevel.METADATA)))
-        filter.bug_notification_level = BugNotificationLevel.METADATA
-        self.assertEqual(
-            [], list(
-                self.getStructuralSubscribers(
-                    bug.bugtasks, level=BugNotificationLevel.COMMENTS)))
-
-
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))

=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
--- lib/lp/bugs/tests/test_structuralsubscription.py	2011-03-07 22:10:32 +0000
+++ lib/lp/bugs/tests/test_structuralsubscription.py	2011-03-07 22:10:38 +0000
@@ -5,7 +5,12 @@
 
 __metaclass__ = type
 
-from storm.store import Store
+from storm.store import (
+    EmptyResultSet,
+    ResultSet,
+    Store,
+    )
+from testtools.matchers import StartsWith
 from zope.security.interfaces import Unauthorized
 
 from canonical.testing.layers import (
@@ -17,8 +22,13 @@
     BugTaskImportance,
     BugTaskStatus,
     )
+from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
 from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
-from lp.bugs.model.structuralsubscription import get_structural_subscribers
+from lp.bugs.model.structuralsubscription import (
+    get_all_structural_subscriptions,
+    get_structural_subscribers,
+    get_structural_subscription_targets,
+    )
 from lp.testing import (
     anonymous_logged_in,
     login_person,
@@ -444,3 +454,213 @@
 
     def makeTarget(self):
         return self.factory.makeProductSeries()
+
+
+class TestGetStructuralSubscriptionTargets(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_product_target(self):
+        product = self.factory.makeProduct()
+        bug = self.factory.makeBug(product=product)
+        bugtask = bug.bugtasks[0]
+        result = get_structural_subscription_targets(bug.bugtasks)
+        self.assertEqual(list(result), [(bugtask, product)])
+
+    def test_milestone_target(self):
+        actor = self.factory.makePerson()
+        login_person(actor)
+        product = self.factory.makeProduct()
+        milestone = self.factory.makeMilestone(product=product)
+        bug = self.factory.makeBug(product=product, milestone=milestone)
+        bugtask = bug.bugtasks[0]
+        result = get_structural_subscription_targets(bug.bugtasks)
+        self.assertEqual(set(result), set(
+            ((bugtask, product), (bugtask, milestone))))
+
+    def test_sourcepackage_target(self):
+        actor = self.factory.makePerson()
+        login_person(actor)
+        distroseries = self.factory.makeDistroSeries()
+        sourcepackage = self.factory.makeSourcePackage(
+            distroseries=distroseries)
+        product = self.factory.makeProduct()
+        bug = self.factory.makeBug(product=product)
+        bug.addTask(actor, sourcepackage)
+        product_bugtask = bug.bugtasks[0]
+        sourcepackage_bugtask = bug.bugtasks[1]
+        result = get_structural_subscription_targets(bug.bugtasks)
+        self.assertEqual(set(result), set(
+            ((product_bugtask, product),
+             (sourcepackage_bugtask, distroseries))))
+
+    def test_distribution_source_package_target(self):
+        actor = self.factory.makePerson()
+        login_person(actor)
+        distribution = self.factory.makeDistribution()
+        dist_sourcepackage = self.factory.makeDistributionSourcePackage(
+            distribution=distribution)
+        product = self.factory.makeProduct()
+        bug = self.factory.makeBug(product=product)
+        bug.addTask(actor, dist_sourcepackage)
+        product_bugtask = bug.bugtasks[0]
+        dist_sourcepackage_bugtask = bug.bugtasks[1]
+        result = get_structural_subscription_targets(bug.bugtasks)
+        self.assertEqual(set(result), set(
+            ((product_bugtask, product),
+             (dist_sourcepackage_bugtask, dist_sourcepackage),
+             (dist_sourcepackage_bugtask, distribution))))
+
+
+class TestGetAllStructuralSubscriptions(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestGetAllStructuralSubscriptions, self).setUp()
+        self.subscriber = self.factory.makePerson()
+        login_person(self.subscriber)
+        self.product = self.factory.makeProduct()
+        self.milestone = self.factory.makeMilestone(product=self.product)
+        self.bug = self.factory.makeBug(
+            product=self.product, milestone=self.milestone)
+
+    def test_no_subscriptions(self):
+        subscriptions = get_all_structural_subscriptions(
+            self.bug.bugtasks, self.subscriber)
+        self.assertEqual([], list(subscriptions))
+
+    def test_one_subscription(self):
+        sub = self.product.addBugSubscription(
+            self.subscriber, self.subscriber)
+        subscriptions = get_all_structural_subscriptions(
+            self.bug.bugtasks, self.subscriber)
+        self.assertEqual([sub], list(subscriptions))
+
+    def test_two_subscriptions(self):
+        sub1 = self.product.addBugSubscription(
+            self.subscriber, self.subscriber)
+        sub2 = self.milestone.addBugSubscription(
+            self.subscriber, self.subscriber)
+        subscriptions = get_all_structural_subscriptions(
+            self.bug.bugtasks, self.subscriber)
+        self.assertEqual(set([sub1, sub2]), set(subscriptions))
+
+    def test_two_bugtasks_one_subscription(self):
+        sub = self.product.addBugSubscription(
+            self.subscriber, self.subscriber)
+        product2 = self.factory.makeProduct()
+        self.bug.addTask(self.subscriber, product2)
+        subscriptions = get_all_structural_subscriptions(
+            self.bug.bugtasks, self.subscriber)
+        self.assertEqual([sub], list(subscriptions))
+
+    def test_two_bugtasks_two_subscriptions(self):
+        sub1 = self.product.addBugSubscription(
+            self.subscriber, self.subscriber)
+        product2 = self.factory.makeProduct()
+        self.bug.addTask(self.subscriber, product2)
+        sub2 = product2.addBugSubscription(
+            self.subscriber, self.subscriber)
+        subscriptions = get_all_structural_subscriptions(
+            self.bug.bugtasks, self.subscriber)
+        self.assertEqual(set([sub1, sub2]), set(subscriptions))
+
+    def test_ignore_other_subscriptions(self):
+        sub1 = self.product.addBugSubscription(
+            self.subscriber, self.subscriber)
+        another_subscriber = self.factory.makePerson()
+        login_person(another_subscriber)
+        sub2 = self.product.addBugSubscription(
+            another_subscriber, another_subscriber)
+        subscriptions = get_all_structural_subscriptions(
+            self.bug.bugtasks, self.subscriber)
+        self.assertEqual([sub1], list(subscriptions))
+        subscriptions = get_all_structural_subscriptions(
+            self.bug.bugtasks, another_subscriber)
+        self.assertEqual([sub2], list(subscriptions))
+
+
+class TestGetStructuralSubscribers(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def make_product_with_bug(self):
+        product = self.factory.makeProduct()
+        bug = self.factory.makeBug(product=product)
+        return product, bug
+
+    def test_getStructuralSubscribers_no_subscribers(self):
+        # If there are no subscribers for any of the bug's targets then no
+        # subscribers will be returned by get_structural_subscribers().
+        product, bug = self.make_product_with_bug()
+        subscribers = get_structural_subscribers(bug, None, None, None)
+        self.assertIsInstance(subscribers, (ResultSet, EmptyResultSet))
+        self.assertEqual([], list(subscribers))
+
+    def test_getStructuralSubscribers_single_target(self):
+        # Subscribers for any of the bug's targets are returned.
+        subscriber = self.factory.makePerson()
+        login_person(subscriber)
+        product, bug = self.make_product_with_bug()
+        product.addBugSubscription(subscriber, subscriber)
+        self.assertEqual(
+            [subscriber], list(
+                get_structural_subscribers(bug, None, None, None)))
+
+    def test_getStructuralSubscribers_multiple_targets(self):
+        # Subscribers for any of the bug's targets are returned.
+        actor = self.factory.makePerson()
+        login_person(actor)
+
+        subscriber1 = self.factory.makePerson()
+        subscriber2 = self.factory.makePerson()
+
+        product1 = self.factory.makeProduct(owner=actor)
+        product1.addBugSubscription(subscriber1, subscriber1)
+        product2 = self.factory.makeProduct(owner=actor)
+        product2.addBugSubscription(subscriber2, subscriber2)
+
+        bug = self.factory.makeBug(product=product1)
+        bug.addTask(actor, product2)
+
+        subscribers = get_structural_subscribers(bug, None, None, None)
+        self.assertIsInstance(subscribers, ResultSet)
+        self.assertEqual(set([subscriber1, subscriber2]), set(subscribers))
+
+    def test_getStructuralSubscribers_recipients(self):
+        # If provided, get_structural_subscribers() calls the appropriate
+        # methods on a BugNotificationRecipients object.
+        subscriber = self.factory.makePerson()
+        login_person(subscriber)
+        product, bug = self.make_product_with_bug()
+        product.addBugSubscription(subscriber, subscriber)
+        recipients = BugNotificationRecipients()
+        subscribers = get_structural_subscribers(bug, recipients, None, None)
+        # The return value is a list only when populating recipients.
+        self.assertIsInstance(subscribers, list)
+        self.assertEqual([subscriber], recipients.getRecipients())
+        reason, header = recipients.getReason(subscriber)
+        self.assertThat(
+            reason, StartsWith(
+                u"You received this bug notification because "
+                u"you are subscribed to "))
+        self.assertThat(header, StartsWith(u"Subscriber "))
+
+    def test_getStructuralSubscribers_level(self):
+        # get_structural_subscribers() respects the given level.
+        subscriber = self.factory.makePerson()
+        login_person(subscriber)
+        product, bug = self.make_product_with_bug()
+        subscription = product.addBugSubscription(subscriber, subscriber)
+        filter = subscription.bug_filters.one()
+        filter.bug_notification_level = BugNotificationLevel.METADATA
+        self.assertEqual(
+            [subscriber], list(
+                get_structural_subscribers(
+                    bug, None, BugNotificationLevel.METADATA, None)))
+        filter.bug_notification_level = BugNotificationLevel.METADATA
+        self.assertEqual(
+            [], list(
+                get_structural_subscribers(
+                    bug, None, BugNotificationLevel.COMMENTS, None)))