1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 08:12:40 +01:00

Support a "withPaths()" API in DifferentialRevisionQuery, and use it on the revision view

Summary: Ref T13639. Move away from "withPath(..., ...)" to "withPaths(...)".

Test Plan: {F8539323}

Maniphest Tasks: T13639

Differential Revision: https://secure.phabricator.com/D21619
This commit is contained in:
epriestley 2021-03-15 09:53:41 -07:00
parent 26c68942bd
commit 925c9a71e7
2 changed files with 78 additions and 13 deletions

View file

@ -986,6 +986,8 @@ final class DifferentialRevisionViewController
PhabricatorRepository $repository) { PhabricatorRepository $repository) {
assert_instances_of($changesets, 'DifferentialChangeset'); assert_instances_of($changesets, 'DifferentialChangeset');
$viewer = $this->getViewer();
$paths = array(); $paths = array();
foreach ($changesets as $changeset) { foreach ($changesets as $changeset) {
$paths[] = $changeset->getAbsoluteRepositoryPath( $paths[] = $changeset->getAbsoluteRepositoryPath(
@ -997,34 +999,30 @@ final class DifferentialRevisionViewController
return array(); return array();
} }
$path_map = id(new DiffusionPathIDQuery($paths))->loadPathIDs();
if (!$path_map) {
return array();
}
$recent = (PhabricatorTime::getNow() - phutil_units('30 days in seconds')); $recent = (PhabricatorTime::getNow() - phutil_units('30 days in seconds'));
$query = id(new DifferentialRevisionQuery()) $query = id(new DifferentialRevisionQuery())
->setViewer($this->getRequest()->getUser()) ->setViewer($viewer)
->withIsOpen(true) ->withIsOpen(true)
->withUpdatedEpochBetween($recent, null) ->withUpdatedEpochBetween($recent, null)
->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED)
->setLimit(10) ->setLimit(10)
->needFlags(true) ->needFlags(true)
->needDrafts(true) ->needDrafts(true)
->needReviewers(true); ->needReviewers(true)
->withRepositoryPHIDs(
foreach ($path_map as $path => $path_id) { array(
$query->withPath($repository->getID(), $path_id); $repository->getPHID(),
} ))
->withPaths($paths);
$results = $query->execute(); $results = $query->execute();
// Strip out *this* revision. // Strip out *this* revision.
foreach ($results as $key => $result) { foreach ($results as $key => $result) {
if ($result->getID() == $this->revisionID) { if ($result->getID() == $this->revisionID) {
unset($results[$key]); unset($results[$key]);
break;
} }
} }

View file

@ -27,6 +27,7 @@ final class DifferentialRevisionQuery
private $createdEpochMin; private $createdEpochMin;
private $createdEpochMax; private $createdEpochMax;
private $noReviewers; private $noReviewers;
private $paths;
const ORDER_MODIFIED = 'order-modified'; const ORDER_MODIFIED = 'order-modified';
const ORDER_CREATED = 'order-created'; const ORDER_CREATED = 'order-created';
@ -62,6 +63,18 @@ final class DifferentialRevisionQuery
return $this; return $this;
} }
/**
* Find revisions affecting one or more items in a list of paths.
*
* @param list<string> 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 * Filter results to revisions authored by one of the given PHIDs. Calling
* this function will clear anything set by previous calls to * this function will clear anything set by previous calls to
@ -576,6 +589,14 @@ final class DifferentialRevisionQuery
$path_table->getTableName()); $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) { if ($this->commitHashes) {
$joins[] = qsprintf( $joins[] = qsprintf(
$conn, $conn,
@ -635,6 +656,7 @@ final class DifferentialRevisionQuery
* @task internal * @task internal
*/ */
protected function buildWhereClause(AphrontDatabaseConnection $conn) { protected function buildWhereClause(AphrontDatabaseConnection $conn) {
$viewer = $this->getViewer();
$where = array(); $where = array();
if ($this->pathIDs) { if ($this->pathIDs) {
@ -651,6 +673,45 @@ final class DifferentialRevisionQuery
$where[] = $path_clauses; $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) { if ($this->authors) {
$where[] = qsprintf( $where[] = qsprintf(
$conn, $conn,
@ -782,6 +843,12 @@ final class DifferentialRevisionQuery
return true; 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) { if (count($this->ccs) > 1) {
return true; return true;
} }