Merge lp:~cjwatson/launchpad/person-ppas-timeout into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18368
Proposed branch: lp:~cjwatson/launchpad/person-ppas-timeout
Merge into: lp:launchpad
Diff against target: 111 lines (+33/-9)
4 files modified
lib/lp/registry/browser/person.py (+8/-3)
lib/lp/registry/browser/tests/test_person.py (+16/-1)
lib/lp/registry/model/person.py (+1/-2)
lib/lp/soyuz/browser/archive.py (+8/-3)
To merge this branch: bzr merge lp:~cjwatson/launchpad/person-ppas-timeout
Reviewer Review Type Date Requested Status
William Grant (community) code Approve
Review via email: mp+322995@code.launchpad.net

Commit message

Precache permissions for archives returned by Person.getVisiblePPAs.

Description of the change

Person.getVisiblePPAs uselessly included subscribed PPAs, on which the requesting user wouldn't have launchpad.View unless the subscribed PPAs happened to also match one of the other criteria.

Once the return value of Person.getVisiblePPAs is actually guaranteed to be launchpad.View-safe, precaching permissions then fixes large query counts on Person:+index in cases where the requesting user has upload permission on many PPAs owned by the context person.

I made the same change to Person:+activate-ppa for completeness, although that can't currently cause a problem in practice because it can only be used by the context person or by admins, thereby bypassing most of the slow bits of the security adapter.

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
1=== modified file 'lib/lp/registry/browser/person.py'
2--- lib/lp/registry/browser/person.py 2017-02-08 02:44:59 +0000
3+++ lib/lp/registry/browser/person.py 2017-04-22 15:21:55 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Person-related view classes."""
10@@ -245,7 +245,10 @@
11 stepto,
12 structured,
13 )
14-from lp.services.webapp.authorization import check_permission
15+from lp.services.webapp.authorization import (
16+ check_permission,
17+ precache_permission_for_objects,
18+ )
19 from lp.services.webapp.batching import BatchNavigator
20 from lp.services.webapp.breadcrumb import DisplaynameBreadcrumb
21 from lp.services.webapp.interfaces import (
22@@ -2017,7 +2020,9 @@
23
24 @cachedproperty
25 def visible_ppas(self):
26- return self.context.getVisiblePPAs(self.user)
27+ ppas = self.context.getVisiblePPAs(self.user)
28+ precache_permission_for_objects(self.request, 'launchpad.View', ppas)
29+ return ppas
30
31 @property
32 def time_zone_offset(self):
33
34=== modified file 'lib/lp/registry/browser/tests/test_person.py'
35--- lib/lp/registry/browser/tests/test_person.py 2017-02-08 02:44:59 +0000
36+++ lib/lp/registry/browser/tests/test_person.py 2017-04-22 15:21:55 +0000
37@@ -1,4 +1,4 @@
38-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
39+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
40 # GNU Affero General Public License version 3 (see the file LICENSE).
41
42 __metaclass__ = type
43@@ -349,6 +349,21 @@
44 view = create_initialized_view(person, '+index')
45 self.assertTrue(view.should_show_gpgkeys_section)
46
47+ def test_ppas_query_count(self):
48+ owner = self.factory.makePerson()
49+
50+ def create_ppa_and_permission():
51+ ppa = self.factory.makeArchive(
52+ owner=owner, purpose=ArchivePurpose.PPA, private=True)
53+ ppa.newComponentUploader(self.user, 'main')
54+
55+ recorder1, recorder2 = record_two_runs(
56+ lambda: self.getMainText(owner, '+index'),
57+ create_ppa_and_permission, 5,
58+ login_method=lambda: login_person(owner),
59+ record_request=True)
60+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
61+
62
63 class TestPersonViewKarma(TestCaseWithFactory):
64
65
66=== modified file 'lib/lp/registry/model/person.py'
67--- lib/lp/registry/model/person.py 2017-04-11 06:36:15 +0000
68+++ lib/lp/registry/model/person.py 2017-04-22 15:21:55 +0000
69@@ -3050,8 +3050,7 @@
70 from lp.soyuz.model.archive import get_enabled_archive_filter
71
72 filter = get_enabled_archive_filter(
73- user, purpose=ArchivePurpose.PPA,
74- include_public=True, include_subscribed=True)
75+ user, purpose=ArchivePurpose.PPA, include_public=True)
76 return Store.of(self).find(
77 Archive,
78 Archive.owner == self,
79
80=== modified file 'lib/lp/soyuz/browser/archive.py'
81--- lib/lp/soyuz/browser/archive.py 2016-01-26 15:47:37 +0000
82+++ lib/lp/soyuz/browser/archive.py 2017-04-22 15:21:55 +0000
83@@ -1,4 +1,4 @@
84-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
85+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
86 # GNU Affero General Public License version 3 (see the file LICENSE).
87
88 """Browser views for archive."""
89@@ -109,7 +109,10 @@
90 Navigation,
91 stepthrough,
92 )
93-from lp.services.webapp.authorization import check_permission
94+from lp.services.webapp.authorization import (
95+ check_permission,
96+ precache_permission_for_objects,
97+ )
98 from lp.services.webapp.batching import BatchNavigator
99 from lp.services.webapp.escaping import structured
100 from lp.services.webapp.interfaces import (
101@@ -1885,7 +1888,9 @@
102
103 @cachedproperty
104 def visible_ppas(self):
105- return self.context.getVisiblePPAs(self.user)
106+ ppas = self.context.getVisiblePPAs(self.user)
107+ precache_permission_for_objects(self.request, 'launchpad.View', ppas)
108+ return ppas
109
110 @property
111 def initial_values(self):