Merge lp:~stevenk/launchpad/preload-subscriptions into lp:launchpad

Proposed by Steve Kowalik
Status: Merged
Approved by: William Grant
Approved revision: no longer in the source branch.
Merged at revision: 16521
Proposed branch: lp:~stevenk/launchpad/preload-subscriptions
Merge into: lp:launchpad
Diff against target: 298 lines (+66/-43)
6 files modified
lib/lp/app/browser/tales.py (+14/-33)
lib/lp/security.py (+2/-0)
lib/lp/soyuz/browser/archivesubscription.py (+19/-7)
lib/lp/soyuz/interfaces/archivesubscriber.py (+2/-1)
lib/lp/soyuz/model/archive.py (+4/-0)
lib/lp/soyuz/tests/test_archive_subscriptions.py (+25/-2)
To merge this branch: bzr merge lp:~stevenk/launchpad/preload-subscriptions
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+152073@code.launchpad.net

Commit message

Preload archives and their owners, and pre-cache permissions for Person:+archivesubscriptions.

Description of the change

Preload archives and their owners, and pre-cache permissions for Person:+archivesubscriptions.

It turns out the bulk of the queries were trying to work out if the PPA references could be shown. Since subscribers are able to see Archive:+index I dropped the permission required to SubscriberView, and made the code make use of check_permission.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py 2013-02-18 05:42:23 +0000
+++ lib/lp/app/browser/tales.py 2013-03-07 01:49:22 +0000
@@ -5,7 +5,7 @@
55
6__metaclass__ = type6__metaclass__ = type
77
8import bisect8from bisect import bisect
9from datetime import (9from datetime import (
10 datetime,10 datetime,
11 timedelta,11 timedelta,
@@ -70,10 +70,6 @@
70from lp.registry.interfaces.product import IProduct70from lp.registry.interfaces.product import IProduct
71from lp.registry.interfaces.projectgroup import IProjectGroup71from lp.registry.interfaces.projectgroup import IProjectGroup
72from lp.services.utils import total_seconds72from lp.services.utils import total_seconds
73from lp.services.webapp import (
74 canonical_url,
75 urlappend,
76 )
77from lp.services.webapp.authorization import check_permission73from lp.services.webapp.authorization import check_permission
78from lp.services.webapp.canonicalurl import nearest_adapter74from lp.services.webapp.canonicalurl import nearest_adapter
79from lp.services.webapp.error import SystemErrorView75from lp.services.webapp.error import SystemErrorView
@@ -95,13 +91,14 @@
95 get_facet,91 get_facet,
96 )92 )
97from lp.services.webapp.publisher import (93from lp.services.webapp.publisher import (
94 canonical_url,
98 LaunchpadView,95 LaunchpadView,
99 nearest,96 nearest,
100 )97 )
101from lp.services.webapp.session import get_cookie_domain98from lp.services.webapp.session import get_cookie_domain
99from lp.services.webapp.url import urlappend
102from lp.soyuz.enums import ArchivePurpose100from lp.soyuz.enums import ArchivePurpose
103from lp.soyuz.interfaces.archive import IPPA101from lp.soyuz.interfaces.archive import IPPA
104from lp.soyuz.interfaces.archivesubscriber import IArchiveSubscriberSet
105from lp.soyuz.interfaces.binarypackagename import IBinaryAndSourcePackageName102from lp.soyuz.interfaces.binarypackagename import IBinaryAndSourcePackageName
106103
107104
@@ -1863,19 +1860,16 @@
18631860
1864 _link_summary_template = '%(display_name)s'1861 _link_summary_template = '%(display_name)s'
1865 _link_permission = 'launchpad.View'1862 _link_permission = 'launchpad.View'
1863 _reference_permission = 'launchpad.SubscriberView'
1866 _reference_template = "ppa:%(owner_name)s/%(ppa_name)s"1864 _reference_template = "ppa:%(owner_name)s/%(ppa_name)s"
18671865
1868 final_traversable_names = {1866 final_traversable_names = {'reference': 'reference'}
1869 'reference': 'reference',
1870 }
1871 final_traversable_names.update(1867 final_traversable_names.update(
1872 CustomizableFormatter.final_traversable_names)1868 CustomizableFormatter.final_traversable_names)
18731869
1874 def _link_summary_values(self):1870 def _link_summary_values(self):
1875 """See CustomizableFormatter._link_summary_values."""1871 """See CustomizableFormatter._link_summary_values."""
1876 return {1872 return {'display_name': self._context.displayname}
1877 'display_name': self._context.displayname,
1878 }
18791873
1880 def link(self, view_name):1874 def link(self, view_name):
1881 """Return html including a link for the context PPA.1875 """Return html including a link for the context PPA.
@@ -1893,29 +1887,17 @@
1893 if check_permission(self._link_permission, self._context):1887 if check_permission(self._link_permission, self._context):
1894 url = self.url(view_name)1888 url = self.url(view_name)
1895 return '<a href="%s" class="%s">%s</a>' % (url, css, summary)1889 return '<a href="%s" class="%s">%s</a>' % (url, css, summary)
1896 else:1890 elif check_permission(self._reference_permission, self._context):
1897 if not self._context.private:1891 return '<span class="%s">%s</span>' % (css, summary)
1898 return '<span class="%s">%s</span>' % (css, summary)1892 return ''
1899 else:
1900 return ''
19011893
1902 def reference(self, view_name=None, rootsite=None):1894 def reference(self, view_name=None, rootsite=None):
1903 """Return the text PPA reference for a PPA."""1895 """Return the text PPA reference for a PPA."""
1904 # XXX: noodles 2010-02-11 bug=336779: This following check1896 if not check_permission(self._reference_permission, self._context):
1905 # should be replaced with the normal check_permission once1897 return ''
1906 # permissions for archive subscribers has been resolved.
1907 if self._context.private:
1908 request = get_current_browser_request()
1909 person = IPerson(request.principal)
1910 subscriptions = getUtility(IArchiveSubscriberSet).getBySubscriber(
1911 person, self._context)
1912 if subscriptions.is_empty():
1913 return ''
1914
1915 return self._reference_template % {1898 return self._reference_template % {
1916 'owner_name': self._context.owner.name,1899 'owner_name': self._context.owner.name,
1917 'ppa_name': self._context.name,1900 'ppa_name': self._context.name}
1918 }
19191901
19201902
1921class SpecificationBranchFormatterAPI(CustomizableFormatter):1903class SpecificationBranchFormatterAPI(CustomizableFormatter):
@@ -2353,7 +2335,7 @@
2353 if seconds < second_boundaries[-1]:2335 if seconds < second_boundaries[-1]:
2354 # Use the built-in bisection algorithm to locate the index2336 # Use the built-in bisection algorithm to locate the index
2355 # of the item which "seconds" sorts after.2337 # of the item which "seconds" sorts after.
2356 matching_element_index = bisect.bisect(second_boundaries, seconds)2338 matching_element_index = bisect(second_boundaries, seconds)
23572339
2358 # Return the corresponding display value.2340 # Return the corresponding display value.
2359 return display_values[matching_element_index]2341 return display_values[matching_element_index]
@@ -2510,8 +2492,7 @@
25102492
2511 def __init__(self, context):2493 def __init__(self, context):
2512 here = os.path.dirname(os.path.realpath(__file__))2494 here = os.path.dirname(os.path.realpath(__file__))
2513 self.path = os.path.join(2495 self.path = os.path.join(here, '../templates/base-layout.pt')
2514 here, '../templates/base-layout.pt')
25152496
25162497
2517class PageMacroDispatcher:2498class PageMacroDispatcher:
25182499
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2013-02-28 01:02:50 +0000
+++ lib/lp/security.py 2013-03-07 01:49:22 +0000
@@ -2487,6 +2487,8 @@
2487 usedfor = IArchive2487 usedfor = IArchive
24882488
2489 def checkAuthenticated(self, user):2489 def checkAuthenticated(self, user):
2490 if user.person in self.obj._known_subscribers:
2491 return True
2490 if super(SubscriberViewArchive, self).checkAuthenticated(user):2492 if super(SubscriberViewArchive, self).checkAuthenticated(user):
2491 return True2493 return True
2492 filter = get_enabled_archive_filter(2494 filter = get_enabled_archive_filter(
24932495
=== modified file 'lib/lp/soyuz/browser/archivesubscription.py'
--- lib/lp/soyuz/browser/archivesubscription.py 2012-12-12 05:27:55 +0000
+++ lib/lp/soyuz/browser/archivesubscription.py 2013-03-07 01:49:22 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the1# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Browser views related to archive subscriptions."""4"""Browser views related to archive subscriptions."""
@@ -14,7 +14,10 @@
14 ]14 ]
1515
16import datetime16import datetime
17from operator import attrgetter17from operator import (
18 attrgetter,
19 itemgetter,
20 )
1821
19import pytz22import pytz
20from zope.app.form import CustomWidgetFactory23from zope.app.form import CustomWidgetFactory
@@ -40,8 +43,12 @@
40from lp.app.widgets.date import DateWidget43from lp.app.widgets.date import DateWidget
41from lp.app.widgets.popup import PersonPickerWidget44from lp.app.widgets.popup import PersonPickerWidget
42from lp.registry.interfaces.person import IPersonSet45from lp.registry.interfaces.person import IPersonSet
46from lp.services.database.bulk import load_related
43from lp.services.fields import PersonChoice47from lp.services.fields import PersonChoice
44from lp.services.propertycache import cachedproperty48from lp.services.propertycache import (
49 cachedproperty,
50 get_property_cache,
51 )
45from lp.services.webapp.batching import (52from lp.services.webapp.batching import (
46 BatchNavigator,53 BatchNavigator,
47 StormRangeFactory,54 StormRangeFactory,
@@ -56,6 +63,7 @@
56 IArchiveSubscriberSet,63 IArchiveSubscriberSet,
57 IPersonalArchiveSubscription,64 IPersonalArchiveSubscription,
58 )65 )
66from lp.soyuz.model.archive import Archive
5967
6068
61def archive_subscription_ui_adapter(archive_subscription):69def archive_subscription_ui_adapter(archive_subscription):
@@ -321,6 +329,13 @@
321 subs_with_tokens = subscriber_set.getBySubscriberWithActiveToken(329 subs_with_tokens = subscriber_set.getBySubscriberWithActiveToken(
322 self.context)330 self.context)
323331
332 archives = load_related(
333 Archive, map(itemgetter(0), subs_with_tokens), ['archive_id'])
334 list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
335 [archive.ownerID for archive in archives], need_validity=True))
336 for archive in archives:
337 get_property_cache(archive)._known_subscribers = [self.user]
338
324 # Turn the result set into a list of dicts so it can be easily339 # Turn the result set into a list of dicts so it can be easily
325 # accessed in TAL. Note that we need to ensure that only one340 # accessed in TAL. Note that we need to ensure that only one
326 # PersonalArchiveSubscription is included for each archive,341 # PersonalArchiveSubscription is included for each archive,
@@ -331,15 +346,12 @@
331 for subscription, token in subs_with_tokens:346 for subscription, token in subs_with_tokens:
332 if subscription.archive in unique_archives:347 if subscription.archive in unique_archives:
333 continue348 continue
334
335 unique_archives.add(subscription.archive)349 unique_archives.add(subscription.archive)
336350
337 personal_subscription = PersonalArchiveSubscription(351 personal_subscription = PersonalArchiveSubscription(
338 self.context, subscription.archive)352 self.context, subscription.archive)
339 personal_subscription_tokens.append({353 personal_subscription_tokens.append({
340 'subscription': personal_subscription,354 'subscription': personal_subscription, 'token': token})
341 'token': token
342 })
343355
344 return personal_subscription_tokens356 return personal_subscription_tokens
345357
346358
=== modified file 'lib/lp/soyuz/interfaces/archivesubscriber.py'
--- lib/lp/soyuz/interfaces/archivesubscriber.py 2013-01-07 02:40:55 +0000
+++ lib/lp/soyuz/interfaces/archivesubscriber.py 2013-03-07 01:49:22 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""ArchiveSubscriber interface."""4"""ArchiveSubscriber interface."""
@@ -47,6 +47,7 @@
4747
48 id = Int(title=_('ID'), required=True, readonly=True)48 id = Int(title=_('ID'), required=True, readonly=True)
4949
50 archive_id = Int(title=_('Archive ID'), required=True, readonly=True)
50 archive = exported(Reference(51 archive = exported(Reference(
51 IArchive, title=_("Archive"), required=True, readonly=True,52 IArchive, title=_("Archive"), required=True, readonly=True,
52 description=_("The archive for this subscription.")))53 description=_("The archive for this subscription.")))
5354
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2013-02-20 04:02:11 +0000
+++ lib/lp/soyuz/model/archive.py 2013-03-07 01:49:22 +0000
@@ -442,6 +442,10 @@
442 else:442 else:
443 return None443 return None
444444
445 @cachedproperty
446 def _known_subscribers(self):
447 return set()
448
445 @property449 @property
446 def archive_url(self):450 def archive_url(self):
447 """See `IArchive`."""451 """See `IArchive`."""
448452
=== modified file 'lib/lp/soyuz/tests/test_archive_subscriptions.py'
--- lib/lp/soyuz/tests/test_archive_subscriptions.py 2013-03-06 02:48:35 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py 2013-03-07 01:49:22 +0000
@@ -5,6 +5,8 @@
55
6from urlparse import urljoin6from urlparse import urljoin
77
8from storm.store import Store
9from testtools.matchers import Equals
8from zope.security.interfaces import Unauthorized10from zope.security.interfaces import Unauthorized
9from zope.security.proxy import removeSecurityProxy11from zope.security.proxy import removeSecurityProxy
1012
@@ -17,10 +19,12 @@
17 BrowserTestCase,19 BrowserTestCase,
18 login_person,20 login_person,
19 person_logged_in,21 person_logged_in,
22 StormStatementRecorder,
20 TestCaseWithFactory,23 TestCaseWithFactory,
21 )24 )
22from lp.testing.layers import DatabaseFunctionalLayer25from lp.testing.layers import DatabaseFunctionalLayer
23from lp.testing.mail_helpers import pop_notifications26from lp.testing.mail_helpers import pop_notifications
27from lp.testing.matchers import HasQueryCount
24from lp.testing.pages import (28from lp.testing.pages import (
25 find_tag_by_id,29 find_tag_by_id,
26 setupBrowserForUser,30 setupBrowserForUser,
@@ -102,8 +106,7 @@
102 notifications = pop_notifications()106 notifications = pop_notifications()
103 self.assertEqual(1, len(notifications))107 self.assertEqual(1, len(notifications))
104 self.assertEqual(108 self.assertEqual(
105 self.subscriber.preferredemail.email,109 self.subscriber.preferredemail.email, notifications[0]['to'])
106 notifications[0]['to'])
107110
108 def test_new_commercial_subscription_no_email(self):111 def test_new_commercial_subscription_no_email(self):
109 # As per bug 611568, an email is not sent for112 # As per bug 611568, an email is not sent for
@@ -167,3 +170,23 @@
167 url = urljoin(canonical_url(self.archive), '+packages')170 url = urljoin(canonical_url(self.archive), '+packages')
168 browser = setupBrowserForUser(self.subscriber)171 browser = setupBrowserForUser(self.subscriber)
169 self.assertRaises(Unauthorized, browser.open, url)172 self.assertRaises(Unauthorized, browser.open, url)
173
174
175class PersonArchiveSubscriptions(TestCaseWithFactory):
176
177 layer = DatabaseFunctionalLayer
178
179 def test_query_count(self):
180 subscriber = self.factory.makePerson()
181 for x in range(10):
182 archive = self.factory.makeArchive(private=True)
183 with person_logged_in(archive.owner):
184 archive.newSubscription(subscriber, archive.owner)
185 Store.of(subscriber).flush()
186 Store.of(subscriber).invalidate()
187 with person_logged_in(subscriber):
188 with StormStatementRecorder() as recorder:
189 view = create_initialized_view(
190 subscriber, '+archivesubscriptions', principal=subscriber)
191 view.render()
192 self.assertThat(recorder, HasQueryCount(Equals(9)))