← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wallyworld/launchpad/audit-sharing2-933815 into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/audit-sharing2-933815 into lp:launchpad with lp:~wallyworld/launchpad/audit-sharing-933815 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #933815 in Launchpad itself: "Cannot search and filter +sharing policies"
  https://bugs.launchpad.net/launchpad/+bug/933815

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/audit-sharing2-933815/+merge/99923

== Implementation ==

Next branch in the work to create the +audit-sharing view.
This branch adds the server side support for finding the indirect sharees ie those who can see a pillar because they belong to a team which has been granted access.

The sharing service interface has been kept the same, except for the addition of an optional boolean parameter to methods getPillarSharees and getPillarShareedata. The new parameter is include_indirect which determines whether indirect sharee data should be returned. If so, the teams via which the sharee has been granted access are included in the result. 

The indirect sharee methods execute only 1 additional query over their direct counterparts (to load the teams).

This branch also updates the audit sharing view to use the new service api to get the data model.

Next step will be to wire up the front end .

== Tests ==

Add new tests to:

test_accesspolicy
test_sharing_service
test_pillar_sharing

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/pillar.py
  lib/lp/registry/browser/tests/test_pillar_sharing.py
  lib/lp/registry/interfaces/accesspolicy.py
  lib/lp/registry/interfaces/sharingservice.py
  lib/lp/registry/model/accesspolicy.py
  lib/lp/registry/services/sharingservice.py
  lib/lp/registry/services/tests/test_sharingservice.py
  lib/lp/registry/tests/test_accesspolicy.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/audit-sharing2-933815/+merge/99923
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/audit-sharing2-933815 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2012-03-29 12:42:24 +0000
+++ lib/lp/registry/browser/pillar.py	2012-03-29 12:42:25 +0000
@@ -383,8 +383,9 @@
         return True
 
     def unbatched_sharees(self):
-        """All the sharees for a pillar."""
-        return self._getSharingService().getPillarSharees(self.context)
+        """All the indirect sharees for a pillar."""
+        return self._getSharingService().getPillarSharees(
+            self.context, include_indirect=True)
 
 
 class PillarPersonSharingView(LaunchpadView):

=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-03-29 12:42:24 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-03-29 12:42:25 +0000
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test views that manage sharing."""
+from lp.registry.model.person import Person
 
 __metaclass__ = type
 
@@ -139,6 +140,28 @@
 
     layer = DatabaseFunctionalLayer
 
+    def createSharees(self):
+        login_person(self.owner)
+        self.access_policy = self.factory.makeAccessPolicy(
+            pillar=self.pillar,
+            type=InformationType.PROPRIETARY)
+
+        def makeGrants(x):
+            grantee = self.factory.makePerson(name='name%sall' % x)
+            # Make access policy grant so that 'All' is returned.
+            self.factory.makeAccessPolicyGrant(self.access_policy, grantee)
+            # Make access artifact grants so that 'Some' is returned.
+            grantee = self.factory.makePerson(name='name%ssome' % x)
+            artifact_grant = self.factory.makeAccessArtifactGrant(
+                grantee=grantee)
+            self.factory.makeAccessPolicyArtifact(
+                artifact=artifact_grant.abstract_artifact,
+                policy=self.access_policy)
+        # Make grants for grantees in ascending order so we can slice off the
+        # first elements in the pillar observer results to check batching.
+        for x in range(10):
+            makeGrants(x)
+
     def test_init_without_feature_flag(self):
         # We need a feature flag to enable the view.
         self.assertRaises(
@@ -176,7 +199,18 @@
             view = create_view(self.pillar, name=self.view_name)
             with StormStatementRecorder() as recorder:
                 view.initialize()
-            self.assertThat(recorder, HasQueryCount(LessThan(6)))
+            self.assertThat(recorder, HasQueryCount(
+                LessThan(self.query_count + 1)))
+
+    def test_view_batch_data(self):
+        # Test the expected batching data is in the json request cache.
+        with FeatureFixture(ENABLED_FLAG):
+            view = create_initialized_view(self.pillar, name='+sharing')
+            cache = IJSONRequestCache(view.request)
+            # Test one expected data value (there are many).
+            next_batch = view.sharees().batch.nextBatch()
+            self.assertContentEqual(
+                next_batch.range_memo, cache.objects.get('next')['memo'])
 
     def test_view_write_enabled_without_feature_flag(self):
         # Test that sharing_write_enabled is not set without the feature flag.
@@ -203,28 +237,6 @@
 
     view_name = '+sharing'
 
-    def createSharees(self):
-        login_person(self.owner)
-        self.access_policy = self.factory.makeAccessPolicy(
-            pillar=self.pillar,
-            type=InformationType.PROPRIETARY)
-        self.grantees = []
-
-        def makeGrants(name):
-            grantee = self.factory.makePerson(name=name)
-            self.grantees.append(grantee)
-            # Make access policy grant so that 'All' is returned.
-            self.factory.makeAccessPolicyGrant(self.access_policy, grantee)
-            # Make access artifact grants so that 'Some' is returned.
-            artifact_grant = self.factory.makeAccessArtifactGrant()
-            self.factory.makeAccessPolicyArtifact(
-                artifact=artifact_grant.abstract_artifact,
-                policy=self.access_policy)
-        # Make grants for grantees in ascending order so we can slice off the
-        # first elements in the pillar observer results to check batching.
-        for x in range(10):
-            makeGrants('name%s' % x)
-
     def test_init_with_feature_flag(self):
         # The view works with a feature flag.
         with FeatureFixture(ENABLED_FLAG):
@@ -272,27 +284,19 @@
             batch_size = config.launchpad.default_batch_size
             apgfs = getUtility(IAccessPolicyGrantFlatSource)
             sharees = apgfs.findGranteePermissionsByPolicy(
-                [self.access_policy], self.grantees[:batch_size])
+                [self.access_policy]).order_by(Person.displayname, Person.name)
             sharing_service = getUtility(IService, 'sharing')
-            sharee_data = sharing_service.jsonShareeData(sharees)
+            sharee_data = sharing_service.jsonShareeData(sharees[:batch_size])
             self.assertContentEqual(
                 sharee_data, cache.objects.get('sharee_data'))
 
-    def test_view_batch_data(self):
-        # Test the expected batching data is in the json request cache.
-        with FeatureFixture(ENABLED_FLAG):
-            view = create_initialized_view(self.pillar, name='+sharing')
-            cache = IJSONRequestCache(view.request)
-            # Test one expected data value (there are many).
-            next_batch = view.sharees().batch.nextBatch()
-            self.assertContentEqual(
-                next_batch.range_memo, cache.objects.get('next')['memo'])
-
 
 class TestProductSharingInformationView(
                 PillarSharingInformationViewTestMixin, TestCaseWithFactory):
     """Test the PillarSharingInformationView with products."""
 
+    query_count = 6
+
     def setUp(self):
         super(TestProductSharingInformationView, self).setUp()
         self.driver = self.factory.makePerson()
@@ -307,6 +311,8 @@
                 PillarSharingInformationViewTestMixin, TestCaseWithFactory):
     """Test the PillarSharingInformationView with distributions."""
 
+    query_count = 5
+
     def setUp(self):
         super(TestDistributionSharingInformationView, self).setUp()
         self.driver = self.factory.makePerson()
@@ -348,18 +354,29 @@
             cache = IJSONRequestCache(view.request)
             self.assertIsNotNone(cache.objects.get('information_types'))
             self.assertIsNotNone(cache.objects.get('sharing_permissions'))
+            batch_size = config.launchpad.default_batch_size
+            apgfs = getUtility(IAccessPolicyGrantFlatSource)
+            sharees = apgfs.findIndirectGranteePermissionsByPolicy(
+                [self.access_policy]).order_by(Person.displayname, Person.name)
+            sharing_service = getUtility(IService, 'sharing')
+            sharee_data = sharing_service.jsonShareeData(sharees[:batch_size])
+            self.assertContentEqual(
+                sharee_data, cache.objects.get('sharee_data'))
 
 
 class TestProductAuditSharingView(
                 PillarAuditSharingViewTestMixin, TestCaseWithFactory):
     """Test the PillarSharingInformationView with products."""
 
+    query_count = 7
+
     def setUp(self):
         super(TestProductAuditSharingView, self).setUp()
         self.driver = self.factory.makePerson()
         self.owner = self.factory.makePerson()
         self.pillar = self.factory.makeProduct(
             owner=self.owner, driver=self.driver)
+        self.createSharees()
         login_person(self.driver)
 
 
@@ -367,10 +384,13 @@
                 PillarAuditSharingViewTestMixin, TestCaseWithFactory):
     """Test the PillarSharingInformationView with distributions."""
 
+    query_count = 6
+
     def setUp(self):
         super(TestDistributionAuditSharingView, self).setUp()
         self.driver = self.factory.makePerson()
         self.owner = self.factory.makePerson()
         self.pillar = self.factory.makeDistribution(
             owner=self.owner, driver=self.driver)
+        self.createSharees()
         login_person(self.driver)

=== modified file 'lib/lp/registry/interfaces/accesspolicy.py'
--- lib/lp/registry/interfaces/accesspolicy.py	2012-03-22 09:00:36 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py	2012-03-29 12:42:25 +0000
@@ -238,6 +238,25 @@
             SOME means the person only has specified access artifact grants.
         """
 
