Merge lp:~cjwatson/launchpad/git-translate-trailing-path into lp:launchpad

Proposed by Colin Watson on 2015-03-30
Status: Merged
Approved by: Colin Watson on 2015-03-30
Approved revision: no longer in the source branch.
Merged at revision: 17416
Proposed branch: lp:~cjwatson/launchpad/git-translate-trailing-path
Merge into: lp:launchpad
Diff against target: 251 lines (+65/-35)
6 files modified
lib/lp/code/interfaces/gitlookup.py (+4/-1)
lib/lp/code/model/gitlookup.py (+17/-12)
lib/lp/code/model/gitrepository.py (+2/-2)
lib/lp/code/model/tests/test_gitlookup.py (+23/-11)
lib/lp/code/xmlrpc/git.py (+6/-2)
lib/lp/code/xmlrpc/tests/test_git.py (+13/-7)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-translate-trailing-path
Reviewer Review Type Date Requested Status
William Grant code 2015-03-30 Approve on 2015-03-30
Review via email: mp+254551@code.launchpad.net

Commit message

Make GitLookup.getByPath and GitAPI.translatePath return extra trailing segments from a path, to support external code browsers.

Description of the change

Make GitLookup.getByPath and GitAPI.translatePath return extra trailing segments from a path, to support external code browsers. We'll need this so that you can traverse to https://git.launchpad.net/path/to/repository/with/more/segments and have our cgit integration work out how to translate the leading part of the path to a physical repository.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/interfaces/gitlookup.py'
2--- lib/lp/code/interfaces/gitlookup.py 2015-03-12 15:21:27 +0000
3+++ lib/lp/code/interfaces/gitlookup.py 2015-03-30 10:01:21 +0000
4@@ -139,5 +139,8 @@
5 PROJECT
6 DISTRO/+source/SOURCE
7
8- :return: An `IGitRepository`, or None.
9+ :return: A tuple of (`IGitRepository`, extra_path), or (None, _).
10+ 'extra_path' may be used by applications that need to traverse a
11+ leading part of a path as a repository, such as external code
12+ browsers.
13 """
14
15=== modified file 'lib/lp/code/model/gitlookup.py'
16--- lib/lp/code/model/gitlookup.py 2015-03-12 15:21:27 +0000
17+++ lib/lp/code/model/gitlookup.py 2015-03-30 10:01:21 +0000
18@@ -331,7 +331,10 @@
19 path = self.uriToPath(uri)
20 if path is None:
21 return None
22- return self.getByPath(path)
23+ path, extra_path = self.getByPath(path)
24+ if extra_path:
25+ return None
26+ return path
27
28 def getByUniqueName(self, unique_name):
29 """See `IGitLookup`."""
30@@ -349,16 +352,18 @@
31 def getByPath(self, path):
32 """See `IGitLookup`."""
33 traverser = getUtility(IGitTraverser)
34+ segments = iter(path.split("/"))
35 try:
36- owner, target, repository = traverser.traverse_path(path)
37+ owner, target, repository = traverser.traverse(segments)
38 except (InvalidNamespace, InvalidProductName, NameLookupFailed):
39- return None
40- if repository is not None:
41- return repository
42- if IPerson.providedBy(target):
43- return None
44- repository_set = getUtility(IGitRepositorySet)
45- if owner is None:
46- return repository_set.getDefaultRepository(target)
47- else:
48- return repository_set.getDefaultRepositoryForOwner(owner, target)
49+ return None, None
50+ if repository is None:
51+ if IPerson.providedBy(target):
52+ return None, None
53+ repository_set = getUtility(IGitRepositorySet)
54+ if owner is None:
55+ repository = repository_set.getDefaultRepository(target)
56+ else:
57+ repository = repository_set.getDefaultRepositoryForOwner(
58+ owner, target)
59+ return repository, "/".join(segments)
60
61=== modified file 'lib/lp/code/model/gitrepository.py'
62--- lib/lp/code/model/gitrepository.py 2015-03-24 13:37:54 +0000
63+++ lib/lp/code/model/gitrepository.py 2015-03-30 10:01:21 +0000
64@@ -602,8 +602,8 @@
65
66 def getByPath(self, user, path):
67 """See `IGitRepositorySet`."""
68- repository = getUtility(IGitLookup).getByPath(path)
69- if repository is None:
70+ repository, extra_path = getUtility(IGitLookup).getByPath(path)
71+ if repository is None or extra_path:
72 return None
73 # removeSecurityProxy is safe here since we're explicitly performing
74 # a permission check.
75
76=== modified file 'lib/lp/code/model/tests/test_gitlookup.py'
77--- lib/lp/code/model/tests/test_gitlookup.py 2015-03-12 15:21:27 +0000
78+++ lib/lp/code/model/tests/test_gitlookup.py 2015-03-30 10:01:21 +0000
79@@ -100,7 +100,7 @@
80 def test_project(self):
81 repository = self.factory.makeGitRepository()
82 self.assertEqual(
83- repository, self.lookup.getByPath(repository.unique_name))
84+ (repository, ""), self.lookup.getByPath(repository.unique_name))
85
86 def test_project_default(self):
87 repository = self.factory.makeGitRepository()
88@@ -108,13 +108,13 @@
89 getUtility(IGitRepositorySet).setDefaultRepository(
90 repository.target, repository)
91 self.assertEqual(
92- repository, self.lookup.getByPath(repository.shortened_path))
93+ (repository, ""), self.lookup.getByPath(repository.shortened_path))
94
95 def test_package(self):
96 dsp = self.factory.makeDistributionSourcePackage()
97 repository = self.factory.makeGitRepository(target=dsp)
98 self.assertEqual(
99- repository, self.lookup.getByPath(repository.unique_name))
100+ (repository, ""), self.lookup.getByPath(repository.unique_name))
101
102 def test_package_default(self):
103 dsp = self.factory.makeDistributionSourcePackage()
104@@ -123,31 +123,37 @@
105 getUtility(IGitRepositorySet).setDefaultRepository(
106 repository.target, repository)
107 self.assertEqual(
108- repository, self.lookup.getByPath(repository.shortened_path))
109+ (repository, ""), self.lookup.getByPath(repository.shortened_path))
110
111 def test_personal(self):
112 owner = self.factory.makePerson()
113 repository = self.factory.makeGitRepository(owner=owner, target=owner)
114 self.assertEqual(
115- repository, self.lookup.getByPath(repository.unique_name))
116+ (repository, ""), self.lookup.getByPath(repository.unique_name))
117+
118+ def test_extra_path(self):
119+ repository = self.factory.makeGitRepository()
120+ self.assertEqual(
121+ (repository, "foo/bar"),
122+ self.lookup.getByPath("%s/foo/bar" % repository.unique_name))
123
124 def test_invalid_namespace(self):
125 # If `getByPath` is given a path to something with no default Git
126- # repository, such as a distribution, it returns None.
127+ # repository, such as a distribution, it returns (None, _).
128 distro = self.factory.makeDistribution()
129- self.assertIsNone(self.lookup.getByPath(distro.name))
130+ self.assertIsNone(self.lookup.getByPath(distro.name)[0])
131
132 def test_no_default_git_repository(self):
133 # If `getByPath` is given a path to something that could have a Git
134- # repository but doesn't, it returns None.
135+ # repository but doesn't, it returns (None, _).
136 project = self.factory.makeProduct()
137- self.assertIsNone(self.lookup.getByPath(project.name))
138+ self.assertIsNone(self.lookup.getByPath(project.name)[0])
139
140 def test_bare_person(self):
141 # If `getByPath` is given a path to a person but nothing further, it
142- # returns None even if the person exists.
143+ # returns (None, _) even if the person exists.
144 owner = self.factory.makePerson()
145- self.assertIsNone(self.lookup.getByPath("~%s" % owner.name))
146+ self.assertIsNone(self.lookup.getByPath("~%s" % owner.name)[0])
147
148
149 class TestGetByUrl(TestCaseWithFactory):
150@@ -179,6 +185,12 @@
151 self.assertUrlMatches(
152 "git://git.launchpad.dev/~aa/bb/+git/cc/", repository)
153
154+ def test_getByUrl_with_trailing_segments(self):
155+ # URLs with trailing segments beyond the repository are rejected.
156+ self.makeProjectRepository()
157+ self.assertIsNone(
158+ self.lookup.getByUrl("git://git.launchpad.dev/~aa/bb/+git/cc/foo"))
159+
160 def test_getByUrl_with_git(self):
161 # getByUrl recognises LP repositories for git URLs.
162 repository = self.makeProjectRepository()
163
164=== modified file 'lib/lp/code/xmlrpc/git.py'
165--- lib/lp/code/xmlrpc/git.py 2015-03-16 12:12:35 +0000
166+++ lib/lp/code/xmlrpc/git.py 2015-03-30 10:01:21 +0000
167@@ -69,7 +69,7 @@
168 config.codehosting.internal_git_api_endpoint)
169
170 def _performLookup(self, path):
171- repository = getUtility(IGitLookup).getByPath(path)
172+ repository, extra_path = getUtility(IGitLookup).getByPath(path)
173 if repository is None:
174 return None
175 try:
176@@ -77,7 +77,11 @@
177 except Unauthorized:
178 raise faults.PermissionDenied()
179 writable = check_permission("launchpad.Edit", repository)
180- return {"path": hosting_path, "writable": writable}
181+ return {
182+ "path": hosting_path,
183+ "writable": writable,
184+ "trailing": extra_path,
185+ }
186
187 def _getGitNamespaceExtras(self, path, requester):
188 """Get the namespace, repository name, and callback for the path.
189
190=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
191--- lib/lp/code/xmlrpc/tests/test_git.py 2015-03-16 12:12:35 +0000
192+++ lib/lp/code/xmlrpc/tests/test_git.py 2015-03-30 10:01:21 +0000
193@@ -84,13 +84,15 @@
194 requester, path.lstrip("/"))
195 self.assertIsNotNone(repository)
196 self.assertEqual(
197- {"path": repository.getInternalPath(), "writable": True},
198+ {"path": repository.getInternalPath(), "writable": True,
199+ "trailing": ""},
200 translation)
201 translation = self.git_api.translatePath(
202 path, "write", requester.id, True)
203 login(ANONYMOUS)
204 self.assertEqual(
205- {"path": repository.getInternalPath(), "writable": True},
206+ {"path": repository.getInternalPath(), "writable": True,
207+ "trailing": ""},
208 translation)
209 # But we cannot create another one without the feature flag.
210 message = "You do not have permission to create Git repositories."
211@@ -181,14 +183,16 @@
212 return fault.faultString[len(prefix):].rstrip(".")
213
214 def assertTranslates(self, requester, path, repository, writable,
215- permission="read", can_authenticate=False):
216+ permission="read", can_authenticate=False,
217+ trailing=""):
218 if requester not in (LAUNCHPAD_ANONYMOUS, LAUNCHPAD_SERVICES):
219 requester = requester.id
220 translation = self.git_api.translatePath(
221 path, permission, requester, can_authenticate)
222 login(ANONYMOUS)
223 self.assertEqual(
224- {"path": repository.getInternalPath(), "writable": writable},
225+ {"path": repository.getInternalPath(), "writable": writable,
226+ "trailing": trailing},
227 translation)
228
229 def assertCreates(self, requester, path, can_authenticate=False):
230@@ -204,7 +208,8 @@
231 self.assertIsNotNone(repository)
232 self.assertEqual(requester, repository.registrant)
233 self.assertEqual(
234- {"path": repository.getInternalPath(), "writable": True},
235+ {"path": repository.getInternalPath(), "writable": True,
236+ "trailing": ""},
237 translation)
238 self.assertEqual(
239 [("create", repository.getInternalPath())],
240@@ -355,8 +360,9 @@
241 def test_translatePath_repository_with_trailing_segments(self):
242 requester = self.factory.makePerson()
243 repository = self.factory.makeGitRepository()
244- path = u"/%s/junk" % repository.unique_name
245- self.assertPathTranslationError(requester, path)
246+ path = u"/%s/foo/bar" % repository.unique_name
247+ self.assertTranslates(
248+ requester, path, repository, False, trailing="foo/bar")
249
250 def test_translatePath_no_such_repository(self):
251 requester = self.factory.makePerson()