Merge lp:~abentley/launchpad/push-creates-package into lp:launchpad
- push-creates-package
- Merge into devel
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 13690 | ||||
Proposed branch: | lp:~abentley/launchpad/push-creates-package | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
543 lines (+162/-40) 15 files modified
lib/canonical/launchpad/xmlrpc/configure.zcml (+3/-0) lib/canonical/launchpad/xmlrpc/faults.py (+11/-0) lib/lp/code/tests/helpers.py (+14/-0) lib/lp/code/xmlrpc/codehosting.py (+11/-0) lib/lp/code/xmlrpc/tests/test_codehosting.py (+25/-5) lib/lp/codehosting/inmemory.py (+21/-8) lib/lp/codehosting/tests/test_acceptance.py (+19/-9) lib/lp/codehosting/vfs/branchfs.py (+2/-5) lib/lp/codehosting/vfs/tests/test_branchfs.py (+13/-2) lib/lp/registry/errors.py (+5/-0) lib/lp/registry/interfaces/person.py (+4/-8) lib/lp/registry/model/person.py (+1/-1) lib/lp/registry/model/sourcepackagename.py (+8/-1) lib/lp/registry/tests/test_person.py (+1/-1) lib/lp/registry/tests/test_sourcepackagename.py (+24/-0) |
||||
To merge this branch: | bzr merge lp:~abentley/launchpad/push-creates-package | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Abel Deuring (community) | code | Approve | |
Review via email: mp+71366@code.launchpad.net |
Commit message
Push creates source package names.
Description of the change
= Summary =
Fix bug #386596: pushing to a packaging branch can't create a new package
== Proposed fix ==
Support creating new source package names in createBranch
== Pre-implementation notes ==
None
== Implementation details ==
In the xmlrpc-backed version, catch NoSuchSourcePac
In the memory-backed version, create the sourcepackagename if not already present.
In both cases, SourcePackageNa
createBranch translates InvalidName to the new InvalidSourcePa
The vfs transport converts InvalidSourcePa
== Tests ==
bin/test -t test_createBran
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/canonical
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Abel Deuring (adeuring) : | # |
Preview Diff
1 | === modified file 'lib/canonical/launchpad/xmlrpc/configure.zcml' | |||
2 | --- lib/canonical/launchpad/xmlrpc/configure.zcml 2010-11-08 14:16:17 +0000 | |||
3 | +++ lib/canonical/launchpad/xmlrpc/configure.zcml 2011-08-15 13:18:36 +0000 | |||
4 | @@ -207,4 +207,7 @@ | |||
5 | 207 | <class class="canonical.launchpad.xmlrpc.faults.AccountSuspended"> | 207 | <class class="canonical.launchpad.xmlrpc.faults.AccountSuspended"> |
6 | 208 | <require like_class="xmlrpclib.Fault" /> | 208 | <require like_class="xmlrpclib.Fault" /> |
7 | 209 | </class> | 209 | </class> |
8 | 210 | <class class="canonical.launchpad.xmlrpc.faults.InvalidSourcePackageName"> | ||
9 | 211 | <require like_class="xmlrpclib.Fault" /> | ||
10 | 212 | </class> | ||
11 | 210 | </configure> | 213 | </configure> |
12 | 211 | 214 | ||
13 | === modified file 'lib/canonical/launchpad/xmlrpc/faults.py' | |||
14 | --- lib/canonical/launchpad/xmlrpc/faults.py 2010-12-12 23:09:36 +0000 | |||
15 | +++ lib/canonical/launchpad/xmlrpc/faults.py 2011-08-15 13:18:36 +0000 | |||
16 | @@ -22,6 +22,7 @@ | |||
17 | 22 | 'InvalidBranchUniqueName', | 22 | 'InvalidBranchUniqueName', |
18 | 23 | 'InvalidProductIdentifier', | 23 | 'InvalidProductIdentifier', |
19 | 24 | 'InvalidBranchUrl', | 24 | 'InvalidBranchUrl', |
20 | 25 | 'InvalidSourcePackageName', | ||
21 | 25 | 'OopsOccurred', | 26 | 'OopsOccurred', |
22 | 26 | 'NoBranchWithID', | 27 | 'NoBranchWithID', |
23 | 27 | 'NoLinkedBranch', | 28 | 'NoLinkedBranch', |
24 | @@ -476,3 +477,13 @@ | |||
25 | 476 | def __init__(self, server_op, oopsid): | 477 | def __init__(self, server_op, oopsid): |
26 | 477 | LaunchpadFault.__init__(self, server_op=server_op, oopsid=oopsid) | 478 | LaunchpadFault.__init__(self, server_op=server_op, oopsid=oopsid) |
27 | 478 | self.oopsid = oopsid | 479 | self.oopsid = oopsid |
28 | 480 | |||
29 | 481 | |||
30 | 482 | class InvalidSourcePackageName(LaunchpadFault): | ||
31 | 483 | |||
32 | 484 | error_code = 390 | ||
33 | 485 | msg_template = ("%(name)s is not a valid source package name.") | ||
34 | 486 | |||
35 | 487 | def __init__(self, name): | ||
36 | 488 | self.name = name | ||
37 | 489 | LaunchpadFault.__init__(self, name=name) | ||
38 | 479 | 490 | ||
39 | === modified file 'lib/lp/code/tests/helpers.py' | |||
40 | --- lib/lp/code/tests/helpers.py 2011-08-03 11:00:11 +0000 | |||
41 | +++ lib/lp/code/tests/helpers.py 2011-08-15 13:18:36 +0000 | |||
42 | @@ -6,6 +6,7 @@ | |||
43 | 6 | __metaclass__ = type | 6 | __metaclass__ = type |
44 | 7 | __all__ = [ | 7 | __all__ = [ |
45 | 8 | 'add_revision_to_branch', | 8 | 'add_revision_to_branch', |
46 | 9 | 'get_non_existant_source_package_branch_unique_name', | ||
47 | 9 | 'make_erics_fooix_project', | 10 | 'make_erics_fooix_project', |
48 | 10 | 'make_linked_package_branch', | 11 | 'make_linked_package_branch', |
49 | 11 | 'make_merge_proposal_without_reviewers', | 12 | 'make_merge_proposal_without_reviewers', |
50 | @@ -313,3 +314,16 @@ | |||
51 | 313 | for vote in proposal.votes: | 314 | for vote in proposal.votes: |
52 | 314 | removeSecurityProxy(vote).destroySelf() | 315 | removeSecurityProxy(vote).destroySelf() |
53 | 315 | return proposal | 316 | return proposal |
54 | 317 | |||
55 | 318 | |||
56 | 319 | def get_non_existant_source_package_branch_unique_name(owner, factory): | ||
57 | 320 | """Return the unique name for a non-existanct source package branch. | ||
58 | 321 | |||
59 | 322 | Neither the branch nor the source package name will exist. | ||
60 | 323 | """ | ||
61 | 324 | distroseries = factory.makeDistroSeries() | ||
62 | 325 | source_package = factory.getUniqueString('source-package') | ||
63 | 326 | branch = factory.getUniqueString('branch') | ||
64 | 327 | return '~%s/%s/%s/%s/%s' % ( | ||
65 | 328 | owner, distroseries.distribution.name, distroseries.name, | ||
66 | 329 | source_package, branch) | ||
67 | 316 | 330 | ||
68 | === modified file 'lib/lp/code/xmlrpc/codehosting.py' | |||
69 | --- lib/lp/code/xmlrpc/codehosting.py 2011-05-25 20:16:58 +0000 | |||
70 | +++ lib/lp/code/xmlrpc/codehosting.py 2011-08-15 13:18:36 +0000 | |||
71 | @@ -64,6 +64,10 @@ | |||
72 | 64 | LAUNCHPAD_SERVICES, | 64 | LAUNCHPAD_SERVICES, |
73 | 65 | ) | 65 | ) |
74 | 66 | from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch | 66 | from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch |
75 | 67 | from lp.registry.errors import ( | ||
76 | 68 | InvalidName, | ||
77 | 69 | NoSuchSourcePackageName, | ||
78 | 70 | ) | ||
79 | 67 | from lp.registry.interfaces.person import ( | 71 | from lp.registry.interfaces.person import ( |
80 | 68 | IPersonSet, | 72 | IPersonSet, |
81 | 69 | NoSuchPerson, | 73 | NoSuchPerson, |
82 | @@ -72,6 +76,7 @@ | |||
83 | 72 | InvalidProductName, | 76 | InvalidProductName, |
84 | 73 | NoSuchProduct, | 77 | NoSuchProduct, |
85 | 74 | ) | 78 | ) |
86 | 79 | from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet | ||
87 | 75 | from lp.services.scripts.interfaces.scriptactivity import IScriptActivitySet | 80 | from lp.services.scripts.interfaces.scriptactivity import IScriptActivitySet |
88 | 76 | from lp.services.utils import iter_split | 81 | from lp.services.utils import iter_split |
89 | 77 | 82 | ||
90 | @@ -220,6 +225,12 @@ | |||
91 | 220 | except NoSuchProduct, e: | 225 | except NoSuchProduct, e: |
92 | 221 | return faults.NotFound( | 226 | return faults.NotFound( |
93 | 222 | "Project '%s' does not exist." % e.name) | 227 | "Project '%s' does not exist." % e.name) |
94 | 228 | except NoSuchSourcePackageName as e: | ||
95 | 229 | try: | ||
96 | 230 | getUtility(ISourcePackageNameSet).new(e.name) | ||
97 | 231 | except InvalidName: | ||
98 | 232 | return faults.InvalidSourcePackageName(e.name) | ||
99 | 233 | return self.createBranch(login_id, branch_path) | ||
100 | 223 | except NameLookupFailed, e: | 234 | except NameLookupFailed, e: |
101 | 224 | return faults.NotFound(str(e)) | 235 | return faults.NotFound(str(e)) |
102 | 225 | try: | 236 | try: |
103 | 226 | 237 | ||
104 | === modified file 'lib/lp/code/xmlrpc/tests/test_codehosting.py' | |||
105 | --- lib/lp/code/xmlrpc/tests/test_codehosting.py 2011-08-03 11:00:11 +0000 | |||
106 | +++ lib/lp/code/xmlrpc/tests/test_codehosting.py 2011-08-15 13:18:36 +0000 | |||
107 | @@ -418,19 +418,39 @@ | |||
108 | 418 | message = "No such distribution series: 'ningnangnong'." | 418 | message = "No such distribution series: 'ningnangnong'." |
109 | 419 | self.assertEqual(faults.NotFound(message), fault) | 419 | self.assertEqual(faults.NotFound(message), fault) |
110 | 420 | 420 | ||
111 | 421 | def test_createBranch_missing_sourcepackagename(self): | ||
112 | 422 | # If createBranch is called with the path to a missing source | ||
113 | 423 | # package, it will create the source package. | ||
114 | 424 | owner = self.factory.makePerson() | ||
115 | 425 | distroseries = self.factory.makeDistroSeries() | ||
116 | 426 | branch_name = self.factory.getUniqueString() | ||
117 | 427 | unique_name = '/~%s/%s/%s/ningnangnong/%s' % ( | ||
118 | 428 | owner.name, distroseries.distribution.name, distroseries.name, | ||
119 | 429 | branch_name) | ||
120 | 430 | branch_id = self.codehosting_api.createBranch( | ||
121 | 431 | owner.id, escape(unique_name)) | ||
122 | 432 | login(ANONYMOUS) | ||
123 | 433 | branch = self.branch_lookup.get(branch_id) | ||
124 | 434 | self.assertEqual(owner, branch.owner) | ||
125 | 435 | self.assertEqual(distroseries, branch.distroseries) | ||
126 | 436 | self.assertEqual( | ||
127 | 437 | 'ningnangnong', branch.sourcepackagename.name) | ||
128 | 438 | self.assertEqual(branch_name, branch.name) | ||
129 | 439 | self.assertEqual(owner, branch.registrant) | ||
130 | 440 | self.assertEqual(BranchType.HOSTED, branch.branch_type) | ||
131 | 441 | |||
132 | 421 | def test_createBranch_invalid_sourcepackagename(self): | 442 | def test_createBranch_invalid_sourcepackagename(self): |
135 | 422 | # If createBranch is called with the path to an invalid source | 443 | # If createBranch is called with an invalid path, it will fault. |
134 | 423 | # package, it will return a Fault saying so. | ||
136 | 424 | owner = self.factory.makePerson() | 444 | owner = self.factory.makePerson() |
137 | 425 | distroseries = self.factory.makeDistroSeries() | 445 | distroseries = self.factory.makeDistroSeries() |
138 | 426 | branch_name = self.factory.getUniqueString() | 446 | branch_name = self.factory.getUniqueString() |
140 | 427 | unique_name = '/~%s/%s/%s/ningnangnong/%s' % ( | 447 | unique_name = '/~%s/%s/%s/ningn%%20angnong/%s' % ( |
141 | 428 | owner.name, distroseries.distribution.name, distroseries.name, | 448 | owner.name, distroseries.distribution.name, distroseries.name, |
142 | 429 | branch_name) | 449 | branch_name) |
143 | 430 | fault = self.codehosting_api.createBranch( | 450 | fault = self.codehosting_api.createBranch( |
144 | 431 | owner.id, escape(unique_name)) | 451 | owner.id, escape(unique_name)) |
147 | 432 | message = "No such source package: 'ningnangnong'." | 452 | self.assertEqual( |
148 | 433 | self.assertEqual(faults.NotFound(message), fault) | 453 | faults.InvalidSourcePackageName('ningn%20angnong'), fault) |
149 | 434 | 454 | ||
150 | 435 | def test_createBranch_using_branch_alias(self): | 455 | def test_createBranch_using_branch_alias(self): |
151 | 436 | # Branches can be created using the branch alias and the full unique | 456 | # Branches can be created using the branch alias and the full unique |
152 | 437 | 457 | ||
153 | === modified file 'lib/lp/codehosting/inmemory.py' | |||
154 | --- lib/lp/codehosting/inmemory.py 2011-08-03 11:00:11 +0000 | |||
155 | +++ lib/lp/codehosting/inmemory.py 2011-08-15 13:18:36 +0000 | |||
156 | @@ -25,7 +25,10 @@ | |||
157 | 25 | 25 | ||
158 | 26 | from canonical.database.constants import UTC_NOW | 26 | from canonical.database.constants import UTC_NOW |
159 | 27 | from canonical.launchpad.xmlrpc import faults | 27 | from canonical.launchpad.xmlrpc import faults |
161 | 28 | from lp.app.validators import LaunchpadValidationError | 28 | from lp.app.validators import ( |
162 | 29 | LaunchpadValidationError, | ||
163 | 30 | ) | ||
164 | 31 | from lp.app.validators.name import valid_name | ||
165 | 29 | from lp.code.bzr import ( | 32 | from lp.code.bzr import ( |
166 | 30 | BranchFormat, | 33 | BranchFormat, |
167 | 31 | ControlFormat, | 34 | ControlFormat, |
168 | @@ -51,6 +54,7 @@ | |||
169 | 51 | ProductBranchTarget, | 54 | ProductBranchTarget, |
170 | 52 | ) | 55 | ) |
171 | 53 | from lp.code.xmlrpc.codehosting import datetime_from_tuple | 56 | from lp.code.xmlrpc.codehosting import datetime_from_tuple |
172 | 57 | from lp.registry.errors import InvalidName | ||
173 | 54 | from lp.registry.interfaces.pocket import PackagePublishingPocket | 58 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
174 | 55 | from lp.services.utils import iter_split | 59 | from lp.services.utils import iter_split |
175 | 56 | from lp.services.xmlrpc import LaunchpadFault | 60 | from lp.services.xmlrpc import LaunchpadFault |
176 | @@ -194,6 +198,14 @@ | |||
177 | 194 | self.distroseries._linked_branches[self, pocket] = branch | 198 | self.distroseries._linked_branches[self, pocket] = branch |
178 | 195 | 199 | ||
179 | 196 | 200 | ||
180 | 201 | class SourcePackageNameSet(ObjectSet): | ||
181 | 202 | |||
182 | 203 | def new(self, name_string): | ||
183 | 204 | if not valid_name(name_string): | ||
184 | 205 | raise InvalidName(name_string) | ||
185 | 206 | return self._add(FakeSourcePackageName(name_string)) | ||
186 | 207 | |||
187 | 208 | |||
188 | 197 | @adapter(FakeSourcePackage) | 209 | @adapter(FakeSourcePackage) |
189 | 198 | @implementer(IBranchTarget) | 210 | @implementer(IBranchTarget) |
190 | 199 | def fake_source_package_to_branch_target(fake_package): | 211 | def fake_source_package_to_branch_target(fake_package): |
191 | @@ -454,9 +466,7 @@ | |||
192 | 454 | return distroseries | 466 | return distroseries |
193 | 455 | 467 | ||
194 | 456 | def makeSourcePackageName(self): | 468 | def makeSourcePackageName(self): |
198 | 457 | sourcepackagename = FakeSourcePackageName(self.getUniqueString()) | 469 | return self._sourcepackagename_set.new(self.getUniqueString()) |
196 | 458 | self._sourcepackagename_set._add(sourcepackagename) | ||
197 | 459 | return sourcepackagename | ||
199 | 460 | 470 | ||
200 | 461 | def makeSourcePackage(self, distroseries=None, sourcepackagename=None): | 471 | def makeSourcePackage(self, distroseries=None, sourcepackagename=None): |
201 | 462 | if distroseries is None: | 472 | if distroseries is None: |
202 | @@ -659,9 +669,12 @@ | |||
203 | 659 | sourcepackagename = self._sourcepackagename_set.getByName( | 669 | sourcepackagename = self._sourcepackagename_set.getByName( |
204 | 660 | data['sourcepackagename']) | 670 | data['sourcepackagename']) |
205 | 661 | if sourcepackagename is None: | 671 | if sourcepackagename is None: |
209 | 662 | raise faults.NotFound( | 672 | try: |
210 | 663 | "No such source package: '%s'." | 673 | sourcepackagename = self._sourcepackagename_set.new( |
211 | 664 | % (data['sourcepackagename'],)) | 674 | data['sourcepackagename']) |
212 | 675 | except InvalidName: | ||
213 | 676 | raise faults.InvalidSourcePackageName( | ||
214 | 677 | data['sourcepackagename']) | ||
215 | 665 | sourcepackage = self._factory.makeSourcePackage( | 678 | sourcepackage = self._factory.makeSourcePackage( |
216 | 666 | distroseries, sourcepackagename) | 679 | distroseries, sourcepackagename) |
217 | 667 | else: | 680 | else: |
218 | @@ -890,7 +903,7 @@ | |||
219 | 890 | self._product_set = ObjectSet() | 903 | self._product_set = ObjectSet() |
220 | 891 | self._distribution_set = ObjectSet() | 904 | self._distribution_set = ObjectSet() |
221 | 892 | self._distroseries_set = ObjectSet() | 905 | self._distroseries_set = ObjectSet() |
223 | 893 | self._sourcepackagename_set = ObjectSet() | 906 | self._sourcepackagename_set = SourcePackageNameSet() |
224 | 894 | self._factory = FakeObjectFactory( | 907 | self._factory = FakeObjectFactory( |
225 | 895 | self._branch_set, self._person_set, self._product_set, | 908 | self._branch_set, self._person_set, self._product_set, |
226 | 896 | self._distribution_set, self._distroseries_set, | 909 | self._distribution_set, self._distroseries_set, |
227 | 897 | 910 | ||
228 | === modified file 'lib/lp/codehosting/tests/test_acceptance.py' | |||
229 | --- lib/lp/codehosting/tests/test_acceptance.py 2011-08-12 14:57:27 +0000 | |||
230 | +++ lib/lp/codehosting/tests/test_acceptance.py 2011-08-15 13:18:36 +0000 | |||
231 | @@ -35,6 +35,9 @@ | |||
232 | 35 | from lp.code.enums import BranchType | 35 | from lp.code.enums import BranchType |
233 | 36 | from lp.code.interfaces.branch import IBranchSet | 36 | from lp.code.interfaces.branch import IBranchSet |
234 | 37 | from lp.code.interfaces.branchnamespace import get_branch_namespace | 37 | from lp.code.interfaces.branchnamespace import get_branch_namespace |
235 | 38 | from lp.code.tests.helpers import ( | ||
236 | 39 | get_non_existant_source_package_branch_unique_name, | ||
237 | 40 | ) | ||
238 | 38 | from lp.codehosting import ( | 41 | from lp.codehosting import ( |
239 | 39 | get_bzr_path, | 42 | get_bzr_path, |
240 | 40 | get_BZR_PLUGIN_PATH_for_subprocess, | 43 | get_BZR_PLUGIN_PATH_for_subprocess, |
241 | @@ -56,7 +59,7 @@ | |||
242 | 56 | 59 | ||
243 | 57 | 60 | ||
244 | 58 | class ForkingServerForTests(object): | 61 | class ForkingServerForTests(object): |
246 | 59 | """Map starting/stopping a LPForkingService with setUp() and tearDown().""" | 62 | """Map starting/stopping a LPForkingService to setUp() and tearDown().""" |
247 | 60 | 63 | ||
248 | 61 | def __init__(self): | 64 | def __init__(self): |
249 | 62 | self.process = None | 65 | self.process = None |
250 | @@ -68,8 +71,8 @@ | |||
251 | 68 | env = os.environ.copy() | 71 | env = os.environ.copy() |
252 | 69 | env['BZR_PLUGIN_PATH'] = BZR_PLUGIN_PATH | 72 | env['BZR_PLUGIN_PATH'] = BZR_PLUGIN_PATH |
253 | 70 | # TODO: We probably want to use a random disk path for | 73 | # TODO: We probably want to use a random disk path for |
256 | 71 | # forking_daemon_socket, but we need to update config so that the | 74 | # forking_daemon_socket, but we need to update config so that |
257 | 72 | # CodeHosting service can find it. | 75 | # the CodeHosting service can find it. |
258 | 73 | # The main problem is that CodeHostingTac seems to start a tac | 76 | # The main problem is that CodeHostingTac seems to start a tac |
259 | 74 | # server directly from the disk configs, and doesn't use the | 77 | # server directly from the disk configs, and doesn't use the |
260 | 75 | # in-memory config. So we can't just override the memory | 78 | # in-memory config. So we can't just override the memory |
261 | @@ -83,14 +86,14 @@ | |||
262 | 83 | self.process = process | 86 | self.process = process |
263 | 84 | # Wait for it to indicate it is running | 87 | # Wait for it to indicate it is running |
264 | 85 | # The first line should be "Preloading" indicating it is ready | 88 | # The first line should be "Preloading" indicating it is ready |
266 | 86 | preloading_line = process.stderr.readline() | 89 | process.stderr.readline() |
267 | 87 | # The next line is the "Listening on socket" line | 90 | # The next line is the "Listening on socket" line |
269 | 88 | socket_line = process.stderr.readline() | 91 | process.stderr.readline() |
270 | 89 | # Now it is ready | 92 | # Now it is ready |
271 | 90 | 93 | ||
272 | 91 | def tearDown(self): | 94 | def tearDown(self): |
275 | 92 | # SIGTERM is the graceful exit request, potentially we could wait a bit | 95 | # SIGTERM is the graceful exit request, potentially we could wait a |
276 | 93 | # and send something stronger? | 96 | # bit and send something stronger? |
277 | 94 | if self.process is not None and self.process.poll() is None: | 97 | if self.process is not None and self.process.poll() is None: |
278 | 95 | os.kill(self.process.pid, signal.SIGTERM) | 98 | os.kill(self.process.pid, signal.SIGTERM) |
279 | 96 | self.process.wait() | 99 | self.process.wait() |
280 | @@ -102,7 +105,6 @@ | |||
281 | 102 | os.remove(self.socket_path) | 105 | os.remove(self.socket_path) |
282 | 103 | 106 | ||
283 | 104 | 107 | ||
284 | 105 | |||
285 | 106 | class SSHServerLayer(ZopelessAppServerLayer): | 108 | class SSHServerLayer(ZopelessAppServerLayer): |
286 | 107 | 109 | ||
287 | 108 | _tac_handler = None | 110 | _tac_handler = None |
288 | @@ -611,6 +613,15 @@ | |||
289 | 611 | self.local_branch_path, remote_url, | 613 | self.local_branch_path, remote_url, |
290 | 612 | ['Permission denied:', 'Transport operation not possible:']) | 614 | ['Permission denied:', 'Transport operation not possible:']) |
291 | 613 | 615 | ||
292 | 616 | def test_push_new_branch_of_non_existant_source_package_name(self): | ||
293 | 617 | ZopelessAppServerLayer.txn.begin() | ||
294 | 618 | unique_name = get_non_existant_source_package_branch_unique_name( | ||
295 | 619 | 'testuser', self.factory) | ||
296 | 620 | ZopelessAppServerLayer.txn.commit() | ||
297 | 621 | remote_url = self.getTransportURL(unique_name) | ||
298 | 622 | self.push(self.local_branch_path, remote_url) | ||
299 | 623 | self.assertBranchesMatch(self.local_branch_path, remote_url) | ||
300 | 624 | |||
301 | 614 | def test_can_push_loom_branch(self): | 625 | def test_can_push_loom_branch(self): |
302 | 615 | # We can push and pull a loom branch. | 626 | # We can push and pull a loom branch. |
303 | 616 | self.makeLoomBranchAndTree('loom') | 627 | self.makeLoomBranchAndTree('loom') |
304 | @@ -696,7 +707,6 @@ | |||
305 | 696 | urllib2.urlopen(web_status_url) | 707 | urllib2.urlopen(web_status_url) |
306 | 697 | 708 | ||
307 | 698 | 709 | ||
308 | 699 | |||
309 | 700 | def make_server_tests(base_suite, servers): | 710 | def make_server_tests(base_suite, servers): |
310 | 701 | from lp.codehosting.tests.helpers import ( | 711 | from lp.codehosting.tests.helpers import ( |
311 | 702 | CodeHostingTestProviderAdapter) | 712 | CodeHostingTestProviderAdapter) |
312 | 703 | 713 | ||
313 | === modified file 'lib/lp/codehosting/vfs/branchfs.py' | |||
314 | --- lib/lp/codehosting/vfs/branchfs.py 2011-08-06 17:37:47 +0000 | |||
315 | +++ lib/lp/codehosting/vfs/branchfs.py 2011-08-15 13:18:36 +0000 | |||
316 | @@ -46,9 +46,6 @@ | |||
317 | 46 | __metaclass__ = type | 46 | __metaclass__ = type |
318 | 47 | __all__ = [ | 47 | __all__ = [ |
319 | 48 | 'AsyncLaunchpadTransport', | 48 | 'AsyncLaunchpadTransport', |
320 | 49 | 'BadUrlLaunchpad', | ||
321 | 50 | 'BadUrlScheme', | ||
322 | 51 | 'BadUrlSsh', | ||
323 | 52 | 'branch_id_to_path', | 49 | 'branch_id_to_path', |
324 | 53 | 'DirectDatabaseLaunchpadServer', | 50 | 'DirectDatabaseLaunchpadServer', |
325 | 54 | 'get_lp_server', | 51 | 'get_lp_server', |
326 | @@ -599,7 +596,8 @@ | |||
327 | 599 | # exist. You may supply --create-prefix to create all leading | 596 | # exist. You may supply --create-prefix to create all leading |
328 | 600 | # parent directories", which is just misleading. | 597 | # parent directories", which is just misleading. |
329 | 601 | fault = trap_fault( | 598 | fault = trap_fault( |
331 | 602 | fail, faults.NotFound, faults.PermissionDenied) | 599 | fail, faults.NotFound, faults.PermissionDenied, |
332 | 600 | faults.InvalidSourcePackageName) | ||
333 | 603 | faultString = fault.faultString | 601 | faultString = fault.faultString |
334 | 604 | if isinstance(faultString, unicode): | 602 | if isinstance(faultString, unicode): |
335 | 605 | faultString = faultString.encode('utf-8') | 603 | faultString = faultString.encode('utf-8') |
336 | @@ -747,4 +745,3 @@ | |||
337 | 747 | DeferredBlockingProxy(codehosting_client), user_id, branch_transport, | 745 | DeferredBlockingProxy(codehosting_client), user_id, branch_transport, |
338 | 748 | seen_new_branch_hook) | 746 | seen_new_branch_hook) |
339 | 749 | return lp_server | 747 | return lp_server |
340 | 750 | |||
341 | 751 | 748 | ||
342 | === modified file 'lib/lp/codehosting/vfs/tests/test_branchfs.py' | |||
343 | --- lib/lp/codehosting/vfs/tests/test_branchfs.py 2011-08-12 05:03:42 +0000 | |||
344 | +++ lib/lp/codehosting/vfs/tests/test_branchfs.py 2011-08-15 13:18:36 +0000 | |||
345 | @@ -49,7 +49,6 @@ | |||
346 | 49 | from twisted.internet import defer | 49 | from twisted.internet import defer |
347 | 50 | 50 | ||
348 | 51 | from canonical.launchpad.webapp import errorlog | 51 | from canonical.launchpad.webapp import errorlog |
349 | 52 | from canonical.launchpad.webapp.errorlog import ErrorReportingUtility | ||
350 | 53 | from canonical.testing.layers import ( | 52 | from canonical.testing.layers import ( |
351 | 54 | ZopelessDatabaseLayer, | 53 | ZopelessDatabaseLayer, |
352 | 55 | ) | 54 | ) |
353 | @@ -798,6 +797,17 @@ | |||
354 | 798 | errors.PermissionDenied, message, | 797 | errors.PermissionDenied, message, |
355 | 799 | transport.mkdir, '~%s/%s/some-name' % (person.name, product.name)) | 798 | transport.mkdir, '~%s/%s/some-name' % (person.name, product.name)) |
356 | 800 | 799 | ||
357 | 800 | def test_createBranch_invalid_package_name(self): | ||
358 | 801 | # When createBranch raises faults.InvalidSourcePackageName, the | ||
359 | 802 | # transport should translate this to a PermissionDenied exception | ||
360 | 803 | transport = self.getTransport() | ||
361 | 804 | series = self.factory.makeDistroSeries() | ||
362 | 805 | unique_name = '~%s/%s/%s/spaced%%20name/branch' % ( | ||
363 | 806 | self.requester.name, series.distribution.name, series.name) | ||
364 | 807 | return self.assertFiresFailureWithSubstring( | ||
365 | 808 | errors.PermissionDenied, "is not a valid source package name", | ||
366 | 809 | transport.mkdir, unique_name) | ||
367 | 810 | |||
368 | 801 | def test_rmdir(self): | 811 | def test_rmdir(self): |
369 | 802 | transport = self.getTransport() | 812 | transport = self.getTransport() |
370 | 803 | self.assertFiresFailure( | 813 | self.assertFiresFailure( |
371 | @@ -1155,7 +1165,8 @@ | |||
372 | 1155 | self.addCleanup(memory_server.stop_server) | 1165 | self.addCleanup(memory_server.stop_server) |
373 | 1156 | return memory_server | 1166 | return memory_server |
374 | 1157 | 1167 | ||
376 | 1158 | def _setUpLaunchpadServer(self, user_id, codehosting_api, backing_transport): | 1168 | def _setUpLaunchpadServer(self, user_id, codehosting_api, |
377 | 1169 | backing_transport): | ||
378 | 1159 | server = LaunchpadServer( | 1170 | server = LaunchpadServer( |
379 | 1160 | XMLRPCWrapper(codehosting_api), user_id, backing_transport) | 1171 | XMLRPCWrapper(codehosting_api), user_id, backing_transport) |
380 | 1161 | server.start_server() | 1172 | server.start_server() |
381 | 1162 | 1173 | ||
382 | === modified file 'lib/lp/registry/errors.py' | |||
383 | --- lib/lp/registry/errors.py 2011-07-27 08:13:18 +0000 | |||
384 | +++ lib/lp/registry/errors.py 2011-08-15 13:18:36 +0000 | |||
385 | @@ -8,6 +8,7 @@ | |||
386 | 8 | 'CannotTransitionToCountryMirror', | 8 | 'CannotTransitionToCountryMirror', |
387 | 9 | 'CountryMirrorAlreadySet', | 9 | 'CountryMirrorAlreadySet', |
388 | 10 | 'DeleteSubscriptionError', | 10 | 'DeleteSubscriptionError', |
389 | 11 | 'InvalidName', | ||
390 | 11 | 'JoinNotAllowed', | 12 | 'JoinNotAllowed', |
391 | 12 | 'MirrorNotOfficial', | 13 | 'MirrorNotOfficial', |
392 | 13 | 'MirrorHasNoHTTPURL', | 14 | 'MirrorHasNoHTTPURL', |
393 | @@ -42,6 +43,10 @@ | |||
394 | 42 | """The name given for a person is already in use by other person.""" | 43 | """The name given for a person is already in use by other person.""" |
395 | 43 | 44 | ||
396 | 44 | 45 | ||
397 | 46 | class InvalidName(Exception): | ||
398 | 47 | """The name given for a person is not valid.""" | ||
399 | 48 | |||
400 | 49 | |||
401 | 45 | class NoSuchDistroSeries(NameLookupFailed): | 50 | class NoSuchDistroSeries(NameLookupFailed): |
402 | 46 | """Raised when we try to find a DistroSeries that doesn't exist.""" | 51 | """Raised when we try to find a DistroSeries that doesn't exist.""" |
403 | 47 | _message_prefix = "No such distribution series" | 52 | _message_prefix = "No such distribution series" |
404 | 48 | 53 | ||
405 | === modified file 'lib/lp/registry/interfaces/person.py' | |||
406 | --- lib/lp/registry/interfaces/person.py 2011-07-29 06:08:15 +0000 | |||
407 | +++ lib/lp/registry/interfaces/person.py 2011-08-15 13:18:36 +0000 | |||
408 | @@ -27,7 +27,6 @@ | |||
409 | 27 | 'ITeamCreation', | 27 | 'ITeamCreation', |
410 | 28 | 'ITeamReassignment', | 28 | 'ITeamReassignment', |
411 | 29 | 'ImmutableVisibilityError', | 29 | 'ImmutableVisibilityError', |
412 | 30 | 'InvalidName', | ||
413 | 31 | 'NoSuchPerson', | 30 | 'NoSuchPerson', |
414 | 32 | 'OPEN_TEAM_POLICY', | 31 | 'OPEN_TEAM_POLICY', |
415 | 33 | 'PersonCreationRationale', | 32 | 'PersonCreationRationale', |
416 | @@ -1405,7 +1404,8 @@ | |||
417 | 1405 | ) | 1404 | ) |
418 | 1406 | @export_factory_operation(Interface, []) # Really IArchive. | 1405 | @export_factory_operation(Interface, []) # Really IArchive. |
419 | 1407 | @operation_for_version("beta") | 1406 | @operation_for_version("beta") |
421 | 1408 | def createPPA(name=None, displayname=None, description=None, private=False): | 1407 | def createPPA(name=None, displayname=None, description=None, |
422 | 1408 | private=False): | ||
423 | 1409 | """Create a PPA. | 1409 | """Create a PPA. |
424 | 1410 | 1410 | ||
425 | 1411 | :param name: A string with the name of the new PPA to create. If | 1411 | :param name: A string with the name of the new PPA to create. If |
426 | @@ -2255,8 +2255,8 @@ | |||
427 | 2255 | addresses associated with. | 2255 | addresses associated with. |
428 | 2256 | 2256 | ||
429 | 2257 | When merging teams, from_person must have no IMailingLists | 2257 | When merging teams, from_person must have no IMailingLists |
432 | 2258 | associated with it. If it has active members they will be deactivated - | 2258 | associated with it. If it has active members they will be deactivated |
433 | 2259 | and reviewer must be supplied. | 2259 | - and reviewer must be supplied. |
434 | 2260 | 2260 | ||
435 | 2261 | :param from_person: An IPerson or ITeam that is a duplicate. | 2261 | :param from_person: An IPerson or ITeam that is a duplicate. |
436 | 2262 | :param to_person: An IPerson or ITeam that is a master. | 2262 | :param to_person: An IPerson or ITeam that is a master. |
437 | @@ -2447,10 +2447,6 @@ | |||
438 | 2447 | """A change in team membership visibility is not allowed.""" | 2447 | """A change in team membership visibility is not allowed.""" |
439 | 2448 | 2448 | ||
440 | 2449 | 2449 | ||
441 | 2450 | class InvalidName(Exception): | ||
442 | 2451 | """The name given for a person is not valid.""" | ||
443 | 2452 | |||
444 | 2453 | |||
445 | 2454 | class NoSuchPerson(NameLookupFailed): | 2450 | class NoSuchPerson(NameLookupFailed): |
446 | 2455 | """Raised when we try to look up an IPerson that doesn't exist.""" | 2451 | """Raised when we try to look up an IPerson that doesn't exist.""" |
447 | 2456 | 2452 | ||
448 | 2457 | 2453 | ||
449 | === modified file 'lib/lp/registry/model/person.py' | |||
450 | --- lib/lp/registry/model/person.py 2011-08-01 15:28:09 +0000 | |||
451 | +++ lib/lp/registry/model/person.py 2011-08-15 13:18:36 +0000 | |||
452 | @@ -199,6 +199,7 @@ | |||
453 | 199 | HasRequestedReviewsMixin, | 199 | HasRequestedReviewsMixin, |
454 | 200 | ) | 200 | ) |
455 | 201 | from lp.registry.errors import ( | 201 | from lp.registry.errors import ( |
456 | 202 | InvalidName, | ||
457 | 202 | JoinNotAllowed, | 203 | JoinNotAllowed, |
458 | 203 | NameAlreadyTaken, | 204 | NameAlreadyTaken, |
459 | 204 | PPACreationError, | 205 | PPACreationError, |
460 | @@ -225,7 +226,6 @@ | |||
461 | 225 | ) | 226 | ) |
462 | 226 | from lp.registry.interfaces.person import ( | 227 | from lp.registry.interfaces.person import ( |
463 | 227 | ImmutableVisibilityError, | 228 | ImmutableVisibilityError, |
464 | 228 | InvalidName, | ||
465 | 229 | IPerson, | 229 | IPerson, |
466 | 230 | IPersonSet, | 230 | IPersonSet, |
467 | 231 | IPersonSettings, | 231 | IPersonSettings, |
468 | 232 | 232 | ||
469 | === modified file 'lib/lp/registry/model/sourcepackagename.py' | |||
470 | --- lib/lp/registry/model/sourcepackagename.py 2010-11-09 08:38:23 +0000 | |||
471 | +++ lib/lp/registry/model/sourcepackagename.py 2011-08-15 13:18:36 +0000 | |||
472 | @@ -25,7 +25,11 @@ | |||
473 | 25 | ) | 25 | ) |
474 | 26 | from canonical.launchpad.helpers import ensure_unicode | 26 | from canonical.launchpad.helpers import ensure_unicode |
475 | 27 | from lp.app.errors import NotFoundError | 27 | from lp.app.errors import NotFoundError |
477 | 28 | from lp.registry.errors import NoSuchSourcePackageName | 28 | from lp.app.validators.name import valid_name |
478 | 29 | from lp.registry.errors import ( | ||
479 | 30 | InvalidName, | ||
480 | 31 | NoSuchSourcePackageName, | ||
481 | 32 | ) | ||
482 | 29 | from lp.registry.interfaces.sourcepackagename import ( | 33 | from lp.registry.interfaces.sourcepackagename import ( |
483 | 30 | ISourcePackageName, | 34 | ISourcePackageName, |
484 | 31 | ISourcePackageNameSet, | 35 | ISourcePackageNameSet, |
485 | @@ -90,6 +94,9 @@ | |||
486 | 90 | return SourcePackageName.selectOneBy(name=name) | 94 | return SourcePackageName.selectOneBy(name=name) |
487 | 91 | 95 | ||
488 | 92 | def new(self, name): | 96 | def new(self, name): |
489 | 97 | if not valid_name(name): | ||
490 | 98 | raise InvalidName( | ||
491 | 99 | "%s is not a valid name for a source package." % name) | ||
492 | 93 | return SourcePackageName(name=name) | 100 | return SourcePackageName(name=name) |
493 | 94 | 101 | ||
494 | 95 | def getOrCreateByName(self, name): | 102 | def getOrCreateByName(self, name): |
495 | 96 | 103 | ||
496 | === modified file 'lib/lp/registry/tests/test_person.py' | |||
497 | --- lib/lp/registry/tests/test_person.py 2011-08-03 11:00:11 +0000 | |||
498 | +++ lib/lp/registry/tests/test_person.py 2011-08-15 13:18:36 +0000 | |||
499 | @@ -47,6 +47,7 @@ | |||
500 | 47 | from lp.bugs.model.bug import Bug | 47 | from lp.bugs.model.bug import Bug |
501 | 48 | from lp.bugs.model.bugtask import get_related_bugtasks_search_params | 48 | from lp.bugs.model.bugtask import get_related_bugtasks_search_params |
502 | 49 | from lp.registry.errors import ( | 49 | from lp.registry.errors import ( |
503 | 50 | InvalidName, | ||
504 | 50 | NameAlreadyTaken, | 51 | NameAlreadyTaken, |
505 | 51 | PrivatePersonLinkageError, | 52 | PrivatePersonLinkageError, |
506 | 52 | ) | 53 | ) |
507 | @@ -55,7 +56,6 @@ | |||
508 | 55 | from lp.registry.interfaces.nameblacklist import INameBlacklistSet | 56 | from lp.registry.interfaces.nameblacklist import INameBlacklistSet |
509 | 56 | from lp.registry.interfaces.person import ( | 57 | from lp.registry.interfaces.person import ( |
510 | 57 | ImmutableVisibilityError, | 58 | ImmutableVisibilityError, |
511 | 58 | InvalidName, | ||
512 | 59 | IPersonSet, | 59 | IPersonSet, |
513 | 60 | PersonCreationRationale, | 60 | PersonCreationRationale, |
514 | 61 | PersonVisibility, | 61 | PersonVisibility, |
515 | 62 | 62 | ||
516 | === added file 'lib/lp/registry/tests/test_sourcepackagename.py' | |||
517 | --- lib/lp/registry/tests/test_sourcepackagename.py 1970-01-01 00:00:00 +0000 | |||
518 | +++ lib/lp/registry/tests/test_sourcepackagename.py 2011-08-15 13:18:36 +0000 | |||
519 | @@ -0,0 +1,24 @@ | |||
520 | 1 | # Copyright 2011 Canonical Ltd. This software is licensed under the | ||
521 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
522 | 3 | |||
523 | 4 | """Tests for SourcePackageName""" | ||
524 | 5 | |||
525 | 6 | __metaclass__ = type | ||
526 | 7 | |||
527 | 8 | from testtools.testcase import ExpectedException | ||
528 | 9 | |||
529 | 10 | from canonical.testing.layers import DatabaseLayer | ||
530 | 11 | from lp.registry.errors import InvalidName | ||
531 | 12 | from lp.registry.model.sourcepackagename import SourcePackageNameSet | ||
532 | 13 | from lp.testing import TestCase | ||
533 | 14 | |||
534 | 15 | |||
535 | 16 | class TestSourcePackageNameSet(TestCase): | ||
536 | 17 | |||
537 | 18 | layer = DatabaseLayer | ||
538 | 19 | |||
539 | 20 | def test_invalid_name(self): | ||
540 | 21 | with ExpectedException( | ||
541 | 22 | InvalidName, | ||
542 | 23 | 'invalid%20name is not a valid name for a source package.'): | ||
543 | 24 | SourcePackageNameSet().new('invalid%20name') |