launchpad-reviewers team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Tim Penhey has proposed merging lp:~thumper/launchpad/short-lp-name-for-private-branches into lp:launchpad/devel with lp:~thumper/launchpad/xmlrpc-lp-name-resolution as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch is the final part of providing short lp name for private
branches that are linked to official series.

There is a little drive by clean up in the tests for imports and
removing unneeded syncUpdate calls.

I think we have got all the test failures here.
-- 
https://code.launchpad.net/~thumper/launchpad/short-lp-name-for-private-branches/+merge/34509
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/short-lp-name-for-private-branches into lp:launchpad/devel.

=== modified file 'lib/lp/code/doc/branch-xmlrpc.txt'
--- lib/lp/code/doc/branch-xmlrpc.txt	2010-02-19 03:06:12 +0000
+++ lib/lp/code/doc/branch-xmlrpc.txt	2010-09-03 00:35:58 +0000
@@ -338,120 +338,4 @@
 The URLs that are likely to be faster or provide write access appear
 earlier in the list.
 
-
-lp:project
-==========
-
-If you specify a path with a single segment, then Launchpad treats that
-path as the name of a project, and returns URLs to the branch
-associated with the project's development focus series.
-
-    >>> results = branchset_api.resolve_lp_path('evolution')
-    >>> for url in results['urls']:
-    ...     print url
-    bzr+ssh://bazaar.launchpad.dev/~vcs-imports/evolution/main
-    http://bazaar.launchpad.dev/~vcs-imports/evolution/main
-
-
-Launchpad returns a fault if the development focus doesn't have branch
-associated with it.
-
-    >>> branchset_api.resolve_lp_path('firefox')
-    Traceback (most recent call last):
-    ...
-    Fault: <Fault 170: 'Mozilla Firefox has no default branch.'>
-
-
-Launchpad also returns a fault if the project doesn't exist.
-
-    >>> branchset_api.resolve_lp_path('doesntexist')
-    Traceback (most recent call last):
-    ...
-    Fault: <Fault 10: 'No such project: doesntexist'>
-
-
-lp:project/series
-=================
-
-If you specify a path with two segments, Launchpad treats that as a
-reference to a particular series on a project.
-
-    >>> results = branchset_api.resolve_lp_path('evolution/trunk')
-    >>> for url in results['urls']:
-    ...     print url
-    bzr+ssh://bazaar.launchpad.dev/~vcs-imports/evolution/main
-    http://bazaar.launchpad.dev/~vcs-imports/evolution/main
-
-
-If the series doesn't exist, then the server returns a fault.
-
-    >>> branchset_api.resolve_lp_path('firefox/doesntexist')
-    Traceback (most recent call last):
-    ...
-    Fault: <Fault 180: 'Project firefox has no series called "doesntexist"'>
-
-
-Similarly if the project doesn't exist:
-
-    >>> branchset_api.resolve_lp_path('doesntexist/trunk')
-    Traceback (most recent call last):
-    ...
-    Fault: <Fault 10: 'No such project: doesntexist'>
-
-
-lp:~owner/product/branch
-========================
-
-Anything longer than two segments is treated as a unique branch name
-followed by an arbitrary path:
-
-    >>> results = branchset_api.resolve_lp_path('~vcs-imports/evolution/main')
-    >>> for url in results['urls']:
-    ...     print url
-    bzr+ssh://bazaar.launchpad.dev/~vcs-imports/evolution/main
-    http://bazaar.launchpad.dev/~vcs-imports/evolution/main
-
-
-Launchpad will still resolve the branch even if it doesn't exist:
-
-    >>> results = branchset_api.resolve_lp_path('~mark/+junk/doesntexist')
-    >>> for url in results['urls']:
-    ...     print url
-    bzr+ssh://bazaar.launchpad.dev/~mark/%2Bjunk/doesntexist
-    http://bazaar.launchpad.dev/~mark/%2Bjunk/doesntexist
-
-
-But if the project or the user doesn't exist, Launchpad will return a fault:
-
-    >>> branchset_api.resolve_lp_path('~doesntexist/+junk/trunk')
-    Traceback (most recent call last):
-    ...
-    Fault: <Fault 200: 'No such person or team: doesntexist'>
-
-    >>> branchset_api.resolve_lp_path('~mark/doesntexist/trunk')
-    Traceback (most recent call last):
-    ...
-    Fault: <Fault 10: 'No such project: doesntexist'>
-
-
-Remote branches
-===============
-
-Some branches on Launchpad are 'remote' branches. These branches are
-not hosted on Launchpad and are only available (if at all) on remote
-sites. For these branches, resolve_lp_path returns only a single URL.
-
-    >>> from lp.code.enums import BranchType
-    >>> from lp.registry.interfaces.person import IPersonSet
-    >>> arbitrary_person = getUtility(IPersonSet).getByName('name12')
-    >>> from lp.code.interfaces.branchnamespace import (
-    ...     get_branch_namespace)
-    >>> namespace = get_branch_namespace(arbitrary_person)
-    >>> namespace.createBranch(
-    ...     branch_type=BranchType.REMOTE, name='remote',
-    ...     registrant=arbitrary_person,
-    ...     url='http://example.com/remote_branch')
-    <Branch u'~name12/+junk/remote' (...)>
-    >>> results = branchset_api.resolve_lp_path('~name12/+junk/remote')
-    >>> results['urls']
-    ['http://example.com/remote_branch']
+For more tests see `lp.code.xmlrpc.tests.test_branch.py`.

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2010-08-27 02:11:36 +0000
+++ lib/lp/code/interfaces/branch.py	2010-09-03 00:35:58 +0000
@@ -1303,9 +1303,7 @@
     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
+        if not self.target.supports_short_identites:
             identities = []
         else:
             identities = [

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/tests/test_branch.py	2010-09-03 00:35:58 +0000
@@ -28,13 +28,6 @@
 from canonical.config import config
 from canonical.database.constants import UTC_NOW
 from canonical.launchpad import _
-from canonical.launchpad.ftests import (
-    ANONYMOUS,
-    login,
-    login_person,
-    logout,
-    syncUpdate,
-    )
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.webapp.interfaces import IOpenLaunchBag
 from canonical.testing import (
@@ -120,6 +113,11 @@
 from lp.registry.model.sourcepackage import SourcePackage
 from lp.services.osutils import override_environ
 from lp.testing import (
+    ANONYMOUS,
+    celebrity_logged_in,
+    login,
+    login_person,
+    logout,
     person_logged_in,
     run_with_login,
     TestCase,
@@ -904,16 +902,12 @@
         self.assertBzrIdentity(branch, linked_branch.bzr_path)
 
     def test_private_linked_to_product(self):
-        # If a branch is private, then the bzr identity is the unique name,
-        # even if it's linked to a product. Of course, you have to be able to
-        # see the branch at all.
+        # Private branches also have a short lp:url.
         branch = self.factory.makeProductBranch(private=True)
-        owner = removeSecurityProxy(branch).owner
-        login_person(owner)
-        self.addCleanup(logout)
-        product = removeSecurityProxy(branch.product)
-        ICanHasLinkedBranch(product).setBranch(branch)
-        self.assertBzrIdentity(branch, branch.unique_name)
+        with celebrity_logged_in('admin'):
+            product = branch.product
+            ICanHasLinkedBranch(product).setBranch(branch)
+            self.assertBzrIdentity(branch, product.name)
 
     def test_linked_to_series_and_dev_focus(self):
         # If a branch is the development focus branch for a product and the
@@ -1048,7 +1042,6 @@
         deleted.
         """
         self.product.development_focus.branch = self.branch
-        syncUpdate(self.product.development_focus)
         self.assertEqual(self.branch.canBeDeleted(), False,
                          "A branch that is a user branch for a product series"
                          " is not deletable.")
@@ -1056,7 +1049,6 @@
 
     def test_productSeriesTranslationsBranchDisablesDeletion(self):
         self.product.development_focus.translations_branch = self.branch
-        syncUpdate(self.product.development_focus)
         self.assertEqual(self.branch.canBeDeleted(), False,
                          "A branch that is a translations branch for a "
                          "product series is not deletable.")
@@ -1219,7 +1211,6 @@
         # Disable this merge proposal, to allow creating a new identical one
         lp_admins = getUtility(ILaunchpadCelebrities).admin
         merge_proposal1.rejectBranch(lp_admins, 'null:')
-        syncUpdate(merge_proposal1)
         merge_proposal2 = self.branch.addLandingTarget(
             self.branch.owner, target_branch, prerequisite_branch)
         return merge_proposal1, merge_proposal2
@@ -1603,7 +1594,6 @@
         proposal = self.source.addLandingTarget(
             self.user, self.target, self.prerequisite)
         proposal.rejectBranch(self.user, 'some_revision')
-        syncUpdate(proposal)
         self.source.addLandingTarget(
             self.user, self.target, self.prerequisite)
 

=== modified file 'lib/lp/code/stories/branches/xx-private-branch-listings.txt'
--- lib/lp/code/stories/branches/xx-private-branch-listings.txt	2009-11-17 09:08:17 +0000
+++ lib/lp/code/stories/branches/xx-private-branch-listings.txt	2010-09-03 00:35:58 +0000
@@ -244,14 +244,14 @@
 
     >>> print extract_text(
     ...     find_tag_by_id(admin_browser.contents, 'branch-details'))
-    lp://dev/~landscape-developers/landscape/trunk - Landscape Developers ...
+    lp://dev/landscape - Landscape Developers ...
 
 Landscape developers can see it.
 
     >>> landscape_dev_browser.open('http://launchpad.dev/landscape/trunk')
     >>> print extract_text(find_tag_by_id(
     ...     landscape_dev_browser.contents, 'branch-details'))
-    lp://dev/~landscape-developers/landscape/trunk - Landscape Developers ...
+    lp://dev/landscape - Landscape Developers ...
 
 But normal people can't.
 

=== modified file 'lib/lp/code/tests/test_branch.py'
--- lib/lp/code/tests/test_branch.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/tests/test_branch.py	2010-09-03 00:35:58 +0000
@@ -16,8 +16,8 @@
     BranchSubscriptionNotificationLevel,
     CodeReviewNotificationLevel,
     )
+from lp.code.interfaces.codehosting import SUPPORTED_SCHEMES
 from lp.code.tests.helpers import make_official_package_branch
-from lp.code.xmlrpc.branch import PublicCodehostingAPI
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.testing import (
     run_with_login,
@@ -347,7 +347,7 @@
 
         url_pattern = '%%s://bazaar.launchpad.dev/~%s/%s/%s' % (
             branch.owner.name, branch.product.name, branch.name)
-        for scheme in PublicCodehostingAPI.supported_schemes:
+        for scheme in SUPPORTED_SCHEMES:
             public_url = branch.composePublicURL(scheme)
             self.assertEqual(url_pattern % scheme, public_url)