Merge lp:~cjwatson/launchpad/queue-api-overrides into lp:launchpad
- queue-api-overrides
- Merge into devel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | j.c.sackett | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 15575 | ||||
Proposed branch: | lp:~cjwatson/launchpad/queue-api-overrides | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
518 lines (+241/-20) 5 files modified
lib/lp/soyuz/browser/queue.py (+5/-2) lib/lp/soyuz/doc/distroseriesqueue.txt (+5/-5) lib/lp/soyuz/interfaces/queue.py (+37/-2) lib/lp/soyuz/model/queue.py (+43/-7) lib/lp/soyuz/tests/test_packageupload.py (+151/-4) |
||||
To merge this branch: | bzr merge lp:~cjwatson/launchpad/queue-api-overrides | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
j.c.sackett (community) | Approve | ||
Review via email: mp+113437@code.launchpad.net |
Commit message
Export PackageUpload.
Description of the change
== Summary ==
Replace the queue tool in Launchpad with an API client (part five).
== Proposed fix ==
This branch exports the override methods on PackageUpload. With this in addition to what's already landed, the client should be feature-complete, and the remaining work should consist of cleanups.
== Pre-implementation notes ==
I've gone round a few times with various people, particularly William Grant, on the exact way to export all of this stuff, because I gather that we want to avoid exposing the current data model in order that it can be rearranged in the future. This has led to the following design choices:
* Everything is on devel. The only clients for this should be tools such as those in lp:ubuntu-archive-tools, which can be kept up to date if there's a need to change these interfaces.
* There are source packages with lots of binaries that sometimes need to be overridden individually (e.g. linux) and API requests aren't especially fast. In a previous branch (currently QAed and awaiting deployment) I amended Archive.
I extracted this branch from https:/
== Implementation details ==
The only thing I think is notable here is that I copied the new-component-
== LOC Rationale ==
+218. As with https:/
== Tests ==
bin/test -vvct test_packageupload
== Demo and Q/A ==
http://
Preview Diff
1 | === modified file 'lib/lp/soyuz/browser/queue.py' | |||
2 | --- lib/lp/soyuz/browser/queue.py 2012-07-05 09:49:55 +0000 | |||
3 | +++ lib/lp/soyuz/browser/queue.py 2012-07-06 16:20:24 +0000 | |||
4 | @@ -45,6 +45,7 @@ | |||
5 | 45 | from lp.soyuz.interfaces.queue import ( | 45 | from lp.soyuz.interfaces.queue import ( |
6 | 46 | IPackageUpload, | 46 | IPackageUpload, |
7 | 47 | IPackageUploadSet, | 47 | IPackageUploadSet, |
8 | 48 | QueueAdminUnauthorizedError, | ||
9 | 48 | QueueInconsistentStateError, | 49 | QueueInconsistentStateError, |
10 | 49 | ) | 50 | ) |
11 | 50 | from lp.soyuz.interfaces.section import ISectionSet | 51 | from lp.soyuz.interfaces.section import ISectionSet |
12 | @@ -388,7 +389,8 @@ | |||
13 | 388 | }] | 389 | }] |
14 | 389 | binary_overridden = queue_item.overrideBinaries( | 390 | binary_overridden = queue_item.overrideBinaries( |
15 | 390 | binary_changes, allowed_components) | 391 | binary_changes, allowed_components) |
17 | 391 | except QueueInconsistentStateError as info: | 392 | except (QueueAdminUnauthorizedError, |
18 | 393 | QueueInconsistentStateError) as info: | ||
19 | 392 | failure.append("FAILED: %s (%s)" % | 394 | failure.append("FAILED: %s (%s)" % |
20 | 393 | (queue_item.displayname, info)) | 395 | (queue_item.displayname, info)) |
21 | 394 | continue | 396 | continue |
22 | @@ -409,7 +411,8 @@ | |||
23 | 409 | 411 | ||
24 | 410 | try: | 412 | try: |
25 | 411 | getattr(self, 'queue_action_' + action)(queue_item) | 413 | getattr(self, 'queue_action_' + action)(queue_item) |
27 | 412 | except QueueInconsistentStateError as info: | 414 | except (QueueAdminUnauthorizedError, |
28 | 415 | QueueInconsistentStateError) as info: | ||
29 | 413 | failure.append('FAILED: %s (%s)' % | 416 | failure.append('FAILED: %s (%s)' % |
30 | 414 | (queue_item.displayname, info)) | 417 | (queue_item.displayname, info)) |
31 | 415 | else: | 418 | else: |
32 | 416 | 419 | ||
33 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue.txt' | |||
34 | --- lib/lp/soyuz/doc/distroseriesqueue.txt 2012-07-03 09:29:35 +0000 | |||
35 | +++ lib/lp/soyuz/doc/distroseriesqueue.txt 2012-07-06 16:20:24 +0000 | |||
36 | @@ -648,7 +648,7 @@ | |||
37 | 648 | In addition to these parameters, you must also supply | 648 | In addition to these parameters, you must also supply |
38 | 649 | "allowed_components", which is a sequence of IComponent. Any overrides | 649 | "allowed_components", which is a sequence of IComponent. Any overrides |
39 | 650 | must have the existing and new component in this sequence otherwise | 650 | must have the existing and new component in this sequence otherwise |
41 | 651 | QueueInconsistentStateError is raised. | 651 | QueueAdminUnauthorizedError is raised. |
42 | 652 | 652 | ||
43 | 653 | The alsa-utils source is already in the queue with component "main" | 653 | The alsa-utils source is already in the queue with component "main" |
44 | 654 | and section "base". | 654 | and section "base". |
45 | @@ -673,7 +673,7 @@ | |||
46 | 673 | ... allowed_components=(universe,)) | 673 | ... allowed_components=(universe,)) |
47 | 674 | Traceback (most recent call last): | 674 | Traceback (most recent call last): |
48 | 675 | ... | 675 | ... |
50 | 676 | QueueInconsistentStateError: No rights to override to restricted | 676 | QueueAdminUnauthorizedError: No rights to override to restricted |
51 | 677 | 677 | ||
52 | 678 | Allowing "restricted" still won't work because the original component | 678 | Allowing "restricted" still won't work because the original component |
53 | 679 | is "main": | 679 | is "main": |
54 | @@ -683,7 +683,7 @@ | |||
55 | 683 | ... allowed_components=(restricted,)) | 683 | ... allowed_components=(restricted,)) |
56 | 684 | Traceback (most recent call last): | 684 | Traceback (most recent call last): |
57 | 685 | ... | 685 | ... |
59 | 686 | QueueInconsistentStateError: No rights to override from main | 686 | QueueAdminUnauthorizedError: No rights to override from main |
60 | 687 | 687 | ||
61 | 688 | Specifying both main and restricted allows the override to restricted/web. | 688 | Specifying both main and restricted allows the override to restricted/web. |
62 | 689 | overrideSource() returns True if it completed the task. | 689 | overrideSource() returns True if it completed the task. |
63 | @@ -719,13 +719,13 @@ | |||
64 | 719 | ... binary_changes, allowed_components=(universe,)) | 719 | ... binary_changes, allowed_components=(universe,)) |
65 | 720 | Traceback (most recent call last): | 720 | Traceback (most recent call last): |
66 | 721 | ... | 721 | ... |
68 | 722 | QueueInconsistentStateError: No rights to override to restricted | 722 | QueueAdminUnauthorizedError: No rights to override to restricted |
69 | 723 | 723 | ||
70 | 724 | >>> print item.overrideBinaries( | 724 | >>> print item.overrideBinaries( |
71 | 725 | ... binary_changes, allowed_components=(restricted,)) | 725 | ... binary_changes, allowed_components=(restricted,)) |
72 | 726 | Traceback (most recent call last): | 726 | Traceback (most recent call last): |
73 | 727 | ... | 727 | ... |
75 | 728 | QueueInconsistentStateError: No rights to override from main | 728 | QueueAdminUnauthorizedError: No rights to override from main |
76 | 729 | 729 | ||
77 | 730 | >>> print item.overrideBinaries( | 730 | >>> print item.overrideBinaries( |
78 | 731 | ... binary_changes, allowed_components=(main,restricted)) | 731 | ... binary_changes, allowed_components=(main,restricted)) |
79 | 732 | 732 | ||
80 | === modified file 'lib/lp/soyuz/interfaces/queue.py' | |||
81 | --- lib/lp/soyuz/interfaces/queue.py 2012-07-04 13:02:32 +0000 | |||
82 | +++ lib/lp/soyuz/interfaces/queue.py 2012-07-06 16:20:24 +0000 | |||
83 | @@ -16,6 +16,7 @@ | |||
84 | 16 | 'IPackageUploadCustom', | 16 | 'IPackageUploadCustom', |
85 | 17 | 'IPackageUploadSet', | 17 | 'IPackageUploadSet', |
86 | 18 | 'NonBuildableSourceUploadError', | 18 | 'NonBuildableSourceUploadError', |
87 | 19 | 'QueueAdminUnauthorizedError', | ||
88 | 19 | 'QueueBuildAcceptError', | 20 | 'QueueBuildAcceptError', |
89 | 20 | 'QueueInconsistentStateError', | 21 | 'QueueInconsistentStateError', |
90 | 21 | 'QueueSourceAcceptError', | 22 | 'QueueSourceAcceptError', |
91 | @@ -26,12 +27,15 @@ | |||
92 | 26 | 27 | ||
93 | 27 | from lazr.enum import DBEnumeratedType | 28 | from lazr.enum import DBEnumeratedType |
94 | 28 | from lazr.restful.declarations import ( | 29 | from lazr.restful.declarations import ( |
95 | 30 | call_with, | ||
96 | 29 | error_status, | 31 | error_status, |
97 | 30 | export_as_webservice_entry, | 32 | export_as_webservice_entry, |
98 | 31 | export_read_operation, | 33 | export_read_operation, |
99 | 32 | export_write_operation, | 34 | export_write_operation, |
100 | 33 | exported, | 35 | exported, |
101 | 34 | operation_for_version, | 36 | operation_for_version, |
102 | 37 | operation_parameters, | ||
103 | 38 | REQUEST_USER, | ||
104 | 35 | ) | 39 | ) |
105 | 36 | from lazr.restful.fields import Reference | 40 | from lazr.restful.fields import Reference |
106 | 37 | from zope.interface import ( | 41 | from zope.interface import ( |
107 | @@ -42,10 +46,12 @@ | |||
108 | 42 | Bool, | 46 | Bool, |
109 | 43 | Choice, | 47 | Choice, |
110 | 44 | Datetime, | 48 | Datetime, |
111 | 49 | Dict, | ||
112 | 45 | Int, | 50 | Int, |
113 | 46 | List, | 51 | List, |
114 | 47 | TextLine, | 52 | TextLine, |
115 | 48 | ) | 53 | ) |
116 | 54 | from zope.security.interfaces import Unauthorized | ||
117 | 49 | 55 | ||
118 | 50 | from lp import _ | 56 | from lp import _ |
119 | 51 | from lp.soyuz.enums import PackageUploadStatus | 57 | from lp.soyuz.enums import PackageUploadStatus |
120 | @@ -69,6 +75,10 @@ | |||
121 | 69 | """ | 75 | """ |
122 | 70 | 76 | ||
123 | 71 | 77 | ||
124 | 78 | class QueueAdminUnauthorizedError(Unauthorized): | ||
125 | 79 | """User not permitted to perform a queue administration operation.""" | ||
126 | 80 | |||
127 | 81 | |||
128 | 72 | class NonBuildableSourceUploadError(QueueInconsistentStateError): | 82 | class NonBuildableSourceUploadError(QueueInconsistentStateError): |
129 | 73 | """Source upload will not result in any build record. | 83 | """Source upload will not result in any build record. |
130 | 74 | 84 | ||
131 | @@ -394,7 +404,14 @@ | |||
132 | 394 | :param logger: Specify a logger object if required. Mainly for tests. | 404 | :param logger: Specify a logger object if required. Mainly for tests. |
133 | 395 | """ | 405 | """ |
134 | 396 | 406 | ||
136 | 397 | def overrideSource(new_component, new_section, allowed_components): | 407 | @operation_parameters( |
137 | 408 | new_component=TextLine(title=u"The new component name."), | ||
138 | 409 | new_section=TextLine(title=u"The new section name.")) | ||
139 | 410 | @call_with(allowed_components=None, user=REQUEST_USER) | ||
140 | 411 | @export_write_operation() | ||
141 | 412 | @operation_for_version('devel') | ||
142 | 413 | def overrideSource(new_component=None, new_section=None, | ||
143 | 414 | allowed_components=None, user=None): | ||
144 | 398 | """Override the source package contained in this queue item. | 415 | """Override the source package contained in this queue item. |
145 | 399 | 416 | ||
146 | 400 | :param new_component: An IComponent to replace the existing one | 417 | :param new_component: An IComponent to replace the existing one |
147 | @@ -403,6 +420,8 @@ | |||
148 | 403 | in the upload's source. | 420 | in the upload's source. |
149 | 404 | :param allowed_components: A sequence of components that the | 421 | :param allowed_components: A sequence of components that the |
150 | 405 | callsite is allowed to override from and to. | 422 | callsite is allowed to override from and to. |
151 | 423 | :param user: The user requesting the override change, used if | ||
152 | 424 | allowed_components is None. | ||
153 | 406 | 425 | ||
154 | 407 | :raises QueueInconsistentStateError: if either the existing | 426 | :raises QueueInconsistentStateError: if either the existing |
155 | 408 | or the new_component are not in the allowed_components | 427 | or the new_component are not in the allowed_components |
156 | @@ -414,7 +433,21 @@ | |||
157 | 414 | :return: True if the source was overridden. | 433 | :return: True if the source was overridden. |
158 | 415 | """ | 434 | """ |
159 | 416 | 435 | ||
161 | 417 | def overrideBinaries(changes, allowed_components): | 436 | @operation_parameters( |
162 | 437 | changes=List( | ||
163 | 438 | title=u"A sequence of changes to apply.", | ||
164 | 439 | description=( | ||
165 | 440 | u"Each item may have a 'name' item which specifies the binary " | ||
166 | 441 | "package name to override; otherwise, the change applies to " | ||
167 | 442 | "all binaries in the upload. It may also have 'component', " | ||
168 | 443 | "'section', and 'priority' items which replace the " | ||
169 | 444 | "corresponding existing one in the upload's overridden " | ||
170 | 445 | "binaries."), | ||
171 | 446 | value_type=Dict(key_type=TextLine()))) | ||
172 | 447 | @call_with(allowed_components=None, user=REQUEST_USER) | ||
173 | 448 | @export_write_operation() | ||
174 | 449 | @operation_for_version('devel') | ||
175 | 450 | def overrideBinaries(changes, allowed_components=None, user=None): | ||
176 | 418 | """Override binary packages in a binary queue item. | 451 | """Override binary packages in a binary queue item. |
177 | 419 | 452 | ||
178 | 420 | :param changes: A sequence of mappings of changes to apply. Each | 453 | :param changes: A sequence of mappings of changes to apply. Each |
179 | @@ -426,6 +459,8 @@ | |||
180 | 426 | left unchanged. | 459 | left unchanged. |
181 | 427 | :param allowed_components: A sequence of components that the | 460 | :param allowed_components: A sequence of components that the |
182 | 428 | callsite is allowed to override from and to. | 461 | callsite is allowed to override from and to. |
183 | 462 | :param user: The user requesting the override change, used if | ||
184 | 463 | allowed_components is None. | ||
185 | 429 | 464 | ||
186 | 430 | :raises QueueInconsistentStateError: if either the existing | 465 | :raises QueueInconsistentStateError: if either the existing |
187 | 431 | or the new_component are not in the allowed_components | 466 | or the new_component are not in the allowed_components |
188 | 432 | 467 | ||
189 | === modified file 'lib/lp/soyuz/model/queue.py' | |||
190 | --- lib/lp/soyuz/model/queue.py 2012-07-06 13:51:34 +0000 | |||
191 | +++ lib/lp/soyuz/model/queue.py 2012-07-06 16:20:24 +0000 | |||
192 | @@ -84,6 +84,7 @@ | |||
193 | 84 | PriorityNotFound, | 84 | PriorityNotFound, |
194 | 85 | SectionNotFound, | 85 | SectionNotFound, |
195 | 86 | ) | 86 | ) |
196 | 87 | from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet | ||
197 | 87 | from lp.soyuz.interfaces.component import IComponentSet | 88 | from lp.soyuz.interfaces.component import IComponentSet |
198 | 88 | from lp.soyuz.interfaces.packagecopyjob import IPackageCopyJobSource | 89 | from lp.soyuz.interfaces.packagecopyjob import IPackageCopyJobSource |
199 | 89 | from lp.soyuz.interfaces.publishing import ( | 90 | from lp.soyuz.interfaces.publishing import ( |
200 | @@ -99,6 +100,7 @@ | |||
201 | 99 | IPackageUploadSet, | 100 | IPackageUploadSet, |
202 | 100 | IPackageUploadSource, | 101 | IPackageUploadSource, |
203 | 101 | NonBuildableSourceUploadError, | 102 | NonBuildableSourceUploadError, |
204 | 103 | QueueAdminUnauthorizedError, | ||
205 | 102 | QueueBuildAcceptError, | 104 | QueueBuildAcceptError, |
206 | 103 | QueueInconsistentStateError, | 105 | QueueInconsistentStateError, |
207 | 104 | QueueSourceAcceptError, | 106 | QueueSourceAcceptError, |
208 | @@ -1038,7 +1040,7 @@ | |||
209 | 1038 | allowed_component_names = [ | 1040 | allowed_component_names = [ |
210 | 1039 | component.name for component in allowed_components] | 1041 | component.name for component in allowed_components] |
211 | 1040 | if copy_job.component_name not in allowed_component_names: | 1042 | if copy_job.component_name not in allowed_component_names: |
213 | 1041 | raise QueueInconsistentStateError( | 1043 | raise QueueAdminUnauthorizedError( |
214 | 1042 | "No rights to override from %s" % copy_job.component_name) | 1044 | "No rights to override from %s" % copy_job.component_name) |
215 | 1043 | copy_job.addSourceOverride(SourceOverride( | 1045 | copy_job.addSourceOverride(SourceOverride( |
216 | 1044 | copy_job.package_name, new_component, new_section)) | 1046 | copy_job.package_name, new_component, new_section)) |
217 | @@ -1055,7 +1057,7 @@ | |||
218 | 1055 | if old_component not in allowed_components: | 1057 | if old_component not in allowed_components: |
219 | 1056 | # The old component is not in the list of allowed components | 1058 | # The old component is not in the list of allowed components |
220 | 1057 | # to override. | 1059 | # to override. |
222 | 1058 | raise QueueInconsistentStateError( | 1060 | raise QueueAdminUnauthorizedError( |
223 | 1059 | "No rights to override from %s" % old_component.name) | 1061 | "No rights to override from %s" % old_component.name) |
224 | 1060 | source.sourcepackagerelease.override( | 1062 | source.sourcepackagerelease.override( |
225 | 1061 | component=new_component, section=new_section) | 1063 | component=new_component, section=new_section) |
226 | @@ -1068,7 +1070,8 @@ | |||
227 | 1068 | 1070 | ||
228 | 1069 | return made_changes | 1071 | return made_changes |
229 | 1070 | 1072 | ||
231 | 1071 | def overrideSource(self, new_component, new_section, allowed_components): | 1073 | def overrideSource(self, new_component=None, new_section=None, |
232 | 1074 | allowed_components=None, user=None): | ||
233 | 1072 | """See `IPackageUpload`.""" | 1075 | """See `IPackageUpload`.""" |
234 | 1073 | if new_component is None and new_section is None: | 1076 | if new_component is None and new_section is None: |
235 | 1074 | # Nothing needs overriding, bail out. | 1077 | # Nothing needs overriding, bail out. |
236 | @@ -1077,8 +1080,19 @@ | |||
237 | 1077 | new_component = self._nameToComponent(new_component) | 1080 | new_component = self._nameToComponent(new_component) |
238 | 1078 | new_section = self._nameToSection(new_section) | 1081 | new_section = self._nameToSection(new_section) |
239 | 1079 | 1082 | ||
240 | 1083 | if allowed_components is None and user is not None: | ||
241 | 1084 | # Get a list of components for which the user has rights to | ||
242 | 1085 | # override to or from. | ||
243 | 1086 | permission_set = getUtility(IArchivePermissionSet) | ||
244 | 1087 | permissions = permission_set.componentsForQueueAdmin( | ||
245 | 1088 | self.distroseries.main_archive, user) | ||
246 | 1089 | allowed_components = set( | ||
247 | 1090 | permission.component for permission in permissions) | ||
248 | 1091 | assert allowed_components is not None, ( | ||
249 | 1092 | "Must provide allowed_components for non-webservice calls.") | ||
250 | 1093 | |||
251 | 1080 | if new_component not in list(allowed_components) + [None]: | 1094 | if new_component not in list(allowed_components) + [None]: |
253 | 1081 | raise QueueInconsistentStateError( | 1095 | raise QueueAdminUnauthorizedError( |
254 | 1082 | "No rights to override to %s" % new_component.name) | 1096 | "No rights to override to %s" % new_component.name) |
255 | 1083 | 1097 | ||
256 | 1084 | return ( | 1098 | return ( |
257 | @@ -1113,7 +1127,7 @@ | |||
258 | 1113 | 1127 | ||
259 | 1114 | return changes_by_name, changes_for_all | 1128 | return changes_by_name, changes_for_all |
260 | 1115 | 1129 | ||
262 | 1116 | def overrideBinaries(self, changes, allowed_components): | 1130 | def overrideBinaries(self, changes, allowed_components=None, user=None): |
263 | 1117 | """See `IPackageUpload`.""" | 1131 | """See `IPackageUpload`.""" |
264 | 1118 | if not self.contains_build: | 1132 | if not self.contains_build: |
265 | 1119 | return False | 1133 | return False |
266 | @@ -1122,6 +1136,17 @@ | |||
267 | 1122 | # Nothing needs overriding, bail out. | 1136 | # Nothing needs overriding, bail out. |
268 | 1123 | return False | 1137 | return False |
269 | 1124 | 1138 | ||
270 | 1139 | if allowed_components is None and user is not None: | ||
271 | 1140 | # Get a list of components for which the user has rights to | ||
272 | 1141 | # override to or from. | ||
273 | 1142 | permission_set = getUtility(IArchivePermissionSet) | ||
274 | 1143 | permissions = permission_set.componentsForQueueAdmin( | ||
275 | 1144 | self.distroseries.main_archive, user) | ||
276 | 1145 | allowed_components = set( | ||
277 | 1146 | permission.component for permission in permissions) | ||
278 | 1147 | assert allowed_components is not None, ( | ||
279 | 1148 | "Must provide allowed_components for non-webservice calls.") | ||
280 | 1149 | |||
281 | 1125 | changes_by_name, changes_for_all = self._filterBinaryChanges(changes) | 1150 | changes_by_name, changes_for_all = self._filterBinaryChanges(changes) |
282 | 1126 | 1151 | ||
283 | 1127 | new_components = set() | 1152 | new_components = set() |
284 | @@ -1135,12 +1160,23 @@ | |||
285 | 1135 | component.name | 1160 | component.name |
286 | 1136 | for component in new_components.difference(allowed_components)) | 1161 | for component in new_components.difference(allowed_components)) |
287 | 1137 | if disallowed_components: | 1162 | if disallowed_components: |
289 | 1138 | raise QueueInconsistentStateError( | 1163 | raise QueueAdminUnauthorizedError( |
290 | 1139 | "No rights to override to %s" % | 1164 | "No rights to override to %s" % |
291 | 1140 | ", ".join(disallowed_components)) | 1165 | ", ".join(disallowed_components)) |
292 | 1141 | 1166 | ||
293 | 1142 | made_changes = False | 1167 | made_changes = False |
294 | 1143 | for build in self.builds: | 1168 | for build in self.builds: |
295 | 1169 | # See if the new component requires a new archive on the build. | ||
296 | 1170 | for component in new_components: | ||
297 | 1171 | distroarchseries = build.build.distro_arch_series | ||
298 | 1172 | distribution = distroarchseries.distroseries.distribution | ||
299 | 1173 | new_archive = distribution.getArchiveByComponent( | ||
300 | 1174 | component.name) | ||
301 | 1175 | if new_archive != build.build.archive: | ||
302 | 1176 | raise QueueInconsistentStateError( | ||
303 | 1177 | "Overriding component to '%s' failed because it " | ||
304 | 1178 | "would require a new archive." % component.name) | ||
305 | 1179 | |||
306 | 1144 | for binarypackage in build.build.binarypackages: | 1180 | for binarypackage in build.build.binarypackages: |
307 | 1145 | change = changes_by_name.get( | 1181 | change = changes_by_name.get( |
308 | 1146 | binarypackage.name, changes_for_all) | 1182 | binarypackage.name, changes_for_all) |
309 | @@ -1148,7 +1184,7 @@ | |||
310 | 1148 | if binarypackage.component not in allowed_components: | 1184 | if binarypackage.component not in allowed_components: |
311 | 1149 | # The old component is not in the list of allowed | 1185 | # The old component is not in the list of allowed |
312 | 1150 | # components to override. | 1186 | # components to override. |
314 | 1151 | raise QueueInconsistentStateError( | 1187 | raise QueueAdminUnauthorizedError( |
315 | 1152 | "No rights to override from %s" % | 1188 | "No rights to override from %s" % |
316 | 1153 | binarypackage.component.name) | 1189 | binarypackage.component.name) |
317 | 1154 | binarypackage.override(**change) | 1190 | binarypackage.override(**change) |
318 | 1155 | 1191 | ||
319 | === modified file 'lib/lp/soyuz/tests/test_packageupload.py' | |||
320 | --- lib/lp/soyuz/tests/test_packageupload.py 2012-07-05 13:01:48 +0000 | |||
321 | +++ lib/lp/soyuz/tests/test_packageupload.py 2012-07-06 16:20:24 +0000 | |||
322 | @@ -42,6 +42,7 @@ | |||
323 | 42 | from lp.soyuz.interfaces.queue import ( | 42 | from lp.soyuz.interfaces.queue import ( |
324 | 43 | IPackageUpload, | 43 | IPackageUpload, |
325 | 44 | IPackageUploadSet, | 44 | IPackageUploadSet, |
326 | 45 | QueueAdminUnauthorizedError, | ||
327 | 45 | QueueInconsistentStateError, | 46 | QueueInconsistentStateError, |
328 | 46 | ) | 47 | ) |
329 | 47 | from lp.soyuz.interfaces.section import ISectionSet | 48 | from lp.soyuz.interfaces.section import ISectionSet |
330 | @@ -444,8 +445,7 @@ | |||
331 | 444 | only_allowed_component = self.factory.makeComponent() | 445 | only_allowed_component = self.factory.makeComponent() |
332 | 445 | section = self.factory.makeSection() | 446 | section = self.factory.makeSection() |
333 | 446 | self.assertRaises( | 447 | self.assertRaises( |
336 | 447 | QueueInconsistentStateError, | 448 | QueueAdminUnauthorizedError, pu.overrideSource, |
335 | 448 | pu.overrideSource, | ||
337 | 449 | only_allowed_component, section, [only_allowed_component]) | 449 | only_allowed_component, section, [only_allowed_component]) |
338 | 450 | 450 | ||
339 | 451 | def test_overrideSource_checks_permission_for_new_component(self): | 451 | def test_overrideSource_checks_permission_for_new_component(self): |
340 | @@ -454,8 +454,7 @@ | |||
341 | 454 | disallowed_component = self.factory.makeComponent() | 454 | disallowed_component = self.factory.makeComponent() |
342 | 455 | section = self.factory.makeSection() | 455 | section = self.factory.makeSection() |
343 | 456 | self.assertRaises( | 456 | self.assertRaises( |
346 | 457 | QueueInconsistentStateError, | 457 | QueueAdminUnauthorizedError, pu.overrideSource, |
345 | 458 | pu.overrideSource, | ||
347 | 459 | disallowed_component, section, [current_component]) | 458 | disallowed_component, section, [current_component]) |
348 | 460 | 459 | ||
349 | 461 | def test_overrideSource_ignores_None_component_change(self): | 460 | def test_overrideSource_ignores_None_component_change(self): |
350 | @@ -971,6 +970,9 @@ | |||
351 | 971 | def test_edit_permissions(self): | 970 | def test_edit_permissions(self): |
352 | 972 | self.assertRequiresEdit("acceptFromQueue") | 971 | self.assertRequiresEdit("acceptFromQueue") |
353 | 973 | self.assertRequiresEdit("rejectFromQueue") | 972 | self.assertRequiresEdit("rejectFromQueue") |
354 | 973 | self.assertRequiresEdit("overrideSource", new_component="main") | ||
355 | 974 | self.assertRequiresEdit( | ||
356 | 975 | "overrideBinaries", changes=[{"component": "main"}]) | ||
357 | 974 | 976 | ||
358 | 975 | def test_acceptFromQueue_archive_admin(self): | 977 | def test_acceptFromQueue_archive_admin(self): |
359 | 976 | # acceptFromQueue as an archive admin accepts the upload. | 978 | # acceptFromQueue as an archive admin accepts the upload. |
360 | @@ -1034,6 +1036,45 @@ | |||
361 | 1034 | for file in upload.sourcepackagerelease.files] | 1036 | for file in upload.sourcepackagerelease.files] |
362 | 1035 | self.assertContentEqual(source_file_urls, ws_source_file_urls) | 1037 | self.assertContentEqual(source_file_urls, ws_source_file_urls) |
363 | 1036 | 1038 | ||
364 | 1039 | def test_overrideSource_limited_component_permissions(self): | ||
365 | 1040 | # Overriding between two components requires queue admin of both. | ||
366 | 1041 | person = self.makeQueueAdmin([self.universe]) | ||
367 | 1042 | upload, ws_upload = self.makeSourcePackageUpload( | ||
368 | 1043 | person, component=self.universe) | ||
369 | 1044 | |||
370 | 1045 | self.assertEqual("New", ws_upload.status) | ||
371 | 1046 | self.assertEqual("universe", ws_upload.component_name) | ||
372 | 1047 | self.assertRaises(Unauthorized, ws_upload.overrideSource, | ||
373 | 1048 | new_component="main") | ||
374 | 1049 | |||
375 | 1050 | with admin_logged_in(): | ||
376 | 1051 | upload.overrideSource( | ||
377 | 1052 | new_component=self.main, | ||
378 | 1053 | allowed_components=[self.main, self.universe]) | ||
379 | 1054 | transaction.commit() | ||
380 | 1055 | self.assertEqual("main", upload.component_name) | ||
381 | 1056 | self.assertRaises(Unauthorized, ws_upload.overrideSource, | ||
382 | 1057 | new_component="universe") | ||
383 | 1058 | |||
384 | 1059 | def test_overrideSource_changes_properties(self): | ||
385 | 1060 | # Running overrideSource changes the corresponding properties. | ||
386 | 1061 | person = self.makeQueueAdmin([self.main, self.universe]) | ||
387 | 1062 | upload, ws_upload = self.makeSourcePackageUpload( | ||
388 | 1063 | person, component=self.universe) | ||
389 | 1064 | with person_logged_in(person): | ||
390 | 1065 | new_section = self.factory.makeSection() | ||
391 | 1066 | transaction.commit() | ||
392 | 1067 | |||
393 | 1068 | self.assertEqual("New", ws_upload.status) | ||
394 | 1069 | self.assertEqual("universe", ws_upload.component_name) | ||
395 | 1070 | self.assertNotEqual(new_section.name, ws_upload.section_name) | ||
396 | 1071 | ws_upload.overrideSource( | ||
397 | 1072 | new_component="main", new_section=new_section.name) | ||
398 | 1073 | self.assertEqual("main", ws_upload.component_name) | ||
399 | 1074 | self.assertEqual(new_section.name, ws_upload.section_name) | ||
400 | 1075 | ws_upload.overrideSource(new_component="universe") | ||
401 | 1076 | self.assertEqual("universe", ws_upload.component_name) | ||
402 | 1077 | |||
403 | 1037 | def assertBinaryPropertiesMatch(self, arch, bpr, ws_binary): | 1078 | def assertBinaryPropertiesMatch(self, arch, bpr, ws_binary): |
404 | 1038 | expected_binary = { | 1079 | expected_binary = { |
405 | 1039 | "is_new": True, | 1080 | "is_new": True, |
406 | @@ -1078,6 +1119,112 @@ | |||
407 | 1078 | for file in bpr.files] | 1119 | for file in bpr.files] |
408 | 1079 | self.assertContentEqual(binary_file_urls, ws_binary_file_urls) | 1120 | self.assertContentEqual(binary_file_urls, ws_binary_file_urls) |
409 | 1080 | 1121 | ||
410 | 1122 | def test_overrideBinaries_limited_component_permissions(self): | ||
411 | 1123 | # Overriding between two components requires queue admin of both. | ||
412 | 1124 | person = self.makeQueueAdmin([self.universe]) | ||
413 | 1125 | upload, ws_upload = self.makeBinaryPackageUpload( | ||
414 | 1126 | person, binarypackagename="hello", component=self.universe) | ||
415 | 1127 | |||
416 | 1128 | self.assertEqual("New", ws_upload.status) | ||
417 | 1129 | self.assertEqual( | ||
418 | 1130 | set(["universe"]), | ||
419 | 1131 | set(binary["component"] | ||
420 | 1132 | for binary in ws_upload.getBinaryProperties())) | ||
421 | 1133 | self.assertRaises( | ||
422 | 1134 | Unauthorized, ws_upload.overrideBinaries, | ||
423 | 1135 | changes=[{"component": "main"}]) | ||
424 | 1136 | |||
425 | 1137 | with admin_logged_in(): | ||
426 | 1138 | upload.overrideBinaries( | ||
427 | 1139 | [{"component": self.main}], | ||
428 | 1140 | allowed_components=[self.main, self.universe]) | ||
429 | 1141 | transaction.commit() | ||
430 | 1142 | |||
431 | 1143 | self.assertEqual( | ||
432 | 1144 | set(["main"]), | ||
433 | 1145 | set(binary["component"] | ||
434 | 1146 | for binary in ws_upload.getBinaryProperties())) | ||
435 | 1147 | self.assertRaises( | ||
436 | 1148 | Unauthorized, ws_upload.overrideBinaries, | ||
437 | 1149 | changes=[{"component": "universe"}]) | ||
438 | 1150 | |||
439 | 1151 | def test_overrideBinaries_disallows_new_archive(self): | ||
440 | 1152 | # overrideBinaries refuses to override the component to something | ||
441 | 1153 | # that requires a different archive. | ||
442 | 1154 | partner = self.factory.makeComponent("partner") | ||
443 | 1155 | self.factory.makeComponentSelection( | ||
444 | 1156 | distroseries=self.distroseries, component=partner) | ||
445 | 1157 | person = self.makeQueueAdmin([self.universe, partner]) | ||
446 | 1158 | upload, ws_upload = self.makeBinaryPackageUpload( | ||
447 | 1159 | person, component=self.universe) | ||
448 | 1160 | |||
449 | 1161 | self.assertEqual( | ||
450 | 1162 | "universe", ws_upload.getBinaryProperties()[0]["component"]) | ||
451 | 1163 | self.assertRaises( | ||
452 | 1164 | BadRequest, ws_upload.overrideBinaries, | ||
453 | 1165 | changes=[{"component": "partner"}]) | ||
454 | 1166 | |||
455 | 1167 | def test_overrideBinaries_without_name_changes_all_properties(self): | ||
456 | 1168 | # Running overrideBinaries with a change entry containing no "name" | ||
457 | 1169 | # field changes the corresponding properties of all binaries. | ||
458 | 1170 | person = self.makeQueueAdmin([self.main, self.universe]) | ||
459 | 1171 | upload, ws_upload = self.makeBinaryPackageUpload( | ||
460 | 1172 | person, component=self.universe) | ||
461 | 1173 | with person_logged_in(person): | ||
462 | 1174 | new_section = self.factory.makeSection() | ||
463 | 1175 | transaction.commit() | ||
464 | 1176 | |||
465 | 1177 | self.assertEqual("New", ws_upload.status) | ||
466 | 1178 | for binary in ws_upload.getBinaryProperties(): | ||
467 | 1179 | self.assertEqual("universe", binary["component"]) | ||
468 | 1180 | self.assertNotEqual(new_section.name, binary["section"]) | ||
469 | 1181 | self.assertEqual("OPTIONAL", binary["priority"]) | ||
470 | 1182 | changes = [{ | ||
471 | 1183 | "component": "main", | ||
472 | 1184 | "section": new_section.name, | ||
473 | 1185 | "priority": "extra", | ||
474 | 1186 | }] | ||
475 | 1187 | ws_upload.overrideBinaries(changes=changes) | ||
476 | 1188 | for binary in ws_upload.getBinaryProperties(): | ||
477 | 1189 | self.assertEqual("main", binary["component"]) | ||
478 | 1190 | self.assertEqual(new_section.name, binary["section"]) | ||
479 | 1191 | self.assertEqual("EXTRA", binary["priority"]) | ||
480 | 1192 | |||
481 | 1193 | def test_overrideBinaries_with_name_changes_selected_properties(self): | ||
482 | 1194 | # Running overrideBinaries with change entries containing "name" | ||
483 | 1195 | # fields changes the corresponding properties of only the selected | ||
484 | 1196 | # binaries. | ||
485 | 1197 | person = self.makeQueueAdmin([self.main, self.universe]) | ||
486 | 1198 | upload, ws_upload = self.makeBinaryPackageUpload( | ||
487 | 1199 | person, component=self.universe) | ||
488 | 1200 | with person_logged_in(person): | ||
489 | 1201 | new_section = self.factory.makeSection() | ||
490 | 1202 | transaction.commit() | ||
491 | 1203 | |||
492 | 1204 | self.assertEqual("New", ws_upload.status) | ||
493 | 1205 | ws_binaries = ws_upload.getBinaryProperties() | ||
494 | 1206 | for binary in ws_binaries: | ||
495 | 1207 | self.assertEqual("universe", binary["component"]) | ||
496 | 1208 | self.assertNotEqual(new_section.name, binary["section"]) | ||
497 | 1209 | self.assertEqual("OPTIONAL", binary["priority"]) | ||
498 | 1210 | change_one = { | ||
499 | 1211 | "name": ws_binaries[0]["name"], | ||
500 | 1212 | "component": "main", | ||
501 | 1213 | "priority": "standard", | ||
502 | 1214 | } | ||
503 | 1215 | change_two = { | ||
504 | 1216 | "name": ws_binaries[1]["name"], | ||
505 | 1217 | "section": new_section.name, | ||
506 | 1218 | } | ||
507 | 1219 | ws_upload.overrideBinaries(changes=[change_one, change_two]) | ||
508 | 1220 | ws_binaries = ws_upload.getBinaryProperties() | ||
509 | 1221 | self.assertEqual("main", ws_binaries[0]["component"]) | ||
510 | 1222 | self.assertNotEqual(new_section.name, ws_binaries[0]["section"]) | ||
511 | 1223 | self.assertEqual("STANDARD", ws_binaries[0]["priority"]) | ||
512 | 1224 | self.assertEqual("universe", ws_binaries[1]["component"]) | ||
513 | 1225 | self.assertEqual(new_section.name, ws_binaries[1]["section"]) | ||
514 | 1226 | self.assertEqual("OPTIONAL", ws_binaries[1]["priority"]) | ||
515 | 1227 | |||
516 | 1081 | def test_custom_info(self): | 1228 | def test_custom_info(self): |
517 | 1082 | # API clients can inspect properties of custom uploads. | 1229 | # API clients can inspect properties of custom uploads. |
518 | 1083 | person = self.makeQueueAdmin([self.universe]) | 1230 | person = self.makeQueueAdmin([self.universe]) |
This looks good, thanks for breaking it out into smaller branches.