Page MenuHomePhabricator
Files F91720

D7805.diff
All Users
Actions

D7805.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
@@ -508,6 +508,7 @@
'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php',
'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php',
'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php',
+ 'DiffusionLowLevelCommitFieldsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php',
'DiffusionLowLevelCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php',
'DiffusionLowLevelGitRefQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php',
'DiffusionLowLevelMercurialBranchesQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php',
@@ -2894,6 +2895,7 @@
'DiffusionLastModifiedController' => 'DiffusionController',
'DiffusionLintController' => 'DiffusionController',
'DiffusionLintDetailsController' => 'DiffusionController',
+ 'DiffusionLowLevelCommitFieldsQuery' => 'DiffusionLowLevelQuery',
'DiffusionLowLevelCommitQuery' => 'DiffusionLowLevelQuery',
'DiffusionLowLevelGitRefQuery' => 'DiffusionLowLevelQuery',
'DiffusionLowLevelMercurialBranchesQuery' => 'DiffusionLowLevelQuery',
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php
new file mode 100644
--- /dev/null
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php
@@ -0,0 +1,86 @@
+<?php
+
+final class DiffusionLowLevelCommitFieldsQuery
+ extends DiffusionLowLevelQuery {
+
+ private $ref;
+
+ public function withCommitRef(DiffusionCommitRef $ref) {
+ $this->ref = $ref;
+ return $this;
+ }
+
+ public function executeQuery() {
+ $ref = $this->ref;
+ $message = $ref->getMessage();
+ $hashes = $ref->getHashes();
+
+ $params = array(
+ 'corpus' => $message,
+ 'partial' => true,
+ );
+
+ $result = id(new ConduitCall('differential.parsecommitmessage', $params))
+ ->setUser(PhabricatorUser::getOmnipotentUser())
+ ->execute();
+ $fields = $result['fields'];
+
+ // If there is no "Differential Revision:" field in the message, try to
+ // identify the revision by doing a hash lookup.
+
+ $revision_id = idx($fields, 'revisionID');
+ if (!$revision_id && $hashes) {
+ $hash_list = array();
+ foreach ($hashes as $hash) {
+ $hash_list[] = array($hash->getHashType(), $hash->getHashValue());
+ }
+ $revisions = id(new DifferentialRevisionQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withCommitHashes($hash_list)
+ ->execute();
+
+ if (!empty($revisions)) {
+ $revision = $this->pickBestRevision($revisions);
+ $fields['revisionID'] = $revision->getID();
+ }
+ }
+
+ return $fields;
+ }
+
+
+ /**
+ * When querying for revisions by hash, more than one revision may be found.
+ * This function identifies the "best" revision from such a set. Typically,
+ * there is only one revision found. Otherwise, we try to pick an accepted
+ * revision first, followed by an open revision, and otherwise we go with a
+ * closed or abandoned revision as a last resort.
+ */
+ private function pickBestRevision(array $revisions) {
+ assert_instances_of($revisions, 'DifferentialRevision');
+
+ // If we have more than one revision of a given status, choose the most
+ // recently updated one.
+ $revisions = msort($revisions, 'getDateModified');
+ $revisions = array_reverse($revisions);
+
+ // Try to find an accepted revision first.
+ $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
+ foreach ($revisions as $revision) {
+ if ($revision->getStatus() == $status_accepted) {
+ return $revision;
+ }
+ }
+
+ // Try to find an open revision.
+ foreach ($revisions as $revision) {
+ if (!$revision->isClosed()) {
+ return $revision;
+ }
+ }
+
+ // Settle for whatever's left.
+ return head($revisions);
+ }
+
+}
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -43,43 +43,19 @@
$author_phid);
}
- $call = new ConduitCall(
- 'differential.parsecommitmessage',
- array(
- 'corpus' => $message,
- 'partial' => true,
- ));
- $call->setUser(PhabricatorUser::getOmnipotentUser());
- $result = $call->execute();
-
- $field_values = $result['fields'];
+ $field_values = id(new DiffusionLowLevelCommitFieldsQuery())
+ ->setRepository($repository)
+ ->withCommitRef($ref)
+ ->execute();
+ $revision_id = idx($field_values, 'revisionID');
if (!empty($field_values['reviewedByPHIDs'])) {
$data->setCommitDetail(
'reviewerPHID',
reset($field_values['reviewedByPHIDs']));
}
- $revision_id = idx($field_values, 'revisionID');
- if (!$revision_id && $hashes) {
- $hash_list = array();
- foreach ($hashes as $hash) {
- $hash_list[] = array($hash->getHashType(), $hash->getHashValue());
- }
- $revisions = id(new DifferentialRevisionQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withCommitHashes($hash_list)
- ->execute();
-
- if (!empty($revisions)) {
- $revision = $this->identifyBestRevision($revisions);
- $revision_id = $revision->getID();
- }
- }
-
- $data->setCommitDetail(
- 'differential.revisionID',
- $revision_id);
+ $data->setCommitDetail('differential.revisionID', $revision_id);
if ($author_phid != $commit->getAuthorPHID()) {
$commit->setAuthorPHID($author_phid);
@@ -386,65 +362,6 @@
return null;
}
- /**
- * When querying for revisions by hash, more than one revision may be found.
- * This function identifies the "best" revision from such a set. Typically,
- * there is only one revision found. Otherwise, we try to pick an accepted
- * revision first, followed by an open revision, and otherwise we go with a
- * closed or abandoned revision as a last resort.
- */
- private function identifyBestRevision(array $revisions) {
- assert_instances_of($revisions, 'DifferentialRevision');
- // get the simplest, common case out of the way
- if (count($revisions) == 1) {
- return reset($revisions);
- }
-
- $first_choice = array();
- $second_choice = array();
- $third_choice = array();
- foreach ($revisions as $revision) {
- switch ($revision->getStatus()) {
- // "Accepted" revisions -- ostensibly what we're looking for!
- case ArcanistDifferentialRevisionStatus::ACCEPTED:
- $first_choice[] = $revision;
- break;
- // "Open" revisions
- case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW:
- case ArcanistDifferentialRevisionStatus::NEEDS_REVISION:
- $second_choice[] = $revision;
- break;
- // default is a wtf? here
- default:
- case ArcanistDifferentialRevisionStatus::ABANDONED:
- case ArcanistDifferentialRevisionStatus::CLOSED:
- $third_choice[] = $revision;
- break;
- }
- }
-
- // go down the ladder like a bro at last call
- if (!empty($first_choice)) {
- return $this->identifyMostRecentRevision($first_choice);
- }
- if (!empty($second_choice)) {
- return $this->identifyMostRecentRevision($second_choice);
- }
- if (!empty($third_choice)) {
- return $this->identifyMostRecentRevision($third_choice);
- }
- }
-
- /**
- * Given a set of revisions, returns the revision with the latest
- * updated time. This is ostensibly the most recent revision.
- */
- private function identifyMostRecentRevision(array $revisions) {
- assert_instances_of($revisions, 'DifferentialRevision');
- $revisions = msort($revisions, 'getDateModified');
- return end($revisions);
- }
-
private function resolveUserPHID(
PhabricatorRepositoryCommit $commit,
$user_name) {

File Metadata

Mime Type
text/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/nk/z4/arnrlqdx3abotqwu
Default Alt Text
D7805.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility