Merge lp:~thumper/launchpad/package-branch-listing-speedup into lp:launchpad

[Tim Penhey]

Refactor the bzr_identity calculation.

This branch changes how bzr identities are worked out.

Instead of a chunk of logic sitting in a stand alone function, the algorithm
now says: work out the links a branch has, sort them, and pick the first.

tests:
  test_branchlisting.py
  TestBranchLinksAndIdentites
  TestBzrIdentity
  TestLinkedBranchSorting

qa:
  look at https://code.edge.launchpad.net/rhythmbox-ubuntuone-music-store
  and make sure it looks right

changes:

pre and mid impl calls with mwhudson

lib/lp/code/browser/branchlisting.py
 * this is mostly simplification
 * the BranchListingItem now inherits from BzrIdentityMixin, this is to have
   the 'self' variable pointing at the BranchListingItem rather than the
   delegated branch object. This allows the associatedProductSeries and
   associatedSuiteSourcePackages to be accessed to avoid database calls.
 * the is_dev_focus is no longer needed, so the methods to work it out are
   removed

lib/lp/code/browser/tests/test_branchlisting.py
 * adds a single test to make sure there are no extra queries executed
   to work out the bzr_identity of a BranchListingItem

lib/lp/code/configure.zcml
 * Makes the is_personal_branch, branchIdentities, and branchLinks
   methods available through IBranch

lib/lp/code/interfaces/branch.py
 * Removes the bazaar_identity
 * Adds the BzrIdentityMixin

lib/lp/code/interfaces/linkedbranch.py
 * Makes the context accessible from the object.

lib/lp/code/model/branch.py
 * Implements the is_personal_branch and adds in the bzr identity mixin class.
 * The bzr_identity function is removed as it is now provided by the mixin.

lib/lp/code/model/linkedbranch.py
 * Link objects are now sortable, so the most 'important' links appear first.

lib/lp/code/model/tests/test_branch.py
lib/lp/code/model/tests/test_linkedbranch.py
 * implement the tests mentioned above

lib/lp/testing/factory.py
 * There were many places where a string was being passed in for a
   sourcepackagename. This was then being passed around as a
   sourcepackagename object, which it isn't. These changes add the
   ability to pass in a string for the sourcepackagename, and the
   correct object will be created.

[Michael Hudson-Doyle (mwhudson)]

Overall, yay, this branch makes things simpler and more correct.

The mixin is still a bit odd, but hopefully noone will have to touch
this code for a few years now :-)

Just a few comments below, mostly simple clarity stuff. Thanks for
fixing the icky bug!

Cheers,
mwh

> === modified file 'lib/lp/code/interfaces/branch.py'
> --- lib/lp/code/interfaces/branch.py 2010-04-06 17:19:23 +0000
> +++ lib/lp/code/interfaces/branch.py 2010-04-08 04:14:00 +0000
> @@ -8,7 +8,6 @@
> __metaclass__ = type
>
> __all__ = [
> - 'bazaar_identity',
> 'BRANCH_NAME_VALIDATION_ERROR_MESSAGE',
> 'branch_name_validator',
> 'BranchCannotBePrivate',
> @@ -21,6 +20,7 @@
> 'BranchExists',
> 'BranchTargetError',
> 'BranchTypeError',
> + 'BzrIdentityMixin',
> 'CannotDeleteBranch',
> 'DEFAULT_BRANCH_STATUS_IN_LISTING',
> 'get_blacklisted_hostnames',
> @@ -32,12 +32,10 @@
> 'IBranchNavigationMenu',
> 'IBranchSet',
> 'NoSuchBranch',
> - 'UnknownBranchTypeError',
> 'user_has_special_branch_access',
> ]
>
> from cgi import escape
> -from operator import attrgetter
> import re
>
> from zope.component import getUtility
> @@ -69,12 +67,14 @@
> )
> from lp.code.interfaces.branchlookup import IBranchLookup
> from lp.code.interfaces.branchtarget import IHasBranchTarget
> +from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
> from lp.code.interfaces.hasbranches import IHasMergeProposals
> from lp.code.interfaces.hasrecipes import IHasRecipes
> from canonical.launchpad.interfaces.launchpad import (
> ILaunchpadCelebrities, IPrivacy)
> from lp.registry.interfaces.role import IHasOwner
> from lp.registry.interfaces.person import IPerson
> +from lp.registry.interfaces.pocket import PackagePublishingPocket
> from canonical.launchpad.webapp.interfaces import (
> ITableBatchNavigator, NameLookupFailed)
> from canonical.launchpad.webapp.menu import structured
> @@ -498,6 +498,14 @@
> "None if not a package branch."),
> schema=Interface, required=False, readonly=True))
>
> + is_personal_branch = exported(
> + Bool(
> + title=_("Personal Branch"), required=False,
> + readonly=True, default=False,
> + description=_(
> + "A branch is a personal branch if it is not associated "
> + "with a project nor a source package.")))

I guess a direct test for this would be nice, although it's so very
simple perhaps it's not needed...

> code_reviewer = Attribute(
> "The reviewer if set, otherwise the owner of the branch.")
>
> @@ -925,6 +933,17 @@
> def associatedSuiteSourcePackages():
> """Return the suite source packages that this branch is linked to."""
>
> + def branchLinks():
> + """Return a sorted list of ICanHasLinkedBranch objects."""
> +
> + def branchIdentites():

Missing an 'i' here :)

