Merge lp:~wgrant/launchpad/bug-750949 into lp:launchpad

Proposed by William Grant
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 12748
Proposed branch: lp:~wgrant/launchpad/bug-750949
Merge into: lp:launchpad
Diff against target: 124 lines (+41/-7)
4 files modified
lib/canonical/launchpad/webapp/publication.py (+2/-0)
lib/lp/bugs/browser/bugattachment.py (+3/-0)
lib/lp/bugs/browser/tests/test_bugtask.py (+28/-3)
lib/lp/bugs/model/bug.py (+8/-4)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-750949
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+56295@code.launchpad.net

Commit message

[r=lifeless][bug=750949] Preload attachment LibraryFileContents to eliminate another BugTask:+index scaling issue.

Description of the change

Bug comments display the size of any associated attachments, which requires the LibraryFileContent. This branch expands the LFA preloading to LFCs too, and adds a test.

Ahhh, the test. Browsing to a bug page caused canonical_url(attachment) to die, which I eventually tracked down to BugAttachmentURL depending on a value in ILaunchBag. While one could argue that BugAttachmentURL should be fixed, how it should be fixed is not entirely clear. So, I opted to avoid hours more of future confusion by clearing LaunchBag at the end of each request. It's already cleared at the start, so it won't affect the webapp. Some tests touch LaunchBag inappropriately, but that's mostly to get the current logged in user, not anything from a browser request.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/webapp/publication.py'
--- lib/canonical/launchpad/webapp/publication.py 2011-03-29 00:11:57 +0000
+++ lib/canonical/launchpad/webapp/publication.py 2011-04-05 06:57:34 +0000
@@ -708,6 +708,8 @@
708708
709 da.clear_request_started()709 da.clear_request_started()
710710
711 getUtility(IOpenLaunchBag).clear()
712
711 # Maintain operational statistics.713 # Maintain operational statistics.
712 if getattr(request, '_wants_retry', False):714 if getattr(request, '_wants_retry', False):
713 OpStats.stats['retries'] += 1715 OpStats.stats['retries'] += 1
714716
=== modified file 'lib/lp/bugs/browser/bugattachment.py'
--- lib/lp/bugs/browser/bugattachment.py 2011-03-28 00:43:40 +0000
+++ lib/lp/bugs/browser/bugattachment.py 2011-04-05 06:57:34 +0000
@@ -92,6 +92,9 @@
92 usedfor = IBugAttachmentSet92 usedfor = IBugAttachmentSet
9393
9494
95# Despite declaring compliance with ICanonicalUrlData, the LaunchBag
96# dependency means this tends towards the "not canonical at all" end of
97# the canonicalness scale. Beware.
95class BugAttachmentURL:98class BugAttachmentURL:
96 """Bug URL creation rules."""99 """Bug URL creation rules."""
97 implements(ICanonicalUrlData)100 implements(ICanonicalUrlData)
98101
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2011-04-05 06:57:34 +0000
@@ -20,7 +20,10 @@
20from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities20from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
21from canonical.launchpad.webapp import canonical_url21from canonical.launchpad.webapp import canonical_url
22from canonical.launchpad.webapp.servers import LaunchpadTestRequest22from canonical.launchpad.webapp.servers import LaunchpadTestRequest
23from canonical.testing.layers import DatabaseFunctionalLayer23from canonical.testing.layers import (
24 DatabaseFunctionalLayer,
25 LaunchpadFunctionalLayer,
26 )
24from lp.bugs.browser.bugtask import (27from lp.bugs.browser.bugtask import (
25 BugTaskEditView,28 BugTaskEditView,
26 BugTasksAndNominationsView,29 BugTasksAndNominationsView,
@@ -30,11 +33,15 @@
30from lp.services.propertycache import get_property_cache33from lp.services.propertycache import get_property_cache
31from lp.soyuz.interfaces.component import IComponentSet34from lp.soyuz.interfaces.component import IComponentSet
32from lp.testing import (35from lp.testing import (
36 celebrity_logged_in,
33 person_logged_in,37 person_logged_in,
34 TestCaseWithFactory,38 TestCaseWithFactory,
35 )39 )
36from lp.testing._webservice import QueryCollector40from lp.testing._webservice import QueryCollector
37from lp.testing.matchers import HasQueryCount41from lp.testing.matchers import (
42 BrowsesWithQueryLimit,
43 HasQueryCount,
44 )
38from lp.testing.sampledata import (45from lp.testing.sampledata import (
39 ADMIN_EMAIL,46 ADMIN_EMAIL,
40 NO_PRIVILEGE_EMAIL,47 NO_PRIVILEGE_EMAIL,
@@ -45,7 +52,7 @@
4552
46class TestBugTaskView(TestCaseWithFactory):53class TestBugTaskView(TestCaseWithFactory):
4754
48 layer = DatabaseFunctionalLayer55 layer = LaunchpadFunctionalLayer
4956
50 def invalidate_caches(self, obj):57 def invalidate_caches(self, obj):
51 store = Store.of(obj)58 store = Store.of(obj)
@@ -84,6 +91,24 @@
84 LessThan(count_with_no_teams + 3),91 LessThan(count_with_no_teams + 3),
85 ))92 ))
8693
94 def test_rendered_query_counts_constant_with_attachments(self):
95 with celebrity_logged_in('admin'):
96 browses_under_limit = BrowsesWithQueryLimit(
97 71, self.factory.makePerson())
98
99 # First test with a single attachment.
100 task = self.factory.makeBugTask()
101 self.factory.makeBugAttachment(bug=task.bug)
102 self.assertThat(task, browses_under_limit)
103
104 with celebrity_logged_in('admin'):
105 # And now with 10.
106 task = self.factory.makeBugTask()
107 self.factory.makeBugTask(bug=task.bug)
108 for i in range(10):
109 self.factory.makeBugAttachment(bug=task.bug)
110 self.assertThat(task, browses_under_limit)
111
87 def test_interesting_activity(self):112 def test_interesting_activity(self):
88 # The interesting_activity property returns a tuple of interesting113 # The interesting_activity property returns a tuple of interesting
89 # `BugActivityItem`s.114 # `BugActivityItem`s.
90115
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-04-01 00:47:03 +0000
+++ lib/lp/bugs/model/bug.py 2011-04-05 06:57:34 +0000
@@ -87,7 +87,10 @@
87from canonical.launchpad.components.decoratedresultset import (87from canonical.launchpad.components.decoratedresultset import (
88 DecoratedResultSet,88 DecoratedResultSet,
89 )89 )
90from canonical.launchpad.database.librarian import LibraryFileAlias90from canonical.launchpad.database.librarian import (
91 LibraryFileAlias,
92 LibraryFileContent,
93 )
91from canonical.launchpad.database.message import (94from canonical.launchpad.database.message import (
92 Message,95 Message,
93 MessageChunk,96 MessageChunk,
@@ -1935,10 +1938,11 @@
1935 # See bug 542274 for more details.1938 # See bug 542274 for more details.
1936 store = Store.of(self)1939 store = Store.of(self)
1937 return store.find(1940 return store.find(
1938 (BugAttachment, LibraryFileAlias),1941 (BugAttachment, LibraryFileAlias, LibraryFileContent),
1939 BugAttachment.bug == self,1942 BugAttachment.bug == self,
1940 BugAttachment.libraryfile == LibraryFileAlias.id,1943 BugAttachment.libraryfileID == LibraryFileAlias.id,
1941 LibraryFileAlias.content != None).order_by(BugAttachment.id)1944 LibraryFileContent.id == LibraryFileAlias.contentID,
1945 ).order_by(BugAttachment.id)
19421946
1943 @property1947 @property
1944 def attachments(self):1948 def attachments(self):