Merge lp:~lifeless/launchpad/bug-787765 into lp:launchpad

[Robert Collins]

[r=allenap][bug=787765] Optimise expose_user_administered_teams_to_js.

expose_user_administered_teams_to_js is both slow and used many times. Its not meant to be slow, so I've slightly tuned it (set intersection is faster than list intersection - way faster with storm objects), and its not meant to be used many times so I've worked around that by short-circuiting it for now as a pragmatic tradeoff. I've filed a techdebt bug about the fact we're using it inappropriately, but fixing that is high, not critical - and likely to be seriously fiddly so I'm not attempting that now. I've run this past Gary as a preimp.

[Gavin Panella (allenap)]

review: Approve

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2011-05-17 15:41:09 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2011-05-26 22:20:31 +0000
@@ -431,33 +431,41 @@
 def expose_user_administered_teams_to_js(request, user, context,
         absoluteURL=absoluteURL):
     """Make the list of teams the user administers available to JavaScript."""
+    # XXX: Robert Collins workaround multiple calls making this cause timeouts:
+    # see bug 788510.
+    objects = IJSONRequestCache(request).objects
+    if 'administratedTeams' in objects:
+        return
     info = []
     api_request = IWebServiceClientRequest(request)
     is_distro = IDistribution.providedBy(context)
+    if is_distro:
+        # If the context is a distro AND a bug supervisor is set then we only
+        # allow subscriptions from members of the bug supervisor team.
+        bug_supervisor = context.bug_supervisor
+    else:
+        bug_supervisor = None
     if user is not None:
-        administrated_teams = user.administrated_teams
+        administrated_teams = set(user.administrated_teams)
         if administrated_teams:
             # Get this only if we need to.
-            membership = list(user.teams_participated_in)
-            for team in administrated_teams:
-                # If the user is not a member of the team itself, then
-                # skip it, because structural subscriptions and their
-                # filters can only be edited by the subscriber.
-                # This can happen if the user is an owner but not a member.
-                if not team in membership:
-                    continue
-                # If the context is a distro AND a bug supervisor is set
-                # AND the admininistered team is not a member of the bug
-                # supervisor team THEN skip it.
-                if (is_distro and context.bug_supervisor is not None and
-                    not team.inTeam(context.bug_supervisor)):
+            membership = set(user.teams_participated_in)
+            # Only consider teams the user is both in and administers:
+            #  If the user is not a member of the team itself, then
+            # skip it, because structural subscriptions and their
+            # filters can only be edited by the subscriber.
+            # This can happen if the user is an owner but not a member.
+            administers_and_in = membership.intersection(administrated_teams)
+            for team in administers_and_in:
+                if (bug_supervisor is not None and
+                    not team.inTeam(bug_supervisor)):
                     continue
                 info.append({
                     'link': absoluteURL(team, api_request),
                     'title': team.title,
                     'url': canonical_url(team),
                 })
-    IJSONRequestCache(request).objects['administratedTeams'] = info
+    objects['administratedTeams'] = info
 
 
 def expose_user_subscriptions_to_js(user, subscriptions, request,

=== modified file 'lib/lp/bugs/browser/tests/test_expose.py'
--- lib/lp/bugs/browser/tests/test_expose.py	2011-05-03 13:38:30 +0000
+++ lib/lp/bugs/browser/tests/test_expose.py	2011-05-26 22:20:31 +0000
@@ -44,7 +44,8 @@
     """
     implements(IWebServiceClientRequest, IJSONRequestCache)
 
-    objects = {}
+    def __init__(self):
+        self.objects = {}
 
 
 class FakeTeam: