Skip to content

Commit e80c59c

Browse files
author
epriestley
committed
Introduce basic bin/mail with a resend workflow
Summary: Fixes T2458. Ref T2843. @tido's email from T2843 has exhausted its retries and failed, but we want to try it again with the patch from D5464 to capture the actual error. This sort of thing has come up a few times in debugging, too. Also fixed some stuff that came up while debugging this. Test Plan: - Ran command with no args. - Ran resend with no args. - Ran resend with bad IDs. - Ran resend with already-queued messages, got "already queued" error. - Ran resend with already-sent message, got requeue. Reviewers: btrahan, tido Reviewed By: tido CC: aran Maniphest Tasks: T2458, T2843 Differential Revision: https://secure.phabricator.com/D5493
1 parent 9ca2bb9 commit e80c59c

File tree

8 files changed

+140
-34
lines changed

8 files changed

+140
-34
lines changed

bin/mail

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../scripts/mail/manage_mail.php

scripts/mail/create_worker_tasks.php

Lines changed: 0 additions & 29 deletions
This file was deleted.

scripts/mail/manage_mail.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#!/usr/bin/env php
2+
<?php
3+
4+
$root = dirname(dirname(dirname(__FILE__)));
5+
require_once $root.'/scripts/__init_script__.php';
6+
7+
$args = new PhutilArgumentParser($argv);
8+
$args->setTagline('manage mail');
9+
$args->setSynopsis(<<<EOSYNOPSIS
10+
**mail** __command__ [__options__]
11+
Manage Phabricator mail stuff.
12+
13+
EOSYNOPSIS
14+
);
15+
$args->parseStandardArguments();
16+
17+
$workflows = array(
18+
new PhabricatorMailManagementResendWorkflow(),
19+
new PhutilHelpArgumentWorkflow(),
20+
);
21+
22+
$args->parseWorkflows($workflows);

src/__phutil_library_map__.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,8 @@
10481048
'PhabricatorMailImplementationPHPMailerLiteAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationPHPMailerLiteAdapter.php',
10491049
'PhabricatorMailImplementationSendGridAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationSendGridAdapter.php',
10501050
'PhabricatorMailImplementationTestAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php',
1051+
'PhabricatorMailManagementResendWorkflow' => 'applications/metamta/management/PhabricatorMailManagementResendWorkflow.php',
1052+
'PhabricatorMailManagementWorkflow' => 'applications/metamta/management/PhabricatorMailManagementWorkflow.php',
10511053
'PhabricatorMailReplyHandler' => 'applications/metamta/replyhandler/PhabricatorMailReplyHandler.php',
10521054
'PhabricatorMailingListsController' => 'applications/mailinglists/controller/PhabricatorMailingListsController.php',
10531055
'PhabricatorMailingListsEditController' => 'applications/mailinglists/controller/PhabricatorMailingListsEditController.php',
@@ -2714,6 +2716,8 @@
27142716
'PhabricatorMailImplementationPHPMailerLiteAdapter' => 'PhabricatorMailImplementationAdapter',
27152717
'PhabricatorMailImplementationSendGridAdapter' => 'PhabricatorMailImplementationAdapter',
27162718
'PhabricatorMailImplementationTestAdapter' => 'PhabricatorMailImplementationAdapter',
2719+
'PhabricatorMailManagementResendWorkflow' => 'PhabricatorSearchManagementWorkflow',
2720+
'PhabricatorMailManagementWorkflow' => 'PhutilArgumentWorkflow',
27172721
'PhabricatorMailingListsController' => 'PhabricatorController',
27182722
'PhabricatorMailingListsEditController' => 'PhabricatorMailingListsController',
27192723
'PhabricatorMailingListsListController' => 'PhabricatorMailingListsController',
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
3+
final class PhabricatorMailManagementResendWorkflow
4+
extends PhabricatorSearchManagementWorkflow {
5+
6+
protected function didConstruct() {
7+
$this
8+
->setName('resend')
9+
->setSynopsis('Send mail again.')
10+
->setExamples(
11+
"**resend** --id 1 --id 2")
12+
->setArguments(
13+
array(
14+
array(
15+
'name' => 'id',
16+
'param' => 'id',
17+
'help' => 'Send mail with a given ID again.',
18+
'repeat' => true,
19+
),
20+
));
21+
}
22+
23+
public function execute(PhutilArgumentParser $args) {
24+
$console = PhutilConsole::getConsole();
25+
26+
$ids = $args->getArg('id');
27+
if (!$ids) {
28+
throw new PhutilArgumentUsageException(
29+
"Use the '--id' flag to specify one or more messages to resend.");
30+
}
31+
32+
$messages = id(new PhabricatorMetaMTAMail())->loadAllWhere(
33+
'id IN (%Ld)',
34+
$ids);
35+
36+
if ($ids) {
37+
$ids = array_fuse($ids);
38+
$missing = array_diff_key($ids, $messages);
39+
if ($missing) {
40+
throw new PhutilArgumentUsageException(
41+
"Some specified messages do not exist: ".
42+
implode(', ', array_keys($missing)));
43+
}
44+
}
45+
46+
foreach ($messages as $message) {
47+
if ($message->getStatus() == PhabricatorMetaMTAMail::STATUS_QUEUE) {
48+
if ($message->getWorkerTaskID()) {
49+
$console->writeOut(
50+
"Message #%d is already queued with an assigned send task.\n",
51+
$message->getID());
52+
continue;
53+
}
54+
}
55+
56+
$message->setStatus(PhabricatorMetaMTAMail::STATUS_QUEUE);
57+
$message->setRetryCount(0);
58+
$message->setNextRetry(time());
59+
60+
$message->save();
61+
62+
$mailer_task = PhabricatorWorker::scheduleTask(
63+
'PhabricatorMetaMTAWorker',
64+
$message->getID());
65+
66+
$message->setWorkerTaskID($mailer_task->getID());
67+
$message->save();
68+
69+
$console->writeOut(
70+
"Queued message #%d for resend.\n",
71+
$message->getID());
72+
}
73+
}
74+
75+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
abstract class PhabricatorMailManagementWorkflow
4+
extends PhutilArgumentWorkflow {
5+
6+
final public function isExecutable() {
7+
return true;
8+
}
9+
10+
}

src/applications/metamta/storage/PhabricatorMetaMTAAttachment.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,20 @@ public function setMimeType($mimetype) {
3737
$this->mimetype = $mimetype;
3838
return $this;
3939
}
40+
41+
public function toDictionary() {
42+
return array(
43+
'filename' => $this->getFilename(),
44+
'mimetype' => $this->getMimetype(),
45+
'data' => $this->getData(),
46+
);
47+
}
48+
49+
public static function newFromDictionary(array $dict) {
50+
return new PhabricatorMetaMTAAttachment(
51+
idx($dict, 'data'),
52+
idx($dict, 'filename'),
53+
idx($dict, 'mimetype'));
54+
}
55+
4056
}

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,17 +165,23 @@ public function addHeader($name, $value) {
165165
}
166166

