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

Proposed by Colin Watson on 2015-03-30
Status: Merged
Merged at revision: 17417
Proposed branch: lp:~cjwatson/launchpad/git-translate-trailing-path-defaults
Merge into: lp:launchpad
Diff against target: 141 lines (+39/-14)
4 files modified
lib/lp/code/interfaces/gitlookup.py (+2/-1)
lib/lp/code/model/gitlookup.py (+21/-9)
lib/lp/code/model/tests/test_gitlookup.py (+11/-2)
lib/lp/registry/browser/person.py (+5/-2)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-translate-trailing-path-defaults
Reviewer Review Type Date Requested Status
Colin Watson Approve on 2015-03-30
Review via email: mp+254587@code.launchpad.net

Commit message

Make GitLookup.getByPath handle trailing path segments on objects which are also traversable.

Description of the change

Make GitLookup.getByPath handle trailing path segments on objects which are also traversable.

This fixes a case I missed in https://code.launchpad.net/~cjwatson/launchpad/git-translate-trailing-path/+merge/254551: for example, for cgit support, /PROJECT/commit/?id=sha1 needs to be translatable to the default repository for PROJECT with some trailing path information, even though the traversal code knows how to traverse from projects. This requires some adjustment of internal interfaces since we may have to push back a segment.

To post a comment you must log in.
Colin Watson (cjwatson) :
review: Approve

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-30 10:00:48 +0000
3+++ lib/lp/code/interfaces/gitlookup.py 2015-03-30 14:55:25 +0000
4@@ -53,7 +53,8 @@
5 :return: A tuple of::
6 * an `IPerson`, or None;
7 * an `IHasGitRepositories`;
8- * an `IGitRepository`, or None.
9+ * an `IGitRepository`, or None;
10+ * a trailing path segment, or None.
11 """
12
13 def traverse_path(path):
14
15=== modified file 'lib/lp/code/model/gitlookup.py'
16--- lib/lp/code/model/gitlookup.py 2015-03-30 09:46:03 +0000
17+++ lib/lp/code/model/gitlookup.py 2015-03-30 14:55:25 +0000
18@@ -261,26 +261,34 @@
19 else:
20 target = owner
21 traversable = adapt(owner, IGitTraversable)
22+ trailing = None
23 segments_iter = SegmentIterator(segments)
24 while traversable is not None:
25 try:
26 name = next(segments_iter)
27 except StopIteration:
28 break
29- owner, target, repository = traversable.traverse(
30- owner, name, segments_iter)
31+ try:
32+ owner, target, repository = traversable.traverse(
33+ owner, name, segments_iter)
34+ except InvalidNamespace:
35+ if target is not None or repository is not None:
36+ # We have some information, so the rest may consist of
37+ # trailing path information.
38+ trailing = name
39+ break
40 if repository is not None:
41 break
42 traversable = adapt(target, IGitTraversable)
43 if target is None or not IHasGitRepositories.providedBy(target):
44 raise InvalidNamespace("/".join(segments_iter.traversed))
45- return owner, target, repository
46+ return owner, target, repository, trailing
47
48 def traverse_path(self, path):
49 """See `IGitTraverser`."""
50 segments = iter(path.split("/"))
51- owner, target, repository = self.traverse(segments)
52- if list(segments):
53+ owner, target, repository, trailing = self.traverse(segments)
54+ if trailing or list(segments):
55 raise InvalidNamespace(path)
56 return owner, target, repository
57
58@@ -340,9 +348,10 @@
59 """See `IGitLookup`."""
60 try:
61 if unique_name.startswith("~"):
62+ traverser = getUtility(IGitTraverser)
63 segments = iter(unique_name.split("/"))
64- _, _, repository = getUtility(IGitTraverser).traverse(segments)
65- if repository is None or list(segments):
66+ _, _, repository, trailing = traverser.traverse(segments)
67+ if repository is None or trailing or list(segments):
68 raise InvalidNamespace(unique_name)
69 return repository
70 except (InvalidNamespace, NameLookupFailed):
71@@ -354,7 +363,7 @@
72 traverser = getUtility(IGitTraverser)
73 segments = iter(path.split("/"))
74 try:
75- owner, target, repository = traverser.traverse(segments)
76+ owner, target, repository, trailing = traverser.traverse(segments)
77 except (InvalidNamespace, InvalidProductName, NameLookupFailed):
78 return None, None
79 if repository is None:
80@@ -366,4 +375,7 @@
81 else:
82 repository = repository_set.getDefaultRepositoryForOwner(
83 owner, target)
84- return repository, "/".join(segments)
85+ trailing_segments = list(segments)
86+ if trailing:
87+ trailing_segments.insert(0, trailing)
88+ return repository, "/".join(trailing_segments)
89
90=== modified file 'lib/lp/code/model/tests/test_gitlookup.py'
91--- lib/lp/code/model/tests/test_gitlookup.py 2015-03-30 09:46:03 +0000
92+++ lib/lp/code/model/tests/test_gitlookup.py 2015-03-30 14:55:25 +0000
93@@ -137,6 +137,15 @@
94 (repository, "foo/bar"),
95 self.lookup.getByPath("%s/foo/bar" % repository.unique_name))
96
97+ def test_default_extra_path(self):
98+ repository = self.factory.makeGitRepository()
99+ with person_logged_in(repository.target.owner):
100+ getUtility(IGitRepositorySet).setDefaultRepository(
101+ repository.target, repository)
102+ self.assertEqual(
103+ (repository, "foo/bar"),
104+ self.lookup.getByPath("%s/foo/bar" % repository.shortened_path))
105+
106 def test_invalid_namespace(self):
107 # If `getByPath` is given a path to something with no default Git
108 # repository, such as a distribution, it returns (None, _).
109@@ -502,8 +511,8 @@
110 owner=person, target=person, name=u"repository")
111 segments = ["~person", "+git", "repository"]
112 self.assertEqual(
113- (person, person, repository),
114+ (person, person, repository, None),
115 self.traverser.traverse(iter(segments)))
116 self.assertEqual(
117- (person, person, repository),
118+ (person, person, repository, None),
119 self.traverser.traverse(iter(segments[1:]), owner=person))
120
121=== modified file 'lib/lp/registry/browser/person.py'
122--- lib/lp/registry/browser/person.py 2015-03-17 16:22:56 +0000
123+++ lib/lp/registry/browser/person.py 2015-03-30 14:55:25 +0000
124@@ -375,12 +375,15 @@
125 num_segments = len(segments)
126 iter_segments = iter(segments)
127 traverser = getUtility(IGitTraverser)
128- _, target, repository = traverser.traverse(
129+ _, target, repository, trailing = traverser.traverse(
130 iter_segments, owner=self.context)
131 if repository is None:
132 raise NotFoundError
133 # Subtract one because the pillar has already been traversed.
134- for _ in range(num_segments - len(list(iter_segments)) - 1):
135+ num_traversed = num_segments - len(list(iter_segments)) - 1
136+ if trailing:
137+ num_traversed -= 1
138+ for _ in range(num_traversed):
139 self.request.stepstogo.consume()
140
141 if IProduct.providedBy(target):