Merge lp:~thumper/launchpad/branch-index-slowness into lp:launchpad

[Tim Penhey]

Fix time-outs on the branch index page for branches with lots of linked bugs.

This branch provides a DecoratedBranch object to cache database queries. An extra method is added to the branch class to get linked bugs and all their bug tasks.

As a part of this branch, only bugs in an incomplete state are shown on series branches.

[Paul Hummer (rockstar)]

<rockstar> thumper, line 150 of your diff, why not use isinstance?
<thumper> rockstar: because I didn't think of it :)
<rockstar> thumper, okay. Can you remedy that?
<thumper> yep
<rockstar> thumper, do you have some data to go with this branch that might be good to go to the list?
<thumper> rockstar: what are you asking exactly
<rockstar> thumper, I think query count numbers along with the changes in this branch would be good to post to the list.
<thumper> rockstar: I'll be writing something up, yes

review: Approve (code)

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2010-05-30 04:06:48 +0000
+++ lib/lp/code/browser/branch.py	2010-06-11 05:08:31 +0000
@@ -25,13 +25,16 @@
     'BranchURL',
     'BranchView',
     'BranchSubscriptionsView',
+    'DecoratedBranch',
     'DecoratedBug',
     'RegisterBranchMergeProposalView',
     'TryImportAgainView',
     ]
 
 import cgi
+from collections import defaultdict
 from datetime import datetime, timedelta
+from operator import attrgetter
 
 import pytz
 import simplejson
@@ -78,6 +81,7 @@
 from canonical.widgets.lazrjs import vocabulary_to_choice_edit_items
 
 from lp.bugs.interfaces.bug import IBug
+from lp.bugs.interfaces.bugtask import UNRESOLVED_BUGTASK_STATUSES
 from lp.code.browser.branchref import BranchRef
 from lp.code.browser.branchmergeproposal import (
     latest_proposals_for_each_branch)
@@ -90,7 +94,7 @@
     CodeImportAlreadyRequested, CodeImportAlreadyRunning,
     CodeImportNotInReviewedState, InvalidBranchMergeProposal)
 from lp.code.interfaces.branch import (
-    BranchCreationForbidden, BranchExists, IBranch,
+    BranchCreationForbidden, BranchExists, BzrIdentityMixin, IBranch,
     user_has_special_branch_access)
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
 from lp.code.interfaces.branchtarget import IBranchTarget
@@ -327,17 +331,103 @@
 
 class DecoratedBug:
     """Provide some additional attributes to a normal bug."""
-    delegates(IBug)
+    delegates(IBug, 'bug')
 
-    def __init__(self, context, branch):
-        self.context = context
+    def __init__(self, bug, branch, tasks=None):
+        self.bug = bug
         self.branch = branch
+        self.tasks = tasks
+        if self.tasks is None:
+            self.tasks = self.bug.bugtasks
+
+    @property
+    def bugtasks(self):
+        return self.tasks
+
+    @property
+    def default_bugtask(self):
+        return self.tasks[0]
+
+    def getBugTask(self, target):
+        # Copied from Bug.getBugTarget to avoid importing.
+        for bugtask in self.bugtasks:
+            if bugtask.target == target:
+                return bugtask
+        return None
 
     @property
     def bugtask(self):
         """Return the bugtask for the branch project, or the default bugtask.
         """
