diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b54d5ef98d..e1bd968d3b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1233,6 +1233,8 @@ phutil_register_library_map(array( 'PhabricatorMarkupInterface' => 'infrastructure/markup/PhabricatorMarkupInterface.php', 'PhabricatorMarkupOneOff' => 'infrastructure/markup/PhabricatorMarkupOneOff.php', 'PhabricatorMercurialGraphStream' => 'applications/repository/daemon/PhabricatorMercurialGraphStream.php', + 'PhabricatorMetaMTAActor' => 'applications/metamta/query/PhabricatorMetaMTAActor.php', + 'PhabricatorMetaMTAActorQuery' => 'applications/metamta/query/PhabricatorMetaMTAActorQuery.php', 'PhabricatorMetaMTAAttachment' => 'applications/metamta/storage/PhabricatorMetaMTAAttachment.php', 'PhabricatorMetaMTAConfigOptions' => 'applications/config/option/PhabricatorMetaMTAConfigOptions.php', 'PhabricatorMetaMTAController' => 'applications/metamta/controller/PhabricatorMetaMTAController.php', @@ -3183,6 +3185,7 @@ phutil_register_library_map(array( 'PhabricatorManiphestTaskTestDataGenerator' => 'PhabricatorTestDataGenerator', 'PhabricatorMarkupCache' => 'PhabricatorCacheDAO', 'PhabricatorMarkupOneOff' => 'PhabricatorMarkupInterface', + 'PhabricatorMetaMTAActorQuery' => 'PhabricatorQuery', 'PhabricatorMetaMTAConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorMetaMTAController' => 'PhabricatorController', 'PhabricatorMetaMTADAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/metamta/query/PhabricatorMetaMTAActor.php b/src/applications/metamta/query/PhabricatorMetaMTAActor.php new file mode 100644 index 0000000000..8580a7bc3b --- /dev/null +++ b/src/applications/metamta/query/PhabricatorMetaMTAActor.php @@ -0,0 +1,50 @@ +name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setEmailAddress($email_address) { + $this->emailAddress = $email_address; + return $this; + } + + public function getEmailAddress() { + return $this->emailAddress; + } + + public function setPHID($phid) { + $this->phid = $phid; + return $this; + } + + public function getPHID() { + return $this->phid; + } + + public function setUndeliverable($reason) { + $this->reasons[] = $reason; + return $this; + } + + public function isDeliverable() { + return empty($this->reasons); + } + + public function getUndeliverableReasons() { + return $this->reasons; + } + +} diff --git a/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php new file mode 100644 index 0000000000..6df46bb7a6 --- /dev/null +++ b/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php @@ -0,0 +1,181 @@ +viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + public function execute() { + $phids = array_fuse($this->phids); + $actors = array(); + $type_map = array(); + foreach ($phids as $phid) { + $type_map[phid_get_type($phid)][] = $phid; + $actors[$phid] = id(new PhabricatorMetaMTAActor())->setPHID($phid); + } + + foreach ($type_map as $type => $phids) { + switch ($type) { + case PhabricatorPHIDConstants::PHID_TYPE_USER: + $this->loadUserActors($actors, $phids); + break; + case PhabricatorPHIDConstants::PHID_TYPE_XUSR: + $this->loadExternalUserActors($actors, $phids); + break; + case PhabricatorPHIDConstants::PHID_TYPE_MLST: + $this->loadMailingListActors($actors, $phids); + break; + default: + $this->loadUnknownActors($actors, $phids); + break; + } + } + + return $actors; + } + + private function loadUserActors(array $actors, array $phids) { + assert_instances_of($actors, 'PhabricatorMetaMTAActor'); + + $emails = id(new PhabricatorUserEmail())->loadAllWhere( + 'userPHID IN (%Ls) AND isPrimary = 1', + $phids); + $emails = mpull($emails, null, 'getUserPHID'); + + $users = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($phids) + ->execute(); + $users = mpull($users, null, 'getPHID'); + + foreach ($phids as $phid) { + $actor = $actors[$phid]; + + $user = idx($users, $phid); + if (!$user) { + $actor->setUndeliverable( + pht('Unable to load user record for this PHID.')); + } else { + $actor->setName($this->getUserName($user)); + if ($user->getIsDisabled()) { + $actor->setUndeliverable( + pht('This user is disabled; disabled users do not receive mail.')); + } + if ($user->getIsSystemAgent()) { + $actor->setUndeliverable( + pht('This user is a bot; bot accounts do not receive mail.')); + } + } + + $email = idx($emails, $phid); + if (!$email) { + $actor->setUndeliverable( + pht('Unable to load email record for this PHID.')); + } else { + $actor->setEmailAddress($email->getAddress()); + } + } + } + + private function loadExternalUserActors(array $actors, array $phids) { + assert_instances_of($actors, 'PhabricatorMetaMTAActor'); + + $xusers = id(new PhabricatorExternalAccountQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($phids) + ->execute(); + $xusers = mpull($xusers, null, 'getPHID'); + + foreach ($phids as $phid) { + $actor = $actors[$phid]; + + $xuser = idx($xusers, $phid); + if (!$xuser) { + $actor->setUndeliverable( + pht('Unable to load external user record for this PHID.')); + continue; + } + + $actor->setName($xuser->getDisplayName()); + + if ($xuser->getAccountType() != 'email') { + $actor->setUndeliverable( + pht( + 'Only external accounts of type "email" are deliverable; this '. + 'account has a different type.')); + continue; + } + + $actor->setEmailAddress($xuser->getAccountID()); + } + } + + private function loadMailingListActors(array $actors, array $phids) { + assert_instances_of($actors, 'PhabricatorMetaMTAActor'); + + $lists = id(new PhabricatorMetaMTAMailingList())->loadAllWhere( + 'phid IN (%Ls)', + $phids); + + foreach ($phids as $phid) { + $actor = $actors[$phid]; + + $list = idx($lists, $phid); + if (!$list) { + $actor->setUndeliverable( + pht( + 'Unable to load mailing list record for this PHID.')); + continue; + } + + $actor->setName($list->getName()); + $actor->setEmailAddress($list->getEmail()); + } + } + + private function loadUnknownActors(array $actors, array $phids) { + foreach ($phids as $phid) { + $actor = $actors[$phid]; + $actor->setUndeliverable(pht('This PHID type is not mailable.')); + } + } + + + /** + * Small helper function to make sure we format the username properly as + * specified by the `metamta.user-address-format` configuration value. + */ + private function getUserName(PhabricatorUser $user) { + $format = PhabricatorEnv::getEnvConfig('metamta.user-address-format'); + + switch ($format) { + case 'short': + $name = $user->getUserName(); + break; + case 'real': + $name = $user->getRealName(); + break; + case 'full': + default: + $name = $user->getFullName(); + break; + } + + return $name; + } + +} diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index 1a606a09a1..baa2445546 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -66,6 +66,10 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return $this; } + public function getMailTags() { + return $this->getParam('mailtags', array()); + } + /** * In Gmail, conversations will be broken if you reply to a thread and the * server sends back a response without referencing your Message-ID, even if @@ -242,6 +246,18 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return $this; } + public function getToPHIDs() { + return $this->getParam('to', array()); + } + + public function getRawToAddresses() { + return $this->getParam('raw-to', array()); + } + + public function getCcPHIDs() { + return $this->getParam('cc', array()); + } + /** * 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 @@ -343,52 +359,13 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { try { $params = $this->parameters; - $phids = array(); - foreach ($params as $key => $value) { - switch ($key) { - case 'to': - $params[$key] = $this->buildToList(); - break; - case 'cc': - $params[$key] = $this->buildCCList(); - break; - } - } + $actors = $this->loadAllActors(); + $deliverable_actors = $this->filterDeliverableActors($actors); - foreach ($params as $key => $value) { - switch ($key) { - case 'from': - $value = array($value); - /* fallthrough */ - case 'to': - case 'cc': - foreach ($value as $phid) { - $type = phid_get_type($phid); - $phids[$phid] = $type; - } - break; - } - } - - $this->loadEmailAndNameDataFromPHIDs($phids); - - $default = PhabricatorEnv::getEnvConfig('metamta.default-address'); + $default_from = PhabricatorEnv::getEnvConfig('metamta.default-address'); if (empty($params['from'])) { - $mailer->setFrom($default); - } else { - $from = $params['from']; - - if (!PhabricatorEnv::getEnvConfig('metamta.can-send-as-user')) { - if (empty($params['reply-to'])) { - $params['reply-to'] = $phids[$from]['email']; - $params['reply-to-name'] = $phids[$from]['name']; - } - $mailer->setFrom( - $default, - $phids[$from]['name']); - unset($params['from']); - } + $mailer->setFrom($default_from); } $is_first = idx($params, 'is-first-message'); @@ -405,25 +382,45 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { foreach ($params as $key => $value) { switch ($key) { case 'from': - $mailer->setFrom($phids[$from]['email']); + $from = $value; + $actor = $actors[$from]; + + $actor_email = $actor->getEmailAddress(); + $actor_name = $actor->getName(); + $can_send_as_user = $actor_email && + PhabricatorEnv::getEnvConfig('metamta.can-send-as-user'); + + if ($can_send_as_user) { + $mailer->setFrom($actor_email); + } else { + $from_email = coalesce($actor_email, $default_from); + $from_name = coalesce($actor_name, pht('Phabricator')); + + if (empty($params['reply-to'])) { + $params['reply-to'] = $from_email; + $params['reply-to-name'] = $from_name; + } + + $mailer->setFrom($default_from, $from_name); + } break; case 'reply-to': $mailer->addReplyTo($value, $reply_to_name); break; case 'to': - $to_emails = $this->filterSendable($value, $phids); - if ($to_emails) { - $add_to = array_merge($add_to, $to_emails); - } + $to_actors = array_select_keys($deliverable_actors, $value); + $add_to = array_merge( + $add_to, + mpull($to_actors, 'getEmailAddress')); break; case 'raw-to': $add_to = array_merge($add_to, $value); break; case 'cc': - $cc_emails = $this->filterSendable($value, $phids); - if ($cc_emails) { - $add_cc = $cc_emails; - } + $cc_actors = array_select_keys($deliverable_actors, $value); + $add_cc = array_merge( + $add_cc, + mpull($cc_actors, 'getEmailAddress')); break; case 'headers': foreach ($value as $pair) { @@ -573,8 +570,8 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { if (!$add_to && !$add_cc) { $this->setStatus(self::STATUS_VOID); $this->setMessage( - "Message has no valid recipients: all To/CC are disabled or ". - "configured not to receive this mail."); + "Message has no valid recipients: all To/Cc are disabled, invalid, ". + "or configured not to receive this mail."); return $this->save(); } @@ -610,6 +607,9 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { } } + $add_to = array_unique($add_to); + $add_cc = array_unique($add_cc); + $mailer->addTos($add_to); if ($add_cc) { $mailer->addCCs($add_cc); @@ -691,126 +691,6 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return base64_encode($base); } - private function loadEmailAndNameDataFromPHIDs(array &$phids) { - $users = array(); - $xusrs = array(); - $mlsts = array(); - // first iteration - group by types to do data fetches - foreach ($phids as $phid => $type) { - switch ($type) { - case PhabricatorPHIDConstants::PHID_TYPE_USER: - $users[] = $phid; - break; - case PhabricatorPHIDConstants::PHID_TYPE_XUSR: - $xusrs[] = $phid; - break; - case PhabricatorPHIDConstants::PHID_TYPE_MLST: - $mlsts[] = $phid; - break; - } - } - - $user_emails = array(); - if ($users) { - $user_emails = id(new PhabricatorUserEmail())->loadAllWhere( - 'userPHID IN (%Ls) AND isPrimary = 1', $users); - $users = id(new PhabricatorUser())->loadAllWhere( - 'phid IN (%Ls)', $users); - $user_emails = mpull($user_emails, null, 'getUserPHID'); - $users = mpull($users, null, 'getPHID'); - } - - if ($xusrs) { - $xusrs = id(new PhabricatorExternalAccount())->loadAllWhere( - 'phid IN (%Ls)', $xusrs); - $xusrs = mpull($xusrs, null, 'getPHID'); - } - - if ($mlsts) { - $mlsts = id(new PhabricatorMetaMTAMailingList())->loadAllWhere( - 'phid IN (%Ls)', $mlsts); - $mlsts = mpull($mlsts, null, 'getPHID'); - } - - // second iteration - create entries for each phid - $default = PhabricatorEnv::getEnvConfig('metamta.default-address'); - foreach ($phids as $phid => &$value) { - $name = ''; - $email = $default; - $is_mailable = false; - switch ($value) { - case PhabricatorPHIDConstants::PHID_TYPE_USER: - $user = idx($users, $phid); - if ($user) { - $name = $this->getUserName($user); - $is_mailable = !$user->getIsDisabled() - && !$user->getIsSystemAgent(); - } - $email = isset($user_emails[$phid]) ? - $user_emails[$phid]->getAddress() : - $default; - break; - case PhabricatorPHIDConstants::PHID_TYPE_XUSR: - $xusr = $xusrs[$phid]; - if ($xusr) { - $name = $xusr->getDisplayName(); - $email = $xusr->getAccountID(); - $accountType = $xusr->getAccountType(); - if ($accountType == 'email') { - $is_mailable = true; - } - } - break; - case PhabricatorPHIDConstants::PHID_TYPE_MLST: - $mlst = $mlsts[$phid]; - if ($mlst) { - $name = $mlst->getName(); - $email = $mlst->getEmail(); - $is_mailable = true; - } - break; - } - $value = array( - 'name' => $name, - 'email' => $email, - 'mailable' => $is_mailable, - ); - } - } - - /** - * Small helper function to make sure we format the username properly as - * specified by the `metamta.user-address-format` configuration value. - */ - private function getUserName($user) { - $format = PhabricatorEnv::getEnvConfig('metamta.user-address-format'); - - switch ($format) { - case 'short': - $name = $user->getUserName(); - break; - case 'real': - $name = $user->getRealName(); - break; - case 'full': - default: - $name = $user->getFullName(); - break; - } - - return $name; - } - - private function filterSendable($value, $phids) { - $result = array(); - foreach ($value as $phid) { - if (isset($phids[$phid]) && $phids[$phid]['mailable']) { - $result[$phid] = $phids[$phid]['email']; - } - } - return $result; - } - public static function shouldMultiplexAllMail() { return PhabricatorEnv::getEnvConfig('metamta.one-mail-per-recipient'); } @@ -821,60 +701,90 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { /** * Get all of the recipients for this mail, after preference filters are - * applied. This list has all objects to whom delivery will be attempted, but - * does not exclude recipeints two whom delivery may be impossible. + * applied. This list has all objects to whom delivery will be attempted. * * @return list A list of all recipients to whom delivery will be * attempted. * @task recipients */ public function buildRecipientList() { - return $this->resolveRecipients( + $actors = $this->loadActors( array_merge( - $this->getRawToPHIDs(), - $this->getRawCCPHIDs())); + $this->getToPHIDs(), + $this->getCcPHIDs())); + $actors = $this->filterDeliverableActors($actors); + return mpull($actors, 'getPHID'); } + public function loadAllActors() { + $actor_phids = array_merge( + array($this->getParam('from')), + $this->getToPHIDs(), + $this->getCcPHIDs()); - /** - * Filter out duplicate, invalid, or excluded recipients from a PHID list. - * - * @param list Unfiltered recipients. - * @return list Filtered recipients. - * - * @task recipients - */ - private function resolveRecipients(array $phids) { - if (!$phids) { + return $this->loadActors($actor_phids); + } + + private function filterDeliverableActors(array $actors) { + assert_instances_of($actors, 'PhabricatorMetaMTAActor'); + $deliverable_actors = array(); + foreach ($actors as $phid => $actor) { + if ($actor->isDeliverable()) { + $deliverable_actors[$phid] = $actor; + } + } + return $deliverable_actors; + } + + private function loadActors(array $actor_phids) { + $actor_phids = array_filter($actor_phids); + $viewer = PhabricatorUser::getOmnipotentUser(); + + $actors = id(new PhabricatorMetaMTAActorQuery()) + ->setViewer($viewer) + ->withPHIDs($actor_phids) + ->execute(); + + if (!$actors) { return array(); } - $phids = array_fuse($phids); + // Exclude explicit recipients. + foreach ($this->getExcludeMailRecipientPHIDs() as $phid) { + $actor = idx($actors, $phid); + if (!$actor) { + continue; + } + $actor->setUndeliverable( + pht( + 'This message is a response to another email message, and this '. + 'recipient received the original email message, so we are not '. + 'sending them this substantially similar message (for example, '. + 'the sender used "Reply All" instead of "Reply" in response to '. + 'mail from Phabricator).')); + } - - // Exclude PHIDs explicitly marked for exclusion. We use this to prevent - // recipients of an accidental "Reply All" from receiving the followup - // mail from Phabricator. - $exclude = $this->getExcludeMailRecipientPHIDs(); - $exclude = array_fill_keys($exclude, true); - $phids = array_diff_key($phids, $exclude); - - - // If the actor is a recipient and has configured their preferences not to - // send them mail about their own actions, drop them from the recipient - // list. - $from = $this->getParam('from'); - if (isset($phids[$from])) { - $from_user = id(new PhabricatorUser())->loadOneWhere( - 'phid = %s', - $from); + // Exclude the actor if their preferences are set. + $from_phid = $this->getParam('from'); + $from_actor = idx($actors, $from_phid); + if ($from_actor) { + $from_user = id(new PhabricatorPeopleQuery()) + ->setViewer($viewer) + ->withPHIDs(array($from_phid)) + ->execute(); + $from_user = head($from_user); if ($from_user && !$this->getOverrideNoSelfMailPreference()) { $pref_key = PhabricatorUserPreferences::PREFERENCE_NO_SELF_MAIL; $exclude_self = $from_user ->loadPreferences() ->getPreference($pref_key); if ($exclude_self) { - unset($phids[$from]); + $from_actor->setUndeliverable( + pht( + 'This recipient is the user whose actions caused delivery of '. + 'this message, but they have set preferences so they do not '. + 'receive mail about their own actions (Settings > Email '. + 'Preferences > Self Actions).')); } } } @@ -884,18 +794,13 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { // of email (for example, a user who says they don't want emails about task // CC changes). $tags = $this->getParam('mailtags'); - if ($tags && $phids) { + if ($tags) { $all_prefs = id(new PhabricatorUserPreferences())->loadAllWhere( 'userPHID in (%Ls)', - $phids); + $actor_phids); $all_prefs = mpull($all_prefs, null, 'getUserPHID'); - foreach ($phids as $phid) { - $prefs = idx($all_prefs, $phid); - if (!$prefs) { - continue; - } - + foreach ($all_prefs as $phid => $prefs) { $user_mailtags = $prefs->getPreference( PhabricatorUserPreferences::PREFERENCE_MAILTAGS, array()); @@ -911,56 +816,17 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { } if (!$send) { - unset($phids[$phid]); + $actors[$phid]->setUndeliverable( + pht( + 'This mail has tags which control which users receive it, and '. + 'this recipient has not elected to receive mail with any of '. + 'the tags on this message (Settings > Email Preferences).')); } } } - return array_keys($phids); + return $actors; } - /** - * @task recipients - */ - private function buildToList() { - return $this->resolveRecipients($this->getRawToPHIDs()); - } - - - /** - * @task recipients - */ - private function buildCCList() { - return $this->resolveRecipients($this->getRawCCPHIDs()); - } - - - /** - * @task recipients - */ - private function getRawToPHIDs() { - $to = $this->getParam('to', array()); - return $this->filterRawPHIDList($to); - } - - - /** - * @task recipients - */ - private function getRawCCPHIDs() { - $cc = $this->getParam('cc', array()); - return $this->filterRawPHIDList($cc); - } - - - /** - * @task recipients - */ - private function filterRawPHIDList(array $list) { - $list = array_filter($list); - $list = array_unique($list); - return array_values($list); - } - }