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

[Robert Collins]

[r=lifeless][ui=none] [r=lifeless][bug=722956] Eager load productseries and suite series for the api /branches collection.

The API for /branches is nearly guaranteed to have branches from many different contexts, and we were doing late evaluation on many attributes trying to render the collection. This fixes 2 of those cases and lays the ground work to fix more when the page becomes an issue again.

[Stuart Bishop (stub)]

The new eager_load parameter is exposed to the webservice. What happens when web service requests use this parameter? This seems a quick hack rather than an API improvement.

I suspect we need a new method (that is not exposed to the webservice) rather than adding an optional parameter to the existing method. Or cast the result set to an eager result set (results = IEagerResultSet(foo.getBranches())). Or just a helper (results = foo.getBranches(); eager_load_branches(results)).

Its annoying that the seemingly arbitrary and undocumented ordering in associatedSuiteSourcePackages needs to be duplicated in the eager load method. We should comment this code dependency for when someone wants to change it.

review: Needs Information

[Robert Collins (lifeless)]

On Mon, Feb 28, 2011 at 9:27 PM, Stuart Bishop
<email address hidden> wrote:
> Review: Needs Information
> The new eager_load parameter is exposed to the webservice. What happens when web service requests use this parameter? This seems a quick hack rather than an API improvement.

Its not exposed to the webservice. The getBranches parameters in the
web service is controlled via the @operation_parameters decorator on
IHasBranches. The variant that I have enabled eager loading for is
only exposed as a collection in the web service: setting a default
parameter lets the method be used in code, and used as a collection in
the webservice, with the default being what the webservice needs.

> I suspect we need a new method (that is not exposed to the webservice) rather than adding an optional parameter to the existing method. Or cast the result set to an eager result set (results = IEagerResultSet(foo.getBranches())). Or just a helper (results = foo.getBranches(); eager_load_branches(results)).

I don't think any of those are needed, because my change isn't exposed
as part of the web service contract; its invisible.

> Its annoying that the seemingly arbitrary and undocumented ordering in associatedSuiteSourcePackages needs to be duplicated in the eager load method. We should comment this code dependency for when someone wants to change it.

I will comment on both sides of the eager load to point this out.

Thanks for the review,
Rob

[Robert Collins (lifeless)]

I've addressed Stuarts comments; approving to make ec2land happy.

review: Approve

=== modified file 'lib/lp/app/doc/tales.txt'
--- lib/lp/app/doc/tales.txt	2011-02-03 17:59:50 +0000
+++ lib/lp/app/doc/tales.txt	2011-03-01 04:23:03 +0000
@@ -498,6 +498,8 @@
 
     >>> login_person(fooix.owner, LaunchpadTestRequest())
     >>> fooix.development_focus.branch = branch
+    >>> from lp.services.propertycache import clear_property_cache
+    >>> clear_property_cache(branch)
     >>> print test_tales("branch/fmt:bzr-link", branch=branch)
     <a href=".../~eric/fooix/bar" class="sprite branch">lp://dev/fooix</a>
 

=== modified file 'lib/lp/app/widgets/suggestion.py'
--- lib/lp/app/widgets/suggestion.py	2011-02-08 15:43:30 +0000
+++ lib/lp/app/widgets/suggestion.py	2011-03-01 04:23:03 +0000
@@ -231,7 +231,9 @@
         collection = branch.target.collection.targetedBy(logged_in_user,
             since)
         collection = collection.visibleByUser(logged_in_user)
-        branches = collection.getBranches().config(distinct=True)
+        # May actually need some eager loading, but the API isn't fine grained
+        # yet.
+        branches = collection.getBranches(eager_load=False).config(distinct=True)
         target_branches = list(branches.config(limit=5))
         # If there is a development focus branch, make sure it is always
         # shown, and as the first item.

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2011-02-11 15:09:53 +0000
+++ lib/lp/code/browser/branchlisting.py	2011-03-01 04:23:03 +0000
@@ -650,7 +650,7 @@
         if lifecycle_status is not None:
             collection = collection.withLifecycleStatus(*lifecycle_status)
         collection = collection.visibleByUser(self.user)
