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 @@ -986,6 +986,8 @@ PhabricatorRepository $repository) { assert_instances_of($changesets, 'DifferentialChangeset'); + $viewer = $this->getViewer(); + $paths = array(); foreach ($changesets as $changeset) { $paths[] = $changeset->getAbsoluteRepositoryPath( @@ -997,34 +999,30 @@ return array(); } - $path_map = id(new DiffusionPathIDQuery($paths))->loadPathIDs(); - - if (!$path_map) { - return array(); - } - $recent = (PhabricatorTime::getNow() - phutil_units('30 days in seconds')); $query = id(new DifferentialRevisionQuery()) - ->setViewer($this->getRequest()->getUser()) + ->setViewer($viewer) ->withIsOpen(true) ->withUpdatedEpochBetween($recent, null) ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setLimit(10) ->needFlags(true) ->needDrafts(true) - ->needReviewers(true); - - foreach ($path_map as $path => $path_id) { - $query->withPath($repository->getID(), $path_id); - } + ->needReviewers(true) + ->withRepositoryPHIDs( + array( + $repository->getPHID(), + )) + ->withPaths($paths); $results = $query->execute(); // Strip out *this* revision. foreach ($results as $key => $result) { if ($result->getID() == $this->revisionID) { - unset($results[$key]); + unset($results[$key]); + break; } } 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 @@ -27,6 +27,7 @@ private $createdEpochMin; private $createdEpochMax; private $noReviewers; + private $paths; const ORDER_MODIFIED = 'order-modified'; const ORDER_CREATED = 'order-created'; @@ -62,6 +63,18 @@ return $this; } + /** + * Find revisions affecting one or more items in a list of paths. + * + * @param list List of file paths. + * @return this + * @task config + */ + public function withPaths(array $paths) { + $this->paths = $paths; + return $this; + } + /** * Filter results to revisions authored by one of the given PHIDs. Calling * this function will clear anything set by previous calls to @@ -576,6 +589,14 @@ $path_table->getTableName()); } + if ($this->paths) { + $path_table = new DifferentialAffectedPath(); + $joins[] = qsprintf( + $conn, + 'JOIN %R paths ON paths.revisionID = r.id', + $path_table); + } + if ($this->commitHashes) { $joins[] = qsprintf( $conn, @@ -635,6 +656,7 @@ * @task internal */ protected function buildWhereClause(AphrontDatabaseConnection $conn) { + $viewer = $this->getViewer(); $where = array(); if ($this->pathIDs) { @@ -651,6 +673,45 @@ $where[] = $path_clauses; } + if ($this->paths !== null) { + $paths = $this->paths; + + $path_map = id(new DiffusionPathIDQuery($paths)) + ->loadPathIDs(); + + if (!$path_map) { + // If none of the paths have entries in the PathID table, we can not + // possibly find any revisions affecting them. + throw new PhabricatorEmptyQueryException(); + } + + $where[] = qsprintf( + $conn, + 'paths.pathID IN (%Ld)', + array_fuse($path_map)); + + // If we have repository PHIDs, additionally constrain this query to + // try to help MySQL execute it efficiently. + if ($this->repositoryPHIDs !== null) { + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->setParentQuery($this) + ->withPHIDs($this->repositoryPHIDs) + ->execute(); + + if (!$repositories) { + throw new PhabricatorEmptyQueryException(); + } + + $repository_ids = mpull($repositories, 'getID'); + + $where[] = qsprintf( + $conn, + 'paths.repositoryID IN (%Ld)', + $repository_ids); + } + } + if ($this->authors) { $where[] = qsprintf( $conn, @@ -782,6 +843,12 @@ return true; } + if ($this->paths) { + // (If we have exactly one repository and exactly one path, we don't + // technically need to group, but it's simpler to always group.) + return true; + } + if (count($this->ccs) > 1) { return true; }