mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 00:42:41 +01:00
Support an "Ancestors Of: ..." constraint in commit queries
Summary: Ref T13137. See PHI609. An install would like to filter audit requests on a particular branch, e.g. "master". This is difficult in the general case because we can not apply this constraint efficiently under every conceivable data shape, but we can do a reasonable job in most practical cases. See T13137#238822 for more detailed discussion on the approach here. This is a bit rough, but should do the job for now. Test Plan: - Filtered commits by various branches, e.g. "master"; "lfs". Saw correct-seeming results. - Stubbed out the "just list everything" path to hit the `diffusion.internal.ancestors` path, saw the same correct-seeming results. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13137 Differential Revision: https://secure.phabricator.com/D19431
This commit is contained in:
parent
397645b273
commit
5b640a434c
4 changed files with 211 additions and 1 deletions
|
@ -816,6 +816,7 @@ phutil_register_library_map(array(
|
|||
'DiffusionHovercardEngineExtension' => 'applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php',
|
||||
'DiffusionInlineCommentController' => 'applications/diffusion/controller/DiffusionInlineCommentController.php',
|
||||
'DiffusionInlineCommentPreviewController' => 'applications/diffusion/controller/DiffusionInlineCommentPreviewController.php',
|
||||
'DiffusionInternalAncestorsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionInternalAncestorsConduitAPIMethod.php',
|
||||
'DiffusionInternalGitRawDiffQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php',
|
||||
'DiffusionLastModifiedController' => 'applications/diffusion/controller/DiffusionLastModifiedController.php',
|
||||
'DiffusionLastModifiedQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php',
|
||||
|
@ -6149,6 +6150,7 @@ phutil_register_library_map(array(
|
|||
'DiffusionHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
|
||||
'DiffusionInlineCommentController' => 'PhabricatorInlineCommentController',
|
||||
'DiffusionInlineCommentPreviewController' => 'PhabricatorInlineCommentPreviewController',
|
||||
'DiffusionInternalAncestorsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
|
||||
'DiffusionInternalGitRawDiffQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
|
||||
'DiffusionLastModifiedController' => 'DiffusionController',
|
||||
'DiffusionLastModifiedQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
|
||||
|
|
|
@ -53,6 +53,10 @@ final class PhabricatorCommitSearchEngine
|
|||
$query->withUnreachable($map['unreachable']);
|
||||
}
|
||||
|
||||
if ($map['ancestorsOf']) {
|
||||
$query->withAncestorsOf($map['ancestorsOf']);
|
||||
}
|
||||
|
||||
return $query;
|
||||
}
|
||||
|
||||
|
@ -103,6 +107,13 @@ final class PhabricatorCommitSearchEngine
|
|||
pht(
|
||||
'Find or exclude unreachable commits which are not ancestors of '.
|
||||
'any branch, tag, or ref.')),
|
||||
id(new PhabricatorSearchStringListField())
|
||||
->setLabel(pht('Ancestors Of'))
|
||||
->setKey('ancestorsOf')
|
||||
->setDescription(
|
||||
pht(
|
||||
'Find commits which are ancestors of a particular ref, '.
|
||||
'like "master".')),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
@ -0,0 +1,51 @@
|
|||
<?php
|
||||
|
||||
final class DiffusionInternalAncestorsConduitAPIMethod
|
||||
extends DiffusionQueryConduitAPIMethod {
|
||||
|
||||
public function isInternalAPI() {
|
||||
return true;
|
||||
}
|
||||
|
||||
public function getAPIMethodName() {
|
||||
return 'diffusion.internal.ancestors';
|
||||
}
|
||||
|
||||
public function getMethodDescription() {
|
||||
return pht('Internal method for filtering ref ancestors.');
|
||||
}
|
||||
|
||||
protected function defineReturnType() {
|
||||
return 'list<string>';
|
||||
}
|
||||
|
||||
protected function defineCustomParamTypes() {
|
||||
return array(
|
||||
'ref' => 'required string',
|
||||
'commits' => 'required list<string>',
|
||||
);
|
||||
}
|
||||
|
||||
protected function getResult(ConduitAPIRequest $request) {
|
||||
$drequest = $this->getDiffusionRequest();
|
||||
$repository = $drequest->getRepository();
|
||||
|
||||
$commits = $request->getValue('commits');
|
||||
$ref = $request->getValue('ref');
|
||||
|
||||
$graph = new PhabricatorGitGraphStream($repository, $ref);
|
||||
|
||||
$keep = array();
|
||||
foreach ($commits as $identifier) {
|
||||
try {
|
||||
$graph->getCommitDate($identifier);
|
||||
$keep[] = $identifier;
|
||||
} catch (Exception $ex) {
|
||||
// Not an ancestor.
|
||||
}
|
||||
}
|
||||
|
||||
return $keep;
|
||||
}
|
||||
|
||||
}
|
|
@ -22,10 +22,14 @@ final class DiffusionCommitQuery
|
|||
private $epochMin;
|
||||
private $epochMax;
|
||||
private $importing;
|
||||
private $ancestorsOf;
|
||||
|
||||
private $needCommitData;
|
||||
private $needDrafts;
|
||||
|
||||
private $mustFilterRefs = false;
|
||||
private $refRepository;
|
||||
|
||||
public function withIDs(array $ids) {
|
||||
$this->ids = $ids;
|
||||
return $this;
|
||||
|
@ -92,7 +96,7 @@ final class DiffusionCommitQuery
|
|||
}
|
||||
|
||||
public function withRepositoryIDs(array $repository_ids) {
|
||||
$this->repositoryIDs = $repository_ids;
|
||||
$this->repositoryIDs = array_unique($repository_ids);
|
||||
return $this;
|
||||
}
|
||||
|
||||
|
@ -152,6 +156,11 @@ final class DiffusionCommitQuery
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function withAncestorsOf(array $refs) {
|
||||
$this->ancestorsOf = $refs;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getIdentifierMap() {
|
||||
if ($this->identifierMap === null) {
|
||||
throw new Exception(
|
||||
|
@ -307,6 +316,54 @@ final class DiffusionCommitQuery
|
|||
protected function didFilterPage(array $commits) {
|
||||
$viewer = $this->getViewer();
|
||||
|
||||
if ($this->mustFilterRefs) {
|
||||
// If this flag is set, the query has an "Ancestors Of" constraint and
|
||||
// at least one of the constraining refs had too many ancestors for us
|
||||
// to apply the constraint with a big "commitIdentifier IN (%Ls)" clause.
|
||||
// We're going to filter each page and hope we get a full result set
|
||||
// before the query overheats.
|
||||
|
||||
$ancestor_list = mpull($commits, 'getCommitIdentifier');
|
||||
$ancestor_list = array_values($ancestor_list);
|
||||
|
||||
foreach ($this->ancestorsOf as $ref) {
|
||||
try {
|
||||
$ancestor_list = DiffusionQuery::callConduitWithDiffusionRequest(
|
||||
$viewer,
|
||||
DiffusionRequest::newFromDictionary(
|
||||
array(
|
||||
'repository' => $this->refRepository,
|
||||
'user' => $viewer,
|
||||
)),
|
||||
'diffusion.internal.ancestors',
|
||||
array(
|
||||
'ref' => $ref,
|
||||
'commits' => $ancestor_list,
|
||||
));
|
||||
} catch (ConduitClientException $ex) {
|
||||
throw new PhabricatorSearchConstraintException(
|
||||
$ex->getMessage());
|
||||
}
|
||||
|
||||
if (!$ancestor_list) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
$ancestor_list = array_fuse($ancestor_list);
|
||||
foreach ($commits as $key => $commit) {
|
||||
$identifier = $commit->getCommitIdentifier();
|
||||
if (!isset($ancestor_list[$identifier])) {
|
||||
$this->didRejectResult($commit);
|
||||
unset($commits[$key]);
|
||||
}
|
||||
}
|
||||
|
||||
if (!$commits) {
|
||||
return $commits;
|
||||
}
|
||||
}
|
||||
|
||||
if ($this->needCommitData) {
|
||||
$data = id(new PhabricatorRepositoryCommitData())->loadAllWhere(
|
||||
'commitID in (%Ld)',
|
||||
|
@ -364,6 +421,95 @@ final class DiffusionCommitQuery
|
|||
$this->withRepositoryIDs($repository_ids);
|
||||
}
|
||||
|
||||
if ($this->ancestorsOf !== null) {
|
||||
if (count($this->repositoryIDs) !== 1) {
|
||||
throw new PhabricatorSearchConstraintException(
|
||||
pht(
|
||||
'To search for commits which are ancestors of particular refs, '.
|
||||
'you must constrain the search to exactly one repository.'));
|
||||
}
|
||||
|
||||
$repository_id = head($this->repositoryIDs);
|
||||
$history_limit = $this->getRawResultLimit() * 32;
|
||||
$viewer = $this->getViewer();
|
||||
|
||||
$repository = id(new PhabricatorRepositoryQuery())
|
||||
->setViewer($viewer)
|
||||
->withIDs(array($repository_id))
|
||||
->executeOne();
|
||||
|
||||
if (!$repository) {
|
||||
throw new PhabricatorEmptyQueryException();
|
||||
}
|
||||
|
||||
if ($repository->isSVN()) {
|
||||
throw new PhabricatorSearchConstraintException(
|
||||
pht(
|
||||
'Subversion does not support searching for ancestors of '.
|
||||
'a particular ref. This operation is not meaningful in '.
|
||||
'Subversion.'));
|
||||
}
|
||||
|
||||
if ($repository->isHg()) {
|
||||
throw new PhabricatorSearchConstraintException(
|
||||
pht(
|
||||
'Mercurial does not currently support searching for ancestors of '.
|
||||
'a particular ref.'));
|
||||
}
|
||||
|
||||
$can_constrain = true;
|
||||
$history_identifiers = array();
|
||||
foreach ($this->ancestorsOf as $key => $ref) {
|
||||
try {
|
||||
$raw_history = DiffusionQuery::callConduitWithDiffusionRequest(
|
||||
$viewer,
|
||||
DiffusionRequest::newFromDictionary(
|
||||
array(
|
||||
'repository' => $repository,
|
||||
'user' => $viewer,
|
||||
)),
|
||||
'diffusion.historyquery',
|
||||
array(
|
||||
'commit' => $ref,
|
||||
'limit' => $history_limit,
|
||||
));
|
||||
} catch (ConduitClientException $ex) {
|
||||
throw new PhabricatorSearchConstraintException(
|
||||
$ex->getMessage());
|
||||
}
|
||||
|
||||
$ref_identifiers = array();
|
||||
foreach ($raw_history['pathChanges'] as $change) {
|
||||
$ref_identifiers[] = $change['commitIdentifier'];
|
||||
}
|
||||
|
||||
// If this ref had fewer total commits than the limit, we're safe to
|
||||
// apply the constraint as a large `IN (...)` query for a list of
|
||||
// commit identifiers. This is efficient.
|
||||
if ($history_limit) {
|
||||
if (count($ref_identifiers) >= $history_limit) {
|
||||
$can_constrain = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
$history_identifiers += array_fuse($ref_identifiers);
|
||||
}
|
||||
|
||||
// If all refs had a small number of ancestors, we can just put the
|
||||
// constraint into the query here and we're done. Otherwise, we need
|
||||
// to filter each page after it comes out of the MySQL layer.
|
||||
if ($can_constrain) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'commit.commitIdentifier IN (%Ls)',
|
||||
$history_identifiers);
|
||||
} else {
|
||||
$this->mustFilterRefs = true;
|
||||
$this->refRepository = $repository;
|
||||
}
|
||||
}
|
||||
|
||||
if ($this->ids !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
|
|
Loading…
Reference in a new issue