Merge lp:~cjwatson/launchpad/git-finish-sharing into lp:launchpad
- git-finish-sharing
- Merge into devel
Proposed by
Colin Watson
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | no longer in the source branch. |
Merged at revision: | 17366 |
Proposed branch: | lp:~cjwatson/launchpad/git-finish-sharing |
Merge into: | lp:launchpad |
Prerequisite: | lp:~cjwatson/launchpad/git-collection |
Diff against target: |
1160 lines (+429/-105) 8 files modified
lib/lp/code/model/gitrepository.py (+9/-4) lib/lp/code/model/hasgitrepositories.py (+3/-3) lib/lp/code/model/tests/test_gitrepository.py (+136/-1) lib/lp/registry/services/sharingservice.py (+44/-37) lib/lp/registry/services/tests/test_sharingservice.py (+163/-45) lib/lp/registry/tests/test_accesspolicy.py (+8/-1) lib/lp/registry/tests/test_sharingjob.py (+58/-13) lib/lp/security.py (+8/-1) |
To merge this branch: | bzr merge lp:~cjwatson/launchpad/git-finish-sharing |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+250663@code.launchpad.net |
Commit message
Use Git repository collections to implement visibility and sharing.
Description of the change
Use Git repository collections to implement visibility and sharing.
There are a few bits we can't do until there's some analogue of BranchSubscription, but otherwise this should be a reasonably complete privacy implementation now.
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) : | # |
review:
Approve
(code)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/code/model/gitrepository.py' | |||
2 | --- lib/lp/code/model/gitrepository.py 2015-02-26 14:25:26 +0000 | |||
3 | +++ lib/lp/code/model/gitrepository.py 2015-02-27 11:27:39 +0000 | |||
4 | @@ -26,6 +26,7 @@ | |||
5 | 26 | ) | 26 | ) |
6 | 27 | from zope.component import getUtility | 27 | from zope.component import getUtility |
7 | 28 | from zope.interface import implements | 28 | from zope.interface import implements |
8 | 29 | from zope.security.proxy import removeSecurityProxy | ||
9 | 29 | 30 | ||
10 | 30 | from lp.app.enums import ( | 31 | from lp.app.enums import ( |
11 | 31 | InformationType, | 32 | InformationType, |
12 | @@ -39,6 +40,7 @@ | |||
13 | 39 | GitDefaultConflict, | 40 | GitDefaultConflict, |
14 | 40 | GitTargetError, | 41 | GitTargetError, |
15 | 41 | ) | 42 | ) |
16 | 43 | from lp.code.interfaces.gitcollection import IAllGitRepositories | ||
17 | 42 | from lp.code.interfaces.gitlookup import IGitLookup | 44 | from lp.code.interfaces.gitlookup import IGitLookup |
18 | 43 | from lp.code.interfaces.gitnamespace import ( | 45 | from lp.code.interfaces.gitnamespace import ( |
19 | 44 | get_git_namespace, | 46 | get_git_namespace, |
20 | @@ -291,9 +293,8 @@ | |||
21 | 291 | elif user.id in self._known_viewers: | 293 | elif user.id in self._known_viewers: |
22 | 292 | return True | 294 | return True |
23 | 293 | else: | 295 | else: |
27 | 294 | # XXX cjwatson 2015-02-06: Fill this in once IGitCollection is | 296 | return not getUtility(IAllGitRepositories).withIds( |
28 | 295 | # in place. | 297 | self.id).visibleByUser(user).is_empty() |
26 | 296 | return False | ||
29 | 297 | 298 | ||
30 | 298 | def getAllowedInformationTypes(self, user): | 299 | def getAllowedInformationTypes(self, user): |
31 | 299 | """See `IGitRepository`.""" | 300 | """See `IGitRepository`.""" |
32 | @@ -359,7 +360,11 @@ | |||
33 | 359 | def getByPath(self, user, path): | 360 | def getByPath(self, user, path): |
34 | 360 | """See `IGitRepositorySet`.""" | 361 | """See `IGitRepositorySet`.""" |
35 | 361 | repository = getUtility(IGitLookup).getByPath(path) | 362 | repository = getUtility(IGitLookup).getByPath(path) |
37 | 362 | if repository is not None and repository.visibleByUser(user): | 363 | if repository is None: |
38 | 364 | return None | ||
39 | 365 | # removeSecurityProxy is safe here since we're explicitly performing | ||
40 | 366 | # a permission check. | ||
41 | 367 | if removeSecurityProxy(repository).visibleByUser(user): | ||
42 | 363 | return repository | 368 | return repository |
43 | 364 | return None | 369 | return None |
44 | 365 | 370 | ||
45 | 366 | 371 | ||
46 | === modified file 'lib/lp/code/model/hasgitrepositories.py' | |||
47 | --- lib/lp/code/model/hasgitrepositories.py 2015-02-10 01:03:48 +0000 | |||
48 | +++ lib/lp/code/model/hasgitrepositories.py 2015-02-27 11:27:39 +0000 | |||
49 | @@ -8,6 +8,7 @@ | |||
50 | 8 | 8 | ||
51 | 9 | from zope.component import getUtility | 9 | from zope.component import getUtility |
52 | 10 | 10 | ||
53 | 11 | from lp.code.interfaces.gitcollection import IGitCollection | ||
54 | 11 | from lp.code.interfaces.gitrepository import IGitRepositorySet | 12 | from lp.code.interfaces.gitrepository import IGitRepositorySet |
55 | 12 | 13 | ||
56 | 13 | 14 | ||
57 | @@ -23,6 +24,5 @@ | |||
58 | 23 | 24 | ||
59 | 24 | def getGitRepositories(self, visible_by_user=None, eager_load=False): | 25 | def getGitRepositories(self, visible_by_user=None, eager_load=False): |
60 | 25 | """See `IHasGitRepositories`.""" | 26 | """See `IHasGitRepositories`.""" |
64 | 26 | # XXX cjwatson 2015-02-06: Fill this in once IGitCollection is in | 27 | collection = IGitCollection(self).visibleByUser(visible_by_user) |
65 | 27 | # place. | 28 | return collection.getRepositories(eager_load=eager_load) |
63 | 28 | raise NotImplementedError | ||
66 | 29 | 29 | ||
67 | === modified file 'lib/lp/code/model/tests/test_gitrepository.py' | |||
68 | --- lib/lp/code/model/tests/test_gitrepository.py 2015-02-26 11:43:12 +0000 | |||
69 | +++ lib/lp/code/model/tests/test_gitrepository.py 2015-02-27 11:27:39 +0000 | |||
70 | @@ -34,11 +34,21 @@ | |||
71 | 34 | IGitRepository, | 34 | IGitRepository, |
72 | 35 | IGitRepositorySet, | 35 | IGitRepositorySet, |
73 | 36 | ) | 36 | ) |
75 | 37 | from lp.registry.enums import BranchSharingPolicy | 37 | from lp.registry.enums import ( |
76 | 38 | BranchSharingPolicy, | ||
77 | 39 | PersonVisibility, | ||
78 | 40 | TeamMembershipPolicy, | ||
79 | 41 | ) | ||
80 | 42 | from lp.registry.interfaces.accesspolicy import ( | ||
81 | 43 | IAccessArtifactSource, | ||
82 | 44 | IAccessPolicyArtifactSource, | ||
83 | 45 | IAccessPolicySource, | ||
84 | 46 | ) | ||
85 | 38 | from lp.registry.interfaces.persondistributionsourcepackage import ( | 47 | from lp.registry.interfaces.persondistributionsourcepackage import ( |
86 | 39 | IPersonDistributionSourcePackageFactory, | 48 | IPersonDistributionSourcePackageFactory, |
87 | 40 | ) | 49 | ) |
88 | 41 | from lp.registry.interfaces.personproduct import IPersonProductFactory | 50 | from lp.registry.interfaces.personproduct import IPersonProductFactory |
89 | 51 | from lp.registry.tests.test_accesspolicy import get_policies_for_artifact | ||
90 | 42 | from lp.services.database.constants import UTC_NOW | 52 | from lp.services.database.constants import UTC_NOW |
91 | 43 | from lp.services.webapp.authorization import check_permission | 53 | from lp.services.webapp.authorization import check_permission |
92 | 44 | from lp.testing import ( | 54 | from lp.testing import ( |
93 | @@ -134,6 +144,15 @@ | |||
94 | 134 | self.repository_set.setDefaultRepository(project, repository) | 144 | self.repository_set.setDefaultRepository(project, repository) |
95 | 135 | self.assertGitIdentity(repository, project.name) | 145 | self.assertGitIdentity(repository, project.name) |
96 | 136 | 146 | ||
97 | 147 | def test_git_identity_private_default_for_project(self): | ||
98 | 148 | # Private repositories also have a short lp: URL. | ||
99 | 149 | project = self.factory.makeProduct() | ||
100 | 150 | repository = self.factory.makeGitRepository( | ||
101 | 151 | target=project, information_type=InformationType.USERDATA) | ||
102 | 152 | with admin_logged_in(): | ||
103 | 153 | self.repository_set.setDefaultRepository(project, repository) | ||
104 | 154 | self.assertGitIdentity(repository, project.name) | ||
105 | 155 | |||
106 | 137 | def test_git_identity_default_for_package(self): | 156 | def test_git_identity_default_for_package(self): |
107 | 138 | # If a repository is the default for a package, then its Git | 157 | # If a repository is the default for a package, then its Git |
108 | 139 | # identity uses the path to that package. | 158 | # identity uses the path to that package. |
109 | @@ -302,6 +321,57 @@ | |||
110 | 302 | self.assertEqual(namespace, repository.namespace) | 321 | self.assertEqual(namespace, repository.namespace) |
111 | 303 | 322 | ||
112 | 304 | 323 | ||
113 | 324 | class TestGitRepositoryPrivacy(TestCaseWithFactory): | ||
114 | 325 | """Tests for Git repository privacy.""" | ||
115 | 326 | |||
116 | 327 | layer = DatabaseFunctionalLayer | ||
117 | 328 | |||
118 | 329 | def setUp(self): | ||
119 | 330 | # Use an admin user as we aren't checking edit permissions here. | ||
120 | 331 | super(TestGitRepositoryPrivacy, self).setUp("admin@canonical.com") | ||
121 | 332 | |||
122 | 333 | def test_personal_repositories_for_private_teams_are_private(self): | ||
123 | 334 | team = self.factory.makeTeam( | ||
124 | 335 | membership_policy=TeamMembershipPolicy.MODERATED, | ||
125 | 336 | visibility=PersonVisibility.PRIVATE) | ||
126 | 337 | repository = self.factory.makeGitRepository(owner=team, target=team) | ||
127 | 338 | self.assertTrue(repository.private) | ||
128 | 339 | self.assertEqual( | ||
129 | 340 | InformationType.PROPRIETARY, repository.information_type) | ||
130 | 341 | |||
131 | 342 | def test__reconcileAccess_for_project_repository(self): | ||
132 | 343 | # _reconcileAccess uses a project policy for a project repository. | ||
133 | 344 | repository = self.factory.makeGitRepository( | ||
134 | 345 | information_type=InformationType.USERDATA) | ||
135 | 346 | [artifact] = getUtility(IAccessArtifactSource).ensure([repository]) | ||
136 | 347 | getUtility(IAccessPolicyArtifactSource).deleteByArtifact([artifact]) | ||
137 | 348 | removeSecurityProxy(repository)._reconcileAccess() | ||
138 | 349 | self.assertContentEqual( | ||
139 | 350 | getUtility(IAccessPolicySource).find( | ||
140 | 351 | [(repository.target, InformationType.USERDATA)]), | ||
141 | 352 | get_policies_for_artifact(repository)) | ||
142 | 353 | |||
143 | 354 | def test__reconcileAccess_for_package_repository(self): | ||
144 | 355 | # Git repository privacy isn't yet supported for distributions, so | ||
145 | 356 | # no AccessPolicyArtifact is created for a package repository. | ||
146 | 357 | repository = self.factory.makeGitRepository( | ||
147 | 358 | target=self.factory.makeDistributionSourcePackage(), | ||
148 | 359 | information_type=InformationType.USERDATA) | ||
149 | 360 | removeSecurityProxy(repository)._reconcileAccess() | ||
150 | 361 | self.assertEqual([], get_policies_for_artifact(repository)) | ||
151 | 362 | |||
152 | 363 | def test__reconcileAccess_for_personal_repository(self): | ||
153 | 364 | # _reconcileAccess uses a person policy for a personal repository. | ||
154 | 365 | team_owner = self.factory.makeTeam() | ||
155 | 366 | repository = self.factory.makeGitRepository( | ||
156 | 367 | owner=team_owner, target=team_owner, | ||
157 | 368 | information_type=InformationType.USERDATA) | ||
158 | 369 | removeSecurityProxy(repository)._reconcileAccess() | ||
159 | 370 | self.assertContentEqual( | ||
160 | 371 | getUtility(IAccessPolicySource).findByTeam([team_owner]), | ||
161 | 372 | get_policies_for_artifact(repository)) | ||
162 | 373 | |||
163 | 374 | |||
164 | 305 | class TestGitRepositoryGetAllowedInformationTypes(TestCaseWithFactory): | 375 | class TestGitRepositoryGetAllowedInformationTypes(TestCaseWithFactory): |
165 | 306 | """Test `IGitRepository.getAllowedInformationTypes`.""" | 376 | """Test `IGitRepository.getAllowedInformationTypes`.""" |
166 | 307 | 377 | ||
167 | @@ -354,6 +424,17 @@ | |||
168 | 354 | self.assertFalse( | 424 | self.assertFalse( |
169 | 355 | check_permission("launchpad.Moderate", repository)) | 425 | check_permission("launchpad.Moderate", repository)) |
170 | 356 | 426 | ||
171 | 427 | def test_methods_smoketest(self): | ||
172 | 428 | # Users with launchpad.Moderate can call transitionToInformationType. | ||
173 | 429 | project = self.factory.makeProduct() | ||
174 | 430 | repository = self.factory.makeGitRepository(target=project) | ||
175 | 431 | with person_logged_in(project.owner): | ||
176 | 432 | project.setBranchSharingPolicy(BranchSharingPolicy.PUBLIC) | ||
177 | 433 | repository.transitionToInformationType( | ||
178 | 434 | InformationType.PRIVATESECURITY, project.owner) | ||
179 | 435 | self.assertEqual( | ||
180 | 436 | InformationType.PRIVATESECURITY, repository.information_type) | ||
181 | 437 | |||
182 | 357 | def test_attribute_smoketest(self): | 438 | def test_attribute_smoketest(self): |
183 | 358 | # Users with launchpad.Moderate can set attributes. | 439 | # Users with launchpad.Moderate can set attributes. |
184 | 359 | project = self.factory.makeProduct() | 440 | project = self.factory.makeProduct() |
185 | @@ -486,6 +567,47 @@ | |||
186 | 486 | repository.setTarget(target=owner, user=owner) | 567 | repository.setTarget(target=owner, user=owner) |
187 | 487 | self.assertEqual(owner, repository.target) | 568 | self.assertEqual(owner, repository.target) |
188 | 488 | 569 | ||
189 | 570 | def test_private_personal_forbidden_for_public_teams(self): | ||
190 | 571 | # Only private teams can have private personal repositories. | ||
191 | 572 | owner = self.factory.makeTeam() | ||
192 | 573 | repository = self.factory.makeGitRepository( | ||
193 | 574 | owner=owner, information_type=InformationType.USERDATA) | ||
194 | 575 | with admin_logged_in(): | ||
195 | 576 | self.assertRaises( | ||
196 | 577 | GitTargetError, repository.setTarget, target=owner, user=owner) | ||
197 | 578 | |||
198 | 579 | def test_private_personal_allowed_for_private_teams(self): | ||
199 | 580 | # Only private teams can have private personal repositories. | ||
200 | 581 | owner = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE) | ||
201 | 582 | with person_logged_in(owner): | ||
202 | 583 | repository = self.factory.makeGitRepository( | ||
203 | 584 | owner=owner, information_type=InformationType.USERDATA) | ||
204 | 585 | repository.setTarget(target=owner, user=owner) | ||
205 | 586 | self.assertEqual(owner, repository.target) | ||
206 | 587 | |||
207 | 588 | def test_reconciles_access(self): | ||
208 | 589 | # setTarget calls _reconcileAccess to make the sharing schema | ||
209 | 590 | # match the new target. | ||
210 | 591 | repository = self.factory.makeGitRepository( | ||
211 | 592 | information_type=InformationType.USERDATA) | ||
212 | 593 | new_project = self.factory.makeProduct() | ||
213 | 594 | with admin_logged_in(): | ||
214 | 595 | repository.setTarget(target=new_project, user=repository.owner) | ||
215 | 596 | self.assertEqual( | ||
216 | 597 | new_project, get_policies_for_artifact(repository)[0].pillar) | ||
217 | 598 | |||
218 | 599 | def test_reconciles_access_personal(self): | ||
219 | 600 | # setTarget calls _reconcileAccess to make the sharing schema | ||
220 | 601 | # correct for a private personal repository. | ||
221 | 602 | owner = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE) | ||
222 | 603 | with person_logged_in(owner): | ||
223 | 604 | repository = self.factory.makeGitRepository( | ||
224 | 605 | owner=owner, target=owner, | ||
225 | 606 | information_type=InformationType.USERDATA) | ||
226 | 607 | repository.setTarget(target=owner, user=owner) | ||
227 | 608 | self.assertEqual( | ||
228 | 609 | owner, get_policies_for_artifact(repository)[0].person) | ||
229 | 610 | |||
230 | 489 | def test_public_to_proprietary_only_project(self): | 611 | def test_public_to_proprietary_only_project(self): |
231 | 490 | # A repository cannot be moved to a target where the sharing policy | 612 | # A repository cannot be moved to a target where the sharing policy |
232 | 491 | # does not allow it. | 613 | # does not allow it. |
233 | @@ -525,6 +647,19 @@ | |||
234 | 525 | person = self.factory.makePerson() | 647 | person = self.factory.makePerson() |
235 | 526 | self.assertIsNone(self.repository_set.getByPath(person, "nonexistent")) | 648 | self.assertIsNone(self.repository_set.getByPath(person, "nonexistent")) |
236 | 527 | 649 | ||
237 | 650 | def test_getByPath_inaccessible(self): | ||
238 | 651 | # If the given user cannot view the matched repository, then | ||
239 | 652 | # getByPath returns None. | ||
240 | 653 | owner = self.factory.makePerson() | ||
241 | 654 | repository = self.factory.makeGitRepository( | ||
242 | 655 | owner=owner, information_type=InformationType.USERDATA) | ||
243 | 656 | with person_logged_in(owner): | ||
244 | 657 | path = repository.shortened_path | ||
245 | 658 | self.assertEqual( | ||
246 | 659 | repository, self.repository_set.getByPath(owner, path)) | ||
247 | 660 | self.assertIsNone( | ||
248 | 661 | self.repository_set.getByPath(self.factory.makePerson(), path)) | ||
249 | 662 | |||
250 | 528 | def test_setDefaultRepository_refuses_person(self): | 663 | def test_setDefaultRepository_refuses_person(self): |
251 | 529 | # setDefaultRepository refuses if the target is a person. | 664 | # setDefaultRepository refuses if the target is a person. |
252 | 530 | person = self.factory.makePerson() | 665 | person = self.factory.makePerson() |
253 | 531 | 666 | ||
254 | === modified file 'lib/lp/registry/services/sharingservice.py' | |||
255 | --- lib/lp/registry/services/sharingservice.py 2015-02-16 13:01:34 +0000 | |||
256 | +++ lib/lp/registry/services/sharingservice.py 2015-02-27 11:27:39 +0000 | |||
257 | @@ -9,6 +9,7 @@ | |||
258 | 9 | ] | 9 | ] |
259 | 10 | 10 | ||
260 | 11 | from itertools import product | 11 | from itertools import product |
261 | 12 | from operator import attrgetter | ||
262 | 12 | 13 | ||
263 | 13 | from lazr.restful.interfaces import IWebBrowserOriginatingRequest | 14 | from lazr.restful.interfaces import IWebBrowserOriginatingRequest |
264 | 14 | from lazr.restful.utils import get_current_web_service_request | 15 | from lazr.restful.utils import get_current_web_service_request |
265 | @@ -36,6 +37,7 @@ | |||
266 | 36 | from lp.bugs.interfaces.bugtask import IBugTaskSet | 37 | from lp.bugs.interfaces.bugtask import IBugTaskSet |
267 | 37 | from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams | 38 | from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams |
268 | 38 | from lp.code.interfaces.branchcollection import IAllBranches | 39 | from lp.code.interfaces.branchcollection import IAllBranches |
269 | 40 | from lp.code.interfaces.gitcollection import IAllGitRepositories | ||
270 | 39 | from lp.registry.enums import ( | 41 | from lp.registry.enums import ( |
271 | 40 | BranchSharingPolicy, | 42 | BranchSharingPolicy, |
272 | 41 | BugSharingPolicy, | 43 | BugSharingPolicy, |
273 | @@ -227,7 +229,11 @@ | |||
274 | 227 | branches = list(wanted_branches.getBranches()) | 229 | branches = list(wanted_branches.getBranches()) |
275 | 228 | # Load the Git repositories. | 230 | # Load the Git repositories. |
276 | 229 | gitrepositories = [] | 231 | gitrepositories = [] |
278 | 230 | # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place. | 232 | if gitrepository_ids: |
279 | 233 | all_gitrepositories = getUtility(IAllGitRepositories) | ||
280 | 234 | wanted_gitrepositories = all_gitrepositories.visibleByUser( | ||
281 | 235 | user).withIds(*gitrepository_ids) | ||
282 | 236 | gitrepositories = list(wanted_gitrepositories.getRepositories()) | ||
283 | 231 | specifications = [] | 237 | specifications = [] |
284 | 232 | if specification_ids: | 238 | if specification_ids: |
285 | 233 | specifications = load(Specification, specification_ids) | 239 | specifications = load(Specification, specification_ids) |
286 | @@ -319,48 +325,52 @@ | |||
287 | 319 | gitrepositories=None, specifications=None, | 325 | gitrepositories=None, specifications=None, |
288 | 320 | ignore_permissions=False): | 326 | ignore_permissions=False): |
289 | 321 | """See `ISharingService`.""" | 327 | """See `ISharingService`.""" |
293 | 322 | bugs_by_id = {} | 328 | bug_ids = [] |
294 | 323 | branches_by_id = {} | 329 | branch_ids = [] |
295 | 324 | gitrepositories_by_id = {} | 330 | gitrepository_ids = [] |
296 | 325 | for bug in bugs or []: | 331 | for bug in bugs or []: |
297 | 326 | if (not ignore_permissions | 332 | if (not ignore_permissions |
298 | 327 | and not check_permission('launchpad.View', bug)): | 333 | and not check_permission('launchpad.View', bug)): |
299 | 328 | raise Unauthorized | 334 | raise Unauthorized |
301 | 329 | bugs_by_id[bug.id] = bug | 335 | bug_ids.append(bug.id) |
302 | 330 | for branch in branches or []: | 336 | for branch in branches or []: |
303 | 331 | if (not ignore_permissions | 337 | if (not ignore_permissions |
304 | 332 | and not check_permission('launchpad.View', branch)): | 338 | and not check_permission('launchpad.View', branch)): |
305 | 333 | raise Unauthorized | 339 | raise Unauthorized |
307 | 334 | branches_by_id[branch.id] = branch | 340 | branch_ids.append(branch.id) |
308 | 335 | for gitrepository in gitrepositories or []: | 341 | for gitrepository in gitrepositories or []: |
309 | 336 | if (not ignore_permissions | 342 | if (not ignore_permissions |
310 | 337 | and not check_permission('launchpad.View', gitrepository)): | 343 | and not check_permission('launchpad.View', gitrepository)): |
311 | 338 | raise Unauthorized | 344 | raise Unauthorized |
313 | 339 | gitrepositories_by_id[gitrepository.id] = gitrepository | 345 | gitrepository_ids.append(gitrepository.id) |
314 | 340 | for spec in specifications or []: | 346 | for spec in specifications or []: |
315 | 341 | if (not ignore_permissions | 347 | if (not ignore_permissions |
316 | 342 | and not check_permission('launchpad.View', spec)): | 348 | and not check_permission('launchpad.View', spec)): |
317 | 343 | raise Unauthorized | 349 | raise Unauthorized |
318 | 344 | 350 | ||
319 | 345 | # Load the bugs. | 351 | # Load the bugs. |
324 | 346 | visible_bug_ids = [] | 352 | visible_bugs = [] |
325 | 347 | if bugs_by_id: | 353 | if bug_ids: |
326 | 348 | param = BugTaskSearchParams( | 354 | param = BugTaskSearchParams(user=person, bug=any(*bug_ids)) |
323 | 349 | user=person, bug=any(*bugs_by_id.keys())) | ||
327 | 350 | visible_bug_ids = set(getUtility(IBugTaskSet).searchBugIds(param)) | 355 | visible_bug_ids = set(getUtility(IBugTaskSet).searchBugIds(param)) |
329 | 351 | visible_bugs = [bugs_by_id[bug_id] for bug_id in visible_bug_ids] | 356 | visible_bugs = [bug for bug in bugs if bug.id in visible_bug_ids] |
330 | 352 | 357 | ||
331 | 353 | # Load the branches. | 358 | # Load the branches. |
332 | 354 | visible_branches = [] | 359 | visible_branches = [] |
334 | 355 | if branches_by_id: | 360 | if branch_ids: |
335 | 356 | all_branches = getUtility(IAllBranches) | 361 | all_branches = getUtility(IAllBranches) |
336 | 357 | wanted_branches = all_branches.visibleByUser(person).withIds( | 362 | wanted_branches = all_branches.visibleByUser(person).withIds( |
338 | 358 | *branches_by_id.keys()) | 363 | *branch_ids) |
339 | 359 | visible_branches = list(wanted_branches.getBranches()) | 364 | visible_branches = list(wanted_branches.getBranches()) |
340 | 360 | 365 | ||
341 | 361 | # Load the Git repositories. | 366 | # Load the Git repositories. |
342 | 362 | visible_gitrepositories = [] | 367 | visible_gitrepositories = [] |
344 | 363 | # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place. | 368 | if gitrepository_ids: |
345 | 369 | all_gitrepositories = getUtility(IAllGitRepositories) | ||
346 | 370 | wanted_gitrepositories = all_gitrepositories.visibleByUser( | ||
347 | 371 | person).withIds(*gitrepository_ids) | ||
348 | 372 | visible_gitrepositories = list( | ||
349 | 373 | wanted_gitrepositories.getRepositories()) | ||
350 | 364 | 374 | ||
351 | 365 | # Load the specifications. | 375 | # Load the specifications. |
352 | 366 | visible_specs = [] | 376 | visible_specs = [] |
353 | @@ -378,40 +388,37 @@ | |||
354 | 378 | def getInvisibleArtifacts(self, person, bugs=None, branches=None, | 388 | def getInvisibleArtifacts(self, person, bugs=None, branches=None, |
355 | 379 | gitrepositories=None): | 389 | gitrepositories=None): |
356 | 380 | """See `ISharingService`.""" | 390 | """See `ISharingService`.""" |
366 | 381 | bugs_by_id = {} | 391 | bug_ids = list(map(attrgetter("id"), bugs or [])) |
367 | 382 | branches_by_id = {} | 392 | branch_ids = list(map(attrgetter("id"), branches or [])) |
368 | 383 | gitrepositories_by_id = {} | 393 | gitrepository_ids = list(map(attrgetter("id"), gitrepositories or [])) |
360 | 384 | for bug in bugs or []: | ||
361 | 385 | bugs_by_id[bug.id] = bug | ||
362 | 386 | for branch in branches or []: | ||
363 | 387 | branches_by_id[branch.id] = branch | ||
364 | 388 | for gitrepository in gitrepositories or []: | ||
365 | 389 | gitrepositories_by_id[gitrepository.id] = gitrepository | ||
369 | 390 | 394 | ||
370 | 391 | # Load the bugs. | 395 | # Load the bugs. |
375 | 392 | visible_bug_ids = set() | 396 | invisible_bugs = [] |
376 | 393 | if bugs_by_id: | 397 | if bug_ids: |
377 | 394 | param = BugTaskSearchParams( | 398 | param = BugTaskSearchParams(user=person, bug=any(*bug_ids)) |
374 | 395 | user=person, bug=any(*bugs_by_id.keys())) | ||
378 | 396 | visible_bug_ids = set(getUtility(IBugTaskSet).searchBugIds(param)) | 399 | visible_bug_ids = set(getUtility(IBugTaskSet).searchBugIds(param)) |
381 | 397 | invisible_bug_ids = set(bugs_by_id.keys()).difference(visible_bug_ids) | 400 | invisible_bugs = [ |
382 | 398 | invisible_bugs = [bugs_by_id[bug_id] for bug_id in invisible_bug_ids] | 401 | bug for bug in bugs if bug.id not in visible_bug_ids] |
383 | 399 | 402 | ||
384 | 400 | # Load the branches. | 403 | # Load the branches. |
385 | 401 | invisible_branches = [] | 404 | invisible_branches = [] |
387 | 402 | if branches_by_id: | 405 | if branch_ids: |
388 | 403 | all_branches = getUtility(IAllBranches) | 406 | all_branches = getUtility(IAllBranches) |
389 | 404 | visible_branch_ids = all_branches.visibleByUser(person).withIds( | 407 | visible_branch_ids = all_branches.visibleByUser(person).withIds( |
393 | 405 | *branches_by_id.keys()).getBranchIds() | 408 | *branch_ids).getBranchIds() |
391 | 406 | invisible_branch_ids = ( | ||
392 | 407 | set(branches_by_id.keys()).difference(visible_branch_ids)) | ||
394 | 408 | invisible_branches = [ | 409 | invisible_branches = [ |
397 | 409 | branches_by_id[branch_id] | 410 | branch for branch in branches |
398 | 410 | for branch_id in invisible_branch_ids] | 411 | if branch.id not in visible_branch_ids] |
399 | 411 | 412 | ||
400 | 412 | # Load the Git repositories. | 413 | # Load the Git repositories. |
401 | 413 | invisible_gitrepositories = [] | 414 | invisible_gitrepositories = [] |
403 | 414 | # XXX cjwatson 2015-02-16: Fill in once IGitCollection is in place. | 415 | if gitrepository_ids: |
404 | 416 | all_gitrepositories = getUtility(IAllGitRepositories) | ||
405 | 417 | visible_gitrepository_ids = all_gitrepositories.visibleByUser( | ||
406 | 418 | person).withIds(*gitrepository_ids).getRepositoryIds() | ||
407 | 419 | invisible_gitrepositories = [ | ||
408 | 420 | gitrepository for gitrepository in gitrepositories | ||
409 | 421 | if gitrepository.id not in visible_gitrepository_ids] | ||
410 | 415 | 422 | ||
411 | 416 | return invisible_bugs, invisible_branches, invisible_gitrepositories | 423 | return invisible_bugs, invisible_branches, invisible_gitrepositories |
412 | 417 | 424 | ||
413 | 418 | 425 | ||
414 | === modified file 'lib/lp/registry/services/tests/test_sharingservice.py' | |||
415 | --- lib/lp/registry/services/tests/test_sharingservice.py 2015-02-16 13:01:34 +0000 | |||
416 | +++ lib/lp/registry/services/tests/test_sharingservice.py 2015-02-27 11:27:39 +0000 | |||
417 | @@ -23,6 +23,7 @@ | |||
418 | 23 | CodeReviewNotificationLevel, | 23 | CodeReviewNotificationLevel, |
419 | 24 | ) | 24 | ) |
420 | 25 | from lp.code.interfaces.branch import IBranch | 25 | from lp.code.interfaces.branch import IBranch |
421 | 26 | from lp.code.interfaces.gitrepository import IGitRepository | ||
422 | 26 | from lp.registry.enums import ( | 27 | from lp.registry.enums import ( |
423 | 27 | BranchSharingPolicy, | 28 | BranchSharingPolicy, |
424 | 28 | BugSharingPolicy, | 29 | BugSharingPolicy, |
425 | @@ -928,12 +929,14 @@ | |||
426 | 928 | [InformationType.USERDATA]) | 929 | [InformationType.USERDATA]) |
427 | 929 | 930 | ||
428 | 930 | def _assert_revokeAccessGrants(self, pillar, bugs, branches, | 931 | def _assert_revokeAccessGrants(self, pillar, bugs, branches, |
430 | 931 | specifications): | 932 | gitrepositories, specifications): |
431 | 932 | artifacts = [] | 933 | artifacts = [] |
432 | 933 | if bugs: | 934 | if bugs: |
433 | 934 | artifacts.extend(bugs) | 935 | artifacts.extend(bugs) |
434 | 935 | if branches: | 936 | if branches: |
435 | 936 | artifacts.extend(branches) | 937 | artifacts.extend(branches) |
436 | 938 | if gitrepositories: | ||
437 | 939 | artifacts.extend(gitrepositories) | ||
438 | 937 | if specifications: | 940 | if specifications: |
439 | 938 | artifacts.extend(specifications) | 941 | artifacts.extend(specifications) |
440 | 939 | policy = self.factory.makeAccessPolicy(pillar=pillar, | 942 | policy = self.factory.makeAccessPolicy(pillar=pillar, |
441 | @@ -961,6 +964,8 @@ | |||
442 | 961 | branch.subscribe(person, | 964 | branch.subscribe(person, |
443 | 962 | BranchSubscriptionNotificationLevel.NOEMAIL, None, | 965 | BranchSubscriptionNotificationLevel.NOEMAIL, None, |
444 | 963 | CodeReviewNotificationLevel.NOEMAIL, pillar.owner) | 966 | CodeReviewNotificationLevel.NOEMAIL, pillar.owner) |
445 | 967 | # XXX cjwatson 2015-02-05: subscribe to Git repositories when | ||
446 | 968 | # implemented | ||
447 | 964 | for spec in specifications or []: | 969 | for spec in specifications or []: |
448 | 965 | spec.subscribe(person) | 970 | spec.subscribe(person) |
449 | 966 | 971 | ||
450 | @@ -973,7 +978,7 @@ | |||
451 | 973 | 978 | ||
452 | 974 | self.service.revokeAccessGrants( | 979 | self.service.revokeAccessGrants( |
453 | 975 | pillar, grantee, pillar.owner, bugs=bugs, branches=branches, | 980 | pillar, grantee, pillar.owner, bugs=bugs, branches=branches, |
455 | 976 | specifications=specifications) | 981 | gitrepositories=gitrepositories, specifications=specifications) |
456 | 977 | with block_on_job(self): | 982 | with block_on_job(self): |
457 | 978 | transaction.commit() | 983 | transaction.commit() |
458 | 979 | 984 | ||
459 | @@ -987,18 +992,22 @@ | |||
460 | 987 | self.assertNotIn(grantee, bug.getDirectSubscribers()) | 992 | self.assertNotIn(grantee, bug.getDirectSubscribers()) |
461 | 988 | for branch in branches or []: | 993 | for branch in branches or []: |
462 | 989 | self.assertNotIn(grantee, branch.subscribers) | 994 | self.assertNotIn(grantee, branch.subscribers) |
463 | 995 | # XXX cjwatson 2015-02-05: check revocation of subscription to Git | ||
464 | 996 | # repositories when implemented | ||
465 | 990 | for spec in specifications or []: | 997 | for spec in specifications or []: |
466 | 991 | self.assertNotIn(grantee, spec.subscribers) | 998 | self.assertNotIn(grantee, spec.subscribers) |
467 | 992 | 999 | ||
469 | 993 | # Someone else still has access to the bugs and branches. | 1000 | # Someone else still has access to the artifacts. |
470 | 994 | grants = accessartifact_grant_source.findByArtifact( | 1001 | grants = accessartifact_grant_source.findByArtifact( |
471 | 995 | access_artifacts, [someone]) | 1002 | access_artifacts, [someone]) |
472 | 996 | self.assertEqual(1, grants.count()) | 1003 | self.assertEqual(1, grants.count()) |
474 | 997 | # Someone else still has subscriptions to the bugs and branches. | 1004 | # Someone else still has subscriptions to the artifacts. |
475 | 998 | for bug in bugs or []: | 1005 | for bug in bugs or []: |
476 | 999 | self.assertIn(someone, bug.getDirectSubscribers()) | 1006 | self.assertIn(someone, bug.getDirectSubscribers()) |
477 | 1000 | for branch in branches or []: | 1007 | for branch in branches or []: |
478 | 1001 | self.assertIn(someone, branch.subscribers) | 1008 | self.assertIn(someone, branch.subscribers) |
479 | 1009 | # XXX cjwatson 2015-02-05: check subscription to Git repositories | ||
480 | 1010 | # when implemented | ||
481 | 1002 | for spec in specifications or []: | 1011 | for spec in specifications or []: |
482 | 1003 | self.assertIn(someone, spec.subscribers) | 1012 | self.assertIn(someone, spec.subscribers) |
483 | 1004 | 1013 | ||
484 | @@ -1010,7 +1019,7 @@ | |||
485 | 1010 | bug = self.factory.makeBug( | 1019 | bug = self.factory.makeBug( |
486 | 1011 | target=distro, owner=owner, | 1020 | target=distro, owner=owner, |
487 | 1012 | information_type=InformationType.USERDATA) | 1021 | information_type=InformationType.USERDATA) |
489 | 1013 | self._assert_revokeAccessGrants(distro, [bug], None, None) | 1022 | self._assert_revokeAccessGrants(distro, [bug], None, None, None) |
490 | 1014 | 1023 | ||
491 | 1015 | def test_revokeAccessGrantsBranches(self): | 1024 | def test_revokeAccessGrantsBranches(self): |
492 | 1016 | owner = self.factory.makePerson() | 1025 | owner = self.factory.makePerson() |
493 | @@ -1019,7 +1028,17 @@ | |||
494 | 1019 | branch = self.factory.makeBranch( | 1028 | branch = self.factory.makeBranch( |
495 | 1020 | product=product, owner=owner, | 1029 | product=product, owner=owner, |
496 | 1021 | information_type=InformationType.USERDATA) | 1030 | information_type=InformationType.USERDATA) |
498 | 1022 | self._assert_revokeAccessGrants(product, None, [branch], None) | 1031 | self._assert_revokeAccessGrants(product, None, [branch], None, None) |
499 | 1032 | |||
500 | 1033 | def test_revokeAccessGrantsGitRepositories(self): | ||
501 | 1034 | owner = self.factory.makePerson() | ||
502 | 1035 | product = self.factory.makeProduct(owner=owner) | ||
503 | 1036 | login_person(owner) | ||
504 | 1037 | gitrepository = self.factory.makeGitRepository( | ||
505 | 1038 | target=product, owner=owner, | ||
506 | 1039 | information_type=InformationType.USERDATA) | ||
507 | 1040 | self._assert_revokeAccessGrants( | ||
508 | 1041 | product, None, None, [gitrepository], None) | ||
509 | 1023 | 1042 | ||
510 | 1024 | def test_revokeAccessGrantsSpecifications(self): | 1043 | def test_revokeAccessGrantsSpecifications(self): |
511 | 1025 | owner = self.factory.makePerson() | 1044 | owner = self.factory.makePerson() |
512 | @@ -1030,15 +1049,18 @@ | |||
513 | 1030 | specification = self.factory.makeSpecification( | 1049 | specification = self.factory.makeSpecification( |
514 | 1031 | product=product, owner=owner, | 1050 | product=product, owner=owner, |
515 | 1032 | information_type=InformationType.EMBARGOED) | 1051 | information_type=InformationType.EMBARGOED) |
517 | 1033 | self._assert_revokeAccessGrants(product, None, None, [specification]) | 1052 | self._assert_revokeAccessGrants( |
518 | 1053 | product, None, None, None, [specification]) | ||
519 | 1034 | 1054 | ||
520 | 1035 | def _assert_revokeTeamAccessGrants(self, pillar, bugs, branches, | 1055 | def _assert_revokeTeamAccessGrants(self, pillar, bugs, branches, |
522 | 1036 | specifications): | 1056 | gitrepositories, specifications): |
523 | 1037 | artifacts = [] | 1057 | artifacts = [] |
524 | 1038 | if bugs: | 1058 | if bugs: |
525 | 1039 | artifacts.extend(bugs) | 1059 | artifacts.extend(bugs) |
526 | 1040 | if branches: | 1060 | if branches: |
527 | 1041 | artifacts.extend(branches) | 1061 | artifacts.extend(branches) |
528 | 1062 | if gitrepositories: | ||
529 | 1063 | artifacts.extend(gitrepositories) | ||
530 | 1042 | if specifications: | 1064 | if specifications: |
531 | 1043 | artifacts.extend(specifications) | 1065 | artifacts.extend(specifications) |
532 | 1044 | policy = self.factory.makeAccessPolicy(pillar=pillar, | 1066 | policy = self.factory.makeAccessPolicy(pillar=pillar, |
533 | @@ -1065,6 +1087,8 @@ | |||
534 | 1065 | branch.subscribe( | 1087 | branch.subscribe( |
535 | 1066 | person, BranchSubscriptionNotificationLevel.NOEMAIL, | 1088 | person, BranchSubscriptionNotificationLevel.NOEMAIL, |
536 | 1067 | None, CodeReviewNotificationLevel.NOEMAIL, pillar.owner) | 1089 | None, CodeReviewNotificationLevel.NOEMAIL, pillar.owner) |
537 | 1090 | # XXX cjwatson 2015-02-05: subscribe to Git repositories when | ||
538 | 1091 | # implemented | ||
539 | 1068 | # Subscribing somebody to a specification does not yet imply | 1092 | # Subscribing somebody to a specification does not yet imply |
540 | 1069 | # granting access to this person. | 1093 | # granting access to this person. |
541 | 1070 | if specifications: | 1094 | if specifications: |
542 | @@ -1075,12 +1099,16 @@ | |||
543 | 1075 | 1099 | ||
544 | 1076 | # Check that grantees have expected access grants and subscriptions. | 1100 | # Check that grantees have expected access grants and subscriptions. |
545 | 1077 | for person in [team_grantee, person_grantee]: | 1101 | for person in [team_grantee, person_grantee]: |
547 | 1078 | visible_bugs, visible_branches, _, visible_specs = ( | 1102 | (visible_bugs, visible_branches, visible_gitrepositories, |
548 | 1103 | visible_specs) = ( | ||
549 | 1079 | self.service.getVisibleArtifacts( | 1104 | self.service.getVisibleArtifacts( |
550 | 1080 | person, bugs=bugs, branches=branches, | 1105 | person, bugs=bugs, branches=branches, |
551 | 1106 | gitrepositories=gitrepositories, | ||
552 | 1081 | specifications=specifications)) | 1107 | specifications=specifications)) |
553 | 1082 | self.assertContentEqual(bugs or [], visible_bugs) | 1108 | self.assertContentEqual(bugs or [], visible_bugs) |
554 | 1083 | self.assertContentEqual(branches or [], visible_branches) | 1109 | self.assertContentEqual(branches or [], visible_branches) |
555 | 1110 | # XXX cjwatson 2015-02-05: check Git repositories when | ||
556 | 1111 | # subscription is implemented | ||
557 | 1084 | self.assertContentEqual(specifications or [], visible_specs) | 1112 | self.assertContentEqual(specifications or [], visible_specs) |
558 | 1085 | for person in [team_grantee, person_grantee]: | 1113 | for person in [team_grantee, person_grantee]: |
559 | 1086 | for bug in bugs or []: | 1114 | for bug in bugs or []: |
560 | @@ -1088,7 +1116,7 @@ | |||
561 | 1088 | 1116 | ||
562 | 1089 | self.service.revokeAccessGrants( | 1117 | self.service.revokeAccessGrants( |
563 | 1090 | pillar, team_grantee, pillar.owner, bugs=bugs, branches=branches, | 1118 | pillar, team_grantee, pillar.owner, bugs=bugs, branches=branches, |
565 | 1091 | specifications=specifications) | 1119 | gitrepositories=gitrepositories, specifications=specifications) |
566 | 1092 | with block_on_job(self): | 1120 | with block_on_job(self): |
567 | 1093 | transaction.commit() | 1121 | transaction.commit() |
568 | 1094 | 1122 | ||
569 | @@ -1103,11 +1131,14 @@ | |||
570 | 1103 | for person in [team_grantee, person_grantee]: | 1131 | for person in [team_grantee, person_grantee]: |
571 | 1104 | for bug in bugs or []: | 1132 | for bug in bugs or []: |
572 | 1105 | self.assertNotIn(person, bug.getDirectSubscribers()) | 1133 | self.assertNotIn(person, bug.getDirectSubscribers()) |
574 | 1106 | visible_bugs, visible_branches, _, visible_specs = ( | 1134 | (visible_bugs, visible_branches, visible_gitrepositories, |
575 | 1135 | visible_specs) = ( | ||
576 | 1107 | self.service.getVisibleArtifacts( | 1136 | self.service.getVisibleArtifacts( |
578 | 1108 | person, bugs=bugs, branches=branches)) | 1137 | person, bugs=bugs, branches=branches, |
579 | 1138 | gitrepositories=gitrepositories)) | ||
580 | 1109 | self.assertContentEqual([], visible_bugs) | 1139 | self.assertContentEqual([], visible_bugs) |
581 | 1110 | self.assertContentEqual([], visible_branches) | 1140 | self.assertContentEqual([], visible_branches) |
582 | 1141 | self.assertContentEqual([], visible_gitrepositories) | ||
583 | 1111 | self.assertContentEqual([], visible_specs) | 1142 | self.assertContentEqual([], visible_specs) |
584 | 1112 | 1143 | ||
585 | 1113 | def test_revokeTeamAccessGrantsBugs(self): | 1144 | def test_revokeTeamAccessGrantsBugs(self): |
586 | @@ -1118,7 +1149,7 @@ | |||
587 | 1118 | bug = self.factory.makeBug( | 1149 | bug = self.factory.makeBug( |
588 | 1119 | target=distro, owner=owner, | 1150 | target=distro, owner=owner, |
589 | 1120 | information_type=InformationType.USERDATA) | 1151 | information_type=InformationType.USERDATA) |
591 | 1121 | self._assert_revokeTeamAccessGrants(distro, [bug], None, None) | 1152 | self._assert_revokeTeamAccessGrants(distro, [bug], None, None, None) |
592 | 1122 | 1153 | ||
593 | 1123 | def test_revokeTeamAccessGrantsBranches(self): | 1154 | def test_revokeTeamAccessGrantsBranches(self): |
594 | 1124 | # Users with launchpad.Edit can delete all access for a grantee. | 1155 | # Users with launchpad.Edit can delete all access for a grantee. |
595 | @@ -1127,7 +1158,20 @@ | |||
596 | 1127 | login_person(owner) | 1158 | login_person(owner) |
597 | 1128 | branch = self.factory.makeBranch( | 1159 | branch = self.factory.makeBranch( |
598 | 1129 | owner=owner, information_type=InformationType.USERDATA) | 1160 | owner=owner, information_type=InformationType.USERDATA) |
600 | 1130 | self._assert_revokeTeamAccessGrants(product, None, [branch], None) | 1161 | self._assert_revokeTeamAccessGrants( |
601 | 1162 | product, None, [branch], None, None) | ||
602 | 1163 | |||
603 | 1164 | # XXX cjwatson 2015-02-05: Enable this once GitRepositorySubscription is | ||
604 | 1165 | # implemented. | ||
605 | 1166 | def disabled_test_revokeTeamAccessGrantsGitRepositories(self): | ||
606 | 1167 | # Users with launchpad.Edit can delete all access for a grantee. | ||
607 | 1168 | owner = self.factory.makePerson() | ||
608 | 1169 | product = self.factory.makeProduct(owner=owner) | ||
609 | 1170 | login_person(owner) | ||
610 | 1171 | gitrepository = self.factory.makeGitRepository( | ||
611 | 1172 | owner=owner, information_type=InformationType.USERDATA) | ||
612 | 1173 | self._assert_revokeTeamAccessGrants( | ||
613 | 1174 | product, None, None, [gitrepository], None) | ||
614 | 1131 | 1175 | ||
615 | 1132 | def test_revokeTeamAccessGrantsSpecifications(self): | 1176 | def test_revokeTeamAccessGrantsSpecifications(self): |
616 | 1133 | # Users with launchpad.Edit can delete all access for a grantee. | 1177 | # Users with launchpad.Edit can delete all access for a grantee. |
617 | @@ -1140,7 +1184,7 @@ | |||
618 | 1140 | product=product, owner=owner, | 1184 | product=product, owner=owner, |
619 | 1141 | information_type=InformationType.EMBARGOED) | 1185 | information_type=InformationType.EMBARGOED) |
620 | 1142 | self._assert_revokeTeamAccessGrants( | 1186 | self._assert_revokeTeamAccessGrants( |
622 | 1143 | product, None, None, [specification]) | 1187 | product, None, None, None, [specification]) |
623 | 1144 | 1188 | ||
624 | 1145 | def _assert_revokeAccessGrantsUnauthorized(self): | 1189 | def _assert_revokeAccessGrantsUnauthorized(self): |
625 | 1146 | # revokeAccessGrants raises an Unauthorized exception if the user | 1190 | # revokeAccessGrants raises an Unauthorized exception if the user |
626 | @@ -1163,9 +1207,9 @@ | |||
627 | 1163 | login_person(self.factory.makePerson()) | 1207 | login_person(self.factory.makePerson()) |
628 | 1164 | self._assert_revokeAccessGrantsUnauthorized() | 1208 | self._assert_revokeAccessGrantsUnauthorized() |
629 | 1165 | 1209 | ||
631 | 1166 | def test_revokeAccessGrants_without_bugs_or_branches(self): | 1210 | def test_revokeAccessGrants_without_artifacts(self): |
632 | 1167 | # The revokeAccessGrants method raises a ValueError if called without | 1211 | # The revokeAccessGrants method raises a ValueError if called without |
634 | 1168 | # specifying either bugs or branches. | 1212 | # specifying any artifacts. |
635 | 1169 | owner = self.factory.makePerson() | 1213 | owner = self.factory.makePerson() |
636 | 1170 | product = self.factory.makeProduct(owner=owner) | 1214 | product = self.factory.makeProduct(owner=owner) |
637 | 1171 | grantee = self.factory.makePerson() | 1215 | grantee = self.factory.makePerson() |
638 | @@ -1174,24 +1218,27 @@ | |||
639 | 1174 | ValueError, self.service.revokeAccessGrants, | 1218 | ValueError, self.service.revokeAccessGrants, |
640 | 1175 | product, grantee, product.owner) | 1219 | product, grantee, product.owner) |
641 | 1176 | 1220 | ||
644 | 1177 | def _assert_ensureAccessGrants(self, user, bugs, branches, specifications, | 1221 | def _assert_ensureAccessGrants(self, user, bugs, branches, gitrepositories, |
645 | 1178 | grantee=None): | 1222 | specifications, grantee=None): |
646 | 1179 | # Creating access grants works as expected. | 1223 | # Creating access grants works as expected. |
647 | 1180 | if not grantee: | 1224 | if not grantee: |
648 | 1181 | grantee = self.factory.makePerson() | 1225 | grantee = self.factory.makePerson() |
649 | 1182 | self.service.ensureAccessGrants( | 1226 | self.service.ensureAccessGrants( |
650 | 1183 | [grantee], user, bugs=bugs, branches=branches, | 1227 | [grantee], user, bugs=bugs, branches=branches, |
652 | 1184 | specifications=specifications) | 1228 | gitrepositories=gitrepositories, specifications=specifications) |
653 | 1185 | 1229 | ||
654 | 1186 | # Check that grantee has expected access grants. | 1230 | # Check that grantee has expected access grants. |
655 | 1187 | shared_bugs = [] | 1231 | shared_bugs = [] |
656 | 1188 | shared_branches = [] | 1232 | shared_branches = [] |
657 | 1233 | shared_gitrepositories = [] | ||
658 | 1189 | shared_specifications = [] | 1234 | shared_specifications = [] |
659 | 1190 | all_pillars = [] | 1235 | all_pillars = [] |
660 | 1191 | for bug in bugs or []: | 1236 | for bug in bugs or []: |
661 | 1192 | all_pillars.extend(bug.affected_pillars) | 1237 | all_pillars.extend(bug.affected_pillars) |
662 | 1193 | for branch in branches or []: | 1238 | for branch in branches or []: |
663 | 1194 | all_pillars.append(branch.target.context) | 1239 | all_pillars.append(branch.target.context) |
664 | 1240 | for gitrepository in gitrepositories or []: | ||
665 | 1241 | all_pillars.append(gitrepository.target) | ||
666 | 1195 | for specification in specifications or []: | 1242 | for specification in specifications or []: |
667 | 1196 | all_pillars.append(specification.target) | 1243 | all_pillars.append(specification.target) |
668 | 1197 | policies = getUtility(IAccessPolicySource).findByPillar(all_pillars) | 1244 | policies = getUtility(IAccessPolicySource).findByPillar(all_pillars) |
669 | @@ -1203,10 +1250,13 @@ | |||
670 | 1203 | shared_bugs.append(a.concrete_artifact) | 1250 | shared_bugs.append(a.concrete_artifact) |
671 | 1204 | elif IBranch.providedBy(a.concrete_artifact): | 1251 | elif IBranch.providedBy(a.concrete_artifact): |
672 | 1205 | shared_branches.append(a.concrete_artifact) | 1252 | shared_branches.append(a.concrete_artifact) |
673 | 1253 | elif IGitRepository.providedBy(a.concrete_artifact): | ||
674 | 1254 | shared_gitrepositories.append(a.concrete_artifact) | ||
675 | 1206 | elif ISpecification.providedBy(a.concrete_artifact): | 1255 | elif ISpecification.providedBy(a.concrete_artifact): |
676 | 1207 | shared_specifications.append(a.concrete_artifact) | 1256 | shared_specifications.append(a.concrete_artifact) |
677 | 1208 | self.assertContentEqual(bugs or [], shared_bugs) | 1257 | self.assertContentEqual(bugs or [], shared_bugs) |
678 | 1209 | self.assertContentEqual(branches or [], shared_branches) | 1258 | self.assertContentEqual(branches or [], shared_branches) |
679 | 1259 | self.assertContentEqual(gitrepositories or [], shared_gitrepositories) | ||
680 | 1210 | self.assertContentEqual(specifications or [], shared_specifications) | 1260 | self.assertContentEqual(specifications or [], shared_specifications) |
681 | 1211 | 1261 | ||
682 | 1212 | def test_ensureAccessGrantsBugs(self): | 1262 | def test_ensureAccessGrantsBugs(self): |
683 | @@ -1217,7 +1267,7 @@ | |||
684 | 1217 | bug = self.factory.makeBug( | 1267 | bug = self.factory.makeBug( |
685 | 1218 | target=distro, owner=owner, | 1268 | target=distro, owner=owner, |
686 | 1219 | information_type=InformationType.USERDATA) | 1269 | information_type=InformationType.USERDATA) |
688 | 1220 | self._assert_ensureAccessGrants(owner, [bug], None, None) | 1270 | self._assert_ensureAccessGrants(owner, [bug], None, None, None) |
689 | 1221 | 1271 | ||
690 | 1222 | def test_ensureAccessGrantsBranches(self): | 1272 | def test_ensureAccessGrantsBranches(self): |
691 | 1223 | # Access grants can be created for branches. | 1273 | # Access grants can be created for branches. |
692 | @@ -1227,7 +1277,18 @@ | |||
693 | 1227 | branch = self.factory.makeBranch( | 1277 | branch = self.factory.makeBranch( |
694 | 1228 | product=product, owner=owner, | 1278 | product=product, owner=owner, |
695 | 1229 | information_type=InformationType.USERDATA) | 1279 | information_type=InformationType.USERDATA) |
697 | 1230 | self._assert_ensureAccessGrants(owner, None, [branch], None) | 1280 | self._assert_ensureAccessGrants(owner, None, [branch], None, None) |
698 | 1281 | |||
699 | 1282 | def test_ensureAccessGrantsGitRepositories(self): | ||
700 | 1283 | # Access grants can be created for Git repositories. | ||
701 | 1284 | owner = self.factory.makePerson() | ||
702 | 1285 | product = self.factory.makeProduct(owner=owner) | ||
703 | 1286 | login_person(owner) | ||
704 | 1287 | gitrepository = self.factory.makeGitRepository( | ||
705 | 1288 | target=product, owner=owner, | ||
706 | 1289 | information_type=InformationType.USERDATA) | ||
707 | 1290 | self._assert_ensureAccessGrants( | ||
708 | 1291 | owner, None, None, [gitrepository], None) | ||
709 | 1231 | 1292 | ||
710 | 1232 | def test_ensureAccessGrantsSpecifications(self): | 1293 | def test_ensureAccessGrantsSpecifications(self): |
711 | 1233 | # Access grants can be created for branches. | 1294 | # Access grants can be created for branches. |
712 | @@ -1244,7 +1305,8 @@ | |||
713 | 1244 | with person_logged_in(owner): | 1305 | with person_logged_in(owner): |
714 | 1245 | specification.transitionToInformationType( | 1306 | specification.transitionToInformationType( |
715 | 1246 | InformationType.PROPRIETARY, owner) | 1307 | InformationType.PROPRIETARY, owner) |
717 | 1247 | self._assert_ensureAccessGrants(owner, None, None, [specification]) | 1308 | self._assert_ensureAccessGrants( |
718 | 1309 | owner, None, None, None, [specification]) | ||
719 | 1248 | 1310 | ||
720 | 1249 | def test_ensureAccessGrantsExisting(self): | 1311 | def test_ensureAccessGrantsExisting(self): |
721 | 1250 | # Any existing access grants are retained and new ones created. | 1312 | # Any existing access grants are retained and new ones created. |
722 | @@ -1263,7 +1325,7 @@ | |||
723 | 1263 | # Test with a new bug as well as the one for which access is already | 1325 | # Test with a new bug as well as the one for which access is already |
724 | 1264 | # granted. | 1326 | # granted. |
725 | 1265 | self._assert_ensureAccessGrants( | 1327 | self._assert_ensureAccessGrants( |
727 | 1266 | owner, [bug, bug2], None, None, grantee) | 1328 | owner, [bug, bug2], None, None, None, grantee) |
728 | 1267 | 1329 | ||
729 | 1268 | def _assert_ensureAccessGrantsUnauthorized(self, user): | 1330 | def _assert_ensureAccessGrantsUnauthorized(self, user): |
730 | 1269 | # ensureAccessGrants raises an Unauthorized exception if the user | 1331 | # ensureAccessGrants raises an Unauthorized exception if the user |
731 | @@ -1331,7 +1393,8 @@ | |||
732 | 1331 | self._assert_updatePillarSharingPoliciesUnauthorized(anyone) | 1393 | self._assert_updatePillarSharingPoliciesUnauthorized(anyone) |
733 | 1332 | 1394 | ||
734 | 1333 | def create_shared_artifacts(self, product, grantee, user): | 1395 | def create_shared_artifacts(self, product, grantee, user): |
736 | 1334 | # Create some shared bugs and branches. | 1396 | # Create some shared bugs, branches, Git repositories, and |
737 | 1397 | # specifications. | ||
738 | 1335 | bugs = [] | 1398 | bugs = [] |
739 | 1336 | bug_tasks = [] | 1399 | bug_tasks = [] |
740 | 1337 | for x in range(0, 10): | 1400 | for x in range(0, 10): |
741 | @@ -1346,6 +1409,12 @@ | |||
742 | 1346 | product=product, owner=product.owner, | 1409 | product=product, owner=product.owner, |
743 | 1347 | information_type=InformationType.USERDATA) | 1410 | information_type=InformationType.USERDATA) |
744 | 1348 | branches.append(branch) | 1411 | branches.append(branch) |
745 | 1412 | gitrepositories = [] | ||
746 | 1413 | for x in range(0, 10): | ||
747 | 1414 | gitrepository = self.factory.makeGitRepository( | ||
748 | 1415 | target=product, owner=product.owner, | ||
749 | 1416 | information_type=InformationType.USERDATA) | ||
750 | 1417 | gitrepositories.append(gitrepository) | ||
751 | 1349 | specs = [] | 1418 | specs = [] |
752 | 1350 | for x in range(0, 10): | 1419 | for x in range(0, 10): |
753 | 1351 | spec = self.factory.makeSpecification( | 1420 | spec = self.factory.makeSpecification( |
754 | @@ -1371,9 +1440,11 @@ | |||
755 | 1371 | grant_access(bug, i == 9) | 1440 | grant_access(bug, i == 9) |
756 | 1372 | for i, branch in enumerate(branches): | 1441 | for i, branch in enumerate(branches): |
757 | 1373 | grant_access(branch, i == 9) | 1442 | grant_access(branch, i == 9) |
758 | 1443 | for i, gitrepository in enumerate(gitrepositories): | ||
759 | 1444 | grant_access(gitrepository, i == 9) | ||
760 | 1374 | getUtility(IService, 'sharing').ensureAccessGrants( | 1445 | getUtility(IService, 'sharing').ensureAccessGrants( |
761 | 1375 | [grantee], product.owner, specifications=specs[:9]) | 1446 | [grantee], product.owner, specifications=specs[:9]) |
763 | 1376 | return bug_tasks, branches, specs | 1447 | return bug_tasks, branches, gitrepositories, specs |
764 | 1377 | 1448 | ||
765 | 1378 | def test_getSharedArtifacts(self): | 1449 | def test_getSharedArtifacts(self): |
766 | 1379 | # Test the getSharedArtifacts method. | 1450 | # Test the getSharedArtifacts method. |
767 | @@ -1384,14 +1455,16 @@ | |||
768 | 1384 | login_person(owner) | 1455 | login_person(owner) |
769 | 1385 | grantee = self.factory.makePerson() | 1456 | grantee = self.factory.makePerson() |
770 | 1386 | user = self.factory.makePerson() | 1457 | user = self.factory.makePerson() |
773 | 1387 | bug_tasks, branches, specs = self.create_shared_artifacts( | 1458 | bug_tasks, branches, gitrepositories, specs = ( |
774 | 1388 | product, grantee, user) | 1459 | self.create_shared_artifacts(product, grantee, user)) |
775 | 1389 | 1460 | ||
776 | 1390 | # Check the results. | 1461 | # Check the results. |
778 | 1391 | shared_bugtasks, shared_branches, _, shared_specs = ( | 1462 | (shared_bugtasks, shared_branches, shared_gitrepositories, |
779 | 1463 | shared_specs) = ( | ||
780 | 1392 | self.service.getSharedArtifacts(product, grantee, user)) | 1464 | self.service.getSharedArtifacts(product, grantee, user)) |
781 | 1393 | self.assertContentEqual(bug_tasks[:9], shared_bugtasks) | 1465 | self.assertContentEqual(bug_tasks[:9], shared_bugtasks) |
782 | 1394 | self.assertContentEqual(branches[:9], shared_branches) | 1466 | self.assertContentEqual(branches[:9], shared_branches) |
783 | 1467 | self.assertContentEqual(gitrepositories[:9], shared_gitrepositories) | ||
784 | 1395 | self.assertContentEqual(specs[:9], shared_specs) | 1468 | self.assertContentEqual(specs[:9], shared_specs) |
785 | 1396 | 1469 | ||
786 | 1397 | def _assert_getSharedProjects(self, product, who=None): | 1470 | def _assert_getSharedProjects(self, product, who=None): |
787 | @@ -1531,7 +1604,7 @@ | |||
788 | 1531 | login_person(owner) | 1604 | login_person(owner) |
789 | 1532 | grantee = self.factory.makePerson() | 1605 | grantee = self.factory.makePerson() |
790 | 1533 | user = self.factory.makePerson() | 1606 | user = self.factory.makePerson() |
792 | 1534 | bug_tasks, ignored, ignored = self.create_shared_artifacts( | 1607 | bug_tasks, _, _, _ = self.create_shared_artifacts( |
793 | 1535 | product, grantee, user) | 1608 | product, grantee, user) |
794 | 1536 | 1609 | ||
795 | 1537 | # Check the results. | 1610 | # Check the results. |
796 | @@ -1547,7 +1620,7 @@ | |||
797 | 1547 | login_person(owner) | 1620 | login_person(owner) |
798 | 1548 | grantee = self.factory.makePerson() | 1621 | grantee = self.factory.makePerson() |
799 | 1549 | user = self.factory.makePerson() | 1622 | user = self.factory.makePerson() |
801 | 1550 | ignored, branches, ignored = self.create_shared_artifacts( | 1623 | _, branches, _, _ = self.create_shared_artifacts( |
802 | 1551 | product, grantee, user) | 1624 | product, grantee, user) |
803 | 1552 | 1625 | ||
804 | 1553 | # Check the results. | 1626 | # Check the results. |
805 | @@ -1555,6 +1628,23 @@ | |||
806 | 1555 | product, grantee, user) | 1628 | product, grantee, user) |
807 | 1556 | self.assertContentEqual(branches[:9], shared_branches) | 1629 | self.assertContentEqual(branches[:9], shared_branches) |
808 | 1557 | 1630 | ||
809 | 1631 | def test_getSharedGitRepositories(self): | ||
810 | 1632 | # Test the getSharedGitRepositories method. | ||
811 | 1633 | owner = self.factory.makePerson() | ||
812 | 1634 | product = self.factory.makeProduct( | ||
813 | 1635 | owner=owner, specification_sharing_policy=( | ||
814 | 1636 | SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY)) | ||
815 | 1637 | login_person(owner) | ||
816 | 1638 | grantee = self.factory.makePerson() | ||
817 | 1639 | user = self.factory.makePerson() | ||
818 | 1640 | _, _, gitrepositories, _ = self.create_shared_artifacts( | ||
819 | 1641 | product, grantee, user) | ||
820 | 1642 | |||
821 | 1643 | # Check the results. | ||
822 | 1644 | shared_gitrepositories = self.service.getSharedGitRepositories( | ||
823 | 1645 | product, grantee, user) | ||
824 | 1646 | self.assertContentEqual(gitrepositories[:9], shared_gitrepositories) | ||
825 | 1647 | |||
826 | 1558 | def test_getSharedSpecifications(self): | 1648 | def test_getSharedSpecifications(self): |
827 | 1559 | # Test the getSharedSpecifications method. | 1649 | # Test the getSharedSpecifications method. |
828 | 1560 | owner = self.factory.makePerson() | 1650 | owner = self.factory.makePerson() |
829 | @@ -1564,7 +1654,7 @@ | |||
830 | 1564 | login_person(owner) | 1654 | login_person(owner) |
831 | 1565 | grantee = self.factory.makePerson() | 1655 | grantee = self.factory.makePerson() |
832 | 1566 | user = self.factory.makePerson() | 1656 | user = self.factory.makePerson() |
834 | 1567 | ignored, ignored, specifications = self.create_shared_artifacts( | 1657 | _, _, _, specifications = self.create_shared_artifacts( |
835 | 1568 | product, grantee, user) | 1658 | product, grantee, user) |
836 | 1569 | 1659 | ||
837 | 1570 | # Check the results. | 1660 | # Check the results. |
838 | @@ -1592,6 +1682,16 @@ | |||
839 | 1592 | login_person(owner) | 1682 | login_person(owner) |
840 | 1593 | self._assert_getPeopleWithoutAccess(product, branch) | 1683 | self._assert_getPeopleWithoutAccess(product, branch) |
841 | 1594 | 1684 | ||
842 | 1685 | def test_getPeopleWithAccessGitRepositories(self): | ||
843 | 1686 | # Test the getPeopleWithoutAccess method with Git repositories. | ||
844 | 1687 | owner = self.factory.makePerson() | ||
845 | 1688 | product = self.factory.makeProduct(owner=owner) | ||
846 | 1689 | gitrepository = self.factory.makeGitRepository( | ||
847 | 1690 | target=product, owner=owner, | ||
848 | 1691 | information_type=InformationType.USERDATA) | ||
849 | 1692 | login_person(owner) | ||
850 | 1693 | self._assert_getPeopleWithoutAccess(product, gitrepository) | ||
851 | 1694 | |||
852 | 1595 | def _assert_getPeopleWithoutAccess(self, product, artifact): | 1695 | def _assert_getPeopleWithoutAccess(self, product, artifact): |
853 | 1596 | access_artifact = self.factory.makeAccessArtifact(concrete=artifact) | 1696 | access_artifact = self.factory.makeAccessArtifact(concrete=artifact) |
854 | 1597 | # Make some people to check. people[:5] will not have access. | 1697 | # Make some people to check. people[:5] will not have access. |
855 | @@ -1647,6 +1747,12 @@ | |||
856 | 1647 | product=product, owner=owner, | 1747 | product=product, owner=owner, |
857 | 1648 | information_type=InformationType.USERDATA) | 1748 | information_type=InformationType.USERDATA) |
858 | 1649 | branches.append(branch) | 1749 | branches.append(branch) |
859 | 1750 | gitrepositories = [] | ||
860 | 1751 | for x in range(0, 10): | ||
861 | 1752 | gitrepository = self.factory.makeGitRepository( | ||
862 | 1753 | target=product, owner=owner, | ||
863 | 1754 | information_type=InformationType.USERDATA) | ||
864 | 1755 | gitrepositories.append(gitrepository) | ||
865 | 1650 | 1756 | ||
866 | 1651 | specifications = [] | 1757 | specifications = [] |
867 | 1652 | for x in range(0, 10): | 1758 | for x in range(0, 10): |
868 | @@ -1662,46 +1768,58 @@ | |||
869 | 1662 | artifact=access_artifact, grantee=grantee, grantor=owner) | 1768 | artifact=access_artifact, grantee=grantee, grantor=owner) |
870 | 1663 | return access_artifact | 1769 | return access_artifact |
871 | 1664 | 1770 | ||
873 | 1665 | # Grant access to some of the bugs and branches. | 1771 | # Grant access to some of the artifacts. |
874 | 1666 | for bug in bugs[:5]: | 1772 | for bug in bugs[:5]: |
875 | 1667 | grant_access(bug) | 1773 | grant_access(bug) |
876 | 1668 | for branch in branches[:5]: | 1774 | for branch in branches[:5]: |
877 | 1669 | grant_access(branch) | 1775 | grant_access(branch) |
878 | 1776 | for gitrepository in gitrepositories[:5]: | ||
879 | 1777 | grant_access(gitrepository) | ||
880 | 1670 | for spec in specifications[:5]: | 1778 | for spec in specifications[:5]: |
881 | 1671 | grant_access(spec) | 1779 | grant_access(spec) |
883 | 1672 | return grantee, owner, branches, bugs, specifications | 1780 | return grantee, owner, bugs, branches, gitrepositories, specifications |
884 | 1673 | 1781 | ||
885 | 1674 | def test_getVisibleArtifacts(self): | 1782 | def test_getVisibleArtifacts(self): |
886 | 1675 | # Test the getVisibleArtifacts method. | 1783 | # Test the getVisibleArtifacts method. |
888 | 1676 | grantee, ignore, branches, bugs, specs = self._make_Artifacts() | 1784 | grantee, ignore, bugs, branches, gitrepositories, specs = ( |
889 | 1785 | self._make_Artifacts()) | ||
890 | 1677 | # Check the results. | 1786 | # Check the results. |
892 | 1678 | shared_bugs, shared_branches, _, shared_specs = ( | 1787 | shared_bugs, shared_branches, shared_gitrepositories, shared_specs = ( |
893 | 1679 | self.service.getVisibleArtifacts( | 1788 | self.service.getVisibleArtifacts( |
895 | 1680 | grantee, bugs=bugs, branches=branches, specifications=specs)) | 1789 | grantee, bugs=bugs, branches=branches, |
896 | 1790 | gitrepositories=gitrepositories, specifications=specs)) | ||
897 | 1681 | self.assertContentEqual(bugs[:5], shared_bugs) | 1791 | self.assertContentEqual(bugs[:5], shared_bugs) |
898 | 1682 | self.assertContentEqual(branches[:5], shared_branches) | 1792 | self.assertContentEqual(branches[:5], shared_branches) |
899 | 1793 | self.assertContentEqual(gitrepositories[:5], shared_gitrepositories) | ||
900 | 1683 | self.assertContentEqual(specs[:5], shared_specs) | 1794 | self.assertContentEqual(specs[:5], shared_specs) |
901 | 1684 | 1795 | ||
902 | 1685 | def test_getVisibleArtifacts_grant_on_pillar(self): | 1796 | def test_getVisibleArtifacts_grant_on_pillar(self): |
903 | 1686 | # getVisibleArtifacts() returns private specifications if | 1797 | # getVisibleArtifacts() returns private specifications if |
904 | 1687 | # user has a policy grant for the pillar of the specification. | 1798 | # user has a policy grant for the pillar of the specification. |
907 | 1688 | ignore, owner, branches, bugs, specs = self._make_Artifacts() | 1799 | _, owner, bugs, branches, gitrepositories, specs = ( |
908 | 1689 | shared_bugs, shared_branches, _, shared_specs = ( | 1800 | self._make_Artifacts()) |
909 | 1801 | shared_bugs, shared_branches, shared_gitrepositories, shared_specs = ( | ||
910 | 1690 | self.service.getVisibleArtifacts( | 1802 | self.service.getVisibleArtifacts( |
912 | 1691 | owner, bugs=bugs, branches=branches, specifications=specs)) | 1803 | owner, bugs=bugs, branches=branches, |
913 | 1804 | gitrepositories=gitrepositories, specifications=specs)) | ||
914 | 1692 | self.assertContentEqual(bugs, shared_bugs) | 1805 | self.assertContentEqual(bugs, shared_bugs) |
915 | 1693 | self.assertContentEqual(branches, shared_branches) | 1806 | self.assertContentEqual(branches, shared_branches) |
916 | 1807 | self.assertContentEqual(gitrepositories, shared_gitrepositories) | ||
917 | 1694 | self.assertContentEqual(specs, shared_specs) | 1808 | self.assertContentEqual(specs, shared_specs) |
918 | 1695 | 1809 | ||
919 | 1696 | def test_getInvisibleArtifacts(self): | 1810 | def test_getInvisibleArtifacts(self): |
920 | 1697 | # Test the getInvisibleArtifacts method. | 1811 | # Test the getInvisibleArtifacts method. |
922 | 1698 | grantee, ignore, branches, bugs, specs = self._make_Artifacts() | 1812 | grantee, ignore, bugs, branches, gitrepositories, specs = ( |
923 | 1813 | self._make_Artifacts()) | ||
924 | 1699 | # Check the results. | 1814 | # Check the results. |
926 | 1700 | not_shared_bugs, not_shared_branches, _ = ( | 1815 | not_shared_bugs, not_shared_branches, not_shared_gitrepositories = ( |
927 | 1701 | self.service.getInvisibleArtifacts( | 1816 | self.service.getInvisibleArtifacts( |
929 | 1702 | grantee, bugs=bugs, branches=branches)) | 1817 | grantee, bugs=bugs, branches=branches, |
930 | 1818 | gitrepositories=gitrepositories)) | ||
931 | 1703 | self.assertContentEqual(bugs[5:], not_shared_bugs) | 1819 | self.assertContentEqual(bugs[5:], not_shared_bugs) |
932 | 1704 | self.assertContentEqual(branches[5:], not_shared_branches) | 1820 | self.assertContentEqual(branches[5:], not_shared_branches) |
933 | 1821 | self.assertContentEqual( | ||
934 | 1822 | gitrepositories[5:], not_shared_gitrepositories) | ||
935 | 1705 | 1823 | ||
936 | 1706 | def _assert_getVisibleArtifacts_bug_change(self, change_callback): | 1824 | def _assert_getVisibleArtifacts_bug_change(self, change_callback): |
937 | 1707 | # Test the getVisibleArtifacts method excludes bugs after a change of | 1825 | # Test the getVisibleArtifacts method excludes bugs after a change of |
938 | @@ -1723,7 +1841,7 @@ | |||
939 | 1723 | information_type=InformationType.USERDATA) | 1841 | information_type=InformationType.USERDATA) |
940 | 1724 | bugs.append(bug) | 1842 | bugs.append(bug) |
941 | 1725 | 1843 | ||
943 | 1726 | shared_bugs, shared_branches, _, shared_specs = ( | 1844 | shared_bugs, shared_branches, shared_gitrepositories, shared_specs = ( |
944 | 1727 | self.service.getVisibleArtifacts(grantee, bugs=bugs)) | 1845 | self.service.getVisibleArtifacts(grantee, bugs=bugs)) |
945 | 1728 | self.assertContentEqual(bugs, shared_bugs) | 1846 | self.assertContentEqual(bugs, shared_bugs) |
946 | 1729 | 1847 | ||
947 | @@ -1731,7 +1849,7 @@ | |||
948 | 1731 | for x in range(0, 5): | 1849 | for x in range(0, 5): |
949 | 1732 | change_callback(bugs[x], owner) | 1850 | change_callback(bugs[x], owner) |
950 | 1733 | # Check the results. | 1851 | # Check the results. |
952 | 1734 | shared_bugs, shared_branches, _, shared_specs = ( | 1852 | shared_bugs, shared_branches, shared_gitrepositories, shared_specs = ( |
953 | 1735 | self.service.getVisibleArtifacts(grantee, bugs=bugs)) | 1853 | self.service.getVisibleArtifacts(grantee, bugs=bugs)) |
954 | 1736 | self.assertContentEqual(bugs[5:], shared_bugs) | 1854 | self.assertContentEqual(bugs[5:], shared_bugs) |
955 | 1737 | 1855 | ||
956 | 1738 | 1856 | ||
957 | === modified file 'lib/lp/registry/tests/test_accesspolicy.py' | |||
958 | --- lib/lp/registry/tests/test_accesspolicy.py 2013-06-20 05:50:00 +0000 | |||
959 | +++ lib/lp/registry/tests/test_accesspolicy.py 2015-02-27 11:27:39 +0000 | |||
960 | @@ -1,4 +1,4 @@ | |||
962 | 1 | # Copyright 2011-2012 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2011-2015 Canonical Ltd. This software is licensed under the |
963 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
964 | 3 | 3 | ||
965 | 4 | __metaclass__ = type | 4 | __metaclass__ = type |
966 | @@ -280,6 +280,13 @@ | |||
967 | 280 | return self.factory.makeBranch() | 280 | return self.factory.makeBranch() |
968 | 281 | 281 | ||
969 | 282 | 282 | ||
970 | 283 | class TestAccessArtifactGitRepository(BaseAccessArtifactTests, | ||
971 | 284 | TestCaseWithFactory): | ||
972 | 285 | |||
973 | 286 | def getConcreteArtifact(self): | ||
974 | 287 | return self.factory.makeGitRepository() | ||
975 | 288 | |||
976 | 289 | |||
977 | 283 | class TestAccessArtifactBug(BaseAccessArtifactTests, | 290 | class TestAccessArtifactBug(BaseAccessArtifactTests, |
978 | 284 | TestCaseWithFactory): | 291 | TestCaseWithFactory): |
979 | 285 | 292 | ||
980 | 286 | 293 | ||
981 | === modified file 'lib/lp/registry/tests/test_sharingjob.py' | |||
982 | --- lib/lp/registry/tests/test_sharingjob.py 2014-06-19 06:38:53 +0000 | |||
983 | +++ lib/lp/registry/tests/test_sharingjob.py 2015-02-27 11:27:39 +0000 | |||
984 | @@ -1,4 +1,4 @@ | |||
986 | 1 | # Copyright 2012 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012-2015 Canonical Ltd. This software is licensed under the |
987 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
988 | 3 | 3 | ||
989 | 4 | """Tests for SharingJobs.""" | 4 | """Tests for SharingJobs.""" |
990 | @@ -118,6 +118,16 @@ | |||
991 | 118 | 'for branch_ids=[%d], requestor=%s>' | 118 | 'for branch_ids=[%d], requestor=%s>' |
992 | 119 | % (branch.id, requestor.name), repr(job)) | 119 | % (branch.id, requestor.name), repr(job)) |
993 | 120 | 120 | ||
994 | 121 | def test_repr_gitrepositories(self): | ||
995 | 122 | requestor = self.factory.makePerson() | ||
996 | 123 | gitrepository = self.factory.makeGitRepository() | ||
997 | 124 | job = getUtility(IRemoveArtifactSubscriptionsJobSource).create( | ||
998 | 125 | requestor, artifacts=[gitrepository]) | ||
999 | 126 | self.assertEqual( | ||
1000 | 127 | '<REMOVE_ARTIFACT_SUBSCRIPTIONS job reconciling subscriptions ' | ||
1001 | 128 | 'for gitrepository_ids=[%d], requestor=%s>' | ||
1002 | 129 | % (gitrepository.id, requestor.name), repr(job)) | ||
1003 | 130 | |||
1004 | 121 | def test_repr_specifications(self): | 131 | def test_repr_specifications(self): |
1005 | 122 | requestor = self.factory.makePerson() | 132 | requestor = self.factory.makePerson() |
1006 | 123 | specification = self.factory.makeSpecification() | 133 | specification = self.factory.makeSpecification() |
1007 | @@ -303,6 +313,9 @@ | |||
1008 | 303 | branch = self.factory.makeBranch( | 313 | branch = self.factory.makeBranch( |
1009 | 304 | owner=owner, product=product, | 314 | owner=owner, product=product, |
1010 | 305 | information_type=InformationType.USERDATA) | 315 | information_type=InformationType.USERDATA) |
1011 | 316 | gitrepository = self.factory.makeGitRepository( | ||
1012 | 317 | owner=owner, target=product, | ||
1013 | 318 | information_type=InformationType.USERDATA) | ||
1014 | 306 | specification = self.factory.makeSpecification( | 319 | specification = self.factory.makeSpecification( |
1015 | 307 | owner=owner, product=product, | 320 | owner=owner, product=product, |
1016 | 308 | information_type=InformationType.PROPRIETARY) | 321 | information_type=InformationType.PROPRIETARY) |
1017 | @@ -317,6 +330,8 @@ | |||
1018 | 317 | branch.subscribe(artifact_indirect_grantee, | 330 | branch.subscribe(artifact_indirect_grantee, |
1019 | 318 | BranchSubscriptionNotificationLevel.NOEMAIL, None, | 331 | BranchSubscriptionNotificationLevel.NOEMAIL, None, |
1020 | 319 | CodeReviewNotificationLevel.NOEMAIL, owner) | 332 | CodeReviewNotificationLevel.NOEMAIL, owner) |
1021 | 333 | # XXX cjwatson 2015-02-05: Fill this in once we have | ||
1022 | 334 | # GitRepositorySubscription. | ||
1023 | 320 | # Subscribing somebody to a specification does not automatically | 335 | # Subscribing somebody to a specification does not automatically |
1024 | 321 | # create an artifact grant. | 336 | # create an artifact grant. |
1025 | 322 | spec_artifact = self.factory.makeAccessArtifact(specification) | 337 | spec_artifact = self.factory.makeAccessArtifact(specification) |
1026 | @@ -325,10 +340,10 @@ | |||
1027 | 325 | with person_logged_in(product.owner): | 340 | with person_logged_in(product.owner): |
1028 | 326 | specification.subscribe(artifact_indirect_grantee, owner) | 341 | specification.subscribe(artifact_indirect_grantee, owner) |
1029 | 327 | 342 | ||
1032 | 328 | # pick one of the concrete artifacts (bug, branch or spec) | 343 | # pick one of the concrete artifacts (bug, branch, Git repository, |
1033 | 329 | # and subscribe the teams and persons. | 344 | # or spec) and subscribe the teams and persons. |
1034 | 330 | concrete_artifact, get_pillars, get_subscribers = configure_test( | 345 | concrete_artifact, get_pillars, get_subscribers = configure_test( |
1036 | 331 | bug, branch, specification, policy_team_grantee, | 346 | bug, branch, gitrepository, specification, policy_team_grantee, |
1037 | 332 | policy_indirect_grantee, artifact_team_grantee, owner) | 347 | policy_indirect_grantee, artifact_team_grantee, owner) |
1038 | 333 | 348 | ||
1039 | 334 | # Subscribing policy_team_grantee has created an artifact grant so we | 349 | # Subscribing policy_team_grantee has created an artifact grant so we |
1040 | @@ -361,6 +376,8 @@ | |||
1041 | 361 | self.assertIn(artifact_team_grantee, subscribers) | 376 | self.assertIn(artifact_team_grantee, subscribers) |
1042 | 362 | self.assertIn(artifact_indirect_grantee, bug.getDirectSubscribers()) | 377 | self.assertIn(artifact_indirect_grantee, bug.getDirectSubscribers()) |
1043 | 363 | self.assertIn(artifact_indirect_grantee, branch.subscribers) | 378 | self.assertIn(artifact_indirect_grantee, branch.subscribers) |
1044 | 379 | # XXX cjwatson 2015-02-05: Fill this in once we have | ||
1045 | 380 | # GitRepositorySubscription. | ||
1046 | 364 | self.assertIn(artifact_indirect_grantee, | 381 | self.assertIn(artifact_indirect_grantee, |
1047 | 365 | removeSecurityProxy(specification).subscribers) | 382 | removeSecurityProxy(specification).subscribers) |
1048 | 366 | 383 | ||
1049 | @@ -373,9 +390,9 @@ | |||
1050 | 373 | return removeSecurityProxy( | 390 | return removeSecurityProxy( |
1051 | 374 | concrete_artifact).getDirectSubscribers() | 391 | concrete_artifact).getDirectSubscribers() |
1052 | 375 | 392 | ||
1056 | 376 | def configure_test(bug, branch, specification, policy_team_grantee, | 393 | def configure_test(bug, branch, gitrepository, specification, |
1057 | 377 | policy_indirect_grantee, artifact_team_grantee, | 394 | policy_team_grantee, policy_indirect_grantee, |
1058 | 378 | owner): | 395 | artifact_team_grantee, owner): |
1059 | 379 | concrete_artifact = bug | 396 | concrete_artifact = bug |
1060 | 380 | bug.subscribe(policy_team_grantee, owner) | 397 | bug.subscribe(policy_team_grantee, owner) |
1061 | 381 | bug.subscribe(policy_indirect_grantee, owner) | 398 | bug.subscribe(policy_indirect_grantee, owner) |
1062 | @@ -393,9 +410,9 @@ | |||
1063 | 393 | def get_subscribers(concrete_artifact): | 410 | def get_subscribers(concrete_artifact): |
1064 | 394 | return concrete_artifact.subscribers | 411 | return concrete_artifact.subscribers |
1065 | 395 | 412 | ||
1069 | 396 | def configure_test(bug, branch, specification, policy_team_grantee, | 413 | def configure_test(bug, branch, gitrepository, specification, |
1070 | 397 | policy_indirect_grantee, artifact_team_grantee, | 414 | policy_team_grantee, policy_indirect_grantee, |
1071 | 398 | owner): | 415 | artifact_team_grantee, owner): |
1072 | 399 | concrete_artifact = branch | 416 | concrete_artifact = branch |
1073 | 400 | branch.subscribe( | 417 | branch.subscribe( |
1074 | 401 | policy_team_grantee, | 418 | policy_team_grantee, |
1075 | @@ -414,6 +431,27 @@ | |||
1076 | 414 | self._assert_artifact_change_unsubscribes( | 431 | self._assert_artifact_change_unsubscribes( |
1077 | 415 | change_callback, configure_test) | 432 | change_callback, configure_test) |
1078 | 416 | 433 | ||
1079 | 434 | def _assert_gitrepository_change_unsubscribes(self, change_callback): | ||
1080 | 435 | |||
1081 | 436 | def get_pillars(concrete_artifact): | ||
1082 | 437 | return [concrete_artifact.product] | ||
1083 | 438 | |||
1084 | 439 | def get_subscribers(concrete_artifact): | ||
1085 | 440 | return concrete_artifact.subscribers | ||
1086 | 441 | |||
1087 | 442 | def configure_test(bug, branch, gitrepository, specification, | ||
1088 | 443 | policy_team_grantee, policy_indirect_grantee, | ||
1089 | 444 | artifact_team_grantee, owner): | ||
1090 | 445 | concrete_artifact = gitrepository | ||
1091 | 446 | # XXX cjwatson 2015-02-05: Fill this in once we have | ||
1092 | 447 | # GitRepositorySubscription. | ||
1093 | 448 | return concrete_artifact, get_pillars, get_subscribers | ||
1094 | 449 | |||
1095 | 450 | # XXX cjwatson 2015-02-05: Uncomment once we have | ||
1096 | 451 | # GitRepositorySubscription. | ||
1097 | 452 | #self._assert_artifact_change_unsubscribes( | ||
1098 | 453 | # change_callback, configure_test) | ||
1099 | 454 | |||
1100 | 417 | def _assert_specification_change_unsubscribes(self, change_callback): | 455 | def _assert_specification_change_unsubscribes(self, change_callback): |
1101 | 418 | 456 | ||
1102 | 419 | def get_pillars(concrete_artifact): | 457 | def get_pillars(concrete_artifact): |
1103 | @@ -422,9 +460,9 @@ | |||
1104 | 422 | def get_subscribers(concrete_artifact): | 460 | def get_subscribers(concrete_artifact): |
1105 | 423 | return concrete_artifact.subscribers | 461 | return concrete_artifact.subscribers |
1106 | 424 | 462 | ||
1110 | 425 | def configure_test(bug, branch, specification, policy_team_grantee, | 463 | def configure_test(bug, branch, gitrepository, specification, |
1111 | 426 | policy_indirect_grantee, artifact_team_grantee, | 464 | policy_team_grantee, policy_indirect_grantee, |
1112 | 427 | owner): | 465 | artifact_team_grantee, owner): |
1113 | 428 | naked_spec = removeSecurityProxy(specification) | 466 | naked_spec = removeSecurityProxy(specification) |
1114 | 429 | naked_spec.subscribe(policy_team_grantee, owner) | 467 | naked_spec.subscribe(policy_team_grantee, owner) |
1115 | 430 | naked_spec.subscribe(policy_indirect_grantee, owner) | 468 | naked_spec.subscribe(policy_indirect_grantee, owner) |
1116 | @@ -444,6 +482,13 @@ | |||
1117 | 444 | 482 | ||
1118 | 445 | self._assert_branch_change_unsubscribes(change_information_type) | 483 | self._assert_branch_change_unsubscribes(change_information_type) |
1119 | 446 | 484 | ||
1120 | 485 | def test_change_information_type_gitrepository(self): | ||
1121 | 486 | def change_information_type(gitrepository): | ||
1122 | 487 | removeSecurityProxy(gitrepository).information_type = ( | ||
1123 | 488 | InformationType.PRIVATESECURITY) | ||
1124 | 489 | |||
1125 | 490 | self._assert_gitrepository_change_unsubscribes(change_information_type) | ||
1126 | 491 | |||
1127 | 447 | def test_change_information_type_specification(self): | 492 | def test_change_information_type_specification(self): |
1128 | 448 | def change_information_type(specification): | 493 | def change_information_type(specification): |
1129 | 449 | removeSecurityProxy(specification).information_type = ( | 494 | removeSecurityProxy(specification).information_type = ( |
1130 | 450 | 495 | ||
1131 | === modified file 'lib/lp/security.py' | |||
1132 | --- lib/lp/security.py 2015-02-23 03:22:37 +0000 | |||
1133 | +++ lib/lp/security.py 2015-02-27 11:27:39 +0000 | |||
1134 | @@ -1,4 +1,4 @@ | |||
1136 | 1 | # Copyright 2009-2014 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2009-2015 Canonical Ltd. This software is licensed under the |
1137 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
1138 | 3 | 3 | ||
1139 | 4 | """Security policies for using content objects.""" | 4 | """Security policies for using content objects.""" |
1140 | @@ -83,6 +83,7 @@ | |||
1141 | 83 | ) | 83 | ) |
1142 | 84 | from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference | 84 | from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference |
1143 | 85 | from lp.code.interfaces.diff import IPreviewDiff | 85 | from lp.code.interfaces.diff import IPreviewDiff |
1144 | 86 | from lp.code.interfaces.gitcollection import IGitCollection | ||
1145 | 86 | from lp.code.interfaces.gitrepository import ( | 87 | from lp.code.interfaces.gitrepository import ( |
1146 | 87 | IGitRepository, | 88 | IGitRepository, |
1147 | 88 | user_has_special_git_repository_access, | 89 | user_has_special_git_repository_access, |
1148 | @@ -1020,6 +1021,12 @@ | |||
1149 | 1020 | if not mp.is_empty(): | 1021 | if not mp.is_empty(): |
1150 | 1021 | return True | 1022 | return True |
1151 | 1022 | 1023 | ||
1152 | 1024 | # Grant visibility to people who can see Git repositories owned | ||
1153 | 1025 | # by the private team. | ||
1154 | 1026 | team_repositories = IGitCollection(self.obj) | ||
1155 | 1027 | if not team_repositories.visibleByUser(user.person).is_empty(): | ||
1156 | 1028 | return True | ||
1157 | 1029 | |||
1158 | 1023 | # Grant visibility to users in a team that has the private team as | 1030 | # Grant visibility to users in a team that has the private team as |
1159 | 1024 | # a member, so that they can see the team properly in member | 1031 | # a member, so that they can see the team properly in member |
1160 | 1025 | # listings. | 1032 | # listings. |