Merge ~ilasc/launchpad:revision-status-submission-api into launchpad:master
- Git
- lp:~ilasc/launchpad
- revision-status-submission-api
- Merge into master
Status: | Merged |
---|---|
Approved by: | Ioana Lasc |
Approved revision: | 52d3757e7b293bad0e038c6e85e96b0b08089e43 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~ilasc/launchpad:revision-status-submission-api |
Merge into: | launchpad:master |
Diff against target: |
953 lines (+625/-3) 11 files modified
lib/lp/_schema_circular_imports.py (+8/-1) lib/lp/code/browser/configure.zcml (+6/-0) lib/lp/code/browser/gitrepository.py (+13/-0) lib/lp/code/configure.zcml (+34/-0) lib/lp/code/enums.py (+50/-0) lib/lp/code/interfaces/gitrepository.py (+180/-0) lib/lp/code/interfaces/webservice.py (+2/-0) lib/lp/code/model/gitrepository.py (+147/-1) lib/lp/code/model/tests/test_gitrepository.py (+137/-0) lib/lp/security.py (+19/-0) lib/lp/testing/factory.py (+29/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+410373@code.launchpad.net |
Commit message
Add revision status submission API
Description of the change
Colin Watson (cjwatson) : | # |
Colin Watson (cjwatson) : | # |
Colin Watson (cjwatson) : | # |
Ioana Lasc (ilasc) wrote : | # |
Colin Watson (cjwatson) wrote : | # |
There is a *lot* of formatting noise in this MP; for example it isn't at all clear what substantive changes have been made to `lib/lp/
Some of the rest of my comments may collide with work you've already done locally; if so, sorry about that, and hopefully you can make sense of them. I think these comments should at least be enough to let you move forward, although they aren't a complete review yet.
Colin Watson (cjwatson) : | # |
Ioana Lasc (ilasc) wrote : | # |
MP now ready for review.
Colin Watson (cjwatson) wrote : | # |
Getting there, despite the large volume of comments here!
I think the main thing is to make sure that the API is the right sort of shape so that we don't paint ourselves into a corner. Consider making the API method that creates a new report check a feature rule for the time being, so that we can limit this to only authorized clients until we're sure that we're ready.
Ioana Lasc (ilasc) wrote : | # |
While a large portion of comments have been updated on this MP there is still below set of things currently being worked on:
For this MP:
1: make the API method that creates a new report check a feature rule for the time being, so that we can limit this to only authorized clients until we're sure that we're ready.
2: export a method that takes the log as bytes, and creates the artifact object internally
3: Unit test for setLog over the API
4: on RevisionStatusA
5: add at least a basic security adapter for the newky added IRevisionStatus
7: Unit Test for the result mutator
Next MP / Jira Issue:
1: putting `IRevisionStatus*` in a separate file (and the same for the implementation) rather than having it in the same module as `IGitRepository*`
Colin Watson (cjwatson) wrote : | # |
I understand this is still work in progress, so I haven't done a full review, but I had a quick look through this today and had a couple of comments that might be useful to you at this stage.
Ioana Lasc (ilasc) wrote : | # |
Indeed the comments were helpful Colin, thanks! This is now ready for review together with the DB patch (https:/
Colin Watson (cjwatson) wrote : | # |
Summary of the main points here, after which I think this will be good to go:
* fix ambiguity in `RevisionStatus
* export `IRevisionStatu
* fix type of `IRevisionStatu
* declare a proper feature rule exception
* fix confusing definition of `EditRevisionSt
Ioana Lasc (ilasc) wrote : | # |
Thanks Colin! Addressed all comments and when moving TestRevisionSta
Added TestRevisionSta
HTTP/1.1 404 Not Found
Content-Length: 112
Content-Type: text/plain;
Object: <GitRepository '~person-
Looks like I need to define traversal / navigation for reports.
Ioana Lasc (ilasc) wrote : | # |
Navigation now works and DB patch changes are included in this version of the MP.
I do have however one last issue in test_setLogOnRe
Colin Watson (cjwatson) wrote : | # |
Iiiiiinteresting.
The problem here is that access tokens are scoped to a particular target, and if the context isn't exactly equal to the target then authenticating with an access token fails. We could work around this by moving the `setLog` method back to `IGitRepository` (with a more precise way to identify the correct revision status report), but I don't think that's really the best approach. Instead, I've proposed https:/
Preview Diff
1 | diff --git a/lib/lp/_schema_circular_imports.py b/lib/lp/_schema_circular_imports.py |
2 | index af21a01..e239ac2 100644 |
3 | --- a/lib/lp/_schema_circular_imports.py |
4 | +++ b/lib/lp/_schema_circular_imports.py |
5 | @@ -64,7 +64,10 @@ from lp.code.interfaces.codereviewcomment import ICodeReviewComment |
6 | from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference |
7 | from lp.code.interfaces.diff import IPreviewDiff |
8 | from lp.code.interfaces.gitref import IGitRef |
9 | -from lp.code.interfaces.gitrepository import IGitRepository |
10 | +from lp.code.interfaces.gitrepository import ( |
11 | + IGitRepository, |
12 | + IRevisionStatusReport, |
13 | + ) |
14 | from lp.code.interfaces.gitrule import ( |
15 | IGitNascentRule, |
16 | IGitNascentRuleGrant, |
17 | @@ -528,6 +531,10 @@ patch_collection_return_type( |
18 | patch_list_parameter_type( |
19 | IGitRepository, 'setRules', 'rules', InlineObject(schema=IGitNascentRule)) |
20 | |
21 | +# IRevisionStatusReport |
22 | +patch_reference_property( |
23 | + IRevisionStatusReport, 'git_repository', IGitRepository) |
24 | + |
25 | # ILiveFSFile |
26 | patch_reference_property(ILiveFSFile, 'livefsbuild', ILiveFSBuild) |
27 | |
28 | diff --git a/lib/lp/code/browser/configure.zcml b/lib/lp/code/browser/configure.zcml |
29 | index f7bab39..31b3938 100644 |
30 | --- a/lib/lp/code/browser/configure.zcml |
31 | +++ b/lib/lp/code/browser/configure.zcml |
32 | @@ -963,6 +963,12 @@ |
33 | factory="lp.code.browser.gitrepository.GitRepositoryBreadcrumb" |
34 | permission="zope.Public"/> |
35 | |
36 | + <browser:url |
37 | + for="lp.code.interfaces.gitrepository.IRevisionStatusReport" |
38 | + path_expression="string:+status/${id}" |
39 | + attribute_to_parent="git_repository" |
40 | + rootsite="code"/> |
41 | + |
42 | <browser:defaultView |
43 | for="lp.code.interfaces.gitref.IGitRef" |
44 | name="+index"/> |
45 | diff --git a/lib/lp/code/browser/gitrepository.py b/lib/lp/code/browser/gitrepository.py |
46 | index 92a1b14..ddd79ca 100644 |
47 | --- a/lib/lp/code/browser/gitrepository.py |
48 | +++ b/lib/lp/code/browser/gitrepository.py |
49 | @@ -106,6 +106,7 @@ from lp.code.interfaces.gitrepository import ( |
50 | ContributorGitIdentity, |
51 | IGitRepository, |
52 | IGitRepositorySet, |
53 | + IRevisionStatusReportSet, |
54 | ) |
55 | from lp.code.vocabularies.gitrule import GitPermissionsVocabulary |
56 | from lp.registry.interfaces.person import ( |
57 | @@ -190,6 +191,18 @@ class GitRepositoryNavigation(WebhookTargetNavigationMixin, Navigation): |
58 | |
59 | usedfor = IGitRepository |
60 | |
61 | + @stepthrough('+status') |
62 | + def traverse_status(self, id): |
63 | + try: |
64 | + report_id = int(id) |
65 | + except ValueError: |
66 | + raise NotFoundError(report_id) |
67 | + report = getUtility( |
68 | + IRevisionStatusReportSet).getByID(report_id) |
69 | + if report is None: |
70 | + raise NotFoundError(report_id) |
71 | + return report |
72 | + |
73 | @stepto("+ref") |
74 | def traverse_ref(self): |
75 | segments = list(self.request.getTraversalStack()) |
76 | diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml |
77 | index 6417c89..fb8e0d5 100644 |
78 | --- a/lib/lp/code/configure.zcml |
79 | +++ b/lib/lp/code/configure.zcml |
80 | @@ -953,6 +953,40 @@ |
81 | <allow interface="lp.code.interfaces.gitref.IGitRefRemoteSet" /> |
82 | </securedutility> |
83 | |
84 | + <!-- RevisionStatusReport --> |
85 | + |
86 | + <class class="lp.code.model.gitrepository.RevisionStatusReport"> |
87 | + <require |
88 | + permission="launchpad.View" |
89 | + interface="lp.code.interfaces.gitrepository.IRevisionStatusReportView |
90 | + lp.code.interfaces.gitrepository.IRevisionStatusReportEditableAttributes" /> |
91 | + <require |
92 | + permission="launchpad.Edit" |
93 | + interface="lp.code.interfaces.gitrepository.IRevisionStatusReportEdit" |
94 | + set_schema="lp.code.interfaces.gitrepository.IRevisionStatusReportEditableAttributes" /> |
95 | + </class> |
96 | + <class class="lp.code.model.gitrepository.RevisionStatusArtifact"> |
97 | + <require |
98 | + permission="launchpad.View" |
99 | + interface="lp.code.interfaces.gitrepository.IRevisionStatusArtifact" /> |
100 | + </class> |
101 | + <class class="lp.code.model.gitrepository.RevisionStatusReportSet"> |
102 | + <allow interface="lp.code.interfaces.gitrepository.IRevisionStatusReportSet" /> |
103 | + </class> |
104 | + <securedutility |
105 | + class="lp.code.model.gitrepository.RevisionStatusReportSet" |
106 | + provides="lp.code.interfaces.gitrepository.IRevisionStatusReportSet"> |
107 | + <allow interface="lp.code.interfaces.gitrepository.IRevisionStatusReportSet" /> |
108 | + </securedutility> |
109 | + <class class="lp.code.model.gitrepository.RevisionStatusArtifactSet"> |
110 | + <allow interface="lp.code.interfaces.gitrepository.IRevisionStatusArtifactSet" /> |
111 | + </class> |
112 | + <securedutility |
113 | + class="lp.code.model.gitrepository.RevisionStatusArtifactSet" |
114 | + provides="lp.code.interfaces.gitrepository.IRevisionStatusArtifactSet"> |
115 | + <allow interface="lp.code.interfaces.gitrepository.IRevisionStatusArtifactSet" /> |
116 | + </securedutility> |
117 | + |
118 | <!-- Git repository access rules --> |
119 | |
120 | <class class="lp.code.model.gitrule.GitRule"> |
121 | diff --git a/lib/lp/code/enums.py b/lib/lp/code/enums.py |
122 | index 3765754..2dbd287 100644 |
123 | --- a/lib/lp/code/enums.py |
124 | +++ b/lib/lp/code/enums.py |
125 | @@ -29,6 +29,8 @@ __all__ = [ |
126 | 'GitRepositoryType', |
127 | 'NON_CVS_RCS_TYPES', |
128 | 'RevisionControlSystems', |
129 | + 'RevisionStatusArtifactType', |
130 | + 'RevisionStatusResult', |
131 | 'TargetRevisionControlSystems', |
132 | ] |
133 | |
134 | @@ -252,6 +254,54 @@ class GitPermissionType(EnumeratedType): |
135 | CAN_FORCE_PUSH = Item("Can force-push") |
136 | |
137 | |
138 | +class RevisionStatusArtifactType(DBEnumeratedType): |
139 | + LOG = DBItem(0, """ |
140 | + Log |
141 | + |
142 | + The log produced by the check job. |
143 | + """) |
144 | + |
145 | + |
146 | +class RevisionStatusResult(DBEnumeratedType): |
147 | + """Revision Status Result""" |
148 | + |
149 | + WAITING = DBItem(0, """ |
150 | + Waiting |
151 | + |
152 | + The check job is waiting to be run. |
153 | + """) |
154 | + |
155 | + RUNNING = DBItem(1, """ |
156 | + Running |
157 | + |
158 | + The check job is currently running. |
159 | + """) |
160 | + |
161 | + SUCCEEDED = DBItem(2, """ |
162 | + Succeeded |
163 | + |
164 | + The check job ran successfully. |
165 | + """) |
166 | + |
167 | + FAILED = DBItem(3, """ |
168 | + Failed |
169 | + |
170 | + The check job failed. |
171 | + """) |
172 | + |
173 | + SKIPPED = DBItem(4, """ |
174 | + Skipped |
175 | + |
176 | + The check job was skipped. |
177 | + """) |
178 | + |
179 | + CANCELLED = DBItem(5, """ |
180 | + Cancelled |
181 | + |
182 | + The check job was cancelled. |
183 | + """) |
184 | + |
185 | + |
186 | class BranchLifecycleStatusFilter(EnumeratedType): |
187 | """Branch Lifecycle Status Filter |
188 | |
189 | diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py |
190 | index cb12a5d..47fb9a6 100644 |
191 | --- a/lib/lp/code/interfaces/gitrepository.py |
192 | +++ b/lib/lp/code/interfaces/gitrepository.py |
193 | @@ -13,9 +13,15 @@ __all__ = [ |
194 | 'IGitRepositoryExpensiveRequest', |
195 | 'IGitRepositorySet', |
196 | 'IHasGitRepositoryURL', |
197 | + 'IRevisionStatusArtifact', |
198 | + 'IRevisionStatusArtifactSet', |
199 | + 'IRevisionStatusReport', |
200 | + 'IRevisionStatusReportSet', |
201 | + 'RevisionStatusReportsFeatureDisabled', |
202 | 'user_has_special_git_repository_access', |
203 | ] |
204 | |
205 | +import http.client |
206 | import re |
207 | from textwrap import dedent |
208 | |
209 | @@ -23,6 +29,7 @@ from lazr.lifecycle.snapshot import doNotSnapshot |
210 | from lazr.restful.declarations import ( |
211 | call_with, |
212 | collection_default_content, |
213 | + error_status, |
214 | export_destructor_operation, |
215 | export_factory_operation, |
216 | export_operation_as, |
217 | @@ -37,6 +44,7 @@ from lazr.restful.declarations import ( |
218 | operation_returns_collection_of, |
219 | operation_returns_entry, |
220 | REQUEST_USER, |
221 | + scoped, |
222 | ) |
223 | from lazr.restful.fields import ( |
224 | CollectionField, |
225 | @@ -50,6 +58,7 @@ from zope.interface import ( |
226 | ) |
227 | from zope.schema import ( |
228 | Bool, |
229 | + Bytes, |
230 | Choice, |
231 | Datetime, |
232 | Int, |
233 | @@ -57,10 +66,12 @@ from zope.schema import ( |
234 | Text, |
235 | TextLine, |
236 | ) |
237 | +from zope.security.interfaces import Unauthorized |
238 | |
239 | from lp import _ |
240 | from lp.app.enums import InformationType |
241 | from lp.app.validators import LaunchpadValidationError |
242 | +from lp.app.validators.attachment import attachment_size_constraint |
243 | from lp.code.enums import ( |
244 | BranchMergeProposalStatus, |
245 | BranchSubscriptionDiffSize, |
246 | @@ -69,6 +80,8 @@ from lp.code.enums import ( |
247 | GitListingSort, |
248 | GitRepositoryStatus, |
249 | GitRepositoryType, |
250 | + RevisionStatusArtifactType, |
251 | + RevisionStatusResult, |
252 | ) |
253 | from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository |
254 | from lp.code.interfaces.hasgitrepositories import IHasGitRepositories |
255 | @@ -91,6 +104,7 @@ from lp.services.fields import ( |
256 | InlineObject, |
257 | PersonChoice, |
258 | PublicPersonChoice, |
259 | + URIField, |
260 | ) |
261 | from lp.services.webhooks.interfaces import IWebhookTarget |
262 | |
263 | @@ -813,6 +827,152 @@ class IGitRepositoryExpensiveRequest(Interface): |
264 | that is not an admin or a registry expert.""" |
265 | |
266 | |
267 | +@error_status(http.client.UNAUTHORIZED) |
268 | +class RevisionStatusReportsFeatureDisabled(Unauthorized): |
269 | + """Only certain users can access APIs for revision status reports.""" |
270 | + |
271 | + def __init__(self): |
272 | + super(RevisionStatusReportsFeatureDisabled, self).__init__( |
273 | + "You do not have permission to create revision status reports") |
274 | + |
275 | + |
276 | +class IRevisionStatusReportView(Interface): |
277 | + """`IRevisionStatusReport` attributes that require launchpad.View.""" |
278 | + |
279 | + id = Int(title=_("ID"), required=True, readonly=True) |
280 | + |
281 | + date_created = exported(Datetime( |
282 | + title=_("When the report was created."), required=True, readonly=True)) |
283 | + date_started = exported(Datetime( |
284 | + title=_("When the report was started.")), readonly=False) |
285 | + date_finished = exported(Datetime( |
286 | + title=_("When the report has finished.")), readonly=False) |
287 | + |
288 | + |
289 | +class IRevisionStatusReportEditableAttributes(Interface): |
290 | + """`IRevisionStatusReport` attributes that can be edited. |
291 | + |
292 | + These attributes need launchpad.View to see, and launchpad.Edit to change. |
293 | + """ |
294 | + |
295 | + title = exported(TextLine( |
296 | + title=_("A short title for the report."), required=True)) |
297 | + |
298 | + git_repository = exported(Reference( |
299 | + title=_("The Git repository for which this report is built."), |
300 | + # Really IGitRepository, patched in _schema_circular_imports.py. |
301 | + schema=Interface, required=True, readonly=True)) |
302 | + |
303 | + commit_sha1 = exported(TextLine( |
304 | + title=_("The Git commit for which this report is built."), |
305 | + required=True, readonly=True)) |
306 | + |
307 | + url = exported(URIField(title=_("URL"), required=False, readonly=True, |
308 | + description=_("The external url of the report."))) |
309 | + |
310 | + result_summary = exported(TextLine( |
311 | + title=_("A short summary of the result."), required=False)) |
312 | + |
313 | + result = exported(Choice( |
314 | + title=_('Result of the report'), readonly=True, |
315 | + required=False, vocabulary=RevisionStatusResult)) |
316 | + |
317 | + @mutator_for(result) |
318 | + @operation_parameters(result=copy_field(result)) |
319 | + @export_write_operation() |
320 | + @operation_for_version("devel") |
321 | + def transitionToNewResult(result): |
322 | + """Set the RevisionStatusReport result. |
323 | + |
324 | + Set the revision status report result.""" |
325 | + |
326 | + |
327 | +class IRevisionStatusReportEdit(Interface): |
328 | + """`IRevisionStatusReport` attributes that require launchpad.Edit.""" |
329 | + |
330 | + @operation_parameters( |
331 | + log_data=Bytes(title=_("The content of the artifact in bytes."), |
332 | + constraint=attachment_size_constraint)) |
333 | + @scoped(AccessTokenScope.REPOSITORY_BUILD_STATUS.title) |
334 | + @export_write_operation() |
335 | + @export_operation_as(name="setLog") |
336 | + @operation_for_version("devel") |
337 | + def api_setLog(log_data): |
338 | + """Set a new log on an existing status report. |
339 | + |
340 | + :param log_data: The contents (in bytes) of the log. |
341 | + """ |
342 | + |
343 | + |
344 | +@exported_as_webservice_entry(as_of="beta") |
345 | +class IRevisionStatusReport(IRevisionStatusReportView, |
346 | + IRevisionStatusReportEditableAttributes, |
347 | + IRevisionStatusReportEdit): |
348 | + """An revision status report for a Git commit.""" |
349 | + |
350 | + |
351 | +class IRevisionStatusReportSet(Interface): |
352 | + """The set of all revision status reports.""" |
353 | + |
354 | + def new(creator, title, git_repository, commit_sha1, date_created=None, |
355 | + url=None, result_summary=None, result=None, date_started=None, |
356 | + date_finished=None, log=None): |
357 | + """Return a new revision status report. |
358 | + |
359 | + :param title: A text string. |
360 | + :param git_repository: An `IGitRepository` for which the report |
361 | + is being created. |
362 | + :param commit_sha1: The sha1 of the commit for which the report |
363 | + is being created. |
364 | + :param date_created: The date when the report is being created. |
365 | + :param url: External URL to view result of report. |
366 | + :param result_summary: A short summary of the result. |
367 | + :param result: The result of the check job for this revision. |
368 | + :param date_started: DateTime that report was started. |
369 | + :param date_finished: DateTime that report was completed. |
370 | + :param log: Stores the content of the artifact for this report. |
371 | + """ |
372 | + |
373 | + def getByID(id): |
374 | + """Returns the RevisionStatusReport for a given ID.""" |
375 | + |
376 | + def findByRepository(repository): |
377 | + """Returns the set of RevisionStatusReport for a repository.""" |
378 | + |
379 | + |
380 | +class IRevisionStatusArtifactSet(Interface): |
381 | + """The set of all revision status artifacts.""" |
382 | + |
383 | + def new(lfa, report): |
384 | + """Return a new revision status artifact. |
385 | + |
386 | + :param lfa: An `ILibraryFileAlias`. |
387 | + :param report: An `IRevisionStatusReport` for which the |
388 | + artifact is being created. |
389 | + """ |
390 | + |
391 | + def getByID(id): |
392 | + """Returns the RevisionStatusArtifact for a given ID.""" |
393 | + |
394 | + def findByReport(report): |
395 | + """Returns the set of artifacts for a given report.""" |
396 | + |
397 | + |
398 | +class IRevisionStatusArtifact(Interface): |
399 | + id = Int(title=_("ID"), required=True, readonly=True) |
400 | + |
401 | + report = Attribute( |
402 | + "The `RevisionStatusReport` that this artifact is linked to.") |
403 | + |
404 | + library_file = Attribute( |
405 | + "The `LibraryFileAlias` object containing information for " |
406 | + "a revision status report.") |
407 | + |
408 | + artifact_type = Choice( |
409 | + title=_('The type of artifact, only log for now.'), |
410 | + vocabulary=RevisionStatusArtifactType) |
411 | + |
412 | + |
413 | class IGitRepositoryEdit(IWebhookTarget, IAccessTokenTarget): |
414 | """IGitRepository methods that require launchpad.Edit permission.""" |
415 | |
416 | @@ -1015,6 +1175,26 @@ class IGitRepositoryEdit(IWebhookTarget, IAccessTokenTarget): |
417 | :raise: CannotDeleteGitRepository if the repository cannot be deleted. |
418 | """ |
419 | |
420 | + @operation_parameters( |
421 | + title=copy_field(IRevisionStatusReport["title"]), |
422 | + commit_sha1=copy_field(IRevisionStatusReport["commit_sha1"]), |
423 | + url=copy_field(IRevisionStatusReport["url"]), |
424 | + result_summary=copy_field(IRevisionStatusReport["result_summary"]), |
425 | + result=copy_field(IRevisionStatusReport["result"])) |
426 | + @scoped(AccessTokenScope.REPOSITORY_BUILD_STATUS.title) |
427 | + @call_with(user=REQUEST_USER) |
428 | + @export_factory_operation(IRevisionStatusReport, []) |
429 | + @operation_for_version("devel") |
430 | + def newStatusReport(title, commit_sha1, url, result_summary, result, user): |
431 | + """Create a new status report. |
432 | + |
433 | + :param title: The name of the new report. |
434 | + :param commit_sha1: The commit sha1 for the report. |
435 | + :param url: The external link of the status report. |
436 | + :param result_summary: The description of the new report. |
437 | + :param result: The result of the new report. |
438 | + """ |
439 | + |
440 | |
441 | # XXX cjwatson 2015-01-19 bug=760849: "beta" is a lie to get WADL |
442 | # generation working. Individual attributes must set their version to |
443 | diff --git a/lib/lp/code/interfaces/webservice.py b/lib/lp/code/interfaces/webservice.py |
444 | index 02546e9..bd94298 100644 |
445 | --- a/lib/lp/code/interfaces/webservice.py |
446 | +++ b/lib/lp/code/interfaces/webservice.py |
447 | @@ -31,6 +31,7 @@ __all__ = [ |
448 | 'IGitSubscription', |
449 | 'IHasGitRepositories', |
450 | 'IPreviewDiff', |
451 | + 'IRevisionStatusReport', |
452 | 'ISourcePackageRecipe', |
453 | 'ISourcePackageRecipeBuild', |
454 | ] |
455 | @@ -66,6 +67,7 @@ from lp.code.interfaces.gitref import IGitRef |
456 | from lp.code.interfaces.gitrepository import ( |
457 | IGitRepository, |
458 | IGitRepositorySet, |
459 | + IRevisionStatusReport, |
460 | ) |
461 | from lp.code.interfaces.gitsubscription import IGitSubscription |
462 | from lp.code.interfaces.hasgitrepositories import IHasGitRepositories |
463 | diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py |
464 | index 812f1d7..501ef29 100644 |
465 | --- a/lib/lp/code/model/gitrepository.py |
466 | +++ b/lib/lp/code/model/gitrepository.py |
467 | @@ -6,6 +6,7 @@ __all__ = [ |
468 | 'GitRepository', |
469 | 'GitRepositorySet', |
470 | 'parse_git_commits', |
471 | + 'RevisionStatusReport', |
472 | ] |
473 | |
474 | from collections import ( |
475 | @@ -19,6 +20,7 @@ from datetime import ( |
476 | import email |
477 | from fnmatch import fnmatch |
478 | from functools import partial |
479 | +import io |
480 | from itertools import ( |
481 | chain, |
482 | groupby, |
483 | @@ -102,6 +104,8 @@ from lp.code.enums import ( |
484 | GitPermissionType, |
485 | GitRepositoryStatus, |
486 | GitRepositoryType, |
487 | + RevisionStatusArtifactType, |
488 | + RevisionStatusResult, |
489 | ) |
490 | from lp.code.errors import ( |
491 | CannotDeleteGitRepository, |
492 | @@ -131,6 +135,10 @@ from lp.code.interfaces.gitrepository import ( |
493 | GitIdentityMixin, |
494 | IGitRepository, |
495 | IGitRepositorySet, |
496 | + IRevisionStatusArtifactSet, |
497 | + IRevisionStatusReport, |
498 | + IRevisionStatusReportSet, |
499 | + RevisionStatusReportsFeatureDisabled, |
500 | user_has_special_git_repository_access, |
501 | ) |
502 | from lp.code.interfaces.gitrule import ( |
503 | @@ -205,6 +213,7 @@ from lp.services.identity.interfaces.account import ( |
504 | ) |
505 | from lp.services.job.interfaces.job import JobStatus |
506 | from lp.services.job.model.job import Job |
507 | +from lp.services.librarian.interfaces import ILibraryFileAliasSet |
508 | from lp.services.macaroons.interfaces import IMacaroonIssuer |
509 | from lp.services.macaroons.model import MacaroonIssuerBase |
510 | from lp.services.mail.notificationrecipientset import NotificationRecipientSet |
511 | @@ -219,8 +228,9 @@ from lp.services.webhooks.model import WebhookTargetMixin |
512 | from lp.snappy.interfaces.snap import ISnapSet |
513 | |
514 | |
515 | -logger = logging.getLogger(__name__) |
516 | +REVISION_STATUS_REPORT_ALLOW_CREATE = 'revision_status_report.allow_create' |
517 | |
518 | +logger = logging.getLogger(__name__) |
519 | |
520 | object_type_map = { |
521 | "commit": GitObjectType.COMMIT, |
522 | @@ -292,6 +302,132 @@ def git_repository_modified(repository, event): |
523 | send_git_repository_modified_notifications(repository, event) |
524 | |
525 | |
526 | +@implementer(IRevisionStatusReport) |
527 | +class RevisionStatusReport(StormBase): |
528 | + __storm_table__ = 'RevisionStatusReport' |
529 | + |
530 | + id = Int(primary=True) |
531 | + |
532 | + creator_id = Int(name="creator", allow_none=False) |
533 | + creator = Reference(creator_id, "Person.id") |
534 | + |
535 | + title = Unicode(name='name', allow_none=False) |
536 | + |
537 | + git_repository_id = Int(name='git_repository', allow_none=False) |
538 | + git_repository = Reference(git_repository_id, 'GitRepository.id') |
539 | + |
540 | + commit_sha1 = Unicode(name='commit_sha1', allow_none=False) |
541 | + |
542 | + url = Unicode(name='url', allow_none=True) |
543 | + |
544 | + result_summary = Unicode(name='description', allow_none=True) |
545 | + |
546 | + result = DBEnum(name='result', allow_none=True, enum=RevisionStatusResult) |
547 | + |
548 | + date_created = DateTime( |
549 | + name='date_created', tzinfo=pytz.UTC, allow_none=False) |
550 | + |
551 | + date_started = DateTime(name='date_started', tzinfo=pytz.UTC, |
552 | + allow_none=True) |
553 | + date_finished = DateTime(name='date_finished', tzinfo=pytz.UTC, |
554 | + allow_none=True) |
555 | + |
556 | + def __init__(self, git_repository, user, title, commit_sha1, |
557 | + url, result_summary, result): |
558 | + super().__init__() |
559 | + self.creator = user |
560 | + self.git_repository = git_repository |
561 | + self.title = title |
562 | + self.commit_sha1 = commit_sha1 |
563 | + self.url = url |
564 | + self.result_summary = result_summary |
565 | + self.result = result |
566 | + self.date_created = UTC_NOW |
567 | + |
568 | + def api_setLog(self, log_data): |
569 | + filename = '%s-%s.txt' % (self.title, self.commit_sha1) |
570 | + |
571 | + lfa = getUtility(ILibraryFileAliasSet).create( |
572 | + name=filename, size=len(log_data), |
573 | + file=io.BytesIO(log_data), contentType='text/plain') |
574 | + |
575 | + getUtility(IRevisionStatusArtifactSet).new(lfa, self) |
576 | + |
577 | + def transitionToNewResult(self, result): |
578 | + if self.result == RevisionStatusResult.WAITING: |
579 | + if result == RevisionStatusResult.RUNNING: |
580 | + self.date_started == UTC_NOW |
581 | + else: |
582 | + self.date_finished = UTC_NOW |
583 | + self.result = result |
584 | + |
585 | + |
586 | +@implementer(IRevisionStatusReportSet) |
587 | +class RevisionStatusReportSet: |
588 | + |
589 | + def new(self, creator, title, git_repository, commit_sha1, |
590 | + url=None, result_summary=None, result=None, |
591 | + date_started=None, date_finished=None, log=None): |
592 | + """See `IRevisionStatusReportSet`.""" |
593 | + store = IStore(RevisionStatusReport) |
594 | + report = RevisionStatusReport(git_repository, creator, title, |
595 | + commit_sha1, url, result_summary, |
596 | + result) |
597 | + store.add(report) |
598 | + return report |
599 | + |
600 | + def getByID(self, id): |
601 | + return IStore( |
602 | + RevisionStatusReport).find(RevisionStatusReport, id=id).one() |
603 | + |
604 | + def findByRepository(self, repository): |
605 | + return IStore(RevisionStatusReport).find( |
606 | + RevisionStatusReport, |
607 | + RevisionStatusReport.git_repository == repository) |
608 | + |
609 | + |
610 | +class RevisionStatusArtifact(StormBase): |
611 | + __storm_table__ = 'RevisionStatusArtifact' |
612 | + |
613 | + id = Int(primary=True) |
614 | + |
615 | + library_file_id = Int(name='library_file', allow_none=False) |
616 | + library_file = Reference(library_file_id, 'LibraryFileAlias.id') |
617 | + |
618 | + report_id = Int(name='report', allow_none=False) |
619 | + report = Reference(report_id, 'RevisionStatusReport.id') |
620 | + |
621 | + artifact_type = DBEnum(name='type', allow_none=False, |
622 | + enum=RevisionStatusArtifactType) |
623 | + |
624 | + def __init__(self, library_file, report): |
625 | + super().__init__() |
626 | + self.library_file = library_file |
627 | + self.report = report |
628 | + self.artifact_type = RevisionStatusArtifactType.LOG |
629 | + |
630 | + |
631 | +@implementer(IRevisionStatusArtifactSet) |
632 | +class RevisionStatusArtifactSet: |
633 | + |
634 | + def new(self, lfa, report): |
635 | + """See `IRevisionStatusArtifactSet`.""" |
636 | + store = IStore(RevisionStatusArtifact) |
637 | + artifact = RevisionStatusArtifact(lfa, report) |
638 | + store.add(artifact) |
639 | + return artifact |
640 | + |
641 | + def getById(self, id): |
642 | + return IStore(RevisionStatusArtifact).find( |
643 | + RevisionStatusArtifact, |
644 | + RevisionStatusArtifact.id == id).one() |
645 | + |
646 | + def findByReport(self, report): |
647 | + return IStore(RevisionStatusArtifact).find( |
648 | + RevisionStatusArtifact, |
649 | + RevisionStatusArtifact.report == report) |
650 | + |
651 | + |
652 | @implementer(IGitRepository, IHasOwner, IPrivacy, IInformationType) |
653 | class GitRepository(StormBase, WebhookTargetMixin, AccessTokenTargetMixin, |
654 | GitIdentityMixin): |
655 | @@ -501,6 +637,16 @@ class GitRepository(StormBase, WebhookTargetMixin, AccessTokenTargetMixin, |
656 | def collectGarbage(self): |
657 | getUtility(IGitHostingClient).collectGarbage(self.getInternalPath()) |
658 | |
659 | + def newStatusReport(self, user, title, commit_sha1, url=None, |
660 | + result_summary=None, result=None): |
661 | + |
662 | + if not getFeatureFlag(REVISION_STATUS_REPORT_ALLOW_CREATE): |
663 | + raise RevisionStatusReportsFeatureDisabled() |
664 | + |
665 | + report = RevisionStatusReport(self, user, title, commit_sha1, |
666 | + url, result_summary, result) |
667 | + return report |
668 | + |
669 | @property |
670 | def namespace(self): |
671 | """See `IGitRepository`.""" |
672 | diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py |
673 | index 252b535..764d13c 100644 |
674 | --- a/lib/lp/code/model/tests/test_gitrepository.py |
675 | +++ b/lib/lp/code/model/tests/test_gitrepository.py |
676 | @@ -10,6 +10,7 @@ from datetime import ( |
677 | import email |
678 | from functools import partial |
679 | import hashlib |
680 | +import io |
681 | import json |
682 | |
683 | from breezy import urlutils |
684 | @@ -65,6 +66,7 @@ from lp.code.enums import ( |
685 | GitObjectType, |
686 | GitRepositoryStatus, |
687 | GitRepositoryType, |
688 | + RevisionStatusResult, |
689 | TargetRevisionControlSystems, |
690 | ) |
691 | from lp.code.errors import ( |
692 | @@ -95,6 +97,8 @@ from lp.code.interfaces.gitrepository import ( |
693 | IGitRepository, |
694 | IGitRepositorySet, |
695 | IGitRepositoryView, |
696 | + IRevisionStatusArtifactSet, |
697 | + IRevisionStatusReportSet, |
698 | ) |
699 | from lp.code.interfaces.gitrule import ( |
700 | IGitNascentRule, |
701 | @@ -121,6 +125,7 @@ from lp.code.model.gitrepository import ( |
702 | DeletionCallable, |
703 | DeletionOperation, |
704 | GitRepository, |
705 | + REVISION_STATUS_REPORT_ALLOW_CREATE, |
706 | ) |
707 | from lp.code.tests.helpers import GitHostingFixture |
708 | from lp.code.xmlrpc.git import GitAPI |
709 | @@ -571,6 +576,24 @@ class TestGitRepository(TestCaseWithFactory): |
710 | self.assertThat(recorder2, HasQueryCount.byEquality(recorder1)) |
711 | self.assertEqual(7, recorder1.count) |
712 | |
713 | + def test_findRevisionStatusReport(self): |
714 | + repository = removeSecurityProxy(self.factory.makeGitRepository()) |
715 | + title = self.factory.getUniqueUnicode('report-title') |
716 | + commit_sha1 = hashlib.sha1(b"Some content").hexdigest() |
717 | + result_summary = "120/120 tests passed" |
718 | + |
719 | + report = self.factory.makeRevisionStatusReport( |
720 | + user=repository.owner, git_repository=repository, |
721 | + title=title, commit_sha1=commit_sha1, |
722 | + result_summary=result_summary, |
723 | + result=RevisionStatusResult.SUCCEEDED) |
724 | + |
725 | + with person_logged_in(repository.owner): |
726 | + result = getUtility( |
727 | + IRevisionStatusReportSet).getByID( |
728 | + report.id) |
729 | + self.assertEqual(report, result) |
730 | + |
731 | |
732 | class TestGitIdentityMixin(TestCaseWithFactory): |
733 | """Test the defaults and identities provided by GitIdentityMixin.""" |
734 | @@ -4241,6 +4264,66 @@ class TestGitRepositoryWebservice(TestCaseWithFactory): |
735 | self.assertEqual( |
736 | InformationType.PUBLIC, repository_db.information_type) |
737 | |
738 | + def test_newRevisionStatusReport_featureFlagDisabled(self): |
739 | + repository = self.factory.makeGitRepository() |
740 | + requester = repository.owner |
741 | + webservice = webservice_for_person(None, default_api_version="devel") |
742 | + with person_logged_in(requester): |
743 | + repository_url = api_url(repository) |
744 | + |
745 | + secret, _ = self.factory.makeAccessToken( |
746 | + owner=requester, target=repository, |
747 | + scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS]) |
748 | + header = {'Authorization': 'Token %s' % secret} |
749 | + |
750 | + response = webservice.named_post( |
751 | + repository_url, "newStatusReport", |
752 | + headers=header, title="CI", |
753 | + commit_sha1=hashlib.sha1( |
754 | + self.factory.getUniqueBytes()).hexdigest(), |
755 | + url='https://launchpad.net/', |
756 | + result_summary="120/120 tests passed", |
757 | + result="Succeeded") |
758 | + |
759 | + self.assertEqual(401, response.status) |
760 | + self.assertIn( |
761 | + b'You do not have permission to create revision status reports', |
762 | + response.body) |
763 | + |
764 | + def test_newRevisionStatusReport(self): |
765 | + repository = self.factory.makeGitRepository() |
766 | + requester = repository.owner |
767 | + webservice = webservice_for_person(None, default_api_version="devel") |
768 | + with person_logged_in(requester): |
769 | + repository_url = api_url(repository) |
770 | + |
771 | + secret, _ = self.factory.makeAccessToken( |
772 | + owner=requester, target=repository, |
773 | + scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS]) |
774 | + header = {'Authorization': 'Token %s' % secret} |
775 | + |
776 | + self.useFixture(FeatureFixture( |
777 | + {REVISION_STATUS_REPORT_ALLOW_CREATE: "on"})) |
778 | + |
779 | + response = webservice.named_post( |
780 | + repository_url, "newStatusReport", |
781 | + headers=header, title="CI", |
782 | + commit_sha1=hashlib.sha1( |
783 | + self.factory.getUniqueBytes()).hexdigest(), |
784 | + url='https://launchpad.net/', |
785 | + result_summary="120/120 tests passed", |
786 | + result="Succeeded") |
787 | + self.assertEqual(201, response.status) |
788 | + |
789 | + with person_logged_in(requester): |
790 | + results = getUtility( |
791 | + IRevisionStatusReportSet).findByRepository(repository) |
792 | + reports = list(results) |
793 | + urls = [webservice.getAbsoluteUrl('%s/+status/%s' % ( |
794 | + api_url(repository), |
795 | + report.id)) for report in reports] |
796 | + self.assertIn(response.getHeader("Location"), urls) |
797 | + |
798 | def test_set_target(self): |
799 | # The repository owner can move the repository to another target; |
800 | # this redirects to the new location. |
801 | @@ -4831,6 +4914,60 @@ class TestGitRepositoryWebservice(TestCaseWithFactory): |
802 | response.body) |
803 | |
804 | |
805 | +class TestRevisionStatusReportWebservice(TestCaseWithFactory): |
806 | + layer = LaunchpadFunctionalLayer |
807 | + |
808 | + def setUp(self): |
809 | + super(TestRevisionStatusReportWebservice, self).setUp() |
810 | + self.repository = self.factory.makeGitRepository() |
811 | + self.requester = self.repository.owner |
812 | + title = self.factory.getUniqueUnicode('report-title') |
813 | + commit_sha1 = hashlib.sha1(b"Some content").hexdigest() |
814 | + result_summary = "120/120 tests passed" |
815 | + |
816 | + self.report = self.factory.makeRevisionStatusReport( |
817 | + user=self.repository.owner, git_repository=self.repository, |
818 | + title=title, commit_sha1=commit_sha1, |
819 | + result_summary=result_summary, |
820 | + result=RevisionStatusResult.SUCCEEDED) |
821 | + |
822 | + self.webservice = webservice_for_person( |
823 | + None, default_api_version="devel") |
824 | + with person_logged_in(self.requester): |
825 | + self.report_url = api_url(self.report) |
826 | + |
827 | + secret, _ = self.factory.makeAccessToken( |
828 | + owner=self.requester, target=self.repository, |
829 | + scopes=[AccessTokenScope.REPOSITORY_BUILD_STATUS]) |
830 | + self.header = {'Authorization': 'Token %s' % secret} |
831 | + |
832 | + def test_setLogOnRevisionStatusReport(self): |
833 | + content = b'log_content_data' |
834 | + filesize = len(content) |
835 | + sha1 = hashlib.sha1(content).hexdigest() |
836 | + md5 = hashlib.md5(content).hexdigest() |
837 | + response = self.webservice.named_post( |
838 | + self.report_url, "setLog", |
839 | + headers=self.header, |
840 | + log_data=io.BytesIO(content)) |
841 | + self.assertEqual(200, response.status) |
842 | + |
843 | + # A report may have multiple artifacts. |
844 | + # We verify that the content we just submitted via API now |
845 | + # matches one of the artifacts in the DB for the report. |
846 | + with person_logged_in(self.requester): |
847 | + artifacts = list(getUtility( |
848 | + IRevisionStatusArtifactSet).findByReport(self.report)) |
849 | + lfcs = [artifact.library_file.content for artifact in artifacts] |
850 | + sha1_of_all_artifacts = [lfc.sha1 for lfc in lfcs] |
851 | + md5_of_all_artifacts = [lfc.md5 for lfc in lfcs] |
852 | + filesizes_of_all_artifacts = [lfc.filesize for lfc in lfcs] |
853 | + |
854 | + self.assertIn(sha1, sha1_of_all_artifacts) |
855 | + self.assertIn(md5, md5_of_all_artifacts) |
856 | + self.assertIn(filesize, filesizes_of_all_artifacts) |
857 | + |
858 | + |
859 | class TestGitRepositoryMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory): |
860 | """Test GitRepository macaroon issuing and verification.""" |
861 | |
862 | diff --git a/lib/lp/security.py b/lib/lp/security.py |
863 | index 8f66fbb..8435167 100644 |
864 | --- a/lib/lp/security.py |
865 | +++ b/lib/lp/security.py |
866 | @@ -99,6 +99,8 @@ from lp.code.interfaces.gitcollection import IGitCollection |
867 | from lp.code.interfaces.gitref import IGitRef |
868 | from lp.code.interfaces.gitrepository import ( |
869 | IGitRepository, |
870 | + IRevisionStatusArtifact, |
871 | + IRevisionStatusReport, |
872 | user_has_special_git_repository_access, |
873 | ) |
874 | from lp.code.interfaces.gitrule import ( |
875 | @@ -683,6 +685,23 @@ class EditSpecificationByRelatedPeople(AuthorizationBase): |
876 | self.obj, ['owner', 'drafter', 'assignee', 'approver'])) |
877 | |
878 | |
879 | +class EditRevisionStatusReport(AuthorizationBase): |
880 | + """The owner of a Git repository can edit its status reports.""" |
881 | + permission = 'launchpad.Edit' |
882 | + usedfor = IRevisionStatusReport |
883 | + |
884 | + def checkAuthenticated(self, user): |
885 | + return user.isOwner(self.obj.git_repository) |
886 | + |
887 | + |
888 | +class EditRevisionStatusArtifact(DelegatedAuthorization): |
889 | + permission = 'launchpad.Edit' |
890 | + usedfor = IRevisionStatusArtifact |
891 | + |
892 | + def __init__(self, obj): |
893 | + super().__init__(obj, obj.report, 'launchpad.Edit') |
894 | + |
895 | + |
896 | class AdminSpecification(AuthorizationBase): |
897 | permission = 'launchpad.Admin' |
898 | usedfor = ISpecification |
899 | diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py |
900 | index 9d59ba8..db48ed3 100644 |
901 | --- a/lib/lp/testing/factory.py |
902 | +++ b/lib/lp/testing/factory.py |
903 | @@ -131,7 +131,10 @@ from lp.code.interfaces.gitref import ( |
904 | IGitRef, |
905 | IGitRefRemoteSet, |
906 | ) |
907 | -from lp.code.interfaces.gitrepository import IGitRepository |
908 | +from lp.code.interfaces.gitrepository import ( |
909 | + IGitRepository, |
910 | + IRevisionStatusArtifactSet, |
911 | + ) |
912 | from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch |
913 | from lp.code.interfaces.revision import IRevisionSet |
914 | from lp.code.interfaces.sourcepackagerecipe import ( |
915 | @@ -146,6 +149,7 @@ from lp.code.model.diff import ( |
916 | Diff, |
917 | PreviewDiff, |
918 | ) |
919 | +from lp.code.model.gitrepository import IRevisionStatusReportSet |
920 | from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet |
921 | from lp.oci.interfaces.ocirecipe import IOCIRecipeSet |
922 | from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet |
923 | @@ -1843,6 +1847,30 @@ class BareLaunchpadObjectFactory(ObjectFactory): |
924 | grantee, grantor, can_create=can_create, can_push=can_push, |
925 | can_force_push=can_force_push) |
926 | |
927 | + def makeRevisionStatusReport(self, user=None, title=None, |
928 | + git_repository=None, commit_sha1=None, |
929 | + result_summary=None, url=None, result=None): |
930 | + """Create a new RevisionStatusReport.""" |
931 | + if title is None: |
932 | + title = self.getUniqueUnicode() |
933 | + if git_repository is None: |
934 | + git_repository = self.makeGitRepository() |
935 | + if user is None: |
936 | + user = git_repository.owner |
937 | + if commit_sha1 is None: |
938 | + commit_sha1 = hashlib.sha1(self.getUniqueBytes()).hexdigest() |
939 | + return getUtility(IRevisionStatusReportSet).new( |
940 | + user, title, git_repository, commit_sha1, result_summary, |
941 | + url, result) |
942 | + |
943 | + def makeRevisionStatusArtifact(self, lfa=None, report=None): |
944 | + """Create a new RevisionStatusArtifact.""" |
945 | + if lfa is None: |
946 | + lfa = self.makeLibraryFileAlias() |
947 | + if report is None: |
948 | + report = self.makeRevisionStatusReport() |
949 | + return getUtility(IRevisionStatusArtifactSet).new(lfa, report) |
950 | + |
951 | def makeBug(self, target=None, owner=None, bug_watch_url=None, |
952 | information_type=None, date_closed=None, title=None, |
953 | date_created=None, description=None, comment=None, |
This is the branch in progress with the API test (test_newRevisi onStatusReport) failing and the model test (test_findRevis ionStatusReport ) passing. There are quite a few pre-commit long lines truncated as part of the MP as well in order to be able to commit.
In parallel I will be working on further changes on the model side + Unit Tests (to cover updates and deletes and the RevisionStatusA rtifact) - I won't be updating the MP so that input in current direction and failing unit test (test_newRevisi onStatusReport) can be provided.