Launchpad itself

Merge lp:~lifeless/launchpad/bugtask+index into lp:launchpad

  1. bugtask+index
  2. Merge into devel
Proposed by Robert Collins
Status: Merged
Approved by: Jonathan Lange
Approved revision: no longer in the source branch.
Merged at revision: 11611
Proposed branch: lp:~lifeless/launchpad/bugtask+index
Merge into: lp:launchpad
Diff against target: 1096 lines (+283/-141)
23 files modified
lib/lp/bugs/browser/bug.py (+32/-26)
lib/lp/bugs/browser/bugtask.py (+14/-11)
lib/lp/bugs/browser/tests/test_bugtask.py (+39/-32)
lib/lp/bugs/configure.zcml (+3/-0)
lib/lp/bugs/interfaces/bug.py (+3/-0)
lib/lp/bugs/interfaces/bugtarget.py (+3/-0)
lib/lp/bugs/interfaces/bugtask.py (+1/-0)
lib/lp/bugs/model/bug.py (+106/-28)
lib/lp/bugs/model/bugtarget.py (+16/-15)
lib/lp/bugs/model/bugtask.py (+13/-5)
lib/lp/bugs/templates/bug-portlet-actions.pt (+2/-2)
lib/lp/bugs/templates/bugtask-index.pt (+1/-1)
lib/lp/registry/configure.zcml (+1/-0)
lib/lp/registry/interfaces/product.py (+1/-0)
lib/lp/registry/interfaces/projectgroup.py (+2/-0)
lib/lp/registry/model/distributionsourcepackage.py (+3/-0)
lib/lp/registry/model/distroseries.py (+3/-0)
lib/lp/registry/model/milestone.py (+3/-2)
lib/lp/registry/model/person.py (+2/-0)
lib/lp/registry/model/productseries.py (+3/-0)
lib/lp/registry/model/projectgroup.py (+20/-1)
lib/lp/registry/model/sourcepackage.py (+3/-0)
lib/lp/registry/vocabularies.py (+9/-18)
To merge this branch: bzr merge lp:~lifeless/launchpad/bugtask+index
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+36117@code.launchpad.net

Commit message

Less queries in BugTask:+index.

Description of the change

Take a basic bugtask:+index down from 110 queries to 64. Thats 46 gone! Woo!

Probably broken, but lets throw it at ec2land and see what happens.

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

Hey Rob,

Looks good to me. Very pleased with the improvements to performance & API clarity.

Only a few minor code clarity & style issues. Please fix them & land.

jml

In def _bug_attachments(self):
  * Spaces after colons
  * Proper sentence case for the docstring.

The comment:
+ # Unwrap the security proxy.
confuses me. I don't see how it's unwrapping the security proxy, or why you would need to. Could you please either delete the comment or expand it?

The "sort and group" comments don't help. Please delete them.

