← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/git-subscriptions-by-path into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-subscriptions-by-path into lp:launchpad with lp:~cjwatson/launchpad/git-subscription-tests as a prerequisite.

Commit message:
Allow subscribing to only particular reference paths within a Git repository.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-subscriptions-by-path/+merge/310471

This is needed as a soft prerequisite for beefing up subscriptions to send revision information, since users may very well want to e.g. subscribe to changes to master without subscribing to random other branches.

It's conceivable that people may want to have separate subscriptions to different ref patterns (e.g. "send me diffs for changes to master, but only tell me summary information elsewhere".  I haven't attempted to solve that here (subscriptions are still unique up to person and repository), because it's not especially obvious how to do the UI.  That can always be retrofitted later if there's demand.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-subscriptions-by-path into lp:launchpad.
=== modified file 'lib/lp/code/browser/branchsubscription.py'
--- lib/lp/code/browser/branchsubscription.py	2015-09-28 17:38:45 +0000
+++ lib/lp/code/browser/branchsubscription.py	2016-11-09 18:21:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -232,7 +232,7 @@
                 '%s was already subscribed to this branch with: '
                 % person.displayname,
                 subscription.notification_level, subscription.max_diff_lines,
-                review_level)
+                subscription.review_level)
 
 
 class BranchSubscriptionEditView(LaunchpadEditFormView):

=== modified file 'lib/lp/code/browser/gitsubscription.py'
--- lib/lp/code/browser/gitsubscription.py	2015-09-28 17:38:45 +0000
+++ lib/lp/code/browser/gitsubscription.py	2016-11-09 18:21:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -11,7 +11,12 @@
     'GitSubscriptionEditView',
     ]
 
+from lazr.restful.interface import (
+    copy_field,
+    use_template,
+    )
 from zope.component import getUtility
+from zope.interface import Interface
 
 from lp.app.browser.launchpadform import (
     action,
@@ -60,11 +65,35 @@
             key=lambda subscription: subscription.person.displayname)
 
 
+# XXX cjwatson 2016-11-09: We should just use IGitSubscription directly, but
+# we have to modify the description of `paths`: WADL generation demands that
+# the "*" characters be quoted as inline literals to avoid being treated as
+# mismatched emphasis characters, but "``" looks weird in HTML.  Perhaps we
+# should arrange for forms to run descriptions through pydoc so that we
+# don't display unprocessed markup?
+class IGitSubscriptionSchema(Interface):
+
+    use_template(IGitSubscription, include=[
+        "person",
+        "notification_level",
+        "max_diff_lines",
+        "review_level",
+        ])
+    paths = copy_field(IGitSubscription["paths"], description=(
+        u"A space-separated list of patterns matching subscribed reference "
+        u"paths.  For example, 'refs/heads/master refs/heads/next' "
+        u"matches just those two branches, while 'refs/heads/releases/*' "
+        u"matches all branches under 'refs/heads/releases/'.  Leave this "
+        u"blank to subscribe to the whole repository."))
+
+
 class _GitSubscriptionView(LaunchpadFormView):
     """Contains the common functionality of the Add and Edit views."""
 
-    schema = IGitSubscription
-    field_names = ['notification_level', 'max_diff_lines', 'review_level']
+    schema = IGitSubscriptionSchema
+    field_names = [
+        'paths', 'notification_level', 'max_diff_lines', 'review_level',
+        ]
 
     LEVELS_REQUIRING_LINES_SPECIFICATION = (
         BranchSubscriptionNotificationLevel.DIFFSONLY,
@@ -83,17 +112,25 @@
 
     cancel_url = next_url
 
-    def add_notification_message(self, initial, notification_level,
+    def add_notification_message(self, initial, paths, notification_level,
                                  max_diff_lines, review_level):
+        items = []
+        if paths is not None:
+            items.append("Only paths matching %(paths)s.")
+        items.append("%(notification_level)s")
         if notification_level in self.LEVELS_REQUIRING_LINES_SPECIFICATION:
-            lines_message = "<li>%s</li>" % max_diff_lines.description
-        else:
-            lines_message = ""
-
-        format_str = "%%s<ul><li>%%s</li>%s<li>%%s</li></ul>" % lines_message
+            items.append("%(max_diff_lines)s")
+        items.append("%(review_level)s")
+        format_str = (
+            "%(initial)s<ul>" +
+            "".join("<li>%s</li>" % item for item in items) + "</ul>")
         message = structured(
-            format_str, initial, notification_level.description,
-            review_level.description)
+            format_str, initial=initial, paths=paths,
+            notification_level=notification_level.description,
+            max_diff_lines=(
+                max_diff_lines.description
+                if max_diff_lines is not None else None),
+            review_level=review_level.description)
         self.request.response.addNotification(message)
 
     def optional_max_diff_lines(self, notification_level, max_diff_lines):
@@ -115,6 +152,7 @@
             self.request.response.addNotification(
                 "You are already subscribed to this repository.")
         else:
+            paths = data.get("paths")
             notification_level = data["notification_level"]
             max_diff_lines = self.optional_max_diff_lines(
                 notification_level, data["max_diff_lines"])
@@ -122,11 +160,11 @@
 
             self.context.subscribe(
                 self.user, notification_level, max_diff_lines, review_level,
-                self.user)
+                self.user, paths=paths)
 
             self.add_notification_message(
                 "You have subscribed to this repository with: ",
-                notification_level, max_diff_lines, review_level)
+                paths, notification_level, max_diff_lines, review_level)
 
 
 class GitSubscriptionEditOwnView(_GitSubscriptionView):
@@ -146,15 +184,19 @@
             # This is the case of URL hacking or stale page.
             return {}
         else:
-            return {"notification_level": subscription.notification_level,
-                    "max_diff_lines": subscription.max_diff_lines,
-                    "review_level": subscription.review_level}
+            return {
+                "paths": subscription.paths,
+                "notification_level": subscription.notification_level,
+                "max_diff_lines": subscription.max_diff_lines,
+                "review_level": subscription.review_level,
+                }
 
     @action("Change")
     def change_details(self, action, data):
         # Be proactive in the checking to catch the stale post problem.
         if self.context.hasSubscription(self.user):
             subscription = self.context.getSubscription(self.user)
+            subscription.paths = data.get("paths")
             subscription.notification_level = data["notification_level"]
             subscription.max_diff_lines = self.optional_max_diff_lines(
                 subscription.notification_level,
@@ -163,6 +205,7 @@
 
             self.add_notification_message(
                 "Subscription updated to: ",
+                subscription.paths,
                 subscription.notification_level,
                 subscription.max_diff_lines,
                 subscription.review_level)
@@ -186,7 +229,9 @@
     """View used to subscribe someone other than the current user."""
 
     field_names = [
-        "person", "notification_level", "max_diff_lines", "review_level"]
+        "person", "paths", "notification_level", "max_diff_lines",
+        "review_level",
+        ]
     for_input = True
 
     # Since we are subscribing other people, the current user
@@ -214,6 +259,7 @@
         to the repository.  Launchpad Admins are special and they can
         subscribe any team.
         """
+        paths = data.get("paths")
         notification_level = data["notification_level"]
         max_diff_lines = self.optional_max_diff_lines(
             notification_level, data["max_diff_lines"])
@@ -223,17 +269,17 @@
         if subscription is None:
             self.context.subscribe(
                 person, notification_level, max_diff_lines, review_level,
-                self.user)
+                self.user, paths=paths)
             self.add_notification_message(
                 "%s has been subscribed to this repository with: "
-                % person.displayname, notification_level, max_diff_lines,
-                review_level)
+                % person.displayname,
+                paths, notification_level, max_diff_lines, review_level)
         else:
             self.add_notification_message(
                 "%s was already subscribed to this repository with: "
                 % person.displayname,
-                subscription.notification_level, subscription.max_diff_lines,
-                review_level)
+                subscription.paths, subscription.notification_level,
+                subscription.max_diff_lines, subscription.review_level)
 
 
 class GitSubscriptionEditView(LaunchpadEditFormView):
@@ -243,8 +289,15 @@
     through the repository action item to edit the user's own subscription.
     This is the only current way to edit a team repository subscription.
     """
-    schema = IGitSubscription
-    field_names = ["notification_level", "max_diff_lines", "review_level"]
+    schema = IGitSubscriptionSchema
+    field_names = [
+        "paths", "notification_level", "max_diff_lines", "review_level",
+        ]
+
+    @property
+    def adapters(self):
+        """See `LaunchpadFormView`."""
+        return {IGitSubscriptionSchema: self.context}
 
     @property
     def page_title(self):

=== modified file 'lib/lp/code/browser/tests/test_gitsubscription.py'
--- lib/lp/code/browser/tests/test_gitsubscription.py	2016-11-09 18:21:16 +0000
+++ lib/lp/code/browser/tests/test_gitsubscription.py	2016-11-09 18:21:16 +0000
@@ -114,7 +114,7 @@
         with person_logged_in(subscriber):
             subscription = repository.getSubscription(subscriber)
         self.assertThat(subscription, MatchesStructure.byEquality(
-            person=subscriber, repository=repository,
+            person=subscriber, repository=repository, paths=None,
             notification_level=(
                 BranchSubscriptionNotificationLevel.ATTRIBUTEONLY),
             max_diff_lines=None,
@@ -145,6 +145,7 @@
             None, CodeReviewNotificationLevel.FULL, subscriber)
         browser = self.getViewBrowser(repository, user=subscriber)
         browser.getLink('Edit your subscription').click()
+        browser.getControl('Paths').value = 'refs/heads/master'
         browser.getControl('Notification Level').displayValue = [
             'Branch attribute and revision notifications']
         browser.getControl('Generated Diff Size Limit').displayValue = [
@@ -152,6 +153,7 @@
         browser.getControl('Change').click()
         self.assertTextMatchesExpressionIgnoreWhitespace(
             'Subscription updated to: '
+            'Only paths matching refs/heads/master. '
             'Send notifications for both branch attribute updates and new '
             'revisions added to the branch. '
             'Limit the generated diff to 5000 lines. '
@@ -161,6 +163,7 @@
             subscription = repository.getSubscription(subscriber)
         self.assertThat(subscription, MatchesStructure.byEquality(
             person=subscriber, repository=repository,
+            paths='refs/heads/master',
             notification_level=BranchSubscriptionNotificationLevel.FULL,
             max_diff_lines=BranchSubscriptionDiffSize.FIVEKLINES,
             review_level=CodeReviewNotificationLevel.FULL))

=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py	2016-11-09 18:21:16 +0000
+++ lib/lp/code/interfaces/gitref.py	2016-11-09 18:21:16 +0000
@@ -189,7 +189,7 @@
         "Persons subscribed to the repository containing this reference.")
 
     def subscribe(person, notification_level, max_diff_lines,
-                  code_review_level, subscribed_by):
+                  code_review_level, subscribed_by, paths=None):
         """Subscribe this person to the repository containing this reference.
 
         :param person: The `Person` to subscribe.
@@ -201,6 +201,8 @@
             cause notification.
         :param subscribed_by: The person who is subscribing the subscriber.
             Most often the subscriber themselves.
+        :param paths: An optional space-separated list of patterns matching
+            reference paths to subscribe to.
         :return: A new or existing `GitSubscription`.
         """
 

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2016-10-14 15:08:36 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2016-11-09 18:21:16 +0000
@@ -415,14 +415,23 @@
             vocabulary=BranchSubscriptionDiffSize),
         code_review_level=Choice(
             title=_("The level of code review notification emails."),
-            vocabulary=CodeReviewNotificationLevel))
+            vocabulary=CodeReviewNotificationLevel),
+        paths=TextLine(
+            title=_("Paths"), required=False,
+            description=_(
+                "A space-separated list of patterns matching reference paths "
+                "to subscribe to.  For example, ``refs/heads/master "
+                "refs/heads/next`` matches just those two branches, while "
+                "``refs/heads/releases/*`` matches all branches under "
+                "``refs/heads/releases/``.  Omit this parameter to subscribe "
+                "to the whole repository.")))
     # Really IGitSubscription, patched in _schema_circular_imports.py.
     @operation_returns_entry(Interface)
     @call_with(subscribed_by=REQUEST_USER)
     @export_write_operation()
     @operation_for_version("devel")
     def subscribe(person, notification_level, max_diff_lines,
-                  code_review_level, subscribed_by):
+                  code_review_level, subscribed_by, paths=None):
         """Subscribe this person to the repository.
 
         :param person: The `Person` to subscribe.
@@ -434,6 +443,8 @@
             cause notification.
         :param subscribed_by: The person who is subscribing the subscriber.
             Most often the subscriber themselves.
+        :param paths: An optional space-separated list of patterns matching
+            reference paths to subscribe to.
         :return: A new or existing `GitSubscription`.
         """
 
@@ -469,11 +480,14 @@
         :return: A `ResultSet`.
         """
 
-    def getNotificationRecipients():
+    def getNotificationRecipients(path=None):
         """Return a complete INotificationRecipientSet instance.
 
         The INotificationRecipientSet instance contains the subscribers
         and their subscriptions.
+
+        :param path: If not None, only consider subscriptions that match
+            this reference path.
         """
 
     landing_targets = exported(CollectionField(

=== modified file 'lib/lp/code/interfaces/gitsubscription.py'
--- lib/lp/code/interfaces/gitsubscription.py	2015-04-21 09:29:00 +0000
+++ lib/lp/code/interfaces/gitsubscription.py	2016-11-09 18:21:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Git repository subscription interfaces."""
@@ -22,6 +22,7 @@
 from zope.schema import (
     Choice,
     Int,
+    TextLine,
     )
 
 from lp import _
@@ -58,6 +59,14 @@
         Reference(
             title=_("Repository ID"), required=True, readonly=True,
             schema=IGitRepository))
+    paths = exported(TextLine(
+        title=_("Paths"), required=False,
+        description=_(
+            "A space-separated list of patterns matching subscribed reference "
+            "paths.  For example, ``refs/heads/master refs/heads/next`` "
+            "matches just those two branches, while ``refs/heads/releases/*`` "
+            "matches all branches under ``refs/heads/releases/``.  Leave this "
+            "blank to subscribe to the whole repository.")))
     notification_level = exported(
         Choice(
             title=_("Notification Level"), required=True,
@@ -97,3 +106,6 @@
     @operation_for_version("devel")
     def canBeUnsubscribedByUser(user):
         """Can the user unsubscribe the subscriber from the repository?"""
+
+    def matchesPath(path):
+        """Does this subscription match this reference path?"""

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2016-11-09 18:21:16 +0000
+++ lib/lp/code/model/gitref.py	2016-11-09 18:21:16 +0000
@@ -168,11 +168,11 @@
         return self.repository.subscribers
 
     def subscribe(self, person, notification_level, max_diff_lines,
-                  code_review_level, subscribed_by):
+                  code_review_level, subscribed_by, paths=None):
         """See `IGitRef`."""
         return self.repository.subscribe(
             person, notification_level, max_diff_lines, code_review_level,
-            subscribed_by)
+            subscribed_by, paths=paths)
 
     def getSubscription(self, person):
         """See `IGitRef`."""
@@ -184,7 +184,7 @@
 
     def getNotificationRecipients(self):
         """See `IGitRef`."""
-        return self.repository.getNotificationRecipients()
+        return self.repository.getNotificationRecipients(path=self.path)
 
     @property
     def landing_targets(self):

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2016-10-14 15:08:58 +0000
+++ lib/lp/code/model/gitrepository.py	2016-11-09 18:21:16 +0000
@@ -795,7 +795,7 @@
             person.anyone_can_join())
 
     def subscribe(self, person, notification_level, max_diff_lines,
-                  code_review_level, subscribed_by):
+                  code_review_level, subscribed_by, paths=None):
         """See `IGitRepository`."""
         if not self.userCanBeSubscribed(person):
             raise SubscriptionPrivacyViolation(
@@ -806,12 +806,13 @@
         subscription = self.getSubscription(person)
         if subscription is None:
             subscription = GitSubscription(
-                person=person, repository=self,
+                person=person, repository=self, paths=paths,
                 notification_level=notification_level,
                 max_diff_lines=max_diff_lines, review_level=code_review_level,
                 subscribed_by=subscribed_by)
             Store.of(subscription).flush()
         else:
+            subscription.paths = paths
             subscription.notification_level = notification_level
             subscription.max_diff_lines = max_diff_lines
             subscription.review_level = code_review_level
@@ -869,10 +870,12 @@
             artifact, [person])
         store.flush()
 