167167
public function addAttachment(PhabricatorMetaMTAAttachment $attachment) {
168-
$this->parameters['attachments'][] = $attachment;
168+
$this->parameters['attachments'][] = $attachment->toDictionary();
169169
return $this;
170170
}
171171

172172
public function getAttachments() {
173-
return $this->getParam('attachments');
173+
$dicts = $this->getParam('attachments');
174+
175+
$result = array();
176+
foreach ($dicts as $dict) {
177+
$result[] = PhabricatorMetaMTAAttachment::newFromDictionary($dict);
178+
}
179+
return $result;
174180
}
175181

176182
public function setAttachments(array $attachments) {
177183
assert_instances_of($attachments, 'PhabricatorMetaMTAAttachment');
178-
$this->setParam('attachments', $attachments);
184+
$this->setParam('attachments', mpull($attachments, 'toDictionary'));
179185
return $this;
180186
}
181187

@@ -430,6 +436,7 @@ public function sendNow(
430436
}
431437
break;
432438
case 'attachments':
439+
$value = $this->getAttachments();
433440
foreach ($value as $attachment) {
434441
$mailer->addAttachment(
435442
$attachment->getData(),
@@ -715,13 +722,13 @@ private function loadEmailAndNameDataFromPHIDs(array &$phids) {
715722
$is_mailable = false;
716723
switch ($value) {
717724
case PhabricatorPHIDConstants::PHID_TYPE_USER:
718-
$user = $users[$phid];
725+
$user = idx($users, $phid);
719726
if ($user) {
720727
$name = $this->getUserName($user);
721728
$is_mailable = !$user->getIsDisabled()
722729
&& !$user->getIsSystemAgent();
723730
}
724-
$email = $user_emails[$phid] ?
731+
$email = isset($user_emails[$phid]) ?
725732
$user_emails[$phid]->getAddress() :
726733
$default;
727734
break;

0 commit comments

Comments
 (0)