Merge lp:~cjwatson/launchpad/git-default-branch into lp:launchpad
- git-default-branch
- Merge into devel
Proposed by
Colin Watson
Status: | Merged |
---|---|
Merged at revision: | 17568 |
Proposed branch: | lp:~cjwatson/launchpad/git-default-branch |
Merge into: | lp:launchpad |
Prerequisite: | lp:~cjwatson/launchpad/git-detect-merges |
Diff against target: |
436 lines (+196/-6) 11 files modified
lib/lp/code/browser/gitrepository.py (+14/-1) lib/lp/code/browser/tests/test_gitrepository.py (+48/-0) lib/lp/code/configure.zcml (+4/-2) lib/lp/code/errors.py (+20/-1) lib/lp/code/interfaces/githosting.py (+14/-0) lib/lp/code/interfaces/gitrepository.py (+15/-1) lib/lp/code/model/githosting.py (+19/-0) lib/lp/code/model/gitjob.py (+7/-0) lib/lp/code/model/gitrepository.py (+19/-0) lib/lp/code/model/tests/test_gitjob.py (+6/-1) lib/lp/code/model/tests/test_gitrepository.py (+30/-0) |
To merge this branch: | bzr merge lp:~cjwatson/launchpad/git-default-branch |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+261892@code.launchpad.net |
Commit message
Allow getting and setting the default branch of a Git repository.
Description of the change
Allow getting and setting the default branch of a Git repository.
To post a comment you must log in.
Revision history for this message
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/browser/gitrepository.py' |
2 | --- lib/lp/code/browser/gitrepository.py 2015-06-12 08:04:42 +0000 |
3 | +++ lib/lp/code/browser/gitrepository.py 2015-06-15 10:02:42 +0000 |
4 | @@ -21,7 +21,10 @@ |
5 | |
6 | from lazr.lifecycle.event import ObjectModifiedEvent |
7 | from lazr.lifecycle.snapshot import Snapshot |
8 | -from lazr.restful.interface import copy_field |
9 | +from lazr.restful.interface import ( |
10 | + copy_field, |
11 | + use_template, |
12 | + ) |
13 | from storm.expr import Desc |
14 | from zope.event import notify |
15 | from zope.interface import ( |
16 | @@ -266,6 +269,7 @@ |
17 | This is necessary to make various fields editable that are not |
18 | normally editable through the interface. |
19 | """ |
20 | + use_template(IGitRepository, include=["default_branch"]) |
21 | information_type = copy_field( |
22 | IGitRepository["information_type"], readonly=False, |
23 | vocabulary=InformationTypeVocabulary(types=info_types)) |
24 | @@ -378,6 +382,7 @@ |
25 | "owner", |
26 | "name", |
27 | "information_type", |
28 | + "default_branch", |
29 | ] |
30 | |
31 | custom_widget("information_type", LaunchpadRadioWidgetWithDescription) |
32 | @@ -416,6 +421,14 @@ |
33 | (owner.displayname, target.displayname)) |
34 | except GitRepositoryExists as e: |
35 | self._setRepositoryExists(e.existing_repository) |
36 | + if "default_branch" in data: |
37 | + default_branch = data["default_branch"] |
38 | + if (default_branch is not None and |
39 | + self.context.getRefByPath(default_branch) is None): |
40 | + self.setFieldError( |
41 | + "default_branch", |
42 | + "This repository does not contain a reference named " |
43 | + "'%s'." % default_branch) |
44 | |
45 | |
46 | class GitRepositoryDeletionView(LaunchpadFormView): |
47 | |
48 | === modified file 'lib/lp/code/browser/tests/test_gitrepository.py' |
49 | --- lib/lp/code/browser/tests/test_gitrepository.py 2015-06-05 13:10:18 +0000 |
50 | +++ lib/lp/code/browser/tests/test_gitrepository.py 2015-06-15 10:02:42 +0000 |
51 | @@ -15,6 +15,7 @@ |
52 | DocTestMatches, |
53 | Equals, |
54 | ) |
55 | +import transaction |
56 | from zope.component import getUtility |
57 | from zope.publisher.interfaces import NotFound |
58 | from zope.security.proxy import removeSecurityProxy |
59 | @@ -22,6 +23,7 @@ |
60 | from lp.app.enums import InformationType |
61 | from lp.app.interfaces.launchpad import ILaunchpadCelebrities |
62 | from lp.app.interfaces.services import IService |
63 | +from lp.code.interfaces.githosting import IGitHostingClient |
64 | from lp.code.interfaces.revision import IRevisionSet |
65 | from lp.registry.enums import BranchSharingPolicy |
66 | from lp.registry.interfaces.person import PersonVisibility |
67 | @@ -37,6 +39,8 @@ |
68 | record_two_runs, |
69 | TestCaseWithFactory, |
70 | ) |
71 | +from lp.testing.fakemethod import FakeMethod |
72 | +from lp.testing.fixture import ZopeUtilityFixture |
73 | from lp.testing.layers import DatabaseFunctionalLayer |
74 | from lp.testing.matchers import HasQueryCount |
75 | from lp.testing.pages import ( |
76 | @@ -343,6 +347,50 @@ |
77 | self.assertEqual( |
78 | repository.information_type, InformationType.PUBLICSECURITY) |
79 | |
80 | + def test_change_default_branch(self): |
81 | + # An authorised user can change the default branch to one that |
82 | + # exists. They may omit "refs/heads/". |
83 | + hosting_client = FakeMethod() |
84 | + hosting_client.setProperties = FakeMethod() |
85 | + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient)) |
86 | + person = self.factory.makePerson() |
87 | + repository = self.factory.makeGitRepository(owner=person) |
88 | + master, new = self.factory.makeGitRefs( |
89 | + repository=repository, |
90 | + paths=[u"refs/heads/master", u"refs/heads/new"]) |
91 | + removeSecurityProxy(repository)._default_branch = u"refs/heads/master" |
92 | + browser = self.getUserBrowser( |
93 | + canonical_url(repository) + "/+edit", user=person) |
94 | + browser.getControl(name="field.default_branch").value = u"new" |
95 | + browser.getControl("Change Git Repository").click() |
96 | + with person_logged_in(person): |
97 | + self.assertEqual( |
98 | + [((repository.getInternalPath(),), |
99 | + {u"default_branch": u"refs/heads/new"})], |
100 | + hosting_client.setProperties.calls) |
101 | + self.assertEqual(u"refs/heads/new", repository.default_branch) |
102 | + |
103 | + def test_change_default_branch_nonexistent(self): |
104 | + # Trying to change the default branch to one that doesn't exist |
105 | + # displays an error. |
106 | + person = self.factory.makePerson() |
107 | + repository = self.factory.makeGitRepository(owner=person) |
108 | + [master] = self.factory.makeGitRefs( |
109 | + repository=repository, paths=[u"refs/heads/master"]) |
110 | + removeSecurityProxy(repository)._default_branch = u"refs/heads/master" |
111 | + form = { |
112 | + "field.default_branch": "refs/heads/new", |
113 | + "field.actions.change": "Change Git Repository", |
114 | + } |
115 | + transaction.commit() |
116 | + with person_logged_in(person): |
117 | + view = create_initialized_view(repository, name="+edit", form=form) |
118 | + self.assertEqual( |
119 | + ["This repository does not contain a reference named " |
120 | + "'refs/heads/new'."], |
121 | + view.errors) |
122 | + self.assertEqual(u"refs/heads/master", repository.default_branch) |
123 | + |
124 | |
125 | class TestGitRepositoryEditViewInformationTypes(TestCaseWithFactory): |
126 | """Tests for GitRepositoryEditView.getInformationTypesToShow.""" |
127 | |
128 | === modified file 'lib/lp/code/configure.zcml' |
129 | --- lib/lp/code/configure.zcml 2015-06-12 17:29:59 +0000 |
130 | +++ lib/lp/code/configure.zcml 2015-06-15 10:02:42 +0000 |
131 | @@ -787,7 +787,8 @@ |
132 | permission="launchpad.View" |
133 | interface="lp.app.interfaces.launchpad.IPrivacy |
134 | lp.code.interfaces.gitrepository.IGitRepositoryView |
135 | - lp.code.interfaces.gitrepository.IGitRepositoryModerateAttributes" /> |
136 | + lp.code.interfaces.gitrepository.IGitRepositoryModerateAttributes |
137 | + lp.code.interfaces.gitrepository.IGitRepositoryEditableAttributes" /> |
138 | <require |
139 | permission="launchpad.Moderate" |
140 | interface="lp.code.interfaces.gitrepository.IGitRepositoryModerate" |
141 | @@ -795,7 +796,8 @@ |
142 | set_attributes="date_last_modified" /> |
143 | <require |
144 | permission="launchpad.Edit" |
145 | - interface="lp.code.interfaces.gitrepository.IGitRepositoryEdit" /> |
146 | + interface="lp.code.interfaces.gitrepository.IGitRepositoryEdit" |
147 | + set_schema="lp.code.interfaces.gitrepository.IGitRepositoryEditableAttributes" /> |
148 | </class> |
149 | <subscriber |
150 | for="lp.code.interfaces.gitrepository.IGitRepository zope.lifecycleevent.interfaces.IObjectModifiedEvent" |
151 | |
152 | === modified file 'lib/lp/code/errors.py' |
153 | --- lib/lp/code/errors.py 2015-05-19 11:29:24 +0000 |
154 | +++ lib/lp/code/errors.py 2015-06-15 10:02:42 +0000 |
155 | @@ -43,6 +43,7 @@ |
156 | 'InvalidNamespace', |
157 | 'NoLinkedBranch', |
158 | 'NoSuchBranch', |
159 | + 'NoSuchGitReference', |
160 | 'NoSuchGitRepository', |
161 | 'PrivateBranchRecipe', |
162 | 'ReviewNotPending', |
163 | @@ -61,7 +62,10 @@ |
164 | from bzrlib.plugins.builder.recipe import RecipeParseError |
165 | from lazr.restful.declarations import error_status |
166 | |
167 | -from lp.app.errors import NameLookupFailed |
168 | +from lp.app.errors import ( |
169 | + NameLookupFailed, |
170 | + NotFoundError, |
171 | + ) |
172 | |
173 | # Annotate the RecipeParseError's with a 400 webservice status. |
174 | error_status(httplib.BAD_REQUEST)(RecipeParseError) |
175 | @@ -402,6 +406,21 @@ |
176 | _message_prefix = "No such Git repository" |
177 | |
178 | |
179 | +class NoSuchGitReference(NotFoundError): |
180 | + """Raised when we try to look up a Git reference that does not exist.""" |
181 | + |
182 | + def __init__(self, repository, path): |
183 | + self.repository = repository |
184 | + self.path = path |
185 | + self.message = ( |
186 | + "The repository at %s does not contain a reference named '%s'." % |
187 | + (repository.display_name, path)) |
188 | + NotFoundError.__init__(self, self.message) |
189 | + |
190 | + def __str__(self): |
191 | + return self.message |
192 | + |
193 | + |
194 | @error_status(httplib.CONFLICT) |
195 | class GitDefaultConflict(Exception): |
196 | """Raised when trying to set a Git repository as the default for |
197 | |
198 | === modified file 'lib/lp/code/interfaces/githosting.py' |
199 | --- lib/lp/code/interfaces/githosting.py 2015-06-15 09:49:42 +0000 |
200 | +++ lib/lp/code/interfaces/githosting.py 2015-06-15 10:02:42 +0000 |
201 | @@ -23,6 +23,20 @@ |
202 | other physical path. |
203 | """ |
204 | |
205 | + def getProperties(path): |
206 | + """Get properties of this repository. |
207 | + |
208 | + :param path: Physical path of the repository on the hosting service. |
209 | + :return: A dict of properties. |
210 | + """ |
211 | + |
212 | + def setProperties(path, **props): |
213 | + """Set properties of this repository. |
214 | + |
215 | + :param path: Physical path of the repository on the hosting service. |
216 | + :param props: Properties to set. |
217 | + """ |
218 | + |
219 | def getRefs(path): |
220 | """Get all refs in this repository. |
221 | |
222 | |
223 | === modified file 'lib/lp/code/interfaces/gitrepository.py' |
224 | --- lib/lp/code/interfaces/gitrepository.py 2015-06-12 17:55:28 +0000 |
225 | +++ lib/lp/code/interfaces/gitrepository.py 2015-06-15 10:02:42 +0000 |
226 | @@ -552,6 +552,19 @@ |
227 | """ |
228 | |
229 | |
230 | +class IGitRepositoryEditableAttributes(Interface): |
231 | + """IGitRepository attributes that can be edited. |
232 | + |
233 | + These attributes need launchpad.View to see, and launchpad.Edit to change. |
234 | + """ |
235 | + |
236 | + default_branch = exported(TextLine( |
237 | + title=_("Default branch"), required=False, readonly=False, |
238 | + description=_( |
239 | + "The full path to the default branch for this repository, e.g. " |
240 | + "refs/heads/master."))) |
241 | + |
242 | + |
243 | class IGitRepositoryEdit(Interface): |
244 | """IGitRepository methods that require launchpad.Edit permission.""" |
245 | |
246 | @@ -619,7 +632,8 @@ |
247 | |
248 | |
249 | class IGitRepository(IGitRepositoryView, IGitRepositoryModerateAttributes, |
250 | - IGitRepositoryModerate, IGitRepositoryEdit): |
251 | + IGitRepositoryModerate, IGitRepositoryEditableAttributes, |
252 | + IGitRepositoryEdit): |
253 | """A Git repository.""" |
254 | |
255 | # Mark repositories as exported entries for the Launchpad API. |
256 | |
257 | === modified file 'lib/lp/code/model/githosting.py' |
258 | --- lib/lp/code/model/githosting.py 2015-06-15 09:49:42 +0000 |
259 | +++ lib/lp/code/model/githosting.py 2015-06-15 10:02:42 +0000 |
260 | @@ -68,6 +68,9 @@ |
261 | def _post(self, path, **kwargs): |
262 | return self._request("post", path, **kwargs) |
263 | |
264 | + def _patch(self, path, **kwargs): |
265 | + return self._request("patch", path, **kwargs) |
266 | + |
267 | def _delete(self, path, **kwargs): |
268 | return self._request("delete", path, **kwargs) |
269 | |
270 | @@ -83,6 +86,22 @@ |
271 | raise GitRepositoryCreationFault( |
272 | "Failed to create Git repository: %s" % unicode(e)) |
273 | |
274 | + def getProperties(self, path): |
275 | + """See `IGitHostingClient`.""" |
276 | + try: |
277 | + return self._get("/repo/%s" % path) |
278 | + except Exception as e: |
279 | + raise GitRepositoryScanFault( |
280 | + "Failed to get properties of Git repository: %s" % unicode(e)) |
281 | + |
282 | + def setProperties(self, path, **props): |
283 | + """See `IGitHostingClient`.""" |
284 | + try: |
285 | + self._patch("/repo/%s" % path, json_data=props) |
286 | + except Exception as e: |
287 | + raise GitRepositoryScanFault( |
288 | + "Failed to set properties of Git repository: %s" % unicode(e)) |
289 | + |
290 | def getRefs(self, path): |
291 | """See `IGitHostingClient`.""" |
292 | try: |
293 | |
294 | === modified file 'lib/lp/code/model/gitjob.py' |
295 | --- lib/lp/code/model/gitjob.py 2015-06-12 17:55:28 +0000 |
296 | +++ lib/lp/code/model/gitjob.py 2015-06-15 10:02:42 +0000 |
297 | @@ -28,6 +28,7 @@ |
298 | classProvides, |
299 | implements, |
300 | ) |
301 | +from zope.security.proxy import removeSecurityProxy |
302 | |
303 | from lp.app.errors import NotFoundError |
304 | from lp.code.interfaces.githosting import IGitHostingClient |
305 | @@ -212,6 +213,12 @@ |
306 | hosting_path, refs_to_upsert, logger=log) |
307 | self.repository.synchroniseRefs( |
308 | refs_to_upsert, refs_to_remove, logger=log) |
309 | + props = getUtility(IGitHostingClient).getProperties( |
310 | + hosting_path) |
311 | + # We don't want ref canonicalisation, nor do we want to send |
312 | + # this change back to the hosting service. |
313 | + removeSecurityProxy(self.repository)._default_branch = ( |
314 | + props["default_branch"]) |
315 | except LostObjectError: |
316 | log.info( |
317 | "Skipping repository %s because it has been deleted." % |
318 | |
319 | === modified file 'lib/lp/code/model/gitrepository.py' |
320 | --- lib/lp/code/model/gitrepository.py 2015-06-12 17:55:28 +0000 |
321 | +++ lib/lp/code/model/gitrepository.py 2015-06-15 10:02:42 +0000 |
322 | @@ -72,6 +72,7 @@ |
323 | CannotDeleteGitRepository, |
324 | GitDefaultConflict, |
325 | GitTargetError, |
326 | + NoSuchGitReference, |
327 | ) |
328 | from lp.code.event.git import GitRefsUpdatedEvent |
329 | from lp.code.interfaces.branchmergeproposal import ( |
330 | @@ -206,6 +207,8 @@ |
331 | owner_default = Bool(name='owner_default', allow_none=False) |
332 | target_default = Bool(name='target_default', allow_none=False) |
333 | |
334 | + _default_branch = Unicode(name='default_branch', allow_none=True) |
335 | + |
336 | def __init__(self, registrant, owner, target, name, information_type, |
337 | date_created, reviewer=None, description=None): |
338 | super(GitRepository, self).__init__() |
339 | @@ -407,6 +410,22 @@ |
340 | GitRef.repository_id == self.id, |
341 | GitRef.path.startswith(u"refs/heads/")).order_by(GitRef.path) |
342 | |
343 | + @property |
344 | + def default_branch(self): |
345 | + """See `IGitRepository`.""" |
346 | + return self._default_branch |
347 | + |
348 | + @default_branch.setter |
349 | + def default_branch(self, value): |
350 | + """See `IGitRepository`.""" |
351 | + ref = self.getRefByPath(value) |
352 | + if ref is None: |
353 | + raise NoSuchGitReference(self, value) |
354 | + if self._default_branch != ref.path: |
355 | + self._default_branch = ref.path |
356 | + getUtility(IGitHostingClient).setProperties( |
357 | + self.getInternalPath(), default_branch=ref.path) |
358 | + |
359 | def getRefByPath(self, path): |
360 | paths = [path] |
361 | if not path.startswith(u"refs/heads/"): |
362 | |
363 | === modified file 'lib/lp/code/model/tests/test_gitjob.py' |
364 | --- lib/lp/code/model/tests/test_gitjob.py 2015-06-15 09:27:33 +0000 |
365 | +++ lib/lp/code/model/tests/test_gitjob.py 2015-06-15 10:02:42 +0000 |
366 | @@ -52,9 +52,10 @@ |
367 | |
368 | implements(IGitHostingClient) |
369 | |
370 | - def __init__(self, refs, commits): |
371 | + def __init__(self, refs, commits, default_branch=u"refs/heads/master"): |
372 | self._refs = refs |
373 | self._commits = commits |
374 | + self._default_branch = default_branch |
375 | |
376 | def getRefs(self, paths): |
377 | return self._refs |
378 | @@ -62,6 +63,9 @@ |
379 | def getCommits(self, path, commit_oids, logger=None): |
380 | return self._commits |
381 | |
382 | + def getProperties(self, path): |
383 | + return {u"default_branch": self._default_branch} |
384 | + |
385 | |
386 | class TestGitJob(TestCaseWithFactory): |
387 | """Tests for `GitJob`.""" |
388 | @@ -160,6 +164,7 @@ |
389 | with dbuser("branchscanner"): |
390 | JobRunner([job]).runAll() |
391 | self.assertRefsMatch(repository.refs, repository, paths) |
392 | + self.assertEqual(u"refs/heads/master", repository.default_branch) |
393 | |
394 | def test_logs_bad_ref_info(self): |
395 | repository = self.factory.makeGitRepository() |
396 | |
397 | === modified file 'lib/lp/code/model/tests/test_gitrepository.py' |
398 | --- lib/lp/code/model/tests/test_gitrepository.py 2015-06-12 17:55:28 +0000 |
399 | +++ lib/lp/code/model/tests/test_gitrepository.py 2015-06-15 10:02:42 +0000 |
400 | @@ -1217,6 +1217,36 @@ |
401 | ) for path, sha1 in expected_sha1s] |
402 | self.assertThat(repository.refs, MatchesSetwise(*matchers)) |
403 | |
404 | + def test_set_default_branch(self): |
405 | + hosting_client = FakeMethod() |
406 | + hosting_client.setProperties = FakeMethod() |
407 | + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient)) |
408 | + repository = self.factory.makeGitRepository() |
409 | + self.factory.makeGitRefs( |
410 | + repository=repository, |
411 | + paths=(u"refs/heads/master", u"refs/heads/new")) |
412 | + removeSecurityProxy(repository)._default_branch = u"refs/heads/master" |
413 | + with person_logged_in(repository.owner): |
414 | + repository.default_branch = u"new" |
415 | + self.assertEqual( |
416 | + [((repository.getInternalPath(),), |
417 | + {u"default_branch": u"refs/heads/new"})], |
418 | + hosting_client.setProperties.calls) |
419 | + self.assertEqual(u"refs/heads/new", repository.default_branch) |
420 | + |
421 | + def test_set_default_branch_unchanged(self): |
422 | + hosting_client = FakeMethod() |
423 | + hosting_client.setProperties = FakeMethod() |
424 | + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient)) |
425 | + repository = self.factory.makeGitRepository() |
426 | + self.factory.makeGitRefs( |
427 | + repository=repository, paths=[u"refs/heads/master"]) |
428 | + removeSecurityProxy(repository)._default_branch = u"refs/heads/master" |
429 | + with person_logged_in(repository.owner): |
430 | + repository.default_branch = u"master" |
431 | + self.assertEqual([], hosting_client.setProperties.calls) |
432 | + self.assertEqual(u"refs/heads/master", repository.default_branch) |
433 | + |
434 | |
435 | class TestGitRepositoryGetAllowedInformationTypes(TestCaseWithFactory): |
436 | """Test `IGitRepository.getAllowedInformationTypes`.""" |