Merge lp:~stevenk/launchpad/bugtask-activity-preload into lp:launchpad

[Steve Kowalik]

Pre-load persons for Bug:+activity.

Change Bug:+activity to bulk load the people from ValidPersonCache. With 10 BugActivity rows, the query count drops from 13 to 5, and with 50 rows the query count drops from 53 to 5. This also required adding a personID attribute onto IBugActivity.

[Ian Booth (wallyworld)]

Look good.

review: Approve

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2012-08-26 22:50:37 +0000
+++ lib/lp/bugs/browser/bug.py	2012-09-10 06:51:58 +0000
@@ -103,6 +103,7 @@
     get_structural_subscriptions_for_bug,
     )
 from lp.registry.enums import PRIVATE_INFORMATION_TYPES
+from lp.registry.interfaces.person import IPersonSet
 from lp.registry.vocabularies import InformationTypeVocabulary
 from lp.services.fields import DuplicateBug
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
@@ -605,6 +606,13 @@
 
     page_title = 'Activity log'
 
+    @property
+    def activity(self):
+        activity = IBug(self.context).activity
+        list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+            [a.personID for a in activity], need_validity=True))
+        return activity
+
 
 class BugSubscriptionPortletDetails:
     """A mixin used to collate bug subscription details for a view."""

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2012-09-06 00:01:38 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2012-09-10 06:51:58 +0000
@@ -5,20 +5,29 @@
 
 __metaclass__ = type
 
+from datetime import (
+    datetime,
+    timedelta,
+    )
+
 from BeautifulSoup import BeautifulSoup
+import pytz
 import simplejson
 from soupmatchers import (
     HTMLContains,
     Tag,
     )
+from storm.store import Store
 from testtools.matchers import (
     Contains,
+    Equals,
     MatchesAll,
     Not,
     )
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.bugs.adapters.bugchange import BugAttachmentChange
 from lp.registry.enums import (
     BugSharingPolicy,
     InformationType,
@@ -35,12 +44,14 @@
     BrowserTestCase,
     login_person,
     person_logged_in,
+    StormStatementRecorder,
     TestCaseWithFactory,
     )
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
+from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import find_tag_by_id
 from lp.testing.views import (
     create_initialized_view,
@@ -673,3 +684,26 @@
             'table',
             {'id': 'affected-software', 'class': 'listing'})
         self.assertIsNotNone(table)
+
+
+class TestBugActivityView(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def test_bug_activity_query_count(self):
+        # Bug:+activity doesn't make O(n) queries based on the amount of
+        # activity.
+        bug = self.factory.makeBug()
+        ten_minutes_ago = datetime.now(pytz.UTC) - timedelta(minutes=10)
+        with person_logged_in(bug.owner):
+            attachment = self.factory.makeBugAttachment(bug=bug)
+            for i in range(10):
+                bug.addChange(BugAttachmentChange(
+                    ten_minutes_ago, self.factory.makePerson(), 'attachment',
+                    None, attachment))
+        Store.of(bug).invalidate()
+        with StormStatementRecorder() as recorder:
+            view = create_initialized_view(
+                bug.default_bugtask, name='+activity')
+            view.render()
+        self.assertThat(recorder, HasQueryCount(Equals(7)))

=== modified file 'lib/lp/bugs/interfaces/bugactivity.py'
--- lib/lp/bugs/interfaces/bugactivity.py	2011-12-24 16:54:44 +0000
+++ lib/lp/bugs/interfaces/bugactivity.py	2012-09-10 06:51:58 +0000
@@ -16,7 +16,10 @@
     export_as_webservice_entry,
     exported,
     )
-from zope.interface import Interface
+from zope.interface import (
+    Attribute,
+    Interface,
+    )
 from zope.schema import (
     Datetime,
     Text,
@@ -42,6 +45,7 @@
                  description=_("The date on which this activity occurred."),
                  readonly=True))
 
+    personID = Attribute('DB ID for Person')
     person = exported(PersonChoice(
         title=_('Person'), required=True, vocabulary='ValidPersonOrTeam',
         readonly=True, description=_("The person's Launchpad ID or "

=== modified file 'lib/lp/bugs/templates/bug-activity.pt'
--- lib/lp/bugs/templates/bug-activity.pt	2010-10-10 21:54:16 +0000
+++ lib/lp/bugs/templates/bug-activity.pt	2012-09-10 06:51:58 +0000
@@ -20,7 +20,7 @@
             </tr>
           </thead>
           <tbody>
-            <tr tal:repeat="log context/bug/activity">
+            <tr tal:repeat="log view/activity">
               <tal:comment condition="nothing">
                 XXX: Gavin Panella 2009-08-12 bug=412963: Using strftime()
                 here because fmt:datetime changes timezone, even though we