From 6bec3d2e4fc27eeff624559b9a78d851879bf375 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 30 Apr 2011 22:51:25 -0700 Subject: [PATCH] Simplify and demuddle MetaMTA send pathways Summary: I pretty shortsightedly made sending a side effect of save() in the case that a server is configured for immediate sending. Move this out, make it explicit, and get rid of all the tangles surrounding it. The web tool now ignores the server setting and only repsects the checkbox, which makes far more sense. Test Plan: Sent mails from Maniphest, Differential, and the web console. Also ran all the unit tests. Verified headers from Maniphest. Reviewed By: rm Reviewers: aran, rm CC: tuomaspelkonen, rm, jungejason, aran Differential Revision: 200 --- .../email/PhabricatorEmailLoginController.php | 4 +-- .../mail/base/DifferentialMail.php | 3 +-- .../ManiphestTransactionEditor.php | 11 +++----- .../send/PhabricatorMetaMTASendController.php | 5 ++-- .../metamta/controller/send/__init__.php | 1 - .../storage/mail/PhabricatorMetaMTAMail.php | 25 +++++++++++-------- ...habricatorRepositoryCommitHeraldWorker.php | 2 +- 7 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/applications/auth/controller/email/PhabricatorEmailLoginController.php b/src/applications/auth/controller/email/PhabricatorEmailLoginController.php index bb43020bfc..2fe6d8aa38 100644 --- a/src/applications/auth/controller/email/PhabricatorEmailLoginController.php +++ b/src/applications/auth/controller/email/PhabricatorEmailLoginController.php @@ -74,9 +74,9 @@ class PhabricatorEmailLoginController extends PhabricatorAuthController { $target_user->getPHID(), )); $mail->setBody( - "blah blah blah ". + "here is your link ". PhabricatorEnv::getURI('/login/etoken/'.$etoken.'/').'?email='.phutil_escape_uri($target_user->getEmail())); - $mail->save(); + $mail->saveAndSend(); $view = new AphrontRequestFailureView(); $view->setHeader('Check Your Email'); diff --git a/src/applications/differential/mail/base/DifferentialMail.php b/src/applications/differential/mail/base/DifferentialMail.php index 4f1e98d073..4d21db2a22 100644 --- a/src/applications/differential/mail/base/DifferentialMail.php +++ b/src/applications/differential/mail/base/DifferentialMail.php @@ -101,8 +101,7 @@ abstract class DifferentialMail { $mail->setRelatedPHID($this->getRevision()->getPHID()); - // Save this to the MetaMTA queue for later delivery to the MTA. - $mail->save(); + $mail->saveAndSend(); } protected function buildSubject() { diff --git a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php index 5ab49be629..8d4ac3430d 100644 --- a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php @@ -173,10 +173,7 @@ class ManiphestTransactionEditor { "TASK DETAIL\n". " ".$task_uri."\n"; - $base = substr(md5($task->getPHID()), 0, 27).' '.pack("N", time()); - $thread_index = base64_encode($base); - - $message_id = 'getPHID().'>'; + $thread_id = 'getPHID().'>'; id(new PhabricatorMetaMTAMail()) ->setSubject( @@ -184,12 +181,10 @@ class ManiphestTransactionEditor { ->setFrom($transaction->getAuthorPHID()) ->addTos($email_to) ->addCCs($email_cc) - ->addHeader('Thread-Index', $thread_index) ->addHeader('Thread-Topic', 'Maniphest Task '.$task->getID()) - ->addHeader('In-Reply-To', $message_id) - ->addHeader('References', $message_id) + ->setThreadID($thread_id, $is_create) ->setRelatedPHID($task->getPHID()) ->setBody($body) - ->save(); + ->saveAndSend(); } } diff --git a/src/applications/metamta/controller/send/PhabricatorMetaMTASendController.php b/src/applications/metamta/controller/send/PhabricatorMetaMTASendController.php index 716d356711..ee86d9e649 100644 --- a/src/applications/metamta/controller/send/PhabricatorMetaMTASendController.php +++ b/src/applications/metamta/controller/send/PhabricatorMetaMTASendController.php @@ -33,9 +33,8 @@ class PhabricatorMetaMTASendController extends PhabricatorMetaMTAController { $mail->setSimulatedFailureCount($request->getInt('failures')); $mail->setIsHTML($request->getInt('html')); $mail->save(); - if ($request->getInt('immediately') && - !PhabricatorEnv::getEnvConfig('metamta.send-immediately')) { - $mail->sendNow($force_send = true); + if ($request->getInt('immediately')) { + $mail->sendNow(); } return id(new AphrontRedirectResponse()) diff --git a/src/applications/metamta/controller/send/__init__.php b/src/applications/metamta/controller/send/__init__.php index 1af138fcd7..a30c192610 100644 --- a/src/applications/metamta/controller/send/__init__.php +++ b/src/applications/metamta/controller/send/__init__.php @@ -9,7 +9,6 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/metamta/controller/base'); phutil_require_module('phabricator', 'applications/metamta/storage/mail'); -phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/checkbox'); phutil_require_module('phabricator', 'view/form/control/submit'); diff --git a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php index 0507acfe92..089fff624e 100644 --- a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php @@ -35,8 +35,6 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { protected $nextRetry; protected $relatedPHID; - private $skipSendOnSave; - public function __construct() { $this->status = self::STATUS_QUEUE; @@ -133,20 +131,27 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return $this; } - public function save() { - $try_send = (PhabricatorEnv::getEnvConfig('metamta.send-immediately')) && - (!$this->getID()) && - (!$this->skipSendOnSave); + /** + * Save a newly created mail to the database and attempt to send it + * immediately if the server is configured for immediate sends. When + * applications generate new mail they should generally use this method to + * deliver it. If the server doesn't use immediate sends, this has the same + * effect as calling save(): the mail will eventually be delivered by the + * MetaMTA daemon. + * + * @return this + */ + public function saveAndSend() { + $ret = $this->save(); - $ret = parent::save(); - - if ($try_send) { + if (PhabricatorEnv::getEnvConfig('metamta.send-immediately')) { $this->sendNow(); } return $ret; } + public function buildDefaultMailer() { $class_name = PhabricatorEnv::getEnvConfig('metamta.mail-adapter'); PhutilSymbolLoader::loadClass($class_name); @@ -181,8 +186,6 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { } } - $this->skipSendOnSave = true; - try { $parameters = $this->parameters; $phids = array(); diff --git a/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php index d180c148bf..ece3ca535f 100644 --- a/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php +++ b/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php @@ -133,6 +133,6 @@ EOBODY; $mailer->setFrom($author_phid); } - $mailer->save(); + $mailer->saveAndSend(); } }