Merge lp:~allenap/launchpad/revert-subscribe-to-tag-bug-151129 into lp:launchpad

[Gavin Panella]

[r=allenap][ui=none][bug=151129][rollback=11972] Subscribing to tags has performance problems, reverting for now.

Subscribing to tags has performance problems. See the linked bug for more information. This branch reverts the change.

[Gavin Panella (allenap)]

review: Approve

[Francis J. Lacoste (flacoste)]

Trivial.

review: Approve

=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py	2010-11-19 14:33:01 +0000
+++ lib/lp/registry/model/structuralsubscription.py	2010-11-25 21:03:11 +0000
@@ -2,26 +2,14 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
-__all__ = [
-    'StructuralSubscription',
-    'StructuralSubscriptionTargetMixin',
-    ]
+__all__ = ['StructuralSubscription',
+           'StructuralSubscriptionTargetMixin']
 
 from sqlobject import ForeignKey
 from storm.expr import (
-    Alias,
     And,
-    CompoundOper,
-    Except,
-    In,
-    Intersect,
     LeftJoin,
-    NamedFunc,
-    Not,
     Or,
-    Select,
-    SQL,
-    Union,
     )
 from storm.store import Store
 from zope.component import (
@@ -46,7 +34,6 @@
 from lp.bugs.model.bugsubscriptionfilterstatus import (
     BugSubscriptionFilterStatus,
     )
-from lp.bugs.model.bugsubscriptionfiltertag import BugSubscriptionFilterTag
 from lp.registry.enum import BugNotificationLevel
 from lp.registry.errors import (
     DeleteSubscriptionError,
@@ -484,242 +471,50 @@
 
     def getSubscriptionsForBugTask(self, bugtask, level):
         """See `IStructuralSubscriptionTarget`."""
-        set_builder = BugFilterSetBuilder(
-            bugtask, level, self.__helper.join)
-        return Store.of(self.__helper.pillar).find(
-            StructuralSubscription, In(
-                StructuralSubscription.id,
-                set_builder.subscriptions))
-
-
-class ArrayAgg(NamedFunc):
-    __slots__ = ()
-    name = "ARRAY_AGG"
-
-
-class ArrayContains(CompoundOper):
-    __slots__ = ()
-    oper = "@>"
-
-
-class BugFilterSetBuilder:
-    """A convenience class to build queries for getSubscriptionsForBugTask."""
-
-    def __init__(self, bugtask, level, join_condition):
-        """Initialize a new set builder for bug filters.
-
-        :param bugtask: The `IBugTask` to match against.
-        :param level: A member of `BugNotificationLevel`.
-        :param join_condition: A condition for selecting structural
-            subscriptions. Generally this should limit the subscriptions to a
-            particular target (i.e. project or distribution).
-        """
-        self.status = bugtask.status
-        self.importance = bugtask.importance
-        # The list() gets around some weirdness with security proxies; Storm
-        # does not know how to compile an expression with a proxied list.
-        self.tags = list(bugtask.bug.tags)
-        # Set up common conditions.
-        self.base_conditions = And(
+        origin = [
+            StructuralSubscription,
+            LeftJoin(
+                BugSubscriptionFilter,
+                BugSubscriptionFilter.structural_subscription_id == (
+                    StructuralSubscription.id)),
+            LeftJoin(
+                BugSubscriptionFilterStatus,
+                BugSubscriptionFilterStatus.filter_id == (
+                    BugSubscriptionFilter.id)),
+            LeftJoin(
+                BugSubscriptionFilterImportance,
+                BugSubscriptionFilterImportance.filter_id == (
+                    BugSubscriptionFilter.id)),
+            ]
+
+        if len(bugtask.bug.tags) == 0:
+            tag_conditions = [
+                BugSubscriptionFilter.include_any_tags == False,
+                ]
+        else:
+            tag_conditions = [
+                BugSubscriptionFilter.exclude_any_tags == False,
+                ]
+
+        conditions = [
             StructuralSubscription.bug_notification_level >= level,
-            join_condition)
-        # Set up common filter conditions.
-        if len(self.tags) == 0:
-            self.filter_conditions = And(
-                # When the bug has no tags, filters with include_any_tags set
-                # can never match.
-                Not(BugSubscriptionFilter.include_any_tags),
-                self.base_conditions)
-        else:
-            self.filter_conditions = And(
-                # When the bug has tags, filters with exclude_any_tags set can
-                # never match.
-                Not(BugSubscriptionFilter.exclude_any_tags),
-                self.base_conditions)
-
-    @property
-    def subscriptions_without_filters(self):
-        """Subscriptions without filters."""
-        return Select(
-            StructuralSubscription.id,
-            tables=(
-                StructuralSubscription,
-                LeftJoin(
-                    BugSubscriptionFilter,
-                    BugSubscriptionFilter.structural_subscription_id == (
-                        StructuralSubscription.id))),
-            where=And(
+            Or(
+                # There's no filter or ...
                 BugSubscriptionFilter.id == None,
-                self.base_conditions))
-
-    def _filters_matching_x(self, join, where_condition, **extra):
-        """Return an expression yielding `(subscription_id, filter_id)` rows.
-
-        The expressions returned by this function are used in set (union,
-        intersect, except) operations at the *filter* level. However, the
-        interesting result of these set operations is the structural
-        subscription, hence both columns are included in the expressions
-        generated. Since a structural subscription can have zero or more
-        filters, and a filter can never be associated with more than one
-        subscription, the set operations are unaffected.
-        """
-        return Select(
-            columns=(
-                # Alias this column so it can be selected in
-                # subscriptions_matching.
-                Alias(
-                    BugSubscriptionFilter.structural_subscription_id,
-                    "structural_subscription_id"),
-                BugSubscriptionFilter.id),
-            tables=(
-                StructuralSubscription, BugSubscriptionFilter, join),
-            where=And(
-                BugSubscriptionFilter.structural_subscription_id == (
-                    StructuralSubscription.id),
-                self.filter_conditions,
-                where_condition),
-            **extra)
-
-    @property
-    def filters_matching_status(self):
-        """Filters with the given bugtask's status."""
-        join = LeftJoin(
-            BugSubscriptionFilterStatus,
-            BugSubscriptionFilterStatus.filter_id == (
-                BugSubscriptionFilter.id))
-        condition = Or(
-            BugSubscriptionFilterStatus.id == None,
-            BugSubscriptionFilterStatus.status == self.status)
-        return self._filters_matching_x(join, condition)
-
-    @property
-    def filters_matching_importance(self):
-        """Filters with the given bugtask's importance."""
-        join = LeftJoin(
-            BugSubscriptionFilterImportance,
-            BugSubscriptionFilterImportance.filter_id == (
-                BugSubscriptionFilter.id))
-        condition = Or(
-            BugSubscriptionFilterImportance.id == None,
-            BugSubscriptionFilterImportance.importance == self.importance)
-        return self._filters_matching_x(join, condition)
-
-    @property
-    def filters_without_include_tags(self):
-        """Filters with no tags required."""
-        join = LeftJoin(
-            BugSubscriptionFilterTag,
-            And(BugSubscriptionFilterTag.filter_id == (
-                    BugSubscriptionFilter.id),
-                BugSubscriptionFilterTag.include))
-        return self._filters_matching_x(
-            join, BugSubscriptionFilterTag.id == None)
-
-    @property
-    def filters_matching_any_include_tags(self):
-        """Filters including any of the bug's tags."""
-        condition = And(
-            BugSubscriptionFilterTag.filter_id == (
-                BugSubscriptionFilter.id),
-            BugSubscriptionFilterTag.include,
-            Not(BugSubscriptionFilter.find_all_tags),
-            In(BugSubscriptionFilterTag.tag, self.tags))
-        return self._filters_matching_x(
-            BugSubscriptionFilterTag, condition)
-
-    @property
-    def filters_matching_any_exclude_tags(self):
-        """Filters excluding any of the bug's tags."""
-        condition = And(
-            BugSubscriptionFilterTag.filter_id == (
-                BugSubscriptionFilter.id),
-            Not(BugSubscriptionFilterTag.include),
-            Not(BugSubscriptionFilter.find_all_tags),
-            In(BugSubscriptionFilterTag.tag, self.tags))
-        return self._filters_matching_x(
-            BugSubscriptionFilterTag, condition)
-
-    def _filters_matching_all_x_tags(self, where_condition):
-        """Return an expression yielding `(subscription_id, filter_id)` rows.
-
-        This joins to `BugSubscriptionFilterTag` and calls up to
-        `_filters_matching_x`, and groups by filter. Conditions are added to
-        ensure that all rows in each group are a subset of the bug's tags.
-        """
-        tags_array = "ARRAY[%s]::TEXT[]" % ",".join(
-            quote(tag) for tag in self.tags)
-        return self._filters_matching_x(
-            BugSubscriptionFilterTag,
-            And(
-                BugSubscriptionFilterTag.filter_id == (
-                    BugSubscriptionFilter.id),
-                BugSubscriptionFilter.find_all_tags,
-                self.filter_conditions,
-                where_condition),
-            group_by=(
-                BugSubscriptionFilter.structural_subscription_id,
-                BugSubscriptionFilter.id),
-            having=ArrayContains(
-                SQL(tags_array), ArrayAgg(
-                    BugSubscriptionFilterTag.tag)))
-
-    @property
-    def filters_matching_all_include_tags(self):
-        """Filters including the bug's tags."""
-        return self._filters_matching_all_x_tags(
-            BugSubscriptionFilterTag.include)
-
-    @property
-    def filters_matching_all_exclude_tags(self):
-        """Filters excluding the bug's tags."""
-        return self._filters_matching_all_x_tags(
-            Not(BugSubscriptionFilterTag.include))
-
-    @property
-    def filters_matching_include_tags(self):
-        """Filters with tag filters including the bug."""
-        return Union(
-            self.filters_matching_any_include_tags,
-            self.filters_matching_all_include_tags)
-
-    @property
-    def filters_matching_exclude_tags(self):
-        """Filters with tag filters excluding the bug."""
-        return Union(
-            self.filters_matching_any_exclude_tags,
-            self.filters_matching_all_exclude_tags)
-
-    @property
-    def filters_matching_tags(self):
-        """Filters with tag filters matching the bug."""
-        if len(self.tags) == 0:
-            # The filter's required tags must be an empty set. The filter's
-            # excluded tags can be anything so no condition is needed.
-            return self.filters_without_include_tags
-        else:
-            return Except(
-                Union(self.filters_without_include_tags,
-                      self.filters_matching_include_tags),
-                self.filters_matching_exclude_tags)
-
-    @property
-    def filters_matching(self):
-        """Filters matching the bug."""
-        return Intersect(
-            self.filters_matching_status,
-            self.filters_matching_importance,
-            self.filters_matching_tags)
-
-    @property
-    def subscriptions_with_matching_filters(self):
-        """Subscriptions with one or more filters matching the bug."""
-        return Select(
-            # I don't know of a more Storm-like way of doing this.
-            SQL("filters_matching.structural_subscription_id"),
-            tables=Alias(self.filters_matching, "filters_matching"))
-
-    @property
-    def subscriptions(self):
-        return Union(
-            self.subscriptions_without_filters,
-            self.subscriptions_with_matching_filters)
+                # There is a filter and ...
+                And(
+                    # There's no status filter, or there is a status filter
+                    # and and it matches.
+                    Or(BugSubscriptionFilterStatus.id == None,
+                       BugSubscriptionFilterStatus.status == bugtask.status),
+                    # There's no importance filter, or there is an importance
+                    # filter and it matches.
+                    Or(BugSubscriptionFilterImportance.id == None,
+                       BugSubscriptionFilterImportance.importance == (
+                            bugtask.importance)),
+                    # Any number of conditions relating to tags.
+                    *tag_conditions)),
+            ]
+
+        return Store.of(self.__helper.pillar).using(*origin).find(
+            StructuralSubscription, self.__helper.join, *conditions)
 
=== modified file 'lib/lp/registry/tests/test_structuralsubscriptiontarget.py'
--- lib/lp/registry/tests/test_structuralsubscriptiontarget.py	2010-11-18 21:29:44 +0000
+++ lib/lp/registry/tests/test_structuralsubscriptiontarget.py	2010-11-25 21:03:11 +0000
@@ -32,6 +32,7 @@
     BugTaskImportance,
     BugTaskStatus,
     )
+from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
 from lp.bugs.tests.test_bugtarget import bugtarget_filebug
 from lp.registry.enum import BugNotificationLevel
 from lp.registry.errors import (
@@ -56,11 +57,10 @@
 from lp.testing.matchers import Provides
 
 
-class RestrictedStructuralSubscriptionTestBase:
-    """Tests suitable for a target that restricts structural subscriptions."""
+class StructuralSubscriptionTestBase:
 
     def setUp(self):
-        super(RestrictedStructuralSubscriptionTestBase, self).setUp()
+        super(StructuralSubscriptionTestBase, self).setUp()
         self.ordinary_subscriber = self.factory.makePerson()
         self.bug_supervisor_subscriber = self.factory.makePerson()
         self.team_owner = self.factory.makePerson()
@@ -124,12 +124,7 @@
             self.ordinary_subscriber, self.ordinary_subscriber)
 
 
-class UnrestrictedStructuralSubscriptionTestBase(
-    RestrictedStructuralSubscriptionTestBase):
-    """
-    Tests suitable for a target that does not restrict structural
-    subscriptions.
-    """
+class UnrestrictedStructuralSubscription(StructuralSubscriptionTestBase):
 
     def test_structural_subscription_by_ordinary_user(self):
         # ordinary users can subscribe themselves
@@ -172,276 +167,198 @@
             None)
 
 
-class FilteredStructuralSubscriptionTestBase:
+class FilteredStructuralSubscriptionTestBase(StructuralSubscriptionTestBase):
     """Tests for filtered structural subscriptions."""
 
-    layer = LaunchpadFunctionalLayer
-
-    def makeTarget(self):
-        raise NotImplementedError(self.makeTarget)
-
     def makeBugTask(self):
         return self.factory.makeBugTask(target=self.target)
 
-    def setUp(self):
-        super(FilteredStructuralSubscriptionTestBase, self).setUp()
-        self.ordinary_subscriber = self.factory.makePerson()
-        login_person(self.ordinary_subscriber)
-        self.target = self.makeTarget()
-        self.bugtask = self.makeBugTask()
-        self.bug = self.bugtask.bug
-        self.subscription = self.target.addSubscription(
-            self.ordinary_subscriber, self.ordinary_subscriber)
-        self.subscription.bug_notification_level = (
-            BugNotificationLevel.COMMENTS)
-
-    def assertSubscriptions(
-        self, expected_subscriptions, level=BugNotificationLevel.NOTHING):
-        observed_subscriptions = list(
-            self.target.getSubscriptionsForBugTask(self.bugtask, level))
-        self.assertEqual(expected_subscriptions, observed_subscriptions)
-
     def test_getSubscriptionsForBugTask(self):
         # If no one has a filtered subscription for the given bug, the result
         # of getSubscriptionsForBugTask() is the same as for
         # getSubscriptions().
+        bugtask = self.makeBugTask()
         subscriptions = self.target.getSubscriptions(
             min_bug_notification_level=BugNotificationLevel.NOTHING)
-        self.assertSubscriptions(list(subscriptions))
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual(list(subscriptions), list(subscriptions_for_bugtask))
 
     def test_getSubscriptionsForBugTask_with_filter_on_status(self):
         # If a status filter exists for a subscription, the result of
         # getSubscriptionsForBugTask() may be a subset of getSubscriptions().
+        bugtask = self.makeBugTask()
+
+        # Create a new subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.COMMENTS
 
         # Without any filters the subscription is found.
-        self.assertSubscriptions([self.subscription])
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
         # Filter the subscription to bugs in the CONFIRMED state.
-        subscription_filter = self.subscription.newBugFilter()
+        subscription_filter = BugSubscriptionFilter()
+        subscription_filter.structural_subscription = subscription
         subscription_filter.statuses = [BugTaskStatus.CONFIRMED]
 
         # With the filter the subscription is not found.
-        self.assertSubscriptions([])
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted, the subscription is found again.
-        subscription_filter.statuses = [self.bugtask.status]
-        self.assertSubscriptions([self.subscription])
+        subscription_filter.statuses = [bugtask.status]
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
     def test_getSubscriptionsForBugTask_with_filter_on_importance(self):
         # If an importance filter exists for a subscription, the result of
         # getSubscriptionsForBugTask() may be a subset of getSubscriptions().
