From d8c601f21b195511dad4ed9d63bd0d7961db251b Mon Sep 17 00:00:00 2001
From: Nick Harper <nharper@fb.com>
Date: Mon, 27 Feb 2012 17:11:25 -0800
Subject: [PATCH] Move functionality of PhabricatorMetaMTADaemon to a worker
 task

Summary:
This will allow sending mail to be done by task workers. See T750.

Task ID: #

Blame Rev:

Test Plan:
- started taskmaster daemon in test env
- used "send new test message" feature in MetMTA (with send now unchecked)
- confirmed receipt of 1 email
- repeated 2 & 3 with send now checked

Revert Plan:

Tags:

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T388, T750

Differential Revision: https://secure.phabricator.com/D1723
---
 src/__phutil_library_map__.php                |  4 +-
 .../PhabricatorDaemonConsoleController.php    |  4 +-
 .../daemon/mta/PhabricatorMetaMTADaemon.php   | 35 -------------
 .../storage/mail/PhabricatorMetaMTAMail.php   | 22 ++++++++
 .../metamta/storage/mail/__init__.php         |  1 +
 .../worker/PhabricatorMetaMTAWorker.php       | 51 +++++++++++++++++++
 .../{daemon/mta => worker}/__init__.php       |  4 +-
 .../configuring_outbound_email.diviner        | 11 ++--
 .../configuration/managing_daemons.diviner    |  2 -
 9 files changed, 84 insertions(+), 50 deletions(-)
 delete mode 100644 src/applications/metamta/daemon/mta/PhabricatorMetaMTADaemon.php
 create mode 100644 src/applications/metamta/worker/PhabricatorMetaMTAWorker.php
 rename src/applications/metamta/{daemon/mta => worker}/__init__.php (62%)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index bdd1fdebe0..234b1324b2 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -600,7 +600,6 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTAAttachment' => 'applications/metamta/storage/mail',
     'PhabricatorMetaMTAController' => 'applications/metamta/controller/base',
     'PhabricatorMetaMTADAO' => 'applications/metamta/storage/base',
-    'PhabricatorMetaMTADaemon' => 'applications/metamta/daemon/mta',
     'PhabricatorMetaMTAEmailBodyParser' => 'applications/metamta/parser',
     'PhabricatorMetaMTAEmailBodyParserTestCase' => 'applications/metamta/parser/__tests__',
     'PhabricatorMetaMTAListController' => 'applications/metamta/controller/list',
@@ -615,6 +614,7 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTASendController' => 'applications/metamta/controller/send',
     'PhabricatorMetaMTASendGridReceiveController' => 'applications/metamta/controller/sendgridreceive',
     'PhabricatorMetaMTAViewController' => 'applications/metamta/controller/view',
+    'PhabricatorMetaMTAWorker' => 'applications/metamta/worker',
     'PhabricatorMySQLFileStorageEngine' => 'applications/files/engine/mysql',
     'PhabricatorOAuthClientAuthorization' => 'applications/oauthserver/storage/clientauthorization',
     'PhabricatorOAuthClientAuthorizationBaseController' => 'applications/oauthserver/controller/clientauthorization/base',
@@ -1368,7 +1368,6 @@ phutil_register_library_map(array(
     'PhabricatorMailImplementationTestAdapter' => 'PhabricatorMailImplementationAdapter',
     'PhabricatorMetaMTAController' => 'PhabricatorController',
     'PhabricatorMetaMTADAO' => 'PhabricatorLiskDAO',
-    'PhabricatorMetaMTADaemon' => 'PhabricatorDaemon',
     'PhabricatorMetaMTAEmailBodyParserTestCase' => 'PhabricatorTestCase',
     'PhabricatorMetaMTAListController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTAMail' => 'PhabricatorMetaMTADAO',
@@ -1382,6 +1381,7 @@ phutil_register_library_map(array(
     'PhabricatorMetaMTASendController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTASendGridReceiveController' => 'PhabricatorMetaMTAController',
     'PhabricatorMetaMTAViewController' => 'PhabricatorMetaMTAController',
+    'PhabricatorMetaMTAWorker' => 'PhabricatorWorker',
     'PhabricatorMySQLFileStorageEngine' => 'PhabricatorFileStorageEngine',
     'PhabricatorOAuthClientAuthorization' => 'PhabricatorOAuthServerDAO',
     'PhabricatorOAuthClientAuthorizationBaseController' => 'PhabricatorOAuthServerController',
diff --git a/src/applications/daemon/controller/console/PhabricatorDaemonConsoleController.php b/src/applications/daemon/controller/console/PhabricatorDaemonConsoleController.php
index b9f88b4c59..cedc0b2ba6 100644
--- a/src/applications/daemon/controller/console/PhabricatorDaemonConsoleController.php
+++ b/src/applications/daemon/controller/console/PhabricatorDaemonConsoleController.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -75,7 +75,7 @@ class PhabricatorDaemonConsoleController extends PhabricatorDaemonController {
         'ID',
         'Class',
         'Owner',
-        'Expries',
+        'Expires',
         'Failures',
         '',
       ));
diff --git a/src/applications/metamta/daemon/mta/PhabricatorMetaMTADaemon.php b/src/applications/metamta/daemon/mta/PhabricatorMetaMTADaemon.php
deleted file mode 100644
index 43f2c8fc4f..0000000000
--- a/src/applications/metamta/daemon/mta/PhabricatorMetaMTADaemon.php
+++ /dev/null
@@ -1,35 +0,0 @@
-<?php
-
-/*
- * Copyright 2011 Facebook, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-class PhabricatorMetaMTADaemon extends PhabricatorDaemon {
-
-  public function run() {
-    echo "OK. Sending mail";
-    do {
-      $mail = id(new PhabricatorMetaMTAMail())->loadAllWhere(
-        'status = %s AND nextRetry <= %d LIMIT 10',
-        PhabricatorMetaMTAMail::STATUS_QUEUE,
-        time());
-      foreach ($mail as $message) {
-        $message->sendNow();
-      }
-      $this->sleep(1);
-    } while (true);
-  }
-
-}
diff --git a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php
index d1e5cfa9b8..ff333bed1d 100644
--- a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php
+++ b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php
@@ -171,6 +171,15 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
     return $this;
   }
 
+  public function getWorkerTaskID() {
+    return $this->getParam('worker-task');
+  }
+
+  public function setWorkerTaskID($id) {
+    $this->setParam('worker-task', $id);
+    return $this;
+  }
+
   /**
    * Flag that this is an auto-generated bulk message and should have bulk
    * headers added to it if appropriate. Broadly, this means some flavor of
@@ -223,6 +232,19 @@ class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
     return $ret;
   }
 
+  protected function didWriteData() {
+    parent::didWriteData();
+
+    if (!$this->getWorkerTaskID()) {
+      $mailer_task = new PhabricatorWorkerTask();
+      $mailer_task->setTaskClass('PhabricatorMetaMTAWorker');
+      $mailer_task->setData($this->getID());
+      $mailer_task->save();
+      $this->setWorkerTaskID($mailer_task->getID());
+      $this->save();
+    }
+  }
+
 
   public function buildDefaultMailer() {
     $class_name = PhabricatorEnv::getEnvConfig('metamta.mail-adapter');
diff --git a/src/applications/metamta/storage/mail/__init__.php b/src/applications/metamta/storage/mail/__init__.php
index 16046a8a08..4407f44774 100644
--- a/src/applications/metamta/storage/mail/__init__.php
+++ b/src/applications/metamta/storage/mail/__init__.php
@@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/metamta/storage/base');
 phutil_require_module('phabricator', 'applications/people/storage/preferences');
 phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
+phutil_require_module('phabricator', 'infrastructure/daemon/workers/storage/task');
 phutil_require_module('phabricator', 'infrastructure/env');
 
 phutil_require_module('phutil', 'symbols');
diff --git a/src/applications/metamta/worker/PhabricatorMetaMTAWorker.php b/src/applications/metamta/worker/PhabricatorMetaMTAWorker.php
new file mode 100644
index 0000000000..10643b0d17
--- /dev/null
+++ b/src/applications/metamta/worker/PhabricatorMetaMTAWorker.php
@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class PhabricatorMetaMTAWorker
+  extends PhabricatorWorker {
+
+  private $message;
+
+  public function getRequiredLeaseTime() {
+    $message_id = $this->getTaskData();
+
+    $this->message = id(new PhabricatorMetaMTAMail())->loadOneWhere(
+      'id = %d', $this->getTaskData());
+    if (!$this->message) {
+      return null;
+    }
+
+    $lease_duration = max($this->message->getNextRetry() - time(), 0) + 15;
+    return $lease_duration;
+  }
+
+  public function doWork() {
+    $message = $this->message;
+    if (!$message
+        || $message->getStatus() != PhabricatorMetaMTAMail::STATUS_QUEUE) {
+      return;
+    }
+    $id = $message->getID();
+    $message->sendNow();
+    // task failed if the message is still queued
+    // (instead of sent, void, or failed)
+    if ($message->getStatus() == PhabricatorMetaMTAMail::STATUS_QUEUE) {
+      throw new Exception('Failed to send message');
+    }
+  }
+}
diff --git a/src/applications/metamta/daemon/mta/__init__.php b/src/applications/metamta/worker/__init__.php
similarity index 62%
rename from src/applications/metamta/daemon/mta/__init__.php
rename to src/applications/metamta/worker/__init__.php
index 65094218e2..7da9aefb58 100644
--- a/src/applications/metamta/daemon/mta/__init__.php
+++ b/src/applications/metamta/worker/__init__.php
@@ -7,9 +7,9 @@
 
 
 phutil_require_module('phabricator', 'applications/metamta/storage/mail');
-phutil_require_module('phabricator', 'infrastructure/daemon/base');
+phutil_require_module('phabricator', 'infrastructure/daemon/workers/worker');
 
 phutil_require_module('phutil', 'utils');
 
 
-phutil_require_source('PhabricatorMetaMTADaemon.php');
+phutil_require_source('PhabricatorMetaMTAWorker.php');
diff --git a/src/docs/configuration/configuring_outbound_email.diviner b/src/docs/configuration/configuring_outbound_email.diviner
index d0bfce871e..564f8db100 100644
--- a/src/docs/configuration/configuring_outbound_email.diviner
+++ b/src/docs/configuration/configuring_outbound_email.diviner
@@ -133,18 +133,15 @@ disable outbound mail, if you don't want to send mail or don't want to configure
 it yet. Just set **metamta.mail-adapter** to
 "PhabricatorMailImplementationTestAdapter".
 
-= Configuring the MetaMTA Daemon =
+= Configuring MetaMTA to Send Mail Using a Daemon =
 
 Regardless of how you are sending outbound email, you can move the handoff to
 the MTA out of the main process and into a daemon. This will greatly improve
 application performance if your mailer is slow, like Amazon SES. In particular,
 commenting on Differential Revisions and Maniphest Tasks sends outbound email.
 
-To use the MetaMTA daemon:
-
-  - set **metamta.send-immediately** to ##false## in your configuration; and
-  - launch a ##metamta## daemon with ##phabricator/bin/phd launch metamta##.
-
+If you set **metamta.send-immediately** to ##false## in your configuration,
+MetaMTA will queue mail to be send by a PhabricatorTaskmasterDaemon.
 For more information on using daemons, see @{article:Managing Daemons with phd}.
 
 = Testing and Debugging Outbound Email =
@@ -169,4 +166,4 @@ Continue by:
   - @{article:Configuring Inbound Email} so users can reply to email they
   receive about revisions and tasks to interact with them; or
   - learning about daemons with @{article:Managing Daemons with phd}; or
-  - returning to the @{article:Configuration Guide}.
\ No newline at end of file
+  - returning to the @{article:Configuration Guide}.
diff --git a/src/docs/configuration/managing_daemons.diviner b/src/docs/configuration/managing_daemons.diviner
index fdc7dfe9ed..a0b9f99b26 100644
--- a/src/docs/configuration/managing_daemons.diviner
+++ b/src/docs/configuration/managing_daemons.diviner
@@ -65,8 +65,6 @@ You can get a list of launchable daemons with **phd list**:
 
   - **libphutil test daemons** are not generally useful unless you are
     developing daemon infrastructure or debugging a daemon problem;
-  - **PhabricatorMetaMTADaemon** sends mail in the background, see
-    @{article:Configuring Outbound Email} for details;
   - **PhabricatorTaskmasterDaemon** runs a generic task queue; and
   - **PhabricatorRepository** daemons track repositories, descriptions are
     available in the @{article:Diffusion User Guide}.