launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/recognize-branch-id-stacked-location into lp:launchpad with lp:~thumper/launchpad/bzr-transport-branch-id-access as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #377519 in Launchpad itself: "Stacked on location breaks if the stacked upon branch is renamed"
  https://bugs.launchpad.net/launchpad/+bug/377519

For more details, see:
https://code.launchpad.net/~thumper/launchpad/recognize-branch-id-stacked-location/+merge/56289

The +branch-id stacking location needs to be understood
by the branchChanged method to set the database field
appropriately.  We store this in the DB to show it on
the branch page, and to stop branches that are being
stacked on from being deleted.
-- 
https://code.launchpad.net/~thumper/launchpad/recognize-branch-id-stacked-location/+merge/56289
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/recognize-branch-id-stacked-location into lp:launchpad.

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2011-03-29 00:11:57 +0000
+++ lib/lp/code/model/branch.py	2011-04-05 04:51:29 +0000
@@ -118,7 +118,10 @@
 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.codehosting import compose_public_url
+from lp.code.interfaces.codehosting import (
+    BRANCH_ID_ALIAS_PREFIX,
+    compose_public_url,
+    )
 from lp.code.interfaces.seriessourcepackagebranch import (
     IFindOfficialBranchLinks,
     )
@@ -997,6 +1000,17 @@
         self.last_mirror_attempt = UTC_NOW
         self.next_mirror_time = None
 
+    def _findStackedBranch(self, stacked_on_location):
+        location = stacked_on_location.strip('/')
+        if location.startswith(BRANCH_ID_ALIAS_PREFIX + '/'):
+            try:
+                branch_id = int(location.split('/', 1)[1])
+            except (ValueError, IndexError):
+                return None
+            return getUtility(IBranchLookup).get(branch_id)
+        else:
+            return getUtility(IBranchLookup).getByUniqueName(location)
+
     def branchChanged(self, stacked_on_location, last_revision_id,
                       control_format, branch_format, repository_format):
         """See `IBranch`."""
@@ -1004,8 +1018,7 @@
         if stacked_on_location == '' or stacked_on_location is None:
             stacked_on_branch = None
         else:
-            stacked_on_branch = getUtility(IBranchLookup).getByUniqueName(
-                stacked_on_location.strip('/'))
+            stacked_on_branch = self._findStackedBranch(stacked_on_location)
             if stacked_on_branch is None:
                 self.mirror_status_message = (
                     'Invalid stacked on location: ' + stacked_on_location)

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2011-03-29 00:11:57 +0000
+++ lib/lp/code/model/tests/test_branch.py	2011-04-05 04:51:29 +0000
@@ -79,6 +79,7 @@
     )
 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
 from lp.code.interfaces.branchrevision import IBranchRevision
+from lp.code.interfaces.codehosting import BRANCH_ID_ALIAS_PREFIX
 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.code.interfaces.seriessourcepackagebranch import (
     IFindOfficialBranchLinks,
@@ -180,6 +181,17 @@
             stacked_on.unique_name, '', *self.arbitrary_formats)
         self.assertEqual(stacked_on, branch.stacked_on)
 
+    def test_branchChanged_sets_stacked_on_branch_id_alias(self):
+        # branchChanged sets the stacked_on attribute based on the id of the
+        # branch if it is valid.
+        branch = self.factory.makeAnyBranch()
+        stacked_on = self.factory.makeAnyBranch()
+        login_person(branch.owner)
+        stacked_on_location = '/%s/%s' % (
+            BRANCH_ID_ALIAS_PREFIX, stacked_on.id)
+        branch.branchChanged(stacked_on_location, '', *self.arbitrary_formats)
+        self.assertEqual(stacked_on, branch.stacked_on)
+
     def test_branchChanged_unsets_stacked_on(self):
         # branchChanged clears the stacked_on attribute on the branch if '' is
         # passed in as the stacked_on location.