+        bugtask = self.makeBugTask()
+
+        # Create a new subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.COMMENTS
 
         # Without any filters the subscription is found.
-        self.assertSubscriptions([self.subscription])
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
         # Filter the subscription to bugs in the CRITICAL state.
-        subscription_filter = self.subscription.newBugFilter()
+        subscription_filter = BugSubscriptionFilter()
+        subscription_filter.structural_subscription = subscription
         subscription_filter.importances = [BugTaskImportance.CRITICAL]
 
         # With the filter the subscription is not found.
-        self.assertSubscriptions([])
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted, the subscription is found again.
-        subscription_filter.importances = [self.bugtask.importance]
-        self.assertSubscriptions([self.subscription])
+        subscription_filter.importances = [bugtask.importance]
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
     def test_getSubscriptionsForBugTask_with_filter_on_level(self):
         # All structural subscriptions have a level for bug notifications
         # which getSubscriptionsForBugTask() observes.
+        bugtask = self.makeBugTask()
 
-        # Adjust the subscription level to METADATA.
-        self.subscription.bug_notification_level = (
-            BugNotificationLevel.METADATA)
+        # Create a new METADATA level subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.METADATA
 
         # The subscription is found when looking for NOTHING or above.
-        self.assertSubscriptions(
-            [self.subscription], BugNotificationLevel.NOTHING)
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
         # The subscription is found when looking for METADATA or above.
-        self.assertSubscriptions(
-            [self.subscription], BugNotificationLevel.METADATA)
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.METADATA)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
         # The subscription is not found when looking for COMMENTS or above.
-        self.assertSubscriptions(
-            [], BugNotificationLevel.COMMENTS)
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.COMMENTS)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
     def test_getSubscriptionsForBugTask_with_filter_include_any_tags(self):
         # If a subscription filter has include_any_tags, a bug with one or
         # more tags is matched.
+        bugtask = self.makeBugTask()
 
-        subscription_filter = self.subscription.newBugFilter()
+        # Create a new subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.COMMENTS
+        subscription_filter = subscription.newBugFilter()
         subscription_filter.include_any_tags = True
 
         # Without any tags the subscription is not found.
-        self.assertSubscriptions([])
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # With any tag the subscription is found.
-        self.bug.tags = ["foo"]
-        self.assertSubscriptions([self.subscription])
+        bugtask.bug.tags = ["foo"]
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
     def test_getSubscriptionsForBugTask_with_filter_exclude_any_tags(self):
         # If a subscription filter has exclude_any_tags, only bugs with no
         # tags are matched.
+        bugtask = self.makeBugTask()
 
-        subscription_filter = self.subscription.newBugFilter()
+        # Create a new subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.COMMENTS
+        subscription_filter = subscription.newBugFilter()
         subscription_filter.exclude_any_tags = True
 
         # Without any tags the subscription is found.
-        self.assertSubscriptions([self.subscription])
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
         # With any tag the subscription is not found.
-        self.bug.tags = ["foo"]
-        self.assertSubscriptions([])
-
-    def test_getSubscriptionsForBugTask_with_filter_for_any_tag(self):
-        # If a subscription filter specifies that any of one or more specific
-        # tags must be present, bugs with any of those tags are matched.
-
-        # Looking for either the "foo" or the "bar" tag.
-        subscription_filter = self.subscription.newBugFilter()
-        subscription_filter.tags = [u"foo", u"bar"]
-        subscription_filter.find_all_tags = False
-
-        # Without either tag the subscription is not found.
-        self.assertSubscriptions([])
-
-        # With either tag the subscription is found.
-        self.bug.tags = ["bar", "baz"]
-        self.assertSubscriptions([self.subscription])
-
-    def test_getSubscriptionsForBugTask_with_filter_for_all_tags(self):
-        # If a subscription filter specifies that all of one or more specific
-        # tags must be present, bugs with all of those tags are matched.
-
-        # Looking for both the "foo" and the "bar" tag.
-        subscription_filter = self.subscription.newBugFilter()
-        subscription_filter.tags = [u"foo", u"bar"]
-        subscription_filter.find_all_tags = True
-
-        # Without either tag the subscription is not found.
-        self.assertSubscriptions([])
-
-        # Without only one of the required tags the subscription is not found.
-        self.bug.tags = ["foo"]
-        self.assertSubscriptions([])
-
-        # With both required tags the subscription is found.
-        self.bug.tags = ["foo", "bar"]
-        self.assertSubscriptions([self.subscription])
-
-    def test_getSubscriptionsForBugTask_with_filter_for_not_any_tag(self):
-        # If a subscription filter specifies that any of one or more specific
-        # tags must not be present, bugs without any of those tags are
-        # matched.
-
-        # Looking to exclude the "foo" or "bar" tags.
-        subscription_filter = self.subscription.newBugFilter()
-        subscription_filter.tags = [u"-foo", u"-bar"]
-        subscription_filter.find_all_tags = False
-
-        # Without either tag the subscription is found.
-        self.assertSubscriptions([self.subscription])
-
-        # With either tag the subscription is no longer found.
-        self.bug.tags = ["foo"]
-        self.assertSubscriptions([])
-
-    def test_getSubscriptionsForBugTask_with_filter_for_not_all_tags(self):
-        # If a subscription filter specifies that all of one or more specific
-        # tags must not be present, bugs without all of those tags are
-        # matched.
-
-        # Looking to exclude the "foo" and "bar" tags.
-        subscription_filter = self.subscription.newBugFilter()
-        subscription_filter.tags = [u"-foo", u"-bar"]
-        subscription_filter.find_all_tags = True
-
-        # Without either tag the subscription is found.
-        self.assertSubscriptions([self.subscription])
-
-        # With only one of the excluded tags the subscription is found.
-        self.bug.tags = ["foo"]
-        self.assertSubscriptions([self.subscription])
-
-        # With both tags the subscription is no longer found.
-        self.bug.tags = ["foo", "bar"]
-        self.assertSubscriptions([])
+        bugtask.bug.tags = ["foo"]
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
     def test_getSubscriptionsForBugTask_with_multiple_filters(self):
         # If multiple filters exist for a subscription, all filters must
         # match.
+        bugtask = self.makeBugTask()
 