> + """A list of aliases for a branch.
> +
> + Returns a list of tuples of bzr identity and context object. There is
> + at least one alias for any branch, and that is the branch itself. For
> + linked branches, the co...

review: Approve

[Tim Penhey (thumper)]

> Just a few comments below, mostly simple clarity stuff. Thanks for
> fixing the icky bug!

Yeah, it'll be good to get this cleaned up.

> > + def branchIdentites():
> Missing an 'i' here :)

Fixed.

> > + """A list of aliases for a branch.
> > +
> > + Returns a list of tuples of bzr identity and context object.
> > There is + at least one alias for any branch, and that is the
> > branch itself. For + linked branches, the context object is the
> > appropriate linked object. + """
>
> I'm afraid these two docstrings both need work. The first doesn't
> explain which ICanHasLinkedBranch objects are returned, nor what being
> 'sorted' means in this context.
>
> The second I think needs to explain itself a bit more as well. It
> seems to blur the concept of 'alias' (a concept we don't have anywhere
> else) and the linked object that the alias is derived from. Can you
> just try to expand it a bit, maybe with an example or two?
>
> In this branch it's not really obvious why branchLinks and
> branchIdentities have a separate existence to the bzr_identity
> property. I guess this is preparation for that portlet you want to
> add? I hope if the implementation of that doesn't require all these
> variants we can shrink them down a bit.

Updated.

> > === modified file 'lib/lp/code/model/linkedbranch.py'
> > + # When a project gets the series they are ordered
> > alphabetically + # by name.
>
> I don't think this sentence makes sense.
>
> I don't suppose it really matters, but sorting series by name isn't
> how it's usually done in Launchpad; sorting trunk first and then by
> comparing date_created is more common I think?

I looked at the product code to see how it returned them. Generally it just
sorted alphabetically, which I think is OK.

Comment updated too.

> > === modified file 'lib/lp/code/model/tests/test_branch.py'
> > +class TestBranchLinksAndIdentites(TestCaseWithFactory):
> > + """Test IBranch.branchLinks and IBranch.branchIdentities."""
> > +
> > + layer = DatabaseFunctionalLayer
> > +
> > + def test_default_identities(self):
> > + # If there are no links, the branch identities is just the
> > unique + # name.
>
> This sentence isn't very grammatical, maybe "If there are no links,
> the only branch identity is the unique name"?

Updated.

> > + def test_linked_to_product_series(self):
> > + # If a branch is linked to a non-development series of a product
> > and + # not linked to the product itself, then the product is not
> > returned + # in the links.
>
> I think the more positive "only the product series is returned in the
> links" is easier to understand than "the product is not returned".

Updated too.

> > + def test_linked_to_package_not_current_series(self):
> > + # If a branch is the development focus branch for a product,
> > then it's + # bzr identity is lp:product.
>
> This comment looks entirely wrong :-)

And this.

> > class TestBzrIdentity(TestCaseWithFactory):
> > """Test IBranch.bzr_identity."""
>
> You don't test here any branches that are linked to both products and
> sou...

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-04-08 08:55:10 +0000
+++ database/schema/security.cfg	2010-04-12 05:33:38 +0000
@@ -1677,6 +1677,7 @@
 public.product                          = SELECT
 public.productseries                    = SELECT
 public.project                          = SELECT
+public.seriessourcepackagebranch        = SELECT
 public.sourcepackagename                = SELECT
 public.staticdiff                       = SELECT, INSERT
 public.teamparticipation                = SELECT

=== modified file 'lib/lp/archiveuploader/tests/test_recipeuploads.py'
--- lib/lp/archiveuploader/tests/test_recipeuploads.py	2010-03-06 04:57:40 +0000
+++ lib/lp/archiveuploader/tests/test_recipeuploads.py	2010-04-12 05:33:38 +0000
@@ -33,7 +33,7 @@
         self.recipe = self.factory.makeSourcePackageRecipe()
         self.build = getUtility(ISourcePackageRecipeBuildSource).new(
             sourcepackage=self.factory.makeSourcePackage(
-                sourcename='bar', distroseries=self.breezy),
+                sourcepackagename='bar', distroseries=self.breezy),
             recipe=self.recipe, archive=self.factory.makeArchive(
                 distribution=self.ubuntu, owner=self.recipe.owner),
             requester=self.recipe.owner)

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2010-02-16 20:36:48 +0000
+++ lib/lp/code/browser/branchlisting.py	2010-04-12 05:33:38 +0000
@@ -74,7 +74,7 @@
 from lp.code.enums import (
     BranchLifecycleStatus, BranchLifecycleStatusFilter, BranchType)
 from lp.code.interfaces.branch import (
-    bazaar_identity, DEFAULT_BRANCH_STATUS_IN_LISTING, IBranch,
+    BzrIdentityMixin, DEFAULT_BRANCH_STATUS_IN_LISTING, IBranch,
     IBranchBatchNavigator, IBranchListingQueryOptimiser)
 from lp.code.interfaces.branchcollection import IAllBranches
 from lp.code.interfaces.branchnamespace import IBranchNamespacePolicy
@@ -137,7 +137,7 @@
             return HasBadgeBase.getBadge(self, badge_name)
 
 
-class BranchListingItem(BranchBadges):
+class BranchListingItem(BzrIdentityMixin, BranchBadges):
     """A decorated branch.
 
     Some attributes that we want to display are too convoluted or expensive
@@ -148,7 +148,7 @@
 
     def __init__(self, branch, last_commit, now, show_bug_badge,
                  show_blueprint_badge, show_mp_badge,
-                 associated_product_series, suite_source_packages, is_dev_focus):
+                 associated_product_series, suite_source_packages):
         BranchBadges.__init__(self, branch)
         self.last_commit = last_commit
         self.show_bug_badge = show_bug_badge
@@ -157,7 +157,6 @@
         self._now = now
         self.associated_product_series = associated_product_series
         self.suite_source_packages = suite_source_packages
-        self.is_development_focus = is_dev_focus
 
     def associatedProductSeries(self):
         """Override the IBranch.associatedProductSeries."""
@@ -173,11 +172,6 @@
                 if series.status != SeriesStatus.OBSOLETE]
 
     @property
-    def bzr_identity(self):
-        """Produce the bzr identity from our known associated series."""
-        return bazaar_identity(self, self.is_development_focus)
-
-    @property
     def since_updated(self):
         """How long since the branch was last updated."""
         return self._now - self.context.date_last_modified
@@ -397,30 +391,6 @@
             self._distro_series_map[distribution] = result
             return result
 
-    def isBranchDevFocus(self, branch,
-                         associated_product_series, suite_source_packages):
-        """Is the branch the development focus?
-
-        For product branches this means that the branch is linked to the
-        development focus series.
-
-        For package branches this means that the branch is linked to the
-        release pocket of the development series.
-        """
-        # Refactor this code to work for model.branch too?
-        # Do we care if a non-product branch is linked to the product series?
-        # Do we care if a non-package branch is linked to the package?
-        # A) not right now.
-        for series in associated_product_series:
-            if series.product.development_focus == series:
-                return True
-        for ssp in suite_source_packages:
-            if (ssp.pocket == PackagePublishingPocket.RELEASE and
-                ssp.distroseries == self.getDistroDevelSeries(
-                    ssp.distribution)):
-                return True
-        return False
-
     @cachedproperty
     def branch_ids_with_bug_links(self):
         """Return a set of branch ids that should show bug badges."""
@@ -470,12 +440,10 @@
         show_mp_badge = branch.id in self.branch_ids_with_merge_proposals
         associated_product_series = self.getProductSeries(branch)
         suite_source_packages = self.getSuiteSourcePackages(branch)
-        is_dev_focus = self.isBranchDevFocus(
-            branch, associated_product_series, suite_source_packages)
         return BranchListingItem(
             branch, last_commit, self._now, show_bug_badge,
             show_blueprint_badge, show_mp_badge,
-            associated_product_series, suite_source_packages, is_dev_focus)
+            associated_product_series, suite_source_packages)
 
     def decoratedBranches(self, branches):
         """Return the decorated branches for the branches passed in."""
@@ -1351,6 +1319,7 @@
     no_sort_by = (BranchListingSort.DEFAULT,)
     extra_columns = ('author', 'product')
     label_template = 'Bazaar branches of %(displayname)s'
+    show_series_links = True
 
     def _getCollection(self):
         return getUtility(IAllBranches).inProject(self.context)

=== modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
--- lib/lp/code/browser/tests/test_branchlisting.py	2010-03-30 09:56:10 +0000
+++ lib/lp/code/browser/tests/test_branchlisting.py	2010-04-12 05:33:38 +0000
@@ -335,6 +335,7 @@
         # There is only one branch.
         batch = view.branches()
         [view_branch] = batch.branches
+        self.assertStatementCount(0, getattr, view_branch, 'bzr_identity')
         self.assertEqual(identity, view_branch.bzr_identity)
 
 

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2010-04-06 20:17:04 +0000
+++ lib/lp/code/configure.zcml	2010-04-12 05:33:38 +0000
@@ -456,6 +456,8 @@
                     associatedProductSeries
                     getProductSeriesPushingTranslations
                     associatedSuiteSourcePackages
+                    branchIdentities
+                    branchLinks
                     subscribe
                     getSubscription
                     hasSubscription

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2010-04-06 17:19:23 +0000
+++ lib/lp/code/interfaces/branch.py	2010-04-12 05:33:38 +0000
@@ -8,7 +8,6 @@
 __metaclass__ = type
 
 __all__ = [
-    'bazaar_identity',
     'BRANCH_NAME_VALIDATION_ERROR_MESSAGE',
     'branch_name_validator',
     'BranchCannotBePrivate',
@@ -21,6 +20,7 @@
     'BranchExists',
     'BranchTargetError',
     'BranchTypeError',
+    'BzrIdentityMixin',
     'CannotDeleteBranch',
     'DEFAULT_BRANCH_STATUS_IN_LISTING',
     'get_blacklisted_hostnames',
@@ -32,12 +32,10 @@
     'IBranchNavigationMenu',
     'IBranchSet',
     'NoSuchBranch',
-    'UnknownBranchTypeError',
     'user_has_special_branch_access',
     ]
 
 from cgi import escape
