Merge lp:~cjwatson/launchpad/git-defaults into lp:launchpad
- git-defaults
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | no longer in the source branch. |
Merged at revision: | 17363 |
Proposed branch: | lp:~cjwatson/launchpad/git-defaults |
Merge into: | lp:launchpad |
Diff against target: |
750 lines (+548/-39) 9 files modified
lib/lp/code/configure.zcml (+8/-1) lib/lp/code/errors.py (+2/-2) lib/lp/code/interfaces/defaultgit.py (+29/-0) lib/lp/code/interfaces/gitrepository.py (+65/-13) lib/lp/code/model/defaultgit.py (+113/-0) lib/lp/code/model/gitrepository.py (+74/-20) lib/lp/code/model/tests/test_gitrepository.py (+239/-3) lib/lp/registry/model/persondistributionsourcepackage.py (+9/-0) lib/lp/registry/model/personproduct.py (+9/-0) |
To merge this branch: | bzr merge lp:~cjwatson/launchpad/git-defaults |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email: mp+250474@code.launchpad.net |
Commit message
Implement target and owner-target default Git repositories.
Description of the change
Implement target and owner-target default Git repositories.
This presents two different layers because of the different ways we'll need to call this. For the webservice, it's simplest to just have {get,set}
I chose to have {get,set}
William Grant (wgrant) : | # |
Preview Diff
1 | === modified file 'lib/lp/code/configure.zcml' | |||
2 | --- lib/lp/code/configure.zcml 2015-02-19 23:57:34 +0000 | |||
3 | +++ lib/lp/code/configure.zcml 2015-02-26 14:40:55 +0000 | |||
4 | @@ -1,4 +1,4 @@ | |||
6 | 1 | <!-- Copyright 2009-2013 Canonical Ltd. This software is licensed under the | 1 | <!-- Copyright 2009-2015 Canonical Ltd. This software is licensed under the |
7 | 2 | GNU Affero General Public License version 3 (see the file LICENSE). | 2 | GNU Affero General Public License version 3 (see the file LICENSE). |
8 | 3 | --> | 3 | --> |
9 | 4 | 4 | ||
10 | @@ -858,6 +858,13 @@ | |||
11 | 858 | <allow interface="lp.code.interfaces.gitnamespace.IGitNamespaceSet" /> | 858 | <allow interface="lp.code.interfaces.gitnamespace.IGitNamespaceSet" /> |
12 | 859 | </securedutility> | 859 | </securedutility> |
13 | 860 | 860 | ||
14 | 861 | <!-- Default Git repositories --> | ||
15 | 862 | |||
16 | 863 | <adapter factory="lp.code.model.defaultgit.ProjectDefaultGitRepository" /> | ||
17 | 864 | <adapter factory="lp.code.model.defaultgit.PackageDefaultGitRepository" /> | ||
18 | 865 | <adapter factory="lp.code.model.defaultgit.OwnerProjectDefaultGitRepository" /> | ||
19 | 866 | <adapter factory="lp.code.model.defaultgit.OwnerPackageDefaultGitRepository" /> | ||
20 | 867 | |||
21 | 861 | <lp:help-folder folder="help" name="+help-code" /> | 868 | <lp:help-folder folder="help" name="+help-code" /> |
22 | 862 | 869 | ||
23 | 863 | <!-- Diffs --> | 870 | <!-- Diffs --> |
24 | 864 | 871 | ||
25 | === modified file 'lib/lp/code/errors.py' | |||
26 | --- lib/lp/code/errors.py 2015-02-11 17:10:09 +0000 | |||
27 | +++ lib/lp/code/errors.py 2015-02-26 14:40:55 +0000 | |||
28 | @@ -1,4 +1,4 @@ | |||
30 | 1 | # Copyright 2009-2013 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2009-2015 Canonical Ltd. This software is licensed under the |
31 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
32 | 3 | 3 | ||
33 | 4 | """Errors used in the lp/code modules.""" | 4 | """Errors used in the lp/code modules.""" |
34 | @@ -20,9 +20,9 @@ | |||
35 | 20 | 'BuildNotAllowedForDistro', | 20 | 'BuildNotAllowedForDistro', |
36 | 21 | 'BranchMergeProposalExists', | 21 | 'BranchMergeProposalExists', |
37 | 22 | 'CannotDeleteBranch', | 22 | 'CannotDeleteBranch', |
38 | 23 | 'CannotHaveLinkedBranch', | ||
39 | 23 | 'CannotUpgradeBranch', | 24 | 'CannotUpgradeBranch', |
40 | 24 | 'CannotUpgradeNonHosted', | 25 | 'CannotUpgradeNonHosted', |
41 | 25 | 'CannotHaveLinkedBranch', | ||
42 | 26 | 'CodeImportAlreadyRequested', | 26 | 'CodeImportAlreadyRequested', |
43 | 27 | 'CodeImportAlreadyRunning', | 27 | 'CodeImportAlreadyRunning', |
44 | 28 | 'CodeImportNotInReviewedState', | 28 | 'CodeImportNotInReviewedState', |
45 | 29 | 29 | ||
46 | === added file 'lib/lp/code/interfaces/defaultgit.py' | |||
47 | --- lib/lp/code/interfaces/defaultgit.py 1970-01-01 00:00:00 +0000 | |||
48 | +++ lib/lp/code/interfaces/defaultgit.py 2015-02-26 14:40:55 +0000 | |||
49 | @@ -0,0 +1,29 @@ | |||
50 | 1 | # Copyright 2015 Canonical Ltd. This software is licensed under the | ||
51 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
52 | 3 | |||
53 | 4 | """Interface for objects that have a default Git repository. | ||
54 | 5 | |||
55 | 6 | A default Git repository is a repository that's somehow officially related | ||
56 | 7 | to an object. It might be a project, a distribution source package, or a | ||
57 | 8 | combination of one of those with an owner to represent that owner's default | ||
58 | 9 | repository for that target. | ||
59 | 10 | """ | ||
60 | 11 | |||
61 | 12 | __metaclass__ = type | ||
62 | 13 | __all__ = [ | ||
63 | 14 | 'ICanHasDefaultGitRepository', | ||
64 | 15 | ] | ||
65 | 16 | |||
66 | 17 | from zope.interface import ( | ||
67 | 18 | Attribute, | ||
68 | 19 | Interface, | ||
69 | 20 | ) | ||
70 | 21 | |||
71 | 22 | |||
72 | 23 | class ICanHasDefaultGitRepository(Interface): | ||
73 | 24 | """Something that has a default Git repository.""" | ||
74 | 25 | |||
75 | 26 | context = Attribute("The object that can have a default Git repository.") | ||
76 | 27 | path = Attribute( | ||
77 | 28 | "The path for the default Git repository. " | ||
78 | 29 | "Note that this will be set even if there is no default repository.") | ||
79 | 0 | 30 | ||
80 | === modified file 'lib/lp/code/interfaces/gitrepository.py' | |||
81 | --- lib/lp/code/interfaces/gitrepository.py 2015-02-19 23:57:34 +0000 | |||
82 | +++ lib/lp/code/interfaces/gitrepository.py 2015-02-26 14:40:55 +0000 | |||
83 | @@ -16,6 +16,7 @@ | |||
84 | 16 | import re | 16 | import re |
85 | 17 | 17 | ||
86 | 18 | from lazr.restful.fields import Reference | 18 | from lazr.restful.fields import Reference |
87 | 19 | from zope.component import getUtility | ||
88 | 19 | from zope.interface import ( | 20 | from zope.interface import ( |
89 | 20 | Attribute, | 21 | Attribute, |
90 | 21 | Interface, | 22 | Interface, |
91 | @@ -32,7 +33,16 @@ | |||
92 | 32 | from lp import _ | 33 | from lp import _ |
93 | 33 | from lp.app.enums import InformationType | 34 | from lp.app.enums import InformationType |
94 | 34 | from lp.app.validators import LaunchpadValidationError | 35 | from lp.app.validators import LaunchpadValidationError |
95 | 36 | from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository | ||
96 | 35 | from lp.code.interfaces.hasgitrepositories import IHasGitRepositories | 37 | from lp.code.interfaces.hasgitrepositories import IHasGitRepositories |
97 | 38 | from lp.registry.interfaces.distributionsourcepackage import ( | ||
98 | 39 | IDistributionSourcePackage, | ||
99 | 40 | ) | ||
100 | 41 | from lp.registry.interfaces.persondistributionsourcepackage import ( | ||
101 | 42 | IPersonDistributionSourcePackageFactory, | ||
102 | 43 | ) | ||
103 | 44 | from lp.registry.interfaces.personproduct import IPersonProductFactory | ||
104 | 45 | from lp.registry.interfaces.product import IProduct | ||
105 | 36 | from lp.registry.interfaces.role import IPersonRoles | 46 | from lp.registry.interfaces.role import IPersonRoles |
106 | 37 | from lp.services.fields import ( | 47 | from lp.services.fields import ( |
107 | 38 | PersonChoice, | 48 | PersonChoice, |
108 | @@ -309,16 +319,44 @@ | |||
109 | 309 | Return None if no match was found. | 319 | Return None if no match was found. |
110 | 310 | """ | 320 | """ |
111 | 311 | 321 | ||
122 | 312 | def getDefaultRepository(target, owner=None): | 322 | def getDefaultRepository(target): |
123 | 313 | """Get the default repository for a target or owner-target. | 323 | """Get the default repository for a target. |
124 | 314 | 324 | ||
125 | 315 | :param target: An `IHasGitRepositories`. | 325 | :param target: An `IHasGitRepositories`. |
126 | 316 | :param owner: An `IPerson`, in which case search for that person's | 326 | |
127 | 317 | default repository for this target; or None, in which case | 327 | :raises GitTargetError: if `target` is an `IPerson`. |
128 | 318 | search for the overall default repository for this target. | 328 | :return: An `IGitRepository`, or None. |
129 | 319 | 329 | """ | |
130 | 320 | :raises GitTargetError: if `target` is an `IPerson`. | 330 | |
131 | 321 | :return: An `IGitRepository`, or None. | 331 | def getDefaultRepositoryForOwner(owner, target): |
132 | 332 | """Get a person's default repository for a target. | ||
133 | 333 | |||
134 | 334 | :param owner: An `IPerson`. | ||
135 | 335 | :param target: An `IHasGitRepositories`. | ||
136 | 336 | |||
137 | 337 | :raises GitTargetError: if `target` is an `IPerson`. | ||
138 | 338 | :return: An `IGitRepository`, or None. | ||
139 | 339 | """ | ||
140 | 340 | |||
141 | 341 | def setDefaultRepository(target, repository): | ||
142 | 342 | """Set the default repository for a target. | ||
143 | 343 | |||
144 | 344 | :param target: An `IHasGitRepositories`. | ||
145 | 345 | :param repository: An `IGitRepository`, or None to unset the default | ||
146 | 346 | repository. | ||
147 | 347 | |||
148 | 348 | :raises GitTargetError: if `target` is an `IPerson`. | ||
149 | 349 | """ | ||
150 | 350 | |||
151 | 351 | def setDefaultRepositoryForOwner(owner, target, repository): | ||
152 | 352 | """Set a person's default repository for a target. | ||
153 | 353 | |||
154 | 354 | :param owner: An `IPerson`. | ||
155 | 355 | :param target: An `IHasGitRepositories`. | ||
156 | 356 | :param repository: An `IGitRepository`, or None to unset the default | ||
157 | 357 | repository. | ||
158 | 358 | |||
159 | 359 | :raises GitTargetError: if `target` is an `IPerson`. | ||
160 | 322 | """ | 360 | """ |
161 | 323 | 361 | ||
162 | 324 | def getRepositories(): | 362 | def getRepositories(): |
163 | @@ -349,9 +387,23 @@ | |||
164 | 349 | 387 | ||
165 | 350 | def getRepositoryDefaults(self): | 388 | def getRepositoryDefaults(self): |
166 | 351 | """See `IGitRepository`.""" | 389 | """See `IGitRepository`.""" |
170 | 352 | # XXX cjwatson 2015-02-06: This will return shortcut defaults once | 390 | defaults = [] |
171 | 353 | # they're implemented. | 391 | if self.target_default: |
172 | 354 | return [] | 392 | defaults.append(ICanHasDefaultGitRepository(self.target)) |
173 | 393 | if self.owner_default: | ||
174 | 394 | if IProduct.providedBy(self.target): | ||
175 | 395 | factory = getUtility(IPersonProductFactory) | ||
176 | 396 | default = factory.create(self.owner, self.target) | ||
177 | 397 | elif IDistributionSourcePackage.providedBy(self.target): | ||
178 | 398 | factory = getUtility(IPersonDistributionSourcePackageFactory) | ||
179 | 399 | default = factory.create(self.owner, self.target) | ||
180 | 400 | else: | ||
181 | 401 | # Also enforced by database constraint. | ||
182 | 402 | raise AssertionError( | ||
183 | 403 | "Only projects or packages can have owner-target default " | ||
184 | 404 | "repositories.") | ||
185 | 405 | defaults.append(ICanHasDefaultGitRepository(default)) | ||
186 | 406 | return sorted(defaults) | ||
187 | 355 | 407 | ||
188 | 356 | def getRepositoryIdentities(self): | 408 | def getRepositoryIdentities(self): |
189 | 357 | """See `IGitRepository`.""" | 409 | """See `IGitRepository`.""" |
190 | 358 | 410 | ||
191 | === added file 'lib/lp/code/model/defaultgit.py' | |||
192 | --- lib/lp/code/model/defaultgit.py 1970-01-01 00:00:00 +0000 | |||
193 | +++ lib/lp/code/model/defaultgit.py 2015-02-26 14:40:55 +0000 | |||
194 | @@ -0,0 +1,113 @@ | |||
195 | 1 | # Copyright 2009-2015 Canonical Ltd. This software is licensed under the | ||
196 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
197 | 3 | |||
198 | 4 | """Implementation of `ICanHasDefaultGitRepository`.""" | ||
199 | 5 | |||
200 | 6 | __metaclass__ = type | ||
201 | 7 | # Don't export anything -- anything you need from this module you can get by | ||
202 | 8 | # adapting another object. | ||
203 | 9 | __all__ = [] | ||
204 | 10 | |||
205 | 11 | from zope.component import adapts | ||
206 | 12 | from zope.interface import implements | ||
207 | 13 | |||
208 | 14 | from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository | ||
209 | 15 | from lp.registry.interfaces.distributionsourcepackage import ( | ||
210 | 16 | IDistributionSourcePackage, | ||
211 | 17 | ) | ||
212 | 18 | from lp.registry.interfaces.persondistributionsourcepackage import ( | ||
213 | 19 | IPersonDistributionSourcePackage, | ||
214 | 20 | ) | ||
215 | 21 | from lp.registry.interfaces.personproduct import IPersonProduct | ||
216 | 22 | from lp.registry.interfaces.product import IProduct | ||
217 | 23 | |||
218 | 24 | |||
219 | 25 | class BaseDefaultGitRepository: | ||
220 | 26 | """Provides the common sorting algorithm.""" | ||
221 | 27 | |||
222 | 28 | def __cmp__(self, other): | ||
223 | 29 | if not ICanHasDefaultGitRepository.providedBy(other): | ||
224 | 30 | raise AssertionError("Can't compare with: %r" % other) | ||
225 | 31 | return cmp(self.sort_order, other.sort_order) | ||
226 | 32 | |||
227 | 33 | def __eq__(self, other): | ||
228 | 34 | return ( | ||
229 | 35 | isinstance(other, self.__class__) and | ||
230 | 36 | self.context == other.context) | ||
231 | 37 | |||
232 | 38 | def __ne__(self, other): | ||
233 | 39 | return not self == other | ||
234 | 40 | |||
235 | 41 | |||
236 | 42 | class ProjectDefaultGitRepository(BaseDefaultGitRepository): | ||
237 | 43 | """Implement a default Git repository for a project.""" | ||
238 | 44 | |||
239 | 45 | adapts(IProduct) | ||
240 | 46 | implements(ICanHasDefaultGitRepository) | ||
241 | 47 | |||
242 | 48 | sort_order = 0 | ||
243 | 49 | |||
244 | 50 | def __init__(self, project): | ||
245 | 51 | self.context = project | ||
246 | 52 | |||
247 | 53 | @property | ||
248 | 54 | def path(self): | ||
249 | 55 | """See `ICanHasDefaultGitRepository`.""" | ||
250 | 56 | return self.context.name | ||
251 | 57 | |||
252 | 58 | |||
253 | 59 | class PackageDefaultGitRepository(BaseDefaultGitRepository): | ||
254 | 60 | """Implement a default Git repository for a distribution source package.""" | ||
255 | 61 | |||
256 | 62 | adapts(IDistributionSourcePackage) | ||
257 | 63 | implements(ICanHasDefaultGitRepository) | ||
258 | 64 | |||
259 | 65 | sort_order = 0 | ||
260 | 66 | |||
261 | 67 | def __init__(self, distro_source_package): | ||
262 | 68 | self.context = distro_source_package | ||
263 | 69 | |||
264 | 70 | @property | ||
265 | 71 | def path(self): | ||
266 | 72 | """See `ICanHasDefaultGitRepository`.""" | ||
267 | 73 | return "%s/+source/%s" % ( | ||
268 | 74 | self.context.distribution.name, | ||
269 | 75 | self.context.sourcepackagename.name) | ||
270 | 76 | |||
271 | 77 | |||
272 | 78 | class OwnerProjectDefaultGitRepository(BaseDefaultGitRepository): | ||
273 | 79 | """Implement an owner's default Git repository for a project.""" | ||
274 | 80 | |||
275 | 81 | adapts(IPersonProduct) | ||
276 | 82 | implements(ICanHasDefaultGitRepository) | ||
277 | 83 | |||
278 | 84 | sort_order = 1 | ||
279 | 85 | |||
280 | 86 | def __init__(self, person_project): | ||
281 | 87 | self.context = person_project | ||
282 | 88 | |||
283 | 89 | @property | ||
284 | 90 | def path(self): | ||
285 | 91 | """See `ICanHasDefaultGitRepository`.""" | ||
286 | 92 | return "~%s/%s" % (self.context.person.name, self.context.product.name) | ||
287 | 93 | |||
288 | 94 | |||
289 | 95 | class OwnerPackageDefaultGitRepository(BaseDefaultGitRepository): | ||
290 | 96 | """Implement an owner's default Git repository for a distribution source | ||
291 | 97 | package.""" | ||
292 | 98 | |||
293 | 99 | adapts(IPersonDistributionSourcePackage) | ||
294 | 100 | implements(ICanHasDefaultGitRepository) | ||
295 | 101 | |||
296 | 102 | sort_order = 1 | ||
297 | 103 | |||
298 | 104 | def __init__(self, person_distro_source_package): | ||
299 | 105 | self.context = person_distro_source_package | ||
300 | 106 | |||
301 | 107 | @property | ||
302 | 108 | def path(self): | ||
303 | 109 | """See `ICanHasDefaultGitRepository`.""" | ||
304 | 110 | dsp = self.context.distro_source_package | ||
305 | 111 | return "~%s/%s/+source/%s" % ( | ||
306 | 112 | self.context.person.name, dsp.distribution.name, | ||
307 | 113 | dsp.sourcepackagename.name) | ||
308 | 0 | 114 | ||
309 | === modified file 'lib/lp/code/model/gitrepository.py' | |||
310 | --- lib/lp/code/model/gitrepository.py 2015-02-19 23:57:34 +0000 | |||
311 | +++ lib/lp/code/model/gitrepository.py 2015-02-26 14:40:55 +0000 | |||
312 | @@ -83,6 +83,7 @@ | |||
313 | 83 | ArrayIntersects, | 83 | ArrayIntersects, |
314 | 84 | ) | 84 | ) |
315 | 85 | from lp.services.propertycache import cachedproperty | 85 | from lp.services.propertycache import cachedproperty |
316 | 86 | from lp.services.webapp.authorization import available_with_permission | ||
317 | 86 | 87 | ||
318 | 87 | 88 | ||
319 | 88 | def git_repository_modified(repository, event): | 89 | def git_repository_modified(repository, event): |
320 | @@ -206,8 +207,9 @@ | |||
321 | 206 | """See `IGitRepository`.""" | 207 | """See `IGitRepository`.""" |
322 | 207 | if value: | 208 | if value: |
323 | 208 | # Check for an existing owner-target default. | 209 | # Check for an existing owner-target default. |
326 | 209 | existing = getUtility(IGitRepositorySet).getDefaultRepository( | 210 | repository_set = getUtility(IGitRepositorySet) |
327 | 210 | self.target, owner=self.owner) | 211 | existing = repository_set.getDefaultRepositoryForOwner( |
328 | 212 | self.owner, self.target) | ||
329 | 211 | if existing is not None: | 213 | if existing is not None: |
330 | 212 | raise GitDefaultConflict( | 214 | raise GitDefaultConflict( |
331 | 213 | existing, self.target, owner=self.owner) | 215 | existing, self.target, owner=self.owner) |
332 | @@ -358,24 +360,76 @@ | |||
333 | 358 | # XXX cjwatson 2015-02-06: Fill this in once IGitLookup is in place. | 360 | # XXX cjwatson 2015-02-06: Fill this in once IGitLookup is in place. |
334 | 359 | raise NotImplementedError | 361 | raise NotImplementedError |
335 | 360 | 362 | ||
354 | 361 | def getDefaultRepository(self, target, owner=None): | 363 | def getDefaultRepository(self, target): |
355 | 362 | """See `IGitRepositorySet`.""" | 364 | """See `IGitRepositorySet`.""" |
356 | 363 | clauses = [] | 365 | clauses = [GitRepository.target_default == True] |
357 | 364 | if IProduct.providedBy(target): | 366 | if IProduct.providedBy(target): |
358 | 365 | clauses.append(GitRepository.project == target) | 367 | clauses.append(GitRepository.project == target) |
359 | 366 | elif IDistributionSourcePackage.providedBy(target): | 368 | elif IDistributionSourcePackage.providedBy(target): |
360 | 367 | clauses.append(GitRepository.distribution == target.distribution) | 369 | clauses.append(GitRepository.distribution == target.distribution) |
361 | 368 | clauses.append( | 370 | clauses.append( |
362 | 369 | GitRepository.sourcepackagename == target.sourcepackagename) | 371 | GitRepository.sourcepackagename == target.sourcepackagename) |
363 | 370 | else: | 372 | else: |
364 | 371 | raise GitTargetError( | 373 | raise GitTargetError( |
365 | 372 | "Personal repositories cannot be defaults for any target.") | 374 | "Personal repositories cannot be defaults for any target.") |
366 | 373 | if owner is not None: | 375 | return IStore(GitRepository).find(GitRepository, *clauses).one() |
367 | 374 | clauses.append(GitRepository.owner == owner) | 376 | |
368 | 375 | clauses.append(GitRepository.owner_default == True) | 377 | def getDefaultRepositoryForOwner(self, owner, target): |
369 | 376 | else: | 378 | """See `IGitRepositorySet`.""" |
370 | 377 | clauses.append(GitRepository.target_default == True) | 379 | clauses = [ |
371 | 378 | return IStore(GitRepository).find(GitRepository, *clauses).one() | 380 | GitRepository.owner == owner, |
372 | 381 | GitRepository.owner_default == True, | ||
373 | 382 | ] | ||
374 | 383 | if IProduct.providedBy(target): | ||
375 | 384 | clauses.append(GitRepository.project == target) | ||
376 | 385 | elif IDistributionSourcePackage.providedBy(target): | ||
377 | 386 | clauses.append(GitRepository.distribution == target.distribution) | ||
378 | 387 | clauses.append( | ||
379 | 388 | GitRepository.sourcepackagename == target.sourcepackagename) | ||
380 | 389 | else: | ||
381 | 390 | raise GitTargetError( | ||
382 | 391 | "Personal repositories cannot be defaults for any target.") | ||
383 | 392 | return IStore(GitRepository).find(GitRepository, *clauses).one() | ||
384 | 393 | |||
385 | 394 | @available_with_permission('launchpad.Edit', 'target') | ||
386 | 395 | def setDefaultRepository(self, target, repository): | ||
387 | 396 | """See `IGitRepositorySet`.""" | ||
388 | 397 | if IPerson.providedBy(target): | ||
389 | 398 | raise GitTargetError( | ||
390 | 399 | "Cannot set a default Git repository for a person, only " | ||
391 | 400 | "for a project or a package.") | ||
392 | 401 | if repository is not None: | ||
393 | 402 | if repository.target != target: | ||
394 | 403 | raise GitTargetError( | ||
395 | 404 | "Cannot set default Git repository to one attached to " | ||
396 | 405 | "another target.") | ||
397 | 406 | repository.setTargetDefault(True) | ||
398 | 407 | else: | ||
399 | 408 | previous = self.getDefaultRepository(target) | ||
400 | 409 | if previous is not None: | ||
401 | 410 | previous.setTargetDefault(False) | ||
402 | 411 | |||
403 | 412 | @available_with_permission('launchpad.Edit', 'owner') | ||
404 | 413 | def setDefaultRepositoryForOwner(self, owner, target, repository): | ||
405 | 414 | """See `IGitRepositorySet`.""" | ||
406 | 415 | if IPerson.providedBy(target): | ||
407 | 416 | raise GitTargetError( | ||
408 | 417 | "Cannot set a default Git repository for a person, only " | ||
409 | 418 | "for a project or a package.") | ||
410 | 419 | if repository is not None: | ||
411 | 420 | if repository.target != target: | ||
412 | 421 | raise GitTargetError( | ||
413 | 422 | "Cannot set default Git repository to one attached to " | ||
414 | 423 | "another target.") | ||
415 | 424 | if repository.owner != owner: | ||
416 | 425 | raise GitTargetError( | ||
417 | 426 | "Cannot set a person's default Git repository to one " | ||
418 | 427 | "owned by somebody else.") | ||
419 | 428 | repository.setOwnerDefault(True) | ||
420 | 429 | else: | ||
421 | 430 | previous = self.getDefaultRepositoryForOwner(owner, target) | ||
422 | 431 | if previous is not None: | ||
423 | 432 | previous.setOwnerDefault(False) | ||
424 | 379 | 433 | ||
425 | 380 | def getRepositories(self): | 434 | def getRepositories(self): |
426 | 381 | """See `IGitRepositorySet`.""" | 435 | """See `IGitRepositorySet`.""" |
427 | 382 | 436 | ||
428 | === modified file 'lib/lp/code/model/tests/test_gitrepository.py' | |||
429 | --- lib/lp/code/model/tests/test_gitrepository.py 2015-02-20 00:56:57 +0000 | |||
430 | +++ lib/lp/code/model/tests/test_gitrepository.py 2015-02-26 14:40:55 +0000 | |||
431 | @@ -6,6 +6,7 @@ | |||
432 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
433 | 7 | 7 | ||
434 | 8 | from datetime import datetime | 8 | from datetime import datetime |
435 | 9 | from functools import partial | ||
436 | 9 | 10 | ||
437 | 10 | from lazr.lifecycle.event import ObjectModifiedEvent | 11 | from lazr.lifecycle.event import ObjectModifiedEvent |
438 | 11 | import pytz | 12 | import pytz |
439 | @@ -24,12 +25,20 @@ | |||
440 | 24 | GitRepositoryCreatorNotOwner, | 25 | GitRepositoryCreatorNotOwner, |
441 | 25 | GitTargetError, | 26 | GitTargetError, |
442 | 26 | ) | 27 | ) |
443 | 28 | from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository | ||
444 | 27 | from lp.code.interfaces.gitnamespace import ( | 29 | from lp.code.interfaces.gitnamespace import ( |
445 | 28 | IGitNamespacePolicy, | 30 | IGitNamespacePolicy, |
446 | 29 | IGitNamespaceSet, | 31 | IGitNamespaceSet, |
447 | 30 | ) | 32 | ) |
449 | 31 | from lp.code.interfaces.gitrepository import IGitRepository | 33 | from lp.code.interfaces.gitrepository import ( |
450 | 34 | IGitRepository, | ||
451 | 35 | IGitRepositorySet, | ||
452 | 36 | ) | ||
453 | 32 | from lp.registry.enums import BranchSharingPolicy | 37 | from lp.registry.enums import BranchSharingPolicy |
454 | 38 | from lp.registry.interfaces.persondistributionsourcepackage import ( | ||
455 | 39 | IPersonDistributionSourcePackageFactory, | ||
456 | 40 | ) | ||
457 | 41 | from lp.registry.interfaces.personproduct import IPersonProductFactory | ||
458 | 33 | from lp.services.database.constants import UTC_NOW | 42 | from lp.services.database.constants import UTC_NOW |
459 | 34 | from lp.services.webapp.authorization import check_permission | 43 | from lp.services.webapp.authorization import check_permission |
460 | 35 | from lp.testing import ( | 44 | from lp.testing import ( |
461 | @@ -96,6 +105,10 @@ | |||
462 | 96 | 105 | ||
463 | 97 | layer = DatabaseFunctionalLayer | 106 | layer = DatabaseFunctionalLayer |
464 | 98 | 107 | ||
465 | 108 | def setUp(self): | ||
466 | 109 | super(TestGitIdentityMixin, self).setUp() | ||
467 | 110 | self.repository_set = getUtility(IGitRepositorySet) | ||
468 | 111 | |||
469 | 99 | def assertGitIdentity(self, repository, identity_path): | 112 | def assertGitIdentity(self, repository, identity_path): |
470 | 100 | """Assert that the Git identity of 'repository' is 'identity_path'. | 113 | """Assert that the Git identity of 'repository' is 'identity_path'. |
471 | 101 | 114 | ||
472 | @@ -111,6 +124,53 @@ | |||
473 | 111 | repository = self.factory.makeGitRepository() | 124 | repository = self.factory.makeGitRepository() |
474 | 112 | self.assertGitIdentity(repository, repository.unique_name) | 125 | self.assertGitIdentity(repository, repository.unique_name) |
475 | 113 | 126 | ||
476 | 127 | def test_git_identity_default_for_project(self): | ||
477 | 128 | # If a repository is the default for a project, then its Git | ||
478 | 129 | # identity is the project name. | ||
479 | 130 | project = self.factory.makeProduct() | ||
480 | 131 | repository = self.factory.makeGitRepository( | ||
481 | 132 | owner=project.owner, target=project) | ||
482 | 133 | with person_logged_in(project.owner): | ||
483 | 134 | self.repository_set.setDefaultRepository(project, repository) | ||
484 | 135 | self.assertGitIdentity(repository, project.name) | ||
485 | 136 | |||
486 | 137 | def test_git_identity_default_for_package(self): | ||
487 | 138 | # If a repository is the default for a package, then its Git | ||
488 | 139 | # identity uses the path to that package. | ||
489 | 140 | dsp = self.factory.makeDistributionSourcePackage() | ||
490 | 141 | repository = self.factory.makeGitRepository(target=dsp) | ||
491 | 142 | with admin_logged_in(): | ||
492 | 143 | self.repository_set.setDefaultRepository(dsp, repository) | ||
493 | 144 | self.assertGitIdentity( | ||
494 | 145 | repository, | ||
495 | 146 | "%s/+source/%s" % ( | ||
496 | 147 | dsp.distribution.name, dsp.sourcepackagename.name)) | ||
497 | 148 | |||
498 | 149 | def test_git_identity_owner_default_for_project(self): | ||
499 | 150 | # If a repository is a person's default for a project, then its Git | ||
500 | 151 | # identity is a combination of the person and project names. | ||
501 | 152 | project = self.factory.makeProduct() | ||
502 | 153 | repository = self.factory.makeGitRepository(target=project) | ||
503 | 154 | with person_logged_in(repository.owner): | ||
504 | 155 | self.repository_set.setDefaultRepositoryForOwner( | ||
505 | 156 | repository.owner, project, repository) | ||
506 | 157 | self.assertGitIdentity( | ||
507 | 158 | repository, "~%s/%s" % (repository.owner.name, project.name)) | ||
508 | 159 | |||
509 | 160 | def test_git_identity_owner_default_for_package(self): | ||
510 | 161 | # If a repository is a person's default for a package, then its Git | ||
511 | 162 | # identity is a combination of the person name and the package path. | ||
512 | 163 | dsp = self.factory.makeDistributionSourcePackage() | ||
513 | 164 | repository = self.factory.makeGitRepository(target=dsp) | ||
514 | 165 | with person_logged_in(repository.owner): | ||
515 | 166 | self.repository_set.setDefaultRepositoryForOwner( | ||
516 | 167 | repository.owner, dsp, repository) | ||
517 | 168 | self.assertGitIdentity( | ||
518 | 169 | repository, | ||
519 | 170 | "~%s/%s/+source/%s" % ( | ||
520 | 171 | repository.owner.name, dsp.distribution.name, | ||
521 | 172 | dsp.sourcepackagename.name)) | ||
522 | 173 | |||
523 | 114 | def test_identities_no_defaults(self): | 174 | def test_identities_no_defaults(self): |
524 | 115 | # If there are no defaults, the only repository identity is the | 175 | # If there are no defaults, the only repository identity is the |
525 | 116 | # unique name. | 176 | # unique name. |
526 | @@ -119,8 +179,54 @@ | |||
527 | 119 | [(repository.unique_name, repository)], | 179 | [(repository.unique_name, repository)], |
528 | 120 | repository.getRepositoryIdentities()) | 180 | repository.getRepositoryIdentities()) |
529 | 121 | 181 | ||
532 | 122 | # XXX cjwatson 2015-02-12: This will need to be expanded once support | 182 | def test_default_for_project(self): |
533 | 123 | # for default repositories is in place. | 183 | # If a repository is the default for a project, then that is the |
534 | 184 | # preferred identity. Target defaults are preferred over | ||
535 | 185 | # owner-target defaults. | ||
536 | 186 | eric = self.factory.makePerson(name="eric") | ||
537 | 187 | fooix = self.factory.makeProduct(name="fooix", owner=eric) | ||
538 | 188 | repository = self.factory.makeGitRepository( | ||
539 | 189 | owner=eric, target=fooix, name=u"fooix-repo") | ||
540 | 190 | with person_logged_in(fooix.owner): | ||
541 | 191 | self.repository_set.setDefaultRepositoryForOwner( | ||
542 | 192 | repository.owner, fooix, repository) | ||
543 | 193 | self.repository_set.setDefaultRepository(fooix, repository) | ||
544 | 194 | eric_fooix = getUtility(IPersonProductFactory).create(eric, fooix) | ||
545 | 195 | self.assertEqual( | ||
546 | 196 | [ICanHasDefaultGitRepository(target) | ||
547 | 197 | for target in (fooix, eric_fooix)], | ||
548 | 198 | repository.getRepositoryDefaults()) | ||
549 | 199 | self.assertEqual( | ||
550 | 200 | [("fooix", fooix), ("~eric/fooix", eric_fooix), | ||
551 | 201 | ("~eric/fooix/+git/fooix-repo", repository)], | ||
552 | 202 | repository.getRepositoryIdentities()) | ||
553 | 203 | |||
554 | 204 | def test_default_for_package(self): | ||
555 | 205 | # If a repository is the default for a package, then that is the | ||
556 | 206 | # preferred identity. Target defaults are preferred over | ||
557 | 207 | # owner-target defaults. | ||
558 | 208 | mint = self.factory.makeDistribution(name="mint") | ||
559 | 209 | eric = self.factory.makePerson(name="eric") | ||
560 | 210 | mint_choc = self.factory.makeDistributionSourcePackage( | ||
561 | 211 | distribution=mint, sourcepackagename="choc") | ||
562 | 212 | repository = self.factory.makeGitRepository( | ||
563 | 213 | owner=eric, target=mint_choc, name=u"choc-repo") | ||
564 | 214 | dsp = repository.target | ||
565 | 215 | with admin_logged_in(): | ||
566 | 216 | self.repository_set.setDefaultRepositoryForOwner( | ||
567 | 217 | repository.owner, dsp, repository) | ||
568 | 218 | self.repository_set.setDefaultRepository(dsp, repository) | ||
569 | 219 | eric_dsp = getUtility(IPersonDistributionSourcePackageFactory).create( | ||
570 | 220 | eric, dsp) | ||
571 | 221 | self.assertEqual( | ||
572 | 222 | [ICanHasDefaultGitRepository(target) | ||
573 | 223 | for target in (dsp, eric_dsp)], | ||
574 | 224 | repository.getRepositoryDefaults()) | ||
575 | 225 | self.assertEqual( | ||
576 | 226 | [("mint/+source/choc", dsp), | ||
577 | 227 | ("~eric/mint/+source/choc", eric_dsp), | ||
578 | 228 | ("~eric/mint/+source/choc/+git/choc-repo", repository)], | ||
579 | 229 | repository.getRepositoryIdentities()) | ||
580 | 124 | 230 | ||
581 | 125 | 231 | ||
582 | 126 | class TestGitRepositoryDateLastModified(TestCaseWithFactory): | 232 | class TestGitRepositoryDateLastModified(TestCaseWithFactory): |
583 | @@ -392,3 +498,133 @@ | |||
584 | 392 | self.assertRaises( | 498 | self.assertRaises( |
585 | 393 | GitTargetError, repository.setTarget, | 499 | GitTargetError, repository.setTarget, |
586 | 394 | target=commercial_project, user=owner) | 500 | target=commercial_project, user=owner) |
587 | 501 | |||
588 | 502 | |||
589 | 503 | class TestGitRepositorySet(TestCaseWithFactory): | ||
590 | 504 | |||
591 | 505 | layer = DatabaseFunctionalLayer | ||
592 | 506 | |||
593 | 507 | def setUp(self): | ||
594 | 508 | super(TestGitRepositorySet, self).setUp() | ||
595 | 509 | self.repository_set = getUtility(IGitRepositorySet) | ||
596 | 510 | |||
597 | 511 | def test_provides_IGitRepositorySet(self): | ||
598 | 512 | # GitRepositorySet instances provide IGitRepositorySet. | ||
599 | 513 | verifyObject(IGitRepositorySet, self.repository_set) | ||
600 | 514 | |||
601 | 515 | def test_setDefaultRepository_refuses_person(self): | ||
602 | 516 | # setDefaultRepository refuses if the target is a person. | ||
603 | 517 | person = self.factory.makePerson() | ||
604 | 518 | repository = self.factory.makeGitRepository(owner=person) | ||
605 | 519 | with person_logged_in(person): | ||
606 | 520 | self.assertRaises( | ||
607 | 521 | GitTargetError, self.repository_set.setDefaultRepository, | ||
608 | 522 | person, repository) | ||
609 | 523 | |||
610 | 524 | def test_setDefaultRepositoryForOwner_refuses_person(self): | ||
611 | 525 | # setDefaultRepositoryForOwner refuses if the target is a person. | ||
612 | 526 | person = self.factory.makePerson() | ||
613 | 527 | repository = self.factory.makeGitRepository(owner=person) | ||
614 | 528 | with person_logged_in(person): | ||
615 | 529 | self.assertRaises( | ||
616 | 530 | GitTargetError, | ||
617 | 531 | self.repository_set.setDefaultRepositoryForOwner, | ||
618 | 532 | person, person, repository) | ||
619 | 533 | |||
620 | 534 | |||
621 | 535 | class TestGitRepositorySetDefaultsMixin: | ||
622 | 536 | |||
623 | 537 | layer = DatabaseFunctionalLayer | ||
624 | 538 | |||
625 | 539 | def setUp(self): | ||
626 | 540 | super(TestGitRepositorySetDefaultsMixin, self).setUp() | ||
627 | 541 | self.repository_set = getUtility(IGitRepositorySet) | ||
628 | 542 | self.get_method = self.repository_set.getDefaultRepository | ||
629 | 543 | self.set_method = self.repository_set.setDefaultRepository | ||
630 | 544 | |||
631 | 545 | def makeGitRepository(self, target): | ||
632 | 546 | return self.factory.makeGitRepository(target=target) | ||
633 | 547 | |||
634 | 548 | def test_default_repository_round_trip(self): | ||
635 | 549 | # A target's default Git repository set using setDefaultRepository* | ||
636 | 550 | # can be retrieved using getDefaultRepository*. | ||
637 | 551 | target = self.makeTarget() | ||
638 | 552 | repository = self.makeGitRepository(target) | ||
639 | 553 | self.assertIsNone(self.get_method(target)) | ||
640 | 554 | with person_logged_in(self.getPersonForLogin(target)): | ||
641 | 555 | self.set_method(target, repository) | ||
642 | 556 | self.assertEqual(repository, self.get_method(target)) | ||
643 | 557 | |||
644 | 558 | def test_set_default_repository_None(self): | ||
645 | 559 | # setDefaultRepository*(target, None) clears the default. | ||
646 | 560 | target = self.makeTarget() | ||
647 | 561 | repository = self.makeGitRepository(target) | ||
648 | 562 | with person_logged_in(self.getPersonForLogin(target)): | ||
649 | 563 | self.set_method(target, repository) | ||
650 | 564 | self.set_method(target, None) | ||
651 | 565 | self.assertIsNone(self.get_method(target)) | ||
652 | 566 | |||
653 | 567 | def test_set_default_repository_different_target(self): | ||
654 | 568 | # setDefaultRepository* refuses if the repository is attached to a | ||
655 | 569 | # different target. | ||
656 | 570 | target = self.makeTarget() | ||
657 | 571 | other_target = self.makeTarget(template=target) | ||
658 | 572 | repository = self.makeGitRepository(other_target) | ||
659 | 573 | with person_logged_in(self.getPersonForLogin(target)): | ||
660 | 574 | self.assertRaises( | ||
661 | 575 | GitTargetError, self.set_method, target, repository) | ||
662 | 576 | |||
663 | 577 | |||
664 | 578 | class TestGitRepositorySetDefaultsProject( | ||
665 | 579 | TestGitRepositorySetDefaultsMixin, TestCaseWithFactory): | ||
666 | 580 | |||
667 | 581 | def makeTarget(self, template=None): | ||
668 | 582 | return self.factory.makeProduct() | ||
669 | 583 | |||
670 | 584 | @staticmethod | ||
671 | 585 | def getPersonForLogin(target): | ||
672 | 586 | return target.owner | ||
673 | 587 | |||
674 | 588 | |||
675 | 589 | class TestGitRepositorySetDefaultsPackage( | ||
676 | 590 | TestGitRepositorySetDefaultsMixin, TestCaseWithFactory): | ||
677 | 591 | |||
678 | 592 | def makeTarget(self, template=None): | ||
679 | 593 | kwargs = {} | ||
680 | 594 | if template is not None: | ||
681 | 595 | kwargs["distribution"] = template.distribution | ||
682 | 596 | return self.factory.makeDistributionSourcePackage(**kwargs) | ||
683 | 597 | |||
684 | 598 | @staticmethod | ||
685 | 599 | def getPersonForLogin(target): | ||
686 | 600 | return target.distribution.owner | ||
687 | 601 | |||
688 | 602 | |||
689 | 603 | class TestGitRepositorySetDefaultsOwnerMixin( | ||
690 | 604 | TestGitRepositorySetDefaultsMixin): | ||
691 | 605 | |||
692 | 606 | def setUp(self): | ||
693 | 607 | super(TestGitRepositorySetDefaultsOwnerMixin, self).setUp() | ||
694 | 608 | self.person = self.factory.makePerson() | ||
695 | 609 | self.get_method = partial( | ||
696 | 610 | self.repository_set.getDefaultRepositoryForOwner, self.person) | ||
697 | 611 | self.set_method = partial( | ||
698 | 612 | self.repository_set.setDefaultRepositoryForOwner, self.person) | ||
699 | 613 | |||
700 | 614 | def makeGitRepository(self, target): | ||
701 | 615 | return self.factory.makeGitRepository(owner=self.person, target=target) | ||
702 | 616 | |||
703 | 617 | def getPersonForLogin(self, target): | ||
704 | 618 | return self.person | ||
705 | 619 | |||
706 | 620 | |||
707 | 621 | class TestGitRepositorySetDefaultsOwnerProject( | ||
708 | 622 | TestGitRepositorySetDefaultsOwnerMixin, | ||
709 | 623 | TestGitRepositorySetDefaultsProject): | ||
710 | 624 | pass | ||
711 | 625 | |||
712 | 626 | |||
713 | 627 | class TestGitRepositorySetDefaultsOwnerPackage( | ||
714 | 628 | TestGitRepositorySetDefaultsOwnerMixin, | ||
715 | 629 | TestGitRepositorySetDefaultsPackage): | ||
716 | 630 | pass | ||
717 | 395 | 631 | ||
718 | === modified file 'lib/lp/registry/model/persondistributionsourcepackage.py' | |||
719 | --- lib/lp/registry/model/persondistributionsourcepackage.py 2015-02-06 13:37:58 +0000 | |||
720 | +++ lib/lp/registry/model/persondistributionsourcepackage.py 2015-02-26 14:40:55 +0000 | |||
721 | @@ -37,3 +37,12 @@ | |||
722 | 37 | def displayname(self): | 37 | def displayname(self): |
723 | 38 | return '%s in %s' % ( | 38 | return '%s in %s' % ( |
724 | 39 | self.person.displayname, self.distro_source_package.displayname) | 39 | self.person.displayname, self.distro_source_package.displayname) |
725 | 40 | |||
726 | 41 | def __eq__(self, other): | ||
727 | 42 | return ( | ||
728 | 43 | IPersonDistributionSourcePackage.providedBy(other) and | ||
729 | 44 | self.person.id == other.person.id and | ||
730 | 45 | self.distro_source_package == other.distro_source_package) | ||
731 | 46 | |||
732 | 47 | def __ne__(self, other): | ||
733 | 48 | return not self == other | ||
734 | 40 | 49 | ||
735 | === modified file 'lib/lp/registry/model/personproduct.py' | |||
736 | --- lib/lp/registry/model/personproduct.py 2011-12-19 23:38:16 +0000 | |||
737 | +++ lib/lp/registry/model/personproduct.py 2015-02-26 14:40:55 +0000 | |||
738 | @@ -38,3 +38,12 @@ | |||
739 | 38 | def displayname(self): | 38 | def displayname(self): |
740 | 39 | return '%s in %s' % ( | 39 | return '%s in %s' % ( |
741 | 40 | self.person.displayname, self.product.displayname) | 40 | self.person.displayname, self.product.displayname) |
742 | 41 | |||
743 | 42 | def __eq__(self, other): | ||
744 | 43 | return ( | ||
745 | 44 | IPersonProduct.providedBy(other) and | ||
746 | 45 | self.person.id == other.person.id and | ||
747 | 46 | self.product.id == other.product.id) | ||
748 | 47 | |||
749 | 48 | def __ne__(self, other): | ||
750 | 49 | return not self == other |
I'm not sure about the target. getDefaultGitRe pository/ setDefaultGitRe pository design. They're yet more domain-specific methods on already huge Registry classes, which just isn't maintainable long-term.
During Disclosure we experimented with SharingService to stop the bloat, which ended up hideous more because of the different artifact types than because of a fundamental issue with the service concept.