D8618.id20446.diff
No OneTemporary
Actions

Size

19 KB

Referenced Files

None

Subscribers

None

D8618.id20446.diff
View Options

	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/editor/PhabricatorRepositoryEditor.php b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
	--- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php
	+++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
	@@ -318,7 +318,7 @@
	$errors = parent::validateTransaction($object, $type, $xactions);

	switch ($type) {
	- case self::TYPE_CREDENTIAL:
	+ case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL:
	$ok = PassphraseCredentialControl::validateTransactions(
	$this->getActor(),
	$xactions);
	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: Wed, Oct 23, 11:48 PM (2 w, 6 d ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 6745456
Default Alt Text: D8618.id20446.diff (19 KB)

D8618.id20446.diffNo OneTemporaryActions

D8618.id20446.diffView Options

File Metadata

Event Timeline

D8618.id20446.diff
No OneTemporary
Actions

D8618.id20446.diff
View Options