-from operator import attrgetter
 import re
 
 from zope.component import getUtility
@@ -69,12 +67,14 @@
     )
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.branchtarget import IHasBranchTarget
+from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.code.interfaces.hasbranches import IHasMergeProposals
 from lp.code.interfaces.hasrecipes import IHasRecipes
 from canonical.launchpad.interfaces.launchpad import (
     ILaunchpadCelebrities, IPrivacy)
 from lp.registry.interfaces.role import IHasOwner
 from lp.registry.interfaces.person import IPerson
+from lp.registry.interfaces.pocket import PackagePublishingPocket
 from canonical.launchpad.webapp.interfaces import (
     ITableBatchNavigator, NameLookupFailed)
 from canonical.launchpad.webapp.menu import structured
@@ -925,6 +925,37 @@
     def associatedSuiteSourcePackages():
         """Return the suite source packages that this branch is linked to."""
 
+    def branchLinks():
+        """Return a sorted list of ICanHasLinkedBranch objects.
+
+        There is one result for each related linked object that the branch is
+        linked to.  For example in the case where a branch is linked to the
+        development series of a project, the link objects for both the project
+        and the development series are returned.
+
+        The sorting uses the defined order of the linked objects where the
+        more important links are sorted first.
+        """
+
+    def branchIdentities():
+        """A list of aliases for a branch.
+
+        Returns a list of tuples of bzr identity and context object.  There is
+        at least one alias for any branch, and that is the branch itself.  For
+        linked branches, the context object is the appropriate linked object.
+
+        Where a branch is linked to a product series or a suite source
+        package, the branch is available through a number of different urls.
+        These urls are the aliases for the branch.
+
+        For example, a branch linked to the development focus of the 'fooix'
+        project is accessible using:
+          lp:fooix - the linked object is the product fooix
+          lp:fooix/trunk - the linked object is the trunk series of fooix
+          lp:~owner/fooix/name - the unique name of the branch where the linked
+            object is the branch itself.
+        """
+
     # subscription-related methods
     @operation_parameters(
         person=Reference(
@@ -1303,50 +1334,48 @@
         """
 
 
-def bazaar_identity(branch, is_dev_focus):
-    """Return the shortest lp: style branch identity."""
-    lp_prefix = config.codehosting.bzr_lp_prefix
-
-    # XXX: TimPenhey 2008-05-06 bug=227602: Since at this stage the launchpad
-    # name resolution is not authenticated, we can't resolve series branches
-    # that end up pointing to private branches, so don't show short names for
-    # the branch if it is private.
-    if branch.private:
-        return lp_prefix + branch.unique_name
-
-    use_series = None
-    # XXX: JonathanLange 2009-03-21 spec=package-branches: This should
-    # probably delegate to IBranch.target. I would do it now if I could figure
-    # what all the optimization code is for.
-    if branch.product is not None:
-        if is_dev_focus:
-            return lp_prefix + branch.product.name
-
-        # If there are no associated series, then use the unique name.
-        associated_series = sorted(
-            branch.associatedProductSeries(), key=attrgetter('datecreated'))
-        if len(associated_series) == 0:
-            return lp_prefix + branch.unique_name
-        # Use the most recently created series.
-        use_series = associated_series[-1]
-        return "%(prefix)s%(product)s/%(series)s" % {
-            'prefix': lp_prefix,
-            'product': use_series.product.name,
-            'series': use_series.name}
-
-    if branch.sourcepackage is not None:
-        if is_dev_focus:
-            return "%(prefix)s%(distro)s/%(packagename)s" % {
-            'prefix': lp_prefix,
-            'distro': branch.distroseries.distribution.name,
-            'packagename': branch.sourcepackagename.name}
-        suite_sourcepackages = branch.associatedSuiteSourcePackages()
-        # Take the first link if there is one.
-        if len(suite_sourcepackages) > 0:
-            suite_source_package = suite_sourcepackages[0]
-            return lp_prefix + suite_source_package.path
-
-    return lp_prefix + branch.unique_name
+class BzrIdentityMixin:
+    """This mixin class determines the bazaar identities.
+
+    Used by both the model branch class and the browser branch listing item.
+    This allows the browser code to cache the associated links which reduces
+    query counts.
+    """
+
+    @property
+    def bzr_identity(self):
+        """See `IBranch`."""
+        identity, context = self.branchIdentities()[0]
+        return identity
+
+    def branchIdentities(self):
+        """See `IBranch`."""
+        lp_prefix = config.codehosting.bzr_lp_prefix
+        if self.private or not self.target.supports_short_identites:
+            # XXX: thumper 2010-04-08, bug 261609
+            # We have to get around to fixing this
+            identities = []
+        else:
+            identities = [
+                (lp_prefix + link.bzr_path, link.context)
+                for link in self.branchLinks()]
+        identities.append((lp_prefix + self.unique_name, self))
+        return identities
+
+    def branchLinks(self):
+        """See `IBranch`."""
+        links = []
+        for suite_sp in self.associatedSuiteSourcePackages():
+            links.append(ICanHasLinkedBranch(suite_sp))
+            if (suite_sp.distribution.currentseries == suite_sp.distroseries
+                and suite_sp.pocket == PackagePublishingPocket.RELEASE):
+                links.append(ICanHasLinkedBranch(
+                        suite_sp.sourcepackage.distribution_sourcepackage))
+        for series in self.associatedProductSeries():
+            links.append(ICanHasLinkedBranch(series))
+            if series.product.development_focus == series:
+                links.append(ICanHasLinkedBranch(series.product))
+        return sorted(links)
 
 
 def user_has_special_branch_access(user):

=== modified file 'lib/lp/code/interfaces/branchtarget.py'
--- lib/lp/code/interfaces/branchtarget.py	2010-03-26 02:36:01 +0000
+++ lib/lp/code/interfaces/branchtarget.py	2010-04-12 05:33:38 +0000
@@ -88,6 +88,9 @@
     supports_merge_proposals = Attribute(
         "Does this target support merge proposals at all?")
 
+    supports_short_identites = Attribute(
+        "Does this target support shortened bazaar identities?")
+
     supports_code_imports = Attribute(
         "Does this target support code imports at all?")
 

=== modified file 'lib/lp/code/interfaces/linkedbranch.py'
--- lib/lp/code/interfaces/linkedbranch.py	2009-11-12 04:28:28 +0000
+++ lib/lp/code/interfaces/linkedbranch.py	2010-04-12 05:33:38 +0000
@@ -25,6 +25,7 @@
 class ICanHasLinkedBranch(Interface):
     """Something that has a linked branch."""
 
+    context = Attribute("The object that can have a linked branch.")
     branch = Attribute("The linked branch.")
     bzr_path = Attribute(
         'The Bazaar branch path for the linked branch. '

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-03-24 18:48:35 +0000
+++ lib/lp/code/model/branch.py	2010-04-12 05:33:38 +0000
@@ -62,8 +62,8 @@
 from lp.code.model.seriessourcepackagebranch import SeriesSourcePackageBranch
 from lp.code.event.branchmergeproposal import NewBranchMergeProposalEvent
 from lp.code.interfaces.branch import (
-    bazaar_identity, BranchCannotBePrivate, BranchCannotBePublic,
-    BranchTargetError, BranchTypeError, CannotDeleteBranch,
+    BranchCannotBePrivate, BranchCannotBePublic,
+    BranchTargetError, BranchTypeError, BzrIdentityMixin, CannotDeleteBranch,
     DEFAULT_BRANCH_STATUS_IN_LISTING, IBranch,
     IBranchNavigationMenu, IBranchSet, user_has_special_branch_access)
 from lp.code.interfaces.branchcollection import IAllBranches
@@ -73,7 +73,6 @@
 from lp.code.interfaces.branchnamespace import IBranchNamespacePolicy
 from lp.code.interfaces.branchpuller import IBranchPuller
 from lp.code.interfaces.branchtarget import IBranchTarget
-from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.code.interfaces.seriessourcepackagebranch import (
     IFindOfficialBranchLinks)
 from lp.registry.interfaces.person import (
@@ -83,7 +82,7 @@
     NotificationRecipientSet)
 
 
-class Branch(SQLBase):
+class Branch(SQLBase, BzrIdentityMixin):
     """A sequence of ordered revisions in Bazaar."""
 
     implements(IBranch, IBranchNavigationMenu)
@@ -464,21 +463,6 @@
     def browse_source_url(self):
         return self.codebrowse_url('files')
 
-    @property
-    def bzr_identity(self):
-        """See `IBranch`."""
-        # Should probably put this into the branch target.
-        if self.product is not None:
-            series_branch = self.product.development_focus.branch
-            is_dev_focus = (series_branch == self)
-        elif self.distroseries is not None:
-            distro_package = self.sourcepackage.distribution_sourcepackage
-            linked_branch = ICanHasLinkedBranch(distro_package)
-            is_dev_focus = (linked_branch.branch == self)
-        else:
-            is_dev_focus = False
-        return bazaar_identity(self, is_dev_focus)
-
     def composePublicURL(self, scheme='http'):
         """See `IBranch`."""
         # Not all protocols work for private branches.

=== modified file 'lib/lp/code/model/branchtarget.py'
--- lib/lp/code/model/branchtarget.py	2010-03-26 02:36:01 +0000
+++ lib/lp/code/model/branchtarget.py	2010-04-12 05:33:38 +0000
@@ -103,6 +103,11 @@
         return True
 
     @property
+    def supports_short_identites(self):
+        """See `IBranchTarget`."""
+        return True
+
+    @property
     def supports_code_imports(self):
         """See `IBranchTarget`."""
         return False
@@ -195,6 +200,11 @@
         return False
 
     @property
+    def supports_short_identites(self):
+        """See `IBranchTarget`."""
+        return False
+
+    @property
     def supports_code_imports(self):
         """See `IBranchTarget`."""
         return False
@@ -276,6 +286,11 @@
         return True
 
     @property
+    def supports_short_identites(self):
+        """See `IBranchTarget`."""
+        return True
+
+    @property
     def supports_code_imports(self):
         """See `IBranchTarget`."""
         return True

=== modified file 'lib/lp/code/model/linkedbranch.py'
--- lib/lp/code/model/linkedbranch.py	2009-08-28 06:39:38 +0000
+++ lib/lp/code/model/linkedbranch.py	2010-04-12 05:33:38 +0000
@@ -11,6 +11,9 @@
 from zope.component import adapts
 from zope.interface import implements
 
+from lazr.enum import EnumeratedType, Item
+
+from lp.archivepublisher.debversion import Version
 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.registry.interfaces.distributionsourcepackage import (
     IDistributionSourcePackage)
@@ -21,98 +24,194 @@
 from lp.registry.interfaces.suitesourcepackage import ISuiteSourcePackage
 
 
-class ProductSeriesLinkedBranch:
+class LinkedBranchOrder(EnumeratedType):
+    """An enum used only for ordering."""
+
+    PRODUCT = Item('Product')
+    DISTRIBUTION_SOURCE_PACKAGE = Item('Distribution Source Package')
+    PRODUCT_SERIES = Item('Product Series')
+    SUITE_SOURCE_PACKAGE = Item('Suite Source Package')
+
+
+class BaseLinkedBranch:
+    """Provides the common sorting algorithm."""
+
+    def __cmp__(self, other):
+        if not ICanHasLinkedBranch.providedBy(other):
+            raise AssertionError("Can't compare with: %r" % other)
+        return cmp(self.sort_order, other.sort_order)
+
+
+class ProductSeriesLinkedBranch(BaseLinkedBranch):
     """Implement a linked branch for a product series."""
 
     adapts(IProductSeries)
     implements(ICanHasLinkedBranch)
 
+    sort_order = LinkedBranchOrder.PRODUCT_SERIES
+
     def __init__(self, product_series):
-        self._product_series = product_series
+        self.context = product_series
+
+    @property
+    def product_series(self):
+        return self.context
+
+    def __cmp__(self, other):
+        result = super(ProductSeriesLinkedBranch, self).__cmp__(other)
+        if result != 0:
+            return result
+        else:
+            # The sorting of the product series uses the same sorting the
+            # product itself uses, which is alphabetically by name.
+            my_parts = (
+                self.product_series.product.name,
+                self.product_series.name)
+            other_parts = (
+                other.product_series.product.name,
+                other.product_series.name)
+            return cmp(my_parts, other_parts)
 
     @property
     def branch(self):
         """See `ICanHasLinkedBranch`."""
-        return self._product_series.branch
+        return self.product_series.branch
 
     @property
     def bzr_path(self):
         """See `ICanHasLinkedBranch`."""
         return '/'.join(
-            [self._product_series.product.name, self._product_series.name])
+            [self.product_series.product.name, self.product_series.name])
 
     def setBranch(self, branch, registrant=None):
         """See `ICanHasLinkedBranch`."""
-        self._product_series.branch = branch
-
-
-class ProductLinkedBranch:
+        self.product_series.branch = branch
+
+
+class ProductLinkedBranch(BaseLinkedBranch):
     """Implement a linked branch for a product."""
 
     adapts(IProduct)
     implements(ICanHasLinkedBranch)
 
+    sort_order = LinkedBranchOrder.PRODUCT
+
     def __init__(self, product):
-        self._product = product
+        self.context = product
+
+    @property
+    def product(self):
+        return self.context
+
+    def __cmp__(self, other):
+        result = super(ProductLinkedBranch, self).__cmp__(other)
+        if result != 0:
+            return result
+        else:
+            return cmp(self.product.name, other.product.name)
 
     @property
     def branch(self):
         """See `ICanHasLinkedBranch`."""
-        return ICanHasLinkedBranch(self._product.development_focus).branch
+        return ICanHasLinkedBranch(self.product.development_focus).branch
 
     @property
     def bzr_path(self):
         """See `ICanHasLinkedBranch`."""
-        return self._product.name
+        return self.product.name
 
     def setBranch(self, branch, registrant=None):
         """See `ICanHasLinkedBranch`."""
-        ICanHasLinkedBranch(self._product.development_focus).setBranch(
+        ICanHasLinkedBranch(self.product.development_focus).setBranch(
             branch, registrant)
 
 
-class PackageLinkedBranch:
+class PackageLinkedBranch(BaseLinkedBranch):
     """Implement a linked branch for a source package pocket."""
 
     adapts(ISuiteSourcePackage)
     implements(ICanHasLinkedBranch)
 
+    sort_order = LinkedBranchOrder.SUITE_SOURCE_PACKAGE
+
     def __init__(self, suite_sourcepackage):
-        self._suite_sourcepackage = suite_sourcepackage
+        self.context = suite_sourcepackage
+
+    @property
+    def suite_sourcepackage(self):
+        return self.context
+
+    def __cmp__(self, other):
+        result = super(PackageLinkedBranch, self).__cmp__(other)
+        if result != 0:
+            return result
+        # The versions are reversed as we want the greater Version to sort
+        # before the lesser one.  Hence self in the other tuple, and other in
+        # the self tuple.  Next compare the distribution name.
+        my_parts = (
+            self.suite_sourcepackage.distribution.name,
+            Version(other.suite_sourcepackage.distroseries.version),
+            self.suite_sourcepackage.sourcepackagename.name,
+            self.suite_sourcepackage.pocket)
+        other_parts = (
+            other.suite_sourcepackage.distribution.name,
+            Version(self.suite_sourcepackage.distroseries.version),
+            other.suite_sourcepackage.sourcepackagename.name,
+            other.suite_sourcepackage.pocket)
+        return cmp(my_parts, other_parts)
 
     @property
     def branch(self):
         """See `ICanHasLinkedBranch`."""
-        package = self._suite_sourcepackage.sourcepackage
-        pocket = self._suite_sourcepackage.pocket
+        package = self.suite_sourcepackage.sourcepackage
+        pocket = self.suite_sourcepackage.pocket
         return package.getBranch(pocket)
 
     @property
     def bzr_path(self):
         """See `ICanHasLinkedBranch`."""
-        return self._suite_sourcepackage.path
+        return self.suite_sourcepackage.path
 
     def setBranch(self, branch, registrant):
         """See `ICanHasLinkedBranch`."""
-        package = self._suite_sourcepackage.sourcepackage
-        pocket = self._suite_sourcepackage.pocket
+        package = self.suite_sourcepackage.sourcepackage
+        pocket = self.suite_sourcepackage.pocket
         package.setBranch(pocket, branch, registrant)
 
 
-class DistributionPackageLinkedBranch:
+class DistributionPackageLinkedBranch(BaseLinkedBranch):
     """Implement a linked branch for an `IDistributionSourcePackage`."""
 
     adapts(IDistributionSourcePackage)
     implements(ICanHasLinkedBranch)
 
+    sort_order = LinkedBranchOrder.DISTRIBUTION_SOURCE_PACKAGE
+
     def __init__(self, distribution_sourcepackage):
-        self._distribution_sourcepackage = distribution_sourcepackage
+        self.context = distribution_sourcepackage
+
+    @property
+    def distribution_sourcepackage(self):
+        return self.context
+
+    def __cmp__(self, other):
+        result = super(DistributionPackageLinkedBranch, self).__cmp__(other)
+        if result != 0:
+            return result
+        else:
+            my_names = (
+                self.distribution_sourcepackage.distribution.name,
+                self.distribution_sourcepackage.sourcepackagename.name)
+            other_names = (
+                other.distribution_sourcepackage.distribution.name,
+                other.distribution_sourcepackage.sourcepackagename.name)
+            return cmp(my_names, other_names)
 
     @property
     def branch(self):
         """See `ICanHasLinkedBranch`."""
         development_package = (
-            self._distribution_sourcepackage.development_version)
+            self.distribution_sourcepackage.development_version)
         if development_package is None:
             return None
         suite_sourcepackage = development_package.getSuiteSourcePackage(
@@ -123,13 +222,13 @@
     def bzr_path(self):
         """See `ICanHasLinkedBranch`."""
         return '/'.join(
-            [self._distribution_sourcepackage.distribution.name,
-             self._distribution_sourcepackage.sourcepackagename.name])
+            [self.distribution_sourcepackage.distribution.name,
+             self.distribution_sourcepackage.sourcepackagename.name])
 
     def setBranch(self, branch, registrant):
         """See `ICanHasLinkedBranch`."""
         development_package = (
-            self._distribution_sourcepackage.development_version)
+            self.distribution_sourcepackage.development_version)
         if development_package is None:
             raise NoSuchDistroSeries('no current series')
         suite_sourcepackage = development_package.getSuiteSourcePackage(

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2010-04-06 17:58:06 +0000
+++ lib/lp/code/model/tests/test_branch.py	2010-04-12 05:33:38 +0000
@@ -483,6 +483,203 @@
         self.assertFalse(branch.upgrade_pending)
 
 
+class TestBranchLinksAndIdentites(TestCaseWithFactory):
+    """Test IBranch.branchLinks and IBranch.branchIdentities."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_default_identities(self):
+        # If there are no links, the only branch identity is the unique name.
+        branch = self.factory.makeAnyBranch()
+        self.assertEqual(
+            [('lp://dev/' + branch.unique_name, branch)],
+            branch.branchIdentities())
+
+    def test_linked_to_product(self):
+        # If a branch is linked to the product, it is also by definition
+        # linked to the development focus of the product.
+        fooix = removeSecurityProxy(self.factory.makeProduct(name='fooix'))
+        fooix.development_focus.name = 'devel'
+        eric = self.factory.makePerson(name='eric')
+        branch = self.factory.makeProductBranch(
+            product=fooix, owner=eric, name='trunk')
+        linked_branch = ICanHasLinkedBranch(fooix)
+        linked_branch.setBranch(branch)
+        self.assertEqual(
+            [linked_branch, ICanHasLinkedBranch(fooix.development_focus)],
+            branch.branchLinks())
+        self.assertEqual(
+            [('lp://dev/fooix', fooix),
+             ('lp://dev/fooix/devel', fooix.development_focus),
+             ('lp://dev/~eric/fooix/trunk', branch)],
+            branch.branchIdentities())
+
+    def test_linked_to_product_series(self):
+        # If a branch is linked to a non-development series of a product and
+        # not linked to the product itself, then only the product series is
+        # returned in the links.
+        fooix = removeSecurityProxy(self.factory.makeProduct(name='fooix'))
+        future = self.factory.makeProductSeries(product=fooix, name='future')
+        eric = self.factory.makePerson(name='eric')
+        branch = self.factory.makeProductBranch(
+            product=fooix, owner=eric, name='trunk')
+        linked_branch = ICanHasLinkedBranch(future)
+        linked_branch.setBranch(branch)
+        self.assertEqual(
+            [linked_branch],
+            branch.branchLinks())
+        self.assertEqual(
+            [('lp://dev/fooix/future', future),
+             ('lp://dev/~eric/fooix/trunk', branch)],
+            branch.branchIdentities())
+
+    def test_linked_to_package(self):
+        # If a branch is linked to a suite source package where the
+        # distroseries is the current series for the distribution, there is a
+        # link for both the distribution source package and the suite source
+        # package.
+        mint = self.factory.makeDistribution(name='mint')
+        dev = self.factory.makeDistroSeries(
+            distribution=mint, version='1.0', name='dev')
+        eric = self.factory.makePerson(name='eric')
+        branch = self.factory.makePackageBranch(
+            distroseries=dev, sourcepackagename='choc', name='tip',
+            owner=eric)
+        dsp = self.factory.makeDistributionSourcePackage('choc', mint)
+        distro_link = ICanHasLinkedBranch(dsp)
+        development_package = dsp.development_version
+        suite_sourcepackage = development_package.getSuiteSourcePackage(
+            PackagePublishingPocket.RELEASE)
+        suite_sp_link = ICanHasLinkedBranch(suite_sourcepackage)
+
+        registrant = getUtility(
+            ILaunchpadCelebrities).ubuntu_branches.teamowner
+        run_with_login(
+            registrant,
+            suite_sp_link.setBranch, branch, registrant)
+
+        self.assertEqual(
+            [distro_link, suite_sp_link],
+            branch.branchLinks())
+        self.assertEqual(
+            [('lp://dev/mint/choc', dsp),
+             ('lp://dev/mint/dev/choc', suite_sourcepackage),
+             ('lp://dev/~eric/mint/dev/choc/tip', branch)],
+            branch.branchIdentities())
+
+    def test_linked_to_package_not_release_pocket(self):
+        # If a branch is linked to a suite source package where the
+        # distroseries is the current series for the distribution, but the
+        # pocket is not the RELEASE pocket, then there is only the link for
+        # the suite source package.
+        mint = self.factory.makeDistribution(name='mint')
+        dev = self.factory.makeDistroSeries(
+            distribution=mint, version='1.0', name='dev')
+        eric = self.factory.makePerson(name='eric')
+        branch = self.factory.makePackageBranch(
+            distroseries=dev, sourcepackagename='choc', name='tip',
+            owner=eric)
+        dsp = self.factory.makeDistributionSourcePackage('choc', mint)
+        development_package = dsp.development_version
+        suite_sourcepackage = development_package.getSuiteSourcePackage(
+            PackagePublishingPocket.BACKPORTS)
+        suite_sp_link = ICanHasLinkedBranch(suite_sourcepackage)
+
+        registrant = getUtility(
+            ILaunchpadCelebrities).ubuntu_branches.teamowner
+        run_with_login(
+            registrant,
+            suite_sp_link.setBranch, branch, registrant)
+
+        self.assertEqual(
+            [suite_sp_link],
+            branch.branchLinks())
+        self.assertEqual(
+            [('lp://dev/mint/dev-backports/choc', suite_sourcepackage),
+             ('lp://dev/~eric/mint/dev/choc/tip', branch)],
+            branch.branchIdentities())
+
+    def test_linked_to_package_not_current_series(self):
+        # If the branch is linked to a suite source package where the distro
+        # series is not the current series, only the suite source package is
+        # returned in the links.
+        mint = self.factory.makeDistribution(name='mint')
+        self.factory.makeDistroSeries(
+            distribution=mint, version='1.0', name='dev')
+        supported = self.factory.makeDistroSeries(
+            distribution=mint, version='0.9', name='supported')
+        eric = self.factory.makePerson(name='eric')
+        branch = self.factory.makePackageBranch(
+            distroseries=supported, sourcepackagename='choc', name='tip',
+            owner=eric)
+        suite_sp = self.factory.makeSuiteSourcePackage(
+            distroseries=supported, sourcepackagename='choc',
+            pocket=PackagePublishingPocket.RELEASE)
+        suite_sp_link = ICanHasLinkedBranch(suite_sp)
+
+        registrant = getUtility(
+            ILaunchpadCelebrities).ubuntu_branches.teamowner
+        run_with_login(
+            registrant,
+            suite_sp_link.setBranch, branch, registrant)
+
+        self.assertEqual(
+            [suite_sp_link],
+            branch.branchLinks())
+        self.assertEqual(
+            [('lp://dev/mint/supported/choc', suite_sp),
+             ('lp://dev/~eric/mint/supported/choc/tip', branch)],
+            branch.branchIdentities())
+
+    def test_linked_across_project_to_package(self):
+        # If a product branch is linked to a suite source package, the links
+        # are the same as if it was a source package branch.
+        mint = self.factory.makeDistribution(name='mint')
+        self.factory.makeDistroSeries(
+            distribution=mint, version='1.0', name='dev')
+        eric = self.factory.makePerson(name='eric')
+        fooix = self.factory.makeProduct(name='fooix')
+        branch = self.factory.makeProductBranch(
+            product=fooix, owner=eric, name='trunk')
+        dsp = self.factory.makeDistributionSourcePackage('choc', mint)
+        distro_link = ICanHasLinkedBranch(dsp)
+        development_package = dsp.development_version
+        suite_sourcepackage = development_package.getSuiteSourcePackage(
+            PackagePublishingPocket.RELEASE)
+        suite_sp_link = ICanHasLinkedBranch(suite_sourcepackage)
+
+        registrant = getUtility(
+            ILaunchpadCelebrities).ubuntu_branches.teamowner
+        run_with_login(
+            registrant,
+            suite_sp_link.setBranch, branch, registrant)
+
+        self.assertEqual(
+            [distro_link, suite_sp_link],
+            branch.branchLinks())
+        self.assertEqual(
+            [('lp://dev/mint/choc', dsp),
+             ('lp://dev/mint/dev/choc', suite_sourcepackage),
+             ('lp://dev/~eric/fooix/trunk', branch)],
+            branch.branchIdentities())
+
+    def test_junk_branch_links(self):
+        # If a junk branch has links, those links are returned in the
+        # branchLinks, but the branchIdentities just has the branch unique
+        # name.
+        eric = self.factory.makePerson(name='eric')
+        branch = self.factory.makePersonalBranch(owner=eric, name='foo')
+        fooix = removeSecurityProxy(self.factory.makeProduct())
+        linked_branch = ICanHasLinkedBranch(fooix)
+        linked_branch.setBranch(branch)
+        self.assertEqual(
+            [linked_branch, ICanHasLinkedBranch(fooix.development_focus)],
+            branch.branchLinks())
+        self.assertEqual(
+            [('lp://dev/~eric/+junk/foo', branch)],
+            branch.branchIdentities())
+
+
 class TestBzrIdentity(TestCaseWithFactory):
     """Test IBranch.bzr_identity."""
 

=== modified file 'lib/lp/code/model/tests/test_branchtarget.py'
--- lib/lp/code/model/tests/test_branchtarget.py	2010-03-18 19:57:20 +0000
+++ lib/lp/code/model/tests/test_branchtarget.py	2010-04-12 05:33:38 +0000
@@ -122,6 +122,10 @@
         # Package branches do support merge proposals.
         self.assertTrue(self.target.supports_merge_proposals)
 
+    def test_supports_short_identites(self):
+        # Package branches do support short bzr identites.
+        self.assertTrue(self.target.supports_short_identites)
+
     def test_displayname(self):
         # The display name of a source package target is the display name of
         # the source package.
@@ -228,6 +232,10 @@
         # Personal branches do not support merge proposals.
         self.assertFalse(self.target.supports_merge_proposals)
 
+    def test_supports_short_identites(self):
+        # Personal branches do not support short bzr identites.
+        self.assertFalse(self.target.supports_short_identites)
+
     def test_displayname(self):
         # The display name of a person branch target is ~$USER/+junk.
         target = IBranchTarget(self.original)
@@ -341,6 +349,10 @@
         # Product branches do support merge proposals.
         self.assertTrue(self.target.supports_merge_proposals)
 
+    def test_supports_short_identites(self):
+        # Product branches do support short bzr identites.
+        self.assertTrue(self.target.supports_short_identites)
+
     def test_displayname(self):
         # The display name of a product branch target is the display name of
         # the product.

=== modified file 'lib/lp/code/model/tests/test_linkedbranch.py'
--- lib/lp/code/model/tests/test_linkedbranch.py	2009-08-28 06:39:38 +0000
+++ lib/lp/code/model/tests/test_linkedbranch.py	2010-04-12 05:33:38 +0000
@@ -201,5 +201,149 @@
             CannotHaveLinkedBranch, get_linked_branch, project)
 
 
+class TestLinkedBranchSorting(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_sorting_different_types(self):
+        # The different types can be sorted together, and sort so that the
+        # results are ordered like:
+        #   Product Link
+        #   Distribution Source Package Link
+        #   Product Series Link
+        #   Package Link
+        product_link = ICanHasLinkedBranch(self.factory.makeProduct())
+        product_series_link = ICanHasLinkedBranch(
+            self.factory.makeProductSeries())
+        distro_sp_link = ICanHasLinkedBranch(
+            self.factory.makeDistributionSourcePackage())
+        package_link = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage())
+
+        links = sorted(
+            [package_link, product_series_link, distro_sp_link, product_link])
+        self.assertIs(product_link, links[0])
+        self.assertIs(distro_sp_link, links[1])
+        self.assertIs(product_series_link, links[2])
+        self.assertIs(package_link, links[3])
+
+    def test_product_sort(self):
+        # If in the extremely unlikely event we have one branch linked as the
+        # trunk of two or more different products (you never know), then the
+        # sorting reverts to the name of the product.
+        aardvark_link = ICanHasLinkedBranch(
+            self.factory.makeProduct(name='aardvark'))
+        meerkat_link = ICanHasLinkedBranch(
+            self.factory.makeProduct(name='meerkat'))
+        zebra_link = ICanHasLinkedBranch(
+            self.factory.makeProduct(name='zebra'))
+        links = sorted(
+            [zebra_link, aardvark_link, meerkat_link])
+        self.assertIs(aardvark_link, links[0])
+        self.assertIs(meerkat_link, links[1])
+        self.assertIs(zebra_link, links[2])
+
+    def test_product_series_sort(self):
+        # Sorting by product series checks the product name first, then series
+        # name.
+        aardvark = self.factory.makeProduct(name='aardvark')
+        zebra = self.factory.makeProduct(name='zebra')
+        aardvark_devel = ICanHasLinkedBranch(
+            self.factory.makeProductSeries(
+                product=aardvark, name='devel'))
+        aardvark_testing = ICanHasLinkedBranch(
+            self.factory.makeProductSeries(
+                product=aardvark, name='testing'))
+        zebra_devel = ICanHasLinkedBranch(
+            self.factory.makeProductSeries(
+                product=zebra, name='devel'))
+        zebra_mashup = ICanHasLinkedBranch(
+            self.factory.makeProductSeries(
+                product=zebra, name='mashup'))
+
+        links = sorted(
+            [zebra_mashup, aardvark_testing, zebra_devel, aardvark_devel])
+        self.assertIs(aardvark_devel, links[0])
+        self.assertIs(aardvark_testing, links[1])
+        self.assertIs(zebra_devel, links[2])
+        self.assertIs(zebra_mashup, links[3])
+
+    def test_distribution_source_package_sort(self):
+        # Sorting of distribution source packages sorts firstly on the
+        # distribution name, then the package name.
+        aardvark = self.factory.makeDistribution(name='aardvark')
+        zebra = self.factory.makeDistribution(name='zebra')
+        aardvark_devel = ICanHasLinkedBranch(
+            self.factory.makeDistributionSourcePackage(
+                distribution=aardvark, sourcepackagename='devel'))
+        aardvark_testing = ICanHasLinkedBranch(
+            self.factory.makeDistributionSourcePackage(
+                distribution=aardvark, sourcepackagename='testing'))
+        zebra_devel = ICanHasLinkedBranch(
+            self.factory.makeDistributionSourcePackage(
+                distribution=zebra, sourcepackagename='devel'))
+        zebra_mashup = ICanHasLinkedBranch(
+            self.factory.makeDistributionSourcePackage(
+                distribution=zebra, sourcepackagename='mashup'))
+
+        links = sorted(
+            [zebra_mashup, aardvark_testing, zebra_devel, aardvark_devel])
+        self.assertIs(aardvark_devel, links[0])
+        self.assertIs(aardvark_testing, links[1])
+        self.assertIs(zebra_devel, links[2])
+        self.assertIs(zebra_mashup, links[3])
+
+    def test_suite_source_package_sort(self):
+        # The sorting of suite source packages checks the distribution first,
+        # then the distroseries version, followed by the source package name,
+        # and finally the pocket.
+        aardvark = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                distroseries=self.factory.makeDistroSeries(
+                    self.factory.makeDistribution(name='aardvark'))))
+        zebra = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                distroseries=self.factory.makeDistroSeries(
+                    self.factory.makeDistribution(name='zebra'))))
+        meerkat = self.factory.makeDistribution(name='meerkat')
+        meerkat_1 = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                self.factory.makeDistroSeries(meerkat, "1.0")))
+        meerkat_2 = self.factory.makeDistroSeries(meerkat, "2.0")
+        meerkat_3 = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                self.factory.makeDistroSeries(meerkat, "3.0")))
+        meerkat_2_devel_release = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                meerkat_2, 'devel', PackagePublishingPocket.RELEASE))
+        meerkat_2_devel_updates = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                meerkat_2, 'devel', PackagePublishingPocket.UPDATES))
+        meerkat_2_devel_backports = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                meerkat_2, 'devel', PackagePublishingPocket.BACKPORTS))
+        meerkat_2_apples = ICanHasLinkedBranch(
+            self.factory.makeSuiteSourcePackage(
+                meerkat_2, 'apples'))
+
+        links = sorted(
+            [meerkat_3,
+             meerkat_2_devel_updates,
+             zebra,
+             meerkat_2_apples,
+             aardvark,
+             meerkat_2_devel_backports,
+             meerkat_1,
+             meerkat_2_devel_release])
+        self.assertIs(aardvark, links[0])
+        self.assertIs(meerkat_3, links[1])
+        self.assertIs(meerkat_2_apples, links[2])
+        self.assertIs(meerkat_2_devel_release, links[3])
+        self.assertIs(meerkat_2_devel_updates, links[4])
+        self.assertIs(meerkat_2_devel_backports, links[5])
+        self.assertIs(meerkat_1, links[6])
+        self.assertIs(zebra, links[7])
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/code/stories/branches/xx-branch-listings.txt'
--- lib/lp/code/stories/branches/xx-branch-listings.txt	2009-08-14 18:02:29 +0000
+++ lib/lp/code/stories/branches/xx-branch-listings.txt	2010-04-12 05:33:38 +0000
@@ -365,8 +365,7 @@
 
 Now when we look at the branches for gnome-terminal, the main branch
 now shows as the "focus of development".  This is indicated by
