diff --git a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php index 5c67c3cf3f..ae845745ac 100644 --- a/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/mail/PhabricatorMetaMTAMail.php @@ -301,14 +301,14 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { $value = array($value); } foreach (array_filter($value) as $phid) { - $phids[] = $phid; + $type = phid_get_type($phid); + $phids[$phid] = $type; } break; } } - $handles = id(new PhabricatorObjectHandleData($phids)) - ->loadHandles(); + $this->loadEmailAndNameDataFromPHIDs($phids); $exclude = array(); @@ -335,14 +335,13 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { } if (!PhabricatorEnv::getEnvConfig('metamta.can-send-as-user')) { - $handle = $handles[$from]; if (empty($params['reply-to'])) { - $params['reply-to'] = $handle->getEmail(); - $params['reply-to-name'] = $handle->getFullName(); + $params['reply-to'] = $phids[$from]['email']; + $params['reply-to-name'] = $phids[$from]['name']; } $mailer->setFrom( $default, - $handle->getFullName()); + $phids[$from]['name']); unset($params['from']); } } @@ -361,30 +360,24 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { foreach ($params as $key => $value) { switch ($key) { case 'from': - $mailer->setFrom($handles[$value]->getEmail()); + $mailer->setFrom($phids[$from]['email']); break; case 'reply-to': $mailer->addReplyTo($value, $reply_to_name); break; case 'to': - $emails = $this->getDeliverableEmailsFromHandles( - $value, - $handles, - $exclude); - if ($emails) { - $add_to = array_merge($add_to, $emails); + $to_emails = $this->filterSendable($value, $phids, $exclude); + if ($to_emails) { + $add_to = array_merge($add_to, $to_emails); } break; case 'raw-to': $add_to = array_merge($add_to, $value); break; case 'cc': - $emails = $this->getDeliverableEmailsFromHandles( - $value, - $handles, - $exclude); - if ($emails) { - $add_cc = $emails; + $cc_emails = $this->filterSendable($value, $phids, $exclude); + if ($cc_emails) { + $add_cc = $cc_emails; } break; case 'headers': @@ -659,27 +652,81 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO { return base64_encode($base); } - private function getDeliverableEmailsFromHandles( - array $phids, - array $handles, - array $exclude) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); + private function loadEmailAndNameDataFromPHIDs(array &$phids) { + $users = 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_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 ($mlsts) { + $mlsts = id(new PhabricatorMetaMTAMailingList())->loadAllWhere( + 'phid IN (%Ls)', $mlsts); + $mlsts = mpull($mlsts, null, 'getPHID'); + } - $emails = array(); - foreach ($phids as $phid) { - if ($handles[$phid]->isDisabled()) { - continue; - } - if (!$handles[$phid]->isComplete()) { - continue; + // 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 = $users[$phid]; + if ($user) { + $name = $user->getFullName(); + $is_mailable = !$user->getIsDisabled() + && !$user->getIsSystemAgent(); + } + $email = $user_emails[$phid] ? + $user_emails[$phid]->getAddress() : + $default; + 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, + ); + } + } + + private function filterSendable($value, $phids, $exclude) { + $result = array(); + foreach ($value as $phid) { if (isset($exclude[$phid])) { continue; } - $emails[$phid] = $handles[$phid]->getEmail(); + if (isset($phids[$phid]) && $phids[$phid]['mailable']) { + $result[$phid] = $phids[$phid]['email']; + } } - - return $emails; + return $result; } public static function shouldMultiplexAllMail() { diff --git a/src/applications/metamta/storage/mail/__init__.php b/src/applications/metamta/storage/mail/__init__.php index 7df205fc8c..2470c3d0ac 100644 --- a/src/applications/metamta/storage/mail/__init__.php +++ b/src/applications/metamta/storage/mail/__init__.php @@ -7,9 +7,12 @@ phutil_require_module('phabricator', 'applications/metamta/storage/base'); +phutil_require_module('phabricator', 'applications/metamta/storage/mailinglist'); +phutil_require_module('phabricator', 'applications/people/storage/email'); 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', 'applications/phid/constants'); +phutil_require_module('phabricator', 'applications/phid/utils'); phutil_require_module('phabricator', 'infrastructure/daemon/workers/storage/task'); phutil_require_module('phabricator', 'infrastructure/env'); diff --git a/src/applications/people/storage/user/PhabricatorUser.php b/src/applications/people/storage/user/PhabricatorUser.php index 0ce125d13e..639b984a33 100644 --- a/src/applications/people/storage/user/PhabricatorUser.php +++ b/src/applications/people/storage/user/PhabricatorUser.php @@ -581,6 +581,10 @@ EOBODY; return self::getDefaultProfileImageURI(); } + public function getFullName() { + return $this->getUsername().' ('.$this->getRealName().')'; + } + public static function loadOneWithEmailAddress($address) { $email = id(new PhabricatorUserEmail())->loadOneWhere( 'address = %s', diff --git a/src/applications/phid/handle/PhabricatorObjectHandle.php b/src/applications/phid/handle/PhabricatorObjectHandle.php index 56fa21e2d4..fb28c92bc9 100644 --- a/src/applications/phid/handle/PhabricatorObjectHandle.php +++ b/src/applications/phid/handle/PhabricatorObjectHandle.php @@ -88,15 +88,6 @@ final class PhabricatorObjectHandle { return $this->type; } - public function setEmail($email) { - $this->email = $email; - return $this; - } - - public function getEmail() { - return $this->email; - } - public function setImageURI($uri) { $this->imageURI = $uri; return $this; diff --git a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php index 8fa322a56d..ae87809767 100644 --- a/src/applications/phid/handle/data/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/data/PhabricatorObjectHandleData.php @@ -149,16 +149,6 @@ final class PhabricatorObjectHandleData { $images = mpull($images, 'getBestURI', 'getPHID'); } - // TODO: This probably should not be part of Handles anymore, only - // MetaMTA actually uses it. - $emails = id(new PhabricatorUserEmail())->loadAllWhere( - 'userPHID IN (%Ls) AND isPrimary = 1', - $phids); - $emails = mpull($emails, 'getAddress', 'getUserPHID'); - - $statuses = id(new PhabricatorUserStatus())->loadCurrentStatuses( - $phids); - foreach ($phids as $phid) { $handle = new PhabricatorObjectHandle(); $handle->setPHID($phid); @@ -169,7 +159,6 @@ final class PhabricatorObjectHandleData { $user = $users[$phid]; $handle->setName($user->getUsername()); $handle->setURI('/p/'.$user->getUsername().'/'); - $handle->setEmail(idx($emails, $phid)); $handle->setFullName( $user->getUsername().' ('.$user->getRealName().')'); $handle->setAlternateID($user->getID()); @@ -177,8 +166,7 @@ final class PhabricatorObjectHandleData { if (isset($statuses[$phid])) { $handle->setStatus($statuses[$phid]->getTextStatus()); } - $handle->setDisabled($user->getIsDisabled() || - $user->getIsSystemAgent()); + $handle->setDisabled($user->getIsDisabled()); $img_uri = idx($images, $user->getProfileImagePHID()); if ($img_uri) { @@ -208,7 +196,6 @@ final class PhabricatorObjectHandleData { $handle->setName('Unknown Mailing List'); } else { $list = $lists[$phid]; - $handle->setEmail($list->getEmail()); $handle->setName($list->getName()); $handle->setURI($list->getURI()); $handle->setFullName($list->getName()); diff --git a/src/applications/phid/handle/data/__init__.php b/src/applications/phid/handle/data/__init__.php index d2622bba45..309d3dc001 100644 --- a/src/applications/phid/handle/data/__init__.php +++ b/src/applications/phid/handle/data/__init__.php @@ -11,7 +11,6 @@ phutil_require_module('arcanist', 'differential/constants/revisionstatus'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/maniphest/constants/owner'); phutil_require_module('phabricator', 'applications/maniphest/constants/status'); -phutil_require_module('phabricator', 'applications/people/storage/email'); phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'applications/people/storage/userstatus'); phutil_require_module('phabricator', 'applications/phid/constants');