Move commit message/metadata field query to a separate class

Summary: Ref T4195. I need to query commit metadata to figure out which revision a commit is associated with. Move this out of the MessageParser so the code can be called from the HookEngine.

Test Plan: Used `reparse.php` to reparse a variety of SVN, Mercurial and Git commits. Used `var_dump()` to verify sensible fields were returned.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7805

src/__phutil_library_map__.php

@@ -508,6 +508,7 @@ phutil_register_library_map(array(
     '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 @@ phutil_register_library_map(array(
     'DiffusionLastModifiedController' => 'DiffusionController',
     'DiffusionLintController' => 'DiffusionController',
     'DiffusionLintDetailsController' => 'DiffusionController',
+    'DiffusionLowLevelCommitFieldsQuery' => 'DiffusionLowLevelQuery',
     'DiffusionLowLevelCommitQuery' => 'DiffusionLowLevelQuery',
     'DiffusionLowLevelGitRefQuery' => 'DiffusionLowLevelQuery',
     'DiffusionLowLevelMercurialBranchesQuery' => 'DiffusionLowLevelQuery',

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);
+  }
+
+}

src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -43,16 +43,11 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
         $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(
@@ -60,26 +55,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
         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 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     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) {