-        return collection.getBranches().order_by(
+        return collection.getBranches(eager_load=False).order_by(
             self._listingSortToOrderBy(self.sort_by))
 
     @property
@@ -809,7 +809,7 @@
         if lifecycle_status is not None:
             collection = collection.withLifecycleStatus(*lifecycle_status)
         collection = collection.visibleByUser(self.user)
-        return collection.getBranches().order_by(
+        return collection.getBranches(eager_load=False).order_by(
             self._branch_order)
 
 
@@ -1501,7 +1501,7 @@
         # We're only interested in active branches.
         collection = self.getBranchCollection().withLifecycleStatus(
             *DEFAULT_BRANCH_STATUS_IN_LISTING)
-        for branch in collection.getBranches():
+        for branch in collection.getBranches(eager_load=False):
             branches.setdefault(branch.distroseries, []).append(branch)
         return branches
 

=== modified file 'lib/lp/code/feed/branch.py'
--- lib/lp/code/feed/branch.py	2010-08-24 10:45:57 +0000
+++ lib/lp/code/feed/branch.py	2011-03-01 04:23:03 +0000
@@ -163,7 +163,7 @@
         from lp.code.model.branch import Branch
         collection = self._getCollection().visibleByUser(
             None).withLifecycleStatus(*DEFAULT_BRANCH_STATUS_IN_LISTING)
-        branches = collection.getBranches()
+        branches = collection.getBranches(eager_load=False)
         branches.order_by(
             Desc(Branch.date_last_modified),
             Asc(Branch.target_suffix),

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2011-02-28 14:08:54 +0000
+++ lib/lp/code/interfaces/branch.py	2011-03-01 04:23:03 +0000
@@ -708,7 +708,10 @@
         """
 
     def associatedSuiteSourcePackages():
-        """Return the suite source packages that this branch is linked to."""
+        """Return the suite source packages that this branch is linked to.
+        
+        :return: A list of suite source packages ordered by pocket.
+        """
 
     def branchLinks():
         """Return a sorted list of ICanHasLinkedBranch objects.
@@ -1286,8 +1289,13 @@
         """
 
     @collection_default_content()
-    def getBranches(limit=50):
-        """Return a collection of branches."""
+    def getBranches(limit=50, eager_load=True):
+        """Return a collection of branches.
+        
+        :param eager_load: If True (the default because this is used in the 
+            web service and it needs the related objects to create links) eager
+            load related objects (products, code imports etc).
+        """
 
 
 class IBranchListingQueryOptimiser(Interface):

=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py	2011-01-20 00:06:17 +0000
+++ lib/lp/code/interfaces/branchcollection.py	2011-03-01 04:23:03 +0000
@@ -57,13 +57,16 @@
             of individuals and teams that own branches in this collection.
         """
 
-    def getBranches():
+    def getBranches(eager_load=False):
         """Return a result set of all branches in this collection.
 
         The returned result set will also join across the specified tables as
         defined by the arguments to this function.  These extra tables are
         joined specificly to allow the caller to sort on values not in the
         Branch table itself.
+
+        :param eager_load: If True trigger eager loading of all the related
+            objects in the collection.
         """
 
     def getMergeProposals(statuses=None, for_branches=None,

=== modified file 'lib/lp/code/interfaces/branchnamespace.py'
--- lib/lp/code/interfaces/branchnamespace.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/interfaces/branchnamespace.py	2011-03-01 04:23:03 +0000
@@ -52,8 +52,11 @@
         a given prefix, use createBranchWithPrefix.
         """
 
-    def getBranches():
-        """Return the branches in this namespace."""
+    def getBranches(eager_load=False):
+        """Return the branches in this namespace.
+        
+        :param eager_load: If True eager load related data for the branches.
+        """
 
     def getBranchName(name):
         """Get the potential unique name for a branch called 'name'.

=== modified file 'lib/lp/code/interfaces/hasbranches.py'
--- lib/lp/code/interfaces/hasbranches.py	2010-10-27 07:15:02 +0000
+++ lib/lp/code/interfaces/hasbranches.py	2011-03-01 04:23:03 +0000
@@ -60,13 +60,15 @@
     @operation_returns_collection_of(Interface) # Really IBranch.
     @export_read_operation()
     def getBranches(status=None, visible_by_user=None,
-                    modified_since=None):
+                    modified_since=None, eager_load=False):
         """Returns all branches with the given lifecycle status.
 
         :param status: A list of statuses to filter with.
         :param visible_by_user: Normally the user who is asking.
         :param modified_since: If set, filters the branches being returned
             to those that have been modified since the specified date/time.
+        :param eager_load: If True load related objects for the whole
+            collection.
         :returns: A list of `IBranch`.
         """
 

=== modified file 'lib/lp/code/interfaces/seriessourcepackagebranch.py'
--- lib/lp/code/interfaces/seriessourcepackagebranch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/interfaces/seriessourcepackagebranch.py	2011-03-01 04:23:03 +0000
@@ -65,6 +65,13 @@
         :return: An `IResultSet` of `ISeriesSourcePackageBranch` objects.
         """
 
+    def findForBranches(branches):
+        """Get the links to source packages from a branch.
+
+        :param branches: A an iterable of `IBranch`.
+        :return: An `IResultSet` of `ISeriesSourcePackageBranch` objects.
+        """
+
     def findForSourcePackage(sourcepackage):
         """Get the links to branches from a source package.
 

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2011-02-27 19:45:44 +0000
+++ lib/lp/code/model/branch.py	2011-03-01 04:23:03 +0000
@@ -142,6 +142,7 @@
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.model.job import Job
 from lp.services.mail.notificationrecipientset import NotificationRecipientSet
+from lp.services.propertycache import cachedproperty
 
 
 class Branch(SQLBase, BzrIdentityMixin):
@@ -724,13 +725,19 @@
             DeleteCodeImport(self.code_import)()
         Store.of(self).flush()
 
+    @cachedproperty
+    def _associatedProductSeries(self):
+        """Helper for eager loading associatedProductSeries."""
+        # This is eager loaded by BranchCollection.getBranches.
+        # Imported here to avoid circular import.
+        from lp.registry.model.productseries import ProductSeries
+        return Store.of(self).find(
+            ProductSeries,
+            ProductSeries.branch == self)
+
     def associatedProductSeries(self):
         """See `IBranch`."""
-        # Imported here to avoid circular import.
-        from lp.registry.model.productseries import ProductSeries
-        return Store.of(self).find(
-            ProductSeries,
-            ProductSeries.branch == self)
+        return self._associatedProductSeries
 
     def getProductSeriesPushingTranslations(self):
         """See `IBranch`."""
@@ -740,14 +747,21 @@
             ProductSeries,
             ProductSeries.translations_branch == self)
 
-    def associatedSuiteSourcePackages(self):
-        """See `IBranch`."""
+    @cachedproperty
+    def _associatedSuiteSourcePackages(self):
+        """Helper for associatedSuiteSourcePackages."""
+        # This is eager loaded by BranchCollection.getBranches.
         series_set = getUtility(IFindOfficialBranchLinks)
-        # Order by the pocket to get the release one first.
+        # Order by the pocket to get the release one first. If changing this be
+        # sure to also change BranchCollection.getBranches.
         links = series_set.findForBranch(self).order_by(
             SeriesSourcePackageBranch.pocket)
         return [link.suite_sourcepackage for link in links]
 
+    def associatedSuiteSourcePackages(self):
+        """See `IBranch`."""
+        return self._associatedSuiteSourcePackages
+
     # subscriptions
     def subscribe(self, person, notification_level, max_diff_lines,
                   code_review_level, subscribed_by):
@@ -1311,7 +1325,8 @@
         branches = all_branches.visibleByUser(
             visible_by_user).withLifecycleStatus(*lifecycle_statuses)
         branches = branches.withBranchType(
-            BranchType.HOSTED, BranchType.MIRRORED).scanned().getBranches()
+            BranchType.HOSTED, BranchType.MIRRORED).scanned().getBranches(
+                eager_load=False)
         branches.order_by(
             Desc(Branch.date_last_modified), Desc(Branch.id))
         if branch_count is not None:
@@ -1327,7 +1342,7 @@
         branches = all_branches.visibleByUser(
             visible_by_user).withLifecycleStatus(*lifecycle_statuses)
         branches = branches.withBranchType(
-            BranchType.IMPORTED).scanned().getBranches()
+            BranchType.IMPORTED).scanned().getBranches(eager_load=False)
         branches.order_by(
             Desc(Branch.date_last_modified), Desc(Branch.id))
         if branch_count is not None:
