Merge ~twom/launchpad:git-branch-picker into launchpad:master
- Git
- lp:~twom/launchpad
- git-branch-picker
- Merge into master
Proposed by
Tom Wardill
Status: | Rejected |
---|---|
Rejected by: | Tom Wardill |
Proposed branch: | ~twom/launchpad:git-branch-picker |
Merge into: | launchpad:master |
Diff against target: |
937 lines (+585/-69) 18 files modified
lib/lp/app/javascript/autocomplete.js (+62/-0) lib/lp/app/javascript/picker/picker.js (+1/-1) lib/lp/app/javascript/picker/picker_patcher.js (+23/-11) lib/lp/app/widgets/templates/form-picker-macros.pt (+1/-3) lib/lp/code/adapters/gitrepository.py (+10/-1) lib/lp/code/browser/widgets/configure.zcml (+9/-0) lib/lp/code/browser/widgets/gitref.py (+43/-26) lib/lp/code/browser/widgets/templates/gitref-picker.pt (+37/-0) lib/lp/code/browser/widgets/templates/gitref.pt (+3/-18) lib/lp/code/browser/widgets/tests/test_gitrefwidget.py (+1/-1) lib/lp/code/configure.zcml (+4/-0) lib/lp/code/interfaces/gitrepository.py (+8/-0) lib/lp/code/model/gitref.py (+1/-0) lib/lp/code/vocabularies/configure.zcml (+22/-0) lib/lp/code/vocabularies/gitref.py (+151/-0) lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py (+189/-0) lib/lp/snappy/browser/tests/test_snap.py (+8/-6) lib/lp/snappy/model/snap.py (+12/-2) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Launchpad code reviewers | Pending | ||
Review via email: mp+392831@code.launchpad.net |
Commit message
Create a vocabulary for GitRef, and use it in a huge-vocabulary call to allow for JS based autocomplete on ref names in the edit snap page.
Description of the change
To post a comment you must log in.
Revision history for this message
Tom Wardill (twom) : | # |
- 5a47e8a... by Tom Wardill
-
Add missing template
Unmerged commits
- 5a47e8a... by Tom Wardill
-
Add missing template
- d7c9af0... by Tom Wardill
-
Use GitRef vocabulary for autocomplete git picker
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/app/javascript/autocomplete.js b/lib/lp/app/javascript/autocomplete.js |
2 | new file mode 100644 |
3 | index 0000000..25627ed |
4 | --- /dev/null |
5 | +++ b/lib/lp/app/javascript/autocomplete.js |
6 | @@ -0,0 +1,62 @@ |
7 | +/* Copyright 2017 Canonical Ltd. This software is licensed under the |
8 | + * GNU Affero General Public License version 3 (see the file LICENSE). |
9 | + */ |
10 | + |
11 | +YUI.add('lp.app.autocomplete', function(Y) { |
12 | + |
13 | +var namespace = Y.namespace('lp.app.autocomplete'); |
14 | + |
15 | +namespace.setupVocabAutocomplete = function(config, node) { |
16 | + var qs = 'name=' + encodeURIComponent(config.vocabulary_name); |
17 | + |
18 | + /* autocomplete will substitute these with appropriate encoding. */ |
19 | + /* XXX cjwatson 2017-07-24: Perhaps we should pass batch={maxResults} |
20 | + * too, but we need to make sure that it doesn't exceed max_batch_size. |
21 | + */ |
22 | + qs += '&search_text={query}'; |
23 | + |
24 | + var uri = ''; |
25 | + if (Y.Lang.isFunction(config.getContextPath)) { |
26 | + var context_path = config.getContextPath(); |
27 | + if (context_path !== null) { |
28 | + uri += config.getContextPath() + '/'; |
29 | + } else { |
30 | + // No context, so we cannot autocomplete. |
31 | + return; |
32 | + } |
33 | + } else if (Y.Lang.isValue(config.context)) { |
34 | + uri += Y.lp.get_url_path( |
35 | + config.context.get('web_link')) + '/'; |
36 | + } |
37 | + uri += '@@+huge-vocabulary'; |
38 | + |
39 | + |
40 | + node.plug(Y.Plugin.AutoComplete, { |
41 | + queryDelay: 500, // milliseconds |
42 | + requestTemplate: '?' + qs, |
43 | + resultHighlighter: 'wordMatch', |
44 | + resultListLocator: 'entries', |
45 | + resultTextLocator: 'value', |
46 | + source: uri |
47 | + }); |
48 | +}; |
49 | + |
50 | +/** |
51 | + * Add autocompletion to a text field. |
52 | + * @param {Object} config Object literal of config name/value pairs. |
53 | + * config.vocabulary_name: the named vocabulary to select from. |
54 | + * config.input_element: the id of the text field to update with the |
55 | + * selected value. |
56 | + */ |
57 | +namespace.addAutocomplete = function(config) { |
58 | + var input_element = Y.one('[id="' + config.input_element + '"]'); |
59 | + // The node may already have been processed. |
60 | + if (input_element.ac) { |
61 | + return; |
62 | + } |
63 | + namespace.setupVocabAutocomplete(config, input_element); |
64 | +}; |
65 | + |
66 | +}, '0.1', {'requires': [ |
67 | + 'autocomplete', 'autocomplete-sources', 'datasource', 'lp' |
68 | +]}); |
69 | diff --git a/lib/lp/app/javascript/picker/picker.js b/lib/lp/app/javascript/picker/picker.js |
70 | index e97b517..ef1e812 100644 |
71 | --- a/lib/lp/app/javascript/picker/picker.js |
72 | +++ b/lib/lp/app/javascript/picker/picker.js |
73 | @@ -24,7 +24,7 @@ ns.Picker = Y.Base.create('picker', Y.lp.ui.PrettyOverlay, [], { |
74 | /** |
75 | * The search input node. |
76 | * |
77 | - * @property _search_button |
78 | + * @property _search_input |
79 | * @type Node |
80 | * @private |
81 | */ |
82 | diff --git a/lib/lp/app/javascript/picker/picker_patcher.js b/lib/lp/app/javascript/picker/picker_patcher.js |
83 | index e220aa8..895b5b7 100644 |
84 | --- a/lib/lp/app/javascript/picker/picker_patcher.js |
85 | +++ b/lib/lp/app/javascript/picker/picker_patcher.js |
86 | @@ -50,7 +50,7 @@ var _addPicker = function(config, show_widget_id) { |
87 | * @param {Object} config Object literal of config name/value pairs. The |
88 | * values listed below are common for all picker types. |
89 | * config.vocabulary_name: the named vocabulary to select from. |
90 | - * config.vocabulary_filters: any vocaulary filters to use. |
91 | + * config.vocabulary_filters: any vocabulary filters to use. |
92 | * config.input_element: the id of the text field to update with the |
93 | * selected value. |
94 | * config.picker_type: the type of picker to create (default or person). |
95 | @@ -619,23 +619,35 @@ namespace.setup_vocab_picker = function (picker, vocabulary, config) { |
96 | // use the context to limit the results to the same project. |
97 | |
98 | var uri = ''; |
99 | + var context_ok = true; |
100 | if (Y.Lang.isFunction(config.getContextPath)) { |
101 | - uri += config.getContextPath() + '/'; |
102 | + var context_path = config.getContextPath(); |
103 | + if (context_path !== null) { |
104 | + uri += config.getContextPath() + '/'; |
105 | + } else { |
106 | + // No context, so proceed straight to validation. |
107 | + picker.set('error', e); |
108 | + picker.set('search_mode', false); |
109 | + picker.fire('validate', search_text); |
110 | + context_ok = false; |
111 | + } |
112 | } else if (Y.Lang.isValue(config.context)) { |
113 | uri += Y.lp.get_url_path( |
114 | config.context.get('web_link')) + '/'; |
115 | } |
116 | uri += '@@+huge-vocabulary?' + qs; |
117 | |
118 | - var yio = (config.yio !== undefined) ? config.yio : Y; |
119 | - yio.io(uri, { |
120 | - headers: {'Accept': 'application/json'}, |
121 | - timeout: 20000, |
122 | - on: { |
123 | - success: success_handler, |
124 | - failure: failure_handler |
125 | - } |
126 | - }); |
127 | + if (context_ok) { |
128 | + var yio = (config.yio !== undefined) ? config.yio : Y; |
129 | + yio.io(uri, { |
130 | + headers: {'Accept': 'application/json'}, |
131 | + timeout: 20000, |
132 | + on: { |
133 | + success: success_handler, |
134 | + failure: failure_handler |
135 | + } |
136 | + }); |
137 | + } |
138 | // Or we can pass in a vocabulary directly. |
139 | } else { |
140 | display_vocabulary(vocabulary, Y.Object.size(vocabulary), 1); |
141 | diff --git a/lib/lp/app/widgets/templates/form-picker-macros.pt b/lib/lp/app/widgets/templates/form-picker-macros.pt |
142 | index 80f5be1..9710e52 100644 |
143 | --- a/lib/lp/app/widgets/templates/form-picker-macros.pt |
144 | +++ b/lib/lp/app/widgets/templates/form-picker-macros.pt |
145 | @@ -33,9 +33,7 @@ |
146 | LPJS.use('node', 'lp.app.picker', function(Y) { |
147 | var config = ${view/json_config}; |
148 | var show_widget_id = '${view/show_widget_id}'; |
149 | - Y.on('domready', function(e) { |
150 | - Y.lp.app.picker.addPicker(config, show_widget_id); |
151 | - }); |
152 | + Y.lp.app.picker.addPicker(config, show_widget_id); |
153 | }); |
154 | "/> |
155 | </metal:form-picker> |
156 | diff --git a/lib/lp/code/adapters/gitrepository.py b/lib/lp/code/adapters/gitrepository.py |
157 | index e55d5bb..653b03e 100644 |
158 | --- a/lib/lp/code/adapters/gitrepository.py |
159 | +++ b/lib/lp/code/adapters/gitrepository.py |
160 | @@ -1,14 +1,16 @@ |
161 | # Copyright 2015-2018 Canonical Ltd. This software is licensed under the |
162 | # GNU Affero General Public License version 3 (see the file LICENSE). |
163 | |
164 | -"""Components related to Git repositories.""" |
165 | +"""Components and adapters related to Git repositories.""" |
166 | |
167 | __metaclass__ = type |
168 | __all__ = [ |
169 | "GitRepositoryDelta", |
170 | + "git_repository_for_snap", |
171 | ] |
172 | |
173 | from lazr.lifecycle.objectdelta import ObjectDelta |
174 | +from zope.component.interfaces import ComponentLookupError |
175 | from zope.interface import implementer |
176 | |
177 | from lp.code.interfaces.gitrepository import ( |
178 | @@ -56,3 +58,10 @@ class GitRepositoryDelta: |
179 | return GitRepositoryDelta(**changes) |
180 | else: |
181 | return None |
182 | + |
183 | + |
184 | +def git_repository_for_snap(snap): |
185 | + """Adapt a snap package to a Git repository.""" |
186 | + if snap.git_repository is None: |
187 | + raise ComponentLookupError |
188 | + return snap.git_repository |
189 | diff --git a/lib/lp/code/browser/widgets/configure.zcml b/lib/lp/code/browser/widgets/configure.zcml |
190 | index 913d8c6..e056e42 100644 |
191 | --- a/lib/lp/code/browser/widgets/configure.zcml |
192 | +++ b/lib/lp/code/browser/widgets/configure.zcml |
193 | @@ -16,4 +16,13 @@ |
194 | permission="zope.Public" |
195 | /> |
196 | |
197 | + <view |
198 | + type="zope.publisher.interfaces.browser.IBrowserRequest" |
199 | + for="zope.schema.interfaces.IChoice |
200 | + lp.code.vocabularies.gitref.GitRefVocabulary" |
201 | + provides="zope.formlib.interfaces.IInputWidget" |
202 | + factory="lp.code.browser.widgets.gitref.GitRefPickerWidget" |
203 | + permission="zope.Public" |
204 | + /> |
205 | + |
206 | </configure> |
207 | diff --git a/lib/lp/code/browser/widgets/gitref.py b/lib/lp/code/browser/widgets/gitref.py |
208 | index cfe8e40..2f4ce93 100644 |
209 | --- a/lib/lp/code/browser/widgets/gitref.py |
210 | +++ b/lib/lp/code/browser/widgets/gitref.py |
211 | @@ -9,7 +9,6 @@ __all__ = [ |
212 | |
213 | import six |
214 | from zope.browserpage import ViewPageTemplateFile |
215 | -from zope.component import getUtility |
216 | from zope.formlib.interfaces import ( |
217 | ConversionError, |
218 | IInputWidget, |
219 | @@ -23,16 +22,11 @@ from zope.formlib.widget import ( |
220 | InputWidget, |
221 | ) |
222 | from zope.interface import implementer |
223 | -from zope.schema import ( |
224 | - Choice, |
225 | - TextLine, |
226 | - ) |
227 | +from zope.schema import Choice |
228 | from zope.schema.interfaces import IChoice |
229 | - |
230 | from lp.app.errors import UnexpectedFormData |
231 | from lp.app.validators import LaunchpadValidationError |
232 | from lp.app.widgets.popup import VocabularyPickerWidget |
233 | -from lp.code.interfaces.gitref import IGitRefRemoteSet |
234 | from lp.code.interfaces.gitrepository import IGitRepository |
235 | from lp.services.fields import URIField |
236 | from lp.services.webapp.interfaces import ( |
237 | @@ -104,21 +98,26 @@ class GitRepositoryPickerWidget(VocabularyPickerWidget): |
238 | class GitRefWidget(BrowserWidget, InputWidget): |
239 | |
240 | template = ViewPageTemplateFile("templates/gitref.pt") |
241 | - display_label = False |
242 | _widgets_set_up = False |
243 | |
244 | # If True, allow entering external repository URLs. |
245 | allow_external = False |
246 | |
247 | + # If True, only allow reference paths to be branches (refs/heads/*). |
248 | + require_branch = False |
249 | + |
250 | def setUpSubWidgets(self): |
251 | if self._widgets_set_up: |
252 | return |
253 | + path_vocabulary = "GitBranch" if self.require_branch else "GitRef" |
254 | fields = [ |
255 | GitRepositoryField( |
256 | __name__="repository", title=u"Git repository", |
257 | required=False, vocabulary="GitRepository", |
258 | allow_external=self.allow_external), |
259 | - TextLine(__name__="path", title=u"Git branch", required=False), |
260 | + Choice( |
261 | + __name__="path", title=u"Git branch", required=False, |
262 | + vocabulary=path_vocabulary), |
263 | ] |
264 | for field in fields: |
265 | setUpWidget( |
266 | @@ -133,7 +132,12 @@ class GitRefWidget(BrowserWidget, InputWidget): |
267 | self.repository_widget.setRenderedValue(value.repository_url) |
268 | else: |
269 | self.repository_widget.setRenderedValue(value.repository) |
270 | - self.path_widget.setRenderedValue(value.path) |
271 | + # if we're only talking about branches, we can deal in the |
272 | + # name, rather than the full ref/heads/* path |
273 | + if self.require_branch: |
274 | + self.path_widget.setRenderedValue(value.name) |
275 | + else: |
276 | + self.path_widget.setRenderedValue(value.path) |
277 | else: |
278 | self.repository_widget.setRenderedValue(None) |
279 | self.path_widget.setRenderedValue(None) |
280 | @@ -174,27 +178,31 @@ class GitRefWidget(BrowserWidget, InputWidget): |
281 | "There is no Git repository named '%s' registered in " |
282 | "Launchpad." % entered_name)) |
283 | if self.path_widget.hasInput(): |
284 | - path = self.path_widget.getInputValue() |
285 | - else: |
286 | - path = None |
287 | - if not path: |
288 | - if self.context.required: |
289 | - raise WidgetInputError( |
290 | - self.name, self.label, |
291 | - LaunchpadValidationError( |
292 | - "Please enter a Git branch path.")) |
293 | + # We've potentially just tried to change the repository that is |
294 | + # involved, or changing from a bzr branch to a git repo, so there |
295 | + # is no existing repository set up. We need to set this so we |
296 | + # can compare the ref against the 'new' repo. |
297 | + from zope.security.proxy import removeSecurityProxy |
298 | + unsecure_vocab = removeSecurityProxy(self.path_widget.vocabulary) |
299 | + if IGitRepository.providedBy(repository): |
300 | + unsecure_vocab.setRepository(repository) |
301 | else: |
302 | - return |
303 | - if self.allow_external and not IGitRepository.providedBy(repository): |
304 | - ref = getUtility(IGitRefRemoteSet).new(repository, path) |
305 | - else: |
306 | - ref = repository.getRefByPath(path) |
307 | - if ref is None: |
308 | + unsecure_vocab.setRepositoryURL(repository) |
309 | + try: |
310 | + ref = self.path_widget.getInputValue() |
311 | + except ConversionError: |
312 | raise WidgetInputError( |
313 | self.name, self.label, |
314 | LaunchpadValidationError( |
315 | "The repository at %s does not contain a branch named " |
316 | - "'%s'." % (repository.display_name, path))) |
317 | + "'%s'." % ( |
318 | + repository.display_name, |
319 | + self.path_widget._getFormInput()))) |
320 | + if not ref and self.context.required: |
321 | + raise WidgetInputError( |
322 | + self.name, self.label, |
323 | + LaunchpadValidationError( |
324 | + "Please enter a Git branch path.")) |
325 | return ref |
326 | |
327 | def error(self): |
328 | @@ -210,3 +218,12 @@ class GitRefWidget(BrowserWidget, InputWidget): |
329 | """See `IBrowserWidget`.""" |
330 | self.setUpSubWidgets() |
331 | return self.template() |
332 | + |
333 | + |
334 | +class GitRefPickerWidget(VocabularyPickerWidget): |
335 | + |
336 | + __call__ = ViewPageTemplateFile("templates/gitref-picker.pt") |
337 | + |
338 | + @property |
339 | + def repository_id(self): |
340 | + return self._prefix + "repository" |
341 | diff --git a/lib/lp/code/browser/widgets/templates/gitref-picker.pt b/lib/lp/code/browser/widgets/templates/gitref-picker.pt |
342 | new file mode 100644 |
343 | index 0000000..d378846 |
344 | --- /dev/null |
345 | +++ b/lib/lp/code/browser/widgets/templates/gitref-picker.pt |
346 | @@ -0,0 +1,37 @@ |
347 | +<tal:root |
348 | + xmlns:tal="http://xml.zope.org/namespaces/tal" |
349 | + xmlns:metal="http://xml.zope.org/namespaces/metal" |
350 | + omit-tag=""> |
351 | + |
352 | +<metal:form-picker use-macro="context/@@form-picker-macros/form-picker"> |
353 | + <script metal:fill-slot="add-picker" tal:content="structure string: |
354 | + LPJS.use('node', 'lp.app.autocomplete', 'lp.app.picker', function(Y) { |
355 | + var config = ${view/json_config}; |
356 | + var repository_id = '${view/repository_id}'; |
357 | + if (repository_id !== '') { |
358 | + config.getContextPath = function() { |
359 | + var repository_value = Y.DOM.byId(repository_id).value; |
360 | + // XXX cjwatson 2017-06-24: We don't have a straightforward |
361 | + // URL parser available to us at the moment. This will do for |
362 | + // now, since we just want to tell the difference between |
363 | + // internal and external repositories. |
364 | + if (repository_value.indexOf('://') !== -1) { |
365 | + // XXX cjwatson 2017-07-24: Is throwing an exception too |
366 | + // much here? This currently bubbles up to the browser |
367 | + // console on e.g. Snap:+edit. |
368 | + throw ( |
369 | + 'Searching for branches in external repositories is ' + |
370 | + 'not available. Enter the branch name directly ' + |
371 | + 'instead.'); |
372 | + } |
373 | + return '/' + repository_value; |
374 | + }; |
375 | + } |
376 | + var show_widget_id = '${view/show_widget_id}'; |
377 | + Y.lp.app.picker.addPicker(config, show_widget_id); |
378 | + Y.lp.app.autocomplete.addAutocomplete(config); |
379 | + }); |
380 | + "/> |
381 | +</metal:form-picker> |
382 | + |
383 | +</tal:root> |
384 | diff --git a/lib/lp/code/browser/widgets/templates/gitref.pt b/lib/lp/code/browser/widgets/templates/gitref.pt |
385 | index f88a652..f972b89 100644 |
386 | --- a/lib/lp/code/browser/widgets/templates/gitref.pt |
387 | +++ b/lib/lp/code/browser/widgets/templates/gitref.pt |
388 | @@ -1,18 +1,3 @@ |
389 | -<table> |
390 | - <tr> |
391 | - <td> |
392 | - <tal:widget define="widget nocall:view/repository_widget"> |
393 | - <metal:block |
394 | - use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" /> |
395 | - </tal:widget> |
396 | - </td> |
397 | - </tr> |
398 | - <tr> |
399 | - <td> |
400 | - <tal:widget define="widget nocall:view/path_widget"> |
401 | - <metal:block |
402 | - use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" /> |
403 | - </tal:widget> |
404 | - </td> |
405 | - </tr> |
406 | -</table> |
407 | +<span title="Repository" tal:content="structure view/repository_widget" /> |
408 | + |
409 | +<span title="Branch" tal:content="structure view/path_widget" /> |
410 | diff --git a/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py b/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py |
411 | index b411651..2e2526c 100644 |
412 | --- a/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py |
413 | +++ b/lib/lp/code/browser/widgets/tests/test_gitrefwidget.py |
414 | @@ -194,7 +194,7 @@ class TestGitRefWidget(WithScenarios, TestCaseWithFactory): |
415 | [ref] = self.factory.makeGitRefs() |
416 | form = { |
417 | "field.git_ref.repository": ref.repository.unique_name, |
418 | - "field.git_ref.path": "non-existent", |
419 | + "field.git_ref.path": u"non-existent", |
420 | } |
421 | self.assertGetInputValueError( |
422 | form, |
423 | diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml |
424 | index 898e645..ace3140 100644 |
425 | --- a/lib/lp/code/configure.zcml |
426 | +++ b/lib/lp/code/configure.zcml |
427 | @@ -841,6 +841,10 @@ |
428 | for="lp.code.interfaces.gitrepository.IGitRepository" |
429 | provides="lp.services.webapp.interfaces.ILaunchpadContainer" |
430 | factory="lp.code.publisher.LaunchpadGitRepositoryContainer"/> |
431 | + <adapter |
432 | + for="lp.snappy.interfaces.snap.ISnap" |
433 | + provides="lp.code.interfaces.gitrepository.IGitRepository" |
434 | + factory="lp.code.adapters.gitrepository.git_repository_for_snap"/> |
435 | <subscriber |
436 | for="lp.code.interfaces.gitrepository.IGitRepository zope.lifecycleevent.interfaces.IObjectModifiedEvent" |
437 | handler="lp.code.model.gitrepository.git_repository_modified"/> |
438 | diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py |
439 | index 192133a..ed17183 100644 |
440 | --- a/lib/lp/code/interfaces/gitrepository.py |
441 | +++ b/lib/lp/code/interfaces/gitrepository.py |
442 | @@ -13,6 +13,7 @@ __all__ = [ |
443 | 'IGitRepository', |
444 | 'IGitRepositoryDelta', |
445 | 'IGitRepositorySet', |
446 | + 'IHasGitRepositoryURL', |
447 | 'user_has_special_git_repository_access', |
448 | ] |
449 | |
450 | @@ -1227,6 +1228,13 @@ class GitIdentityMixin: |
451 | return identities |
452 | |
453 | |
454 | +class IHasGitRepositoryURL(Interface): |
455 | + """Marker interface for objects that have a Git repository URL.""" |
456 | + |
457 | + git_repository_url = Attribute( |
458 | + "The Git repository URL (possibly external)") |
459 | + |
460 | + |
461 | def user_has_special_git_repository_access(user, repository=None): |
462 | """Admins have special access. |
463 | |
464 | diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py |
465 | index f7dc142..406c8de 100644 |
466 | --- a/lib/lp/code/model/gitref.py |
467 | +++ b/lib/lp/code/model/gitref.py |
468 | @@ -933,6 +933,7 @@ class GitRefRemote(GitRefMixin): |
469 | |
470 | def __eq__(self, other): |
471 | return ( |
472 | + other is not None and |
473 | self.repository_url == other.repository_url and |
474 | self.path == other.path) |
475 | |
476 | diff --git a/lib/lp/code/vocabularies/configure.zcml b/lib/lp/code/vocabularies/configure.zcml |
477 | index 5782992..fe8f73b 100644 |
478 | --- a/lib/lp/code/vocabularies/configure.zcml |
479 | +++ b/lib/lp/code/vocabularies/configure.zcml |
480 | @@ -88,4 +88,26 @@ |
481 | <allow interface="zope.schema.interfaces.IVocabularyTokenized"/> |
482 | </class> |
483 | |
484 | + <securedutility |
485 | + name="GitRef" |
486 | + component=".gitref.GitRefVocabulary" |
487 | + provides="zope.schema.interfaces.IVocabularyFactory"> |
488 | + <allow interface="zope.schema.interfaces.IVocabularyFactory"/> |
489 | + </securedutility> |
490 | + |
491 | + <class class=".gitref.GitRefVocabulary"> |
492 | + <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/> |
493 | + </class> |
494 | + |
495 | + <securedutility |
496 | + name="GitBranch" |
497 | + component=".gitref.GitBranchVocabulary" |
498 | + provides="zope.schema.interfaces.IVocabularyFactory"> |
499 | + <allow interface="zope.schema.interfaces.IVocabularyFactory"/> |
500 | + </securedutility> |
501 | + |
502 | + <class class=".gitref.GitBranchVocabulary"> |
503 | + <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/> |
504 | + </class> |
505 | + |
506 | </configure> |
507 | diff --git a/lib/lp/code/vocabularies/gitref.py b/lib/lp/code/vocabularies/gitref.py |
508 | new file mode 100644 |
509 | index 0000000..1e4a0aa |
510 | --- /dev/null |
511 | +++ b/lib/lp/code/vocabularies/gitref.py |
512 | @@ -0,0 +1,151 @@ |
513 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
514 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
515 | + |
516 | +"""Vocabularies that contain Git references.""" |
517 | + |
518 | +from __future__ import absolute_import, print_function, unicode_literals |
519 | + |
520 | +__metaclass__ = type |
521 | +__all__ = [ |
522 | + "GitBranchVocabulary", |
523 | + "GitRefVocabulary", |
524 | + ] |
525 | + |
526 | +from lazr.restful.interfaces import IReference |
527 | +from storm.expr import ( |
528 | + Desc, |
529 | + Like, |
530 | + like_escape, |
531 | + ) |
532 | +from zope.component import getUtility |
533 | +from zope.interface import implementer |
534 | +from zope.schema.vocabulary import SimpleTerm |
535 | +from zope.security.proxy import isinstance as zope_isinstance |
536 | + |
537 | +from lp.code.interfaces.gitref import IGitRefRemoteSet |
538 | +from lp.code.interfaces.gitrepository import ( |
539 | + IGitRepository, |
540 | + IHasGitRepositoryURL, |
541 | + ) |
542 | +from lp.code.model.gitref import ( |
543 | + GitRef, |
544 | + GitRefRemote, |
545 | + ) |
546 | +from lp.services.database.interfaces import IStore |
547 | +from storm.databases.postgres import Case |
548 | +from lp.services.webapp.vocabulary import ( |
549 | + CountableIterator, |
550 | + IHugeVocabulary, |
551 | + StormVocabularyBase, |
552 | + ) |
553 | + |
554 | + |
555 | +@implementer(IHugeVocabulary) |
556 | +class GitRefVocabulary(StormVocabularyBase): |
557 | + """A vocabulary for references in a given Git repository.""" |
558 | + |
559 | + _table = GitRef |
560 | + # In the base case (i.e. not GitBranchVocabulary) this may also be a |
561 | + # more general reference such as refs/tags/foo, but experience suggests |
562 | + # that people find talking about references in the web UI to be |
563 | + # baffling, so we tell a white lie here. |
564 | + displayname = "Select a branch" |
565 | + step_title = "Search" |
566 | + |
567 | + def __init__(self, context): |
568 | + super(GitRefVocabulary, self).__init__(context=context) |
569 | + if IReference.providedBy(context): |
570 | + context = context.context |
571 | + try: |
572 | + self.repository = IGitRepository(context) |
573 | + except TypeError: |
574 | + self.repository = None |
575 | + try: |
576 | + self.repository_url = ( |
577 | + IHasGitRepositoryURL(context).git_repository_url) |
578 | + except TypeError: |
579 | + self.repository_url = None |
580 | + |
581 | + def setRepository(self, repository): |
582 | + """Set the repository after the vocabulary was instantiated.""" |
583 | + self.repository = repository |
584 | + self.repository_url = None |
585 | + |
586 | + def setRepositoryURL(self, repository_url): |
587 | + """Set the repository URL after the vocabulary was instantiated.""" |
588 | + self.repository = None |
589 | + self.repository_url = repository_url |
590 | + |
591 | + def _assertHasRepository(self): |
592 | + if self.repository is None and self.repository_url is None: |
593 | + raise AssertionError( |
594 | + "GitRefVocabulary cannot be used without setting a " |
595 | + "repository or a repository URL.") |
596 | + |
597 | + @property |
598 | + def _order_by(self): |
599 | + rank = Case( |
600 | + cases=[(self._table.path == self.repository.default_branch, 2)], |
601 | + default=1) |
602 | + return [Desc(rank), Desc(self._table.committer_date)] |
603 | + |
604 | + def toTerm(self, ref): |
605 | + """See `StormVocabularyBase`.""" |
606 | + return SimpleTerm(ref, ref.path, ref.name) |
607 | + |
608 | + def getTermByToken(self, token): |
609 | + """See `IVocabularyTokenized`.""" |
610 | + self._assertHasRepository() |
611 | + if self.repository is not None: |
612 | + ref = self.repository.getRefByPath(token) |
613 | + if ref is None: |
614 | + raise LookupError(token) |
615 | + else: |
616 | + ref = getUtility(IGitRefRemoteSet).new(self.repository_url, token) |
617 | + return self.toTerm(ref) |
618 | + |
619 | + def _makePattern(self, query=None): |
620 | + parts = ["%"] |
621 | + if query is not None: |
622 | + parts.extend([query.lower().translate(like_escape), "%"]) |
623 | + return "".join(parts) |
624 | + |
625 | + def searchForTerms(self, query=None, vocab_filter=None): |
626 | + """See `IHugeVocabulary.""" |
627 | + self._assertHasRepository() |
628 | + if self.repository is not None: |
629 | + pattern = self._makePattern(query=query) |
630 | + results = IStore(self._table).find( |
631 | + self._table, |
632 | + self._table.repository_id == self.repository.id, |
633 | + Like(self._table.path, pattern, "!")).order_by(self._order_by) |
634 | + else: |
635 | + results = self.emptySelectResults() |
636 | + return CountableIterator(results.count(), results, self.toTerm) |
637 | + |
638 | + def __len__(self): |
639 | + """See `IVocabulary`.""" |
640 | + return self.searchForTerms().count() |
641 | + |
642 | + def __contains__(self, obj): |
643 | + # We know nothing about GitRefRemote, so we just have to assume |
644 | + # that they exist in the remote repository |
645 | + if zope_isinstance(obj, GitRefRemote): |
646 | + return True |
647 | + if obj in self.repository.refs: |
648 | + return True |
649 | + return False |
650 | + |
651 | + |
652 | +class GitBranchVocabulary(GitRefVocabulary): |
653 | + """A vocabulary for branches in a given Git repository.""" |
654 | + |
655 | + def _makePattern(self, query=None): |
656 | + parts = [] |
657 | + # XXX allow HEAD? |
658 | + if query is None or not query.startswith("refs/heads/"): |
659 | + parts.append("refs/heads/".translate(like_escape)) |
660 | + parts.append("%") |
661 | + if query is not None: |
662 | + parts.extend([query.lower().translate(like_escape), "%"]) |
663 | + return "".join(parts) |
664 | diff --git a/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py b/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py |
665 | new file mode 100644 |
666 | index 0000000..dbbc7d0 |
667 | --- /dev/null |
668 | +++ b/lib/lp/code/vocabularies/tests/test_gitref_vocabularies.py |
669 | @@ -0,0 +1,189 @@ |
670 | +# Copyright 2017 Canonical Ltd. This software is licensed under the |
671 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
672 | + |
673 | +"""Test the Git reference vocabularies.""" |
674 | + |
675 | +from __future__ import absolute_import, print_function, unicode_literals |
676 | + |
677 | +__metaclass__ = type |
678 | + |
679 | +from datetime import ( |
680 | + datetime, |
681 | + timedelta, |
682 | + ) |
683 | + |
684 | +import pytz |
685 | +from testtools.matchers import MatchesStructure |
686 | +from zope.schema.vocabulary import SimpleTerm |
687 | +from zope.security.proxy import removeSecurityProxy |
688 | + |
689 | +from lp.code.vocabularies.gitref import ( |
690 | + GitBranchVocabulary, |
691 | + GitRefVocabulary, |
692 | + ) |
693 | +from lp.services.webapp.vocabulary import IHugeVocabulary |
694 | +from lp.testing import TestCaseWithFactory |
695 | +from lp.testing.layers import ZopelessDatabaseLayer |
696 | + |
697 | + |
698 | +class TestGitRefVocabularyMixin: |
699 | + |
700 | + layer = ZopelessDatabaseLayer |
701 | + |
702 | + def test_getTermByToken(self): |
703 | + [ref] = self.factory.makeGitRefs() |
704 | + vocab = self.vocabulary_class(ref.repository) |
705 | + term = SimpleTerm(ref, ref.path, ref.name) |
706 | + self.assertEqual(term.token, vocab.getTermByToken(ref.name).token) |
707 | + self.assertEqual(term.token, vocab.getTermByToken(ref.path).token) |
708 | + self.assertRaises(LookupError, vocab.getTermByToken, "nonexistent") |
709 | + |
710 | + |
711 | +class TestGitRefVocabulary(TestGitRefVocabularyMixin, TestCaseWithFactory): |
712 | + |
713 | + vocabulary_class = GitRefVocabulary |
714 | + |
715 | + def test_provides_IHugeVocabulary(self): |
716 | + vocab = self.vocabulary_class(self.factory.makeGitRepository()) |
717 | + self.assertProvides(vocab, IHugeVocabulary) |
718 | + |
719 | + def test_init_snap(self): |
720 | + # A vocabulary may be instantiated with anything that can be adapted |
721 | + # to an IGitRepository, such as a Snap configured to build from one. |
722 | + [ref] = self.factory.makeGitRefs() |
723 | + vocab = self.vocabulary_class(self.factory.makeSnap(git_ref=ref)) |
724 | + self.assertEqual(ref.repository, vocab.repository) |
725 | + |
726 | + def test_init_no_repository(self): |
727 | + # The repository is None if the context cannot be adapted to a |
728 | + # repository. |
729 | + vocab = self.vocabulary_class( |
730 | + self.factory.makeSnap(branch=self.factory.makeAnyBranch())) |
731 | + self.assertIsNone(vocab.repository) |
732 | + |
733 | + def test_setRepository(self): |
734 | + # Callers can set the repository after instantiation. |
735 | + vocab = self.vocabulary_class( |
736 | + self.factory.makeSnap(branch=self.factory.makeAnyBranch())) |
737 | + repository = self.factory.makeGitRepository() |
738 | + vocab.setRepository(repository) |
739 | + self.assertEqual(repository, vocab.repository) |
740 | + |
741 | + def test_toTerm(self): |
742 | + [ref] = self.factory.makeGitRefs() |
743 | + self.assertThat( |
744 | + self.vocabulary_class(ref.repository).toTerm(ref), |
745 | + MatchesStructure.byEquality( |
746 | + value=ref, token=ref.path, title=ref.name)) |
747 | + |
748 | + def test_searchForTerms(self): |
749 | + ref_master, ref_next, ref_next_squared, _ = ( |
750 | + self.factory.makeGitRefs(paths=[ |
751 | + "refs/heads/master", "refs/heads/next", |
752 | + "refs/heads/next-squared", "refs/tags/next-1.0"])) |
753 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
754 | + ref_master.path) |
755 | + vocab = self.vocabulary_class(ref_master.repository) |
756 | + self.assertContentEqual( |
757 | + [term.value.path for term in vocab.searchForTerms("master")], |
758 | + ["refs/heads/master"]) |
759 | + self.assertContentEqual( |
760 | + [term.value.path for term in vocab.searchForTerms("next")], |
761 | + ["refs/heads/next", "refs/heads/next-squared", |
762 | + "refs/tags/next-1.0"]) |
763 | + self.assertContentEqual( |
764 | + [term.value.path for term in vocab.searchForTerms( |
765 | + "refs/heads/next")], |
766 | + ["refs/heads/next", "refs/heads/next-squared"]) |
767 | + self.assertContentEqual( |
768 | + [term.value.path for term in vocab.searchForTerms("")], |
769 | + ["refs/heads/master", "refs/heads/next", |
770 | + "refs/heads/next-squared", "refs/tags/next-1.0"]) |
771 | + self.assertContentEqual( |
772 | + [term.token for term in vocab.searchForTerms("nonexistent")], []) |
773 | + |
774 | + def test_searchForTerms_ordering(self): |
775 | + # The default branch (if it matches) is shown first, followed by |
776 | + # other matches in decreasing order of last commit date. |
777 | + ref_master, ref_master_old, ref_master_older = ( |
778 | + self.factory.makeGitRefs(paths=[ |
779 | + "refs/heads/master", "refs/heads/master-old", |
780 | + "refs/heads/master-older"])) |
781 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
782 | + ref_master.path) |
783 | + now = datetime.now(pytz.UTC) |
784 | + removeSecurityProxy(ref_master_old).committer_date = ( |
785 | + now - timedelta(days=1)) |
786 | + removeSecurityProxy(ref_master_older).committer_date = ( |
787 | + now - timedelta(days=2)) |
788 | + vocab = self.vocabulary_class(ref_master.repository) |
789 | + self.assertEqual( |
790 | + [term.value.path for term in vocab.searchForTerms("master")], |
791 | + ["refs/heads/master", "refs/heads/master-old", |
792 | + "refs/heads/master-older"]) |
793 | + |
794 | + def test_len(self): |
795 | + ref_master, _, _, _ = self.factory.makeGitRefs(paths=[ |
796 | + "refs/heads/master", "refs/heads/next", |
797 | + "refs/heads/next-squared", "refs/tags/next-1.0"]) |
798 | + self.assertEqual(4, len(self.vocabulary_class(ref_master.repository))) |
799 | + |
800 | + |
801 | +class TestGitBranchVocabulary(TestGitRefVocabularyMixin, TestCaseWithFactory): |
802 | + |
803 | + vocabulary_class = GitBranchVocabulary |
804 | + |
805 | + def test_toTerm(self): |
806 | + [ref] = self.factory.makeGitRefs() |
807 | + self.assertThat( |
808 | + self.vocabulary_class(ref.repository).toTerm(ref), |
809 | + MatchesStructure.byEquality( |
810 | + value=ref, token=ref.path, title=ref.name)) |
811 | + |
812 | + def test_searchForTerms(self): |
813 | + ref_master, ref_next, ref_next_squared, _ = ( |
814 | + self.factory.makeGitRefs(paths=[ |
815 | + "refs/heads/master", "refs/heads/next", |
816 | + "refs/heads/next-squared", "refs/tags/next-1.0"])) |
817 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
818 | + ref_master.path) |
819 | + vocab = self.vocabulary_class(ref_master.repository) |
820 | + self.assertContentEqual( |
821 | + [term.title for term in vocab.searchForTerms("master")], |
822 | + ["master"]) |
823 | + self.assertContentEqual( |
824 | + [term.title for term in vocab.searchForTerms("next")], |
825 | + ["next", "next-squared"]) |
826 | + self.assertContentEqual( |
827 | + [term.title for term in vocab.searchForTerms("refs/heads/next")], |
828 | + ["next", "next-squared"]) |
829 | + self.assertContentEqual( |
830 | + [term.title for term in vocab.searchForTerms("")], |
831 | + ["master", "next", "next-squared"]) |
832 | + self.assertContentEqual( |
833 | + [term.token for term in vocab.searchForTerms("nonexistent")], []) |
834 | + |
835 | + def test_searchForTerms_ordering(self): |
836 | + # The default branch (if it matches) is shown first, followed by |
837 | + # other matches in decreasing order of last commit date. |
838 | + ref_master, ref_master_old, ref_master_older = ( |
839 | + self.factory.makeGitRefs(paths=[ |
840 | + "refs/heads/master", "refs/heads/master-old", |
841 | + "refs/heads/master-older"])) |
842 | + removeSecurityProxy(ref_master.repository)._default_branch = ( |
843 | + ref_master.path) |
844 | + now = datetime.now(pytz.UTC) |
845 | + removeSecurityProxy(ref_master_old).committer_date = ( |
846 | + now - timedelta(days=1)) |
847 | + removeSecurityProxy(ref_master_older).committer_date = ( |
848 | + now - timedelta(days=2)) |
849 | + vocab = self.vocabulary_class(ref_master.repository) |
850 | + self.assertEqual( |
851 | + [term.title for term in vocab.searchForTerms("master")], |
852 | + ["master", "master-old", "master-older"]) |
853 | + |
854 | + def test_len(self): |
855 | + ref_master, _, _, _ = self.factory.makeGitRefs(paths=[ |
856 | + "refs/heads/master", "refs/heads/next", |
857 | + "refs/heads/next-squared", "refs/tags/next-1.0"]) |
858 | + self.assertEqual(3, len(self.vocabulary_class(ref_master.repository))) |
859 | diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py |
860 | index 10e82da..bbda603 100644 |
861 | --- a/lib/lp/snappy/browser/tests/test_snap.py |
862 | +++ b/lib/lp/snappy/browser/tests/test_snap.py |
863 | @@ -733,9 +733,9 @@ class TestSnapEditView(BaseTestSnapView): |
864 | browser.getControl(name="field.store_distro_series").value = [ |
865 | "ubuntu/%s/%s" % (new_series.name, new_snappy_series.name)] |
866 | browser.getControl("Git", index=0).click() |
867 | - browser.getControl("Git repository").value = ( |
868 | + browser.getControl(name="field.git_ref.repository").value = ( |
869 | new_git_ref.repository.identity) |
870 | - browser.getControl("Git branch").value = new_git_ref.path |
871 | + browser.getControl(name="field.git_ref.path").value = new_git_ref.path |
872 | browser.getControl("Build source tarball").selected = True |
873 | browser.getControl( |
874 | "Automatically build when branch changes").selected = True |
875 | @@ -952,8 +952,9 @@ class TestSnapEditView(BaseTestSnapView): |
876 | private_ref_path = private_ref.path |
877 | browser = self.getViewBrowser(snap, user=self.person) |
878 | browser.getLink("Edit snap package").click() |
879 | - browser.getControl("Git repository").value = private_ref_identity |
880 | - browser.getControl("Git branch").value = private_ref_path |
881 | + browser.getControl(name="field.git_ref.repository").value = ( |
882 | + private_ref_identity) |
883 | + browser.getControl(name="field.git_ref.path").value = private_ref_path |
884 | browser.getControl("Update snap package").click() |
885 | self.assertEqual( |
886 | "A public snap cannot have a private repository.", |
887 | @@ -973,8 +974,9 @@ class TestSnapEditView(BaseTestSnapView): |
888 | git_ref=old_ref, store_series=snappy_series) |
889 | browser = self.getViewBrowser(snap, user=self.person) |
890 | browser.getLink("Edit snap package").click() |
891 | - browser.getControl("Git repository").value = new_repository_url |
892 | - browser.getControl("Git branch").value = new_path |
893 | + browser.getControl( |
894 | + name="field.git_ref.repository").value = new_repository_url |
895 | + browser.getControl(name="field.git_ref.path").value = new_path |
896 | browser.getControl("Update snap package").click() |
897 | login_person(self.person) |
898 | content = find_main_content(browser.contents) |
899 | diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py |
900 | index 1ef8f45..7e3d14c 100644 |
901 | --- a/lib/lp/snappy/model/snap.py |
902 | +++ b/lib/lp/snappy/model/snap.py |
903 | @@ -46,7 +46,10 @@ from zope.component import ( |
904 | getUtility, |
905 | ) |
906 | from zope.event import notify |
907 | -from zope.interface import implementer |
908 | +from zope.interface import ( |
909 | + directlyProvides, |
910 | + implementer, |
911 | + ) |
912 | from zope.security.interfaces import Unauthorized |
913 | from zope.security.proxy import removeSecurityProxy |
914 | |
915 | @@ -82,7 +85,10 @@ from lp.code.interfaces.gitref import ( |
916 | IGitRef, |
917 | IGitRefRemoteSet, |
918 | ) |
919 | -from lp.code.interfaces.gitrepository import IGitRepository |
920 | +from lp.code.interfaces.gitrepository import ( |
921 | + IGitRepository, |
922 | + IHasGitRepositoryURL, |
923 | + ) |
924 | from lp.code.model.branch import Branch |
925 | from lp.code.model.branchcollection import GenericBranchCollection |
926 | from lp.code.model.gitcollection import GenericGitCollection |
927 | @@ -440,6 +446,10 @@ class Snap(Storm, WebhookTargetMixin): |
928 | self.git_repository = None |
929 | self.git_repository_url = None |
930 | self.git_path = None |
931 | + if self.git_repository_url is not None: |
932 | + directlyProvides(self, IHasGitRepositoryURL) |
933 | + else: |
934 | + directlyProvides(self) |
935 | |
936 | @property |
937 | def source(self): |