Launchpad itself

Merge lp:~thumper/launchpad/stack-on-branch-id-alias into lp:launchpad

  1. stack-on-branch-id-alias
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 12874
Proposed branch: lp:~thumper/launchpad/stack-on-branch-id-alias
Merge into: lp:launchpad
Diff against target: 340 lines (+44/-32)
9 files modified
lib/lp/code/interfaces/codehosting.py (+6/-0)
lib/lp/code/model/tests/test_branch.py (+2/-3)
lib/lp/code/model/tests/test_branchlookup.py (+8/-5)
lib/lp/code/xmlrpc/codehosting.py (+3/-2)
lib/lp/code/xmlrpc/tests/test_codehosting.py (+12/-12)
lib/lp/codehosting/inmemory.py (+3/-1)
lib/lp/codehosting/tests/test_rewrite.py (+5/-6)
lib/lp/codehosting/vfs/tests/test_branchfs.py (+2/-1)
lib/lp/codehosting/vfs/tests/test_filesystem.py (+3/-2)
To merge this branch: bzr merge lp:~thumper/launchpad/stack-on-branch-id-alias
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+57621@code.launchpad.net

Commit message

[r=bac][bug=377519] Change the default stacking location to use the branch id alias.

Description of the change

This branch changes the default stacked on location to use the name independent /+branch-id/%(id)s alias.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Tim,

What would you think of a property on IBranch that returned this new value? You construct it a lot here and it'd be nice to do it once.

Approved pending your consideration.

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

Brad,

Instead of fattening the IBranch interface, I added a function in the codehosting interface module that does the assembly. Updated the tests and other call sites to use the new function.

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 2011-04-04 04:14:02 +0000
3+++ lib/lp/code/interfaces/codehosting.py 2011-04-18 23:07:52 +0000
4@@ -8,6 +8,7 @@
5 __metaclass__ = type
6 __all__ = [
7 'BRANCH_ALIAS_PREFIX',
8+ 'branch_id_alias',
9 'BRANCH_ID_ALIAS_PREFIX',
10 'BRANCH_TRANSPORT',
11 'compose_public_url',
12@@ -60,6 +61,11 @@
13 BRANCH_ID_ALIAS_PREFIX = '+branch-id'
14
15
16+def branch_id_alias(branch):
17+ """Return the path using the branch id alias."""
18+ return '/%s/%s' % (BRANCH_ID_ALIAS_PREFIX, branch.id)
19+
20+
21 # The scheme types that are supported for codehosting.
22 SUPPORTED_SCHEMES = 'bzr+ssh', 'http'
23
24
25=== modified file 'lib/lp/code/model/tests/test_branch.py'
26--- lib/lp/code/model/tests/test_branch.py 2011-04-05 04:01:46 +0000
27+++ lib/lp/code/model/tests/test_branch.py 2011-04-18 23:07:52 +0000
28@@ -79,7 +79,7 @@
29 )
30 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
31 from lp.code.interfaces.branchrevision import IBranchRevision
32-from lp.code.interfaces.codehosting import BRANCH_ID_ALIAS_PREFIX
33+from lp.code.interfaces.codehosting import branch_id_alias
34 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
35 from lp.code.interfaces.seriessourcepackagebranch import (
36 IFindOfficialBranchLinks,
37@@ -187,8 +187,7 @@
38 branch = self.factory.makeAnyBranch()
39 stacked_on = self.factory.makeAnyBranch()
40 login_person(branch.owner)
41- stacked_on_location = '/%s/%s' % (
42- BRANCH_ID_ALIAS_PREFIX, stacked_on.id)
43+ stacked_on_location = branch_id_alias(stacked_on)
44 branch.branchChanged(stacked_on_location, '', *self.arbitrary_formats)
45 self.assertEqual(stacked_on, branch.stacked_on)
46
47
48=== modified file 'lib/lp/code/model/tests/test_branchlookup.py'
49--- lib/lp/code/model/tests/test_branchlookup.py 2011-04-05 23:35:12 +0000
50+++ lib/lp/code/model/tests/test_branchlookup.py 2011-04-18 23:07:52 +0000
51@@ -25,7 +25,10 @@
52 ILinkedBranchTraverser,
53 )
54 from lp.code.interfaces.branchnamespace import get_branch_namespace
55-from lp.code.interfaces.codehosting import BRANCH_ID_ALIAS_PREFIX
56+from lp.code.interfaces.codehosting import (
57+ branch_id_alias,
58+ BRANCH_ID_ALIAS_PREFIX,
59+ )
60 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
61 from lp.registry.errors import (
62 NoSuchDistroSeries,
63@@ -137,28 +140,28 @@
64 owner = self.factory.makePerson()
65 branch = self.factory.makeAnyBranch(owner=owner, private=True)
66 with person_logged_in(owner):
67- path = '/%s/%s' % (BRANCH_ID_ALIAS_PREFIX, branch.id)
68+ path = branch_id_alias(branch)
69 result = self.branch_set.getIdAndTrailingPath(path)
70 self.assertEqual((None, None), result)
71
72 def test_branch_id_alias_public(self):
73 # Public branches can be accessed.
74 branch = self.factory.makeAnyBranch()
75- path = '/%s/%s' % (BRANCH_ID_ALIAS_PREFIX, branch.id)
76+ path = branch_id_alias(branch)
77 result = self.branch_set.getIdAndTrailingPath(path)
78 self.assertEqual((branch.id, ''), result)
79
80 def test_branch_id_alias_public_slash(self):
81 # A trailing slash is returned as the extra path.
82 branch = self.factory.makeAnyBranch()
83- path = '/%s/%s/' % (BRANCH_ID_ALIAS_PREFIX, branch.id)
84+ path = '%s/' % branch_id_alias(branch)
85 result = self.branch_set.getIdAndTrailingPath(path)
86 self.assertEqual((branch.id, '/'), result)
87
88 def test_branch_id_alias_public_with_path(self):
89 # All the path after the number is returned as the trailing path.
90 branch = self.factory.makeAnyBranch()
91- path = '/%s/%s/foo' % (BRANCH_ID_ALIAS_PREFIX, branch.id)
92+ path = '%s/foo' % branch_id_alias(branch)
93 result = self.branch_set.getIdAndTrailingPath(path)
94 self.assertEqual((branch.id, '/foo'), result)
95
96
97=== modified file 'lib/lp/code/xmlrpc/codehosting.py'
98--- lib/lp/code/xmlrpc/codehosting.py 2011-04-05 00:50:41 +0000
99+++ lib/lp/code/xmlrpc/codehosting.py 2011-04-18 23:07:52 +0000
100@@ -59,6 +59,7 @@
101 from lp.code.interfaces.branchtarget import IBranchTarget
102 from lp.code.interfaces.codehosting import (
103 BRANCH_ALIAS_PREFIX,
104+ branch_id_alias,
105 BRANCH_ID_ALIAS_PREFIX,
106 BRANCH_TRANSPORT,
107 CONTROL_TRANSPORT,
108@@ -320,12 +321,12 @@
109 if default_branch is None:
110 return
111 try:
112- unique_name = default_branch.unique_name
113+ path = branch_id_alias(default_branch)
114 except Unauthorized:
115 return
116 return (
117 CONTROL_TRANSPORT,
118- {'default_stack_on': escape('/' + unique_name)},
119+ {'default_stack_on': escape(path)},
120 trailing_path)
121
122 def _translateBranchIdAlias(self, requester, path):
123
124=== modified file 'lib/lp/code/xmlrpc/tests/test_codehosting.py'
125--- lib/lp/code/xmlrpc/tests/test_codehosting.py 2011-04-05 23:53:46 +0000
126+++ lib/lp/code/xmlrpc/tests/test_codehosting.py 2011-04-18 23:07:52 +0000
127@@ -41,6 +41,7 @@
128 from lp.code.interfaces.branchtarget import IBranchTarget
129 from lp.code.interfaces.codehosting import (
130 BRANCH_ALIAS_PREFIX,
131+ branch_id_alias,
132 BRANCH_ID_ALIAS_PREFIX,
133 BRANCH_TRANSPORT,
134 CONTROL_TRANSPORT,
135@@ -993,8 +994,8 @@
136 self.assertNotFound(requester, '/%s/.bzr' % BRANCH_ALIAS_PREFIX)
137
138 def test_translatePath_branch_id_alias_bzrdir_content(self):
139- # translatePath('/+branch-id/.bzr/.*') *must* return not found, otherwise
140- # bzr will look for it and we don't have a global bzr dir.
141+ # translatePath('/+branch-id/.bzr/.*') *must* return not found,
142+ # otherwise bzr will look for it and we don't have a global bzr dir.
143 requester = self.factory.makePerson()
144 self.assertNotFound(
145 requester, '/%s/.bzr/branch-format' % BRANCH_ID_ALIAS_PREFIX)
146@@ -1009,7 +1010,7 @@
147 # Make sure the trailing path is returned.
148 requester = self.factory.makePerson()
149 branch = removeSecurityProxy(self.factory.makeAnyBranch())
150- path = escape(u'/%s/%s/foo/bar' % (BRANCH_ID_ALIAS_PREFIX, branch.id))
151+ path = escape(u'%s/foo/bar' % branch_id_alias(branch))
152 translation = self.codehosting_api.translatePath(requester.id, path)
153 self.assertEqual(
154 (BRANCH_TRANSPORT, {'id': branch.id, 'writable': False}, 'foo/bar'),
155@@ -1021,7 +1022,7 @@
156 branch = removeSecurityProxy(
157 self.factory.makeAnyBranch(
158 branch_type=BranchType.HOSTED, owner=requester))
159- path = escape(u'/%s/%s' % (BRANCH_ID_ALIAS_PREFIX, branch.id))
160+ path = escape(branch_id_alias(branch))
161 translation = self.codehosting_api.translatePath(requester.id, path)
162 self.assertEqual(
163 (BRANCH_TRANSPORT, {'id': branch.id, 'writable': False}, ''),
164@@ -1034,7 +1035,7 @@
165 branch = removeSecurityProxy(
166 self.factory.makeAnyBranch(
167 branch_type=BranchType.HOSTED, private=True, owner=requester))
168- path = escape(u'/%s/%s' % (BRANCH_ID_ALIAS_PREFIX, branch.id))
169+ path = escape(branch_id_alias(branch))
170 translation = self.codehosting_api.translatePath(requester.id, path)
171 self.assertEqual(
172 (BRANCH_TRANSPORT, {'id': branch.id, 'writable': False}, ''),
173@@ -1046,17 +1047,16 @@
174 branch = removeSecurityProxy(
175 self.factory.makeAnyBranch(
176 branch_type=BranchType.HOSTED, private=True))
177- path = escape(u'/%s/%s' % (BRANCH_ID_ALIAS_PREFIX, branch.id))
178+ path = escape(branch_id_alias(branch))
179 self.assertPermissionDenied(requester, path)
180
181 def assertTranslationIsControlDirectory(self, translation,
182 default_stacked_on,
183 trailing_path):
184 """Assert that 'translation' points to the right control transport."""
185- unique_name = escape(u'/' + default_stacked_on)
186 expected_translation = (
187 CONTROL_TRANSPORT,
188- {'default_stack_on': unique_name}, trailing_path)
189+ {'default_stack_on': escape(default_stacked_on)}, trailing_path)
190 self.assertEqual(expected_translation, translation)
191
192 def test_translatePath_control_directory(self):
193@@ -1067,7 +1067,7 @@
194 login(ANONYMOUS)
195 self.assertTranslationIsControlDirectory(
196 translation,
197- default_stacked_on=branch.unique_name,
198+ default_stacked_on=branch_id_alias(branch),
199 trailing_path='.bzr')
200
201 def test_translatePath_control_directory_no_stacked_set(self):
202@@ -1093,7 +1093,7 @@
203 login(ANONYMOUS)
204 self.assertTranslationIsControlDirectory(
205 translation,
206- default_stacked_on=branch.unique_name,
207+ default_stacked_on=branch_id_alias(branch),
208 trailing_path='.bzr')
209
210 def test_translatePath_control_directory_other_owner(self):
211@@ -1105,7 +1105,7 @@
212 login(ANONYMOUS)
213 self.assertTranslationIsControlDirectory(
214 translation,
215- default_stacked_on=branch.unique_name,
216+ default_stacked_on=branch_id_alias(branch),
217 trailing_path='.bzr')
218
219 def test_translatePath_control_directory_package_no_focus(self):
220@@ -1131,7 +1131,7 @@
221 login(ANONYMOUS)
222 self.assertTranslationIsControlDirectory(
223 translation,
224- default_stacked_on=branch.unique_name,
225+ default_stacked_on=branch_id_alias(branch),
226 trailing_path='.bzr')
227
228
229
230=== modified file 'lib/lp/codehosting/inmemory.py'
231--- lib/lp/codehosting/inmemory.py 2011-04-05 01:44:12 +0000
232+++ lib/lp/codehosting/inmemory.py 2011-04-18 23:07:52 +0000
233@@ -37,6 +37,7 @@
234 from lp.code.interfaces.branchtarget import IBranchTarget
235 from lp.code.interfaces.codehosting import (
236 BRANCH_ALIAS_PREFIX,
237+ branch_id_alias,
238 BRANCH_ID_ALIAS_PREFIX,
239 BRANCH_TRANSPORT,
240 CONTROL_TRANSPORT,
241@@ -802,9 +803,10 @@
242 return
243 if not self._canRead(requester, default_branch):
244 return
245+ path = branch_id_alias(default_branch)
246 return (
247 CONTROL_TRANSPORT,
248- {'default_stack_on': escape('/' + default_branch.unique_name)},
249+ {'default_stack_on': escape(path)},
250 trailing_path)
251
252 def _serializeBranch(self, requester_id, branch, trailing_path,
253
254=== modified file 'lib/lp/codehosting/tests/test_rewrite.py'
255--- lib/lp/codehosting/tests/test_rewrite.py 2011-04-04 04:14:02 +0000
256+++ lib/lp/codehosting/tests/test_rewrite.py 2011-04-18 23:07:52 +0000
257@@ -16,7 +16,7 @@
258
259 from canonical.config import config
260 from canonical.testing.layers import DatabaseFunctionalLayer
261-from lp.code.interfaces.codehosting import BRANCH_ID_ALIAS_PREFIX
262+from lp.code.interfaces.codehosting import branch_id_alias
263 from lp.codehosting.rewrite import BranchRewriter
264 from lp.codehosting.vfs import branch_id_to_path
265 from lp.services.log.logger import BufferLogger
266@@ -102,8 +102,8 @@
267 self.factory.makePackageBranch(private=False)]
268 transaction.commit()
269 output = [
270- rewriter.rewriteLine("/%s/%s/.bzr/README" % (
271- BRANCH_ID_ALIAS_PREFIX, branch.id))
272+ rewriter.rewriteLine(
273+ "%s/.bzr/README" % branch_id_alias(branch))
274 for branch in branches]
275 expected = [
276 'file:///var/tmp/bazaar.launchpad.dev/mirrors/%s/.bzr/README'
277@@ -116,8 +116,7 @@
278 # 'NULL'. This is translated by apache to a 404.
279 rewriter = self.makeRewriter()
280 branch = self.factory.makeAnyBranch(private=True)
281- path = '/%s/%s' % (
282- BRANCH_ID_ALIAS_PREFIX, removeSecurityProxy(branch).id)
283+ path = branch_id_alias(removeSecurityProxy(branch))
284 transaction.commit()
285 output = [
286 rewriter.rewriteLine("%s/changes" % path),
287@@ -130,7 +129,7 @@
288 rewriter = self.makeRewriter()
289 branch = self.factory.makeAnyBranch()
290 transaction.commit()
291- path = "/%s/%s/.bzr/README" % (BRANCH_ID_ALIAS_PREFIX, branch.id)
292+ path = "%s/.bzr/README" % branch_id_alias(branch)
293 rewriter.rewriteLine(path)
294 rewriter.rewriteLine(path)
295 logging_output_lines = self.getLoggerOutput(
296
297=== modified file 'lib/lp/codehosting/vfs/tests/test_branchfs.py'
298--- lib/lp/codehosting/vfs/tests/test_branchfs.py 2010-12-12 23:09:36 +0000
299+++ lib/lp/codehosting/vfs/tests/test_branchfs.py 2011-04-18 23:07:52 +0000
300@@ -55,6 +55,7 @@
301 )
302 from lp.code.enums import BranchType
303 from lp.code.interfaces.codehosting import (
304+ branch_id_alias,
305 BRANCH_TRANSPORT,
306 CONTROL_TRANSPORT,
307 )
308@@ -266,7 +267,7 @@
309
310 def check_control_file((transport, path)):
311 self.assertEqual(
312- 'default_stack_on = /%s\n' % branch.unique_name,
313+ 'default_stack_on = %s\n' % branch_id_alias(branch),
314 transport.get_bytes(path))
315 return deferred.addCallback(check_control_file)
316
317
318=== modified file 'lib/lp/codehosting/vfs/tests/test_filesystem.py'
319--- lib/lp/codehosting/vfs/tests/test_filesystem.py 2010-08-20 20:31:18 +0000
320+++ lib/lp/codehosting/vfs/tests/test_filesystem.py 2011-04-18 23:07:52 +0000
321@@ -16,6 +16,7 @@
322 from bzrlib.urlutils import escape
323
324 from lp.code.interfaces.branchtarget import IBranchTarget
325+from lp.code.interfaces.codehosting import branch_id_alias
326 from lp.codehosting.inmemory import (
327 InMemoryFrontend,
328 XMLRPCWrapper,
329@@ -139,9 +140,9 @@
330 control_file = transport.get_bytes(
331 '~%s/%s/.bzr/control.conf'
332 % (self.requester.name, product.name))
333+ stacked_on = IBranchTarget(product).default_stacked_on_branch
334 self.assertEqual(
335- 'default_stack_on = /%s'
336- % IBranchTarget(product).default_stacked_on_branch.unique_name,
337+ 'default_stack_on = %s' % branch_id_alias(stacked_on),
338 control_file.strip())
339
340 def test_can_open_product_control_dir(self):