Page MenuHomePhabricator

D8618.id20451.diff
No OneTemporary

D8618.id20451.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1971,6 +1971,8 @@
'PhabricatorRepositoryPushLog' => 'applications/repository/storage/PhabricatorRepositoryPushLog.php',
'PhabricatorRepositoryPushLogQuery' => 'applications/repository/query/PhabricatorRepositoryPushLogQuery.php',
'PhabricatorRepositoryPushLogSearchEngine' => 'applications/repository/query/PhabricatorRepositoryPushLogSearchEngine.php',
+ 'PhabricatorRepositoryPushMailWorker' => 'applications/repository/worker/PhabricatorRepositoryPushMailWorker.php',
+ 'PhabricatorRepositoryPushReplyHandler' => 'applications/repository/mail/PhabricatorRepositoryPushReplyHandler.php',
'PhabricatorRepositoryQuery' => 'applications/repository/query/PhabricatorRepositoryQuery.php',
'PhabricatorRepositoryRefCursor' => 'applications/repository/storage/PhabricatorRepositoryRefCursor.php',
'PhabricatorRepositoryRefCursorQuery' => 'applications/repository/query/PhabricatorRepositoryRefCursorQuery.php',
@@ -4818,6 +4820,8 @@
),
'PhabricatorRepositoryPushLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorRepositoryPushLogSearchEngine' => 'PhabricatorApplicationSearchEngine',
+ 'PhabricatorRepositoryPushMailWorker' => 'PhabricatorWorker',
+ 'PhabricatorRepositoryPushReplyHandler' => 'PhabricatorMailReplyHandler',
'PhabricatorRepositoryQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorRepositoryRefCursor' =>
array(
diff --git a/src/applications/diffusion/data/DiffusionCommitRef.php b/src/applications/diffusion/data/DiffusionCommitRef.php
--- a/src/applications/diffusion/data/DiffusionCommitRef.php
+++ b/src/applications/diffusion/data/DiffusionCommitRef.php
@@ -74,6 +74,11 @@
return $this->formatUser($this->committerName, $this->committerEmail);
}
+ public function getSummary() {
+ return PhabricatorRepositoryCommitData::summarizeCommitMessage(
+ $this->getMessage());
+ }
+
private function formatUser($name, $email) {
if (strlen($name) && strlen($email)) {
return "{$name} <{$email}>";
diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -32,6 +32,7 @@
private $heraldViewerProjects;
private $rejectCode = PhabricatorRepositoryPushLog::REJECT_BROKEN;
private $rejectDetails;
+ private $emailPHIDs = array();
/* -( Config )------------------------------------------------------------- */
@@ -172,6 +173,23 @@
throw $caught;
}
+ if ($this->emailPHIDs) {
+ // If Herald rules triggered email to users, queue a worker to send the
+ // mail. We do this out-of-process so that we block pushes as briefly
+ // as possible.
+
+ // (We do need to pull some commit info here because the commit objects
+ // may not exist yet when this worker runs, which could be immediately.)
+
+ PhabricatorWorker::scheduleTask(
+ 'PhabricatorRepositoryPushMailWorker',
+ array(
+ 'eventPHID' => $event->getPHID(),
+ 'emailPHIDs' => array_values($this->emailPHIDs),
+ 'info' => $this->loadCommitInfoForWorker($all_updates),
+ ));
+ }
+
return 0;
}
@@ -282,6 +300,11 @@
$engine->applyEffects($effects, $adapter, $rules);
$xscript = $engine->getTranscript();
+ // Store any PHIDs we want to send email to for later.
+ foreach ($adapter->getEmailPHIDs() as $email_phid) {
+ $this->emailPHIDs[$email_phid] = $email_phid;
+ }
+
if ($blocking_effect === null) {
foreach ($effects as $effect) {
if ($effect->getAction() == HeraldAdapter::ACTION_BLOCK) {
@@ -982,6 +1005,7 @@
return PhabricatorRepositoryPushLog::initializeNewLog($this->getViewer())
->setPHID($phid)
->setRepositoryPHID($this->getRepository()->getPHID())
+ ->attachRepository($this->getRepository())
->setEpoch(time());
}
@@ -1091,5 +1115,26 @@
}
}
+ private function loadCommitInfoForWorker(array $all_updates) {
+ $type_commit = PhabricatorRepositoryPushLog::REFTYPE_COMMIT;
+
+ $map = array();
+ foreach ($all_updates as $update) {
+ if ($update->getRefType() != $type_commit) {
+ continue;
+ }
+ $map[$update->getRefNew()] = array();
+ }
+
+ foreach ($map as $identifier => $info) {
+ $ref = $this->loadCommitRefForCommit($identifier);
+ $map[$identifier] += array(
+ 'summary' => $ref->getSummary(),
+ 'branches' => $this->loadBranches($identifier),
+ );
+ }
+
+ return $map;
+ }
}
diff --git a/src/applications/diffusion/herald/HeraldPreCommitAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitAdapter.php
--- a/src/applications/diffusion/herald/HeraldPreCommitAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitAdapter.php
@@ -4,6 +4,7 @@
private $log;
private $hookEngine;
+ private $emailPHIDs = array();
public function setPushLog(PhabricatorRepositoryPushLog $log) {
$this->log = $log;
@@ -27,12 +28,16 @@
return $this->log;
}
+ public function getEmailPHIDs() {
+ return array_values($this->emailPHIDs);
+ }
+
public function supportsRuleType($rule_type) {
switch ($rule_type) {
case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
- return true;
case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
+ return true;
default:
return false;
}
@@ -70,10 +75,12 @@
case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
return array(
self::ACTION_BLOCK,
+ self::ACTION_EMAIL,
self::ACTION_NOTHING
);
case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
return array(
+ self::ACTION_EMAIL,
self::ACTION_NOTHING,
);
}
@@ -96,6 +103,15 @@
true,
pht('Did nothing.'));
break;
+ case self::ACTION_EMAIL:
+ foreach ($effect->getTarget() as $phid) {
+ $this->emailPHIDs[$phid] = $phid;
+ }
+ $result[] = new HeraldApplyTranscript(
+ $effect,
+ true,
+ pht('Added mailable to mail targets.'));
+ break;
case self::ACTION_BLOCK:
$result[] = new HeraldApplyTranscript(
$effect,
diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
--- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -18,7 +18,7 @@
public function getAdapterContentDescription() {
return pht(
"React to commits being pushed to hosted repositories.\n".
- "Hook rules can block changes.");
+ "Hook rules can block changes and send push summary mail.");
}
public function getFields() {
diff --git a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php
--- a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php
@@ -20,7 +20,7 @@
public function getAdapterContentDescription() {
return pht(
"React to branches and tags being pushed to hosted repositories.\n".
- "Hook rules can block changes.");
+ "Hook rules can block changes and send push summary mail.");
}
public function getFieldNameMap() {
diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php
--- a/src/applications/herald/storage/HeraldRule.php
+++ b/src/applications/herald/storage/HeraldRule.php
@@ -17,7 +17,7 @@
protected $isDisabled = 0;
protected $triggerObjectPHID;
- protected $configVersion = 34;
+ protected $configVersion = 35;
// phids for which this rule has been applied
private $ruleApplied = self::ATTACHABLE;
diff --git a/src/applications/repository/mail/PhabricatorRepositoryPushReplyHandler.php b/src/applications/repository/mail/PhabricatorRepositoryPushReplyHandler.php
new file mode 100644
--- /dev/null
+++ b/src/applications/repository/mail/PhabricatorRepositoryPushReplyHandler.php
@@ -0,0 +1,27 @@
+<?php
+
+final class PhabricatorRepositoryPushReplyHandler
+ extends PhabricatorMailReplyHandler {
+
+ public function validateMailReceiver($mail_receiver) {
+ return;
+ }
+
+ public function getPrivateReplyHandlerEmailAddress(
+ PhabricatorObjectHandle $handle) {
+ return null;
+ }
+
+ public function getReplyHandlerDomain() {
+ return null;
+ }
+
+ public function getReplyHandlerInstructions() {
+ return null;
+ }
+
+ protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
+ return;
+ }
+
+}
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php b/src/applications/repository/storage/PhabricatorRepositoryCommitData.php
--- a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommitData.php
@@ -24,7 +24,10 @@
public function getSummary() {
$message = $this->getCommitMessage();
+ return self::summarizeCommitMessage($message);
+ }
+ public static function summarizeCommitMessage($message) {
$summary = phutil_split_lines($message, $retain_endings = false);
$summary = head($summary);
$summary = phutil_utf8_shorten($summary, self::SUMMARY_MAX_LENGTH);
diff --git a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php
--- a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php
@@ -45,6 +45,7 @@
private $dangerousChangeDescription = self::ATTACHABLE;
private $pushEvent = self::ATTACHABLE;
+ private $repository = self::ATTACHABLE;
public static function initializeNewLog(PhabricatorUser $viewer) {
return id(new PhabricatorRepositoryPushLog())
@@ -66,10 +67,6 @@
PhabricatorRepositoryPHIDTypePushLog::TYPECONST);
}
- public function getRepository() {
- return $this->getPushEvent()->getRepository();
- }
-
public function attachPushEvent(PhabricatorRepositoryPushEvent $push_event) {
$this->pushEvent = $push_event;
return $this;
@@ -120,6 +117,21 @@
return $this->assertAttached($this->dangerousChangeDescription);
}
+ public function attachRepository(PhabricatorRepository $repository) {
+ // NOTE: Some gymnastics around this because of object construction order
+ // in the hook engine. Particularly, web build the logs before we build
+ // their push event.
+ $this->repository = $repository;
+ return $this;
+ }
+
+ public function getRepository() {
+ if ($this->repository == self::ATTACHABLE) {
+ return $this->getPushEvent()->getRepository();
+ }
+ return $this->assertAttached($this->repository);
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
@@ -131,11 +143,14 @@
}
public function getPolicy($capability) {
- return $this->getPushEvent()->getPolicy($capability);
+ // NOTE: We're passing through the repository rather than the push event
+ // mostly because we need to do policy checks in Herald before we create
+ // the event. The two approaches are equivalent in practice.
+ return $this->getRepository()->getPolicy($capability);
}
public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
- return $this->getPushEvent()->hasAutomaticCapability($capability, $viewer);
+ return $this->getRepository()->hasAutomaticCapability($capability, $viewer);
}
public function describeAutomaticCapability($capability) {
diff --git a/src/applications/repository/worker/PhabricatorRepositoryPushMailWorker.php b/src/applications/repository/worker/PhabricatorRepositoryPushMailWorker.php
new file mode 100644
--- /dev/null
+++ b/src/applications/repository/worker/PhabricatorRepositoryPushMailWorker.php
@@ -0,0 +1,225 @@
+<?php
+
+final class PhabricatorRepositoryPushMailWorker
+ extends PhabricatorWorker {
+
+ public function doWork() {
+ $viewer = PhabricatorUser::getOmnipotentUser();
+
+ $task_data = $this->getTaskData();
+
+ $email_phids = idx($task_data, 'emailPHIDs');
+ if (!$email_phids) {
+ // If we don't have any email targets, don't send any email.
+ return;
+ }
+
+ $event_phid = idx($task_data, 'eventPHID');
+ $event = id(new PhabricatorRepositoryPushEventQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($event_phid))
+ ->needLogs(true)
+ ->executeOne();
+
+ $repository = $event->getRepository();
+ if ($repository->isImporting()) {
+ // If the repository is still importing, don't send email.
+ return;
+ }
+
+ if ($repository->getDetail('herald-disabled')) {
+ // If publishing is disabled, don't send email.
+ return;
+ }
+
+ $logs = $event->getLogs();
+
+ list($ref_lines, $ref_list) = $this->renderRefs($logs);
+ list($commit_lines, $subject_line) = $this->renderCommits(
+ $repository,
+ $logs,
+ idx($task_data, 'info', array()));
+
+ $ref_count = count($ref_lines);
+ $commit_count = count($commit_lines);
+
+ $handles = id(new PhabricatorHandleQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($event->getPusherPHID()))
+ ->execute();
+
+ $pusher_name = $handles[$event->getPusherPHID()]->getName();
+ $repo_name = $repository->getMonogram();
+
+ if ($commit_count) {
+ $overview = pht(
+ '%s pushed %d commit(s) to %s.',
+ $pusher_name,
+ $commit_count,
+ $repo_name);
+ } else {
+ $overview = pht(
+ '%s pushed to %s.',
+ $pusher_name,
+ $repo_name);
+ }
+
+ $details_uri = PhabricatorEnv::getProductionURI(
+ '/diffusion/pushlog/view/'.$event->getID().'/');
+
+ $body = new PhabricatorMetaMTAMailBody();
+ $body->addRawSection($overview);
+
+ $body->addTextSection(pht('DETAILS'), $details_uri);
+
+ if ($commit_lines) {
+ $body->addTextSection(pht('COMMITS'), implode("\n", $commit_lines));
+ }
+
+ if ($ref_lines) {
+ $body->addTextSection(pht('REFERENCES'), implode("\n", $ref_lines));
+ }
+
+ $prefix = PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix');
+
+ $parts = array();
+ if ($commit_count) {
+ $parts[] = pht('%s commit(s)', $commit_count);
+ }
+ if ($ref_count) {
+ $parts[] = implode(', ', $ref_list);
+ }
+ $parts = implode(', ', $parts);
+
+ if ($subject_line) {
+ $subject = pht('(%s) %s', $parts, $subject_line);
+ } else {
+ $subject = pht('(%s)', $parts);
+ }
+
+ $mail = id(new PhabricatorMetaMTAMail())
+ ->setRelatedPHID($event->getPHID())
+ ->setSubjectPrefix($prefix)
+ ->setVarySubjectPrefix(pht('[Push]'))
+ ->setSubject($subject)
+ ->setFrom($event->getPusherPHID())
+ ->setBody($body->render())
+ ->setThreadID($event->getPHID(), $is_new = true)
+ ->addHeader('Thread-Topic', $subject)
+ ->setIsBulk(true);
+
+ $to_handles = id(new PhabricatorHandleQuery())
+ ->setViewer($viewer)
+ ->withPHIDs($email_phids)
+ ->execute();
+
+ $reply_handler = new PhabricatorRepositoryPushReplyHandler();
+ $mails = $reply_handler->multiplexMail(
+ $mail,
+ $to_handles,
+ array());
+
+ foreach ($mails as $mail) {
+ $mail->saveAndSend();
+ }
+ }
+
+ public function renderForDisplay(PhabricatorUser $viewer) {
+ // This data has some sensitive stuff, so don't show it.
+ return null;
+ }
+
+ private function renderRefs(array $logs) {
+ $ref_lines = array();
+ $ref_list = array();
+
+ foreach ($logs as $log) {
+ $type_name = null;
+ $type_prefix = null;
+ switch ($log->getRefType()) {
+ case PhabricatorRepositoryPushLog::REFTYPE_BRANCH:
+ $type_name = pht('branch');
+ break;
+ case PhabricatorRepositoryPushLog::REFTYPE_TAG:
+ $type_name = pht('tag');
+ $type_prefix = pht('tag:');
+ break;
+ case PhabricatorRepositoryPushLog::REFTYPE_BOOKMARK:
+ $type_name = pht('bookmark');
+ $type_prefix = pht('bookmark:');
+ break;
+ case PhabricatorRepositoryPushLog::REFTYPE_COMMIT:
+ default:
+ break;
+ }
+
+ if ($type_name === null) {
+ continue;
+ }
+
+ $flags = $log->getChangeFlags();
+ if ($flags & PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS) {
+ $action = '!';
+ } else if ($flags & PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE) {
+ $action = '-';
+ } else if ($flags & PhabricatorRepositoryPushLog::CHANGEFLAG_REWRITE) {
+ $action = '~';
+ } else if ($flags & PhabricatorRepositoryPushLog::CHANGEFLAG_APPEND) {
+ $action = ' ';
+ } else if ($flags & PhabricatorRepositoryPushLog::CHANGEFLAG_ADD) {
+ $action = '+';
+ } else {
+ $action = '?';
+ }
+
+ $old = nonempty($log->getRefOldShort(), pht('<null>'));
+ $new = nonempty($log->getRefNewShort(), pht('<null>'));
+
+ $name = $log->getRefName();
+
+ $ref_lines[] = "{$action} {$type_name} {$name} {$old} > {$new}";
+ $ref_list[] = $type_prefix.$name;
+ }
+
+ return array(
+ $ref_lines,
+ array_unique($ref_list),
+ );
+ }
+
+ private function renderCommits(
+ PhabricatorRepository $repository,
+ array $logs,
+ array $info) {
+
+ $commit_lines = array();
+ $subject_line = null;
+ foreach ($logs as $log) {
+ if ($log->getRefType() != PhabricatorRepositoryPushLog::REFTYPE_COMMIT) {
+ continue;
+ }
+
+ $commit_info = idx($info, $log->getRefNew(), array());
+
+ $name = $repository->formatCommitName($log->getRefNew());
+
+ $branches = null;
+ if (idx($commit_info, 'branches')) {
+ $branches = ' ('.implode(', ', $commit_info['branches']).')';
+ }
+
+ $summary = null;
+ if (strlen(idx($commit_info, 'summary'))) {
+ $summary = ' '.$commit_info['summary'];
+ }
+
+ $commit_lines[] = "{$name}{$branches}{$summary}";
+ if ($subject_line === null) {
+ $subject_line = "{$name}{$summary}";
+ }
+ }
+
+ return array($commit_lines, $subject_line);
+ }
+
+}
diff --git a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
--- a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
@@ -861,6 +861,20 @@
'%s, %d lines',
),
+ '%s pushed %d commit(s) to %s.' => array(
+ array(
+ array(
+ '%s pushed a commit to %3$s.',
+ '%s pushed %d commits to %s.',
+ ),
+ ),
+ ),
+
+ '%s commit(s)' => array(
+ '1 commit',
+ '%s commits',
+ ),
+
);
}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 2, 7:08 PM (1 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6722496
Default Alt Text
D8618.id20451.diff (19 KB)

Event Timeline