@@ -1341,7 +1356,8 @@
         """See `IBranchSet`."""
         all_branches = getUtility(IAllBranches)
         branches = all_branches.withLifecycleStatus(
-            *lifecycle_statuses).visibleByUser(visible_by_user).getBranches()
+            *lifecycle_statuses).visibleByUser(visible_by_user).getBranches(
+                eager_load=False)
         branches.order_by(
             Desc(Branch.date_created), Desc(Branch.id))
         if branch_count is not None:
@@ -1360,10 +1376,10 @@
         """See `IBranchSet`."""
         return getUtility(IBranchLookup).getByUrls(urls)
 
-    def getBranches(self, limit=50):
+    def getBranches(self, limit=50, eager_load=True):
         """See `IBranchSet`."""
         anon_branches = getUtility(IAllBranches).visibleByUser(None)
-        branches = anon_branches.scanned().getBranches()
+        branches = anon_branches.scanned().getBranches(eager_load=eager_load)
         branches.order_by(
             Desc(Branch.date_last_modified), Desc(Branch.id))
         branches.config(limit=limit)

=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py	2011-01-27 14:02:38 +0000
+++ lib/lp/code/model/branchcollection.py	2011-03-01 04:23:03 +0000
@@ -23,20 +23,26 @@
 from zope.component import getUtility
 from zope.interface import implements
 