-        # Add the "foo" tag to the bug.
-        self.bug.tags = ["foo"]
+        # Create a new subscription on self.target.
+        login_person(self.ordinary_subscriber)
+        subscription = self.target.addSubscription(
+            self.ordinary_subscriber, self.ordinary_subscriber)
+        subscription.bug_notification_level = BugNotificationLevel.COMMENTS
 
         # Filter the subscription to bugs in the CRITICAL state.
-        subscription_filter = self.subscription.newBugFilter()
+        subscription_filter = BugSubscriptionFilter()
+        subscription_filter.structural_subscription = subscription
         subscription_filter.statuses = [BugTaskStatus.CONFIRMED]
         subscription_filter.importances = [BugTaskImportance.CRITICAL]
 
         # With the filter the subscription is not found.
-        self.assertSubscriptions([])
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted to match status but not importance, the
         # subscription is still not found.
-        subscription_filter.statuses = [self.bugtask.status]
-        self.assertSubscriptions([])
+        subscription_filter.statuses = [bugtask.status]
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([], list(subscriptions_for_bugtask))
 
         # If the filter is adjusted to also match importance, the subscription
         # is found again.
-        subscription_filter.importances = [self.bugtask.importance]
-        self.assertSubscriptions([self.subscription])
-
-        # If the filter is given some tag criteria, the subscription is not
-        # found.
-        subscription_filter.tags = [u"-foo", u"bar", u"baz"]
-        subscription_filter.find_all_tags = False
-        self.assertSubscriptions([])
-
-        # After removing the "foo" tag and adding the "bar" tag, the
-        # subscription is found.
-        self.bug.tags = ["bar"]
-        self.assertSubscriptions([self.subscription])
-
-        # Requiring that all tag criteria are fulfilled causes the
-        # subscription to no longer be found.
-        subscription_filter.find_all_tags = True
-        self.assertSubscriptions([])
-
-        # After adding the "baz" tag, the subscription is found again.
-        self.bug.tags = ["bar", "baz"]
-        self.assertSubscriptions([self.subscription])
-
-    def test_getSubscriptionsForBugTask_any_filter_is_a_match(self):
-        # If a subscription has multiple filters, the subscription is selected
-        # when any filter is found to match. Put another way, the filters are
-        # ORed together.
-        subscription_filter1 = self.subscription.newBugFilter()
-        subscription_filter1.statuses = [BugTaskStatus.CONFIRMED]
-        subscription_filter2 = self.subscription.newBugFilter()
-        subscription_filter2.tags = [u"foo"]
-
-        # With the filter the subscription is not found.
-        self.assertSubscriptions([])
-
-        # If the bugtask is adjusted to match the criteria of the first filter
-        # but not those of the second, the subscription is found.
-        self.bugtask.transitionToStatus(
-            BugTaskStatus.CONFIRMED, self.ordinary_subscriber)
-        self.assertSubscriptions([self.subscription])
-
-        # If the filter is adjusted to also match the criteria of the second
-        # filter, the subscription is still found.
-        self.bugtask.bug.tags = [u"foo"]
-        self.assertSubscriptions([self.subscription])
-
-        # If the bugtask is adjusted to no longer match the criteria of the
-        # first filter, the subscription is still found.
-        self.bugtask.transitionToStatus(
-            BugTaskStatus.INPROGRESS, self.ordinary_subscriber)
-        self.assertSubscriptions([self.subscription])
+        subscription_filter.importances = [bugtask.importance]
+        subscriptions_for_bugtask = self.target.getSubscriptionsForBugTask(
+            bugtask, BugNotificationLevel.NOTHING)
+        self.assertEqual([subscription], list(subscriptions_for_bugtask))
 
 
 class TestStructuralSubscriptionForDistro(
-    RestrictedStructuralSubscriptionTestBase, TestCaseWithFactory):
+    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
@@ -504,15 +421,10 @@
             StructuralSubscription)
 
 
-class TestStructuralSubscriptionFiltersForDistro(
-    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
-
-    def makeTarget(self):
-        return self.factory.makeDistribution()
-
-
 class TestStructuralSubscriptionForProduct(
-    UnrestrictedStructuralSubscriptionTestBase, TestCaseWithFactory):
+    UnrestrictedStructuralSubscription,
+    FilteredStructuralSubscriptionTestBase,
+    TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
@@ -521,15 +433,10 @@
         self.target = self.factory.makeProduct()
 
 
-class TestStructuralSubscriptionFiltersForProduct(
-    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
-
-    def makeTarget(self):
-        return self.factory.makeProduct()
-
-
 class TestStructuralSubscriptionForDistroSourcePackage(
-    UnrestrictedStructuralSubscriptionTestBase, TestCaseWithFactory):
+    UnrestrictedStructuralSubscription,
+    FilteredStructuralSubscriptionTestBase,
+    TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
@@ -539,15 +446,10 @@
         self.target = ProxyFactory(self.target)
 
 
-class TestStructuralSubscriptionFiltersForDistroSourcePackage(
-    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
-
-    def makeTarget(self):
-        return self.factory.makeDistributionSourcePackage()
-
-
 class TestStructuralSubscriptionForMilestone(
-    UnrestrictedStructuralSubscriptionTestBase, TestCaseWithFactory):
+    UnrestrictedStructuralSubscription,
+    FilteredStructuralSubscriptionTestBase,
+    TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
@@ -556,19 +458,15 @@
         self.target = self.factory.makeMilestone()
         self.target = ProxyFactory(self.target)
 
-
-class TestStructuralSubscriptionFiltersForMilestone(
-    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
-
-    def makeTarget(self):
-        return self.factory.makeMilestone()
-
     def makeBugTask(self):
+        # XXX Should test with target *and* series_target.
         return self.factory.makeBugTask(target=self.target.series_target)
 
 
 class TestStructuralSubscriptionForDistroSeries(
-    UnrestrictedStructuralSubscriptionTestBase, TestCaseWithFactory):
+    UnrestrictedStructuralSubscription,
+    FilteredStructuralSubscriptionTestBase,
+    TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
@@ -578,15 +476,10 @@
         self.target = ProxyFactory(self.target)
 
 
-class TestStructuralSubscriptionFiltersForDistroSeries(
-    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
-
-    def makeTarget(self):
-        return self.factory.makeDistroSeries()
-
-
 class TestStructuralSubscriptionForProjectGroup(
-    UnrestrictedStructuralSubscriptionTestBase, TestCaseWithFactory):
+    UnrestrictedStructuralSubscription,
+    FilteredStructuralSubscriptionTestBase,
+    TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
@@ -595,20 +488,15 @@
         self.target = self.factory.makeProject()
         self.target = ProxyFactory(self.target)
 
-
-class TestStructuralSubscriptionFiltersForProjectGroup(
-    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
-
-    def makeTarget(self):
-        return self.factory.makeProject()
-
     def makeBugTask(self):
         return self.factory.makeBugTask(
             target=self.factory.makeProduct(project=self.target))
 
 
 class TestStructuralSubscriptionForProductSeries(
-    UnrestrictedStructuralSubscriptionTestBase, TestCaseWithFactory):
+    UnrestrictedStructuralSubscription,
+    FilteredStructuralSubscriptionTestBase,
+    TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
@@ -618,13 +506,6 @@
         self.target = ProxyFactory(self.target)
 
 
-class TestStructuralSubscriptionFiltersForProductSeries(
-    FilteredStructuralSubscriptionTestBase, TestCaseWithFactory):
-
-    def makeTarget(self):
-        return self.factory.makeProductSeries()
-
-
 class TestStructuralSubscriptionTargetHelper(TestCaseWithFactory):
     """Tests for implementations of `IStructuralSubscriptionTargetHelper`."""