From 3a80efa440f042af09e571bf4e372f1e381fdae8 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 12 Aug 2020 12:00:13 -0700
Subject: [PATCH] Build "DiffusionCommitRef" objects from
 "internal.commit.search", not "diffusion.querycommits", in the message parser
 worker

Summary: Ref T13552. Swap the call we're using to build "CommitRef" objects here to the recently-introduced "internal.commit.search" method.

Test Plan: Used "bin/repository reparse --message ..." to reparse commits, added "var_dump()" to inspect results. Saw sensible CommitRef and CommitData objects get built.

Maniphest Tasks: T13552

Differential Revision: https://secure.phabricator.com/D21446
---
 .../diffusion/query/DiffusionQuery.php        | 12 ++---
 .../storage/PhabricatorRepository.php         | 22 ++++++++++
 .../storage/PhabricatorRepositoryCommit.php   | 44 +++++++++++++++++++
 ...habricatorRepositoryCommitParserWorker.php |  5 +++
 ...torRepositoryCommitMessageParserWorker.php | 25 +----------
 5 files changed, 76 insertions(+), 32 deletions(-)

diff --git a/src/applications/diffusion/query/DiffusionQuery.php b/src/applications/diffusion/query/DiffusionQuery.php
index 6675197932..4b2258da06 100644
--- a/src/applications/diffusion/query/DiffusionQuery.php
+++ b/src/applications/diffusion/query/DiffusionQuery.php
@@ -73,17 +73,11 @@ abstract class DiffusionQuery extends PhabricatorQuery {
 
     $params = $params + $core_params;
 
-    $client = $repository->newConduitClient(
+    $future = $repository->newConduitFuture(
       $user,
+      $method,
+      $params,
       $drequest->getIsClusterRequest());
-    if (!$client) {
-      $result = id(new ConduitCall($method, $params))
-        ->setUser($user)
-        ->execute();
-      $future = new ImmediateFuture($result);
-    } else {
-      $future = $client->callMethod($method, $params);
-    }
 
     if (!$return_future) {
       return $future->resolve();
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
index 71afc56281..ca6fd7c484 100644
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -2258,6 +2258,28 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     return $client;
   }
 
+  public function newConduitFuture(
+    PhabricatorUser $viewer,
+    $method,
+    array $params,
+    $never_proxy = false) {
+
+    $client = $this->newConduitClient(
+      $viewer,
+      $never_proxy);
+
+    if (!$client) {
+      $result = id(new ConduitCall($method, $params))
+        ->setUser($viewer)
+        ->execute();
+      $future = new ImmediateFuture($result);
+    } else {
+      $future = $client->callMethod($method, $params);
+    }
+
+    return $future;
+  }
+
   public function getPassthroughEnvironmentalVariables() {
     $env = $_ENV;
 
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
index bfc01ae97c..7093b75370 100644
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -523,6 +523,50 @@ final class PhabricatorRepositoryCommit
     return $data->getCommitDetail('committer');
   }
 
+  public function newCommitRef(PhabricatorUser $viewer) {
+    $repository = $this->getRepository();
+
+    $future = $repository->newConduitFuture(
+      $viewer,
+      'internal.commit.search',
+      array(
+        'constraints' => array(
+          'repositoryPHIDs' => array($repository->getPHID()),
+          'phids' => array($this->getPHID()),
+        ),
+      ));
+    $result = $future->resolve();
+
+    $commit_display = $this->getMonogram();
+
+    if (empty($result['data'])) {
+      throw new Exception(
+        pht(
+          'Unable to retrieve details for commit "%s"!',
+          $commit_display));
+    }
+
+    if (count($result['data']) !== 1) {
+      throw new Exception(
+        pht(
+          'Got too many results (%s) for commit "%s", expected %s.',
+          phutil_count($result['data']),
+          $commit_display,
+          1));
+    }
+
+    $record = head($result['data']);
+    $ref_record = idxv($record, array('fields', 'ref'));
+
+    if (!$ref_record) {
+      throw new Exception(
+        pht(
+          'Unable to retrieve CommitRef record for commit "%s".',
+          $commit_display));
+    }
+
+    return DiffusionCommitRef::newFromDictionary($ref_record);
+  }
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php
index 9d3b2793f8..656b258b40 100644
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php
@@ -124,4 +124,9 @@ abstract class PhabricatorRepositoryCommitParserWorker
 
     return array($link, $suffix);
   }
+
+  final public function getViewer() {
+    return PhabricatorUser::getOmnipotentUser();
+  }
+
 }
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
index ab4719aee5..3eead0a26d 100644
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -14,31 +14,10 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     PhabricatorRepositoryCommit $commit) {
 
     if (!$this->shouldSkipImportStep()) {
-      $viewer = PhabricatorUser::getOmnipotentUser();
+      $viewer = $this->getViewer();
 
-      $refs_raw = DiffusionQuery::callConduitWithDiffusionRequest(
-        $viewer,
-        DiffusionRequest::newFromDictionary(
-          array(
-            'repository' => $repository,
-            'user' => $viewer,
-          )),
-        'diffusion.querycommits',
-        array(
-          'repositoryPHID' => $repository->getPHID(),
-          'phids' => array($commit->getPHID()),
-          'bypassCache' => true,
-          'needMessages' => true,
-        ));
+      $ref = $commit->newCommitRef($viewer);
 
-      if (empty($refs_raw['data'])) {
-        throw new Exception(
-          pht(
-            'Unable to retrieve details for commit "%s"!',
-            $commit->getPHID()));
-      }
-
-      $ref = DiffusionCommitRef::newFromConduitResult(head($refs_raw['data']));
       $this->updateCommitData($ref);
     }