-        return self.branch.target.getBugTask(self.context)
+        return self.branch.target.getBugTask(self)
+
+
+class DecoratedBranch(BzrIdentityMixin):
+    """Wrap a number of the branch accessors to cache results.
+
+    This avoids repeated db queries.
+    """
+    delegates(IBranch, 'branch')
+
+    def __init__(self, branch):
+        self.branch = branch
+
+    @cachedproperty
+    def linked_bugs(self):
+        bugs = defaultdict(list)
+        for bug, task in self.branch.getLinkedBugsAndTasks():
+            bugs[bug].append(task)
+        return [DecoratedBug(bug, self.branch, tasks)
+                for bug, tasks in bugs.iteritems()]
+
+    @property
+    def displayname(self):
+        return self.bzr_identity
+
+    @cachedproperty
+    def bzr_identity(self):
+        return super(DecoratedBranch, self).bzr_identity
+
+    @cachedproperty
+    def is_series_branch(self):
+        # True if linked to a product series or suite source package.
+        return (
+            len(self.associated_product_series) > 0 or
+            len(self.suite_source_packages) > 0)
+
+    def associatedProductSeries(self):
+        """Override the IBranch.associatedProductSeries."""
+        return self.associated_product_series
+
+    def associatedSuiteSourcePackages(self):
+        """Override the IBranch.associatedSuiteSourcePackages."""
+        return self.suite_source_packages
+
+    @cachedproperty
+    def associated_product_series(self):
+        return list(self.branch.associatedProductSeries())
+
+    @cachedproperty
+    def suite_source_packages(self):
+        return list(self.branch.associatedSuiteSourcePackages())
+
+    @cachedproperty
+    def upgrade_pending(self):
+        return self.branch.upgrade_pending
+
+    @cachedproperty
+    def subscriptions(self):
+        return list(self.branch.subscriptions)
+
+    def hasSubscription(self, user):
+        for sub in self.subscriptions:
+            if sub.person == user:
+                return True
+        return False
+
+    @cachedproperty
+    def latest_revisions(self):
+        return list(self.branch.latest_revisions())
 
 
 class BranchView(LaunchpadView, FeedsMixin):
@@ -356,6 +446,10 @@
 
     def initialize(self):
         self.notices = []
+        # Replace our context with a decorated branch, if it is not already
+        # decorated.
+        if not isinstance(self.context, DecoratedBranch):
+            self.context = DecoratedBranch(self.context)
 
     def user_is_subscribed(self):
         """Is the current user subscribed to this branch?"""
@@ -421,13 +515,6 @@
         else:
             return None
 
-    def edit_link_url(self):
-        """Target URL of the Edit link used in the actions portlet."""
-        # XXX: DavidAllouche 2005-12-02 bug=5313:
-        # That should go away when bug #5313 is fixed.
-        linkdata = BranchContextMenu(self.context).edit()
-        return '%s/%s' % (canonical_url(self.context), linkdata.target)
-
     @property
     def user_can_upload(self):
         """Whether the user can upload to this branch."""
@@ -517,8 +604,12 @@
     @cachedproperty
     def linked_bugs(self):
         """Return a list of DecoratedBugs linked to the branch."""
-        return [DecoratedBug(bug, self.context)
-            for bug in self.context.linked_bugs]
+        bugs = self.context.linked_bugs
+        if self.context.is_series_branch:
+            bugs = [
+                bug for bug in bugs
+                if bug.bugtask.status in UNRESOLVED_BUGTASK_STATUSES]
+        return bugs
 
     @cachedproperty
     def latest_code_import_results(self):

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2010-04-13 04:15:33 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2010-06-11 05:08:31 +0000
@@ -3,6 +3,8 @@
 
 """Unit tests for BranchView."""
 
+from __future__ import with_statement
+
 __metaclass__ = type
 __all__ = ['TestBranchView', 'test_suite']
 
@@ -21,6 +23,8 @@
 from canonical.database.constants import UTC_NOW
 
 from lp.app.interfaces.headings import IRootContext
+from lp.bugs.interfaces.bugtask import (
+    BugTaskStatus, UNRESOLVED_BUGTASK_STATUSES)
 from lp.code.browser.branch import (
     BranchAddView, BranchMirrorStatusView, BranchReviewerEditView,
     BranchSparkView, BranchView)
