Merge lp:~abentley/launchpad/branch-url into lp:launchpad

[Aaron Bentley]

[Aaron Bentley (abentley)]

This branch refactors code.model.branch.Branch.composePublicUrl and code.xmlrpc.branch.PublicCodehostingAPI to use common code for generating branch URLs.

Because the latter generates URLs using a unique name if no branch exists, I extracted most of Branch.composePublicUrl into a function, compose_public_url, that takes unique_name as a parameter.

There are no new tests, but the code passes existing tests for composePublicUrl and PublicCodehostingAPI.

[Michael Hudson-Doyle (mwhudson)]

Makes sense to me.

review: Approve

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-02-16 20:47:17 +0000
+++ lib/lp/code/model/branch.py	2010-02-16 20:47:18 +0000
@@ -7,9 +7,11 @@
 __all__ = [
     'Branch',
     'BranchSet',
+    'compose_public_url',
     ]
 
 from datetime import datetime
+import os.path
 
 from bzrlib.branch import Branch as BzrBranch
 from bzrlib.revision import NULL_REVISION
@@ -481,24 +483,11 @@
 
     def composePublicURL(self, scheme='http'):
         """See `IBranch`."""
-        # Avoid circular imports.
-        from lp.code.xmlrpc.branch import PublicCodehostingAPI
-
-        # Accept sftp as a legacy protocol.
-        accepted_schemes = set(PublicCodehostingAPI.supported_schemes)
-        accepted_schemes.add('sftp')
-
         # Not all protocols work for private branches.
         public_schemes = ['http']
-
-        assert scheme in accepted_schemes, "Unknown scheme: %s" % scheme
         assert not (self.private and scheme in public_schemes), (
             "Private branch %s has no public URL." % self.unique_name)
-
-        host = URI(config.codehosting.supermirror_root).host
-        path = '/' + self.unique_name
-
-        return str(URI(scheme=scheme, host=host, path=path))
+        return compose_public_url(scheme, self.unique_name)
 
     @property
     def warehouse_url(self):
@@ -1309,3 +1298,18 @@
     """
     update_trigger_modified_fields(branch)
     send_branch_modified_notifications(branch, event)
+
+
+def compose_public_url(scheme, unique_name, suffix=None):
+    # Avoid circular imports.
+    from lp.code.xmlrpc.branch import PublicCodehostingAPI
+
+    # Accept sftp as a legacy protocol.
+    accepted_schemes = set(PublicCodehostingAPI.supported_schemes)
+    accepted_schemes.add('sftp')
+    assert scheme in accepted_schemes, "Unknown scheme: %s" % scheme
+    host = URI(config.codehosting.supermirror_root).host
+    path = '/' + unique_name
+    if suffix is not None:
+        path = os.path.join(path, suffix)
+    return str(URI(scheme=scheme, host=host, path=path))

=== modified file 'lib/lp/code/xmlrpc/branch.py'
--- lib/lp/code/xmlrpc/branch.py	2009-07-17 00:26:05 +0000
+++ lib/lp/code/xmlrpc/branch.py	2010-02-16 20:47:18 +0000
@@ -11,13 +11,10 @@
     'BranchSetAPI', 'IBranchSetAPI', 'IPublicCodehostingAPI',
     'PublicCodehostingAPI']
 
-import os
 
 from zope.component import getUtility
 from zope.interface import Interface, implements
 
-from lazr.uri import URI
-
 from canonical.config import config
 from lp.bugs.interfaces.bug import IBugSet
 from canonical.launchpad.webapp.interfaces import ILaunchBag, NotFoundError
@@ -185,10 +182,6 @@
 
     supported_schemes = 'bzr+ssh', 'http'
 
-    def _getBazaarHost(self):
-        """Return the hostname for the codehosting server."""
-        return URI(config.codehosting.supermirror_root).host
-
     def _getResultDict(self, branch, suffix=None, supported_schemes=None):
         """Return a result dict with a list of URLs for the given branch.
 
@@ -207,16 +200,13 @@
 
     def _getUniqueNameResultDict(self, unique_name, suffix=None,
                                  supported_schemes=None):
+        from lp.code.model.branch import compose_public_url
         if supported_schemes is None:
             supported_schemes = self.supported_schemes
         result = dict(urls=[])
-        host = self._getBazaarHost()
-        path = '/' + unique_name
-        if suffix is not None:
-            path = os.path.join(path, suffix)
         for scheme in supported_schemes:
             result['urls'].append(
-                str(URI(host=host, scheme=scheme, path=path)))
+                compose_public_url(scheme, unique_name, suffix))
         return result
 
     @return_fault