Launchpad itself

Merge lp:~thumper/launchpad/xmlrpc-lp-name-resolution into lp:launchpad

  1. xmlrpc-lp-name-resolution
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: 11516
Proposed branch: lp:~thumper/launchpad/xmlrpc-lp-name-resolution
Merge into: lp:launchpad
Diff against target: 582 lines (+171/-136)
4 files modified
lib/lp/code/interfaces/codehosting.py (+26/-0)
lib/lp/code/model/branch.py (+1/-18)
lib/lp/code/xmlrpc/branch.py (+42/-14)
lib/lp/code/xmlrpc/tests/test_branch.py (+102/-104)
To merge this branch: bzr merge lp:~thumper/launchpad/xmlrpc-lp-name-resolution
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+34500@code.launchpad.net

Commit message

Resolve linked branches using the +branch alias over XMLRPC

Description of the change

This branch changes the way the XMLRPC server resolves the lp: urls where the alias for a branch is used. lp:~user/... are still resolved the same way, as are public http urls.
However for bzr+ssh urls, branches that are linked to series (either distroseries or product series) are now resolved to bzr+ssh://bazaar.launchpad.net/+branch/<alias> where <alias> is the bit after 'lp:' on the bazaar identity.

This is going to be used to allow private trunk branches to use the lp:<project> syntax for accessing branches.

tests:
  TestExpandURL

As a drive-by the compose_public_url function was moved into lp.code.interfaces.codehosting, which avoids the non-standard import statements that were there to avoid the circular dependencies.

Most of the changes are updates to the tests.

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Hi,

I'm a bit confused about the whole supported_schemes stuff, it seems overly hairy. But it's not new.

The docstring of assertRaises has become a bit garbled, can you have a proof read and fix it up?

Do we have a codehosting acceptance test that accesses a branch by its lp: urls? Maybe we should.

Otherwise all fine. People will be very happy about this :-)

Cheers,
mwh

review: Approve
Revision history for this message
Tim Penhey (thumper) wrote :

We do have acceptance tests that test that the name we resolve to works for branch access. That landed as part of the branch which supports the +branch alias file access over the VFS.

I'll look at the docstring.

Yes it is a bit hairy, but I was trying to work out how to best provide the URLs we are after without losing the flexibility to add something like the anon-bzr protocol.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/interfaces/codehosting.py'
2--- lib/lp/code/interfaces/codehosting.py 2010-08-20 20:31:18 +0000
3+++ lib/lp/code/interfaces/codehosting.py 2010-09-03 04:25:59 +0000
4@@ -9,17 +9,24 @@
5 __all__ = [
6 'BRANCH_ALIAS_PREFIX',
7 'BRANCH_TRANSPORT',
8+ 'compose_public_url',
9 'CONTROL_TRANSPORT',
10 'ICodehostingAPI',
11 'ICodehostingApplication',
12 'LAUNCHPAD_ANONYMOUS',
13 'LAUNCHPAD_SERVICES',
14 'READ_ONLY',
15+ 'SUPPORTED_SCHEMES',
16 'WRITABLE',
17 ]
18
19+import os.path
20+import urllib
21+
22+from lazr.uri import URI
23 from zope.interface import Interface
24
25+from canonical.config import config
26 from canonical.launchpad.validators.name import valid_name
27 from canonical.launchpad.webapp.interfaces import ILaunchpadApplication
28
29@@ -49,6 +56,9 @@
30 # The path prefix for getting at branches via their short name.
31 BRANCH_ALIAS_PREFIX = '+branch'
32
33+# The scheme types that are supported for codehosting.
34+SUPPORTED_SCHEMES = 'bzr+ssh', 'http'
35+
36
37 class ICodehostingApplication(ILaunchpadApplication):
38 """Branch Puller application root."""
39@@ -170,3 +180,19 @@
40 'path_in_transport' is a path relative to that transport. e.g.
41 (BRANCH_TRANSPORT, {'id': 3, 'writable': False}, '.bzr/README').
42 """
43+
44+
45+def compose_public_url(scheme, unique_name, suffix=None):
46+ # Accept sftp as a legacy protocol.
47+ accepted_schemes = set(SUPPORTED_SCHEMES)
48+ accepted_schemes.add('sftp')
49+ assert scheme in accepted_schemes, "Unknown scheme: %s" % scheme
50+ host = URI(config.codehosting.supermirror_root).host
51+ if isinstance(unique_name, unicode):
52+ unique_name = unique_name.encode('utf-8')
53+ # After quoting and encoding, the path should be perfectly
54+ # safe as a plain ASCII string, str() just enforces this
55+ path = '/' + str(urllib.quote(unique_name, safe='/~+'))
56+ if suffix:
57+ path = os.path.join(path, suffix)
58+ return str(URI(scheme=scheme, host=host, path=path))
59
60=== modified file 'lib/lp/code/model/branch.py'
61--- lib/lp/code/model/branch.py 2010-08-27 02:11:36 +0000
62+++ lib/lp/code/model/branch.py 2010-09-03 04:25:59 +0000
63@@ -7,15 +7,12 @@
64 __all__ = [
65 'Branch',
66 'BranchSet',
67- 'compose_public_url',
68 ]
69
70 from datetime import datetime
71-import os.path
72
73 from bzrlib import urlutils
74 from bzrlib.revision import NULL_REVISION
75-from lazr.uri import URI
76 import pytz
77 from sqlobject import (
78 BoolCol,
79@@ -108,6 +105,7 @@
80 from lp.code.interfaces.branchnamespace import IBranchNamespacePolicy
81 from lp.code.interfaces.branchpuller import IBranchPuller
82 from lp.code.interfaces.branchtarget import IBranchTarget
83+from lp.code.interfaces.codehosting import compose_public_url
84 from lp.code.interfaces.seriessourcepackagebranch import (
85 IFindOfficialBranchLinks,
86 )
87@@ -1324,18 +1322,3 @@
88 """
89 update_trigger_modified_fields(branch)
90 send_branch_modified_notifications(branch, event)
91-
92-
93-def compose_public_url(scheme, unique_name, suffix=None):
94- # Avoid circular imports.
95- from lp.code.xmlrpc.branch import PublicCodehostingAPI
96-
97- # Accept sftp as a legacy protocol.
98- accepted_schemes = set(PublicCodehostingAPI.supported_schemes)
99- accepted_schemes.add('sftp')
100- assert scheme in accepted_schemes, "Unknown scheme: %s" % scheme
101- host = URI(config.codehosting.supermirror_root).host
102- path = '/' + urlutils.escape(unique_name)
103- if suffix:
104- path = os.path.join(path, suffix)
105- return str(URI(scheme=scheme, host=host, path=path))
106
107=== modified file 'lib/lp/code/xmlrpc/branch.py'
108--- lib/lp/code/xmlrpc/branch.py 2010-08-20 20:31:18 +0000
109+++ lib/lp/code/xmlrpc/branch.py 2010-09-03 04:25:59 +0000
110@@ -8,8 +8,11 @@
111
112 __metaclass__ = type
113 __all__ = [
114- 'BranchSetAPI', 'IBranchSetAPI', 'IPublicCodehostingAPI',
115- 'PublicCodehostingAPI']
116+ 'BranchSetAPI',
117+ 'IBranchSetAPI',
118+ 'IPublicCodehostingAPI',
119+ 'PublicCodehostingAPI',
120+ ]
121
122
123 from bzrlib import urlutils
124@@ -42,6 +45,11 @@
125 from lp.code.interfaces.branch import IBranch
126 from lp.code.interfaces.branchlookup import IBranchLookup
127 from lp.code.interfaces.branchnamespace import get_branch_namespace
128+from lp.code.interfaces.codehosting import (
129+ BRANCH_ALIAS_PREFIX,
130+ compose_public_url,
131+ SUPPORTED_SCHEMES,
132+ )
133 from lp.registry.interfaces.distroseries import NoSuchDistroSeries
134 from lp.registry.interfaces.person import (
135 IPersonSet,
136@@ -171,8 +179,7 @@
137 def resolve_lp_path(path):
138 """Expand the path segment of an lp: URL into a list of branch URLs.
139
140- This method is added to support Bazaar 0.93. It cannot be removed
141- until we stop supporting Bazaar 0.93.
142+ This method is added to Bazaar in 0.93.
143
144 :return: A dict containing a single 'urls' key that maps to a list of
145 URLs. Clients should use the first URL in the list that they can
146@@ -194,12 +201,25 @@
147
148 implements(IPublicCodehostingAPI)
149
150- supported_schemes = 'bzr+ssh', 'http'
151-
152- def _getResultDict(self, branch, suffix=None, supported_schemes=None):
153+ def _compose_http_url(unique_name, path, suffix):
154+ return compose_public_url('http', unique_name, suffix)
155+
156+ def _compose_bzr_ssh_url(unique_name, path, suffix):
157+ if not path.startswith('~'):
158+ path = '%s/%s' % (BRANCH_ALIAS_PREFIX, path)
159+ return compose_public_url('bzr+ssh', path, suffix)
160+
161+ scheme_funcs = {
162+ 'bzr+ssh': _compose_bzr_ssh_url,
163+ 'http': _compose_http_url,
164+ }
165+
166+ def _getResultDict(self, branch, lp_path, suffix=None,
167+ supported_schemes=None):
168 """Return a result dict with a list of URLs for the given branch.
169
170 :param branch: A Branch object.
171+ :param lp_path: The path that was used to traverse to the branch.
172 :param suffix: The section of the path that follows the branch
173 specification.
174 :return: {'urls': [list_of_branch_urls]}.
175@@ -210,17 +230,18 @@
176 return dict(urls=[branch.url])
177 else:
178 return self._getUniqueNameResultDict(
179- branch.unique_name, suffix, supported_schemes)
180+ branch.unique_name, suffix, supported_schemes, lp_path)
181
182 def _getUniqueNameResultDict(self, unique_name, suffix=None,
183- supported_schemes=None):
184- from lp.code.model.branch import compose_public_url
185+ supported_schemes=None, path=None):
186 if supported_schemes is None:
187- supported_schemes = self.supported_schemes
188+ supported_schemes = SUPPORTED_SCHEMES
189+ if path is None:
190+ path = unique_name
191 result = dict(urls=[])
192 for scheme in supported_schemes:
193 result['urls'].append(
194- compose_public_url(scheme, unique_name, suffix))
195+ self.scheme_funcs[scheme](unique_name, path, suffix))
196 return result
197
198 @return_fault
199@@ -233,7 +254,7 @@
200 strip_path = path.strip('/')
201 if strip_path == '':
202 raise faults.InvalidBranchIdentifier(path)
203- supported_schemes = self.supported_schemes
204+ supported_schemes = SUPPORTED_SCHEMES
205 hot_products = [product.strip() for product
206 in config.codehosting.hot_products.split(',')]
207 if strip_path in hot_products:
208@@ -270,7 +291,14 @@
209 raise faults.CannotHaveLinkedBranch(e.component)
210 except InvalidNamespace, e:
211 raise faults.InvalidBranchUniqueName(urlutils.escape(e.name))
212- return self._getResultDict(branch, suffix, supported_schemes)
213+ # Reverse engineer the actual lp_path that is used, so we need to
214+ # remove any suffix that may be there from the strip_path.
215+ lp_path = strip_path
216+ if suffix is not None:
217+ # E.g. 'project/trunk/filename.txt' the suffix is 'filename.txt'
218+ # we want lp_path to be 'project/trunk'.
219+ lp_path = lp_path[:-(len(suffix)+1)]
220+ return self._getResultDict(branch, lp_path, suffix, supported_schemes)
221
222 def resolve_lp_path(self, path):
223 """See `IPublicCodehostingAPI`."""
224
225=== modified file 'lib/lp/code/xmlrpc/tests/test_branch.py'
226--- lib/lp/code/xmlrpc/tests/test_branch.py 2010-08-20 20:31:18 +0000
227+++ lib/lp/code/xmlrpc/tests/test_branch.py 2010-09-03 04:25:59 +0000
228@@ -3,6 +3,8 @@
229
230 """Unit tests for the public codehosting API."""
231
232+from __future__ import with_statement
233+
234 __metaclass__ = type
235 __all__ = []
236
237@@ -15,17 +17,14 @@
238 from lazr.uri import URI
239 from zope.security.proxy import removeSecurityProxy
240
241-from canonical.config import config
242-from canonical.launchpad.ftests import (
243- login,
244- logout,
245- )
246 from canonical.launchpad.xmlrpc import faults
247 from canonical.testing import DatabaseFunctionalLayer
248 from lp.code.enums import BranchType
249+from lp.code.interfaces.codehosting import BRANCH_ALIAS_PREFIX
250+from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
251 from lp.code.xmlrpc.branch import PublicCodehostingAPI
252 from lp.services.xmlrpc import LaunchpadFault
253-from lp.testing import TestCaseWithFactory
254+from lp.testing import person_logged_in, TestCaseWithFactory
255
256
257 NON_ASCII_NAME = u'nam\N{LATIN SMALL LETTER E WITH ACUTE}'
258@@ -36,43 +35,45 @@
259
260 layer = DatabaseFunctionalLayer
261
262- def setUp(self):
263- """Set up the fixture for these unit tests.
264-
265- - 'project' is an arbitrary Launchpad project.
266- - 'trunk' is a branch on 'project', associated with the development
267- focus.
268+ def makeProdutWithTrunk(self):
269+ """Make a new project with a trunk hosted branch."""
270+ product = self.factory.makeProduct()
271+ # BranchType is only signficiant insofar as it is not a REMOTE branch.
272+ trunk = self.factory.makeProductBranch(
273+ branch_type=BranchType.HOSTED, product=product)
274+ with person_logged_in(product.owner):
275+ ICanHasLinkedBranch(product).setBranch(trunk)
276+ return product, trunk
277+
278+ def assertResolves(self, lp_url_path, public_branch_path, lp_path=None):
279+ """Assert that `lp_url_path` resolves to the specified paths.
280+
281+ :param public_branch_path: The path that is accessible over http.
282+ :param lp_path: The short branch alias that will be resolved over
283+ bzr+ssh. The branch alias prefix is prefixed to this path.
284+ If it is not set, the bzr+ssh resolved name will be checked
285+ against the public_branch_path instead.
286 """
287- TestCaseWithFactory.setUp(self)
288- self.api = PublicCodehostingAPI(None, None)
289- self.product = self.factory.makeProduct()
290- # Associate 'trunk' with the product's development focus. Use
291- # removeSecurityProxy so that we can assign directly to branch.
292- trunk_series = removeSecurityProxy(self.product).development_focus
293- # BranchType is only signficiant insofar as it is not a REMOTE branch.
294- trunk_series.branch = (
295- self.factory.makeProductBranch(
296- branch_type=BranchType.HOSTED, product=self.product))
297-
298- def makePrivateBranch(self, **kwargs):
299- """Create an arbitrary private branch using `makeBranch`."""
300- branch = self.factory.makeAnyBranch(**kwargs)
301- naked_branch = removeSecurityProxy(branch)
302- naked_branch.private = True
303- return branch
304-
305- def assertResolves(self, lp_url_path, unique_name):
306- """Assert that `lp_url_path` path expands to `unique_name`."""
307- results = self.api.resolve_lp_path(lp_url_path)
308+ api = PublicCodehostingAPI(None, None)
309+ results = api.resolve_lp_path(lp_url_path)
310+ if lp_path is None:
311+ ssh_branch_path = public_branch_path
312+ else:
313+ ssh_branch_path = '%s/%s' % (BRANCH_ALIAS_PREFIX, lp_path)
314 # This improves the error message if results happens to be a fault.
315 if isinstance(results, LaunchpadFault):
316 raise results
317 for url in results['urls']:
318- self.assertEqual('/' + unique_name, URI(url).path)
319+ uri = URI(url)
320+ if uri.scheme == 'http':
321+ self.assertEqual('/' + public_branch_path, uri.path)
322+ else:
323+ self.assertEqual('/' + ssh_branch_path, uri.path)
324
325 def assertFault(self, lp_url_path, expected_fault):
326 """Trying to resolve lp_url_path raises the expected fault."""
327- fault = self.api.resolve_lp_path(lp_url_path)
328+ api = PublicCodehostingAPI(None, None)
329+ fault = api.resolve_lp_path(lp_url_path)
330 self.assertTrue(
331 isinstance(fault, xmlrpclib.Fault),
332 "resolve_lp_path(%r) returned %r, not a Fault."
333@@ -87,35 +88,39 @@
334 # containing a list of these URLs, with the faster and more featureful
335 # URLs earlier in the list. We use a dict so we can easily add more
336 # information in the future.
337- trunk = self.product.development_focus.branch
338- results = self.api.resolve_lp_path(self.product.name)
339+ product, trunk = self.makeProdutWithTrunk()
340+ api = PublicCodehostingAPI(None, None)
341+ results = api.resolve_lp_path(product.name)
342 urls = [
343- 'bzr+ssh://bazaar.launchpad.dev/%s' % trunk.unique_name,
344+ 'bzr+ssh://bazaar.launchpad.dev/+branch/%s' % product.name,
345 'http://bazaar.launchpad.dev/%s' % trunk.unique_name]
346 self.assertEqual(dict(urls=urls), results)
347
348 def test_resultDictForHotProduct(self):
349 # If 'project-name' is in the config.codehosting.hot_products list,
350 # lp:project-name will only resolve to the http url.
351- config.push(
352- 'hot_product',
353- '[codehosting]\nhot_products: %s '% self.product.name)
354- self.addCleanup(config.pop, 'hot_product')
355- trunk = self.product.development_focus.branch
356- results = self.api.resolve_lp_path(self.product.name)
357+ product, trunk = self.makeProdutWithTrunk()
358+ self.pushConfig('codehosting', hot_products=product.name)
359+ api = PublicCodehostingAPI(None, None)
360+ results = api.resolve_lp_path(product.name)
361 http_url = 'http://bazaar.launchpad.dev/%s' % trunk.unique_name
362 self.assertEqual(dict(urls=[http_url]), results)
363
364 def test_product_only(self):
365 # lp:product expands to the branch associated with development focus
366- # of the product.
367- trunk = self.product.development_focus.branch
368- self.assertResolves(self.product.name, trunk.unique_name)
369- trunk_series = removeSecurityProxy(self.product).development_focus
370- trunk_series.branch = self.factory.makeProductBranch(
371- branch_type=BranchType.HOSTED, product=self.product)
372- self.assertResolves(
373- self.product.name, trunk_series.branch.unique_name)
374+ # of the product for the anonymous public access, just to the aliased
375+ # short name for bzr+ssh access.
376+ product, trunk = self.makeProdutWithTrunk()
377+ lp_path = product.name
378+ self.assertResolves(lp_path, trunk.unique_name, lp_path)
379+
380+ def test_product_explicit_dev_series(self):
381+ # lp:product/development_focus expands to the branch associated with
382+ # development focus of the product for the anonymous public access,
383+ # just to the aliased short name for bzr+ssh access.
384+ product, trunk = self.makeProdutWithTrunk()
385+ lp_path='%s/%s' % (product.name, product.development_focus.name)
386+ self.assertResolves(lp_path, trunk.unique_name, lp_path)
387
388 def test_product_doesnt_exist(self):
389 # Return a NoSuchProduct fault if the product doesn't exist.
390@@ -165,18 +170,11 @@
391 def test_product_and_series(self):
392 # lp:product/series expands to the branch associated with the product
393 # series 'series' on 'product'.
394- series = self.factory.makeSeries(
395- product=self.product,
396- branch=self.factory.makeProductBranch(product=self.product))
397- self.assertResolves(
398- '%s/%s' % (self.product.name, series.name),
399- series.branch.unique_name)
400-
401- # We can also use product/series notation to reach trunk.
402- self.assertResolves(
403- '%s/%s' % (self.product.name,
404- self.product.development_focus.name),
405- self.product.development_focus.branch.unique_name)
406+ product = self.factory.makeProduct()
407+ branch = branch=self.factory.makeProductBranch(product=product)
408+ series = self.factory.makeSeries(product=product, branch=branch)
409+ lp_path = '%s/%s' % (product.name, series.name)
410+ self.assertResolves(lp_path, branch.unique_name, lp_path)
411
412 def test_development_focus_has_no_branch(self):
413 # Return a NoLinkedBranch fault if the development focus has no branch
414@@ -196,17 +194,19 @@
415 def test_no_such_product_series(self):
416 # Return a NoSuchProductSeries fault if there is no series of the
417 # given name associated with the product.
418+ product = self.factory.makeProduct()
419 self.assertFault(
420- '%s/%s' % (self.product.name, "doesntexist"),
421- faults.NoSuchProductSeries("doesntexist", self.product))
422+ '%s/%s' % (product.name, "doesntexist"),
423+ faults.NoSuchProductSeries("doesntexist", product))
424
425 def test_no_such_product_series_non_ascii(self):
426 # lp:product/<non-ascii-string> returns NoSuchProductSeries with the
427 # name escaped.
428+ product = self.factory.makeProduct()
429 self.assertFault(
430- '%s/%s' % (self.product.name, NON_ASCII_NAME),
431+ '%s/%s' % (product.name, NON_ASCII_NAME),
432 faults.NoSuchProductSeries(
433- urlutils.escape(NON_ASCII_NAME), self.product))
434+ urlutils.escape(NON_ASCII_NAME), product))
435
436 def test_no_such_distro_series(self):
437 # Return a NoSuchDistroSeries fault if there is no series of the given
438@@ -254,34 +254,37 @@
439 def test_branch(self):
440 # The unique name of a branch resolves to the unique name of the
441 # branch.
442- arbitrary_branch = self.factory.makeAnyBranch()
443- self.assertResolves(
444- arbitrary_branch.unique_name, arbitrary_branch.unique_name)
445- trunk = self.product.development_focus.branch
446+ branch = self.factory.makeAnyBranch()
447+ self.assertResolves(branch.unique_name, branch.unique_name)
448+
449+ def test_trunk_accessed_as_branch(self):
450+ # A branch that is the development focus for any product can also be
451+ # accessed through the branch's unique_name.
452+ _ignored, trunk = self.makeProdutWithTrunk()
453 self.assertResolves(trunk.unique_name, trunk.unique_name)
454
455 def test_mirrored_branch(self):
456 # The unique name of a mirrored branch resolves to the unique name of
457 # the branch.
458- arbitrary_branch = self.factory.makeAnyBranch(
459- branch_type=BranchType.MIRRORED)
460- self.assertResolves(
461- arbitrary_branch.unique_name, arbitrary_branch.unique_name)
462+ branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
463+ self.assertResolves(branch.unique_name, branch.unique_name)
464
465 def test_no_such_branch_product(self):
466 # Resolve paths to branches even if there is no branch of that name.
467 # We do this so that users can push new branches to lp: URLs.
468 owner = self.factory.makePerson()
469+ product = self.factory.makeProduct()
470 nonexistent_branch = '~%s/%s/doesntexist' % (
471- owner.name, self.product.name)
472+ owner.name, product.name)
473 self.assertResolves(nonexistent_branch, nonexistent_branch)
474
475 def test_no_such_branch_product_non_ascii(self):
476 # A path to a branch that contains non ascii characters will never
477 # find a branch, but it still resolves rather than erroring.
478 owner = self.factory.makePerson()
479+ product = self.factory.makeProduct()
480 nonexistent_branch = u'~%s/%s/%s' % (
481- owner.name, self.product.name, NON_ASCII_NAME)
482+ owner.name, product.name, NON_ASCII_NAME)
483 self.assertResolves(
484 nonexistent_branch, urlutils.escape(nonexistent_branch))
485
486@@ -291,8 +294,7 @@
487 # the '+junk' project, which doesn't actually exist.
488 owner = self.factory.makePerson()
489 nonexistent_branch = '~%s/+junk/doesntexist' % owner.name
490- self.assertResolves(
491- nonexistent_branch, urlutils.escape(nonexistent_branch))
492+ self.assertResolves(nonexistent_branch, nonexistent_branch)
493
494 def test_no_such_branch_package(self):
495 # Resolve paths to package branches even if there's no branch of that
496@@ -374,26 +376,26 @@
497 def test_trailing_slashes(self):
498 # Trailing slashes are trimmed.
499 # Trailing slashes on lp:product//
500- trunk = self.product.development_focus.branch
501- self.assertResolves(self.product.name + '/', trunk.unique_name)
502- self.assertResolves(self.product.name + '//', trunk.unique_name)
503+ product, trunk = self.makeProdutWithTrunk()
504+ self.assertResolves(
505+ product.name + '/', trunk.unique_name, product.name)
506+ self.assertResolves(
507+ product.name + '//', trunk.unique_name, product.name)
508
509 # Trailing slashes on lp:~owner/product/branch//
510- arbitrary_branch = self.factory.makeAnyBranch()
511- self.assertResolves(
512- arbitrary_branch.unique_name + '/', arbitrary_branch.unique_name)
513- self.assertResolves(
514- arbitrary_branch.unique_name + '//', arbitrary_branch.unique_name)
515+ branch = self.factory.makeAnyBranch()
516+ self.assertResolves(branch.unique_name + '/', branch.unique_name)
517+ self.assertResolves(branch.unique_name + '//', branch.unique_name)
518
519 def test_private_branch(self):
520 # Invisible branches are resolved as if they didn't exist, so that we
521 # reveal the least possile amount of information about them.
522 # For fully specified branch names, this means resolving the lp url.
523- arbitrary_branch = self.makePrivateBranch()
524+ branch = self.factory.makeAnyBranch(private=True)
525 # Removing security proxy to get at the unique_name attribute of a
526 # private branch, and tests are currently running as an anonymous
527 # user.
528- unique_name = removeSecurityProxy(arbitrary_branch).unique_name
529+ unique_name = removeSecurityProxy(branch).unique_name
530 self.assertResolves(unique_name, unique_name)
531
532 def test_private_branch_on_series(self):
533@@ -404,7 +406,7 @@
534 # Removing security proxy because we need to be able to get at
535 # attributes of a private branch and these tests are running as an
536 # anonymous user.
537- branch = removeSecurityProxy(self.makePrivateBranch())
538+ branch = self.factory.makeAnyBranch(private=True)
539 series = self.factory.makeSeries(branch=branch)
540 self.assertFault(
541 '%s/%s' % (series.product.name, series.name),
542@@ -417,11 +419,9 @@
543 # development focus. If that branch is private, other views will
544 # indicate that there is no branch on the development focus. We do the
545 # same.
546- trunk = self.product.development_focus.branch
547- naked_trunk = removeSecurityProxy(trunk)
548- naked_trunk.private = True
549- self.assertFault(
550- self.product.name, faults.NoLinkedBranch(self.product))
551+ product, trunk = self.makeProdutWithTrunk()
552+ removeSecurityProxy(trunk).private = True
553+ self.assertFault(product.name, faults.NoLinkedBranch(product))
554
555 def test_private_branch_as_user(self):
556 # We resolve invisible branches as if they don't exist.
557@@ -433,19 +433,17 @@
558 #
559 # Create the owner explicitly so that we can get its email without
560 # resorting to removeSecurityProxy.
561- email = self.factory.getUniqueEmailAddress()
562- arbitrary_branch = self.makePrivateBranch(
563- owner=self.factory.makePerson(email=email))
564- login(email)
565- self.addCleanup(logout)
566- self.assertResolves(
567- arbitrary_branch.unique_name, arbitrary_branch.unique_name)
568+ owner = self.factory.makePerson()
569+ branch = self.factory.makeAnyBranch(owner=owner, private=True)
570+ path = removeSecurityProxy(branch).unique_name
571+ self.assertResolves(path, path)
572
573 def test_remote_branch(self):
574 # For remote branches, return results that link to the actual remote
575 # branch URL.
576 branch = self.factory.makeAnyBranch(branch_type=BranchType.REMOTE)
577- result = self.api.resolve_lp_path(branch.unique_name)
578+ api = PublicCodehostingAPI(None, None)
579+ result = api.resolve_lp_path(branch.unique_name)
580 self.assertEqual([branch.url], result['urls'])
581
582 def test_remote_branch_no_url(self):