launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/preload-subscriptions-redux into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1148353 in Launchpad itself: "Person:+archivesubscriptions causes O(n) queries per subscription"
  https://bugs.launchpad.net/launchpad/+bug/1148353

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/preload-subscriptions-redux/+merge/152302

It turns out my earlier fix for this work only helped for archivesubscriptions that are held personally, and not for those which are granted to teams. I have solved this by precaching the permissions for the subscriptions, and changing the test to create 5 team subscriptions.

lp.services.webapp.authorization has grown an __all__, since it did not have one.
-- 
https://code.launchpad.net/~stevenk/launchpad/preload-subscriptions-redux/+merge/152302
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/preload-subscriptions-redux into lp:launchpad.

=== modified file 'lib/lp/services/webapp/authorization.py'
--- lib/lp/services/webapp/authorization.py	2012-08-09 03:50:49 +0000
+++ lib/lp/services/webapp/authorization.py	2013-03-07 22:55:36 +0000
@@ -3,6 +3,17 @@
 
 __metaclass__ = type
 
+__all__ = [
+    'available_with_permission',
+    'check_permission',
+    'clear_cache',
+    'iter_authorization',
+    'LaunchpadPermissiveSecurityPolicy',
+    'LaunchpadSecurityPolicy',
+    'LAUNCHPAD_SECURITY_POLICY_CACHE_KEY',
+    'precache_permission_for_objects',
+    ]
+
 from collections import (
     deque,
     Iterable,

=== modified file 'lib/lp/soyuz/browser/archivesubscription.py'
--- lib/lp/soyuz/browser/archivesubscription.py	2013-03-07 01:09:59 +0000
+++ lib/lp/soyuz/browser/archivesubscription.py	2013-03-07 22:55:36 +0000
@@ -49,6 +49,7 @@
     cachedproperty,
     get_property_cache,
     )
+from lp.services.webapp.authorization import precache_permission_for_objects
 from lp.services.webapp.batching import (
     BatchNavigator,
     StormRangeFactory,
@@ -329,8 +330,9 @@
         subs_with_tokens = subscriber_set.getBySubscriberWithActiveToken(
             self.context)
 
-        archives = load_related(
-            Archive, map(itemgetter(0), subs_with_tokens), ['archive_id'])
+        subscriptions = map(itemgetter(0), subs_with_tokens)
+        precache_permission_for_objects(None, 'launchpad.View', subscriptions)
+        archives = load_related(Archive, subscriptions, ['archive_id'])
         list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
             [archive.ownerID for archive in archives], need_validity=True))
         for archive in archives:

=== modified file 'lib/lp/soyuz/tests/test_archive_subscriptions.py'
--- lib/lp/soyuz/tests/test_archive_subscriptions.py	2013-03-07 01:09:59 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py	2013-03-07 22:55:36 +0000
@@ -181,7 +181,11 @@
         for x in range(10):
             archive = self.factory.makeArchive(private=True)
             with person_logged_in(archive.owner):
-                archive.newSubscription(subscriber, archive.owner)
+                if x >= 5:
+                    team = self.factory.makeTeam(members=[subscriber])
+                    archive.newSubscription(team, archive.owner)
+                else:
+                    archive.newSubscription(subscriber, archive.owner)
         Store.of(subscriber).flush()
         Store.of(subscriber).invalidate()
         with person_logged_in(subscriber):