From 73e3057c52f46ec6ddb75fc495a03255c5199c16 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Jan 2019 05:18:26 -0800 Subject: [PATCH] Rename "MetaMTA" mail attachments and add more mail message objects Summary: Depends on D19953. Ref T9141. We have a "MetaMTAAttachment" object, rename it to "MailAttachment". Also add a "Header" object and an "EmailMessage" object. Currently, mail adapters have a large number of methods like `setSubject()`, `addTo()`, etc, that I would like to remove. I'd like the API to be more like `sendMessage(PhabricatorMailExternalMessage $message)`. This is likely a significant simplification anyway, since the implementations of all these methods are just copy/pasted boilerplate anyway (lots of `$this->subject = $subject;`) and this will let Adapters support other message media (SMS, APNS, Whatsapp, etc.) That's a larger change, but move toward a world where we can build a concrete `$message` object for "email" or "sms". The `PhabricatorMailEmailMessage` object is just a dumb, flat object representation of the information an adapter needs to actually send mail. The existing `PhabricatorMetaMTAMail` is a much more complex object and has a lot of rich data (delivery status, related object PHIDs, etc) and is a storage object. The new flow will be something like: - `PhabricatorMetaMTAMail` (possibly renamed) is the storage object for any outbound message on this channel. It tracks message content, acceptable delivery media (SMS vs email), delivery status, related objects, has a PHID, and has a daemon worker associated with delivering it. - It builds a `PhabricatorMailExternalMessage`, which is a simple, flat description of the message it wants to send. The subclass of this object depends on the message medium. For email, this will be an `EmailMessage`. This is just a "bag of strings" sort of object, with appropriate flattened values for the adapter to work with (e.g., Email has email addresses, SMS has phone numbers). - It passes the `ExternalMessage` (which is a `MailMessage` or `SMSMessage` or whatever) to the adapter. - The adapter reads the nice flat properties off it and turns them into an API request, SMTP call, etc. This is sort of how things work today anyway. The major change is that I want to hand off a "bag of strings" object instead of calling `setX()`, `setY()`, `setZ()` with each individual value. Test Plan: Grepped for `MetaMTAAttachment`. This doesn't change any behavior yet. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T9141 Differential Revision: https://secure.phabricator.com/D19954 --- src/__phutil_library_map__.php | 10 +- .../audit/editor/PhabricatorAuditEditor.php | 2 +- .../editor/PhabricatorCalendarEventEditor.php | 2 +- .../editor/DifferentialTransactionEditor.php | 2 +- ...bricatorMailManagementSendTestWorkflow.php | 2 +- .../PhabricatorMailAttachment.php} | 2 +- .../message/PhabricatorMailEmailMessage.php | 103 ++++++++++++++++++ .../PhabricatorMailExternalMessage.php | 10 ++ .../metamta/message/PhabricatorMailHeader.php | 27 +++++ .../storage/PhabricatorMetaMTAMail.php | 6 +- .../view/PhabricatorMetaMTAMailBody.php | 6 +- 11 files changed, 159 insertions(+), 13 deletions(-) rename src/applications/metamta/{storage/PhabricatorMetaMTAAttachment.php => message/PhabricatorMailAttachment.php} (96%) create mode 100644 src/applications/metamta/message/PhabricatorMailEmailMessage.php create mode 100644 src/applications/metamta/message/PhabricatorMailExternalMessage.php create mode 100644 src/applications/metamta/message/PhabricatorMailHeader.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9caa34f021..07ecac9a17 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3373,11 +3373,15 @@ phutil_register_library_map(array( 'PhabricatorMacroTransactionQuery' => 'applications/macro/query/PhabricatorMacroTransactionQuery.php', 'PhabricatorMacroTransactionType' => 'applications/macro/xaction/PhabricatorMacroTransactionType.php', 'PhabricatorMacroViewController' => 'applications/macro/controller/PhabricatorMacroViewController.php', + 'PhabricatorMailAttachment' => 'applications/metamta/message/PhabricatorMailAttachment.php', 'PhabricatorMailConfigTestCase' => 'applications/metamta/storage/__tests__/PhabricatorMailConfigTestCase.php', 'PhabricatorMailEmailHeraldField' => 'applications/metamta/herald/PhabricatorMailEmailHeraldField.php', 'PhabricatorMailEmailHeraldFieldGroup' => 'applications/metamta/herald/PhabricatorMailEmailHeraldFieldGroup.php', + 'PhabricatorMailEmailMessage' => 'applications/metamta/message/PhabricatorMailEmailMessage.php', 'PhabricatorMailEmailSubjectHeraldField' => 'applications/metamta/herald/PhabricatorMailEmailSubjectHeraldField.php', 'PhabricatorMailEngineExtension' => 'applications/metamta/engine/PhabricatorMailEngineExtension.php', + 'PhabricatorMailExternalMessage' => 'applications/metamta/message/PhabricatorMailExternalMessage.php', + 'PhabricatorMailHeader' => 'applications/metamta/message/PhabricatorMailHeader.php', 'PhabricatorMailImplementationAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationAdapter.php', 'PhabricatorMailImplementationAmazonSESAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationAmazonSESAdapter.php', 'PhabricatorMailImplementationMailgunAdapter' => 'applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php', @@ -3443,7 +3447,6 @@ phutil_register_library_map(array( 'PhabricatorMetaMTAApplicationEmailQuery' => 'applications/metamta/query/PhabricatorMetaMTAApplicationEmailQuery.php', 'PhabricatorMetaMTAApplicationEmailTransaction' => 'applications/metamta/storage/PhabricatorMetaMTAApplicationEmailTransaction.php', 'PhabricatorMetaMTAApplicationEmailTransactionQuery' => 'applications/metamta/query/PhabricatorMetaMTAApplicationEmailTransactionQuery.php', - 'PhabricatorMetaMTAAttachment' => 'applications/metamta/storage/PhabricatorMetaMTAAttachment.php', 'PhabricatorMetaMTAConfigOptions' => 'applications/config/option/PhabricatorMetaMTAConfigOptions.php', 'PhabricatorMetaMTAController' => 'applications/metamta/controller/PhabricatorMetaMTAController.php', 'PhabricatorMetaMTADAO' => 'applications/metamta/storage/PhabricatorMetaMTADAO.php', @@ -9182,11 +9185,15 @@ phutil_register_library_map(array( 'PhabricatorMacroTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorMacroTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorMacroViewController' => 'PhabricatorMacroController', + 'PhabricatorMailAttachment' => 'Phobject', 'PhabricatorMailConfigTestCase' => 'PhabricatorTestCase', 'PhabricatorMailEmailHeraldField' => 'HeraldField', 'PhabricatorMailEmailHeraldFieldGroup' => 'HeraldFieldGroup', + 'PhabricatorMailEmailMessage' => 'PhabricatorMailExternalMessage', 'PhabricatorMailEmailSubjectHeraldField' => 'PhabricatorMailEmailHeraldField', 'PhabricatorMailEngineExtension' => 'Phobject', + 'PhabricatorMailExternalMessage' => 'Phobject', + 'PhabricatorMailHeader' => 'Phobject', 'PhabricatorMailImplementationAdapter' => 'Phobject', 'PhabricatorMailImplementationAmazonSESAdapter' => 'PhabricatorMailImplementationPHPMailerLiteAdapter', 'PhabricatorMailImplementationMailgunAdapter' => 'PhabricatorMailImplementationAdapter', @@ -9259,7 +9266,6 @@ phutil_register_library_map(array( 'PhabricatorMetaMTAApplicationEmailQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorMetaMTAApplicationEmailTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorMetaMTAApplicationEmailTransactionQuery' => 'PhabricatorApplicationTransactionQuery', - 'PhabricatorMetaMTAAttachment' => 'Phobject', 'PhabricatorMetaMTAConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorMetaMTAController' => 'PhabricatorController', 'PhabricatorMetaMTADAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 165f8ef84f..1b032b3cf6 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -611,7 +611,7 @@ final class PhabricatorAuditEditor $commit->getCommitIdentifier()); $template->addAttachment( - new PhabricatorMetaMTAAttachment( + new PhabricatorMailAttachment( $raw_patch, $commit_name.'.patch', 'text/x-patch; charset='.$encoding)); diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php index f1b72dc0ea..9e3b23a43d 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php @@ -364,7 +364,7 @@ final class PhabricatorCalendarEventEditor ->setEvents(array($event)) ->writeICSDocument(); - $ics_attachment = new PhabricatorMetaMTAAttachment( + $ics_attachment = new PhabricatorMailAttachment( $ics_data, $event->getICSFilename(), 'text/calendar'); diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 8072c19e9a..8bc1b4d972 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -715,7 +715,7 @@ final class DifferentialTransactionEditor $name = pht('D%s.%s.patch', $object->getID(), $diff->getID()); $mime_type = 'text/x-patch; charset=utf-8'; $body->addAttachment( - new PhabricatorMetaMTAAttachment($patch, $name, $mime_type)); + new PhabricatorMailAttachment($patch, $name, $mime_type)); } } } diff --git a/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php index 8140d64bdd..ab5bd7f336 100644 --- a/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php +++ b/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php @@ -193,7 +193,7 @@ final class PhabricatorMailManagementSendTestWorkflow $data = Filesystem::readFile($attachment); $name = basename($attachment); $mime = Filesystem::getMimeType($attachment); - $file = new PhabricatorMetaMTAAttachment($data, $name, $mime); + $file = new PhabricatorMailAttachment($data, $name, $mime); $mail->addAttachment($file); } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAAttachment.php b/src/applications/metamta/message/PhabricatorMailAttachment.php similarity index 96% rename from src/applications/metamta/storage/PhabricatorMetaMTAAttachment.php rename to src/applications/metamta/message/PhabricatorMailAttachment.php index 256bee46e8..7cc7f32fe2 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAAttachment.php +++ b/src/applications/metamta/message/PhabricatorMailAttachment.php @@ -1,6 +1,6 @@ fromAddress = $from_address; + return $this; + } + + public function getFromAddress() { + return $this->fromAddress; + } + + public function setReplyToAddress(PhutilEmailAddress $address) { + $this->replyToAddress = $address; + return $this; + } + + public function getReplyToAddress() { + return $this->replyToAddress; + } + + public function setToAddresses(array $addresses) { + assert_instances_of($addresses, 'PhutilEmailAddress'); + $this->toAddresses = $addresses; + return $this; + } + + public function getToAddresses() { + return $this->toAddresses; + } + + public function setCCAddresses(array $addresses) { + assert_instances_of($addresses, 'PhutilEmailAddress'); + $this->ccAddresses = $addresses; + return $this; + } + + public function getCCAddresses() { + return $this->ccAddresses; + } + + public function setHeaders(array $headers) { + assert_instances_of($headers, 'PhabricatorMailHeader'); + $this->headers = $headers; + return $this; + } + + public function getHeaders() { + return $this->headers; + } + + public function setAttachments(array $attachments) { + assert_instances_of($attachments, 'PhabricatorMailAttachment'); + $this->attachments = $attachments; + return $this; + } + + public function getAttachments() { + return $this->attachments; + } + + public function setSubject($subject) { + $this->subject = $subject; + return $this; + } + + public function getSubject() { + return $this->subject; + } + + public function setTextBody($text_body) { + $this->textBody = $text_body; + return $this; + } + + public function getTextBody() { + return $this->textBody; + } + + public function setHTMLBody($html_body) { + $this->htmlBody = $html_body; + return $this; + } + + public function getHTMLBody() { + return $this->htmlBody; + } + +} diff --git a/src/applications/metamta/message/PhabricatorMailExternalMessage.php b/src/applications/metamta/message/PhabricatorMailExternalMessage.php new file mode 100644 index 0000000000..f691090d18 --- /dev/null +++ b/src/applications/metamta/message/PhabricatorMailExternalMessage.php @@ -0,0 +1,10 @@ +getPhobjectClassConstant('MESSAGETYPE'); + } + +} diff --git a/src/applications/metamta/message/PhabricatorMailHeader.php b/src/applications/metamta/message/PhabricatorMailHeader.php new file mode 100644 index 0000000000..2e60ee5c7d --- /dev/null +++ b/src/applications/metamta/message/PhabricatorMailHeader.php @@ -0,0 +1,27 @@ +name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setValue($value) { + $this->value = $value; + return $this; + } + + public function getValue() { + return $this->value; + } + +} diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index aeff432af5..15c8a0945a 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -191,7 +191,7 @@ final class PhabricatorMetaMTAMail return $this; } - public function addAttachment(PhabricatorMetaMTAAttachment $attachment) { + public function addAttachment(PhabricatorMailAttachment $attachment) { $this->parameters['attachments'][] = $attachment->toDictionary(); return $this; } @@ -201,7 +201,7 @@ final class PhabricatorMetaMTAMail $result = array(); foreach ($dicts as $dict) { - $result[] = PhabricatorMetaMTAAttachment::newFromDictionary($dict); + $result[] = PhabricatorMailAttachment::newFromDictionary($dict); } return $result; } @@ -236,7 +236,7 @@ final class PhabricatorMetaMTAMail } public function setAttachments(array $attachments) { - assert_instances_of($attachments, 'PhabricatorMetaMTAAttachment'); + assert_instances_of($attachments, 'PhabricatorMailAttachment'); $this->setParam('attachments', mpull($attachments, 'toDictionary')); return $this; } diff --git a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php index 778e05b052..6622f0afeb 100644 --- a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php +++ b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php @@ -155,11 +155,11 @@ final class PhabricatorMetaMTAMailBody extends Phobject { /** * Add an attachment. * - * @param PhabricatorMetaMTAAttachment Attachment. + * @param PhabricatorMailAttachment Attachment. * @return this * @task compose */ - public function addAttachment(PhabricatorMetaMTAAttachment $attachment) { + public function addAttachment(PhabricatorMailAttachment $attachment) { $this->attachments[] = $attachment; return $this; } @@ -187,7 +187,7 @@ final class PhabricatorMetaMTAMailBody extends Phobject { /** * Retrieve attachments. * - * @return list Attachments. + * @return list Attachments. * @task render */ public function getAttachments() { -- 2.51.2