William Grant has proposed merging lp:~wgrant/launchpad/bug-750949 into
lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #750949 in Launchpad itself: "BugTask:+index times out with lots of
attachments"
https://bugs.launchpad.net/launchpad/+bug/750949
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-750949/+merge/56295
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.
--
https://code.launchpad.net/~wgrant/launchpad/bug-750949/+merge/56295
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~wgrant/launchpad/bug-750949 into lp:launchpad.
=== 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 @@
da.clear_request_started()
+ getUtility(IOpenLaunchBag).clear()
+
# Maintain operational statistics.
if getattr(request, '_wants_retry', False):
OpStats.stats['retries'] += 1
=== 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 @@
usedfor = IBugAttachmentSet
+# Despite declaring compliance with ICanonicalUrlData, the LaunchBag
+# dependency means this tends towards the "not canonical at all" end of
+# the canonicalness scale. Beware.
class BugAttachmentURL:
"""Bug URL creation rules."""
implements(ICanonicalUrlData)
=== 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 @@
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.webapp import canonical_url
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
-from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.testing.layers import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
from lp.bugs.browser.bugtask import (
BugTaskEditView,
BugTasksAndNominationsView,
@@ -30,11 +33,15 @@
from lp.services.propertycache import get_property_cache
from lp.soyuz.interfaces.component import IComponentSet
from lp.testing import (
+ celebrity_logged_in,
person_logged_in,
TestCaseWithFactory,
)
from lp.testing._webservice import QueryCollector
-from lp.testing.matchers import HasQueryCount
+from lp.testing.matchers import (
+ BrowsesWithQueryLimit,
+ HasQueryCount,
+ )
from lp.testing.sampledata import (
ADMIN_EMAIL,
NO_PRIVILEGE_EMAIL,
@@ -45,7 +52,7 @@
class TestBugTaskView(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def invalidate_caches(self, obj):
store = Store.of(obj)
@@ -84,6 +91,24 @@
LessThan(count_with_no_teams + 3),
))
+ def test_rendered_query_counts_constant_with_attachments(self):
+ with celebrity_logged_in('admin'):
+ browses_under_limit = BrowsesWithQueryLimit(
+ 71, self.factory.makePerson())
+
+ # First test with a single attachment.
+ task = self.factory.makeBugTask()
+ self.factory.makeBugAttachment(bug=task.bug)
+ self.assertThat(task, browses_under_limit)
+
+ with celebrity_logged_in('admin'):
+ # And now with 10.
+ task = self.factory.makeBugTask()
+ self.factory.makeBugTask(bug=task.bug)
+ for i in range(10):
+ self.factory.makeBugAttachment(bug=task.bug)
+ self.assertThat(task, browses_under_limit)
+
def test_interesting_activity(self):
# The interesting_activity property returns a tuple of interesting
# `BugActivityItem`s.
=== 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 @@
from canonical.launchpad.components.decoratedresultset import (
DecoratedResultSet,
)
-from canonical.launchpad.database.librarian import LibraryFileAlias
+from canonical.launchpad.database.librarian import (
+ LibraryFileAlias,
+ LibraryFileContent,
+ )
from canonical.launchpad.database.message import (
Message,
MessageChunk,
@@ -1935,10 +1938,11 @@
# See bug 542274 for more details.
store = Store.of(self)
return store.find(
- (BugAttachment, LibraryFileAlias),
+ (BugAttachment, LibraryFileAlias, LibraryFileContent),
BugAttachment.bug == self,
- BugAttachment.libraryfile == LibraryFileAlias.id,
- LibraryFileAlias.content != None).order_by(BugAttachment.id)
+ BugAttachment.libraryfileID == LibraryFileAlias.id,
+ LibraryFileContent.id == LibraryFileAlias.contentID,
+ ).order_by(BugAttachment.id)
@property
def attachments(self):
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
