Internally, align commit processing tasks around PHIDs, not IDs

Summary: Ref T13591. This is a minor consistency change to use PHIDs instead of IDs in the commit import processing pipeline. PHIDs are generally more powerful in more contexts and it would be unusual for a modern worker to use an ID here.

Test Plan:
  - Made the "accept either ID or PHID" part of the change only.
  - Pushed a commit, parsed and reparsed it step by step (this tests that "commitID" tasks can still process normally).
  - Made the "write PHIDs" part of the change.
  - Pushed a commit, parsed and reparsed it step by step.
  - Looked at the task row in the database, saw PHID data.

Maniphest Tasks: T13591

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

src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php

@@ -682,7 +682,6 @@ final class PhabricatorRepositoryDiscoveryEngine
 
     $this->queueCommitImportTask(
       $repository,
-      $commit->getID(),
       $commit->getPHID(),
       $task_priority,
       $via = 'discovery');

src/applications/repository/engine/PhabricatorRepositoryEngine.php

@@ -85,10 +85,9 @@ abstract class PhabricatorRepositoryEngine extends Phobject {
 
   final protected function queueCommitImportTask(
     PhabricatorRepository $repository,
-    $commit_id,
     $commit_phid,
     $task_priority,
-    $via = null) {
+    $via) {
 
     $vcs = $repository->getVersionControlSystem();
     switch ($vcs) {
@@ -109,7 +108,7 @@ abstract class PhabricatorRepositoryEngine extends Phobject {
     }
 
     $data = array(
-      'commitID' => $commit_id,
+      'commitPHID' => $commit_phid,
     );
 
     if ($via !== null) {

src/applications/repository/engine/PhabricatorRepositoryRefEngine.php

@@ -597,7 +597,6 @@ final class PhabricatorRepositoryRefEngine
 
         $this->queueCommitImportTask(
           $repository,
-          $row['id'],
           $row['phid'],
           $task_priority,
           $via = 'ref');

src/applications/repository/management/PhabricatorRepositoryManagementReparseWorkflow.php

@@ -247,7 +247,7 @@ final class PhabricatorRepositoryManagementReparseWorkflow
       // all the requested steps explicitly.
 
       $spec = array(
-        'commitID' => $commit->getID(),
+        'commitPHID' => $commit->getPHID(),
         'only' => !$importing,
         'via' => 'reparse',
       );

src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php

@@ -11,30 +11,51 @@ abstract class PhabricatorRepositoryCommitParserWorker
       return $this->commit;
     }
 
-    $commit_id = idx($this->getTaskData(), 'commitID');
+    $viewer = $this->getViewer();
-    if (!$commit_id) {
+    $task_data = $this->getTaskData();
-      throw new PhabricatorWorkerPermanentFailureException(
+
-        pht('No "%s" in task data.', 'commitID'));
+    $commit_query = id(new DiffusionCommitQuery())
+      ->setViewer($viewer);
+
+    $commit_phid = idx($task_data, 'commitPHID');
+
+    // TODO: See T13591. This supports execution of legacy tasks and can
+    // eventually be removed. Newer tasks use "commitPHID" instead of
+    // "commitID".
+    if (!$commit_phid) {
+      $commit_id = idx($task_data, 'commitID');
+      if ($commit_id) {
+        $legacy_commit = id(clone $commit_query)
+          ->withIDs(array($commit_id))
+          ->executeOne();
+        if ($legacy_commit) {
+          $commit_phid = $legacy_commit->getPHID();
+        }
+      }
     }
 
-    $commit = id(new DiffusionCommitQuery())
+    if (!$commit_phid) {
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      throw new PhabricatorWorkerPermanentFailureException(
-      ->withIDs(array($commit_id))
+        pht('Task data has no "commitPHID".'));
+    }
+
+    $commit = id(clone $commit_query)
+      ->withPHIDs(array($commit_phid))
       ->executeOne();
     if (!$commit) {
       throw new PhabricatorWorkerPermanentFailureException(
-        pht('Commit "%s" does not exist.', $commit_id));
+        pht('Commit "%s" does not exist.', $commit_phid));
     }
 
     if ($commit->isUnreachable()) {
       throw new PhabricatorWorkerPermanentFailureException(
         pht(
-          'Commit "%s" (with internal ID "%s") is no longer reachable from '.
+          'Commit "%s" (with PHID "%s") is no longer reachable from any '.
-          'any branch, tag, or ref in this repository, so it will not be '.
+          'branch, tag, or ref in this repository, so it will not be '.
           'imported. This usually means that the branch the commit was on '.
           'was deleted or overwritten.',
           $commit->getMonogram(),
-          $commit_id));
+          $commit_phid));
     }
 
     $this->commit = $commit;
@@ -64,7 +85,7 @@ abstract class PhabricatorRepositoryCommitParserWorker
     $repository = $commit->getRepository();
 
     $data = array(
-      'commitID' => $commit->getID(),
+      'commitPHID' => $commit->getPHID(),
     );
 
     $task_data = $this->getTaskData();
@@ -144,7 +165,7 @@ abstract class PhabricatorRepositoryCommitParserWorker
 
     $commit = id(new DiffusionCommitQuery())
       ->setViewer($viewer)
-      ->withIDs(array(idx($this->getTaskData(), 'commitID')))
+      ->withPHIDs(array(idx($this->getTaskData(), 'commitPHID')))
       ->executeOne();
     if (!$commit) {
       return $suffix;