Merge lp:~cjwatson/launchpad/git-sharing into lp:launchpad

[Colin Watson]

Add sharing service support for Git repositories.

In https://code.launchpad.net/~cjwatson/launchpad/git-namespace/+merge/248995 I said that the next branch in this series would bring up test infrastructure. That turns out to have been a slight lie, because I remembered that creating test objects tends to require the ability to set their information type as well, so the sharing infrastructure needs to be in place. This branch sets that up. It's largely straightforward by analogy with branch handling, although I also did a bit of canonicalisation of argument ordering and ensuring that all calls to the methods in question use keyword arguments (oh for Python 3 and keyword-only arguments!).

Even with this, sharing doesn't quite work, but it's almost there. We can put the last bits of it in place once we have IGitCollection.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2014-06-19 02:12:50 +0000
+++ lib/lp/blueprints/model/specification.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -758,7 +758,7 @@
             # Grant the subscriber access if they can't see the
             # specification.
             service = getUtility(IService, 'sharing')
-            ignored, ignored, shared_specs = service.getVisibleArtifacts(
+            _, _, _, shared_specs = service.getVisibleArtifacts(
                 person, specifications=[self], ignore_permissions=True)
             if not shared_specs:
                 service.ensureAccessGrants(

=== modified file 'lib/lp/blueprints/tests/test_specification.py'
--- lib/lp/blueprints/tests/test_specification.py	2015-01-06 04:52:44 +0000
+++ lib/lp/blueprints/tests/test_specification.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for Specification."""
@@ -490,7 +490,7 @@
                 product=product, information_type=InformationType.PROPRIETARY)
             spec.subscribe(user, subscribed_by=owner)
             service = getUtility(IService, 'sharing')
-            ignored, ignored, shared_specs = service.getVisibleArtifacts(
+            _, _, _, shared_specs = service.getVisibleArtifacts(
                 user, specifications=[spec])
             self.assertEqual([spec], shared_specs)
             # The spec is also returned by getSharedSpecifications(),
@@ -507,7 +507,7 @@
             service.sharePillarInformation(
                 product, user_2, owner, permissions)
             spec.subscribe(user_2, subscribed_by=owner)
-            ignored, ignored, shared_specs = service.getVisibleArtifacts(
+            _, _, _, shared_specs = service.getVisibleArtifacts(
                 user_2, specifications=[spec])
             self.assertEqual([spec], shared_specs)
             self.assertEqual(
@@ -527,7 +527,7 @@
             spec.subscribe(user, subscribed_by=owner)
             spec.unsubscribe(user, unsubscribed_by=owner)
             service = getUtility(IService, 'sharing')
-            ignored, ignored, shared_specs = service.getVisibleArtifacts(
+            _, _, _, shared_specs = service.getVisibleArtifacts(
                 user, specifications=[spec])
             self.assertEqual([], shared_specs)
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2014-11-14 22:10:03 +0000
+++ lib/lp/bugs/model/bug.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Launchpad bug-related database table classes."""
@@ -836,7 +836,7 @@
         # there is at least one bugtask for which access can be checked.
         if self.default_bugtask:
             service = getUtility(IService, 'sharing')
-            bugs, ignored, ignored = service.getVisibleArtifacts(
+            bugs, _, _, _ = service.getVisibleArtifacts(
                 person, bugs=[self], ignore_permissions=True)
             if not bugs:
                 service.ensureAccessGrants(
@@ -1774,7 +1774,7 @@
         if information_type in PRIVATE_INFORMATION_TYPES:
             service = getUtility(IService, 'sharing')
             for person in (who, self.owner):
-                bugs, ignored, ignored = service.getVisibleArtifacts(
+                bugs, _, _, _ = service.getVisibleArtifacts(
                     person, bugs=[self], ignore_permissions=True)
                 if not bugs:
                     # subscribe() isn't sufficient if a subscription

=== modified file 'lib/lp/code/browser/branchsubscription.py'
--- lib/lp/code/browser/branchsubscription.py	2014-11-28 22:07:05 +0000
+++ lib/lp/code/browser/branchsubscription.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -200,7 +200,7 @@
     page_title = label = "Subscribe to branch"
 
     def validate(self, data):
-        if data.has_key('person'):
+        if 'person' in data:
             person = data['person']
             subscription = self.context.getSubscription(person)
             if subscription is None and not self.context.userCanBeSubscribed(
@@ -279,7 +279,7 @@
         url = canonical_url(self.branch)
         # If the subscriber can no longer see the branch, redirect them away.
         service = getUtility(IService, 'sharing')
-        ignored, branches, ignored = service.getVisibleArtifacts(
+        _, branches, _, _ = service.getVisibleArtifacts(
             self.person, branches=[self.branch], ignore_permissions=True)
         if not branches:
             url = canonical_url(self.branch.target)

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2015-02-16 13:41:24 +0000
+++ lib/lp/code/model/branch.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -914,7 +914,7 @@
             subscription.review_level = code_review_level
         # Grant the subscriber access if they can't see the branch.
         service = getUtility(IService, 'sharing')
-        ignored, branches, ignored = service.getVisibleArtifacts(
+        _, branches, _, _ = service.getVisibleArtifacts(
             person, branches=[self], ignore_permissions=True)
         if not branches:
             service.ensureAccessGrants(
@@ -928,7 +928,7 @@
         # currently accessible to the person but which the subscribed_by user
         # has edit permissions for.
         service = getUtility(IService, 'sharing')
-        ignored, invisible_stacked_branches = service.getInvisibleArtifacts(
+        _, invisible_stacked_branches, _ = service.getInvisibleArtifacts(
             person, branches=self.getStackedOnBranches())
         editable_stacked_on_branches = [
             branch for branch in invisible_stacked_branches

=== modified file 'lib/lp/code/model/tests/test_branchsubscription.py'
--- lib/lp/code/model/tests/test_branchsubscription.py	2012-09-19 13:22:42 +0000
+++ lib/lp/code/model/tests/test_branchsubscription.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the BranchSubscrptions model object.."""
@@ -133,7 +133,7 @@
                 None, CodeReviewNotificationLevel.NOEMAIL, owner)
             # The stacked on branch should be visible.
             service = getUtility(IService, 'sharing')
-            ignored, visible_branches, ignored = service.getVisibleArtifacts(
+            _, visible_branches, _, _ = service.getVisibleArtifacts(
                 grantee, branches=[private_stacked_on_branch])
             self.assertContentEqual(
                 [private_stacked_on_branch], visible_branches)
@@ -161,7 +161,7 @@
                 grantee, BranchSubscriptionNotificationLevel.NOEMAIL,
                 None, CodeReviewNotificationLevel.NOEMAIL, owner)
             # The stacked on branch should not be visible.
-            ignored, visible_branches, ignored = service.getVisibleArtifacts(
+            _, visible_branches, _, _ = service.getVisibleArtifacts(
                 grantee, branches=[private_stacked_on_branch])
             self.assertContentEqual([], visible_branches)
             self.assertIn(

=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2014-11-24 01:20:26 +0000
+++ lib/lp/registry/browser/pillar.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Common views for objects that implement `IPillar`."""
@@ -444,12 +444,14 @@
     def _loadSharedArtifacts(self):
         # As a concrete can by linked via more than one policy, we use sets to
         # filter out dupes.
-        self.bugtasks, self.branches, self.specifications = (
+        (self.bugtasks, self.branches, self.gitrepositories,
+         self.specifications) = (
             self.sharing_service.getSharedArtifacts(
                 self.pillar, self.person, self.user))
         bug_ids = set([bugtask.bug.id for bugtask in self.bugtasks])
         self.shared_bugs_count = len(bug_ids)
         self.shared_branches_count = len(self.branches)
+        self.shared_gitrepositories_count = len(self.gitrepositories)
         self.shared_specifications_count = len(self.specifications)
 
     def _build_specification_template_data(self, specs, request):

=== modified file 'lib/lp/registry/interfaces/accesspolicy.py'
--- lib/lp/registry/interfaces/accesspolicy.py	2012-09-21 11:41:56 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for pillar and artifact access policies."""
@@ -35,6 +35,7 @@
     concrete_artifact = Attribute("Concrete artifact")
     bug_id = Attribute("bug_id")
     branch_id = Attribute("branch_id")
+    gitrepository_id = Attribute("gitrepository_id")
     specification_id = Attribute("specification_id")
 
 

=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py	2015-02-06 15:17:07 +0000
+++ lib/lp/registry/interfaces/sharingservice.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for sharing service."""
@@ -108,7 +108,7 @@
 
         :param user: the user making the request. Only artifacts visible to the
              user will be included in the result.
-        :return: a (bugtasks, branches, specifications) tuple
+        :return: a (bugtasks, branches, gitrepositories, specifications) tuple
         """
 
     def checkPillarArtifactAccess(pillar, user):
@@ -148,6 +148,14 @@
         :return: a collection of branches
         """
 
+    def getSharedGitRepositories(pillar, person, user):
+        """Return the Git repositories shared between the pillar and person.
+
+        :param user: the user making the request. Only Git repositories
+             visible to the user will be included in the result.
+        :return: a collection of Git repositories.
+        """
+
     @export_read_operation()
     @call_with(user=REQUEST_USER)
     @operation_parameters(
@@ -163,19 +171,25 @@
         :return: a collection of specifications.
         """
 
-    def getVisibleArtifacts(person, branches=None, bugs=None):
+    def getVisibleArtifacts(person, bugs=None, branches=None,
+                            gitrepositories=None, specifications=None):
         """Return the artifacts shared with person.
 
         Given lists of artifacts, return those a person has access to either
         via a policy grant or artifact grant.
 
         :param person: the person whose access is being checked.
+        :param bugs: the bugs to check for which a person has access.
         :param branches: the branches to check for which a person has access.
-        :param bugs: the bugs to check for which a person has access.
+        :param gitrepositories: the Git repositories to check for which a
+            person has access.
+        :param specifications: the specifications to check for which a
+            person has access.
         :return: a collection of artifacts the person can see.
         """
 
-    def getInvisibleArtifacts(person, branches=None, bugs=None):
+    def getInvisibleArtifacts(person, bugs=None, branches=None,
+                              gitrepositories=None):
         """Return the artifacts which are not shared with person.
 
         Given lists of artifacts, return those a person does not have access to
@@ -184,8 +198,10 @@
           access to private information. Internal use only. *
 
         :param person: the person whose access is being checked.
+        :param bugs: the bugs to check for which a person has access.
         :param branches: the branches to check for which a person has access.
-        :param bugs: the bugs to check for which a person has access.
+        :param gitrepositories: the Git repositories to check for which a
+            person has access.
         :return: a collection of artifacts the person can not see.
         """
 
@@ -304,10 +320,11 @@
         branches=List(
             Reference(schema=IBranch), title=_('Branches'), required=False),
         specifications=List(
-            Reference(schema=ISpecification), title=_('Specifications'), required=False))
+            Reference(schema=ISpecification), title=_('Specifications'),
+            required=False))
     @operation_for_version('devel')
-    def revokeAccessGrants(pillar, grantee, user, branches=None, bugs=None,
-                           specifications=None):
+    def revokeAccessGrants(pillar, grantee, user, bugs=None, branches=None,
+                           gitrepositories=None, specifications=None):
         """Remove a grantee's access to the specified artifacts.
 
         :param pillar: the pillar from which to remove access
@@ -315,6 +332,7 @@
         :param user: the user making the request
         :param bugs: the bugs for which to revoke access
         :param branches: the branches for which to revoke access
+        :param gitrepositories: the Git repositories for which to revoke access
         :param specifications: the specifications for which to revoke access
         """
 
@@ -328,14 +346,15 @@
         branches=List(
             Reference(schema=IBranch), title=_('Branches'), required=False))
     @operation_for_version('devel')
-    def ensureAccessGrants(grantees, user, branches=None, bugs=None,
-                           specifications=None):
+    def ensureAccessGrants(grantees, user, bugs=None, branches=None,
+                           gitrepositories=None, specifications=None):
         """Ensure a grantee has an access grant to the specified artifacts.
 
         :param grantees: the people or teams for whom to grant access
         :param user: the user making the request
         :param bugs: the bugs for which to grant access
         :param branches: the branches for which to grant access
+        :param gitrepositories: the Git repositories for which to grant access
         :param specifications: the specifications for which to grant access
         """
 

=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py	2013-06-20 05:50:00 +0000
+++ lib/lp/registry/model/accesspolicy.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Model classes for pillar and artifact access policies."""
@@ -98,12 +98,16 @@
     bug = Reference(bug_id, 'Bug.id')
     branch_id = Int(name='branch')
     branch = Reference(branch_id, 'Branch.id')
+    gitrepository_id = Int(name='gitrepository')
+    gitrepository = Reference(gitrepository_id, 'GitRepository.id')
     specification_id = Int(name='specification')
     specification = Reference(specification_id, 'Specification.id')
 
     @property
     def concrete_artifact(self):
-        artifact = self.bug or self.branch or self.specification
+        artifact = (
+            self.bug or self.branch or self.gitrepository or
+            self.specification)
         return artifact
 
     @classmethod
@@ -111,10 +115,13 @@
         from lp.blueprints.interfaces.specification import ISpecification
         from lp.bugs.interfaces.bug import IBug
         from lp.code.interfaces.branch import IBranch
+        from lp.code.interfaces.gitrepository import IGitRepository
         if IBug.providedBy(concrete_artifact):
             col = cls.bug
         elif IBranch.providedBy(concrete_artifact):
             col = cls.branch
+        elif IGitRepository.providedBy(concrete_artifact):
+            col = cls.gitrepository
         elif ISpecification.providedBy(concrete_artifact):
             col = cls.specification
         else:
@@ -137,6 +144,7 @@
         from lp.blueprints.interfaces.specification import ISpecification
         from lp.bugs.interfaces.bug import IBug
         from lp.code.interfaces.branch import IBranch
+        from lp.code.interfaces.gitrepository import IGitRepository
 
         existing = list(cls.find(concrete_artifacts))
         if len(existing) == len(concrete_artifacts):
@@ -150,15 +158,17 @@
         insert_values = []
         for concrete in needed:
             if IBug.providedBy(concrete):
-                insert_values.append((concrete, None, None))
+                insert_values.append((concrete, None, None, None))
             elif IBranch.providedBy(concrete):
-                insert_values.append((None, concrete, None))
+                insert_values.append((None, concrete, None, None))
+            elif IGitRepository.providedBy(concrete):
+                insert_values.append((None, None, concrete, None))
             elif ISpecification.providedBy(concrete):
-                insert_values.append((None, None, concrete))
+                insert_values.append((None, None, None, concrete))
             else:
                 raise ValueError("%r is not a supported artifact" % concrete)
         new = create(
-            (cls.bug, cls.branch, cls.specification),
+            (cls.bug, cls.branch, cls.gitrepository, cls.specification),
             insert_values, get_objects=True)
         return list(existing) + new
 

=== modified file 'lib/lp/registry/model/sharingjob.py'
--- lib/lp/registry/model/sharingjob.py	2013-07-04 08:32:03 +0000
+++ lib/lp/registry/model/sharingjob.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Job classes related to the sharing feature are in here."""
@@ -10,7 +10,6 @@
     'RemoveArtifactSubscriptionsJob',
     ]
 
-import contextlib
 import logging
 
 from lazr.delegates import delegates
@@ -58,11 +57,13 @@
 from lp.bugs.model.bugtasksearch import get_bug_privacy_filter_terms
 from lp.code.interfaces.branch import IBranch
 from lp.code.interfaces.branchlookup import IBranchLookup
+from lp.code.interfaces.gitrepository import IGitRepository
 from lp.code.model.branch import (
     Branch,
     get_branch_privacy_filter,
     )
 from lp.code.model.branchsubscription import BranchSubscription
+from lp.code.model.gitrepository import GitRepository
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.sharingjob import (
@@ -85,7 +86,6 @@
     )
 from lp.services.job.runner import BaseRunnableJob
 from lp.services.mail.sendmail import format_address_for_person
-from lp.services.webapp import errorlog
 
 
 class SharingJobType(DBEnumeratedType):
@@ -265,6 +265,7 @@
 
         bug_ids = []
         branch_ids = []
+        gitrepository_ids = []
         specification_ids = []
         if artifacts:
             for artifact in artifacts:
@@ -272,6 +273,8 @@
                     bug_ids.append(artifact.id)
                 elif IBranch.providedBy(artifact):
                     branch_ids.append(artifact.id)
+                elif IGitRepository.providedBy(artifact):
+                    gitrepository_ids.append(artifact.id)
                 elif ISpecification.providedBy(artifact):
                     specification_ids.append(artifact.id)
                 else:
@@ -283,6 +286,7 @@
         metadata = {
             'bug_ids': bug_ids,
             'branch_ids': branch_ids,
+            'gitrepository_ids': gitrepository_ids,
             'specification_ids': specification_ids,
             'information_types': information_types,
             'requestor.id': requestor.id
@@ -315,6 +319,10 @@
         return [getUtility(IBranchLookup).get(id) for id in self.branch_ids]
 
     @property
+    def gitrepository_ids(self):
+        return self.metadata.get('gitrepository_ids', [])
+
+    @property
     def specification_ids(self):
         return self.metadata.get('specification_ids', [])
 
@@ -343,6 +351,7 @@
             'requestor': self.requestor.name,
             'bug_ids': self.bug_ids,
             'branch_ids': self.branch_ids,
+            'gitrepository_ids': self.gitrepository_ids,
             'specification_ids': self.specification_ids,
             'pillar': getattr(self.pillar, 'name', None),
             'grantee': getattr(self.grantee, 'name', None)
@@ -358,10 +367,14 @@
 
         bug_filters = []
         branch_filters = []
+        gitrepository_filters = []
         specification_filters = []
 
         if self.branch_ids:
             branch_filters.append(Branch.id.is_in(self.branch_ids))
+        if self.gitrepository_ids:
+            gitrepository_filters.append(GitRepository.id.is_in(
+                self.gitrepository_ids))
         if self.specification_ids:
             specification_filters.append(Specification.id.is_in(
                 self.specification_ids))
@@ -374,6 +387,9 @@
                         self.information_types))
                 branch_filters.append(
                     Branch.information_type.is_in(self.information_types))
+                gitrepository_filters.append(
+                    GitRepository.information_type.is_in(
+                        self.information_types))
                 specification_filters.append(
                     Specification.information_type.is_in(
                         self.information_types))
@@ -381,12 +397,16 @@
                 bug_filters.append(
                     BugTaskFlat.product == self.product)
                 branch_filters.append(Branch.product == self.product)
+                gitrepository_filters.append(
+                    GitRepository.project == self.product)
                 specification_filters.append(
                     Specification.product == self.product)
             if self.distro:
                 bug_filters.append(
                     BugTaskFlat.distribution == self.distro)
                 branch_filters.append(Branch.distribution == self.distro)
+                gitrepository_filters.append(
+                    GitRepository.distribution == self.distro)
                 specification_filters.append(
                     Specification.distribution == self.distro)
 
@@ -401,6 +421,8 @@
                     Select(
                         TeamParticipation.personID,
                         where=TeamParticipation.team == self.grantee)))
+            # XXX cjwatson 2015-02-05: Fill this in once we have
+            # GitRepositorySubscription.
             specification_filters.append(
                 In(SpecificationSubscription.personID,
                     Select(
@@ -430,6 +452,8 @@
             for sub in branch_subscriptions:
                 sub.branch.unsubscribe(
                     sub.person, self.requestor, ignore_permissions=True)
+        # XXX cjwatson 2015-02-05: Fill this in once we have
+        # GitRepositorySubscription.
         if specification_filters:
             specification_filters.append(Not(*get_specification_privacy_filter(
                 SpecificationSubscription.personID)))

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2013-06-20 05:50:00 +0000
+++ lib/lp/registry/services/sharingservice.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Classes for pillar and artifact sharing service."""
@@ -194,10 +194,12 @@
 
     @available_with_permission('launchpad.Driver', 'pillar')
     def getSharedArtifacts(self, pillar, person, user, include_bugs=True,
-                           include_branches=True, include_specifications=True):
+                           include_branches=True, include_gitrepositories=True,
+                           include_specifications=True):
         """See `ISharingService`."""
         bug_ids = set()
         branch_ids = set()
+        gitrepository_ids = set()
         specification_ids = set()
         for artifact in self.getArtifactGrantsForPersonOnPillar(
             pillar, person):
@@ -205,6 +207,8 @@
                 bug_ids.add(artifact.bug_id)
             elif artifact.branch_id and include_branches:
                 branch_ids.add(artifact.branch_id)
+            elif artifact.gitrepository_id and include_gitrepositories:
+                gitrepository_ids.add(artifact.gitrepository_id)
             elif artifact.specification_id and include_specifications:
                 specification_ids.add(artifact.specification_id)
 
@@ -221,11 +225,14 @@
             wanted_branches = all_branches.visibleByUser(user).withIds(
                 *branch_ids)
             branches = list(wanted_branches.getBranches())
+        # Load the Git repositories.
+        gitrepositories = []
+        # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place.
         specifications = []
         if specification_ids:
             specifications = load(Specification, specification_ids)
 
-        return bugtasks, branches, specifications
+        return bugtasks, branches, gitrepositories, specifications
 
     def checkPillarArtifactAccess(self, pillar, user):
         """See `ISharingService`."""
@@ -245,25 +252,33 @@
     @available_with_permission('launchpad.Driver', 'pillar')
     def getSharedBugs(self, pillar, person, user):
         """See `ISharingService`."""
-        bugtasks, ignore, ignore = self.getSharedArtifacts(
+        bugtasks, _, _, _ = self.getSharedArtifacts(
             pillar, person, user, include_branches=False,
-            include_specifications=False)
+            include_gitrepositories=False, include_specifications=False)
         return bugtasks
 
     @available_with_permission('launchpad.Driver', 'pillar')
     def getSharedBranches(self, pillar, person, user):
         """See `ISharingService`."""
-        ignore, branches, ignore = self.getSharedArtifacts(
+        _, branches, _, _ = self.getSharedArtifacts(
             pillar, person, user, include_bugs=False,
-            include_specifications=False)
+            include_gitrepositories=False, include_specifications=False)
         return branches
 
     @available_with_permission('launchpad.Driver', 'pillar')
+    def getSharedGitRepositories(self, pillar, person, user):
+        """See `ISharingService`."""
+        _, _, gitrepositories, _ = self.getSharedArtifacts(
+            pillar, person, user, include_bugs=False, include_branches=False,
+            include_specifications=False)
+        return gitrepositories
+
+    @available_with_permission('launchpad.Driver', 'pillar')
     def getSharedSpecifications(self, pillar, person, user):
         """See `ISharingService`."""
-        ignore, ignore, specifications = self.getSharedArtifacts(
-            pillar, person, user, include_bugs=False,
-            include_branches=False)
+        _, _, _, specifications = self.getSharedArtifacts(
+            pillar, person, user, include_bugs=False, include_branches=False,
+            include_gitrepositories=False)
         return specifications
 
     def _getVisiblePrivateSpecificationIDs(self, person, specifications):
@@ -300,11 +315,13 @@
             TeamParticipation.personID == person.id,
             In(Specification.id, spec_ids)))
 
-    def getVisibleArtifacts(self, person, branches=None, bugs=None,
-                            specifications=None, ignore_permissions=False):
+    def getVisibleArtifacts(self, person, bugs=None, branches=None,
+                            gitrepositories=None, specifications=None,
+                            ignore_permissions=False):
         """See `ISharingService`."""
         bugs_by_id = {}
         branches_by_id = {}
+        gitrepositories_by_id = {}
         for bug in bugs or []:
             if (not ignore_permissions
                 and not check_permission('launchpad.View', bug)):
@@ -315,6 +332,11 @@
                 and not check_permission('launchpad.View', branch)):
                 raise Unauthorized
             branches_by_id[branch.id] = branch
+        for gitrepository in gitrepositories or []:
+            if (not ignore_permissions
+                and not check_permission('launchpad.View', gitrepository)):
+                raise Unauthorized
+            gitrepositories_by_id[gitrepository.id] = gitrepository
         for spec in specifications or []:
             if (not ignore_permissions
                 and not check_permission('launchpad.View', spec)):
@@ -336,6 +358,11 @@
                 *branches_by_id.keys())
             visible_branches = list(wanted_branches.getBranches())
 
+        # Load the Git repositories.
+        visible_gitrepositories = []
+        # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place.
+
+        # Load the specifications.
         visible_specs = []
         if specifications:
             visible_private_spec_ids = self._getVisiblePrivateSpecificationIDs(
@@ -344,16 +371,22 @@
                 spec for spec in specifications
                 if spec.id in visible_private_spec_ids or not spec.private]
 
-        return visible_bugs, visible_branches, visible_specs
+        return (
+            visible_bugs, visible_branches, visible_gitrepositories,
+            visible_specs)
 
-    def getInvisibleArtifacts(self, person, branches=None, bugs=None):
+    def getInvisibleArtifacts(self, person, bugs=None, branches=None,
+                              gitrepositories=None):
         """See `ISharingService`."""
         bugs_by_id = {}
         branches_by_id = {}
+        gitrepositories_by_id = {}
         for bug in bugs or []:
             bugs_by_id[bug.id] = bug
         for branch in branches or []:
             branches_by_id[branch.id] = branch
+        for gitrepository in gitrepositories or []:
+            gitrepositories_by_id[gitrepository.id] = gitrepository
 
         # Load the bugs.
         visible_bug_ids = set()
@@ -376,7 +409,11 @@
                 branches_by_id[branch_id]
                 for branch_id in invisible_branch_ids]
 
-        return invisible_bugs, invisible_branches
+        # Load the Git repositories.
+        invisible_gitrepositories = []
+        # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place.
+
+        return invisible_bugs, invisible_branches, invisible_gitrepositories
 
     def getPeopleWithoutAccess(self, concrete_artifact, people):
         """See `ISharingService`."""
@@ -722,42 +759,51 @@
         return invisible_types
 
     @available_with_permission('launchpad.Edit', 'pillar')
-    def revokeAccessGrants(self, pillar, grantee, user, branches=None,
-                           bugs=None, specifications=None):
+    def revokeAccessGrants(self, pillar, grantee, user, bugs=None,
+                           branches=None, gitrepositories=None,
+                           specifications=None):
         """See `ISharingService`."""
 
-        if not branches and not bugs and not specifications:
+        if (not bugs and not branches and not gitrepositories and
+            not specifications):
             raise ValueError(
-                "Either bugs, branches or specifications must be specified")
+                "Either bugs, branches, gitrepositories, or specifications "
+                "must be specified")
 
         artifacts = []
+        if bugs:
+            artifacts.extend(bugs)
         if branches:
             artifacts.extend(branches)
-        if bugs:
-            artifacts.extend(bugs)
+        if gitrepositories:
+            artifacts.extend(gitrepositories)
         if specifications:
             artifacts.extend(specifications)
-        # Find the access artifacts associated with the bugs and branches.
+        # Find the access artifacts associated with the bugs, branches, Git
+        # repositories, and specifications.
         accessartifact_source = getUtility(IAccessArtifactSource)
         artifacts_to_delete = accessartifact_source.find(artifacts)
-        # Revoke access to bugs/branches for the specified grantee.
+        # Revoke access to artifacts for the specified grantee.
         getUtility(IAccessArtifactGrantSource).revokeByArtifact(
             artifacts_to_delete, [grantee])
 
         # Create a job to remove subscriptions for artifacts the grantee can no
         # longer see.
-        getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+        return getUtility(IRemoveArtifactSubscriptionsJobSource).create(
             user, artifacts, grantee=grantee, pillar=pillar)
 
-    def ensureAccessGrants(self, grantees, user, branches=None, bugs=None,
-                           specifications=None, ignore_permissions=False):
+    def ensureAccessGrants(self, grantees, user, bugs=None, branches=None,
+                           gitrepositories=None, specifications=None,
+                           ignore_permissions=False):
         """See `ISharingService`."""
 
         artifacts = []
+        if bugs:
+            artifacts.extend(bugs)
         if branches:
             artifacts.extend(branches)
-        if bugs:
-            artifacts.extend(bugs)
+        if gitrepositories:
+            artifacts.extend(gitrepositories)
         if specifications:
             artifacts.extend(specifications)
         if not ignore_permissions:
@@ -767,15 +813,15 @@
                 if not check_permission('launchpad.Edit', artifact):
                     raise Unauthorized
 
-        # Ensure there are access artifacts associated with the bugs and
-        # branches.
+        # Ensure there are access artifacts associated with the bugs,
+        # branches, Git repositories, and specifications.
         artifacts = getUtility(IAccessArtifactSource).ensure(artifacts)
         aagsource = getUtility(IAccessArtifactGrantSource)
         artifacts_with_grants = [
             artifact_grant.abstract_artifact
             for artifact_grant in
             aagsource.find(product(artifacts, grantees))]
-        # Create access to bugs/branches for the specified grantee for which a
+        # Create access to artifacts for the specified grantee for which a
         # grant does not already exist.
         missing_artifacts = set(artifacts) - set(artifacts_with_grants)
         getUtility(IAccessArtifactGrantSource).grant(

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2015-02-06 15:17:07 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2015-02-16 13:41:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -1075,9 +1075,10 @@
 
         # Check that grantees have expected access grants and subscriptions.
         for person in [team_grantee, person_grantee]:
-            visible_bugs, visible_branches, visible_specs = (
+            visible_bugs, visible_branches, _, visible_specs = (
                 self.service.getVisibleArtifacts(
-                    person, branches, bugs, specifications))
+                    person, bugs=bugs, branches=branches,
+                    specifications=specifications))
             self.assertContentEqual(bugs or [], visible_bugs)
             self.assertContentEqual(branches or [], visible_branches)
             self.assertContentEqual(specifications or [], visible_specs)
@@ -1102,8 +1103,9 @@
         for person in [team_grantee, person_grantee]:
             for bug in bugs or []:
                 self.assertNotIn(person, bug.getDirectSubscribers())
-            visible_bugs, visible_branches, visible_specs = (
-                self.service.getVisibleArtifacts(person, branches, bugs))
+            visible_bugs, visible_branches, _, visible_specs = (
+                self.service.getVisibleArtifacts(
+                    person, bugs=bugs, branches=branches))
             self.assertContentEqual([], visible_bugs)
             self.assertContentEqual([], visible_branches)
             self.assertContentEqual([], visible_specs)
@@ -1386,7 +1388,7 @@
             product, grantee, user)
 
         # Check the results.
-        shared_bugtasks, shared_branches, shared_specs = (
+        shared_bugtasks, shared_branches, _, shared_specs = (
             self.service.getSharedArtifacts(product, grantee, user))
         self.assertContentEqual(bug_tasks[:9], shared_bugtasks)
         self.assertContentEqual(branches[:9], shared_branches)
@@ -1673,8 +1675,9 @@
         # Test the getVisibleArtifacts method.
         grantee, ignore, branches, bugs, specs = self._make_Artifacts()
         # Check the results.
-        shared_bugs, shared_branches, shared_specs = (
-            self.service.getVisibleArtifacts(grantee, branches, bugs, specs))
+        shared_bugs, shared_branches, _, shared_specs = (
+            self.service.getVisibleArtifacts(
+                grantee, bugs=bugs, branches=branches, specifications=specs))
         self.assertContentEqual(bugs[:5], shared_bugs)
         self.assertContentEqual(branches[:5], shared_branches)
         self.assertContentEqual(specs[:5], shared_specs)
@@ -1683,8 +1686,9 @@
         # getVisibleArtifacts() returns private specifications if
         # user has a policy grant for the pillar of the specification.
         ignore, owner, branches, bugs, specs = self._make_Artifacts()
-        shared_bugs, shared_branches, shared_specs = (
-            self.service.getVisibleArtifacts(owner, branches, bugs, specs))
+        shared_bugs, shared_branches, _, shared_specs = (
+            self.service.getVisibleArtifacts(
+                owner, bugs=bugs, branches=branches, specifications=specs))
         self.assertContentEqual(bugs, shared_bugs)
         self.assertContentEqual(branches, shared_branches)
         self.assertContentEqual(specs, shared_specs)
@@ -1693,8 +1697,9 @@
         # Test the getInvisibleArtifacts method.
         grantee, ignore, branches, bugs, specs = self._make_Artifacts()
         # Check the results.
-        not_shared_bugs, not_shared_branches = (
-            self.service.getInvisibleArtifacts(grantee, branches, bugs))
+        not_shared_bugs, not_shared_branches, _ = (
+            self.service.getInvisibleArtifacts(
+                grantee, bugs=bugs, branches=branches))
         self.assertContentEqual(bugs[5:], not_shared_bugs)
         self.assertContentEqual(branches[5:], not_shared_branches)
 
@@ -1718,7 +1723,7 @@
                 information_type=InformationType.USERDATA)
             bugs.append(bug)
 
-        shared_bugs, shared_branches, shared_specs = (
+        shared_bugs, shared_branches, _, shared_specs = (
             self.service.getVisibleArtifacts(grantee, bugs=bugs))
         self.assertContentEqual(bugs, shared_bugs)
 
@@ -1726,7 +1731,7 @@
         for x in range(0, 5):
             change_callback(bugs[x], owner)
         # Check the results.
-        shared_bugs, shared_branches, shared_specs = (
+        shared_bugs, shared_branches, _, shared_specs = (
             self.service.getVisibleArtifacts(grantee, bugs=bugs))
         self.assertContentEqual(bugs[5:], shared_bugs)