@@ -32,7 +36,8 @@
 from lp.registry.interfaces.product import IProductSet
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.testing import (
-    login, login_person, logout, ANONYMOUS, TestCaseWithFactory)
+    login, login_person, logout, person_logged_in, ANONYMOUS,
+    TestCaseWithFactory)
 from lp.testing.views import create_initialized_view
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing import (
@@ -261,6 +266,33 @@
         login_person(branch.owner)
         self.assertFalse(view.user_can_upload)
 
+    def _addBugLinks(self, branch):
+        for status in BugTaskStatus.items:
+            bug = self.factory.makeBug(status=status)
+            branch.linkBug(bug, branch.owner)
+
+    def test_linked_bugs(self):
+        # The linked bugs for a non series branch shows all linked bugs.
+        branch = self.factory.makeAnyBranch()
+        with person_logged_in(branch.owner):
+            self._addBugLinks(branch)
+        view = create_initialized_view(branch, '+index')
+        self.assertEqual(len(BugTaskStatus), len(view.linked_bugs))
+        self.assertFalse(view.context.is_series_branch)
+
+    def test_linked_bugs_series_branch(self):
+        # The linked bugs for a series branch shows only unresolved bugs.
+        product = self.factory.makeProduct()
+        branch = self.factory.makeProductBranch(product=product)
+        with person_logged_in(product.owner):
+            product.development_focus.branch = branch
+        with person_logged_in(branch.owner):
+            self._addBugLinks(branch)
+        view = create_initialized_view(branch, '+index')
+        for bug in view.linked_bugs:
+            self.assertTrue(
+                bug.bugtask.status in UNRESOLVED_BUGTASK_STATUSES)
+
 
 class TestBranchAddView(TestCaseWithFactory):
     """Test the BranchAddView view."""

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-05-30 04:06:48 +0000
+++ lib/lp/code/configure.zcml	2010-06-11 05:08:31 +0000
@@ -467,6 +467,7 @@
                     revision_count
                     bug_branches
                     linked_bugs
+                    getLinkedBugsAndTasks
                     linkBug
                     unlinkBug
                     spec_links

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2010-05-25 03:13:25 +0000
+++ lib/lp/code/interfaces/branch.py	2010-06-11 05:08:31 +0000
@@ -609,6 +609,9 @@
         readonly=True,
         value_type=Reference(schema=Interface))) # Really IBug
 
+    def getLinkedBugsAndTasks():
+        """Return a result set for the bugs with their tasks."""
+
     @call_with(registrant=REQUEST_USER)
     @operation_parameters(
         bug=Reference(schema=Interface)) # Really IBug

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-05-28 09:04:16 +0000
+++ lib/lp/code/model/branch.py	2010-06-11 05:08:31 +0000
@@ -245,6 +245,17 @@
         'Bug', joinColumn='branch', otherColumn='bug',
         intermediateTable='BugBranch', orderBy='id')
 
+    def getLinkedBugsAndTasks(self):
+        """Return a result set for the bugs with their tasks."""
+        from lp.bugs.model.bug import Bug
+        from lp.bugs.model.bugbranch import BugBranch
+        from lp.bugs.model.bugtask import BugTask
+        return Store.of(self).find(
+            (Bug, BugTask),
+            BugBranch.branch == self,
+            BugBranch.bug == Bug.id,
+            BugTask.bug == Bug.id)
+
     def linkBug(self, bug, registrant):
         """See `IBranch`."""
         return bug.linkBranch(self, registrant)

=== modified file 'lib/lp/code/templates/branch-import-details.pt'
--- lib/lp/code/templates/branch-import-details.pt	2010-01-22 03:16:44 +0000
+++ lib/lp/code/templates/branch-import-details.pt	2010-06-11 05:08:31 +0000
@@ -2,7 +2,7 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu context/menu:context">
+  tal:define="context_menu view/context/menu:context">
 
   <tal:imported-branch tal:condition="context/branch_type/enumvalue:IMPORTED">
     <div id="import-details" tal:define="branch context;

=== modified file 'lib/lp/code/templates/branch-management.pt'
--- lib/lp/code/templates/branch-management.pt	2009-09-30 12:14:24 +0000
+++ lib/lp/code/templates/branch-management.pt	2010-06-11 05:08:31 +0000
@@ -2,7 +2,7 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu context/menu:context">
+  tal:define="context_menu view/context/menu:context">
 
   <div tal:condition="not: context/revision_count">
     <div tal:condition="context/branch_type/enumvalue:IMPORTED">

=== modified file 'lib/lp/code/templates/branch-metadata.pt'
--- lib/lp/code/templates/branch-metadata.pt	2010-02-17 01:37:22 +0000
+++ lib/lp/code/templates/branch-metadata.pt	2010-06-11 05:08:31 +0000
@@ -2,7 +2,7 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu context/menu:context">
+  tal:define="context_menu view/context/menu:context">
 
   <div class="two-column-list">
     <dl id="branch-format" tal:condition="context/branch_format">

=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
--- lib/lp/code/templates/branch-pending-merges.pt	2010-05-22 04:19:57 +0000
+++ lib/lp/code/templates/branch-pending-merges.pt	2010-06-11 05:08:31 +0000
@@ -2,7 +2,7 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu context/menu:context"
+  tal:define="context_menu view/context/menu:context"
   tal:condition="view/show_merge_links">
 
   <h3>Branch merges</h3>

=== modified file 'lib/lp/code/templates/branch-portlet-subscribers.pt'
--- lib/lp/code/templates/branch-portlet-subscribers.pt	2010-01-13 00:30:26 +0000
+++ lib/lp/code/templates/branch-portlet-subscribers.pt	2010-06-11 05:08:31 +0000
@@ -4,7 +4,7 @@
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
   class="portlet" id="portlet-subscribers">
   <div class="portletBody portletContent"
-       tal:define="context_menu context/menu:context">
+       tal:define="context_menu view/context/menu:context">
 
     <div>
       <div class="actions">

=== modified file 'lib/lp/code/templates/branch-recipes.pt'
--- lib/lp/code/templates/branch-recipes.pt	2010-05-22 02:59:19 +0000
+++ lib/lp/code/templates/branch-recipes.pt	2010-06-11 05:08:31 +0000
@@ -2,7 +2,7 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu context/menu:context"
+  tal:define="context_menu view/context/menu:context"
   id="related-bugs-and-blueprints">
 
   <h3>Related source package recipes</h3>

=== modified file 'lib/lp/code/templates/branch-related-bugs-specs.pt'
--- lib/lp/code/templates/branch-related-bugs-specs.pt	2010-05-25 04:37:27 +0000
+++ lib/lp/code/templates/branch-related-bugs-specs.pt	2010-06-11 05:08:31 +0000
@@ -2,7 +2,7 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu context/menu:context">
+  tal:define="context_menu view/context/menu:context">
 
   <h3>Related bugs</h3>
   <div id="buglinks" class="actions">

=== modified file 'lib/lp/code/templates/branch-revisions.pt'
--- lib/lp/code/templates/branch-revisions.pt	2009-09-08 21:19:07 +0000
+++ lib/lp/code/templates/branch-revisions.pt	2010-06-11 05:08:31 +0000
@@ -2,7 +2,7 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  tal:define="context_menu context/menu:context">
+  tal:define="context_menu view/context/menu:context">
 
   <p tal:condition="not:context/revision_count"
      tal:define="branch context">
@@ -10,7 +10,7 @@
   </p>
 
   <tal:history-available condition="context/revision_count"
-                         define="branch context;
+                         define="branch view/context;
                                  revisions branch/latest_revisions">
     <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-06-10 07:09:44 +0000
+++ lib/lp/testing/factory.py	2010-06-11 05:08:31 +0000
@@ -1092,7 +1092,8 @@
 
     def makeBug(self, product=None, owner=None, bug_watch_url=None,
                 private=False, date_closed=None, title=None,
-                date_created=None, description=None, comment=None):
+                date_created=None, description=None, comment=None,
+                status=None):
         """Create and return a new, arbitrary Bug.
 
         The bug returned uses default values where possible. See
@@ -1114,7 +1115,8 @@
             comment = self.getUniqueString()
         create_bug_params = CreateBugParams(
             owner, title, comment=comment, private=private,
-            datecreated=date_created, description=description)
+            datecreated=date_created, description=description,
+            status=status)
         create_bug_params.setBugTarget(product=product)
         bug = getUtility(IBugSet).createBug(create_bug_params)
         if bug_watch_url is not None: