← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wallyworld/launchpad/sharing-view-batching2-957663 into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/sharing-view-batching2-957663 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #957663 in Launchpad itself: "+sharing view needs support for batching"
  https://bugs.launchpad.net/launchpad/+bug/957663

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sharing-view-batching2-957663/+merge/98602

== Implementation ==

Add proper batching support using StormRangeFactory to +sharing view.
We want the batch to work on the basis of sharees so the existing service and model methods which provided flattened data were augmented with methods to look up the sharees directly. Then once the sharees are batched, the required flattened data is loaded for the batch. This adds 2 extra queries to the view rendering, both of which are fast. One of those is due to policies for a pillar being loaded twice since the service methods take a pillar parameter and the model methods use access policies. This could be optimised but would require a significant interface change and the query is fast.

The use of StormRangeFactory  means that the batch total record count is inaccurate. The existing batch widget rendering infrastructure did not support hiding the "x-y of total records" message. So I had to add code to allow this inaccurate info to be hidden.

== Tests==

Add new tests for the new service and model methods to these test suites:
-test_accesspolicy
-test_sharingservice

Tests were added to test_pillar_sharing to ensure the batching works as expected.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/templates/batchnavigator-navigation-links.pt
  lib/lp/registry/browser/configure.zcml
  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/javascript/sharing/shareetable.js
  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
  lib/lp/services/webapp/batching.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/sharing-view-batching2-957663/+merge/98602
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/sharing-view-batching2-957663 into lp:launchpad.
=== modified file 'lib/lp/app/templates/batchnavigator-navigation-links.pt'
--- lib/lp/app/templates/batchnavigator-navigation-links.pt	2012-02-06 14:53:31 +0000
+++ lib/lp/app/templates/batchnavigator-navigation-links.pt	2012-03-21 14:09:19 +0000
@@ -6,7 +6,8 @@
     next_page_url context/nextBatchURL;
     last_page_url context/lastBatchURL;
     total context/batch/total;
-    size context/batch/size"
+    size context/batch/size;
+    hide_counts context/hide_counts|nothing"
   style="width: 100%;"
   tal:attributes="class view/css_class"
 >
@@ -14,14 +15,16 @@
     <tr>
       <td style="white-space: nowrap"
           class="batch-navigation-index">
-        <strong tal:content="context/batch/startNumber">1</strong>
-        <tal:block condition="python: size > 1">
-          &rarr;
-          <strong tal:content="context/batch/endNumber">10</strong>
-        </tal:block>
-        of
-        <tal:total replace="total">42</tal:total>
-        <tal:heading content="context/heading">results</tal:heading>
+        <tal:batch_counts condition='not:hide_counts'>
+            <strong tal:content="context/batch/startNumber">1</strong>
+            <tal:block condition="python: size > 1">
+              &rarr;
+              <strong tal:content="context/batch/endNumber">10</strong>
+            </tal:block>
+            of
+            <tal:total replace="total">42</tal:total>
+            <tal:heading content="context/heading">results</tal:heading>
+        </tal:batch_counts>
       </td>
       <td style="text-align: right; white-space: nowrap"
           class="batch-navigation-links">

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2012-03-19 14:03:51 +0000
+++ lib/lp/registry/browser/configure.zcml	2012-03-21 14:09:19 +0000
@@ -44,7 +44,7 @@
 
 <facet facet="overview">
     <browser:page
-        for="lp.services.webapp.interfaces.ITableBatchNavigator"
+        for="lp.services.webapp.interfaces.IBatchNavigator"
         name="+sharee-table-view"
         template="../templates/pillar-sharing-table.pt"
         permission="zope.Public" />

=== modified file 'lib/lp/registry/browser/pillar.py'
--- lib/lp/registry/browser/pillar.py	2012-03-20 06:11:07 +0000
+++ lib/lp/registry/browser/pillar.py	2012-03-21 14:09:19 +0000
@@ -50,7 +50,10 @@
 from lp.services.propertycache import cachedproperty
 from lp.services.features import getFeatureFlag
 from lp.services.webapp.authorization import check_permission
-from lp.services.webapp.batching import TableBatchNavigator
+from lp.services.webapp.batching import (
+    BatchNavigator,
+    StormRangeFactory,
+    )
 from lp.services.webapp.menu import (
     ApplicationMenu,
     enabled_with_permission,
@@ -269,9 +272,11 @@
 
     def _getBatchNavigator(self, sharees):
         """Return the batch navigator to be used to batch the sharees."""
-        return TableBatchNavigator(
+        return BatchNavigator(
             sharees, self.request,
-            size=config.launchpad.default_batch_size)
+            hide_counts=True,
+            size=config.launchpad.default_batch_size,
+            range_factory=StormRangeFactory(sharees))
 
     def shareeData(self):
         """Return an `ITableBatchNavigator` for sharees."""
@@ -305,7 +310,9 @@
             raise AssertionError("Ambiguous view name.")
         cache.objects['view_name'] = view_names.pop()
         batch_navigator = self.shareeData()
-        cache.objects['sharee_data'] = list(batch_navigator.batch)
+        cache.objects['sharee_data'] = (
+            self._getSharingService().getPillarShareeData(
+                self.context, batch_navigator.batch))
 
         def _getBatchInfo(batch):
             if batch is None:

=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-03-16 07:07:19 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py	2012-03-21 14:09:19 +0000
@@ -9,17 +9,28 @@
 
 from BeautifulSoup import BeautifulSoup
 from lazr.restful.interfaces import IJSONRequestCache
+from testtools.matchers import (
+    LessThan,
+    MatchesException,
+    Not,
+    Raises,
+    )
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
 
 from lp.app.interfaces.services import IService
+from lp.registry.enums import InformationType
+from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
+from lp.services.webapp.interfaces import StormRangeFactoryError
 from lp.services.webapp.publisher import canonical_url
 from lp.testing import (
     login_person,
+    StormStatementRecorder,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import setupBrowserForUser
 from lp.testing.views import (
     create_initialized_view,
@@ -36,6 +47,28 @@
 
     layer = DatabaseFunctionalLayer
 
+    def createSharees(self):
+        login_person(self.owner)
+        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(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=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_without_feature_flag(self):
         # We need a feature flag to enable the view.
         self.assertRaises(
@@ -85,8 +118,43 @@
             self.assertIsNotNone(cache.objects.get('information_types'))
             self.assertIsNotNone(cache.objects.get('sharing_permissions'))
             aps = getUtility(IService, 'sharing')
-            observers = aps.getPillarSharees(self.pillar)
-            self.assertEqual(observers, cache.objects.get('sharee_data'))
+            batch_size = config.launchpad.default_batch_size
+            observers = aps.getPillarShareeData(
+                self.pillar, grantees=self.grantees[:batch_size])
+            self.assertContentEqual(
+                observers, 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.shareeData().batch.nextBatch()
+            self.assertContentEqual(
+                next_batch.range_memo, cache.objects.get('next')['memo'])
+
+    def test_view_range_factory(self):
+        # Test the view range factory is properly configured.
+        with FeatureFixture(ENABLED_FLAG):
+            view = create_initialized_view(self.pillar, name='+sharing')
+            range_factory = view.shareeData().batch.range_factory
+
+            def test_range_factory():
+                row = range_factory.resultset.get_plain_result_set()[0]
+                range_factory.getOrderValuesFor(row)
+
+            self.assertThat(
+                test_range_factory,
+                Not(Raises(MatchesException(StormRangeFactoryError))))
+
+    def test_view_query_count(self):
+        # Test the query count is within expected limit.
+        with FeatureFixture(ENABLED_FLAG):
+            view = create_view(self.pillar, name='+sharing')
+            with StormStatementRecorder() as recorder:
+                view.initialize()
+            self.assertThat(recorder, HasQueryCount(LessThan(7)))
 
     def test_view_write_enabled_without_feature_flag(self):
         # Test that sharing_write_enabled is not set without the feature flag.
@@ -118,6 +186,7 @@
         self.owner = self.factory.makePerson()
         self.pillar = self.factory.makeProduct(
             owner=self.owner, driver=self.driver)
+        self.createSharees()
         login_person(self.driver)
 
 
@@ -131,4 +200,5 @@
         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-15 08:54:40 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py	2012-03-21 14:09:19 +0000
@@ -214,7 +214,16 @@
 class IAccessPolicyGrantFlatSource(Interface):
     """Experimental query utility to search through the flattened schema."""
 
-    def findGranteesByPolicy(policies, grantees=None):
+    def findGranteesByPolicy(policies):
+        """Find teams or users with access grants for the policies.
+
+        This includes grants for artifacts in the policies.
+
+        :param policies: a collection of `IAccesPolicy`s.
+        :return: a collection of `IPerson`.
+        """
+
+    def findGranteePermissionsByPolicy(policies, grantees=None):
         """Find teams or users with access grants for the policies.
 
         This includes grants for artifacts in the policies.

=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py	2012-03-13 03:06:14 +0000
+++ lib/lp/registry/interfaces/sharingservice.py	2012-03-21 14:09:19 +0000
@@ -56,6 +56,15 @@
     def getPillarSharees(pillar):
         """Return people/teams who can see pillar artifacts."""
 
+    @export_read_operation()
+    @operation_parameters(
+        pillar=Reference(IPillar, title=_('Pillar'), required=True))
+    @operation_for_version('devel')
+    def getPillarShareeData(pillar, grantees=None):
+        """Return people/teams who can see pillar artifacts and what
+        permissions they have for each information type.
+        """
+
     @export_write_operation()
     @call_with(user=REQUEST_USER)
     @operation_parameters(

=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js	2012-03-20 21:00:26 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js	2012-03-21 14:09:19 +0000
@@ -106,7 +106,8 @@
             current_url: window.location,
             cache: cache,
             target: Y.one('#sharee-table'),
-            navigation_indices: navigation_indices
+            navigation_indices: navigation_indices,
+            batch_info_template: '<div></div>'
         });
         navigator.set('backwards_navigation', Y.all('.first,.previous'));
         navigator.set('forwards_navigation', Y.all('.last,.next'));

=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py	2012-03-16 05:34:47 +0000
+++ lib/lp/registry/model/accesspolicy.py	2012-03-21 14:09:19 +0000
@@ -340,7 +340,14 @@
     grantee = Reference(grantee_id, 'Person.id')
 
     @classmethod
-    def findGranteesByPolicy(cls, policies, grantees=None):
+    def findGranteesByPolicy(cls, policies):
+        """See `IAccessPolicyGrantFlatSource`."""
+        ids = [policy.id for policy in policies]
+        return IStore(cls).find(
+            Person, Person.id == cls.grantee_id, cls.policy_id.is_in(ids))
+
+    @classmethod
+    def findGranteePermissionsByPolicy(cls, policies, grantees=None):
         """See `IAccessPolicyGrantFlatSource`."""
         ids = [policy.id for policy in policies]
         sharing_permission_term = SQL("""

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-03-21 02:24:01 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-03-21 14:09:19 +0000
@@ -29,6 +29,7 @@
 from lp.registry.interfaces.sharingservice import ISharingService
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.registry.model.person import Person
 from lp.services.features import getFeatureFlag
 from lp.services.webapp.authorization import available_with_permission
 
@@ -94,13 +95,21 @@
         return sharing_permissions
 
     @available_with_permission('launchpad.Driver', 'pillar')
-    def getPillarSharees(self, pillar, grantees=None):
-        """See `ISharingService`."""
-        policies = getUtility(IAccessPolicySource).findByPillar([pillar])
-        ap_grant_flat = getUtility(IAccessPolicyGrantFlatSource)
-        grant_permissions = ap_grant_flat.findGranteesByPolicy(
-            policies, grantees).order_by(
-                "person_sort_key(Person.displayname, Person.name)")
+    def getPillarSharees(self, pillar):
+        """See `ISharingService`."""
+        policies = getUtility(IAccessPolicySource).findByPillar([pillar])
+        ap_grant_flat = getUtility(IAccessPolicyGrantFlatSource)
+        grantees = ap_grant_flat.findGranteesByPolicy(
+            policies).order_by(Person.displayname)
+        return grantees
+
+    @available_with_permission('launchpad.Driver', 'pillar')
+    def getPillarShareeData(self, pillar, grantees=None):
+        """See `ISharingService`."""
+        policies = getUtility(IAccessPolicySource).findByPillar([pillar])
+        ap_grant_flat = getUtility(IAccessPolicyGrantFlatSource)
+        grant_permissions = ap_grant_flat.findGranteePermissionsByPolicy(
+            policies, grantees)
 
         result = []
         person_by_id = {}
@@ -183,7 +192,7 @@
             self.deletePillarSharee(pillar, sharee, info_types_for_nothing)
 
         # Return sharee data to the caller.
-        sharees = self.getPillarSharees(pillar, [sharee])
+        sharees = self.getPillarShareeData(pillar, [sharee])
         if not sharees:
             return None
         return sharees[0]

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-03-21 02:24:01 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-03-21 14:09:19 +0000
@@ -123,8 +123,8 @@
             distro,
             [InformationType.EMBARGOEDSECURITY, InformationType.USERDATA])
 
-    def _test_getPillarSharees(self, pillar):
-        # getPillarSharees returns the expected data.
+    def _test_getPillarShareeData(self, pillar):
+        # getPillarShareeData returns the expected data.
         access_policy = self.factory.makeAccessPolicy(
             pillar=pillar,
             type=InformationType.PROPRIETARY)
@@ -136,7 +136,7 @@
         self.factory.makeAccessPolicyArtifact(
             artifact=artifact_grant.abstract_artifact, policy=access_policy)
 
-        sharees = self.service.getPillarSharees(pillar)
+        sharees = self.service.getPillarShareeData(pillar)
         expected_sharees = [
             self._makeShareeData(
                 grantee,
@@ -146,22 +146,22 @@
                 [(InformationType.PROPRIETARY, SharingPermission.SOME)])]
         self.assertContentEqual(expected_sharees, sharees)
 
-    def test_getProductSharees(self):
+    def test_getProductShareeData(self):
         # Users with launchpad.Driver can view sharees.
         driver = self.factory.makePerson()
         product = self.factory.makeProduct(driver=driver)
         login_person(driver)
-        self._test_getPillarSharees(product)
+        self._test_getPillarShareeData(product)
 
-    def test_getDistroSharees(self):
+    def test_getDistroShareeData(self):
         # Users with launchpad.Driver can view sharees.
         driver = self.factory.makePerson()
         distro = self.factory.makeDistribution(driver=driver)
         login_person(driver)
-        self._test_getPillarSharees(distro)
+        self._test_getPillarShareeData(distro)
 
-    def test_getPillarShareesQueryCount(self):
-        # getPillarSharees only should use 2 queries regardless of how many
+    def test_getPillarShareeDataQueryCount(self):
+        # getPillarShareeData only should use 2 queries regardless of how many
         # sharees are returned.
         driver = self.factory.makePerson()
         product = self.factory.makeProduct(driver=driver)
@@ -184,19 +184,19 @@
         for x in range(5):
             makeGrants()
         with StormStatementRecorder() as recorder:
-            sharees = self.service.getPillarSharees(product)
+            sharees = self.service.getPillarShareeData(product)
         self.assertEqual(10, len(sharees))
         self.assertThat(recorder, HasQueryCount(Equals(2)))
         # Make some more grants and check again.
         for x in range(5):
             makeGrants()
         with StormStatementRecorder() as recorder:
-            sharees = self.service.getPillarSharees(product)
+            sharees = self.service.getPillarShareeData(product)
         self.assertEqual(20, len(sharees))
         self.assertThat(recorder, HasQueryCount(Equals(2)))
 
-    def test_getPillarSharees_filter_grantees(self):
-        # getPillarSharees only returns grantees in the specified list.
+    def test_getPillarShareeData_filter_grantees(self):
+        # getPillarShareeData only returns grantees in the specified list.
         driver = self.factory.makePerson()
         pillar = self.factory.makeProduct(driver=driver)
         login_person(driver)
@@ -209,13 +209,65 @@
         self.factory.makeAccessPolicyGrant(
             access_policy, grantee_not_in_result)
 
-        sharees = self.service.getPillarSharees(pillar, [grantee_in_result])
+        sharees = self.service.getPillarShareeData(pillar, [grantee_in_result])
         expected_sharees = [
             self._makeShareeData(
                 grantee_in_result,
                 [(InformationType.PROPRIETARY, SharingPermission.ALL)])]
         self.assertContentEqual(expected_sharees, sharees)
 
+    def _test_getPillarShareeDataUnauthorized(self, pillar):
+        # getPillarShareeData raises an Unauthorized exception if the user is
+        # not permitted to do so.
+        access_policy = self.factory.makeAccessPolicy(pillar=pillar)
+        grantee = self.factory.makePerson()
+        self.factory.makeAccessPolicyGrant(access_policy, grantee)
+        self.assertRaises(
+            Unauthorized, self.service.getPillarShareeData, pillar)
+
+    def test_getPillarShareeDataAnonymous(self):
+        # Anonymous users are not allowed.
+        product = self.factory.makeProduct()
+        login(ANONYMOUS)
+        self._test_getPillarShareeDataUnauthorized(product)
+
+    def test_getPillarShareeDataAnyone(self):
+        # Unauthorized users are not allowed.
+        product = self.factory.makeProduct()
+        login_person(self.factory.makePerson())
+        self._test_getPillarShareeDataUnauthorized(product)
+
+    def _test_getPillarSharees(self, pillar):
+        # getPillarSharees returns the expected data.
+        access_policy = self.factory.makeAccessPolicy(
+            pillar=pillar,
+            type=InformationType.PROPRIETARY)
+        grantee = self.factory.makePerson()
+        # Make access policy grant so that 'All' is returned.
+        self.factory.makeAccessPolicyGrant(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=access_policy)
+
+        sharees = self.service.getPillarSharees(pillar)
+        expected_sharees = [grantee, artifact_grant.grantee]
+        self.assertContentEqual(expected_sharees, sharees)
+
+    def test_getProductSharees(self):
+        # Users with launchpad.Driver can view sharees.
+        driver = self.factory.makePerson()
+        product = self.factory.makeProduct(driver=driver)
+        login_person(driver)
+        self._test_getPillarSharees(product)
+
+    def test_getDistroSharees(self):
+        # Users with launchpad.Driver can view sharees.
+        driver = self.factory.makePerson()
+        distro = self.factory.makeDistribution(driver=driver)
+        login_person(driver)
+        self._test_getPillarSharees(distro)
+
     def _test_getPillarShareesUnauthorized(self, pillar):
         # getPillarSharees raises an Unauthorized exception if the user is
         # not permitted to do so.
@@ -295,8 +347,8 @@
         expected_sharee_data = self._makeShareeData(
             sharee, expected_permissions)
         self.assertEqual(expected_sharee_data, sharee_data)
-        # Check that getPillarSharees returns what we expect.
-        [sharee_data] = self.service.getPillarSharees(pillar)
+        # Check that getPillarShareeData returns what we expect.
+        [sharee_data] = self.service.getPillarShareeData(pillar)
         self.assertEqual(expected_sharee_data, sharee_data)
 
     def test_updateProjectGroupSharee_not_allowed(self):
@@ -414,7 +466,7 @@
             another, [(information_types[0], SharingPermission.ALL)])
         expected_data.append(another_person_data)
         self.assertContentEqual(
-            expected_data, self.service.getPillarSharees(pillar))
+            expected_data, self.service.getPillarShareeData(pillar))
 
     def test_deleteProductShareeAll(self):
         # Users with launchpad.Edit can delete all access for a sharee.
@@ -491,9 +543,9 @@
         self.grantor_uri = canonical_url(self.grantor, force_local_path=True)
         transaction.commit()
 
-    def test_getPillarSharees(self):
-        # Test the getPillarSharees method.
-        [json_data] = self._getPillarSharees()
+    def test_getPillarShareeData(self):
+        # Test the getPillarShareeData method.
+        [json_data] = self._getPillarShareeData()
         self.assertEqual('grantee', json_data['name'])
         self.assertEqual(
             {InformationType.USERDATA.name: SharingPermission.ALL.name},
@@ -527,10 +579,10 @@
             '/+services/sharing',
             api_method, api_version='devel', **kwargs).jsonBody()
 
-    def _getPillarSharees(self):
+    def _getPillarShareeData(self):
         pillar_uri = canonical_url(self.pillar, force_local_path=True)
         return self._named_get(
-            'getPillarSharees', pillar=pillar_uri)
+            'getPillarShareeData', pillar=pillar_uri)
 
     def _sharePillarInformation(self):
         pillar_uri = canonical_url(self.pillar, force_local_path=True)
@@ -562,9 +614,9 @@
         transaction.commit()
         self._sharePillarInformation()
 
-    def _getPillarSharees(self):
+    def _getPillarShareeData(self):
         ws_pillar = ws_object(self.launchpad, self.pillar)
-        return self.service.getPillarSharees(pillar=ws_pillar)
+        return self.service.getPillarShareeData(pillar=ws_pillar)
 
     def _sharePillarInformation(self):
         ws_pillar = ws_object(self.launchpad, self.pillar)

=== modified file 'lib/lp/registry/tests/test_accesspolicy.py'
--- lib/lp/registry/tests/test_accesspolicy.py	2012-03-16 06:15:23 +0000
+++ lib/lp/registry/tests/test_accesspolicy.py	2012-03-21 14:09:19 +0000
@@ -449,14 +449,44 @@
         policy = self.factory.makeAccessPolicy()
         policy_grant = self.factory.makeAccessPolicyGrant(policy=policy)
         self.assertContentEqual(
-            [(policy_grant.grantee, policy, 'ALL')],
-            apgfs.findGranteesByPolicy([policy, policy_with_no_grantees]))
-
-        # But not people with grants on artifacts.
-        artifact_grant = self.factory.makeAccessArtifactGrant()
-        self.assertContentEqual(
-            [(policy_grant.grantee, policy, 'ALL')],
-            apgfs.findGranteesByPolicy([policy, policy_with_no_grantees]))
+            [policy_grant.grantee],
+            apgfs.findGranteesByPolicy([policy, policy_with_no_grantees]))
+
+        # But not people with grants on artifacts.
+        artifact_grant = self.factory.makeAccessArtifactGrant()
+        self.assertContentEqual(
+            [policy_grant.grantee],
+            apgfs.findGranteesByPolicy([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)
+        self.assertContentEqual(
+            [policy_grant.grantee, artifact_grant.grantee],
+            apgfs.findGranteesByPolicy([
+                policy, another_policy, policy_with_no_grantees]))
+
+    def findGranteePermissionsByPolicy(self):
+        # findGranteePermissionsByPolicy() returns anyone with a grant for any
+        # of the policies or the policies' artifacts.
+        apgfs = getUtility(IAccessPolicyGrantFlatSource)
+
+        # People with grants on the policy show up.
+        policy_with_no_grantees = self.factory.makeAccessPolicy()
+        policy = self.factory.makeAccessPolicy()
+        policy_grant = self.factory.makeAccessPolicyGrant(policy=policy)
+        self.assertContentEqual(
+            [(policy_grant.grantee, policy, 'ALL')],
+            apgfs.findGranteePermissionsByPolicy(
+                [policy, policy_with_no_grantees]))
+
+        # But not people with grants on artifacts.
+        artifact_grant = self.factory.makeAccessArtifactGrant()
+        self.assertContentEqual(
+            [(policy_grant.grantee, policy, 'ALL')],
+            apgfs.findGranteePermissionsByPolicy(
+                [policy, policy_with_no_grantees]))
 
         # Unless the artifacts are linked to the policy.
         another_policy = self.factory.makeAccessPolicy()
@@ -465,13 +495,13 @@
         self.assertContentEqual(
             [(policy_grant.grantee, policy, 'ALL'),
             (artifact_grant.grantee, another_policy, 'SOME')],
-            apgfs.findGranteesByPolicy([
+            apgfs.findGranteePermissionsByPolicy([
                 policy, another_policy, policy_with_no_grantees]))
 
-    def test_findGranteesByPolicy_filter_grantees(self):
-        # findGranteesByPolicy() returns anyone with a grant for any of
-        # the policies or the policies' artifacts so long as the grantee is in
-        # the specified list of grantees.
+    def test_findGranteePermissionsByPolicy_filter_grantees(self):
+        # findGranteePermissionsByPolicy() returns anyone with a grant for any
+        # of the policies or the policies' artifacts so long as the grantee is
+        # in the specified list of grantees.
         apgfs = getUtility(IAccessPolicyGrantFlatSource)
 
         # People with grants on the policy show up.
@@ -484,7 +514,8 @@
             policy=policy, grantee=grantee_not_in_result)
         self.assertContentEqual(
             [(policy_grant.grantee, policy, 'ALL')],
-            apgfs.findGranteesByPolicy([policy], [grantee_in_result]))
+            apgfs.findGranteePermissionsByPolicy(
+                [policy], [grantee_in_result]))
 
     def test_findArtifactsByGrantee(self):
         # findArtifactsByGrantee() returns the artifacts for grantee for any of

=== modified file 'lib/lp/services/webapp/batching.py'
--- lib/lp/services/webapp/batching.py	2011-12-30 06:14:56 +0000
+++ lib/lp/services/webapp/batching.py	2012-03-21 14:09:19 +0000
@@ -107,6 +107,15 @@
 
 class BatchNavigator(lazr.batchnavigator.BatchNavigator):
 
+    def __init__(self, results, request, start=0, size=20, callback=None,
+                 transient_parameters=None, force_start=False,
+                 range_factory=None, hide_counts=False):
+        super(BatchNavigator, self).__init__(results, request,
+            start=start, size=size, callback=callback,
+            transient_parameters=transient_parameters,
+            force_start=force_start, range_factory=range_factory)
+        self.hide_counts = hide_counts
+
     @property
     def default_batch_size(self):
         return config.launchpad.default_batch_size