+    def findIndirectGranteePermissionsByPolicy(policies):
+        """Find teams or users with access grants for the policies.
+
+        This method is similar to findGranteePermissionsByPolicy, but the
+        results contain people who have access by virtue of team membership.
+
+        :param policies: a collection of `IAccesPolicy`s.
+        :return: a collection of
+            (`IPerson sharee`, `ITeam via`, `IAccessPolicy`, permission)
+            where
+            sharee is the person or team with access
+            via is the team the sharee belongs to in order to gain access.
+            If via is None, then the sharee has direct access.
+            permission is a SharingPermission enum value.
+            ALL means the person has an access policy grant and can see all
+            artifacts for the associated pillar.
+            SOME means the person only has specified access artifact grants.
+        """
+
     def findArtifactsByGrantee(grantee, policies):
         """Find the `IAccessArtifact`s for grantee and policies.
 

=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py	2012-03-26 14:10:32 +0000
+++ lib/lp/registry/interfaces/sharingservice.py	2012-03-29 12:42:25 +0000
@@ -21,6 +21,7 @@
     )
 from lazr.restful.fields import Reference
 from zope.schema import (
+    Bool,
     Choice,
     Dict,
     List,
@@ -54,31 +55,48 @@
 
     @export_read_operation()
     @operation_parameters(
-        pillar=Reference(IPillar, title=_('Pillar'), required=True))
+        pillar=Reference(IPillar, title=_('Pillar'), required=True),
+        include_indirect=Bool(title=_('Include indirect'), required=False))
     @operation_for_version('devel')
-    def getPillarSharees(pillar):
-        """Return people/teams who can see pillar artifacts."""
+    def getPillarSharees(pillar, include_indirect=False):
+        """Return people/teams who can see pillar artifacts.
+
+        :param pillar: the pillar to query
+        :param include_indirect: whether to include people/teams who have
+            access by virtue of the fact they are members of a team which has
+            been granted access.
+        """
 
     @export_read_operation()
     @operation_parameters(
-        pillar=Reference(IPillar, title=_('Pillar'), required=True))
+        pillar=Reference(IPillar, title=_('Pillar'), required=True),
+        include_indirect=Bool(title=_('Include indirect'), required=False))
     @operation_for_version('devel')
-    def getPillarShareeData(pillar):
+    def getPillarShareeData(pillar, include_indirect=False):
         """Return people/teams who can see pillar artifacts.
 
+        :param pillar: the pillar to query
+        :param include_indirect: whether to include people/teams who have
+            access by virtue of the fact they are members of a team which has
+            been granted access.
+
         The result records are json data which includes:
             - person name
+            - the team via which access is granted (if include_indirect=True)
             - permissions they have for each information type.
         """
 
     def jsonShareeData(grant_permissions):
         """Return people/teams who can see pillar artifacts.
 
-        :param grant_permissions: a list of (grantee, accesspolicy, permission)
+        :param grant_permissions: a list of
+            (grantee, {accesspolicy: permission}) or
+            (grantee, {accesspolicy: permission}, [via_team])
             tuples.
 
         The result records are json data which includes:
             - person name
+            - the teams via which the person has access
             - permissions they have for each information type.
         """
 

=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py	2012-03-23 04:12:59 +0000
+++ lib/lp/registry/model/accesspolicy.py	2012-03-29 12:42:25 +0000
@@ -17,9 +17,11 @@
 from storm.expr import (
     And,
     In,
+    Join,
     Or,
     Select,
     SQL,
+    With,
     )
 from storm.properties import (
     DateTime,
@@ -43,6 +45,7 @@
     IAccessPolicyGrant,
     )
 from lp.registry.model.person import Person
+from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.bulk import create
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import DBEnum
@@ -355,37 +358,52 @@
             Person, Person.id == cls.grantee_id, cls.policy_id.is_in(ids))
 
     @classmethod
-    def findGranteePermissionsByPolicy(cls, policies, grantees=None):
-        """See `IAccessPolicyGrantFlatSource`."""
-        policies_by_id = dict((policy.id, policy) for policy in policies)
+    def _populatePermissionsCache(cls, permissions_cache, grantee_ids,
+                                  policies_by_id, persons_by_id):
+        sharing_permission_term = SQL(
+            "CASE MIN(COALESCE(artifact, 0)) WHEN 0 THEN ? ELSE ? END",
+            (SharingPermission.ALL.name, SharingPermission.SOME.name))
+        constraints = [
+            cls.grantee_id.is_in(grantee_ids),
+            cls.policy_id.is_in(policies_by_id.keys())]
+        result_set = IStore(cls).find(
+            (cls.grantee_id, cls.policy_id, sharing_permission_term),
+            *constraints).group_by(cls.grantee_id, cls.policy_id)
+        for (person_id, policy_id, permission) in result_set:
+            person = persons_by_id[person_id]
+            policy = policies_by_id[policy_id]
+            permissions_cache[person][policy] = (
+                SharingPermission.items[str(permission)])
 
+    @classmethod
+    def _populateGranteePermissions(cls, policies_by_id, result_set):
         # A cache for the sharing permissions, keyed on grantee
         permissions_cache = defaultdict(dict)
 
-        def set_permission(person):
+        def set_permission(grantee):
             # Lookup the permissions from the previously loaded cache.
-            return (person[0], permissions_cache[person[0]])
+            return grantee[0], permissions_cache[grantee[0]]
 
-        def load_permissions(people):
+        def load_permissions(grantees):
             # We now have the grantees and policies we want in the result so
             # load any corresponding permissions and cache them.
-            people_by_id = dict(
-                (person[0].id, person[0]) for person in people)
-            sharing_permission_term = SQL(
-                "CASE MIN(COALESCE(artifact, 0)) WHEN 0 THEN ? ELSE ? END",
-                (SharingPermission.ALL.name, SharingPermission.SOME.name))
-            constraints = [
-                cls.grantee_id.is_in(people_by_id.keys()),
-                cls.policy_id.is_in(policies_by_id.keys())]
-            result_set = IStore(cls).find(
-                (cls.grantee_id, cls.policy_id, sharing_permission_term),
-                *constraints).group_by(cls.grantee_id, cls.policy_id)
-            for (person_id, policy_id, permission) in result_set:
-                person = people_by_id[person_id]
-                policy = policies_by_id[policy_id]
-                permissions_cache[person][policy] = (
-                    SharingPermission.items[str(permission)])
+            if permissions_cache:
+                return
+            grantee_ids = set()
+            grantee_by_id = dict()
+            for grantee in grantees:
+                grantee_ids.add(grantee[0].id)
+                grantee_by_id[grantee[0].id] = grantee[0]
+            cls._populatePermissionsCache(
+                permissions_cache, grantee_ids, policies_by_id, grantee_by_id)
+        return DecoratedResultSet(
+            result_set,
+            result_decorator=set_permission, pre_iter_hook=load_permissions)
 
+    @classmethod
+    def findGranteePermissionsByPolicy(cls, policies, grantees=None):
+        """See `IAccessPolicyGrantFlatSource`."""
+        policies_by_id = dict((policy.id, policy) for policy in policies)
         constraints = [cls.policy_id.is_in(policies_by_id.keys())]
         if grantees:
             grantee_ids = [grantee.id for grantee in grantees]
@@ -399,9 +417,86 @@
                 Select(
                     (cls.grantee_id,), where=And(*constraints),
                     distinct=True)))
+        return cls._populateGranteePermissions(policies_by_id, result_set)
+
+    @classmethod
+    def _populateIndirectGranteePermissions(cls,
+                                            policies_by_id, result_set):
+        # A cache for the sharing permissions, keyed on grantee
+        permissions_cache = defaultdict(dict)
+        # A cache of teams belonged to, keyed by grantee
+        via_teams_cache = defaultdict(list)
+        persons_by_id = dict()
+
+        def set_permission(grantee):
+            # Lookup the permissions from the previously loaded cache.
+            via_team_ids = via_teams_cache[grantee[0].id]
+            via_teams = [persons_by_id[team_id] for team_id in via_team_ids]
+            permissions = permissions_cache[grantee[0]]
+            # For access via teams, we need to use the team permissions. If a
+            # person has access via more than one team, we use the most
+            # powerful permission of all that are there.
+            for team in via_teams:
+                team_permissions = permissions_cache[team]
+                for info_type, permission in team_permissions.items():
+                    permission_to_use = permissions.get(info_type, permission)
+                    if permission == SharingPermission.ALL:
+                        permission_to_use = permission
+                    permissions[info_type] = permission_to_use
+            return grantee[0], permissions, via_teams or None
+
+        def load_teams_and_permissions(grantees):
+            # We now have the grantees we want in the result so load any
+            # associated team memberships and permissions and cache them.
+            if permissions_cache:
+                return
+            store = IStore(cls)
+            for grantee in grantees:
+                persons_by_id[grantee[0].id] = grantee[0]
+            # Find any teams associated with the grantees.
+            result_set = store.find(
+                (TeamParticipation.teamID, TeamParticipation.personID),
+                Or(
+                    TeamParticipation.teamID.is_in(persons_by_id.keys()),
+                    TeamParticipation.personID.is_in(persons_by_id.keys())))
+            team_ids = set()
+            for team_id, team_member_id in result_set:
+                if (team_id != team_member_id
+                    and team_member_id in persons_by_id.keys()):
+                        via_teams_cache[team_member_id].append(team_id)
+                team_ids.add(team_id)
+            # Load and cache the required persons.
+            persons = store.find(Person, Person.id.is_in(team_ids))
+            for person in persons:
+                persons_by_id[person.id] = person
+            cls._populatePermissionsCache(
+                permissions_cache, persons_by_id.keys(), policies_by_id,
+                persons_by_id)
+
         return DecoratedResultSet(
             result_set,
-            result_decorator=set_permission, pre_iter_hook=load_permissions)
+            result_decorator=set_permission,
+            pre_iter_hook=load_teams_and_permissions)
+
+    @classmethod
+    def findIndirectGranteePermissionsByPolicy(cls, policies):
+        """See `IAccessPolicyGrantFlatSource`."""
+        policies_by_id = dict((policy.id, policy) for policy in policies)
+        store = IStore(cls)
+        with_expr = With("grantees", store.find(
+            cls.grantee_id, cls.policy_id.is_in(policies_by_id.keys())
+        ).config(distinct=True)._get_select())
+        result_set = store.with_(with_expr).find(
+            (Person,),
+            In(
+                Person.id,
+                Select(
+                    (TeamParticipation.personID,),
+                    tables=(TeamParticipation, Join("grantees",
+                        SQL("grantees.grantee = TeamParticipation.team"))),
+                    distinct=True)))
+        return cls._populateIndirectGranteePermissions(
+            policies_by_id, result_set)
 
     @classmethod
     def findArtifactsByGrantee(cls, grantee, policies):

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-03-26 20:49:13 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-03-29 12:42:25 +0000
@@ -100,32 +100,34 @@
         return sharing_permissions
 
     @available_with_permission('launchpad.Driver', 'pillar')
-    def getPillarSharees(self, pillar):
+    def getPillarSharees(self, pillar, include_indirect=False):
         """See `ISharingService`."""
         policies = getUtility(IAccessPolicySource).findByPillar([pillar])
         ap_grant_flat = getUtility(IAccessPolicyGrantFlatSource)
         # XXX 2012-03-22 wallyworld bug 961836
         # We want to use person_sort_key(Person.displayname, Person.name) but
         # StormRangeFactory doesn't support that yet.
-        grant_permissions = ap_grant_flat.findGranteePermissionsByPolicy(
-            policies).order_by(Person.displayname, Person.name)
+        if include_indirect:
+            grant_permissions = (
+                ap_grant_flat.findIndirectGranteePermissionsByPolicy(
+                    policies).order_by(Person.displayname, Person.name))
+        else:
+            grant_permissions = ap_grant_flat.findGranteePermissionsByPolicy(
+                policies).order_by(Person.displayname, Person.name)
         return grant_permissions
 
     @available_with_permission('launchpad.Driver', 'pillar')
-    def getPillarShareeData(self, pillar):
+    def getPillarShareeData(self, pillar, include_indirect=False):
         """See `ISharingService`."""
-        grant_permissions = list(self.getPillarSharees(pillar))
+        grant_permissions = list(
+            self.getPillarSharees(pillar, include_indirect))
         if not grant_permissions:
             return None
         return self.jsonShareeData(grant_permissions)
 
-    def jsonShareeData(self, grant_permissions):
-        """See `ISharingService`."""
-        result = []
-        request = get_current_web_service_request()
-        browser_request = IWebBrowserOriginatingRequest(request)
-        for (grantee, permissions) in grant_permissions:
-            result.append({
+    def _jsonShareeData(self, request, browser_request,
+                        grantee, permissions, via_teams=None):
+        result = {
                 'name': grantee.name,
                 'meta': 'team' if grantee.is_team else 'person',
                 'display_name': grantee.displayname,
@@ -133,7 +135,23 @@
                 'web_link': absoluteURL(grantee, browser_request),
                 'permissions': dict(
                     (policy.type.name, permission.name)
-                    for (policy, permission) in permissions.iteritems())})
+                    for (policy, permission) in permissions.iteritems())}
+        if via_teams:
+            team_info = [{
+                'display_name': team.displayname,
+                'web_link': absoluteURL(team, browser_request),
+            } for team in via_teams]
+            result['via_teams'] = team_info
+        return result
+
+    def jsonShareeData(self, grant_permissions):
+        result = []
+        request = get_current_web_service_request()
+        browser_request = IWebBrowserOriginatingRequest(request)
+        for grantee_info in grant_permissions:
+            result.append(
+                self._jsonShareeData(
+                    request, browser_request, *grantee_info))
         return result
 
     @available_with_permission('launchpad.Edit', 'pillar')

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-03-23 04:00:29 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-03-29 12:42:25 +0000
@@ -54,7 +54,7 @@
         super(TestSharingService, self).setUp()
         self.service = getUtility(IService, 'sharing')
 
-    def _makeShareeData(self, sharee, policy_permissions):
+    def _makeShareeData(self, sharee, policy_permissions, via_teams=None):
         # Unpack a sharee into its attributes and add in permissions.
         request = get_current_web_service_request()
         sharee_data = {
@@ -65,6 +65,12 @@
             'permissions': {}}
         browser_request = IWebBrowserOriginatingRequest(request)
         sharee_data['web_link'] = absoluteURL(sharee, browser_request)
+        if via_teams:
+            team_info = [{
+                'display_name': team.displayname,
+                'web_link': absoluteURL(team, browser_request),
+            } for team in via_teams]
+            sharee_data['via_teams'] = team_info
         permissions = {}
         for (policy, permission) in policy_permissions:
             permissions[policy.name] = unicode(permission.name)
@@ -139,6 +145,24 @@
              (policy2.type, SharingPermission.SOME)])
         self.assertContentEqual([expected_data], sharees)
 
+    def test_jsonShareeDataIndirect(self):
+        # jsonShareeDataIndirect returns the expected data.
+        product = self.factory.makeProduct()
+        [policy1, policy2] = getUtility(IAccessPolicySource).findByPillar(
+            [product])
+        grantee = self.factory.makePerson()
+        via_teams = [self.factory.makeTeam(), self.factory.makeTeam()]
+        sharees = self.service.jsonShareeData(
+            [(grantee, {
+                policy1: SharingPermission.ALL,
+                policy2: SharingPermission.SOME}, via_teams)])
+        expected_data = self._makeShareeData(
+            grantee,
+            [(policy1.type, SharingPermission.ALL),
+             (policy2.type, SharingPermission.SOME)],
+            via_teams)
+        self.assertContentEqual([expected_data], sharees)
+
     def _assert_getPillarShareeData(self, pillar):
         # getPillarShareeData returns the expected data.
         access_policy = self.factory.makeAccessPolicy(
@@ -176,6 +200,52 @@
         login_person(driver)
         self._assert_getPillarShareeData(distro)
 
+    def _assert_getPillarShareeDataIndirect(self, pillar):
+        # getPillarShareeData returns the expected data for indirect sharees.
+        access_policy = self.factory.makeAccessPolicy(
+            pillar=pillar,
+            type=InformationType.PROPRIETARY)
+        indirect_person_grantee = self.factory.makePerson()
+        team_grantee = self.factory.makeTeam(
+            members=[indirect_person_grantee])
+        # Make access policy grant so that 'All' is returned.
+        self.factory.makeAccessPolicyGrant(access_policy, team_grantee)
+        # Make access artifact grants so that 'Some' is returned.
+        artifact_grant = self.factory.makeAccessArtifactGrant()
+        self.factory.makeAccessPolicyArtifact(
+            artifact=artifact_grant.abstract_artifact, policy=access_policy)
+
+        sharees = self.service.getPillarShareeData(
+            pillar, include_indirect=True)
+        expected_sharees = [
+            self._makeShareeData(
+                member,
+                [(InformationType.PROPRIETARY, SharingPermission.ALL)],
+                [team_grantee]) for member in team_grantee.activemembers]
+        expected_sharees.extend([
+            self._makeShareeData(
+                team_grantee,
+                [(InformationType.PROPRIETARY, SharingPermission.ALL)],
+                None),
+            self._makeShareeData(
+                artifact_grant.grantee,
+                [(InformationType.PROPRIETARY, SharingPermission.SOME)])])
+        self.assertContentEqual(expected_sharees, sharees)
+
+    def test_getProductShareeDataIndirect(self):
+        # Users with launchpad.Driver can view indirect sharees.
+        driver = self.factory.makePerson()
+        product = self.factory.makeProduct(driver=driver)
+        login_person(driver)
+        self._assert_getPillarShareeDataIndirect(product)
+
+    def test_getDistroShareeDataIndirect(self):
+        # Users with launchpad.Driver can view indirect sharees.
+        driver = self.factory.makePerson()
+        distro = self.factory.makeDistribution(driver=driver)
+        login_person(driver)
+        self._assert_getPillarShareeDataIndirect(distro)
+
     def _assert_QueryCount(self, func):
         """ getPillarSharees[Data] only should use 3 queries.
 
@@ -298,6 +368,45 @@
         login_person(self.factory.makePerson())
         self._assert_getPillarShareesUnauthorized(product)
 
+    def _assert_getPillarShareesIndirect(self, pillar):
+        # getPillarSharees returns the expected data for indirect sharees.
+        access_policy = self.factory.makeAccessPolicy(
+            pillar=pillar,
+            type=InformationType.PROPRIETARY)
+        indirect_person_grantee = self.factory.makePerson()
+        team_grantee = self.factory.makeTeam(
+            members=[indirect_person_grantee])
+        # Make access policy grant so that 'All' is returned.
+        self.factory.makeAccessPolicyGrant(access_policy, team_grantee)
+        # Make access artifact grants so that 'Some' is returned.
+        artifact_grant = self.factory.makeAccessArtifactGrant()
+        self.factory.makeAccessPolicyArtifact(
+            artifact=artifact_grant.abstract_artifact, policy=access_policy)
+
+        sharees = self.service.getPillarSharees(pillar, include_indirect=True)
+        policy_info = {access_policy: SharingPermission.ALL}
+        expected_sharees = [(member, policy_info, [team_grantee])
+            for member in team_grantee.activemembers]
+        expected_sharees.append((team_grantee, policy_info, None))
+        expected_sharees.append(
+            (artifact_grant.grantee,
+                 {access_policy: SharingPermission.SOME}, None))
+        self.assertContentEqual(expected_sharees, sharees)
+
+    def test_getProductShareesIndirect(self):
+        # Users with launchpad.Driver can view indirect sharees.
+        driver = self.factory.makePerson()
+        product = self.factory.makeProduct(driver=driver)
+        login_person(driver)
+        self._assert_getPillarShareesIndirect(product)
+
+    def test_getDistroShareesIndirect(self):
+        # Users with launchpad.Driver can view indirect sharees.
+        driver = self.factory.makePerson()
+        distro = self.factory.makeDistribution(driver=driver)
+        login_person(driver)
+        self._assert_getPillarShareesIndirect(distro)
+
     def _assert_sharePillarInformation(self, pillar):
         """sharePillarInformations works and returns the expected data."""
         sharee = self.factory.makePerson()

