Merge lp:~abentley/launchpad/remove-create-merge-proposal-job into lp:launchpad
- remove-create-merge-proposal-job
- Merge into devel
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Aaron Bentley on 2012-04-30 | ||||
| Approved revision: | no longer in the source branch. | ||||
| Merged at revision: | 15178 | ||||
| Proposed branch: | lp:~abentley/launchpad/remove-create-merge-proposal-job | ||||
| Merge into: | lp:launchpad | ||||
| Diff against target: |
2074 lines (+23/-1627) 17 files modified
configs/testrunner/launchpad-lazr.conf (+0/-4) cronscripts/create_merge_proposals.py (+0/-40) lib/lp/code/configure.zcml (+0/-10) lib/lp/code/interfaces/branchmergeproposal.py (+0/-16) lib/lp/code/mail/codehandler.py (+7/-342) lib/lp/code/mail/errortemplates/branch-creation-exception.txt (+0/-2) lib/lp/code/mail/errortemplates/missingmergedirective.txt (+0/-2) lib/lp/code/mail/tests/test_codehandler.py (+13/-723) lib/lp/code/model/branchmergeproposaljob.py (+0/-89) lib/lp/code/model/tests/test_branchmergeproposal.py (+0/-67) lib/lp/code/scripts/tests/test_create_merge_proposals.py (+0/-107) lib/lp/services/config/schema-lazr.conf (+0/-12) lib/lp/services/mail/errortemplates/mergedirectivenotsupported.txt (+3/-0) lib/lp/services/messages/interfaces/message.py (+0/-18) lib/lp/services/messages/model/message.py (+0/-63) lib/lp/services/messages/tests/test_message.py (+0/-49) lib/lp/testing/factory.py (+0/-83) |
||||
| To merge this branch: | bzr merge lp:~abentley/launchpad/remove-create-merge-proposal-job | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Richard Harding (community) | code | 2012-04-27 | Approve on 2012-04-27 |
|
Review via email:
|
|||
Commit Message
Remove CreateMergeProp
Description of the Change
= Summary =
Fix bug #989831: CreateMergeProp
== Proposed fix ==
Remove CreateMergeProp
== Pre-implementation notes ==
Discussed with deryck. We determined that this feature has not been used at all in 2012, and less than 100 times in 2011.
== LOC Rationale ==
Reduces LOC
== Implementation details ==
Deletion. Lots and lots of deletion. (And mostly my own hard work, too!)
== Tests ==
bin/test -v test_codehandler
== Demo and Q/A ==
Send an email to merge@code.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
configs/
lib/lp/
./lib/lp/
489: Line exceeds 80 characters.
1100: Line exceeds 80 characters.
1107: Line exceeds 80 characters.
1699: Line exceeds 80 characters.
./lib/lp/
1: Line exceeds 80 characters.
3: Line exceeds 80 characters.
./configs/
126: Line exceeds 80 characters.
| Aaron Bentley (abentley) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 12-04-27 03:43 PM, Richard Harding wrote:
> My only suggestion is around the email wording. In the effort of
> keeping things simple I'd leave out the bits about reasoning.
In general, when you leave out the bits about reasoning, users
complain and file bugs and the response is disorganized, because the
bug triager is not the person who made the change. I think we can
give users a better experience and more coherent responses by giving
reasoning and directing them to the relevant bug.
> I'd also move the usage of the webui up so that it's more prominent
> in the text.
I think lp-propose-merge should be more prominent because it's a
better fit, and because it's more likely to be informative.
Merge directives are generated and sent using the "bzr send" command.
I wrote both "send" and "lp-propose-merge", and I view
"lp-propose-merge" as its clear successor to "bzr send" for use with
Launchpad. Both command-line tools, and they function similarly, with
"send" launching your email client for you to type the description,
while "lp-propose-merge" launches your editor for you to type the
description.
It seems like the number of people who know about merge proposals but
don't know they can create them via the web UI is very small. So I
think the existence of the lp-propose-merge command is more likely to
be new information.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAk+
llwAnRF2DDIQ8dQ
=3UFR
-----END PGP SIGNATURE-----
Preview Diff
| 1 | === modified file 'configs/testrunner/launchpad-lazr.conf' |
| 2 | --- configs/testrunner/launchpad-lazr.conf 2012-01-18 01:46:02 +0000 |
| 3 | +++ configs/testrunner/launchpad-lazr.conf 2012-04-30 14:10:29 +0000 |
| 4 | @@ -30,10 +30,6 @@ |
| 5 | access_log: /tmp/test-codehosting-access.log |
| 6 | internal_branch_by_id_root: file:///var/tmp/bazaar.launchpad.dev/mirrors |
| 7 | |
| 8 | -[create_merge_proposals] |
| 9 | -oops_prefix: TMPCJ |
| 10 | -error_dir: /var/tmp/codehosting.test |
| 11 | - |
| 12 | [database] |
| 13 | rw_main_master: dbname=launchpad_ftest host=localhost |
| 14 | rw_main_slave: dbname=launchpad_ftest host=localhost |
| 15 | |
| 16 | === removed file 'cronscripts/create_merge_proposals.py' |
| 17 | --- cronscripts/create_merge_proposals.py 2012-01-01 03:14:54 +0000 |
| 18 | +++ cronscripts/create_merge_proposals.py 1970-01-01 00:00:00 +0000 |
| 19 | @@ -1,40 +0,0 @@ |
| 20 | -#!/usr/bin/python -S |
| 21 | -# |
| 22 | -# Copyright 2009 Canonical Ltd. This software is licensed under the |
| 23 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
| 24 | - |
| 25 | -# pylint: disable-msg=W0403 |
| 26 | - |
| 27 | -"""Create BranchMergeProposals from email.""" |
| 28 | - |
| 29 | -__metaclass__ = type |
| 30 | - |
| 31 | -import _pythonpath |
| 32 | - |
| 33 | -from zope.component import getUtility |
| 34 | - |
| 35 | -from lp.code.interfaces.branchmergeproposal import ( |
| 36 | - ICreateMergeProposalJobSource, |
| 37 | - ) |
| 38 | -from lp.services.config import config |
| 39 | -from lp.services.job.runner import JobRunner |
| 40 | -from lp.services.scripts.base import LaunchpadCronScript |
| 41 | -from lp.services.webapp.errorlog import globalErrorUtility |
| 42 | - |
| 43 | - |
| 44 | -class RunCreateMergeProposalJobs(LaunchpadCronScript): |
| 45 | - """Run create merge proposal jobs.""" |
| 46 | - |
| 47 | - def main(self): |
| 48 | - globalErrorUtility.configure('create_merge_proposals') |
| 49 | - job_source = getUtility(ICreateMergeProposalJobSource) |
| 50 | - runner = JobRunner.fromReady(job_source, self.logger) |
| 51 | - runner.runAll() |
| 52 | - self.logger.info( |
| 53 | - 'Ran %d CreateMergeProposalJobs.' % len(runner.completed_jobs)) |
| 54 | - |
| 55 | - |
| 56 | -if __name__ == '__main__': |
| 57 | - script = RunCreateMergeProposalJobs( |
| 58 | - 'create_merge_proposals', config.create_merge_proposals.dbuser) |
| 59 | - script.lock_and_run() |
| 60 | |
| 61 | === modified file 'lib/lp/code/configure.zcml' |
| 62 | --- lib/lp/code/configure.zcml 2012-04-24 06:00:11 +0000 |
| 63 | +++ lib/lp/code/configure.zcml 2012-04-30 14:10:29 +0000 |
| 64 | @@ -289,16 +289,6 @@ |
| 65 | |
| 66 | <!-- Branch Merge Proposal Jobs --> |
| 67 | |
| 68 | - <class class="lp.code.model.branchmergeproposaljob.CreateMergeProposalJob"> |
| 69 | - <allow interface="lp.services.messages.interfaces.message.IMessageJob"/> |
| 70 | - <allow interface="lp.code.interfaces.branchmergeproposal.ICreateMergeProposalJob"/> |
| 71 | - </class> |
| 72 | - <securedutility |
| 73 | - component="lp.code.model.branchmergeproposaljob.CreateMergeProposalJob" |
| 74 | - provides="lp.code.interfaces.branchmergeproposal.ICreateMergeProposalJobSource"> |
| 75 | - <allow interface="lp.code.interfaces.branchmergeproposal.ICreateMergeProposalJobSource"/> |
| 76 | - </securedutility> |
| 77 | - |
| 78 | <class class="lp.code.model.branchmergeproposaljob.MergeProposalNeedsReviewEmailJob"> |
| 79 | <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob"/> |
| 80 | <allow interface="lp.code.interfaces.branchmergeproposal.IMergeProposalNeedsReviewEmailJob"/> |
| 81 | |
| 82 | === modified file 'lib/lp/code/interfaces/branchmergeproposal.py' |
| 83 | --- lib/lp/code/interfaces/branchmergeproposal.py 2011-12-30 06:14:56 +0000 |
| 84 | +++ lib/lp/code/interfaces/branchmergeproposal.py 2012-04-30 14:10:29 +0000 |
| 85 | @@ -15,8 +15,6 @@ |
| 86 | 'IBranchMergeProposalListingBatchNavigator', |
| 87 | 'ICodeReviewCommentEmailJob', |
| 88 | 'ICodeReviewCommentEmailJobSource', |
| 89 | - 'ICreateMergeProposalJob', |
| 90 | - 'ICreateMergeProposalJobSource', |
| 91 | 'IGenerateIncrementalDiffJob', |
| 92 | 'IGenerateIncrementalDiffJobSource', |
| 93 | 'IMergeProposalNeedsReviewEmailJob', |
| 94 | @@ -641,20 +639,6 @@ |
| 95 | IBranchMergeProposal[name].schema = IBranchMergeProposal |
| 96 | |
| 97 | |
| 98 | -class ICreateMergeProposalJob(IRunnableJob): |
| 99 | - """A Job that creates a branch merge proposal. |
| 100 | - |
| 101 | - It uses a Message, which must contain a merge directive. |
| 102 | - """ |
| 103 | - |
| 104 | - |
| 105 | -class ICreateMergeProposalJobSource(IJobSource): |
| 106 | - """Acquire MergeProposalJobs.""" |
| 107 | - |
| 108 | - def create(message_bytes): |
| 109 | - """Return a CreateMergeProposalJob for this message.""" |
| 110 | - |
| 111 | - |
| 112 | class IMergeProposalNeedsReviewEmailJob(IRunnableJob): |
| 113 | """Email about a merge proposal needing a review..""" |
| 114 | |
| 115 | |
| 116 | === modified file 'lib/lp/code/mail/codehandler.py' |
| 117 | --- lib/lp/code/mail/codehandler.py 2012-01-01 02:58:52 +0000 |
| 118 | +++ lib/lp/code/mail/codehandler.py 2012-04-30 14:10:29 +0000 |
| 119 | @@ -1,4 +1,4 @@ |
| 120 | -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the |
| 121 | +# Copyright 2009-2012 Canonical Ltd. This software is licensed under the |
| 122 | # GNU Affero General Public License version 3 (see the file LICENSE). |
| 123 | |
| 124 | __metaclass__ = type |
| 125 | @@ -8,44 +8,22 @@ |
| 126 | import os |
| 127 | import re |
| 128 | |
| 129 | -from bzrlib.branch import Branch |
| 130 | -from bzrlib.errors import ( |
| 131 | - NotAMergeDirective, |
| 132 | - NotBranchError, |
| 133 | - NotStacked, |
| 134 | - ) |
| 135 | -from bzrlib.merge_directive import MergeDirective |
| 136 | -from bzrlib.transport import get_transport |
| 137 | -from bzrlib.urlutils import join as urljoin |
| 138 | -from lazr.uri import URI |
| 139 | from sqlobject import SQLObjectNotFound |
| 140 | import transaction |
| 141 | from zope.component import getUtility |
| 142 | from zope.interface import implements |
| 143 | from zope.security.interfaces import Unauthorized |
| 144 | |
| 145 | -from lp.code.bzr import get_branch_formats |
| 146 | from lp.code.enums import ( |
| 147 | - BranchType, |
| 148 | CodeReviewVote, |
| 149 | ) |
| 150 | from lp.code.errors import ( |
| 151 | - BranchCreationException, |
| 152 | - BranchMergeProposalExists, |
| 153 | UserNotBranchReviewer, |
| 154 | ) |
| 155 | -from lp.code.interfaces.branchlookup import IBranchLookup |
| 156 | from lp.code.interfaces.branchmergeproposal import ( |
| 157 | IBranchMergeProposalGetter, |
| 158 | - ICreateMergeProposalJobSource, |
| 159 | - ) |
| 160 | -from lp.code.interfaces.branchnamespace import ( |
| 161 | - lookup_branch_namespace, |
| 162 | - split_unique_name, |
| 163 | - ) |
| 164 | -from lp.code.interfaces.branchtarget import check_default_stacked_on |
| 165 | -from lp.codehosting.bzrutils import is_branch_stackable |
| 166 | -from lp.codehosting.vfs import get_lp_server |
| 167 | + ) |
| 168 | +from lp.services.config import config |
| 169 | from lp.services.mail.commands import ( |
| 170 | EmailCommand, |
| 171 | EmailCommandCollection, |
| 172 | @@ -65,8 +43,6 @@ |
| 173 | from lp.services.mail.notification import send_process_error_notification |
| 174 | from lp.services.mail.sendmail import simple_sendmail |
| 175 | from lp.services.messages.interfaces.message import IMessageSet |
| 176 | -from lp.services.webapp import urlparse |
| 177 | -from lp.services.webapp.errorlog import globalErrorUtility |
| 178 | from lp.services.webapp.interfaces import ILaunchBag |
| 179 | |
| 180 | |
| 181 | @@ -89,14 +65,6 @@ |
| 182 | """The user-supplied vote is not an acceptable value.""" |
| 183 | |
| 184 | |
| 185 | -class NonLaunchpadTarget(Exception): |
| 186 | - """Target branch is not registered with Launchpad.""" |
| 187 | - |
| 188 | - |
| 189 | -class MissingMergeDirective(Exception): |
| 190 | - """Emailed merge proposal lacks a merge directive""" |
| 191 | - |
| 192 | - |
| 193 | class CodeReviewEmailCommandExecutionContext: |
| 194 | """Passed as the only parameter to each code review email command. |
| 195 | |
| 196 | @@ -283,7 +251,10 @@ |
| 197 | deferred to jobs to create BranchMergeProposals. |
| 198 | """ |
| 199 | if email_addr.startswith('merge@'): |
| 200 | - return self.createMergeProposalJob(mail, email_addr, file_alias) |
| 201 | + body = get_error_message('mergedirectivenotsupported.txt') |
| 202 | + simple_sendmail( |
| 203 | + config.canonical.noreply_from_address, [mail.get('from')], |
| 204 | + 'Merge directive not supported.', body) |
| 205 | else: |
| 206 | try: |
| 207 | return self.processComment(mail, email_addr, file_alias) |
| 208 | @@ -294,23 +265,6 @@ |
| 209 | 'Error Creating Merge Proposal', body) |
| 210 | return True |
| 211 | |
| 212 | - def createMergeProposalJob(self, mail, email_addr, file_alias): |
| 213 | - """Check that the message is signed and create the job.""" |
| 214 | - try: |
| 215 | - ensure_not_weakly_authenticated( |
| 216 | - mail, email_addr, 'not-signed-md.txt', |
| 217 | - 'key-not-registered-md.txt', error_templates) |
| 218 | - except IncomingEmailError, error: |
| 219 | - user = getUtility(ILaunchBag).user |
| 220 | - send_process_error_notification( |
| 221 | - str(user.preferredemail.email), |
| 222 | - 'Submit Request Failure', |
| 223 | - error.message, mail, error.failing_command) |
| 224 | - transaction.abort() |
| 225 | - else: |
| 226 | - getUtility(ICreateMergeProposalJobSource).create(file_alias) |
| 227 | - return True |
| 228 | - |
| 229 | def processCommands(self, context, commands): |
| 230 | """Process the various merge proposal commands against the context.""" |
| 231 | processing_errors = [] |
| 232 | @@ -401,292 +355,3 @@ |
| 233 | return getter.get(merge_proposal_id) |
| 234 | except SQLObjectNotFound: |
| 235 | raise NonExistantBranchMergeProposalAddress(email_addr) |
| 236 | - |
| 237 | - def _acquireBranchesForProposal(self, md, submitter): |
| 238 | - """Find or create DB Branches from a MergeDirective. |
| 239 | - |
| 240 | - If the target is not a Launchpad branch, NonLaunchpadTarget will be |
| 241 | - raised. If the source is not a Launchpad branch, a REMOTE branch will |
| 242 | - be created implicitly, with submitter as its owner/registrant. |
| 243 | - |
| 244 | - :param md: The `MergeDirective` to get branch URLs from. |
| 245 | - :param submitter: The `Person` who requested that the merge be |
| 246 | - performed. |
| 247 | - :return: source_branch, target_branch |
| 248 | - """ |
| 249 | - mp_target = getUtility(IBranchLookup).getByUrl(md.target_branch) |
| 250 | - if mp_target is None: |
| 251 | - raise NonLaunchpadTarget() |
| 252 | - # If the target branch cannot be stacked upon, then don't try to stack |
| 253 | - # upon it or get revisions form it. |
| 254 | - if md.bundle is None or check_default_stacked_on(mp_target) is None: |
| 255 | - mp_source = self._getSourceNoBundle( |
| 256 | - md, mp_target, submitter) |
| 257 | - else: |
| 258 | - mp_source = self._getSourceWithBundle( |
| 259 | - md, mp_target, submitter) |
| 260 | - return mp_source, mp_target |
| 261 | - |
| 262 | - @staticmethod |
| 263 | - def _getNewBranchInfo(url, target_branch, submitter): |
| 264 | - """Return the namespace and basename for a branch. |
| 265 | - |
| 266 | - If an LP URL is provided, the namespace and basename will match the |
| 267 | - LP URL. |
| 268 | - |
| 269 | - Otherwise, the target is used to determine the namespace, and the base |
| 270 | - depends on what was supplied. |
| 271 | - |
| 272 | - If a URL is supplied, its base is used. |
| 273 | - |
| 274 | - If no URL is supplied, 'merge' is used as the base. |
| 275 | - |
| 276 | - :param url: The public URL of the source branch, if any. |
| 277 | - :param target_branch: The target branch. |
| 278 | - :param submitter: The person submitting the merge proposal. |
| 279 | - """ |
| 280 | - if url is not None: |
| 281 | - url = url.rstrip('/') |
| 282 | - branches = getUtility(IBranchLookup) |
| 283 | - unique_name = branches.uriToUniqueName(URI(url)) |
| 284 | - if unique_name is not None: |
| 285 | - namespace_name, base = split_unique_name(unique_name) |
| 286 | - return lookup_branch_namespace(namespace_name), base |
| 287 | - if url is None: |
| 288 | - basename = 'merge' |
| 289 | - else: |
| 290 | - basename = urlparse(url)[2].split('/')[-1] |
| 291 | - namespace = target_branch.target.getNamespace(submitter) |
| 292 | - return namespace, basename |
| 293 | - |
| 294 | - def _getNewBranch(self, branch_type, url, target, submitter): |
| 295 | - """Return a new database branch. |
| 296 | - |
| 297 | - :param branch_type: The type of branch to create. |
| 298 | - :param url: The public location of the branch to create. |
| 299 | - :param product: The product associated with the branch to create. |
| 300 | - :param submitter: The person who requested the merge. |
| 301 | - """ |
| 302 | - namespace, basename = self._getNewBranchInfo(url, target, submitter) |
| 303 | - if branch_type == BranchType.REMOTE: |
| 304 | - db_url = url |
| 305 | - else: |
| 306 | - db_url = None |
| 307 | - return namespace.createBranchWithPrefix( |
| 308 | - branch_type, basename, submitter, url=db_url) |
| 309 | - |
| 310 | - def _getSourceNoBundle(self, md, target, submitter): |
| 311 | - """Get a source branch for a merge directive with no bundle.""" |
| 312 | - source_db_branch = getUtility(IBranchLookup).getByUrl( |
| 313 | - md.source_branch) |
| 314 | - if source_db_branch is None: |
| 315 | - source_db_branch = self._getNewBranch( |
| 316 | - BranchType.REMOTE, md.source_branch, target, submitter) |
| 317 | - return source_db_branch |
| 318 | - |
| 319 | - def _getOrCreateDBBranch(self, md, db_target, submitter): |
| 320 | - """Return the source branch, creating a new branch if necessary.""" |
| 321 | - db_source = None |
| 322 | - if md.source_branch is not None: |
| 323 | - db_source = getUtility(IBranchLookup).getByUrl(md.source_branch) |
| 324 | - if db_source is None: |
| 325 | - db_source = self._getNewBranch( |
| 326 | - BranchType.HOSTED, md.source_branch, db_target, submitter) |
| 327 | - # Commit the transaction to make sure the new source branch is |
| 328 | - # visible to the XMLRPC server which provides the virtual file |
| 329 | - # system information. |
| 330 | - transaction.commit() |
| 331 | - return db_source |
| 332 | - |
| 333 | - def _openSourceBzrBranch(self, source_url, target_url, stacked_url): |
| 334 | - """Open the source bzr branch, creating a new branch if necessary.""" |
| 335 | - try: |
| 336 | - return Branch.open(source_url) |
| 337 | - except NotBranchError: |
| 338 | - bzr_target = Branch.open(target_url) |
| 339 | - transport = get_transport( |
| 340 | - source_url, |
| 341 | - possible_transports=[bzr_target.bzrdir.root_transport]) |
| 342 | - bzrdir = bzr_target.bzrdir.sprout( |
| 343 | - transport.base, bzr_target.last_revision(), |
| 344 | - force_new_repo=True, stacked=True, create_tree_if_local=False, |
| 345 | - possible_transports=[transport], source_branch=bzr_target) |
| 346 | - bzr_branch = bzrdir.open_branch() |
| 347 | - # Set the stacked url to be the relative url for the target. |
| 348 | - bzr_branch.set_stacked_on_url(stacked_url) |
| 349 | - return bzr_branch |
| 350 | - |
| 351 | - def _getSourceWithBundle(self, md, db_target, submitter): |
| 352 | - """Get a source branch for a merge directive with a bundle.""" |
| 353 | - db_source = self._getOrCreateDBBranch(md, db_target, submitter) |
| 354 | - # Make sure that the target branch is stackable so that we only |
| 355 | - # install the revisions unique to the source branch. If the target |
| 356 | - # branch is not stackable, return the existing branch or a new hosted |
| 357 | - # source branch - one that has *no* Bazaar data. Together these |
| 358 | - # prevent users from using Launchpad disk space at a rate that is |
| 359 | - # disproportionately greater than data uploaded. |
| 360 | - mirrored_bzr_target = db_target.getBzrBranch() |
| 361 | - if not is_branch_stackable(mirrored_bzr_target): |
| 362 | - return db_source |
| 363 | - assert db_source.branch_type == BranchType.HOSTED, ( |
| 364 | - "Source branch is not hosted.") |
| 365 | - |
| 366 | - # Create the LP server as if the submitter was pushing a branch to LP. |
| 367 | - lp_server = get_lp_server(submitter.id) |
| 368 | - lp_server.start_server() |
| 369 | - try: |
| 370 | - source_url = urljoin(lp_server.get_url(), db_source.unique_name) |
| 371 | - target_url = urljoin(lp_server.get_url(), db_target.unique_name) |
| 372 | - stacked_url = '/' + db_target.unique_name |
| 373 | - bzr_source = self._openSourceBzrBranch( |
| 374 | - source_url, target_url, stacked_url) |
| 375 | - if is_branch_stackable(bzr_source): |
| 376 | - # Set the stacked on URL if not set. |
| 377 | - try: |
| 378 | - bzr_source.get_stacked_on_url() |
| 379 | - except NotStacked: |
| 380 | - # We don't currently support pulling in the revisions if |
| 381 | - # the source branch exists and isn't stacked. |
| 382 | - # XXX Tim Penhey 2010-07-27 bug 610292 |
| 383 | - # We should fail here and return an oops email to the |
| 384 | - # user. |
| 385 | - return db_source |
| 386 | - self._pullRevisionsFromMergeDirectiveIntoSourceBranch( |
| 387 | - md, target_url, bzr_source) |
| 388 | - # Get the puller to pull the branch into the mirrored area. |
| 389 | - formats = get_branch_formats(bzr_source) |
| 390 | - db_source.branchChanged( |
| 391 | - stacked_url, bzr_source.last_revision(), *formats) |
| 392 | - return db_source |
| 393 | - finally: |
| 394 | - lp_server.stop_server() |
| 395 | - |
| 396 | - def _pullRevisionsFromMergeDirectiveIntoSourceBranch(self, md, |
| 397 | - target_url, |
| 398 | - bzr_branch): |
| 399 | - """Pull the revisions from the merge directive into the branch. |
| 400 | - |
| 401 | - :param md: The merge directive |
| 402 | - :param target_url: The URL of the branch that the merge directive is |
| 403 | - targetting using the user's LP transport. |
| 404 | - :param bzr_branch: The bazaar branch entity for the branch that the |
| 405 | - revisions from the merge directive are being pulled into. |
| 406 | - """ |
| 407 | - # Tell the merge directive to use the user's LP transport URL to get |
| 408 | - # access to any needed but not supplied revisions. |
| 409 | - md.target_branch = target_url |
| 410 | - md.install_revisions(bzr_branch.repository) |
| 411 | - bzr_branch.lock_write() |
| 412 | - try: |
| 413 | - bzr_branch.pull(bzr_branch, stop_revision=md.revision_id, |
| 414 | - overwrite=True) |
| 415 | - finally: |
| 416 | - bzr_branch.unlock() |
| 417 | - |
| 418 | - def findMergeDirectiveAndComment(self, message): |
| 419 | - """Extract the comment and Merge Directive from a SignedMessage.""" |
| 420 | - body = None |
| 421 | - md = None |
| 422 | - for part in message.walk(): |
| 423 | - if part.is_multipart(): |
| 424 | - continue |
| 425 | - payload = part.get_payload(decode=True) |
| 426 | - content_type = part.get('Content-type', 'text/plain').lower() |
| 427 | - if content_type.startswith('text/plain'): |
| 428 | - body = payload |
| 429 | - charset = part.get_param('charset') |
| 430 | - if charset is not None: |
| 431 | - body = body.decode(charset) |
| 432 | - try: |
| 433 | - md = MergeDirective.from_lines(payload.splitlines(True)) |
| 434 | - except NotAMergeDirective: |
| 435 | - pass |
| 436 | - if None not in (body, md): |
| 437 | - return body, md |
| 438 | - else: |
| 439 | - raise MissingMergeDirective() |
| 440 | - |
| 441 | - def processMergeProposal(self, message): |
| 442 | - """Generate a merge proposal (and comment) from an email message. |
| 443 | - |
| 444 | - The message is expected to contain a merge directive in one of its |
| 445 | - parts. Its values are used to generate a BranchMergeProposal. |
| 446 | - If the message has a non-empty body, it is turned into a |
| 447 | - CodeReviewComment. |
| 448 | - """ |
| 449 | - submitter = getUtility(ILaunchBag).user |
| 450 | - try: |
| 451 | - email_body_text, md = self.findMergeDirectiveAndComment(message) |
| 452 | - except MissingMergeDirective: |
| 453 | - body = get_error_message( |
| 454 | - 'missingmergedirective.txt', |
| 455 | - error_templates=error_templates) |
| 456 | - simple_sendmail('merge@code.launchpad.net', |
| 457 | - [message.get('from')], |
| 458 | - 'Error Creating Merge Proposal', body) |
| 459 | - return |
| 460 | - oops_message = ( |
| 461 | - 'target: %r source: %r' % |
| 462 | - (md.target_branch, md.source_branch)) |
| 463 | - with globalErrorUtility.oopsMessage(oops_message): |
| 464 | - try: |
| 465 | - source, target = self._acquireBranchesForProposal( |
| 466 | - md, submitter) |
| 467 | - except NonLaunchpadTarget: |
| 468 | - body = get_error_message('nonlaunchpadtarget.txt', |
| 469 | - error_templates=error_templates, |
| 470 | - target_branch=md.target_branch) |
| 471 | - simple_sendmail('merge@code.launchpad.net', |
| 472 | - [message.get('from')], |
| 473 | - 'Error Creating Merge Proposal', body) |
| 474 | - return |
| 475 | - except BranchCreationException, e: |
| 476 | - body = get_error_message( |
| 477 | - 'branch-creation-exception.txt', |
| 478 | - error_templates=error_templates, |
| 479 | - reason=e) |
| 480 | - simple_sendmail('merge@code.launchpad.net', |
| 481 | - [message.get('from')], |
| 482 | - 'Error Creating Merge Proposal', body) |
| 483 | - return |
| 484 | - with globalErrorUtility.oopsMessage( |
| 485 | - 'target: %r source: %r' % (target, source)): |
| 486 | - try: |
| 487 | - # When creating a merge proposal, we need to gather all |
| 488 | - # necessary arguments to addLandingTarget(). So from the email |
| 489 | - # body we need to extract: reviewer, review type, description. |
| 490 | - description = None |
| 491 | - review_requests = [] |
| 492 | - email_body_text = email_body_text.strip() |
| 493 | - if email_body_text != '': |
| 494 | - description = email_body_text |
| 495 | - review_args = parse_commands( |
| 496 | - email_body_text, ['reviewer']) |
| 497 | - if len(review_args) > 0: |
| 498 | - cmd, args = review_args[0] |
| 499 | - review_request = ( |
| 500 | - CodeEmailCommands.parseReviewRequest(cmd, args)) |
| 501 | - review_requests.append(review_request) |
| 502 | - |
| 503 | - bmp = source.addLandingTarget(submitter, target, |
| 504 | - needs_review=True, |
| 505 | - description=description, |
| 506 | - review_requests=review_requests) |
| 507 | - return bmp |
| 508 | - |
| 509 | - except BranchMergeProposalExists: |
| 510 | - body = get_error_message( |
| 511 | - 'branchmergeproposal-exists.txt', |
| 512 | - error_templates=error_templates, |
| 513 | - source_branch=source.bzr_identity, |
| 514 | - target_branch=target.bzr_identity) |
| 515 | - simple_sendmail('merge@code.launchpad.net', |
| 516 | - [message.get('from')], |
| 517 | - 'Error Creating Merge Proposal', body) |
| 518 | - transaction.abort() |
| 519 | - except IncomingEmailError, error: |
| 520 | - send_process_error_notification( |
| 521 | - str(submitter.preferredemail.email), |
| 522 | - 'Submit Request Failure', |
| 523 | - error.message, email_body_text, error.failing_command) |
| 524 | - transaction.abort() |
| 525 | |
| 526 | === removed file 'lib/lp/code/mail/errortemplates/branch-creation-exception.txt' |
| 527 | --- lib/lp/code/mail/errortemplates/branch-creation-exception.txt 2011-08-11 22:18:34 +0000 |
| 528 | +++ lib/lp/code/mail/errortemplates/branch-creation-exception.txt 1970-01-01 00:00:00 +0000 |
| 529 | @@ -1,2 +0,0 @@ |
| 530 | -Launchpad cannot create the branch requested by your merge directive: |
| 531 | -%(reason)s |
| 532 | |
| 533 | === removed file 'lib/lp/code/mail/errortemplates/missingmergedirective.txt' |
| 534 | --- lib/lp/code/mail/errortemplates/missingmergedirective.txt 2011-08-11 22:18:34 +0000 |
| 535 | +++ lib/lp/code/mail/errortemplates/missingmergedirective.txt 1970-01-01 00:00:00 +0000 |
| 536 | @@ -1,2 +0,0 @@ |
| 537 | -Your email did not contain a merge directive. Please resend your email with |
| 538 | -the merge directive attached. |
| 539 | |
| 540 | === modified file 'lib/lp/code/mail/tests/test_codehandler.py' |
| 541 | --- lib/lp/code/mail/tests/test_codehandler.py 2012-01-20 15:42:44 +0000 |
| 542 | +++ lib/lp/code/mail/tests/test_codehandler.py 2012-04-30 14:10:29 +0000 |
| 543 | @@ -1,4 +1,4 @@ |
| 544 | -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the |
| 545 | +# Copyright 2009-2012 Canonical Ltd. This software is licensed under the |
| 546 | # GNU Affero General Public License version 3 (see the file LICENSE). |
| 547 | |
| 548 | """Testing the CodeHandler.""" |
| 549 | @@ -8,64 +8,38 @@ |
| 550 | from difflib import unified_diff |
| 551 | from textwrap import dedent |
| 552 | |
| 553 | -from bzrlib.branch import Branch |
| 554 | -from bzrlib.urlutils import join as urljoin |
| 555 | -from bzrlib.workingtree import WorkingTree |
| 556 | from storm.store import Store |
| 557 | import transaction |
| 558 | -from zope.component import getUtility |
| 559 | -from zope.interface import ( |
| 560 | - directlyProvidedBy, |
| 561 | - directlyProvides, |
| 562 | - ) |
| 563 | from zope.security.management import setSecurityPolicy |
| 564 | -from zope.security.proxy import removeSecurityProxy |
| 565 | |
| 566 | from lp.code.enums import ( |
| 567 | BranchMergeProposalStatus, |
| 568 | BranchSubscriptionNotificationLevel, |
| 569 | - BranchType, |
| 570 | - BranchVisibilityRule, |
| 571 | CodeReviewNotificationLevel, |
| 572 | CodeReviewVote, |
| 573 | ) |
| 574 | -from lp.code.interfaces.branchlookup import IBranchLookup |
| 575 | from lp.code.mail.codehandler import ( |
| 576 | AddReviewerEmailCommand, |
| 577 | CodeEmailCommands, |
| 578 | CodeHandler, |
| 579 | CodeReviewEmailCommandExecutionContext, |
| 580 | InvalidBranchMergeProposalAddress, |
| 581 | - MissingMergeDirective, |
| 582 | - NonLaunchpadTarget, |
| 583 | UpdateStatusEmailCommand, |
| 584 | VoteEmailCommand, |
| 585 | ) |
| 586 | from lp.code.model.branchmergeproposaljob import ( |
| 587 | BranchMergeProposalJob, |
| 588 | BranchMergeProposalJobType, |
| 589 | - CreateMergeProposalJob, |
| 590 | - MergeProposalNeedsReviewEmailJob, |
| 591 | ) |
| 592 | from lp.code.model.diff import PreviewDiff |
| 593 | from lp.code.tests.helpers import make_merge_proposal_without_reviewers |
| 594 | -from lp.codehosting.vfs import get_lp_server |
| 595 | -from lp.registry.interfaces.person import IPersonSet |
| 596 | from lp.services.config import config |
| 597 | -from lp.services.job.runner import JobRunner |
| 598 | from lp.services.mail.handlers import mail_handlers |
| 599 | from lp.services.mail.interfaces import ( |
| 600 | EmailProcessingError, |
| 601 | - IWeaklyAuthenticatedPrincipal, |
| 602 | ) |
| 603 | from lp.services.messages.model.message import MessageSet |
| 604 | -from lp.services.osutils import override_environ |
| 605 | from lp.services.webapp.authorization import LaunchpadSecurityPolicy |
| 606 | -from lp.services.webapp.interaction import ( |
| 607 | - get_current_principal, |
| 608 | - setupInteraction, |
| 609 | - ) |
| 610 | -from lp.services.webapp.interfaces import IPlacelessAuthUtility |
| 611 | from lp.testing import ( |
| 612 | login, |
| 613 | login_person, |
| 614 | @@ -249,7 +223,7 @@ |
| 615 | review needs-fixing |
| 616 | |
| 617 | |
| 618 | - -- |
| 619 | + --\x20 |
| 620 | For more information about using Launchpad by e-mail, see |
| 621 | https://help.launchpad.net/EmailInterface |
| 622 | or send an email to help@launchpad.net"""), |
| 623 | @@ -394,286 +368,18 @@ |
| 624 | self.assertRaises(InvalidBranchMergeProposalAddress, |
| 625 | self.code_handler.getBranchMergeProposal, 'mp+abc@') |
| 626 | |
| 627 | - def test_acquireBranchesForProposal(self): |
| 628 | - """Ensure CodeHandler._acquireBranchesForProposal works.""" |
| 629 | - target_branch = self.factory.makeAnyBranch() |
| 630 | - source_branch = self.factory.makeAnyBranch() |
| 631 | - md = self.factory.makeMergeDirective(source_branch, target_branch) |
| 632 | - submitter = self.factory.makePerson() |
| 633 | - switch_dbuser(config.processmail.dbuser) |
| 634 | - mp_source, mp_target = self.code_handler._acquireBranchesForProposal( |
| 635 | - md, submitter) |
| 636 | - self.assertEqual(mp_source, source_branch) |
| 637 | - self.assertEqual(mp_target, target_branch) |
| 638 | - transaction.commit() |
| 639 | - |
| 640 | - def test_acquireBranchesForProposalRemoteTarget(self): |
| 641 | - """CodeHandler._acquireBranchesForProposal fails on remote targets.""" |
| 642 | - source_branch = self.factory.makeAnyBranch() |
| 643 | - md = self.factory.makeMergeDirective( |
| 644 | - source_branch, target_branch_url='http://example.com') |
| 645 | - submitter = self.factory.makePerson() |
| 646 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 647 | - self.assertRaises( |
| 648 | - NonLaunchpadTarget, self.code_handler._acquireBranchesForProposal, |
| 649 | - md, submitter) |
| 650 | - transaction.commit() |
| 651 | - |
| 652 | - def test_acquireBranchesForProposalRemoteSource(self): |
| 653 | - """CodeHandler._acquireBranchesForProposal allows remote sources. |
| 654 | - |
| 655 | - If there's no existing remote branch, it creates one, using |
| 656 | - the suffix of the url as a branch name seed. |
| 657 | - """ |
| 658 | - target_branch = self.factory.makeProductBranch() |
| 659 | - source_branch_url = 'http://example.com/suffix' |
| 660 | - md = self.factory.makeMergeDirective( |
| 661 | - source_branch_url=source_branch_url, target_branch=target_branch) |
| 662 | - branches = getUtility(IBranchLookup) |
| 663 | - self.assertIs(None, branches.getByUrl(source_branch_url)) |
| 664 | - submitter = self.factory.makePerson() |
| 665 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 666 | - mp_source, mp_target = self.code_handler._acquireBranchesForProposal( |
| 667 | - md, submitter) |
| 668 | - self.assertEqual(mp_target, target_branch) |
| 669 | - self.assertIsNot(None, mp_source) |
| 670 | - self.assertEqual(mp_source, branches.getByUrl(source_branch_url)) |
| 671 | - self.assertEqual(BranchType.REMOTE, mp_source.branch_type) |
| 672 | - self.assertEqual(mp_target.product, mp_source.product) |
| 673 | - self.assertEqual('suffix', mp_source.name) |
| 674 | - transaction.commit() |
| 675 | - |
| 676 | - def test_acquireBranchesForProposalRemoteSourceDupeName(self): |
| 677 | - """CodeHandler._acquireBranchesForProposal creates names safely. |
| 678 | - |
| 679 | - When creating a new branch, it uses the suffix of the url as a branch |
| 680 | - name seed. If there is already a branch with that name, it appends |
| 681 | - a numeric suffix. |
| 682 | - """ |
| 683 | - target_branch = self.factory.makeProductBranch() |
| 684 | - source_branch_url = 'http://example.com/suffix' |
| 685 | - md = self.factory.makeMergeDirective( |
| 686 | - source_branch_url=source_branch_url, target_branch=target_branch) |
| 687 | - submitter = self.factory.makePerson() |
| 688 | - self.factory.makeProductBranch( |
| 689 | - product=target_branch.product, name='suffix', owner=submitter) |
| 690 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 691 | - mp_source, mp_target = self.code_handler._acquireBranchesForProposal( |
| 692 | - md, submitter) |
| 693 | - self.assertEqual('suffix-1', mp_source.name) |
| 694 | - transaction.commit() |
| 695 | - |
| 696 | - def test_findMergeDirectiveAndComment(self): |
| 697 | - """findMergeDirectiveAndComment works.""" |
| 698 | - md = self.factory.makeMergeDirective() |
| 699 | - message = self.factory.makeSignedMessage( |
| 700 | - body='Hi!\n', attachment_contents=''.join(md.to_lines()), |
| 701 | - force_transfer_encoding=True) |
| 702 | - code_handler = CodeHandler() |
| 703 | - switch_dbuser(config.processmail.dbuser) |
| 704 | - comment, md2 = code_handler.findMergeDirectiveAndComment(message) |
| 705 | - self.assertEqual('Hi!\n', comment) |
| 706 | - self.assertEqual(md.revision_id, md2.revision_id) |
| 707 | - self.assertEqual(md.target_branch, md2.target_branch) |
| 708 | - transaction.commit() |
| 709 | - |
| 710 | - def test_findMergeDirectiveAndCommentEmptyBody(self): |
| 711 | - """findMergeDirectiveAndComment handles empty message bodies. |
| 712 | - |
| 713 | - Empty message bodies are returned verbatim. |
| 714 | - """ |
| 715 | - md = self.factory.makeMergeDirective() |
| 716 | - message = self.factory.makeSignedMessage( |
| 717 | - body='', attachment_contents=''.join(md.to_lines())) |
| 718 | - switch_dbuser(config.processmail.dbuser) |
| 719 | - code_handler = CodeHandler() |
| 720 | - comment, md2 = code_handler.findMergeDirectiveAndComment(message) |
| 721 | - self.assertEqual('', comment) |
| 722 | - transaction.commit() |
| 723 | - |
| 724 | - def test_findMergeDirectiveAndComment_no_content_type(self): |
| 725 | - """Parts with no content-type are treated as text/plain.""" |
| 726 | - md = self.factory.makeMergeDirective() |
| 727 | - message = self.factory.makeSignedMessage( |
| 728 | - body='', attachment_contents=''.join(md.to_lines())) |
| 729 | - body = message.get_payload()[0] |
| 730 | - del body['Content-type'] |
| 731 | - body.set_payload('body') |
| 732 | - switch_dbuser(config.processmail.dbuser) |
| 733 | - code_handler = CodeHandler() |
| 734 | - comment, md2 = code_handler.findMergeDirectiveAndComment(message) |
| 735 | - self.assertEqual('body', comment) |
| 736 | - |
| 737 | - def test_findMergeDirectiveAndComment_case_insensitive(self): |
| 738 | - """findMergeDirectiveAndComment uses case-insensitive content-type.""" |
| 739 | - md = self.factory.makeMergeDirective() |
| 740 | - message = self.factory.makeSignedMessage( |
| 741 | - body='', attachment_contents=''.join(md.to_lines())) |
| 742 | - body = message.get_payload()[0] |
| 743 | - # Unlike dicts, messages append when you assign to a key. So |
| 744 | - # we must delete the first Content-type before adding another. |
| 745 | - del body['Content-type'] |
| 746 | - body['Content-type'] = 'Text/Plain' |
| 747 | - body.set_payload('body') |
| 748 | - switch_dbuser(config.processmail.dbuser) |
| 749 | - code_handler = CodeHandler() |
| 750 | - comment, md2 = code_handler.findMergeDirectiveAndComment(message) |
| 751 | - self.assertEqual('body', comment) |
| 752 | - |
| 753 | - def test_findMergeDirectiveAndCommentUnicodeBody(self): |
| 754 | - """findMergeDirectiveAndComment returns unicode comments.""" |
| 755 | - md = self.factory.makeMergeDirective() |
| 756 | - message = self.factory.makeSignedMessage( |
| 757 | - body=u'\u1234', attachment_contents=''.join(md.to_lines())) |
| 758 | - switch_dbuser(config.processmail.dbuser) |
| 759 | - code_handler = CodeHandler() |
| 760 | - comment, md2 = code_handler.findMergeDirectiveAndComment(message) |
| 761 | - self.assertEqual(u'\u1234', comment) |
| 762 | - transaction.commit() |
| 763 | - |
| 764 | - def test_findMergeDirectiveAndCommentNoMergeDirective(self): |
| 765 | - """findMergeDirectiveAndComment handles missing merge directives. |
| 766 | - |
| 767 | - MissingMergeDirective is raised when no merge directive is present. |
| 768 | - """ |
| 769 | - message = self.factory.makeSignedMessage(body='Hi!\n') |
| 770 | - switch_dbuser(config.processmail.dbuser) |
| 771 | - code_handler = CodeHandler() |
| 772 | - self.assertRaises(MissingMergeDirective, |
| 773 | - code_handler.findMergeDirectiveAndComment, message) |
| 774 | - transaction.commit() |
| 775 | - |
| 776 | - def test_processMergeProposal(self): |
| 777 | - """processMergeProposal creates a merge proposal and comment.""" |
| 778 | - message, file_alias, source, target = ( |
| 779 | - self.factory.makeMergeDirectiveEmail()) |
| 780 | - # Add some revisions so the proposal is ready. |
| 781 | - self.factory.makeRevisionsForBranch(source, count=1) |
| 782 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 783 | - code_handler = CodeHandler() |
| 784 | - pop_notifications() |
| 785 | - bmp = code_handler.processMergeProposal(message) |
| 786 | - self.assertEqual(source, bmp.source_branch) |
| 787 | - self.assertEqual(target, bmp.target_branch) |
| 788 | - self.assertEqual('Hi!', bmp.description) |
| 789 | - # No emails are sent. |
| 790 | - messages = pop_notifications() |
| 791 | - self.assertEqual(0, len(messages)) |
| 792 | - # Only a job created. |
| 793 | - runner = JobRunner.fromReady(MergeProposalNeedsReviewEmailJob) |
| 794 | - self.assertEqual(1, len(list(runner.jobs))) |
| 795 | - transaction.commit() |
| 796 | - |
| 797 | - def test_processMergeProposalEmptyMessage(self): |
| 798 | - """processMergeProposal handles empty message bodies. |
| 799 | - |
| 800 | - Messages with empty bodies produce merge proposals only, not |
| 801 | - comments. |
| 802 | - """ |
| 803 | - message, file_alias, source_branch, target_branch = ( |
| 804 | - self.factory.makeMergeDirectiveEmail(body=' ')) |
| 805 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 806 | - code_handler = CodeHandler() |
| 807 | - bmp = code_handler.processMergeProposal(message) |
| 808 | - self.assertEqual(source_branch, bmp.source_branch) |
| 809 | - self.assertEqual(target_branch, bmp.target_branch) |
| 810 | - self.assertIs(None, bmp.description) |
| 811 | - self.assertEqual(0, bmp.all_comments.count()) |
| 812 | - transaction.commit() |
| 813 | - |
| 814 | - def test_processMergeDirectiveEmailNeedsGPG(self): |
| 815 | - """process creates a merge proposal from a merge directive email.""" |
| 816 | - message, file_alias, source, target = ( |
| 817 | - self.factory.makeMergeDirectiveEmail()) |
| 818 | - # Ensure the message is stored in the librarian. |
| 819 | - # mail.incoming.handleMail also explicitly does this. |
| 820 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 821 | - code_handler = CodeHandler() |
| 822 | - # In order to fake a non-gpg signed email, we say that the current |
| 823 | - # principal direcly provides IWeaklyAuthenticatePrincipal, which is |
| 824 | - # what the surrounding code does. |
| 825 | - cur_principal = get_current_principal() |
| 826 | - directlyProvides( |
| 827 | - cur_principal, directlyProvidedBy(cur_principal), |
| 828 | - IWeaklyAuthenticatedPrincipal) |
| 829 | - code_handler.process(message, 'merge@code.launchpad.net', file_alias) |
| 830 | - |
| 831 | - notification = pop_notifications()[0] |
| 832 | - self.assertEqual('Submit Request Failure', notification['subject']) |
| 833 | - # The returned message is a multipart message, the first part is |
| 834 | - # the message, and the second is the original message. |
| 835 | - message, original = notification.get_payload() |
| 836 | - self.assertEqual(dedent("""\ |
| 837 | - An error occurred while processing a mail you sent to Launchpad's email |
| 838 | - interface. |
| 839 | - |
| 840 | - |
| 841 | - Error message: |
| 842 | - |
| 843 | - All emails to merge@code.launchpad.net must be signed with your OpenPGP |
| 844 | - key. |
| 845 | - |
| 846 | - |
| 847 | - -- |
| 848 | - For more information about using Launchpad by e-mail, see |
| 849 | - https://help.launchpad.net/EmailInterface |
| 850 | - or send an email to help@launchpad.net"""), |
| 851 | - message.get_payload(decode=True)) |
| 852 | - |
| 853 | def test_processWithMergeDirectiveEmail(self): |
| 854 | - """process creates a merge proposal from a merge directive email.""" |
| 855 | - message, file_alias, source, target = ( |
| 856 | - self.factory.makeMergeDirectiveEmail()) |
| 857 | - # Ensure the message is stored in the librarian. |
| 858 | - # mail.incoming.handleMail also explicitly does this. |
| 859 | - switch_dbuser(config.processmail.dbuser) |
| 860 | - code_handler = CodeHandler() |
| 861 | - self.assertEqual(0, source.landing_targets.count()) |
| 862 | - code_handler.process(message, 'merge@code.launchpad.net', file_alias) |
| 863 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 864 | - JobRunner.fromReady(CreateMergeProposalJob).runAll() |
| 865 | - self.assertEqual(target, source.landing_targets[0].target_branch) |
| 866 | - # Ensure the DB operations violate no constraints. |
| 867 | - Store.of(source).flush() |
| 868 | - |
| 869 | - def test_processWithUnicodeMergeDirectiveEmail(self): |
| 870 | - """process creates a comment from a unicode message body.""" |
| 871 | - message, file_alias, source, target = ( |
| 872 | - self.factory.makeMergeDirectiveEmail(body=u'\u1234')) |
| 873 | - # Ensure the message is stored in the librarian. |
| 874 | - # mail.incoming.handleMail also explicitly does this. |
| 875 | - switch_dbuser(config.processmail.dbuser) |
| 876 | - code_handler = CodeHandler() |
| 877 | - self.assertEqual(0, source.landing_targets.count()) |
| 878 | - code_handler.process(message, 'merge@code.launchpad.net', file_alias) |
| 879 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 880 | - JobRunner.fromReady(CreateMergeProposalJob).runAll() |
| 881 | - proposal = source.landing_targets[0] |
| 882 | - self.assertEqual(u'\u1234', proposal.description) |
| 883 | - # Ensure the DB operations violate no constraints. |
| 884 | - Store.of(proposal).flush() |
| 885 | - |
| 886 | - def test_processMergeProposalReviewerRequested(self): |
| 887 | - # The commands in the merge proposal are parsed. |
| 888 | - eric = self.factory.makePerson(name="eric") |
| 889 | - message, file_alias, source_branch, target_branch = ( |
| 890 | - self.factory.makeMergeDirectiveEmail(body=dedent("""\ |
| 891 | - This is the comment. |
| 892 | - |
| 893 | - reviewer eric |
| 894 | - """))) |
| 895 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 896 | - code_handler = CodeHandler() |
| 897 | - pop_notifications() |
| 898 | - bmp = code_handler.processMergeProposal(message) |
| 899 | - pending_reviews = list(bmp.votes) |
| 900 | - self.assertEqual(1, len(pending_reviews)) |
| 901 | - self.assertEqual(eric, pending_reviews[0].reviewer) |
| 902 | - # No emails are sent. |
| 903 | - messages = pop_notifications() |
| 904 | - self.assertEqual(0, len(messages)) |
| 905 | - # Ensure the DB operations violate no constraints. |
| 906 | - Store.of(bmp).flush() |
| 907 | + """process errors if merge@ address used.""" |
| 908 | + message = self.factory.makeSignedMessage() |
| 909 | + file_alias = self.factory.makeLibraryFileAlias( |
| 910 | + content=message.as_string()) |
| 911 | + # mail.incoming.handleMail also explicitly does this. |
| 912 | + switch_dbuser(config.processmail.dbuser) |
| 913 | + code_handler = CodeHandler() |
| 914 | + code_handler.process(message, 'merge@code.launchpad.net', file_alias) |
| 915 | + notification = pop_notifications()[0] |
| 916 | + self.assertEqual( |
| 917 | + 'Merge directive not supported.', notification['Subject']) |
| 918 | |
| 919 | def test_reviewer_with_diff(self): |
| 920 | """Requesting a review with a diff works.""" |
| 921 | @@ -695,161 +401,6 @@ |
| 922 | [vote] = bmp.votes |
| 923 | self.assertEqual(eric, vote.reviewer) |
| 924 | |
| 925 | - def test_processMergeProposalDefaultReviewer(self): |
| 926 | - # If no reviewer was requested in the comment body, then the default |
| 927 | - # reviewer of the target branch is used. |
| 928 | - message, file_alias, source_branch, target_branch = ( |
| 929 | - self.factory.makeMergeDirectiveEmail(body=dedent("""\ |
| 930 | - This is the comment. |
| 931 | - """))) |
| 932 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 933 | - code_handler = CodeHandler() |
| 934 | - pop_notifications() |
| 935 | - bmp = code_handler.processMergeProposal(message) |
| 936 | - # If no reviewer is specified, then the default reviewer of the target |
| 937 | - # branch is requested to review. |
| 938 | - pending_reviews = list(bmp.votes) |
| 939 | - self.assertEqual(1, len(pending_reviews)) |
| 940 | - self.assertEqual( |
| 941 | - target_branch.code_reviewer, |
| 942 | - pending_reviews[0].reviewer) |
| 943 | - # No emails are sent. |
| 944 | - messages = pop_notifications() |
| 945 | - self.assertEqual(0, len(messages)) |
| 946 | - # Ensure the DB operations violate no constraints. |
| 947 | - Store.of(target_branch).flush() |
| 948 | - |
| 949 | - def test_processMergeProposalExists(self): |
| 950 | - """processMergeProposal raises BranchMergeProposalExists |
| 951 | - |
| 952 | - If there is already a merge proposal with the same target and source |
| 953 | - branches of the merge directive, an email is sent to the user. |
| 954 | - """ |
| 955 | - message, file_alias, source, target = ( |
| 956 | - self.factory.makeMergeDirectiveEmail()) |
| 957 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 958 | - code_handler = CodeHandler() |
| 959 | - code_handler.processMergeProposal(message) |
| 960 | - pop_notifications() |
| 961 | - transaction.commit() |
| 962 | - code_handler.processMergeProposal(message) |
| 963 | - [notification] = pop_notifications() |
| 964 | - self.assertEqual( |
| 965 | - notification['Subject'], 'Error Creating Merge Proposal') |
| 966 | - self.assertEqual( |
| 967 | - notification.get_payload(decode=True), |
| 968 | - 'The branch %s is already proposed for merging into %s.\n\n' |
| 969 | - % (source.bzr_identity, target.bzr_identity)) |
| 970 | - self.assertEqual(notification['to'], message['from']) |
| 971 | - |
| 972 | - def test_processMissingMergeDirective(self): |
| 973 | - """process sends an email if the original email lacks an attachment. |
| 974 | - """ |
| 975 | - message = self.factory.makeSignedMessage(body='A body', |
| 976 | - subject='A subject', attachment_contents='') |
| 977 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 978 | - code_handler = CodeHandler() |
| 979 | - code_handler.processMergeProposal(message) |
| 980 | - transaction.commit() |
| 981 | - [notification] = pop_notifications() |
| 982 | - |
| 983 | - self.assertEqual( |
| 984 | - notification['Subject'], 'Error Creating Merge Proposal') |
| 985 | - self.assertEqual( |
| 986 | - notification.get_payload(), |
| 987 | - 'Your email did not contain a merge directive. Please resend ' |
| 988 | - 'your email with\nthe merge directive attached.\n') |
| 989 | - self.assertEqual(notification['to'], |
| 990 | - message['from']) |
| 991 | - |
| 992 | - def makeTargetBranch(self): |
| 993 | - """Helper for getNewBranchInfo tests.""" |
| 994 | - owner = self.factory.makePerson(name='target-owner') |
| 995 | - product = self.factory.makeProduct(name='target-product') |
| 996 | - return self.factory.makeProductBranch(product=product, owner=owner) |
| 997 | - |
| 998 | - def test_getNewBranchInfoNoURL(self): |
| 999 | - """If no URL, target namespace is used, with 'merge' basename.""" |
| 1000 | - submitter = self.factory.makePerson(name='merge-submitter') |
| 1001 | - target = self.makeTargetBranch() |
| 1002 | - code_handler = CodeHandler() |
| 1003 | - namespace, base = code_handler._getNewBranchInfo( |
| 1004 | - None, target, submitter) |
| 1005 | - self.assertEqual('~merge-submitter/target-product', namespace.name) |
| 1006 | - self.assertEqual('merge', base) |
| 1007 | - |
| 1008 | - def test_getNewBranchInfoRemoteURL(self): |
| 1009 | - """If a URL is provided, its base is used, with target namespace.""" |
| 1010 | - submitter = self.factory.makePerson(name='merge-submitter') |
| 1011 | - target = self.makeTargetBranch() |
| 1012 | - code_handler = CodeHandler() |
| 1013 | - namespace, base = code_handler._getNewBranchInfo( |
| 1014 | - 'http://foo/bar', target, submitter) |
| 1015 | - self.assertEqual('~merge-submitter/target-product', namespace.name) |
| 1016 | - self.assertEqual('bar', base) |
| 1017 | - |
| 1018 | - def test_getNewBranchInfoRemoteURLTrailingSlash(self): |
| 1019 | - """Trailing slashes are ignored when determining base.""" |
| 1020 | - submitter = self.factory.makePerson(name='merge-submitter') |
| 1021 | - target = self.makeTargetBranch() |
| 1022 | - code_handler = CodeHandler() |
| 1023 | - namespace, base = code_handler._getNewBranchInfo( |
| 1024 | - 'http://foo/bar/', target, submitter) |
| 1025 | - self.assertEqual('~merge-submitter/target-product', namespace.name) |
| 1026 | - self.assertEqual('bar', base) |
| 1027 | - |
| 1028 | - def test_getNewBranchInfoLPURL(self): |
| 1029 | - """If an LP URL is provided, we attempt to reproduce it exactly.""" |
| 1030 | - submitter = self.factory.makePerson(name='merge-submitter') |
| 1031 | - target = self.makeTargetBranch() |
| 1032 | - self.factory.makeProduct('uproduct') |
| 1033 | - self.factory.makePerson(name='uuser') |
| 1034 | - code_handler = CodeHandler() |
| 1035 | - namespace, base = code_handler._getNewBranchInfo( |
| 1036 | - config.codehosting.supermirror_root + '~uuser/uproduct/bar', |
| 1037 | - target, submitter) |
| 1038 | - self.assertEqual('~uuser/uproduct', namespace.name) |
| 1039 | - self.assertEqual('bar', base) |
| 1040 | - |
| 1041 | - def test_getNewBranchInfoLPURLTrailingSlash(self): |
| 1042 | - """Trailing slashes are permitted in LP URLs.""" |
| 1043 | - submitter = self.factory.makePerson(name='merge-submitter') |
| 1044 | - target = self.makeTargetBranch() |
| 1045 | - self.factory.makeProduct('uproduct') |
| 1046 | - self.factory.makePerson(name='uuser') |
| 1047 | - code_handler = CodeHandler() |
| 1048 | - namespace, base = code_handler._getNewBranchInfo( |
| 1049 | - config.codehosting.supermirror_root + '~uuser/uproduct/bar/', |
| 1050 | - target, submitter) |
| 1051 | - self.assertEqual('~uuser/uproduct', namespace.name) |
| 1052 | - self.assertEqual('bar', base) |
| 1053 | - |
| 1054 | - def test_processNonLaunchpadTarget(self): |
| 1055 | - """When target branch is unknown to Launchpad, the user is notified. |
| 1056 | - """ |
| 1057 | - directive = self.factory.makeMergeDirective( |
| 1058 | - target_branch_url='http://www.example.com') |
| 1059 | - message = self.factory.makeSignedMessage(body='body', |
| 1060 | - subject='This is gonna fail', attachment_contents=''.join( |
| 1061 | - directive.to_lines())) |
| 1062 | - |
| 1063 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 1064 | - code_handler = CodeHandler() |
| 1065 | - code_handler.processMergeProposal(message) |
| 1066 | - transaction.commit() |
| 1067 | - [notification] = pop_notifications() |
| 1068 | - |
| 1069 | - self.assertEqual( |
| 1070 | - notification['Subject'], 'Error Creating Merge Proposal') |
| 1071 | - self.assertEqual( |
| 1072 | - notification.get_payload(decode=True), |
| 1073 | - 'The target branch at %s is not known to Launchpad. It\'s\n' |
| 1074 | - 'possible that your submit branch is not set correctly, or that ' |
| 1075 | - 'your submit\nbranch has not yet been pushed to Launchpad.\n\n' |
| 1076 | - % ('http://www.example.com')) |
| 1077 | - self.assertEqual(notification['to'], |
| 1078 | - message['from']) |
| 1079 | - |
| 1080 | def test_processMissingSubject(self): |
| 1081 | """If the subject is missing, the user is warned by email.""" |
| 1082 | mail = self.factory.makeSignedMessage( |
| 1083 | @@ -874,267 +425,6 @@ |
| 1084 | self.assertEqual(0, bmp.all_comments.count()) |
| 1085 | |
| 1086 | |
| 1087 | -class TestCodeHandlerProcessMergeDirective(TestCaseWithFactory): |
| 1088 | - """Test the merge directive processing parts of the code email hander.""" |
| 1089 | - |
| 1090 | - layer = ZopelessAppServerLayer |
| 1091 | - |
| 1092 | - def setUp(self): |
| 1093 | - TestCaseWithFactory.setUp(self, user='test@canonical.com') |
| 1094 | - self.code_handler = CodeHandler() |
| 1095 | - self._old_policy = setSecurityPolicy(LaunchpadSecurityPolicy) |
| 1096 | - |
| 1097 | - def tearDown(self): |
| 1098 | - setSecurityPolicy(self._old_policy) |
| 1099 | - TestCaseWithFactory.tearDown(self) |
| 1100 | - |
| 1101 | - def _createTargetSourceAndBundle(self, format=None): |
| 1102 | - """Create a merge directive with a bundle and associated branches. |
| 1103 | - |
| 1104 | - The target branch is created in the specified format, or the default |
| 1105 | - format if the format is None. |
| 1106 | - |
| 1107 | - :return: A tuple containing the db_branch relating to the target |
| 1108 | - branch, a bzr_branch of the source branch, and the merge directive |
| 1109 | - containing the revisions in the source branch that aren't in the |
| 1110 | - target branch. |
| 1111 | - """ |
| 1112 | - db_target_branch, target_tree = self.create_branch_and_tree( |
| 1113 | - tree_location='.', format=format) |
| 1114 | - target_tree.branch.set_public_branch(db_target_branch.bzr_identity) |
| 1115 | - # XXX: AaronBentley 2010-08-06 bug=614404: a bzr username is |
| 1116 | - # required to generate the revision-id. |
| 1117 | - with override_environ(BZR_EMAIL='me@example.com'): |
| 1118 | - target_tree.commit('rev1') |
| 1119 | - # Make sure that the created branch has been mirrored. |
| 1120 | - removeSecurityProxy(db_target_branch).branchChanged( |
| 1121 | - '', 'rev1', None, None, None) |
| 1122 | - sprout_bzrdir = target_tree.bzrdir.sprout('source') |
| 1123 | - source_tree = sprout_bzrdir.open_workingtree() |
| 1124 | - source_tree.commit('rev2') |
| 1125 | - message = self.factory.makeBundleMergeDirectiveEmail( |
| 1126 | - source_tree.branch, db_target_branch) |
| 1127 | - return db_target_branch, source_tree.branch, message |
| 1128 | - |
| 1129 | - def _openBazaarBranchAsClient(self, db_branch): |
| 1130 | - """Open the Bazaar branch relating to db_branch as if a client was. |
| 1131 | - |
| 1132 | - The client has write access to the branch. |
| 1133 | - """ |
| 1134 | - lp_server = get_lp_server(db_branch.owner.id) |
| 1135 | - lp_server.start_server() |
| 1136 | - self.addCleanup(lp_server.stop_server) |
| 1137 | - branch_url = urljoin(lp_server.get_url(), db_branch.unique_name) |
| 1138 | - return Branch.open(branch_url) |
| 1139 | - |
| 1140 | - def _processMergeDirective(self, message): |
| 1141 | - """Process the merge directive email.""" |
| 1142 | - switch_dbuser(config.create_merge_proposals.dbuser) |
| 1143 | - code_handler = CodeHandler() |
| 1144 | - # Do the authentication dance as we do in the processing script. |
| 1145 | - authutil = getUtility(IPlacelessAuthUtility) |
| 1146 | - email_addr = message['from'] |
| 1147 | - principal = authutil.getPrincipalByLogin(email_addr) |
| 1148 | - if principal is None: |
| 1149 | - raise AssertionError('No principal found for %s' % email_addr) |
| 1150 | - setupInteraction(principal, email_addr) |
| 1151 | - return code_handler.processMergeProposal(message) |
| 1152 | - |
| 1153 | - def test_nonstackable_target(self): |
| 1154 | - # If the target branch is in a non-stackable format, then the source |
| 1155 | - # branch that is created is an empty hosted branch. The new branch |
| 1156 | - # will not have a mirror requested as there are no revisions, and |
| 1157 | - # there is no branch created in the hosted area. |
| 1158 | - |
| 1159 | - # XXX Tim Penhey 2010-07-27 bug 610292 |
| 1160 | - # We should fail here and return an oops email to the user. |
| 1161 | - self.useBzrBranches() |
| 1162 | - branch, source, message = self._createTargetSourceAndBundle( |
| 1163 | - format="pack-0.92") |
| 1164 | - bmp = self._processMergeDirective(message) |
| 1165 | - self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type) |
| 1166 | - self.assertIs(None, bmp.source_branch.next_mirror_time) |
| 1167 | - |
| 1168 | - def test_stackable_unmirrored_target(self): |
| 1169 | - # If the target branch is in a stackable format but has not been |
| 1170 | - # mirrored, the source branch that is created is an empty hosted |
| 1171 | - # branch. The new branch will not have a mirror requested as there |
| 1172 | - # are no revisions, and there is no branch created in the hosted area. |
| 1173 | - self.useBzrBranches() |
| 1174 | - branch, source, message = self._createTargetSourceAndBundle( |
| 1175 | - format="1.9") |
| 1176 | - # Mark the target branch as "unmirrored", at least as far as the db is |
| 1177 | - # concerned. |
| 1178 | - branch.last_mirrored = None |
| 1179 | - branch.last_mirrored_id = None |
| 1180 | - bmp = self._processMergeDirective(message) |
| 1181 | - self.assertEqual(BranchType.REMOTE, bmp.source_branch.branch_type) |
| 1182 | - |
| 1183 | - def test_stackable_target(self): |
| 1184 | - # If the target branch is in a stackable format, then the source |
| 1185 | - # branch that is created is a hosted branch stacked on the target |
| 1186 | - # branch. The source branch will have the revisions from the bundle, |
| 1187 | - # and a mirror will have been triggered. |
| 1188 | - self.useBzrBranches() |
| 1189 | - branch, source, message = self._createTargetSourceAndBundle( |
| 1190 | - format="1.9") |
| 1191 | - bmp = self._processMergeDirective(message) |
| 1192 | - source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch) |
| 1193 | - self.assertEqual(BranchType.HOSTED, bmp.source_branch.branch_type) |
| 1194 | - self.assertTrue(bmp.source_branch.pending_writes) |
| 1195 | - self.assertEqual( |
| 1196 | - source.last_revision(), source_bzr_branch.last_revision()) |
| 1197 | - |
| 1198 | - def test_branch_stacked(self): |
| 1199 | - # When a branch is created for a merge directive, it is created |
| 1200 | - # stacked on the target branch. |
| 1201 | - self.useBzrBranches() |
| 1202 | - branch, source, message = self._createTargetSourceAndBundle( |
| 1203 | - format="1.9") |
| 1204 | - bmp = self._processMergeDirective(message) |
| 1205 | - # The source branch is stacked on the target. |
| 1206 | - source_bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch) |
| 1207 | - self.assertEqual( |
| 1208 | - '/' + bmp.target_branch.unique_name, |
| 1209 | - source_bzr_branch.get_stacked_on_url()) |
| 1210 | - # Make sure that the source branch doesn't have all the revisions. |
| 1211 | - source_branch_revisions = ( |
| 1212 | - source_bzr_branch.bzrdir.open_repository().all_revision_ids()) |
| 1213 | - # The only revision is the tip revision, as the other revisions are |
| 1214 | - # from the target branch. |
| 1215 | - tip_revision = source_bzr_branch.last_revision() |
| 1216 | - self.assertEqual([tip_revision], source_branch_revisions) |
| 1217 | - |
| 1218 | - def test_source_not_newer(self): |
| 1219 | - # The source branch is created correctly when the source is not newer |
| 1220 | - # than the target, instead of raising DivergedBranches. |
| 1221 | - self.useBzrBranches() |
| 1222 | - branch, source, message = self._createTargetSourceAndBundle( |
| 1223 | - format="1.9") |
| 1224 | - target_tree = WorkingTree.open('.') |
| 1225 | - # XXX: AaronBentley 2010-08-06 bug=614404: a bzr username is |
| 1226 | - # required to generate the revision-id. |
| 1227 | - with override_environ(BZR_EMAIL='me@example.com'): |
| 1228 | - target_tree.commit('rev2b') |
| 1229 | - bmp = self._processMergeDirective(message) |
| 1230 | - lp_branch = self._openBazaarBranchAsClient(bmp.source_branch) |
| 1231 | - self.assertEqual(source.last_revision(), lp_branch.last_revision()) |
| 1232 | - |
| 1233 | - def _createPreexistingSourceAndMessage(self, target_format, |
| 1234 | - source_format, set_stacked=False): |
| 1235 | - """Create the source and target branches and the merge directive.""" |
| 1236 | - db_target_branch, target_tree = self.create_branch_and_tree( |
| 1237 | - 'target', format=target_format) |
| 1238 | - target_tree.branch.set_public_branch(db_target_branch.bzr_identity) |
| 1239 | - # XXX: AaronBentley 2010-08-06 bug=614404: a bzr username is |
| 1240 | - # required to generate the revision-id. |
| 1241 | - with override_environ(BZR_EMAIL='me@example.com'): |
| 1242 | - revid = target_tree.commit('rev1') |
| 1243 | - removeSecurityProxy(db_target_branch).branchChanged( |
| 1244 | - '', revid, None, None, None) |
| 1245 | - |
| 1246 | - db_source_branch, source_tree = self.create_branch_and_tree( |
| 1247 | - 'lpsource', db_target_branch.product, format=source_format) |
| 1248 | - # The branch is not scheduled to be mirrorred. |
| 1249 | - self.assertIs(db_source_branch.next_mirror_time, None) |
| 1250 | - source_tree.pull(target_tree.branch) |
| 1251 | - source_tree.commit('rev2', rev_id='rev2') |
| 1252 | - # bundle_tree is effectively behaving like a local copy of |
| 1253 | - # db_source_branch, and is used to create the merge directive. |
| 1254 | - sprout_bzrdir = source_tree.bzrdir.sprout('source') |
| 1255 | - bundle_tree = sprout_bzrdir.open_workingtree() |
| 1256 | - bundle_tree.commit('rev3', rev_id='rev3') |
| 1257 | - bundle_tree.branch.set_public_branch(db_source_branch.bzr_identity) |
| 1258 | - message = self.factory.makeBundleMergeDirectiveEmail( |
| 1259 | - bundle_tree.branch, db_target_branch, |
| 1260 | - sender=db_source_branch.owner) |
| 1261 | - # Tell the source branch that it is stacked on the target. |
| 1262 | - if set_stacked: |
| 1263 | - stacked_url = '/' + db_target_branch.unique_name |
| 1264 | - branch = self._openBazaarBranchAsClient(db_source_branch) |
| 1265 | - branch.set_stacked_on_url(stacked_url) |
| 1266 | - return db_source_branch, message |
| 1267 | - |
| 1268 | - def test_existing_stacked_branch(self): |
| 1269 | - # A bundle can update an existing branch if they are both stackable, |
| 1270 | - # and the source branch is stacked. |
| 1271 | - self.useBzrBranches() |
| 1272 | - lp_source, message = self._createPreexistingSourceAndMessage( |
| 1273 | - target_format="1.9", source_format="1.9", set_stacked=True) |
| 1274 | - bmp = self._processMergeDirective(message) |
| 1275 | - # The branch merge proposal should use the existing db branch. |
| 1276 | - self.assertEqual(lp_source, bmp.source_branch) |
| 1277 | - bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch) |
| 1278 | - # The branch has been updated. |
| 1279 | - self.assertEqual('rev3', bzr_branch.last_revision()) |
| 1280 | - |
| 1281 | - def test_existing_unstacked_branch(self): |
| 1282 | - # Even if the source and target are stackable, if the source is not |
| 1283 | - # stacked, we don't support stacking something that wasn't stacked |
| 1284 | - # before (yet). |
| 1285 | - self.useBzrBranches() |
| 1286 | - lp_source, message = self._createPreexistingSourceAndMessage( |
| 1287 | - target_format="1.9", source_format="1.9") |
| 1288 | - bmp = self._processMergeDirective(message) |
| 1289 | - # The branch merge proposal should use the existing db branch. |
| 1290 | - self.assertEqual(lp_source, bmp.source_branch) |
| 1291 | - bzr_branch = self._openBazaarBranchAsClient(bmp.source_branch) |
| 1292 | - # The hosted copy of the branch has not been updated. |
| 1293 | - self.assertEqual('rev2', bzr_branch.last_revision()) |
| 1294 | - |
| 1295 | - def test_existing_branch_nonstackable_target(self): |
| 1296 | - # If the target branch is not stackable, then we don't pull any |
| 1297 | - # revisions. |
| 1298 | - self.useBzrBranches() |
| 1299 | - lp_source, message = self._createPreexistingSourceAndMessage( |
| 1300 | - target_format="pack-0.92", source_format="1.9") |
| 1301 | - bmp = self._processMergeDirective(message) |
| 1302 | - # The branch merge proposal should use the existing db branch. |
| 1303 | - self.assertEqual(lp_source, bmp.source_branch) |
| 1304 | - # Now the branch is not scheduled to be mirrorred. |
| 1305 | - self.assertIs(None, lp_source.next_mirror_time) |
| 1306 | - hosted = self._openBazaarBranchAsClient(bmp.source_branch) |
| 1307 | - # The hosted copy has not been updated. |
| 1308 | - self.assertEqual('rev2', hosted.last_revision()) |
| 1309 | - |
| 1310 | - def test_existing_branch_nonstackable_source(self): |
| 1311 | - # If the source branch is not stackable, then we don't pull any |
| 1312 | - # revisions. |
| 1313 | - self.useBzrBranches() |
| 1314 | - lp_source, message = self._createPreexistingSourceAndMessage( |
| 1315 | - target_format="1.9", source_format="pack-0.92") |
| 1316 | - bmp = self._processMergeDirective(message) |
| 1317 | - # The branch merge proposal should use the existing db branch. |
| 1318 | - self.assertEqual(lp_source, bmp.source_branch) |
| 1319 | - # Now the branch is not scheduled to be mirrorred. |
| 1320 | - self.assertIs(None, lp_source.next_mirror_time) |
| 1321 | - hosted = self._openBazaarBranchAsClient(bmp.source_branch) |
| 1322 | - # The hosted copy has not been updated. |
| 1323 | - self.assertEqual('rev2', hosted.last_revision()) |
| 1324 | - |
| 1325 | - def test_forbidden_target(self): |
| 1326 | - """Specifying a branch in a forbidden target generates email.""" |
| 1327 | - self.useBzrBranches() |
| 1328 | - branch, source, message = self._createTargetSourceAndBundle( |
| 1329 | - format="pack-0.92") |
| 1330 | - branch.product.setBranchVisibilityTeamPolicy( |
| 1331 | - None, BranchVisibilityRule.FORBIDDEN) |
| 1332 | - result = self._processMergeDirective(message) |
| 1333 | - self.assertIs(None, result) |
| 1334 | - notifications = pop_notifications() |
| 1335 | - self.assertEqual(1, len(notifications)) |
| 1336 | - self.assertEqual( |
| 1337 | - 'Error Creating Merge Proposal', notifications[0]['subject']) |
| 1338 | - body = notifications[0].get_payload(decode=True) |
| 1339 | - sender = getUtility(IPersonSet).getByEmail(message['from']) |
| 1340 | - expected = ( |
| 1341 | - 'Launchpad cannot create the branch requested by' |
| 1342 | - ' your merge directive:\n' |
| 1343 | - 'You cannot create branches in "~%s/%s"\n' % |
| 1344 | - (sender.name, branch.product.name)) |
| 1345 | - self.assertEqual(expected, body) |
| 1346 | - |
| 1347 | - |
| 1348 | class TestVoteEmailCommand(TestCase): |
| 1349 | """Test the vote and tag processing of the VoteEmailCommand.""" |
| 1350 | |
| 1351 | |
| 1352 | === modified file 'lib/lp/code/model/branchmergeproposaljob.py' |
| 1353 | --- lib/lp/code/model/branchmergeproposaljob.py 2012-04-13 19:20:03 +0000 |
| 1354 | +++ lib/lp/code/model/branchmergeproposaljob.py 2012-04-30 14:10:29 +0000 |
| 1355 | @@ -17,7 +17,6 @@ |
| 1356 | 'BranchMergeProposalJobSource', |
| 1357 | 'BranchMergeProposalJobType', |
| 1358 | 'CodeReviewCommentEmailJob', |
| 1359 | - 'CreateMergeProposalJob', |
| 1360 | 'GenerateIncrementalDiffJob', |
| 1361 | 'MergeProposalNeedsReviewEmailJob', |
| 1362 | 'MergeProposalUpdatedEmailJob', |
| 1363 | @@ -30,7 +29,6 @@ |
| 1364 | datetime, |
| 1365 | timedelta, |
| 1366 | ) |
| 1367 | -from email.utils import parseaddr |
| 1368 | |
| 1369 | from lazr.delegates import delegates |
| 1370 | from lazr.enum import ( |
| 1371 | @@ -69,8 +67,6 @@ |
| 1372 | IBranchMergeProposalJobSource, |
| 1373 | ICodeReviewCommentEmailJob, |
| 1374 | ICodeReviewCommentEmailJobSource, |
| 1375 | - ICreateMergeProposalJob, |
| 1376 | - ICreateMergeProposalJobSource, |
| 1377 | IGenerateIncrementalDiffJob, |
| 1378 | IGenerateIncrementalDiffJobSource, |
| 1379 | IMergeProposalNeedsReviewEmailJob, |
| 1380 | @@ -91,7 +87,6 @@ |
| 1381 | from lp.codehosting.bzrutils import server |
| 1382 | from lp.codehosting.vfs import ( |
| 1383 | get_ro_server, |
| 1384 | - get_rw_server, |
| 1385 | ) |
| 1386 | from lp.registry.interfaces.person import IPersonSet |
| 1387 | from lp.services.config import config |
| 1388 | @@ -107,16 +102,9 @@ |
| 1389 | BaseRunnableJobSource, |
| 1390 | ) |
| 1391 | from lp.services.mail.sendmail import format_address_for_person |
| 1392 | -from lp.services.messages.interfaces.message import IMessageJob |
| 1393 | -from lp.services.messages.model.message import ( |
| 1394 | - MessageJob, |
| 1395 | - MessageJobAction, |
| 1396 | - ) |
| 1397 | from lp.services.webapp import errorlog |
| 1398 | -from lp.services.webapp.interaction import setupInteraction |
| 1399 | from lp.services.webapp.interfaces import ( |
| 1400 | DEFAULT_FLAVOR, |
| 1401 | - IPlacelessAuthUtility, |
| 1402 | IStoreSelector, |
| 1403 | MAIN_STORE, |
| 1404 | MASTER_FLAVOR, |
| 1405 | @@ -403,83 +391,6 @@ |
| 1406 | return format_address_for_person(registrant) |
| 1407 | |
| 1408 | |
| 1409 | -class CreateMergeProposalJob(BaseRunnableJob): |
| 1410 | - """See `ICreateMergeProposalJob` and `ICreateMergeProposalJobSource`.""" |
| 1411 | - |
| 1412 | - classProvides(ICreateMergeProposalJobSource) |
| 1413 | - |
| 1414 | - delegates(IMessageJob) |
| 1415 | - |
| 1416 | - class_action = MessageJobAction.CREATE_MERGE_PROPOSAL |
| 1417 | - |
| 1418 | - implements(ICreateMergeProposalJob) |
| 1419 | - |
| 1420 | - def __init__(self, context): |
| 1421 | - """Create an instance of CreateMergeProposalJob. |
| 1422 | - |
| 1423 | - :param context: a MessageJob. |
| 1424 | - """ |
| 1425 | - self.context = context |
| 1426 | - |
| 1427 | - def __eq__(self, other): |
| 1428 | - return (self.__class__ == other.__class__ and |
| 1429 | - self.context == other.context) |
| 1430 | - |
| 1431 | - @classmethod |
| 1432 | - def create(klass, message_bytes): |
| 1433 | - """See `ICreateMergeProposalJobSource`.""" |
| 1434 | - context = MessageJob( |
| 1435 | - message_bytes, MessageJobAction.CREATE_MERGE_PROPOSAL) |
| 1436 | - return klass(context) |
| 1437 | - |
| 1438 | - @classmethod |
| 1439 | - def iterReady(klass): |
| 1440 | - """Iterate through all ready BranchMergeProposalJobs.""" |
| 1441 | - store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR) |
| 1442 | - jobs = store.find( |
| 1443 | - (MessageJob), |
| 1444 | - And(MessageJob.action == klass.class_action, |
| 1445 | - MessageJob.job == Job.id, |
| 1446 | - Job.id.is_in(Job.ready_jobs))) |
| 1447 | - return (klass(job) for job in jobs) |
| 1448 | - |
| 1449 | - def run(self): |
| 1450 | - """See `ICreateMergeProposalJob`.""" |
| 1451 | - # Avoid circular import |
| 1452 | - from lp.code.mail.codehandler import CodeHandler |
| 1453 | - url = self.context.message_bytes.getURL() |
| 1454 | - with errorlog.globalErrorUtility.oopsMessage('Mail url: %r' % url): |
| 1455 | - message = self.getMessage() |
| 1456 | - # Since the message was checked as signed before it was saved in |
| 1457 | - # the Librarian, just create the principal from the sender and set |
| 1458 | - # up the interaction. |
| 1459 | - name, email_addr = parseaddr(message['From']) |
| 1460 | - authutil = getUtility(IPlacelessAuthUtility) |
| 1461 | - principal = authutil.getPrincipalByLogin(email_addr) |
| 1462 | - if principal is None: |
| 1463 | - raise AssertionError('No principal found for %s' % email_addr) |
| 1464 | - setupInteraction(principal, email_addr) |
| 1465 | - |
| 1466 | - server = get_rw_server() |
| 1467 | - server.start_server() |
| 1468 | - try: |
| 1469 | - return CodeHandler().processMergeProposal(message) |
| 1470 | - finally: |
| 1471 | - server.stop_server() |
| 1472 | - |
| 1473 | - def getOopsRecipients(self): |
| 1474 | - message = self.getMessage() |
| 1475 | - from_ = message['From'] |
| 1476 | - if from_ is None: |
| 1477 | - return [] |
| 1478 | - return [from_] |
| 1479 | - |
| 1480 | - def getOperationDescription(self): |
| 1481 | - message = self.getMessage() |
| 1482 | - return ('creating a merge proposal from message with subject %s' % |
| 1483 | - message['Subject']) |
| 1484 | - |
| 1485 | - |
| 1486 | class CodeReviewCommentEmailJob(BranchMergeProposalJobDerived): |
| 1487 | """A job to send a code review comment. |
| 1488 | |
| 1489 | |
| 1490 | === modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py' |
| 1491 | --- lib/lp/code/model/tests/test_branchmergeproposal.py 2012-02-27 00:49:48 +0000 |
| 1492 | +++ lib/lp/code/model/tests/test_branchmergeproposal.py 2012-04-30 14:10:29 +0000 |
| 1493 | @@ -47,8 +47,6 @@ |
| 1494 | BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES, |
| 1495 | IBranchMergeProposal, |
| 1496 | IBranchMergeProposalGetter, |
| 1497 | - ICreateMergeProposalJob, |
| 1498 | - ICreateMergeProposalJobSource, |
| 1499 | notify_modified, |
| 1500 | ) |
| 1501 | from lp.code.model.branchmergeproposal import ( |
| 1502 | @@ -57,7 +55,6 @@ |
| 1503 | ) |
| 1504 | from lp.code.model.branchmergeproposaljob import ( |
| 1505 | BranchMergeProposalJob, |
| 1506 | - CreateMergeProposalJob, |
| 1507 | MergeProposalNeedsReviewEmailJob, |
| 1508 | UpdatePreviewDiffJob, |
| 1509 | ) |
| 1510 | @@ -68,7 +65,6 @@ |
| 1511 | from lp.registry.interfaces.person import IPersonSet |
| 1512 | from lp.registry.interfaces.product import IProductSet |
| 1513 | from lp.services.database.constants import UTC_NOW |
| 1514 | -from lp.services.messages.interfaces.message import IMessageJob |
| 1515 | from lp.services.webapp import canonical_url |
| 1516 | from lp.services.webapp.testing import verifyObject |
| 1517 | from lp.testing import ( |
| 1518 | @@ -82,14 +78,11 @@ |
| 1519 | ws_object, |
| 1520 | ) |
| 1521 | from lp.testing.factory import ( |
| 1522 | - GPGSigningContext, |
| 1523 | LaunchpadObjectFactory, |
| 1524 | ) |
| 1525 | -from lp.testing.gpgkeys import import_secret_test_key |
| 1526 | from lp.testing.layers import ( |
| 1527 | DatabaseFunctionalLayer, |
| 1528 | LaunchpadFunctionalLayer, |
| 1529 | - LaunchpadZopelessLayer, |
| 1530 | ) |
| 1531 | |
| 1532 | |
| 1533 | @@ -1749,66 +1742,6 @@ |
| 1534 | BranchMergeProposalStatus.REJECTED, first_mp.queue_status) |
| 1535 | |
| 1536 | |
| 1537 | -class TestCreateMergeProposalJob(TestCaseWithFactory): |
| 1538 | - """Tests for CreateMergeProposalJob.""" |
| 1539 | - |
| 1540 | - layer = LaunchpadZopelessLayer |
| 1541 | - |
| 1542 | - def setUp(self): |
| 1543 | - TestCaseWithFactory.setUp(self, user='test@canonical.com') |
| 1544 | - |
| 1545 | - def test_providesInterface(self): |
| 1546 | - """The class and instances correctly implement their interfaces.""" |
| 1547 | - verifyObject(ICreateMergeProposalJobSource, CreateMergeProposalJob) |
| 1548 | - file_alias = self.factory.makeMergeDirectiveEmail()[1] |
| 1549 | - job = CreateMergeProposalJob.create(file_alias) |
| 1550 | - job.context.sync() |
| 1551 | - verifyObject(IMessageJob, job) |
| 1552 | - verifyObject(ICreateMergeProposalJob, job) |
| 1553 | - |
| 1554 | - def test_run_creates_proposal(self): |
| 1555 | - """CreateMergeProposalJob.run should create a merge proposal.""" |
| 1556 | - key = import_secret_test_key() |
| 1557 | - signing_context = GPGSigningContext(key.fingerprint, password='test') |
| 1558 | - message, file_alias, source, target = ( |
| 1559 | - self.factory.makeMergeDirectiveEmail( |
| 1560 | - signing_context=signing_context)) |
| 1561 | - job = CreateMergeProposalJob.create(file_alias) |
| 1562 | - transaction.commit() |
| 1563 | - proposal = job.run() |
| 1564 | - self.assertEqual(proposal.source_branch, source) |
| 1565 | - self.assertEqual(proposal.target_branch, target) |
| 1566 | - |
| 1567 | - def test_getOopsMailController(self): |
| 1568 | - """The sender is notified when creating a bmp from email fails.""" |
| 1569 | - key = import_secret_test_key() |
| 1570 | - signing_context = GPGSigningContext(key.fingerprint, password='test') |
| 1571 | - message, file_alias, source, target = ( |
| 1572 | - self.factory.makeMergeDirectiveEmail( |
| 1573 | - signing_context=signing_context)) |
| 1574 | - job = CreateMergeProposalJob.create(file_alias) |
| 1575 | - transaction.commit() |
| 1576 | - ctrl = job.getOopsMailController('1234') |
| 1577 | - self.assertEqual([message['From']], ctrl.to_addrs) |
| 1578 | - desc = ('creating a merge proposal from message with subject %s' % |
| 1579 | - message['Subject']) |
| 1580 | - self.assertIn(desc, ctrl.body) |
| 1581 | - |
| 1582 | - def test_iterReady_includes_ready_jobs(self): |
| 1583 | - """Ready jobs should be listed.""" |
| 1584 | - file_alias = self.factory.makeMergeDirectiveEmail()[1] |
| 1585 | - job = CreateMergeProposalJob.create(file_alias) |
| 1586 | - self.assertEqual([job], list(CreateMergeProposalJob.iterReady())) |
| 1587 | - |
| 1588 | - def test_iterReady_excludes_unready_jobs(self): |
| 1589 | - """Unready jobs should not be listed.""" |
| 1590 | - file_alias = self.factory.makeMergeDirectiveEmail()[1] |
| 1591 | - job = CreateMergeProposalJob.create(file_alias) |
| 1592 | - job.job.start() |
| 1593 | - job.job.complete() |
| 1594 | - self.assertEqual([], list(CreateMergeProposalJob.iterReady())) |
| 1595 | - |
| 1596 | - |
| 1597 | class TestUpdatePreviewDiff(TestCaseWithFactory): |
| 1598 | """Test the updateMergeDiff method of BranchMergeProposal.""" |
| 1599 | |
| 1600 | |
| 1601 | === removed file 'lib/lp/code/scripts/tests/test_create_merge_proposals.py' |
| 1602 | --- lib/lp/code/scripts/tests/test_create_merge_proposals.py 2012-03-28 11:36:07 +0000 |
| 1603 | +++ lib/lp/code/scripts/tests/test_create_merge_proposals.py 1970-01-01 00:00:00 +0000 |
| 1604 | @@ -1,107 +0,0 @@ |
| 1605 | -#! /usr/bin/python |
| 1606 | -# |
| 1607 | -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the |
| 1608 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
| 1609 | - |
| 1610 | -"""Test the create_merge_proposals script""" |
| 1611 | - |
| 1612 | -from cStringIO import StringIO |
| 1613 | - |
| 1614 | -import transaction |
| 1615 | -from zope.component import getUtility |
| 1616 | - |
| 1617 | -from lp.code.model.branchmergeproposaljob import CreateMergeProposalJob |
| 1618 | -from lp.services.librarian.interfaces import ILibraryFileAliasSet |
| 1619 | -from lp.services.scripts.tests import run_script |
| 1620 | -from lp.testing import TestCaseWithFactory |
| 1621 | -from lp.testing.factory import GPGSigningContext |
| 1622 | -from lp.testing.gpgkeys import import_secret_test_key |
| 1623 | -from lp.testing.layers import ZopelessAppServerLayer |
| 1624 | - |
| 1625 | - |
| 1626 | -class TestCreateMergeProposals(TestCaseWithFactory): |
| 1627 | - |
| 1628 | - layer = ZopelessAppServerLayer |
| 1629 | - |
| 1630 | - def test_create_merge_proposals(self): |
| 1631 | - """Ensure create_merge_proposals runs and creates proposals.""" |
| 1632 | - key = import_secret_test_key() |
| 1633 | - signing_context = GPGSigningContext(key.fingerprint, password='test') |
| 1634 | - email, file_alias, source, target = ( |
| 1635 | - self.factory.makeMergeDirectiveEmail( |
| 1636 | - signing_context=signing_context)) |
| 1637 | - job = CreateMergeProposalJob.create(file_alias) |
| 1638 | - self.assertEqual(0, source.landing_targets.count()) |
| 1639 | - transaction.commit() |
| 1640 | - retcode, stdout, stderr = run_script( |
| 1641 | - 'cronscripts/create_merge_proposals.py', []) |
| 1642 | - self.assertEqual(0, retcode) |
| 1643 | - self.assertTextMatchesExpressionIgnoreWhitespace( |
| 1644 | - "INFO Creating lockfile: " |
| 1645 | - "/var/lock/launchpad-create_merge_proposals.lock\n" |
| 1646 | - "INFO Running <.*CreateMergeProposalJob object at .*?> " |
| 1647 | - "\(ID %s\) in status Waiting\n" |
| 1648 | - "INFO Ran 1 CreateMergeProposalJobs.\n" % job.job.id, stderr) |
| 1649 | - self.assertEqual('', stdout) |
| 1650 | - self.assertEqual(1, source.landing_targets.count()) |
| 1651 | - |
| 1652 | - def createJob(self, branch, tree): |
| 1653 | - """Create merge directive job from this branch.""" |
| 1654 | - tree.branch.set_public_branch(branch.bzr_identity) |
| 1655 | - tree.commit('rev1') |
| 1656 | - source = tree.bzrdir.sprout('source').open_workingtree() |
| 1657 | - source.commit('rev2') |
| 1658 | - message = self.factory.makeBundleMergeDirectiveEmail( |
| 1659 | - source.branch, branch) |
| 1660 | - message_str = message.as_string() |
| 1661 | - library_file_aliases = getUtility(ILibraryFileAliasSet) |
| 1662 | - file_alias = library_file_aliases.create( |
| 1663 | - '*', len(message_str), StringIO(message_str), '*') |
| 1664 | - CreateMergeProposalJob.create(file_alias) |
| 1665 | - return source |
| 1666 | - |
| 1667 | - def jobOutputCheck(self, branch, source): |
| 1668 | - """Run the job and check the output.""" |
| 1669 | - transaction.commit() |
| 1670 | - retcode, stdout, stderr = run_script( |
| 1671 | - 'cronscripts/create_merge_proposals.py', []) |
| 1672 | - self.assertEqual(0, retcode) |
| 1673 | - self.assertEqual( |
| 1674 | - "INFO Creating lockfile: " |
| 1675 | - "/var/lock/launchpad-create_merge_proposals.lock\n" |
| 1676 | - "INFO Ran 1 CreateMergeProposalJobs.\n", stderr) |
| 1677 | - self.assertEqual('', stdout) |
| 1678 | - bmp = branch.landing_candidates[0] |
| 1679 | - local_source = bmp.source_branch.getBzrBranch() |
| 1680 | - # The branch has the correct last revision. |
| 1681 | - self.assertEqual( |
| 1682 | - source.branch.last_revision(), local_source.last_revision()) |
| 1683 | - |
| 1684 | - def disabled_test_merge_directive_with_bundle(self): |
| 1685 | - """Merge directives with bundles generate branches.""" |
| 1686 | - # XXX TimPenhey 2009-04-01 bug 352800 |
| 1687 | - self.useBzrBranches() |
| 1688 | - branch, tree = self.create_branch_and_tree() |
| 1689 | - source = self.createJob(branch, tree) |
| 1690 | - self.jobOutputCheck(branch, source) |
| 1691 | - |
| 1692 | - def disabled_test_merge_directive_with_project(self): |
| 1693 | - """Bundles are handled when the target branch has a project.""" |
| 1694 | - # XXX TimPenhey 2009-04-01 bug 352800 |
| 1695 | - self.useBzrBranches() |
| 1696 | - product = self.factory.makeProduct(project=self.factory.makeProject()) |
| 1697 | - branch, tree = self.create_branch_and_tree(product=product) |
| 1698 | - source = self.createJob(branch, tree) |
| 1699 | - self.jobOutputCheck(branch, source) |
| 1700 | - |
| 1701 | - def test_oops(self): |
| 1702 | - """A bogus request should cause an oops, not an exception.""" |
| 1703 | - file_alias = self.factory.makeLibraryFileAlias(content='bogus') |
| 1704 | - CreateMergeProposalJob.create(file_alias) |
| 1705 | - transaction.commit() |
| 1706 | - retcode, stdout, stderr = run_script( |
| 1707 | - 'cronscripts/create_merge_proposals.py', []) |
| 1708 | - self.assertIn('INFO Creating lockfile:', stderr) |
| 1709 | - self.assertIn('INFO Job resulted in OOPS:', stderr) |
| 1710 | - self.assertIn('INFO Ran 0 CreateMergeProposalJobs.\n', stderr) |
| 1711 | - self.assertEqual('', stdout) |
| 1712 | |
| 1713 | === modified file 'lib/lp/services/config/schema-lazr.conf' |
| 1714 | --- lib/lp/services/config/schema-lazr.conf 2012-04-18 16:23:23 +0000 |
| 1715 | +++ lib/lp/services/config/schema-lazr.conf 2012-04-30 14:10:29 +0000 |
| 1716 | @@ -481,18 +481,6 @@ |
| 1717 | # datatype: string; a url |
| 1718 | licensing_policy_url: https://help.launchpad.net/Legal/ProjectLicensing |
| 1719 | |
| 1720 | - |
| 1721 | -[create_merge_proposals] |
| 1722 | -# The database user which will be used by this process. |
| 1723 | -# datatype: string |
| 1724 | -dbuser: create-merge-proposals |
| 1725 | - |
| 1726 | -# See [error_reports]. |
| 1727 | -error_dir: none |
| 1728 | - |
| 1729 | -# See [error_reports]. |
| 1730 | -oops_prefix: none |
| 1731 | - |
| 1732 | [packaging_translations] |
| 1733 | # The database user which will be used by this process. |
| 1734 | # datatype: string |
| 1735 | |
| 1736 | === added file 'lib/lp/services/mail/errortemplates/mergedirectivenotsupported.txt' |
| 1737 | --- lib/lp/services/mail/errortemplates/mergedirectivenotsupported.txt 1970-01-01 00:00:00 +0000 |
| 1738 | +++ lib/lp/services/mail/errortemplates/mergedirectivenotsupported.txt 2012-04-30 14:10:29 +0000 |
| 1739 | @@ -0,0 +1,3 @@ |
| 1740 | +Launchpad no longer accepts merge directives. See bug 989831 for details. We apologise for the inconvenience. |
| 1741 | + |
| 1742 | +The closest alternative to "bzr send" for Launchpad is "bzr lp-propose-merge". This command is from the launchpad plugin which usually ships with Bazaar. Of course, you can also use the Launchpad web site to create merge proposals. |
| 1743 | |
| 1744 | === modified file 'lib/lp/services/messages/interfaces/message.py' |
| 1745 | --- lib/lp/services/messages/interfaces/message.py 2012-01-06 19:48:06 +0000 |
| 1746 | +++ lib/lp/services/messages/interfaces/message.py 2012-04-30 14:10:29 +0000 |
| 1747 | @@ -11,7 +11,6 @@ |
| 1748 | 'IIndexedMessage', |
| 1749 | 'IMessage', |
| 1750 | 'IMessageChunk', |
| 1751 | - 'IMessageJob', |
| 1752 | 'IMessageSet', |
| 1753 | 'IUserToUserEmail', |
| 1754 | 'IndexedMessage', |
| 1755 | @@ -47,7 +46,6 @@ |
| 1756 | |
| 1757 | from lp import _ |
| 1758 | from lp.app.errors import NotFoundError |
| 1759 | -from lp.services.job.interfaces.job import IJob |
| 1760 | from lp.services.librarian.interfaces import ILibraryFileAlias |
| 1761 | |
| 1762 | |
| 1763 | @@ -287,22 +285,6 @@ |
| 1764 | """ |
| 1765 | |
| 1766 | |
| 1767 | -class IMessageJob(Interface): |
| 1768 | - """Interface for jobs triggered by messages.""" |
| 1769 | - |
| 1770 | - job = Object(schema=IJob, required=True) |
| 1771 | - |
| 1772 | - message_bytes = Object( |
| 1773 | - title=_('Full MIME content of Email.'), required=True, |
| 1774 | - schema=ILibraryFileAlias) |
| 1775 | - |
| 1776 | - def getMessage(): |
| 1777 | - """Return an email.Message representing this job's message.""" |
| 1778 | - |
| 1779 | - def destroySelf(): |
| 1780 | - """Remove this object (and its job) from the database.""" |
| 1781 | - |
| 1782 | - |
| 1783 | class UnknownSender(NotFoundError): |
| 1784 | """Raised if we cannot lookup an email message's sender in the database""" |
| 1785 | |
| 1786 | |
| 1787 | === modified file 'lib/lp/services/messages/model/message.py' |
| 1788 | --- lib/lp/services/messages/model/message.py 2012-04-13 07:19:14 +0000 |
| 1789 | +++ lib/lp/services/messages/model/message.py 2012-04-30 14:10:29 +0000 |
| 1790 | @@ -8,8 +8,6 @@ |
| 1791 | 'DirectEmailAuthorization', |
| 1792 | 'Message', |
| 1793 | 'MessageChunk', |
| 1794 | - 'MessageJob', |
| 1795 | - 'MessageJobAction', |
| 1796 | 'MessageSet', |
| 1797 | 'UserToUserEmail', |
| 1798 | ] |
| 1799 | @@ -32,10 +30,6 @@ |
| 1800 | import os.path |
| 1801 | |
| 1802 | from lazr.config import as_timedelta |
| 1803 | -from lazr.enum import ( |
| 1804 | - DBEnumeratedType, |
| 1805 | - DBItem, |
| 1806 | - ) |
| 1807 | import pytz |
| 1808 | from sqlobject import ( |
| 1809 | BoolCol, |
| 1810 | @@ -67,17 +61,13 @@ |
| 1811 | from lp.services.config import config |
| 1812 | from lp.services.database.constants import UTC_NOW |
| 1813 | from lp.services.database.datetimecol import UtcDateTimeCol |
| 1814 | -from lp.services.database.enumcol import EnumCol |
| 1815 | from lp.services.database.sqlbase import SQLBase |
| 1816 | from lp.services.encoding import guess as ensure_unicode |
| 1817 | -from lp.services.job.model.job import Job |
| 1818 | from lp.services.librarian.interfaces import ILibraryFileAliasSet |
| 1819 | -from lp.services.mail.signedmessage import signed_message_from_string |
| 1820 | from lp.services.messages.interfaces.message import ( |
| 1821 | IDirectEmailAuthorization, |
| 1822 | IMessage, |
| 1823 | IMessageChunk, |
| 1824 | - IMessageJob, |
| 1825 | IMessageSet, |
| 1826 | InvalidEmailMessage, |
| 1827 | IUserToUserEmail, |
| 1828 | @@ -637,59 +627,6 @@ |
| 1829 | Store.of(sender).add(self) |
| 1830 | |
| 1831 | |
| 1832 | -class MessageJobAction(DBEnumeratedType): |
| 1833 | - """MessageJob action |
| 1834 | - |
| 1835 | - The action that a job should perform. |
| 1836 | - """ |
| 1837 | - |
| 1838 | - CREATE_MERGE_PROPOSAL = DBItem(1, """ |
| 1839 | - Create a merge proposal. |
| 1840 | - |
| 1841 | - Create a merge proposal from a message which must contain a merge |
| 1842 | - directive. |
| 1843 | - """) |
| 1844 | - |
| 1845 | - |
| 1846 | -class MessageJob(Storm): |
| 1847 | - """A job for processing messages.""" |
| 1848 | - |
| 1849 | - implements(IMessageJob) |
| 1850 | - # XXX: AaronBentley 2009-02-05 bug=325883: This table is poorly named. |
| 1851 | - __storm_table__ = 'MergeDirectiveJob' |
| 1852 | - |
| 1853 | - id = Int(primary=True) |
| 1854 | - |
| 1855 | - jobID = Int('job', allow_none=False) |
| 1856 | - job = Reference(jobID, Job.id) |
| 1857 | - |
| 1858 | - message_bytesID = Int('merge_directive', allow_none=False) |
| 1859 | - message_bytes = Reference(message_bytesID, 'LibraryFileAlias.id') |
| 1860 | - |
| 1861 | - action = EnumCol(enum=MessageJobAction) |
| 1862 | - |
| 1863 | - def __init__(self, message_bytes, action): |
| 1864 | - Storm.__init__(self) |
| 1865 | - self.job = Job() |
| 1866 | - self.message_bytes = message_bytes |
| 1867 | - self.action = action |
| 1868 | - |
| 1869 | - def destroySelf(self): |
| 1870 | - """See `IMessageJob`.""" |
| 1871 | - self.job.destroySelf() |
| 1872 | - Store.of(self).remove(self) |
| 1873 | - |
| 1874 | - def sync(self): |
| 1875 | - """Update the database with all changes for this object.""" |
| 1876 | - store = Store.of(self) |
| 1877 | - store.flush() |
| 1878 | - store.autoreload(self) |
| 1879 | - |
| 1880 | - def getMessage(self): |
| 1881 | - """See `IMessageJob`.""" |
| 1882 | - return signed_message_from_string(self.message_bytes.read()) |
| 1883 | - |
| 1884 | - |
| 1885 | class DirectEmailAuthorization: |
| 1886 | """See `IDirectEmailAuthorization`.""" |
| 1887 | |
| 1888 | |
| 1889 | === modified file 'lib/lp/services/messages/tests/test_message.py' |
| 1890 | --- lib/lp/services/messages/tests/test_message.py 2012-01-01 02:58:52 +0000 |
| 1891 | +++ lib/lp/services/messages/tests/test_message.py 2012-04-30 14:10:29 +0000 |
| 1892 | @@ -3,7 +3,6 @@ |
| 1893 | |
| 1894 | __metaclass__ = type |
| 1895 | |
| 1896 | -from cStringIO import StringIO |
| 1897 | from email.header import Header |
| 1898 | from email.message import Message |
| 1899 | from email.mime.multipart import MIMEMultipart |
| 1900 | @@ -13,24 +12,14 @@ |
| 1901 | make_msgid, |
| 1902 | ) |
| 1903 | |
| 1904 | -from sqlobject import SQLObjectNotFound |
| 1905 | import transaction |
| 1906 | -from zope.component import getUtility |
| 1907 | |
| 1908 | -from lp.services.job.model.job import Job |
| 1909 | -from lp.services.librarian.interfaces import ILibraryFileAliasSet |
| 1910 | -from lp.services.mail.sendmail import MailController |
| 1911 | -from lp.services.messages.interfaces.message import IMessageJob |
| 1912 | from lp.services.messages.model.message import ( |
| 1913 | - MessageJob, |
| 1914 | - MessageJobAction, |
| 1915 | MessageSet, |
| 1916 | ) |
| 1917 | -from lp.services.webapp.testing import verifyObject |
| 1918 | from lp.testing import ( |
| 1919 | login, |
| 1920 | TestCase, |
| 1921 | - TestCaseWithFactory, |
| 1922 | ) |
| 1923 | from lp.testing.factory import LaunchpadObjectFactory |
| 1924 | from lp.testing.layers import LaunchpadFunctionalLayer |
| 1925 | @@ -200,41 +189,3 @@ |
| 1926 | 'Treating unknown encoding "booga" as latin-1.'): |
| 1927 | result = MessageSet.decode(self.high_characters, 'booga') |
| 1928 | self.assertEqual(self.high_characters.decode('latin-1'), result) |
| 1929 | - |
| 1930 | - |
| 1931 | -class TestMessageJob(TestCaseWithFactory): |
| 1932 | - """Tests for MessageJob.""" |
| 1933 | - |
| 1934 | - layer = LaunchpadFunctionalLayer |
| 1935 | - |
| 1936 | - def test_providesInterface(self): |
| 1937 | - """Ensure that BranchJob implements IBranchJob.""" |
| 1938 | - # Ensure database constraints are satisfied. |
| 1939 | - file_alias = self.factory.makeMergeDirectiveEmail()[1] |
| 1940 | - job = MessageJob(file_alias, MessageJobAction.CREATE_MERGE_PROPOSAL) |
| 1941 | - job.sync() |
| 1942 | - verifyObject(IMessageJob, job) |
| 1943 | - |
| 1944 | - def test_destroySelf_destroys_job(self): |
| 1945 | - """Ensure that MessageJob.destroySelf destroys the Job as well.""" |
| 1946 | - file_alias = self.factory.makeMergeDirectiveEmail()[1] |
| 1947 | - message_job = MessageJob( |
| 1948 | - file_alias, MessageJobAction.CREATE_MERGE_PROPOSAL) |
| 1949 | - job_id = message_job.job.id |
| 1950 | - message_job.destroySelf() |
| 1951 | - self.assertRaises(SQLObjectNotFound, Job.get, job_id) |
| 1952 | - |
| 1953 | - def test_getMessage(self): |
| 1954 | - """getMessage should return a Message with appropriate values.""" |
| 1955 | - ctrl = MailController( |
| 1956 | - 'from@example.com', ['to@example.com'], 'subject', 'body') |
| 1957 | - content = ctrl.makeMessage().as_string() |
| 1958 | - lfa = getUtility(ILibraryFileAliasSet).create( |
| 1959 | - 'message', len(content), StringIO(content), 'text/x-diff') |
| 1960 | - message_job = MessageJob(lfa, MessageJobAction.CREATE_MERGE_PROPOSAL) |
| 1961 | - transaction.commit() |
| 1962 | - message = message_job.getMessage() |
| 1963 | - self.assertEqual('from@example.com', message['From']) |
| 1964 | - self.assertEqual('to@example.com', message['To']) |
| 1965 | - self.assertEqual('subject', message['Subject']) |
| 1966 | - self.assertEqual('body', message.get_payload()) |
| 1967 | |
| 1968 | === modified file 'lib/lp/testing/factory.py' |
| 1969 | --- lib/lp/testing/factory.py 2012-04-23 20:27:38 +0000 |
| 1970 | +++ lib/lp/testing/factory.py 2012-04-30 14:10:29 +0000 |
| 1971 | @@ -45,7 +45,6 @@ |
| 1972 | from types import InstanceType |
| 1973 | import warnings |
| 1974 | |
| 1975 | -from bzrlib.merge_directive import MergeDirective2 |
| 1976 | from bzrlib.plugins.builder.recipe import BaseRecipeBranch |
| 1977 | from bzrlib.revision import Revision as BzrRevision |
| 1978 | from lazr.jobrunner.jobrunner import SuspendJobException |
| 1979 | @@ -4100,87 +4099,6 @@ |
| 1980 | msg.attach(attachment) |
| 1981 | return msg |
| 1982 | |
| 1983 | - def makeBundleMergeDirectiveEmail(self, source_branch, target_branch, |
| 1984 | - signing_context=None, sender=None): |
| 1985 | - """Create a merge directive email from two bzr branches. |
| 1986 | - |
| 1987 | - :param source_branch: The source branch for the merge directive. |
| 1988 | - :param target_branch: The target branch for the merge directive. |
| 1989 | - :param signing_context: A GPGSigningContext instance containing the |
| 1990 | - gpg key to sign with. If None, the message is unsigned. The |
| 1991 | - context also contains the password and gpg signing mode. |
| 1992 | - :param sender: The `Person` that is sending the email. |
| 1993 | - """ |
| 1994 | - md = MergeDirective2.from_objects( |
| 1995 | - source_branch.repository, source_branch.last_revision(), |
| 1996 | - public_branch=source_branch.get_public_branch(), |
| 1997 | - target_branch=target_branch.getInternalBzrUrl(), |
| 1998 | - local_target_branch=target_branch.getInternalBzrUrl(), time=0, |
| 1999 | - timezone=0) |
| 2000 | - email = None |
| 2001 | - if sender is not None: |
| 2002 | - email = removeSecurityProxy(sender).preferredemail.email |
| 2003 | - return self.makeSignedMessage( |
| 2004 | - body='My body', subject='My subject', |
| 2005 | - attachment_contents=''.join(md.to_lines()), |
| 2006 | - signing_context=signing_context, email_address=email) |
| 2007 | - |
| 2008 | - def makeMergeDirective(self, source_branch=None, target_branch=None, |
| 2009 | - source_branch_url=None, target_branch_url=None): |
| 2010 | - """Return a bzr merge directive object. |
| 2011 | - |
| 2012 | - :param source_branch: The source database branch in the merge |
| 2013 | - directive. |
| 2014 | - :param target_branch: The target database branch in the merge |
| 2015 | - directive. |
| 2016 | - :param source_branch_url: The URL of the source for the merge |
| 2017 | - directive. Overrides source_branch. |
| 2018 | - :param target_branch_url: The URL of the target for the merge |
| 2019 | - directive. Overrides target_branch. |
| 2020 | - """ |
| 2021 | - from bzrlib.merge_directive import MergeDirective2 |
| 2022 | - if source_branch_url is not None: |
| 2023 | - assert source_branch is None |
| 2024 | - else: |
| 2025 | - if source_branch is None: |
| 2026 | - source_branch = self.makeAnyBranch() |
| 2027 | - source_branch_url = ( |
| 2028 | - config.codehosting.supermirror_root + |
| 2029 | - source_branch.unique_name) |
| 2030 | - if target_branch_url is not None: |
| 2031 | - assert target_branch is None |
| 2032 | - else: |
| 2033 | - if target_branch is None: |
| 2034 | - target_branch = self.makeAnyBranch() |
| 2035 | - target_branch_url = ( |
| 2036 | - config.codehosting.supermirror_root + |
| 2037 | - target_branch.unique_name) |
| 2038 | - return MergeDirective2( |
| 2039 | - 'revid', 'sha', 0, 0, target_branch_url, |
| 2040 | - source_branch=source_branch_url, base_revision_id='base-revid', |
| 2041 | - patch='') |
| 2042 | - |
| 2043 | - def makeMergeDirectiveEmail(self, body='Hi!\n', signing_context=None): |
| 2044 | - """Create an email with a merge directive attached. |
| 2045 | - |
| 2046 | - :param body: The message body to use for the email. |
| 2047 | - :param signing_context: A GPGSigningContext instance containing the |
| 2048 | - gpg key to sign with. If None, the message is unsigned. The |
| 2049 | - context also contains the password and gpg signing mode. |
| 2050 | - :return: message, file_alias, source_branch, target_branch |
| 2051 | - """ |
| 2052 | - target_branch = self.makeProductBranch() |
| 2053 | - source_branch = self.makeProductBranch( |
| 2054 | - product=target_branch.product) |
| 2055 | - md = self.makeMergeDirective(source_branch, target_branch) |
| 2056 | - message = self.makeSignedMessage(body=body, |
| 2057 | - subject='My subject', attachment_contents=''.join(md.to_lines()), |
| 2058 | - signing_context=signing_context) |
| 2059 | - message_string = message.as_string() |
| 2060 | - file_alias = getUtility(ILibraryFileAliasSet).create( |
| 2061 | - '*', len(message_string), StringIO(message_string), '*') |
| 2062 | - return message, file_alias, source_branch, target_branch |
| 2063 | - |
| 2064 | def makeHWSubmission(self, date_created=None, submission_key=None, |
| 2065 | emailaddress=u'test@canonical.com', |
| 2066 | distroarchseries=None, private=False, |
| 2067 | @@ -4446,7 +4364,6 @@ |
| 2068 | BaseRecipeBranch, |
| 2069 | DSCFile, |
| 2070 | InstanceType, |
| 2071 | - MergeDirective2, |
| 2072 | Message, |
| 2073 | datetime, |
| 2074 | int, |

Thanks for this massive delete. The red is pleasing.
My only suggestion is around the email wording. In the effort of keeping things simple I'd leave out the bits about reasoning. I'd also move the usage of the webui up so that it's more prominent in the text. So for example I'd suggest something like:
Launchpad no longer accepts merge directives, we apologise for the inconvenience.
You may create merge proposals using the Launchpad website or from the command
line using "bzr lp-propose-merge". This command is from the launchpad plugin
which usually ships with Bazaar.