Align "RevisionQuery->needCommitPHIDs()" to use Edges, not the legacy table

Summary:
Depends on D20458. Ref T13276. Although I'm not thrilled about "needCommitPHIDs()", it has a few callers, including custom fields. Allow "need + attach + get" to survive for now since they're reasonably modern, at least.

However, use edges instead of "TABLE_COMMIT" and require `need...()` + `get...()`, removing the direct `load...()`.

Also remove `RevisionQuery->withCommitPHIDs(...)`, which has no callers.

Test Plan:
- Grepped for `loadCommitPHIDs` (only two hits, the private `RevisionQuery` method).
- Called "differential.getrevision", got commits.
- Viewed a revision, saw "Commits: ...".
- Grepped for `withCommitPHIDs()`, no callers on `RevisionQuery` (some other query classes have methods with this name).
- Called "differential.query", got commits.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13276

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

src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php

@@ -43,6 +43,7 @@ final class DifferentialGetRevisionConduitAPIMethod
       ->withIDs(array($revision_id))
       ->setViewer($request->getUser())
       ->needReviewers(true)
+      ->needCommitPHIDs(true)
       ->executeOne();
 
     if (!$revision) {
@@ -59,7 +60,7 @@ final class DifferentialGetRevisionConduitAPIMethod
     $diff_dicts = mpull($diffs, 'getDiffDict');
 
     $commit_dicts = array();
-    $commit_phids = $revision->loadCommitPHIDs();
+    $commit_phids = $revision->getCommitPHIDs();
     $handles = id(new PhabricatorHandleQuery())
       ->setViewer($request->getUser())
       ->withPHIDs($commit_phids)

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -51,6 +51,7 @@ final class DifferentialRevisionViewController
       ->setViewer($viewer)
       ->needReviewers(true)
       ->needReviewerAuthority(true)
+      ->needCommitPHIDs(true)
       ->executeOne();
     if (!$revision) {
       return new Aphront404Response();
@@ -146,7 +147,7 @@ final class DifferentialRevisionViewController
     $object_phids = array_merge(
       $revision->getReviewerPHIDs(),
       $subscriber_phids,
-      $revision->loadCommitPHIDs(),
+      $revision->getCommitPHIDs(),
       array(
         $revision->getAuthorPHID(),
         $viewer->getPHID(),

src/applications/differential/query/DifferentialRevisionQuery.php

@@ -16,7 +16,6 @@ final class DifferentialRevisionQuery
   private $reviewers = array();
   private $revIDs = array();
   private $commitHashes = array();
-  private $commitPHIDs = array();
   private $phids = array();
   private $responsibles = array();
   private $branches = array();
@@ -119,20 +118,6 @@ final class DifferentialRevisionQuery
     return $this;
   }
 
-  /**
-   * Filter results to revisions that have one of the provided PHIDs as
-   * commits. Calling this function will clear anything set by previous calls
-   * to @{method:withCommitPHIDs}.
-   *
-   * @param array List of PHIDs of commits
-   * @return this
-   * @task config
-   */
-  public function withCommitPHIDs(array $commit_phids) {
-    $this->commitPHIDs = $commit_phids;
-    return $this;
-  }
-
   public function withStatuses(array $statuses) {
     $this->statuses = $statuses;
     return $this;
@@ -400,7 +385,7 @@ final class DifferentialRevisionQuery
     $conn_r = $table->establishConnection('r');
 
     if ($this->needCommitPHIDs) {
-      $this->loadCommitPHIDs($conn_r, $revisions);
+      $this->loadCommitPHIDs($revisions);
     }
 
     $need_active = $this->needActiveDiffs;
@@ -606,13 +591,6 @@ final class DifferentialRevisionQuery
         $this->draftAuthors);
     }
 
-    if ($this->commitPHIDs) {
-      $joins[] = qsprintf(
-        $conn,
-        'JOIN %T commits ON commits.revisionID = r.id',
-        DifferentialRevision::TABLE_COMMIT);
-    }
-
     $joins[] = $this->buildJoinClauseParts($conn);
 
     return $this->formatJoinClause($conn, $joins);
@@ -674,13 +652,6 @@ final class DifferentialRevisionQuery
       $where[] = $hash_clauses;
     }
 
-    if ($this->commitPHIDs) {
-      $where[] = qsprintf(
-        $conn,
-        'commits.commitPHID IN (%Ls)',
-        $this->commitPHIDs);
-    }
-
     if ($this->phids) {
       $where[] = qsprintf(
         $conn,
@@ -807,18 +778,26 @@ final class DifferentialRevisionQuery
     );
   }
 
-  private function loadCommitPHIDs($conn_r, array $revisions) {
+  private function loadCommitPHIDs(array $revisions) {
     assert_instances_of($revisions, 'DifferentialRevision');
-    $commit_phids = queryfx_all(
+
-      $conn_r,
+    if (!$revisions) {
-      'SELECT * FROM %T WHERE revisionID IN (%Ld)',
+      return;
-      DifferentialRevision::TABLE_COMMIT,
+    }
-      mpull($revisions, 'getID'));
+
-    $commit_phids = igroup($commit_phids, 'revisionID');
+    $revisions = mpull($revisions, null, 'getPHID');
-    foreach ($revisions as $revision) {
+
-      $phids = idx($commit_phids, $revision->getID(), array());
+    $edge_query = id(new PhabricatorEdgeQuery())
-      $phids = ipull($phids, 'commitPHID');
+      ->withSourcePHIDs(array_keys($revisions))
-      $revision->attachCommitPHIDs($phids);
+      ->withEdgeTypes(
+        array(
+          DifferentialRevisionHasCommitEdgeType::EDGECONST,
+        ));
+    $edge_query->execute();
+
+    foreach ($revisions as $phid => $revision) {
+      $commit_phids = $edge_query->getDestinationPHIDs(array($phid));
+      $revision->attachCommitPHIDs($commit_phids);
     }
   }
 

src/applications/differential/storage/DifferentialRevision.php

@@ -41,7 +41,7 @@ final class DifferentialRevision extends DifferentialDAO
   protected $editPolicy = PhabricatorPolicies::POLICY_USER;
   protected $properties = array();
 
-  private $commits = self::ATTACHABLE;
+  private $commitPHIDs = self::ATTACHABLE;
   private $activeDiff = self::ATTACHABLE;
   private $diffIDs = self::ATTACHABLE;
   private $hashes = self::ATTACHABLE;
@@ -158,23 +158,8 @@ final class DifferentialRevision extends DifferentialDAO
     return '/'.$this->getMonogram();
   }
 
-  public function loadCommitPHIDs() {
-    if (!$this->getID()) {
-      return ($this->commits = array());
-    }
-
-    $commits = queryfx_all(
-      $this->establishConnection('r'),
-      'SELECT commitPHID FROM %T WHERE revisionID = %d',
-      self::TABLE_COMMIT,
-      $this->getID());
-    $commits = ipull($commits, 'commitPHID');
-
-    return ($this->commits = $commits);
-  }
-
   public function getCommitPHIDs() {
-    return $this->assertAttached($this->commits);
+    return $this->assertAttached($this->commitPHIDs);
   }
 
   public function getActiveDiff() {
@@ -202,7 +187,7 @@ final class DifferentialRevision extends DifferentialDAO
   }
 
   public function attachCommitPHIDs(array $phids) {
-    $this->commits = array_values($phids);
+    $this->commitPHIDs = $phids;
     return $this;
   }