-    def getNotificationRecipients(self):
+    def getNotificationRecipients(self, path=None):
         """See `IGitRepository`."""
         recipients = NotificationRecipientSet()
         for subscription in self.subscriptions:
+            if path is not None and not subscription.matchesPath(path):
+                continue
             if subscription.person.is_team:
                 rationale = 'Subscriber @%s' % subscription.person.name
             else:

=== modified file 'lib/lp/code/model/gitsubscription.py'
--- lib/lp/code/model/gitsubscription.py	2015-07-08 16:05:11 +0000
+++ lib/lp/code/model/gitsubscription.py	2016-11-09 18:21:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -6,9 +6,12 @@
     'GitSubscription',
     ]
 
+import fnmatch
+
 from storm.locals import (
     Int,
     Reference,
+    Unicode,
     )
 from zope.interface import implementer
 
@@ -40,6 +43,8 @@
     repository_id = Int(name='repository', allow_none=False)
     repository = Reference(repository_id, 'GitRepository.id')
 
+    paths = Unicode(name='paths', allow_none=True)
+
     notification_level = EnumCol(
         enum=BranchSubscriptionNotificationLevel, notNull=True,
         default=DEFAULT)
@@ -52,19 +57,29 @@
         name='subscribed_by', allow_none=False, validator=validate_person)
     subscribed_by = Reference(subscribed_by_id, 'Person.id')
 
-    def __init__(self, person, repository, notification_level, max_diff_lines,
-                 review_level, subscribed_by):
+    def __init__(self, person, repository, paths, notification_level,
+                 max_diff_lines, review_level, subscribed_by):
         super(GitSubscription, self).__init__()
         self.person = person
         self.repository = repository
+        self.paths = paths
         self.notification_level = notification_level
         self.max_diff_lines = max_diff_lines
         self.review_level = review_level
         self.subscribed_by = subscribed_by
 
     def canBeUnsubscribedByUser(self, user):
-        """See `IBranchSubscription`."""
+        """See `IGitSubscription`."""
         if user is None:
             return False
         permission_check = GitSubscriptionEdit(self)
         return permission_check.checkAuthenticated(IPersonRoles(user))
+
+    def matchesPath(self, path):
+        """See `IGitSubscription`."""
+        if self.paths is None:
+            return True
+        for pattern in self.paths.split():
+            if fnmatch.fnmatch(path, pattern):
+                return True
+        return False

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2016-11-09 18:21:16 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2016-11-09 18:21:16 +0000
@@ -963,6 +963,34 @@
             source_ref=source, target_ref=target,
             prerequisite_ref=prerequisite, **kwargs)
 
+    def test_getNotificationRecipients_path(self):
+        # If the subscription specifies path patterns, then one of them must
+        # match the reference.
+        bmp = self.makeBranchMergeProposal()
+        source_owner = bmp.merge_source.owner
+        target_owner = bmp.merge_target.owner
+        recipients = bmp.getNotificationRecipients(
+            CodeReviewNotificationLevel.STATUS)
+        subscriber_set = set([source_owner, target_owner])
+        self.assertEqual(subscriber_set, set(recipients.keys()))
+        # Subscribing somebody to a non-matching path has no effect.
+        subscriber = self.factory.makePerson()
+        bmp.merge_source.subscribe(
+            subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
+            CodeReviewNotificationLevel.FULL, subscriber, paths=u"no-match")
+        recipients = bmp.getNotificationRecipients(
+            CodeReviewNotificationLevel.STATUS)
+        self.assertEqual(subscriber_set, set(recipients.keys()))
+        # Subscribing somebody to a matching path is effective.
+        bmp.merge_source.subscribe(
+            subscriber, BranchSubscriptionNotificationLevel.NOEMAIL, None,
+            CodeReviewNotificationLevel.FULL, subscriber,
+            paths=bmp.merge_source.path)
+        recipients = bmp.getNotificationRecipients(
+            CodeReviewNotificationLevel.STATUS)
+        subscriber_set.add(subscriber)
+        self.assertEqual(subscriber_set, set(recipients.keys()))
+
 
 class TestMergeProposalWebhooksMixin:
 

=== modified file 'lib/lp/code/model/tests/test_gitsubscription.py'
--- lib/lp/code/model/tests/test_gitsubscription.py	2015-05-14 13:57:51 +0000
+++ lib/lp/code/model/tests/test_gitsubscription.py	2016-11-09 18:21:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the GitSubscription model object."""
@@ -110,6 +110,41 @@
             repository.unsubscribe(subscribee, owner)
             self.assertFalse(repository.visibleByUser(subscribee))
 
+    def test_matchesPath_none_matches_anything(self):
+        # A paths=None subscription matches any path.
+        subscription = self.factory.makeGitSubscription(paths=None)
+        self.assertTrue(subscription.matchesPath(u"refs/heads/master"))
+        self.assertTrue(subscription.matchesPath(u"refs/heads/next"))
+        self.assertTrue(subscription.matchesPath(u"nonsense"))
+
+    def test_matchesPath_exact(self):
+        # A subscription to a single path matches only that path.
+        subscription = self.factory.makeGitSubscription(
+            paths=u"refs/heads/master")
+        self.assertTrue(subscription.matchesPath(u"refs/heads/master"))
+        self.assertFalse(subscription.matchesPath(u"refs/heads/master2"))
+        self.assertFalse(subscription.matchesPath(u"refs/heads/next"))
+
+    def test_matchesPath_fnmatch(self):
+        # A subscription to a path pattern matches anything that fnmatch
+        # accepts.
+        subscription = self.factory.makeGitSubscription(paths=u"refs/heads/*")
+        self.assertTrue(subscription.matchesPath(u"refs/heads/master"))
+        self.assertTrue(subscription.matchesPath(u"refs/heads/next"))
+        self.assertTrue(subscription.matchesPath(u"refs/heads/foo/bar"))
+        self.assertFalse(subscription.matchesPath(u"refs/tags/1.0"))
+
+    def test_matchesPath_multiple(self):
+        # A subscription to multiple space-separated path patterns matches
+        # any of them.
+        subscription = self.factory.makeGitSubscription(
+            paths=u"refs/heads/* refs/tags/1.0")
+        self.assertTrue(subscription.matchesPath(u"refs/heads/master"))
+        self.assertTrue(subscription.matchesPath(u"refs/heads/next"))
+        self.assertTrue(subscription.matchesPath(u"refs/heads/foo/bar"))
+        self.assertTrue(subscription.matchesPath(u"refs/tags/1.0"))
+        self.assertFalse(subscription.matchesPath(u"refs/tags/1.0a"))
+
 
 class TestGitSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
     """Tests for GitSubscription.canBeUnsubscribedByUser."""

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2016-11-03 15:07:36 +0000
+++ lib/lp/testing/factory.py	2016-11-09 18:21:16 +0000
@@ -1779,7 +1779,7 @@
         return repository
 
     def makeGitSubscription(self, repository=None, person=None,
-                            subscribed_by=None):
+                            subscribed_by=None, paths=None):
         """Create a GitSubscription."""
         if repository is None:
             repository = self.makeGitRepository()
@@ -1789,7 +1789,7 @@
             subscribed_by = person
         return repository.subscribe(removeSecurityProxy(person),
             BranchSubscriptionNotificationLevel.NOEMAIL, None,
-            CodeReviewNotificationLevel.NOEMAIL, subscribed_by)
+            CodeReviewNotificationLevel.NOEMAIL, subscribed_by, paths=paths)
 
     def makeGitRefs(self, repository=None, paths=None, **repository_kwargs):
         """Create and return a list of new, arbitrary GitRefs."""


Follow ups