-both the series link in the first column with the branch unique name,
-but also with a star with the badges.
+both the series link in the first column with the branch unique name.
 
     >>> browser.open('http://code.launchpad.dev/gnome-terminal')
     >>> table = find_tag_by_id(browser.contents, 'branchtable')
@@ -375,8 +374,6 @@
     >>> cols = row.fetch('td')
     >>> print extract_text(cols[0])
     lp://dev/gnome-terminal     Series: trunk
-    >>> print cols[1].renderContents()
-    <img src="/@@/favourite-yes" title="focus of development" />
 
 If a branch is associated with more than one series, then the links
 are comma separated and in alphabetical order.

=== modified file 'lib/lp/code/templates/branch-listing.pt'
--- lib/lp/code/templates/branch-listing.pt	2009-12-03 18:33:22 +0000
+++ lib/lp/code/templates/branch-listing.pt	2010-04-12 05:33:38 +0000
@@ -41,159 +41,6 @@
 }
 registerLaunchpadFunction(hookUpFilterSubmission);
 
-LPS.use('io-base', 'node', 'json-parse', function(Y) {
-
-function doUpdate(transaction_id, response, args) {
-    json_values = Y.JSON.parse(response.responseText);
-    // Make sure that the branch isn't empty, and that there
-    // have been commits in the last 90 days
-
-    // Logic has been inverted in the next line to avoid
-    // breaking XHTML compliance of the template due to
-    // ampersand usage.
-    if(!(!json_values.commits || !json_values.count > 0)) {
-        document.getElementById(args.container).innerHTML = '';
-        sparkline(args.container, json_values);
-    }
-}
-
-
-function renderSpark() {
-
-    for (spark_div in branch_sparks) {
-
-        spark_div = branch_sparks[spark_div][0];
-
-        if (Y.one('#' + spark_div) !== null) {
-            json_url = branch_sparks[0][1];
-            var container = spark_div;
-            var uri = json_url;
-            var xhr_config = {
-                on: {
-                    success: doUpdate,
-                },
-                arguments: {
-                    'container': container
-                }
-            };
-
-          Y.io(uri, xhr_config);
-        }
-    }
-}
-
-
-function sparkline(branch_id, o) {
-    var spark_div = document.getElementById(branch_id);
-    var number_commits = parseInt(o.commits.length);
-    var w = 160;
-    var min = 9999;
-    var max = -1;
-    var h = 12;
-    var co = document.createElement("canvas");
-
-    co.style.height = h;
-    co.style.width = w;
-    co.width = w;
-    co.height = h;
-
-    spark_div.appendChild(co);
-
-    // Figure out the max and min number of commits
-
-    // Logic has been inverted in the next line to avoid breaking
-    // XHTML compliance of the template due to ``less than`` symbol
-    // usage (using ``greater than`` is fine).
-    for (var i = 0; number_commits > i; i++) {
-        o.commits[i] = o.commits[i] - 0;
-        if ( min > o.commits[i] ) min = o.commits[i];
-        if ( o.commits[i] > max ) max = o.commits[i];
-    }
-
-    if (co.getContext) {
-        var c = co.getContext("2d");
-        c.strokeStyle = "black";
-        c.lineWidth = 1.0;
-        c.beginPath();
-
-      // Logic has been inverted in the next line to avoid breaking
-      // XHTML compliance of the template due to ``less than`` symbol
-      // usage (using ``greater than`` is fine).
-      for (var i = 0; number_commits > i; i++) {
-          x_pos = (w / number_commits) * i;
-          // We subtract 1 pixel here to compensate for the base line
-          y_pos = (h-1) - (((o.commits[i] - min) / (max - min)) * (h-1));
-
-          if (i == 0) {
-              c.moveTo(x_pos, y_pos);
-          }
-          c.lineTo(x_pos, y_pos);
-
-          // When was the last commit?
-          if(o.commits[i] > 0) {
-              last_commit = i+1;
-          }
-
-      }
-      c.stroke();
-
-      // Draw the base line
-      c.beginPath();
-      c.strokeStyle = "gray";
-      c.moveTo(0,h);
-      c.lineTo(w,h);
-      c.stroke();
-
-      // Marks the point with the most commits
-      c.fillStyle = 'red';
-      c.beginPath();
-      c.arc((w / number_commits) * o.max_commits,
-            h+2 - (((o.commits[o.max_commits] - min) / (max - min)) * h),
-            2, 0, (Math.PI*2), true);
-      c.fill();
-
-      // Shows the number of max commits
-      new_text = document.createTextNode(o.commits[o.max_commits] + ' max commits');
-      new_div = document.createElement('div');
-      new_div.setAttribute('id', 'txt_max_' + branch_id);
-      new_div.appendChild(new_text);
-      document.getElementById(branch_id).appendChild(new_div);
-      new_div.style.fontSize = '8px';
-      new_div.style.color = 'red';
-      new_div.style.position = 'relative';
-      new_div.style.top = '-22px';
-      new_div.style.left = (((w / number_commits) * o.max_commits)-14) + 'px';
-
-
-      // Marks the point of last commit
-      c.fillStyle = 'blue';
-      c.beginPath();
-      c.arc((w / number_commits) * last_commit, h-2,
-            2, 0, (Math.PI*2), true);
-      c.fill();
-
-      // Shows the date of the last commit and time frame
-      new_text = document.createTextNode('90 days - Last commit ' + o.last_commit);
-      new_div = document.createElement('div');
-      new_div.setAttribute('id', 'txt_' + branch_id);
-      new_div.appendChild(new_text);
-      document.getElementById(branch_id).appendChild(new_div);
-      new_div.style.fontSize = '8px';
-      new_div.style.color = 'blue';
-      new_div.style.position = 'relative';
-      new_div.style.left = '2px';
-      new_div.style.top = '-10px';
-
-    }
-}
-
-// XXX TimPenhey 2009-08-03 bug 408207 - disable sparklines
-// Y.on("domready", renderSpark);
-
-});
-
-
-
 </script>
 
   <tal:needs-batch condition="context/has_multiple_pages">