+from canonical.launchpad.components.decoratedresultset import (
+    DecoratedResultSet,
+    )
 from canonical.launchpad.webapp.interfaces import (
     DEFAULT_FLAVOR,
     IStoreSelector,
     MAIN_STORE,
     )
 from canonical.launchpad.webapp.vocabulary import CountableIterator
+from canonical.lazr.utils import safe_hasattr
+from lp.bugs.model.bug import Bug
+from lp.bugs.model.bugbranch import BugBranch
 from lp.code.interfaces.branch import user_has_special_branch_access
 from lp.code.interfaces.branchcollection import (
     IBranchCollection,
     InvalidFilter,
     )
-
-from lp.bugs.model.bug import Bug
-from lp.bugs.model.bugbranch import BugBranch
+from lp.code.interfaces.seriessourcepackagebranch import (
+    IFindOfficialBranchLinks,
+    )
 from lp.code.enums import BranchMergeProposalStatus
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES
@@ -55,6 +61,7 @@
 from lp.registry.model.product import Product
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.registry.model.teammembership import TeamParticipation
+from lp.services.propertycache import get_property_cache
 
 
 class GenericBranchCollection:
@@ -89,7 +96,7 @@
 
     def count(self):
         """See `IBranchCollection`."""
-        return self.getBranches().count()
+        return self.getBranches(eager_load=False).count()
 
     def ownerCounts(self):
         """See `IBranchCollection`."""
@@ -136,7 +143,7 @@
 
     def _getBranchIdQuery(self):
         """Return a Storm 'Select' for the branch IDs in this collection."""
-        select = self.getBranches()._get_select()
+        select = self.getBranches(eager_load=False)._get_select()
         select.columns = (Branch.id,)
         return select
 
@@ -144,11 +151,43 @@
         """Return the where expressions for this collection."""
         return self._branch_filter_expressions
 
-    def getBranches(self):
+    def getBranches(self, eager_load=False):
         """See `IBranchCollection`."""
         tables = [Branch] + self._tables.values()
         expressions = self._getBranchExpressions()
-        return self.store.using(*tables).find(Branch, *expressions)
+        resultset = self.store.using(*tables).find(Branch, *expressions)
+        if not eager_load:
+            return resultset
+        def do_eager_load(rows):
+            branch_ids = set(branch.id for branch in rows)
+            if not branch_ids:
+                return
+            branches = dict((branch.id, branch) for branch in rows)
+            caches = dict((branch.id, get_property_cache(branch))
+                for branch in rows)
+            for cache in caches.values():
+                if not safe_hasattr(cache, '_associatedProductSeries'):
+                    cache._associatedProductSeries = []
+                if not safe_hasattr(cache, '_associatedSuiteSourcePackages'):
+                    cache._associatedSuiteSourcePackages = []
+            # associatedProductSeries
+            # Imported here to avoid circular import.
+            from lp.registry.model.productseries import ProductSeries
+            for productseries in self.store.find(
+                ProductSeries,
+                ProductSeries.branchID.is_in(branch_ids)):
+                cache = caches[productseries.branchID]
+                cache._associatedProductSeries.append(productseries)
+            # associatedSuiteSourcePackages
+            series_set = getUtility(IFindOfficialBranchLinks)
+            # Order by the pocket to get the release one first. If changing
+            # this be sure to also change BranchCollection.getBranches.
+            links = series_set.findForBranches(rows).order_by(
+                SeriesSourcePackageBranch.pocket)
+            for link in links:
+                cache = caches[link.branchID]
+                cache._associatedSuiteSourcePackages.append(link)
+        return DecoratedResultSet(resultset, pre_iter_hook=do_eager_load)
 
     def getMergeProposals(self, statuses=None, for_branches=None,
                           target_branch=None, merged_revnos=None):
@@ -364,7 +403,7 @@
         # of the unique name and sort based on relevance.
         branch = self._getExactMatch(search_term)
         if branch is not None:
-            if branch in self.getBranches():
+            if branch in self.getBranches(eager_load=False):
                 return CountableIterator(1, [branch])
             else:
                 return CountableIterator(0, [])
@@ -397,7 +436,8 @@
 
         # Get the results.
         collection = self._filterBy([Branch.id.is_in(Union(*queries))])
-        results = collection.getBranches().order_by(Branch.name, Branch.id)
+        results = collection.getBranches(eager_load=False).order_by(
+            Branch.name, Branch.id)
         return CountableIterator(results.count(), results)
 
     def scanned(self):

=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2010-10-18 02:37:53 +0000
+++ lib/lp/code/model/branchnamespace.py	2011-03-01 04:23:03 +0000
@@ -218,7 +218,7 @@
             name = "%s-%s" % (prefix, count)
         return name
 
-    def getBranches(self):
+    def getBranches(self, eager_load=False):
         """See `IBranchNamespace`."""
         store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
         return store.find(Branch, self._getBranchesClause())

=== modified file 'lib/lp/code/model/hasbranches.py'
--- lib/lp/code/model/hasbranches.py	2010-10-27 07:15:02 +0000
+++ lib/lp/code/model/hasbranches.py	2011-03-01 04:23:03 +0000
@@ -26,7 +26,7 @@
     """A mixin implementation for `IHasBranches`."""
 
     def getBranches(self, status=None, visible_by_user=None,
-                    modified_since=None):
+                    modified_since=None, eager_load=False):
         """See `IHasBranches`."""
         if status is None:
             status = DEFAULT_BRANCH_STATUS_IN_LISTING
@@ -35,7 +35,7 @@
         collection = collection.withLifecycleStatus(*status)
         if modified_since is not None:
             collection = collection.modifiedSince(modified_since)
-        return collection.getBranches()
+        return collection.getBranches(eager_load=eager_load)
 
 
 class HasMergeProposalsMixin:

=== modified file 'lib/lp/code/model/seriessourcepackagebranch.py'
--- lib/lp/code/model/seriessourcepackagebranch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/seriessourcepackagebranch.py	2011-03-01 04:23:03 +0000
@@ -101,10 +101,15 @@
 
     def findForBranch(self, branch):
         """See `IFindOfficialBranchLinks`."""
+        return self.findForBranches([branch])
+
+    def findForBranches(self, branches):
+        """See `IFindOfficialBranchLinks`."""
+        branch_ids = set(branch.id for branch in branches)
         store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
         return store.find(
             SeriesSourcePackageBranch,
-            SeriesSourcePackageBranch.branch == branch.id)
+            SeriesSourcePackageBranch.branchID.is_in(branch_ids))
 
     def findForSourcePackage(self, sourcepackage):
         """See `IFindOfficialBranchLinks`."""

=== modified file 'lib/lp/code/model/tests/test_branchset.py'
--- lib/lp/code/model/tests/test_branchset.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/model/tests/test_branchset.py	2011-03-01 04:23:03 +0000
@@ -5,32 +5,34 @@
 
 __metaclass__ = type
 
-from unittest import TestLoader
+from testtools.matchers import LessThan
 
+from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.code.interfaces.branch import IBranchSet
 from lp.code.model.branch import BranchSet
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    logout,
+    TestCaseWithFactory,
+    )
+from lp.testing._webservice import QueryCollector
+from lp.testing.matchers import HasQueryCount
 
 
 class TestBranchSet(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
-    def setUp(self):
-        TestCaseWithFactory.setUp(self)
-        self.branch_set = BranchSet()
-
     def test_provides_IBranchSet(self):
         # BranchSet instances provide IBranchSet.
-        self.assertProvides(self.branch_set, IBranchSet)
+        self.assertProvides(BranchSet(), IBranchSet)
 
     def test_getByUrls(self):
         # getByUrls returns a list of branches matching the list of URLs that
         # it's given.
         a = self.factory.makeAnyBranch()
         b = self.factory.makeAnyBranch()
-        branches = self.branch_set.getByUrls(
+        branches = BranchSet().getByUrls(
             [a.bzr_identity, b.bzr_identity])
         self.assertEqual({a.bzr_identity: a, b.bzr_identity: b}, branches)
 
@@ -38,9 +40,21 @@
         # If a branch cannot be found for a URL, then None appears in the list
         # in place of the branch.
         url = 'http://example.com/doesntexist'
-        branches = self.branch_set.getByUrls([url])
+        branches = BranchSet().getByUrls([url])
         self.assertEqual({url: None}, branches)
 
-
-def test_suite():
-    return TestLoader().loadTestsFromName(__name__)
+    def test_api_branches_query_count(self):
+        webservice = LaunchpadWebServiceCaller()
+        collector = QueryCollector()
+        collector.register()
+        self.addCleanup(collector.unregister)
+        # Get 'all' of the 50 branches this collection is limited to - rather
+        # than the default in-test-suite pagination size of 5.
+        url = "/branches?ws.size=50"
+        logout()
+        response = webservice.get(url,
+            headers={'User-Agent': 'AnonNeedsThis'})
+        self.assertEqual(response.status, 200,
+            "Got %d for url %r with response %r" % (
+            response.status, url, response.body))
+        self.assertThat(collector, HasQueryCount(LessThan(13)))

=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py	2011-01-31 02:12:30 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py	2011-03-01 04:23:03 +0000
@@ -53,7 +53,7 @@
         # branches related to the branch target.
         self.assertEqual(self.target.collection.getBranches().count(), 0)
         branch = self.makeBranchForTarget()
-        branches = self.target.collection.getBranches()
+        branches = self.target.collection.getBranches(eager_load=False)
         self.assertEqual([branch], list(branches))
 
     def test_retargetBranch_packageBranch(self):

=== modified file 'lib/lp/code/tests/test_helpers.py'
--- lib/lp/code/tests/test_helpers.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/tests/test_helpers.py	2011-03-01 04:23:03 +0000
@@ -35,6 +35,7 @@
         self.assertIsNot(None, fooix)
         # There should be one branch with one commit.
         [branch] = list(
-            getUtility(IAllBranches).inProduct(fooix).getBranches())
+            getUtility(IAllBranches).inProduct(fooix).getBranches(
+                eager_load=False))
         self.assertEqual(1, branch.revision_count)
         self.assertEqual(commit_time, branch.getTipRevision().revision_date)

=== modified file 'lib/lp/code/vocabularies/branch.py'
--- lib/lp/code/vocabularies/branch.py	2010-12-01 18:58:44 +0000
+++ lib/lp/code/vocabularies/branch.py	2011-03-01 04:23:03 +0000
@@ -69,7 +69,7 @@
         logged_in_user = getUtility(ILaunchBag).user
         collection = self._getCollection().visibleByUser(logged_in_user)
         if query is None:
-            branches = collection.getBranches()
+            branches = collection.getBranches(eager_load=False)
         else:
             branches = collection.search(query)
         return CountableIterator(branches.count(), branches, self.toTerm)

=== modified file 'lib/lp/codehosting/branchdistro.py'
--- lib/lp/codehosting/branchdistro.py	2010-11-05 14:17:11 +0000
+++ lib/lp/codehosting/branchdistro.py	2011-03-01 04:23:03 +0000
@@ -146,7 +146,8 @@
         """
         branches = getUtility(IAllBranches)
         distroseries_branches = branches.inDistroSeries(self.old_distroseries)
-        return distroseries_branches.officialBranches().getBranches()
+        return distroseries_branches.officialBranches().getBranches(
+            eager_load=False)
 
     def checkConsistentOfficialPackageBranch(self, db_branch):
         """Check that `db_branch` is a consistent official package branch.

=== modified file 'lib/lp/codehosting/scanner/mergedetection.py'
--- lib/lp/codehosting/scanner/mergedetection.py	2010-10-08 20:25:27 +0000
+++ lib/lp/codehosting/scanner/mergedetection.py	2011-03-01 04:23:03 +0000
@@ -97,7 +97,7 @@
         BranchLifecycleStatus.DEVELOPMENT,
         BranchLifecycleStatus.EXPERIMENTAL,
         BranchLifecycleStatus.MATURE,
-        BranchLifecycleStatus.ABANDONED).getBranches()
+        BranchLifecycleStatus.ABANDONED).getBranches(eager_load=False)
     for branch in branches:
         last_scanned = branch.last_scanned_id
         # If the branch doesn't have any revisions, not any point setting