diff --git a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php --- a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php @@ -43,6 +43,7 @@ ->withIDs(array($revision_id)) ->setViewer($request->getUser()) ->needReviewers(true) + ->needCommitPHIDs(true) ->executeOne(); if (!$revision) { @@ -59,7 +60,7 @@ $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) diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -51,6 +51,7 @@ ->setViewer($viewer) ->needReviewers(true) ->needReviewerAuthority(true) + ->needCommitPHIDs(true) ->executeOne(); if (!$revision) { return new Aphront404Response(); @@ -146,7 +147,7 @@ $object_phids = array_merge( $revision->getReviewerPHIDs(), $subscriber_phids, - $revision->loadCommitPHIDs(), + $revision->getCommitPHIDs(), array( $revision->getAuthorPHID(), $viewer->getPHID(), diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -16,7 +16,6 @@ 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 @@ 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 @@ $conn_r = $table->establishConnection('r'); if ($this->needCommitPHIDs) { - $this->loadCommitPHIDs($conn_r, $revisions); + $this->loadCommitPHIDs($revisions); } $need_active = $this->needActiveDiffs; @@ -606,13 +591,6 @@ $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 @@ $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 @@ ); } - private function loadCommitPHIDs($conn_r, array $revisions) { + private function loadCommitPHIDs(array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); - $commit_phids = queryfx_all( - $conn_r, - 'SELECT * FROM %T WHERE revisionID IN (%Ld)', - DifferentialRevision::TABLE_COMMIT, - mpull($revisions, 'getID')); - $commit_phids = igroup($commit_phids, 'revisionID'); - foreach ($revisions as $revision) { - $phids = idx($commit_phids, $revision->getID(), array()); - $phids = ipull($phids, 'commitPHID'); - $revision->attachCommitPHIDs($phids); + + if (!$revisions) { + return; + } + + $revisions = mpull($revisions, null, 'getPHID'); + + $edge_query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs(array_keys($revisions)) + ->withEdgeTypes( + array( + DifferentialRevisionHasCommitEdgeType::EDGECONST, + )); + $edge_query->execute(); + + foreach ($revisions as $phid => $revision) { + $commit_phids = $edge_query->getDestinationPHIDs(array($phid)); + $revision->attachCommitPHIDs($commit_phids); } } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -41,7 +41,7 @@ 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 @@ 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 @@ } public function attachCommitPHIDs(array $phids) { - $this->commits = array_values($phids); + $this->commitPHIDs = $phids; return $this; }