@@ -255,9 +102,6 @@
           </tal:associated-series>
         </td>
         <td align="right" style="padding-right: 5px">
-          <tal:dev_focus condition="branch/is_development_focus">
-            <img src="/@@/favourite-yes" title="focus of development"/>
-          </tal:dev_focus>
           <tal:badges replace="structure branch/badges:small"/>
         </td>
         <td>
@@ -292,15 +136,10 @@
                 tal:content="branch/date_last_modified/fmt:datetime">
             2005-02-12 13:45 EST
           </span>
-          <div id="dev_focus_spark" style="top:10px; position:relative; height:18px;"
-               tal:omit-tag="not: branch/is_development_focus">
-            <div tal:attributes="id string:b-${repeat/branch/number}">
-            <span tal:attributes="title branch/date_last_modified/fmt:datetime"
-                  tal:content="branch/since_updated/fmt:approximateduration/use-digits">
-                sometime
-            </span> ago
-            </div>
-          </div>
+          <span tal:attributes="title branch/date_last_modified/fmt:datetime"
+                tal:content="branch/since_updated/fmt:approximateduration/use-digits">
+            sometime
+          </span> ago
         </td>
 
         <tal:no_commit condition="not: branch/last_commit">
@@ -368,10 +207,3 @@
   <tal:navigation replace="structure context/@@+navigation-links-lower" />
 
 </div>
-
-<tal:script
-    replace="structure string:&lt;script id='branch-sparks' type='text/javascript'&gt;" />
-<tal:omit tal:omit-tag="">
-var branch_sparks = <tal:array replace="context/branch_sparks"/>;
-</tal:omit>
-<tal:script replace="structure string:&lt;/script&gt;" />

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-04-08 17:17:15 +0000
+++ lib/lp/testing/factory.py	2010-04-12 05:33:38 +0000
@@ -1773,8 +1773,11 @@
             owner = self.makePerson()
         if distroseries is None:
             distroseries = self.makeDistroSeries()
-        if sourcepackagename is None:
-            sourcepackagename = self.makeSourcePackageName()
+        # Make sure we have a real sourcepackagename object.
+        if (sourcepackagename is None or
+            isinstance(sourcepackagename, basestring)):
+            sourcepackagename = self.getOrMakeSourcePackageName(
+                sourcepackagename)
         if name is None:
             name = self.getUniqueString().decode('utf8')
         if description is None:
@@ -1789,7 +1792,7 @@
                                      sourcename=None):
         """Make a new SourcePackageRecipeBuild."""
         if sourcepackage is None:
-            sourcepackage = self.makeSourcePackage(sourcename=sourcename)
+            sourcepackage = self.makeSourcePackage(sourcename)
         if recipe is None:
             recipe = self.makeSourcePackageRecipe()
         if requester is None:
@@ -2061,11 +2064,13 @@
             return self.makeSourcePackageName()
         return getUtility(ISourcePackageNameSet).getOrCreateByName(name)
 
-    def makeSourcePackage(
-        self, sourcepackagename=None, distroseries=None, sourcename=None):
+    def makeSourcePackage(self, sourcepackagename=None, distroseries=None):
         """Make an `ISourcePackage`."""
-        if sourcepackagename is None:
-            sourcepackagename = self.makeSourcePackageName(sourcename)
+        # Make sure we have a real sourcepackagename object.
+        if (sourcepackagename is None or
+            isinstance(sourcepackagename, basestring)):
+            sourcepackagename = self.getOrMakeSourcePackageName(
+                sourcepackagename)
         if distroseries is None:
             distroseries = self.makeDistroRelease()
         return distroseries.getSourcePackage(sourcepackagename)
@@ -2249,14 +2254,20 @@
             distroseries = self.makeDistroRelease()
         if pocket is None:
             pocket = self.getAnyPocket()
-        if sourcepackagename is None:
-            sourcepackagename = self.makeSourcePackageName()
+        # Make sure we have a real sourcepackagename object.
+        if (sourcepackagename is None or
+            isinstance(sourcepackagename, basestring)):
+            sourcepackagename = self.getOrMakeSourcePackageName(
+                sourcepackagename)
         return SuiteSourcePackage(distroseries, pocket, sourcepackagename)
 
     def makeDistributionSourcePackage(self, sourcepackagename=None,
                                       distribution=None):
-        if sourcepackagename is None:
-            sourcepackagename = self.makeSourcePackageName()
+        # Make sure we have a real sourcepackagename object.
+        if (sourcepackagename is None or
+            isinstance(sourcepackagename, basestring)):
+            sourcepackagename = self.getOrMakeSourcePackageName(
+                sourcepackagename)
         if distribution is None:
             distribution = self.makeDistribution()