=== modified file 'lib/lp/registry/tests/test_accesspolicy.py'
--- lib/lp/registry/tests/test_accesspolicy.py	2012-03-23 03:40:17 +0000
+++ lib/lp/registry/tests/test_accesspolicy.py	2012-03-29 12:42:25 +0000
@@ -537,6 +537,148 @@
             apgfs.findGranteePermissionsByPolicy(
                 [policy], [grantee_in_result]))
 
+    def test_findIndirectGranteePermissionsByPolicy(self):
+        # findIndirectGranteePermissionsByPolicy() returns anyone with a grant
+        # for any of the policies or the policies' artifacts. The result
+        # includes members of teams with access and a list of teams via which
+        # they have gained access.
+        apgfs = getUtility(IAccessPolicyGrantFlatSource)
+
+        # People with grants on the policy show up.
+        policy_with_no_grantees = self.factory.makeAccessPolicy()
+        policy = self.factory.makeAccessPolicy()
+        indirect_person_grantee = self.factory.makePerson()
+        team_grantee = self.factory.makeTeam(members=[indirect_person_grantee])
+        self.factory.makeAccessPolicyGrant(policy=policy, grantee=team_grantee)
+        policy_info = {policy: SharingPermission.ALL}
+        expected_grantees = [(member, policy_info, [team_grantee])
+            for member in team_grantee.activemembers]
+        expected_grantees.append((team_grantee, policy_info, None))
+        self.assertContentEqual(
+            expected_grantees,
+            apgfs.findIndirectGranteePermissionsByPolicy(
+                [policy, policy_with_no_grantees]))
+
+        # But not people with grants on artifacts.
+        artifact = self.factory.makeAccessArtifact()
+        artifact_grant = self.factory.makeAccessArtifactGrant(
+            artifact=artifact, grantee=team_grantee)
+        other_artifact_grant = self.factory.makeAccessArtifactGrant(
+            artifact=artifact)
+        self.assertContentEqual(
+            expected_grantees,
+            apgfs.findIndirectGranteePermissionsByPolicy(
+                [policy, policy_with_no_grantees]))
+
+        # Unless the artifacts are linked to the policy.
+        another_policy = self.factory.makeAccessPolicy()
+        self.factory.makeAccessPolicyArtifact(
+            artifact=artifact_grant.abstract_artifact, policy=another_policy)
+        policy_info = {
+            policy: SharingPermission.ALL,
+            another_policy: SharingPermission.SOME}
+        expected_grantees = [(member, policy_info, [team_grantee])
+            for member in team_grantee.activemembers]
+        expected_grantees.append((team_grantee, policy_info, None))
+        expected_grantees.append(
+            (other_artifact_grant.grantee,
+            {another_policy: SharingPermission.SOME}, None))
+        self.assertContentEqual(
+            expected_grantees,
+            apgfs.findIndirectGranteePermissionsByPolicy([
+                policy, another_policy, policy_with_no_grantees]))
+
+        # Slicing works by person, not by (person, policy).
+        self.assertContentEqual(
+            [(indirect_person_grantee, {
+                policy: SharingPermission.ALL,
+                another_policy: SharingPermission.SOME}, [team_grantee])],
+            apgfs.findIndirectGranteePermissionsByPolicy([
+                policy, another_policy, policy_with_no_grantees]).order_by(
+                    Person.id)[:1])
+
+    def test_findIndirectGranteePermissionsMultiTeam(self):
+        # Test that findIndirectGranteePermissionsByPolicy() works correctly
+        # when a user is granted access via membership of more than one team.
+        apgfs = getUtility(IAccessPolicyGrantFlatSource)
+
+        policy_with_no_grantees = self.factory.makeAccessPolicy()
+        policy = self.factory.makeAccessPolicy()
+        # Make an indirect grantee belonging to team1 and team2.
+        indirect_person_grantee = self.factory.makePerson()
+        team_grantee1 = self.factory.makeTeam(
+            members=[indirect_person_grantee])
+        team_grantee2 = self.factory.makeTeam(
+            members=[indirect_person_grantee])
+        self.factory.makeAccessPolicyGrant(
+            policy=policy, grantee=team_grantee1)
+        self.factory.makeAccessPolicyGrant(
+            policy=policy, grantee=team_grantee2)
+        policy_info = {policy: SharingPermission.ALL}
+        # Indirect grantee has access.
+        expected_grantees = [
+            (indirect_person_grantee, policy_info,
+             [team_grantee1, team_grantee2])]
+        # All other team members have access.
+        expected_grantees.extend([(member, policy_info, [team_grantee1])
+            for member in team_grantee1.activemembers
+            if member != indirect_person_grantee])
+        expected_grantees.extend([(member, policy_info, [team_grantee2])
+            for member in team_grantee2.activemembers
+            if member != indirect_person_grantee])
+        # The team itself has access also.
+        expected_grantees.append((team_grantee1, policy_info, None))
+        expected_grantees.append((team_grantee2, policy_info, None))
+        self.assertContentEqual(
+            expected_grantees,
+            apgfs.findIndirectGranteePermissionsByPolicy(
+                [policy, policy_with_no_grantees]))
+
+    def test_findIndirectGranteePermissionsMultiTeamPermissions(self):
+        # Test that findIndirectGranteePermissionsByPolicy() works correctly
+        # when a user is granted access via membership of more than one team
+        # where each team has a different level of access. If an indirect
+        # grantee has both ALL and SOME, then ALL is shown.
+        apgfs = getUtility(IAccessPolicyGrantFlatSource)
+
+        policy_with_no_grantees = self.factory.makeAccessPolicy()
+        policy = self.factory.makeAccessPolicy()
+        indirect_person_grantee = self.factory.makePerson()
+
+        # One team has ALL access.
+        team_grantee1 = self.factory.makeTeam(
+            members=[indirect_person_grantee])
+        self.factory.makeAccessPolicyGrant(
+            policy=policy, grantee=team_grantee1)
+
+        # Another team has SOME access.
+        indirect_person_grantee2 = self.factory.makePerson()
+        team_grantee2 = self.factory.makeTeam(
+            members=[indirect_person_grantee, indirect_person_grantee2])
+        artifact = self.factory.makeAccessArtifact()
+        artifact_grant = self.factory.makeAccessArtifactGrant(
+            artifact=artifact, grantee=team_grantee2)
+        self.factory.makeAccessPolicyArtifact(
+            artifact=artifact_grant.abstract_artifact, policy=policy)
+
+        policy_info = {policy: SharingPermission.ALL}
+        expected_grantees = [
+            (indirect_person_grantee, policy_info,
+             [team_grantee1, team_grantee2])]
+        expected_grantees.extend([(member, policy_info, [team_grantee1])
+            for member in team_grantee1.activemembers
+            if member != indirect_person_grantee])
+        expected_grantees.append((team_grantee1, policy_info, None))
+        policy_info = {policy: SharingPermission.SOME}
+        expected_grantees.extend([(member, policy_info, [team_grantee2])
+            for member in team_grantee2.activemembers
+            if member != indirect_person_grantee])
+        expected_grantees.append((team_grantee2, policy_info, None))
+        self.assertContentEqual(
+            expected_grantees,
+            apgfs.findIndirectGranteePermissionsByPolicy(
+                [policy, policy_with_no_grantees]))
+
     def test_findArtifactsByGrantee(self):
         # findArtifactsByGrantee() returns the artifacts for grantee for any of
         # the policies.