"# Note that this query and the tp query above can be consolidated" in model/person.py has a spelling mistake. ITYM "top query".

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bug.py'
2--- lib/lp/bugs/browser/bug.py 2010-08-31 11:11:09 +0000
3+++ lib/lp/bugs/browser/bug.py 2010-09-22 21:01:52 +0000
4@@ -299,7 +299,7 @@
5
6 def unlinkcve(self):
7 """Return 'Remove CVE link' Link."""
8- enabled = bool(self.context.bug.cves)
9+ enabled = self.context.bug.has_cves
10 text = 'Remove CVE link'
11 return Link('+unlinkcve', text, icon='remove', enabled=enabled)
12
13@@ -307,31 +307,30 @@
14 """Return the 'Offer mentorship' Link."""
15 text = 'Offer mentorship'
16 user = getUtility(ILaunchBag).user
17- enabled = self.context.bug.canMentor(user)
18+ enabled = False
19 return Link('+mentor', text, icon='add', enabled=enabled)
20
21 def retractmentoring(self):
22 """Return the 'Retract mentorship' Link."""
23 text = 'Retract mentorship'
24 user = getUtility(ILaunchBag).user
25- # We should really only allow people to retract mentoring if the
26- # bug's open and the user's already a mentor.
27- if user and not self.context.bug.is_complete:
28- enabled = self.context.bug.isMentor(user)
29- else:
30- enabled = False
31+ enabled = False
32 return Link('+retractmentoring', text, icon='remove', enabled=enabled)
33
34+ @property
35+ def _bug_question(self):
36+ return self.context.bug.getQuestionCreatedFromBug()
37+
38 def createquestion(self):
39 """Create a question from this bug."""
40 text = 'Convert to a question'
41- enabled = self.context.bug.getQuestionCreatedFromBug() is None
42+ enabled = self._bug_question is None
43 return Link('+create-question', text, enabled=enabled, icon='add')
44
45 def removequestion(self):
46 """Remove the created question from this bug."""
47 text = 'Convert back to a bug'
48- enabled = self.context.bug.getQuestionCreatedFromBug() is not None
49+ enabled = self._bug_question is not None
50 return Link('+remove-question', text, enabled=enabled, icon='remove')
51
52 def activitylog(self):
53@@ -510,29 +509,36 @@
54 else:
55 return 'subscribed-false %s' % dup_class
56
57+ @cachedproperty
58+ def _bug_attachments(self):
59+ """Get a dict of attachment type -> attachments list."""
60+ # Note that this is duplicated with get_comments_for_bugtask
61+ # if you are looking to consolidate things.
62+ result = {BugAttachmentType.PATCH: [],
63+ 'other': []
64+ }
65+ for attachment in self.context.attachments_unpopulated:
66+ info = {
67+ 'attachment': attachment,
68+ 'file': ProxiedLibraryFileAlias(
69+ attachment.libraryfile, attachment),
70+ }
71+ if attachment.type == BugAttachmentType.PATCH:
72+ key = attachment.type
73+ else:
74+ key = 'other'
75+ result[key].append(info)
76+ return result
77+
78 @property
79 def regular_attachments(self):
80 """The list of bug attachments that are not patches."""
81- return [
82- {
83- 'attachment': attachment,
84- 'file': ProxiedLibraryFileAlias(
85- attachment.libraryfile, attachment),
86- }
87- for attachment in self.context.attachments_unpopulated
88- if attachment.type != BugAttachmentType.PATCH]
89+ return self._bug_attachments['other']
90
91 @property
92 def patches(self):
93 """The list of bug attachments that are patches."""
94- return [
95- {
96- 'attachment': attachment,
97- 'file': ProxiedLibraryFileAlias(
98- attachment.libraryfile, attachment),
99- }
100- for attachment in self.context.attachments_unpopulated
101- if attachment.type == BugAttachmentType.PATCH]
102+ return self._bug_attachments[BugAttachmentType.PATCH]
103
104
105 class BugView(LaunchpadView, BugViewMixin):
106
107=== modified file 'lib/lp/bugs/browser/bugtask.py'
108--- lib/lp/bugs/browser/bugtask.py 2010-09-09 17:02:33 +0000
109+++ lib/lp/bugs/browser/bugtask.py 2010-09-22 21:01:52 +0000
110@@ -921,6 +921,9 @@
111 This is particularly useful for views that may render a
112 NullBugTask.
113 """
114+ if self.context.id is not None:
115+ # Fast path for real bugtasks: they have a DB id.
116+ return True
117 params = BugTaskSearchParams(user=self.user, bug=self.context.bug)
118 matching_bugtasks = self.context.target.searchTasks(params)
119 if self.context.productseries is not None:
120@@ -1194,14 +1197,14 @@
121 @property
122 def official_tags(self):
123 """The list of official tags for this bug."""
124- target_official_tags = self.context.target.official_bug_tags
125+ target_official_tags = set(self.context.bug.official_tags)
126 return [tag for tag in self.context.bug.tags
127 if tag in target_official_tags]
128
129 @property
130 def unofficial_tags(self):
131 """The list of unofficial tags for this bug."""
132- target_official_tags = self.context.target.official_bug_tags
133+ target_official_tags = set(self.context.bug.official_tags)
134 return [tag for tag in self.context.bug.tags
135 if tag not in target_official_tags]
136
137@@ -1213,11 +1216,10 @@
138 bug has a task. It is returned as Javascript snippet, to be embedded
139 in the bug page.
140 """
141- available_tags = set()
142- for task in self.context.bug.bugtasks:
143- available_tags.update(task.target.official_bug_tags)
144- return 'var available_official_tags = %s;' % dumps(list(sorted(
145- available_tags)))
146+ # Unwrap the security proxy. - official_tags is a security proxy
147+ # wrapped list.
148+ available_tags = list(self.context.bug.official_tags)
149+ return 'var available_official_tags = %s;' % dumps(available_tags)
150
151 @property
152 def user_is_admin(self):
153@@ -3247,6 +3249,7 @@
154 """Cache the list of bugtasks and set up the release mapping."""
155 # Cache some values, so that we don't have to recalculate them
156 # for each bug task.
157+ # This query is redundant: the publisher also queries all the bugtasks.
158 self.bugtasks = list(self.context.bugtasks)
159 self.many_bugtasks = len(self.bugtasks) >= 10
160 self.cached_milestone_source = CachedMilestoneSourceFactory()
161@@ -3338,7 +3341,6 @@
162
163 upstream_tasks.sort(key=_by_targetname)
164 distro_tasks.sort(key=_by_targetname)
165-
166 all_bugtasks = upstream_tasks + distro_tasks
167
168 # Cache whether the bug was converted to a question, since
169@@ -3402,7 +3404,7 @@
170 # Hide the links when the bug is viewed in a CVE context
171 return self.request.getNearest(ICveSet) == (None, None)
172
173- @property
174+ @cachedproperty
175 def current_user_affected_status(self):
176 """Is the current user marked as affected by this bug?"""
177 return self.context.isUserAffected(self.user)
178@@ -3623,10 +3625,11 @@
179
180 return items
181
182- @property
183+ @cachedproperty
184 def target_has_milestones(self):
185 """Are there any milestones we can target?"""
186- return list(MilestoneVocabulary(self.context)) != []
187+ return MilestoneVocabulary.getMilestoneTarget(
188+ self.context).has_milestones
189
190 def bugtask_canonical_url(self):
191 """Return the canonical url for the bugtask."""
192
193=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
194--- lib/lp/bugs/browser/tests/test_bugtask.py 2010-09-01 01:57:37 +0000
195+++ lib/lp/bugs/browser/tests/test_bugtask.py 2010-09-22 21:01:52 +0000
196@@ -28,6 +28,7 @@
197 setUp,
198 tearDown,
199 )
200+from canonical.launchpad.webapp import canonical_url
201 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
202 from canonical.testing import LaunchpadFunctionalLayer
203 from lp.bugs.browser import bugtask
204@@ -37,11 +38,8 @@
205 BugTasksAndNominationsView,
206 )
207 from lp.bugs.interfaces.bugtask import BugTaskStatus
208-from lp.testing import (
209- person_logged_in,
210- StormStatementRecorder,
211- TestCaseWithFactory,
212- )
213+from lp.testing import TestCaseWithFactory
214+from lp.testing._webservice import QueryCollector
215 from lp.testing.matchers import HasQueryCount
216 from lp.testing.sampledata import (
217 ADMIN_EMAIL,
218@@ -54,16 +52,6 @@
219
220 layer = LaunchpadFunctionalLayer
221
222- def record_view_initialization(self, bugtask, person):
223- self.invalidate_caches(bugtask)
224- # Login first because logging in triggers queries.
225- with nested(person_logged_in(person), StormStatementRecorder()) as (
226- _,
227- recorder):
228- view = BugTaskView(bugtask, LaunchpadTestRequest())
229- view.initialize()
230- return recorder
231-
232 def invalidate_caches(self, obj):
233 store = Store.of(obj)
234 # Make sure everything is in the database.
235@@ -72,24 +60,31 @@
236 # the domain objects)
237 store.invalidate()
238
239- def test_query_counts_constant_with_team_memberships(self):
240+ def test_rendered_query_counts_constant_with_team_memberships(self):
241 login(ADMIN_EMAIL)
242 bugtask = self.factory.makeBugTask()
243- person_no_teams = self.factory.makePerson()
244- person_with_teams = self.factory.makePerson()
245+ person_no_teams = self.factory.makePerson(password='test')
246+ person_with_teams = self.factory.makePerson(password='test')
247 for _ in range(10):
248 self.factory.makeTeam(members=[person_with_teams])
249 # count with no teams
250- recorder = self.record_view_initialization(bugtask, person_no_teams)
251- self.assertThat(recorder, HasQueryCount(LessThan(14)))
252+ url = canonical_url(bugtask)
253+ recorder = QueryCollector()
254+ recorder.register()
255+ self.addCleanup(recorder.unregister)
256+ self.invalidate_caches(bugtask)
257+ self.getUserBrowser(url, person_no_teams)
258+ # This may seem large: it is; there is easily another 30% fat in there.
259+ self.assertThat(recorder, HasQueryCount(LessThan(64)))
260 count_with_no_teams = recorder.count
261 # count with many teams
262- recorder2 = self.record_view_initialization(bugtask, person_with_teams)
263+ self.invalidate_caches(bugtask)
264+ self.getUserBrowser(url, person_with_teams)
265 # Allow an increase of one because storm bug 619017 causes additional
266 # queries, revalidating things unnecessarily. An increase which is
267 # less than the number of new teams shows it is definitely not
268 # growing per-team.
269- self.assertThat(recorder2, HasQueryCount(
270+ self.assertThat(recorder, HasQueryCount(
271 LessThan(count_with_no_teams + 3),
272 ))
273
274@@ -105,23 +100,32 @@
275 self.view = BugTasksAndNominationsView(
276 self.bug, LaunchpadTestRequest())
277
278+ def refresh(self):
279+ # The view caches, to see different scenarios, a refresh is needed.
280+ self.view = BugTasksAndNominationsView(
281+ self.bug, LaunchpadTestRequest())
282+
283 def test_current_user_affected_status(self):
284 self.failUnlessEqual(
285 None, self.view.current_user_affected_status)
286- self.view.context.markUserAffected(self.view.user, True)
287+ self.bug.markUserAffected(self.view.user, True)
288+ self.refresh()
289 self.failUnlessEqual(
290 True, self.view.current_user_affected_status)
291- self.view.context.markUserAffected(self.view.user, False)
292+ self.bug.markUserAffected(self.view.user, False)
293+ self.refresh()
294 self.failUnlessEqual(
295 False, self.view.current_user_affected_status)
296
297 def test_current_user_affected_js_status(self):
298 self.failUnlessEqual(
299 'null', self.view.current_user_affected_js_status)
300- self.view.context.markUserAffected(self.view.user, True)
301+ self.bug.markUserAffected(self.view.user, True)
302+ self.refresh()
303 self.failUnlessEqual(
304 'true', self.view.current_user_affected_js_status)
305- self.view.context.markUserAffected(self.view.user, False)
306+ self.bug.markUserAffected(self.view.user, False)
307+ self.refresh()
308 self.failUnlessEqual(
309 'false', self.view.current_user_affected_js_status)
310
311@@ -148,10 +152,12 @@
312 # logged-in user marked him or herself as affected or not.
313 self.failUnlessEqual(
314 1, self.view.other_users_affected_count)
315- self.view.context.markUserAffected(self.view.user, True)
316+ self.bug.markUserAffected(self.view.user, True)
317+ self.refresh()
318 self.failUnlessEqual(
319 1, self.view.other_users_affected_count)
320- self.view.context.markUserAffected(self.view.user, False)
321+ self.bug.markUserAffected(self.view.user, False)
322+ self.refresh()
323 self.failUnlessEqual(
324 1, self.view.other_users_affected_count)
325
326@@ -161,17 +167,18 @@
327 self.failUnlessEqual(
328 1, self.view.other_users_affected_count)
329 other_user_1 = self.factory.makePerson()
330- self.view.context.markUserAffected(other_user_1, True)
331+ self.bug.markUserAffected(other_user_1, True)
332 self.failUnlessEqual(
333 2, self.view.other_users_affected_count)
334 other_user_2 = self.factory.makePerson()
335- self.view.context.markUserAffected(other_user_2, True)
336+ self.bug.markUserAffected(other_user_2, True)
337 self.failUnlessEqual(
338 3, self.view.other_users_affected_count)
339- self.view.context.markUserAffected(other_user_1, False)
340+ self.bug.markUserAffected(other_user_1, False)
341 self.failUnlessEqual(
342 2, self.view.other_users_affected_count)
343- self.view.context.markUserAffected(self.view.user, True)
344+ self.bug.markUserAffected(self.view.user, True)
345+ self.refresh()
346 self.failUnlessEqual(
347 2, self.view.other_users_affected_count)
348
349
350=== modified file 'lib/lp/bugs/configure.zcml'
351--- lib/lp/bugs/configure.zcml 2010-09-10 16:21:21 +0000
352+++ lib/lp/bugs/configure.zcml 2010-09-22 21:01:52 +0000
353@@ -191,6 +191,7 @@
354 date_fix_released
355 date_left_closed
356 date_closed
357+ productseriesID
358 task_age
359 bug_subscribers
360 is_complete
361@@ -592,6 +593,7 @@
362 isMentor
363 getNullBugTask
364 is_complete
365+ official_tags
366 who_made_private
367 date_made_private
368 userCanView
369@@ -624,6 +626,7 @@
370 initial_message
371 linked_branches
372 watches
373+ has_cves
374 cves
375 cve_links
376 duplicates
377
378=== modified file 'lib/lp/bugs/interfaces/bug.py'
379--- lib/lp/bugs/interfaces/bug.py 2010-08-30 23:50:41 +0000
380+++ lib/lp/bugs/interfaces/bug.py 2010-09-22 21:01:52 +0000
381@@ -269,6 +269,7 @@
382 title=_('CVE entries related to this bug.'),
383 value_type=Reference(schema=ICve),
384 readonly=True))
385+ has_cves = Bool(title=u"True if the bug has cve entries.")
386 cve_links = Attribute('Links between this bug and CVE entries.')
387 subscriptions = exported(
388 doNotSnapshot(CollectionField(
389@@ -406,6 +407,8 @@
390
391 latest_patch = Attribute("The most recent patch of this bug.")
392
393+ official_tags = Attribute("The official bug tags relevant to this bug.")
394+
395 @operation_parameters(
396 subject=optional_message_subject_field(),
397 content=copy_field(IMessage['content']))
398
399=== modified file 'lib/lp/bugs/interfaces/bugtarget.py'
400--- lib/lp/bugs/interfaces/bugtarget.py 2010-08-26 20:22:48 +0000
401+++ lib/lp/bugs/interfaces/bugtarget.py 2010-09-22 21:01:52 +0000
402@@ -363,6 +363,9 @@
403 bugs will be returned.
404 """
405
406+ def _getOfficialTagClause():
407+ """Get the storm clause for finding this targets tags."""
408+
409
410 class IOfficialBugTagTargetPublic(IHasOfficialBugTags):
411 """Public attributes for `IOfficialBugTagTarget`."""
412
413=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
414--- lib/lp/bugs/interfaces/bugtask.py 2010-08-30 19:30:45 +0000
415+++ lib/lp/bugs/interfaces/bugtask.py 2010-09-22 21:01:52 +0000
416@@ -454,6 +454,7 @@
417 title=_('Project'), required=False, vocabulary='Product')
418 productseries = Choice(
419 title=_('Series'), required=False, vocabulary='ProductSeries')
420+ productseriesID = Attribute('The product series ID')
421 sourcepackagename = Choice(
422 title=_("Package"), required=False,
423 vocabulary='SourcePackageName')
424
425=== modified file 'lib/lp/bugs/model/bug.py'
426--- lib/lp/bugs/model/bug.py 2010-09-15 23:40:13 +0000
427+++ lib/lp/bugs/model/bug.py 2010-09-22 21:01:52 +0000
428@@ -70,6 +70,7 @@
429 implements,
430 providedBy,
431 )
432+from zope.security.proxy import removeSecurityProxy
433
434 from canonical.config import config
435 from canonical.database.constants import UTC_NOW
436@@ -161,6 +162,7 @@
437 get_bug_privacy_filter,
438 NullBugTask,
439 )
440+from lp.bugs.model.bugtarget import OfficialBugTag
441 from lp.bugs.model.bugwatch import BugWatch
442 from lp.hardwaredb.interfaces.hwdb import IHWSubmissionBugSet
443 from lp.registry.enum import BugNotificationLevel
444@@ -191,6 +193,7 @@
445 from lp.services.propertycache import (
446 cachedproperty,
447 IPropertyCache,
448+ IPropertyCacheManager,
449 )
450
451
452@@ -348,6 +351,21 @@
453 heat_last_updated = UtcDateTimeCol(default=None)
454 latest_patch_uploaded = UtcDateTimeCol(default=None)
455
456+ @cachedproperty
457+ def _subscriber_cache(self):
458+ """Caches known subscribers."""
459+ return set()
460+
461+ @cachedproperty
462+ def _subscriber_dups_cache(self):
463+ """Caches known subscribers to dupes."""
464+ return set()
465+
466+ @cachedproperty
467+ def _unsubscribed_cache(self):
468+ """Cache known non-subscribers."""
469+ return set()
470+
471 @property
472 def latest_patch(self):
473 """See `IBug`."""
474@@ -531,7 +549,7 @@
475 dn += ' ('+self.name+')'
476 return dn
477
478- @property
479+ @cachedproperty
480 def bugtasks(self):
481 """See `IBug`."""
482 result = BugTask.select('BugTask.bug = %s' % sqlvalues(self.id))
483@@ -541,7 +559,8 @@
484 # Do not use the default orderBy as the prejoins cause ambiguities
485 # across the tables.
486 result = result.orderBy("id")
487- return sorted(result, key=bugtask_sort_key)
488+ result = sorted(result, key=bugtask_sort_key)
489+ return result
490
491 @property
492 def default_bugtask(self):
493@@ -665,6 +684,33 @@
494 BugMessage.message == Message.id).order_by('id')
495 return messages.first()
496
497+ @cachedproperty
498+ def official_tags(self):
499+ """See `IBug`."""
500+ # Da circle of imports forces the locals.
501+ from lp.registry.model.distribution import Distribution
502+ from lp.registry.model.product import Product
503+ table = OfficialBugTag
504+ table = LeftJoin(
505+ table,
506+ Distribution,
507+ OfficialBugTag.distribution_id==Distribution.id)
508+ table = LeftJoin(
509+ table,
510+ Product,
511+ OfficialBugTag.product_id==Product.id)
512+ # When this method is typically called it already has the necessary
513+ # info in memory, so rather than rejoin with Product etc, we do this
514+ # bit in Python. If reviewing performance here feel free to change.
515+ clauses = []
516+ for task in self.bugtasks:
517+ clauses.append(
518+ # Storm cannot compile proxied objects.
519+ removeSecurityProxy(task.target._getOfficialTagClause()))
520+ clause = Or(*clauses)
521+ return list(Store.of(self).using(table).find(OfficialBugTag.tag,
522+ clause).order_by(OfficialBugTag.tag).config(distinct=True))
523+
524 def followup_subject(self):
525 """See `IBug`."""
526 return 'Re: '+ self.title
527@@ -698,6 +744,8 @@
528
529 def unsubscribe(self, person, unsubscribed_by):
530 """See `IBug`."""
531+ # Drop cached subscription info.
532+ IPropertyCacheManager(self).clear()
533 if person is None:
534 person = unsubscribed_by
535
536@@ -737,21 +785,11 @@
537
538 def isSubscribed(self, person):
539 """See `IBug`."""
540- if person is None:
541- return False
542-
543- bs = BugSubscription.selectBy(bug=self, person=person)
544- return bool(bs)
545+ return self.personIsDirectSubscriber(person)
546
547 def isSubscribedToDupes(self, person):
548 """See `IBug`."""
549- if person is None:
550- return False
551-
552- return bool(
553- BugSubscription.select("""
554- bug IN (SELECT id FROM Bug WHERE duplicateof = %d) AND
555- person = %d""" % (self.id, person.id)))
556+ return self.personIsSubscribedToDuplicate(person)
557
558 def getDirectSubscriptions(self):
559 """See `IBug`."""
560@@ -868,9 +906,23 @@
561 def getSubscribersForPerson(self, person):
562 """See `IBug."""
563 assert person is not None
564- return Store.of(self).find(
565- # return people
566- Person,
567+ def cache_unsubscribed(rows):
568+ if not rows:
569+ self._unsubscribed_cache.add(person)
570+ def cache_subscriber(row):
571+ _, subscriber, subscription = row
572+ if subscription.bugID == self.id:
573+ self._subscriber_cache.add(subscriber)
574+ else:
575+ self._subscriber_dups_cache.add(subscriber)
576+ return subscriber
577+ return DecoratedResultSet(Store.of(self).find(
578+ # XXX: RobertCollins 2010-09-22 bug=374777: This SQL(...) is a
579+ # hack; it does not seem to be possible to express DISTINCT ON
580+ # with Storm.
581+ (SQL("DISTINCT ON (Person.name, BugSubscription.person) 0 AS ignore"),
582+ # return people and subscribptions
583+ Person, BugSubscription),
584 # For this bug or its duplicates
585 Or(
586 Bug.id == self.id,
587@@ -890,7 +942,8 @@
588 # bug=https://bugs.edge.launchpad.net/storm/+bug/627137
589 # RBC 20100831
590 SQL("""Person.id = TeamParticipation.team"""),
591- ).order_by(Person.name).config(distinct=True)
592+ ).order_by(Person.name),
593+ cache_subscriber, pre_iter_hook=cache_unsubscribed)
594
595 def getAlsoNotifiedSubscribers(self, recipients=None, level=None):
596 """See `IBug`.
597@@ -1216,6 +1269,11 @@
598 notify(ObjectDeletedEvent(bug_branch, user=user))
599 bug_branch.destroySelf()
600
601+ @cachedproperty
602+ def has_cves(self):
603+ """See `IBug`."""
604+ return bool(self.cves)
605+
606 def linkCVE(self, cve, user):
607 """See `IBug`."""
608 if cve not in self.cves:
609@@ -1314,18 +1372,23 @@
610 question_target = IQuestionTarget(bugtask.target)
611 question = question_target.createQuestionFromBug(self)
612 self.addChange(BugConvertedToQuestion(UTC_NOW, person, question))
613+ IPropertyCache(self)._question_from_bug = question
614
615 notify(BugBecameQuestionEvent(self, question, person))
616 return question
617
618- def getQuestionCreatedFromBug(self):
619- """See `IBug`."""
620+ @cachedproperty
621+ def _question_from_bug(self):
622 for question in self.questions:
623- if (question.owner == self.owner
624+ if (question.ownerID == self.ownerID
625 and question.datecreated == self.datecreated):
626 return question
627 return None
628
629+ def getQuestionCreatedFromBug(self):
630+ """See `IBug`."""
631+ return self._question_from_bug
632+
633 def canMentor(self, user):
634 """See `ICanBeMentored`."""
635 if user is None:
636@@ -1624,10 +1687,13 @@
637
638 def _getTags(self):
639 """Get the tags as a sorted list of strings."""
640- tags = [
641- bugtag.tag
642- for bugtag in BugTag.selectBy(bug=self, orderBy='tag')]
643- return tags
644+ return self._cached_tags
645+
646+ @cachedproperty
647+ def _cached_tags(self):
648+ return list(Store.of(self).find(
649+ BugTag.tag,
650+ BugTag.bugID==self.id).order_by(BugTag.tag))
651
652 def _setTags(self, tags):
653 """Set the tags from a list of strings."""
654@@ -1635,6 +1701,7 @@
655 # and insert the new ones.
656 new_tags = set([tag.lower() for tag in tags])
657 old_tags = set(self.tags)
658+ del IPropertyCache(self)._cached_tags
659 added_tags = new_tags.difference(old_tags)
660 removed_tags = old_tags.difference(new_tags)
661 for removed_tag in removed_tags:
662@@ -1782,12 +1849,17 @@
663
664 def personIsDirectSubscriber(self, person):
665 """See `IBug`."""
666+ if person in self._subscriber_cache:
667+ return True
668+ if person in self._unsubscribed_cache:
669+ return False
670+ if person is None:
671+ return False
672 store = Store.of(self)
673 subscriptions = store.find(
674 BugSubscription,
675 BugSubscription.bug == self,
676 BugSubscription.person == person)
677-
678 return not subscriptions.is_empty()
679
680 def personIsAlsoNotifiedSubscriber(self, person):
681@@ -1803,6 +1875,12 @@
682
683 def personIsSubscribedToDuplicate(self, person):
684 """See `IBug`."""
685+ if person in self._subscriber_dups_cache:
686+ return True
687+ if person in self._unsubscribed_cache:
688+ return False
689+ if person is None:
690+ return False
691 store = Store.of(self)
692 subscriptions_from_dupes = store.find(
693 BugSubscription,
694@@ -2012,13 +2090,13 @@
695
696 # Create the task on a product if one was passed.
697 if params.product:
698- BugTaskSet().createTask(
699+ getUtility(IBugTaskSet).createTask(
700 bug=bug, product=params.product, owner=params.owner,
701 status=params.status)
702
703 # Create the task on a source package name if one was passed.
704 if params.distribution:
705- BugTaskSet().createTask(
706+ getUtility(IBugTaskSet).createTask(
707 bug=bug, distribution=params.distribution,
708 sourcepackagename=params.sourcepackagename,
709 owner=params.owner, status=params.status)
710
711=== modified file 'lib/lp/bugs/model/bugtarget.py'
712--- lib/lp/bugs/model/bugtarget.py 2010-09-02 22:08:12 +0000
713+++ lib/lp/bugs/model/bugtarget.py 2010-09-22 21:01:52 +0000
714@@ -338,22 +338,26 @@
715
716 Using this call in ProjectGroup requires a fix of bug 341203, see
717 below, class OfficialBugTag.
718+
719+ See also `Bug.official_bug_tags` which calculates this efficiently for
720+ a single bug.
721 """
722
723+ def _getOfficialTagClause(self):
724+ if IDistribution.providedBy(self):
725+ return (OfficialBugTag.distribution == self)
726+ elif IProduct.providedBy(self):
727+ return (OfficialBugTag.product == self)
728+ else:
729+ raise AssertionError(
730+ '%s is not a valid official bug target' % self)
731+
732 def _getOfficialTags(self):
733 """Get the official bug tags as a sorted list of strings."""
734 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
735- if IDistribution.providedBy(self):
736- target_clause = (OfficialBugTag.distribution == self)
737- elif IProduct.providedBy(self):
738- target_clause = (OfficialBugTag.product == self)
739- else:
740- raise AssertionError(
741- '%s is not a valid official bug target' % self)
742- tags = [
743- obt.tag for obt
744- in store.find(OfficialBugTag, target_clause).order_by('tag')]
745- return tags
746+ target_clause = self._getOfficialTagClause()
747+ return list(store.find(
748+ OfficialBugTag.tag, target_clause).order_by(OfficialBugTag.tag))
749
750 def _setOfficialTags(self, tags):
751 """Set the official bug tags from a list of strings."""
752@@ -374,10 +378,7 @@
753 If the tag is not defined for this target, None is returned.
754 """
755 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
756- if IDistribution.providedBy(self):
757- target_clause = (OfficialBugTag.distribution == self)
758- else:
759- target_clause = (OfficialBugTag.product == self)
760+ target_clause = self._getOfficialTagClause()
761 return store.find(
762 OfficialBugTag, OfficialBugTag.tag==tag, target_clause).one()
763
764
765=== modified file 'lib/lp/bugs/model/bugtask.py'
766--- lib/lp/bugs/model/bugtask.py 2010-09-09 17:02:33 +0000
767+++ lib/lp/bugs/model/bugtask.py 2010-09-22 21:01:52 +0000
768@@ -43,7 +43,10 @@
769 Or,
770 SQL,
771 )
772-from storm.store import EmptyResultSet
773+from storm.store import (
774+ EmptyResultSet,
775+ Store,
776+ )
777 from storm.zope.interfaces import (
778 IResultSet,
779 ISQLObjectResultSet,
780@@ -399,6 +402,7 @@
781 sourcepackagename=None, distribution=None,
782 distroseries=None):
783 """Initialize a NullBugTask."""
784+ self.id = None
785 self.bug = bug
786 self.product = product
787 self.productseries = productseries
788@@ -756,11 +760,11 @@
789 conjoined_master = bugtask
790 break
791 elif IUpstreamBugTask.providedBy(self):
792- assert self.product.development_focus is not None, (
793+ assert self.product.development_focusID is not None, (
794 'A product should always have a development series.')
795- devel_focus = self.product.development_focus
796+ devel_focusID = self.product.development_focusID
797 for bugtask in bugtasks:
798- if bugtask.productseries == devel_focus:
799+ if bugtask.productseriesID == devel_focusID:
800 conjoined_master = bugtask
801 break
802
803@@ -2367,7 +2371,11 @@
804 bugtask._syncFromConjoinedSlave()
805
806 bugtask.updateTargetNameCache()
807-
808+ del IPropertyCache(bug).bugtasks
809+ # Because of block_implicit_flushes, it is possible for a new bugtask
810+ # to be queued in appropriately, which leads to Bug.bugtasks not
811+ # finding the bugtask.
812+ Store.of(bugtask).flush()
813 return bugtask
814
815 def getStatusCountsForProductSeries(self, user, product_series):
816
817=== modified file 'lib/lp/bugs/templates/bug-portlet-actions.pt'
818--- lib/lp/bugs/templates/bug-portlet-actions.pt 2010-08-02 17:49:45 +0000
819+++ lib/lp/bugs/templates/bug-portlet-actions.pt 2010-09-22 21:01:52 +0000
820@@ -18,8 +18,8 @@
821 class="menu-link-mark-dupe">Mark as duplicate</a>
822 </span>
823 <tal:block
824- tal:condition="context/duplicates"
825- tal:define="number_of_dupes context/duplicates/count"
826+ tal:condition="context/number_of_duplicates"
827+ tal:define="number_of_dupes context/number_of_duplicates"
828 >
829 </tal:block>
830 <tal:block
831
832=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
833--- lib/lp/bugs/templates/bugtask-index.pt 2010-09-07 16:29:19 +0000
834+++ lib/lp/bugs/templates/bugtask-index.pt 2010-09-22 21:01:52 +0000
835@@ -195,7 +195,7 @@
836 </tal:branches>
837 </div><!-- bug-branch-container -->
838
839- <div tal:condition="context/bug/cves" class="cves">
840+ <div tal:condition="context/bug/has_cves" class="cves">
841 <h2>CVE References</h2>
842 <ul>
843 <li class="sprite cve" tal:repeat="cve context/bug/cves">
844
845=== modified file 'lib/lp/registry/configure.zcml'
846--- lib/lp/registry/configure.zcml 2010-09-21 13:39:31 +0000
847+++ lib/lp/registry/configure.zcml 2010-09-22 21:01:52 +0000
848@@ -452,6 +452,7 @@
849 getReleasesAndPublishingHistory
850 upstream_product
851 target_type_display
852+ _getOfficialTagClause
853 official_bug_tags
854 findRelatedArchives
855 findRelatedArchivePublications
856
857=== modified file 'lib/lp/registry/interfaces/product.py'
858--- lib/lp/registry/interfaces/product.py 2010-09-09 17:02:33 +0000
859+++ lib/lp/registry/interfaces/product.py 2010-09-22 21:01:52 +0000
860@@ -660,6 +660,7 @@
861 'The series that represents the master or trunk branch. '
862 'The Bazaar URL lp:<project> points to the development focus '
863 'series branch.')))
864+ development_focusID = Attribute("The development focus ID.")
865
866 name_with_project = Attribute(_("Returns the product name prefixed "
867 "by the project name, if a project is associated with this "
868
869=== modified file 'lib/lp/registry/interfaces/projectgroup.py'
870--- lib/lp/registry/interfaces/projectgroup.py 2010-08-21 18:41:57 +0000
871+++ lib/lp/registry/interfaces/projectgroup.py 2010-09-22 21:01:52 +0000
872@@ -328,6 +328,8 @@
873 def getSeries(series_name):
874 """Return a ProjectGroupSeries object with name `series_name`."""
875
876+ product_milestones = Attribute('all the milestones for all the products.')
877+
878
879 class IProjectGroup(IProjectGroupPublic,
880 IProjectGroupModerate,
881
882=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
883--- lib/lp/registry/model/distributionsourcepackage.py 2010-09-12 17:43:49 +0000
884+++ lib/lp/registry/model/distributionsourcepackage.py 2010-09-22 21:01:52 +0000
885@@ -494,6 +494,9 @@
886 BugTask.sourcepackagename == self.sourcepackagename),
887 user)
888
889+ def _getOfficialTagClause(self):
890+ return self.distribution._getOfficialTagClause()
891+
892 @property
893 def official_bug_tags(self):
894 """See `IHasBugs`."""
895
896=== modified file 'lib/lp/registry/model/distroseries.py'
897--- lib/lp/registry/model/distroseries.py 2010-09-21 19:34:45 +0000
898+++ lib/lp/registry/model/distroseries.py 2010-09-22 21:01:52 +0000
899@@ -743,6 +743,9 @@
900 """Customize `search_params` for this distribution series."""
901 search_params.setDistroSeries(self)
902
903+ def _getOfficialTagClause(self):
904+ return self.distribution._getOfficialTagClause()
905+
906 @property
907 def official_bug_tags(self):
908 """See `IHasBugs`."""
909
910=== modified file 'lib/lp/registry/model/milestone.py'
911--- lib/lp/registry/model/milestone.py 2010-09-14 15:04:06 +0000
912+++ lib/lp/registry/model/milestone.py 2010-09-22 21:01:52 +0000
913@@ -107,8 +107,7 @@
914 result = store.find(Milestone, self._getMilestoneCondition())
915 return result.order_by(self._milestone_order)
916
917- @property
918- def milestones(self):
919+ def _get_milestones(self):
920 """See `IHasMilestones`."""
921 store = Store.of(self)
922 result = store.find(Milestone,
923@@ -116,6 +115,8 @@
924 Milestone.active == True))
925 return result.order_by(self._milestone_order)
926
927+ milestones = property(_get_milestones)
928+
929
930 class MultipleProductReleases(Exception):
931 """Raised when a second ProductRelease is created for a milestone."""
932
933=== modified file 'lib/lp/registry/model/person.py'
934--- lib/lp/registry/model/person.py 2010-09-10 20:27:19 +0000
935+++ lib/lp/registry/model/person.py 2010-09-22 21:01:52 +0000
936@@ -1265,6 +1265,8 @@
937 # The owner is not a member but must retain his rights over
938 # this team. This person may be a member of the owner, and in this
939 # case it'll also have rights over this team.
940+ # Note that this query and the tp query above can be consolidated
941+ # when we get to a finer grained level of optimisations.
942 in_team = self.inTeam(team.teamowner)
943 else:
944 in_team = False
945
946=== modified file 'lib/lp/registry/model/productseries.py'
947--- lib/lp/registry/model/productseries.py 2010-09-21 19:34:45 +0000
948+++ lib/lp/registry/model/productseries.py 2010-09-22 21:01:52 +0000
949@@ -423,6 +423,9 @@
950 """Customize `search_params` for this product series."""
951 search_params.setProductSeries(self)
952
953+ def _getOfficialTagClause(self):
954+ return self.product._getOfficialTagClause()
955+
956 @property
957 def official_bug_tags(self):
958 """See `IHasBugs`."""
959
960=== modified file 'lib/lp/registry/model/projectgroup.py'
961--- lib/lp/registry/model/projectgroup.py 2010-09-09 15:26:11 +0000
962+++ lib/lp/registry/model/projectgroup.py 2010-09-22 21:01:52 +0000
963@@ -93,6 +93,7 @@
964 from lp.registry.model.karma import KarmaContextMixin
965 from lp.registry.model.mentoringoffer import MentoringOffer
966 from lp.registry.model.milestone import (
967+ HasMilestonesMixin,
968 Milestone,
969 ProjectMilestone,
970 )
971@@ -110,7 +111,8 @@
972 MakesAnnouncements, HasSprintsMixin, HasAliasMixin,
973 KarmaContextMixin, BranchVisibilityPolicyMixin,
974 StructuralSubscriptionTargetMixin,
975- HasBranchesMixin, HasMergeProposalsMixin, HasBugHeatMixin):
976+ HasBranchesMixin, HasMergeProposalsMixin, HasBugHeatMixin,
977+ HasMilestonesMixin):
978 """A ProjectGroup"""
979
980 implements(IProjectGroup, IFAQCollection, IHasBugHeat, IHasIcon, IHasLogo,
981@@ -309,6 +311,11 @@
982 """Customize `search_params` for this milestone."""
983 search_params.setProject(self)
984
985+ def _getOfficialTagClause(self):
986+ """See `OfficialBugTagTargetMixin`."""
987+ And(ProjectGroup.id == Product.projectID,
988+ Product.id == OfficialBugTag.productID)
989+
990 @property
991 def official_bug_tags(self):
992 """See `IHasBugs`."""
993@@ -396,6 +403,11 @@
994 """
995 return self.products.count() != 0
996
997+ def _getMilestoneCondition(self):
998+ """See `HasMilestonesMixin`."""
999+ return And(Milestone.productID == Product.id,
1000+ Product.projectID == self.id)
1001+
1002 def _getMilestones(self, only_active):
1003 """Return a list of milestones for this project group.
1004
1005@@ -445,6 +457,13 @@
1006 return self._getMilestones(True)
1007
1008 @property
1009+ def product_milestones(self):
1010+ """Hack to avoid the ProjectMilestone in MilestoneVocabulary."""
1011+ # XXX: bug=644977 Robert Collins - this is a workaround for
1012+ # insconsistency in project group milestone use.
1013+ return self._get_milestones()
1014+
1015+ @property
1016 def all_milestones(self):
1017 """See `IProjectGroup`."""
1018 return self._getMilestones(False)
1019
1020=== modified file 'lib/lp/registry/model/sourcepackage.py'
1021--- lib/lp/registry/model/sourcepackage.py 2010-09-07 00:51:44 +0000
1022+++ lib/lp/registry/model/sourcepackage.py 2010-09-22 21:01:52 +0000
1023@@ -481,6 +481,9 @@
1024 """Customize `search_params` for this source package."""
1025 search_params.setSourcePackage(self)
1026
1027+ def _getOfficialTagClause(self):
1028+ return self.distroseries._getOfficialTagClause()
1029+
1030 @property
1031 def official_bug_tags(self):
1032 """See `IHasBugs`."""
1033
1034=== modified file 'lib/lp/registry/vocabularies.py'
1035--- lib/lp/registry/vocabularies.py 2010-08-31 11:11:09 +0000
1036+++ lib/lp/registry/vocabularies.py 2010-09-22 21:01:52 +0000
1037@@ -1161,16 +1161,18 @@
1038 elif IDistroSeriesBugTask.providedBy(milestone_context):
1039 target = milestone_context.distroseries
1040 elif IProductSeriesBugTask.providedBy(milestone_context):
1041- target = milestone_context.productseries
1042+ target = milestone_context.productseries.product
1043 elif IDistributionSourcePackage.providedBy(milestone_context):
1044 target = milestone_context.distribution
1045 elif ISourcePackage.providedBy(milestone_context):
1046 target = milestone_context.distroseries
1047 elif ISpecification.providedBy(milestone_context):
1048 target = milestone_context.target
1049+ elif IProductSeries.providedBy(milestone_context):
1050+ # Show all the milestones of the product for a product series.
1051+ target = milestone_context.product
1052 elif (IProjectGroup.providedBy(milestone_context) or
1053 IProduct.providedBy(milestone_context) or
1054- IProductSeries.providedBy(milestone_context) or
1055 IDistribution.providedBy(milestone_context) or
1056 IDistroSeries.providedBy(milestone_context)):
1057 target = milestone_context
1058@@ -1196,23 +1198,10 @@
1059 # should be revisited after we've unblocked users.
1060 if target is not None:
1061 if IProjectGroup.providedBy(target):
1062- milestones = shortlist(
1063- (milestone for product in target.products
1064- for milestone in product.milestones),
1065- longest_expected=40)
1066- elif IProductSeries.providedBy(target):
1067- # While some milestones may be associated with a
1068- # productseries, we want to show all milestones for
1069- # the product. Since the database constraint
1070- # "valid_target" ensures that a milestone associated
1071- # with a series is also associated with the product
1072- # itself, we don't need to look up series-related
1073- # milestones.
1074- milestones = shortlist(target.product.milestones,
1075- longest_expected=40)
1076+ milestones_source = target.product_milestones
1077 else:
1078- milestones = shortlist(
1079- target.milestones, longest_expected=40)
1080+ milestones_source = target.milestones
1081+ milestones = shortlist(milestones_source, longest_expected=40)
1082 else:
1083 # We can't use context to reasonably filter the
1084 # milestones, so let's either just grab all of them,
1085@@ -1249,9 +1238,11 @@
1086 product_ids = set(
1087 removeSecurityProxy(milestone).productID
1088 for milestone in milestones)
1089+ product_ids.discard(None)
1090 distro_ids = set(
1091 removeSecurityProxy(milestone).distributionID
1092 for milestone in milestones)
1093+ distro_ids.discard(None)
1094 if len(product_ids) > 0:
1095 list(Product.select("id IN %s" % sqlvalues(product_ids)))
1096 if len(distro_ids) > 0: