Merge lp:~cjwatson/launchpad/repeated-subscription-queries into lp:launchpad

Proposed by Colin Watson on 2018-09-10
Status: Merged
Merged at revision: 18775
Proposed branch: lp:~cjwatson/launchpad/repeated-subscription-queries
Merge into: lp:launchpad
Diff against target: 117 lines (+25/-11)
4 files modified
lib/lp/code/browser/branchsubscription.py (+5/-5)
lib/lp/code/browser/gitsubscription.py (+5/-5)
lib/lp/code/browser/tests/test_branch.py (+1/-1)
lib/lp/code/browser/tests/test_gitrepository.py (+14/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/repeated-subscription-queries
Reviewer Review Type Date Requested Status
William Grant code 2018-09-10 Approve on 2018-09-11
Review via email: mp+354614@code.launchpad.net

Commit message

Avoid unnecessarily-duplicated BranchSubscription/GitSubscription queries.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)
18773. By Colin Watson on 2018-09-11

Merge devel.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/branchsubscription.py'
2--- lib/lp/code/browser/branchsubscription.py 2015-09-28 17:38:45 +0000
3+++ lib/lp/code/browser/branchsubscription.py 2018-09-11 15:06:31 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 __metaclass__ = type
10@@ -42,18 +42,18 @@
11 # Cache permissions so private subscribers can be rendered.
12 # The security adaptor will do the job also but we don't want or need
13 # the expense of running several complex SQL queries.
14- person_ids = [sub.personID for sub in self.context.subscriptions]
15+ subscriptions = list(self.context.subscriptions)
16+ person_ids = [sub.personID for sub in subscriptions]
17 list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
18 person_ids, need_validity=True))
19 if self.user is not None:
20 subscribers = [
21- subscription.person
22- for subscription in self.context.subscriptions]
23+ subscription.person for subscription in subscriptions]
24 precache_permission_for_objects(
25 self.request, "launchpad.LimitedView", subscribers)
26
27 visible_subscriptions = [
28- subscription for subscription in self.context.subscriptions
29+ subscription for subscription in subscriptions
30 if check_permission('launchpad.LimitedView', subscription.person)]
31 return sorted(
32 visible_subscriptions,
33
34=== modified file 'lib/lp/code/browser/gitsubscription.py'
35--- lib/lp/code/browser/gitsubscription.py 2015-09-28 17:38:45 +0000
36+++ lib/lp/code/browser/gitsubscription.py 2018-09-11 15:06:31 +0000
37@@ -1,4 +1,4 @@
38-# Copyright 2015 Canonical Ltd. This software is licensed under the
39+# Copyright 2015-2018 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@@ -42,18 +42,18 @@
44 # Cache permissions so private subscribers can be rendered.
45 # The security adaptor will do the job also but we don't want or
46 # need the expense of running several complex SQL queries.
47- person_ids = [sub.person_id for sub in self.context.subscriptions]
48+ subscriptions = list(self.context.subscriptions)
49+ person_ids = [sub.person_id for sub in subscriptions]
50 list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
51 person_ids, need_validity=True))
52 if self.user is not None:
53 subscribers = [
54- subscription.person
55- for subscription in self.context.subscriptions]
56+ subscription.person for subscription in subscriptions]
57 precache_permission_for_objects(
58 self.request, "launchpad.LimitedView", subscribers)
59
60 visible_subscriptions = [
61- subscription for subscription in self.context.subscriptions
62+ subscription for subscription in subscriptions
63 if check_permission("launchpad.LimitedView", subscription.person)]
64 return sorted(
65 visible_subscriptions,
66
67=== modified file 'lib/lp/code/browser/tests/test_branch.py'
68--- lib/lp/code/browser/tests/test_branch.py 2018-09-07 13:43:50 +0000
69+++ lib/lp/code/browser/tests/test_branch.py 2018-09-11 15:06:31 +0000
70@@ -598,7 +598,7 @@
71 branch, '+branch-portlet-subscriber-content')
72 with StormStatementRecorder() as recorder:
73 view.render()
74- self.assertThat(recorder, HasQueryCount(Equals(9)))
75+ self.assertThat(recorder, HasQueryCount(Equals(6)))
76
77 def test_query_count_index_with_subscribers(self):
78 branch = self.factory.makeBranch()
79
80=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
81--- lib/lp/code/browser/tests/test_gitrepository.py 2018-09-10 15:36:04 +0000
82+++ lib/lp/code/browser/tests/test_gitrepository.py 2018-09-11 15:06:31 +0000
83@@ -13,6 +13,7 @@
84
85 from fixtures import FakeLogger
86 import pytz
87+from storm.store import Store
88 from testtools.matchers import (
89 DocTestMatches,
90 Equals,
91@@ -52,6 +53,7 @@
92 logout,
93 person_logged_in,
94 record_two_runs,
95+ StormStatementRecorder,
96 TestCaseWithFactory,
97 )
98 from lp.testing.layers import (
99@@ -388,6 +390,18 @@
100 self.assertIsNone(
101 find_tag_by_id(browser.contents, 'landing-targets'))
102
103+ def test_query_count_subscriber_content(self):
104+ repository = self.factory.makeGitRepository()
105+ for _ in range(10):
106+ self.factory.makeGitSubscription(repository=repository)
107+ Store.of(repository).flush()
108+ Store.of(repository).invalidate()
109+ view = create_initialized_view(
110+ repository, "+repository-portlet-subscriber-content")
111+ with StormStatementRecorder() as recorder:
112+ view.render()
113+ self.assertThat(recorder, HasQueryCount(Equals(6)))
114+
115
116 class TestGitRepositoryViewPrivateArtifacts(BrowserTestCase):
117 """Tests that Git repositories with private team artifacts can be viewed.