Changeset View
Changeset View
Standalone View
Standalone View
src/applications/diffusion/query/DiffusionCommitQuery.php
Show First 20 Lines • Show All 177 Lines • ▼ Show 20 Lines | final class DiffusionCommitQuery | ||||
protected function willFilterPage(array $commits) { | protected function willFilterPage(array $commits) { | ||||
$repository_ids = mpull($commits, 'getRepositoryID', 'getRepositoryID'); | $repository_ids = mpull($commits, 'getRepositoryID', 'getRepositoryID'); | ||||
$repos = id(new PhabricatorRepositoryQuery()) | $repos = id(new PhabricatorRepositoryQuery()) | ||||
->setViewer($this->getViewer()) | ->setViewer($this->getViewer()) | ||||
->withIDs($repository_ids) | ->withIDs($repository_ids) | ||||
->execute(); | ->execute(); | ||||
$min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; | |||||
$result = array(); | |||||
foreach ($commits as $key => $commit) { | foreach ($commits as $key => $commit) { | ||||
$repo = idx($repos, $commit->getRepositoryID()); | $repo = idx($repos, $commit->getRepositoryID()); | ||||
if ($repo) { | if ($repo) { | ||||
$commit->attachRepository($repo); | $commit->attachRepository($repo); | ||||
} else { | } else { | ||||
unset($commits[$key]); | unset($commits[$key]); | ||||
} | } | ||||
} | |||||
// Build the identifierMap | |||||
if ($this->identifiers !== null) { | if ($this->identifiers !== null) { | ||||
$ids = array_fuse($this->identifiers); | $ids = array_fuse($this->identifiers); | ||||
$min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; | $prefixes = array( | ||||
'r'.$commit->getRepository()->getCallsign(), | |||||
$result = array(); | 'r'.$commit->getRepository()->getCallsign().':', | ||||
foreach ($commits as $commit) { | 'R'.$commit->getRepository()->getID().':', | ||||
$prefix = 'r'.$commit->getRepository()->getCallsign(); | '', // No prefix is valid too and will only match the commitIdentifier | ||||
); | |||||
$suffix = $commit->getCommitIdentifier(); | $suffix = $commit->getCommitIdentifier(); | ||||
if ($commit->getRepository()->isSVN()) { | if ($commit->getRepository()->isSVN()) { | ||||
foreach ($prefixes as $prefix) { | |||||
if (isset($ids[$prefix.$suffix])) { | if (isset($ids[$prefix.$suffix])) { | ||||
$result[$prefix.$suffix][] = $commit; | $result[$prefix.$suffix][] = $commit; | ||||
} | } | ||||
} | |||||
} else { | } else { | ||||
// This awkward construction is so we can link the commits up in O(N) | // This awkward construction is so we can link the commits up in O(N) | ||||
// time instead of O(N^2). | // time instead of O(N^2). | ||||
for ($ii = $min_qualified; $ii <= strlen($suffix); $ii++) { | for ($ii = $min_qualified; $ii <= strlen($suffix); $ii++) { | ||||
$part = substr($suffix, 0, $ii); | $part = substr($suffix, 0, $ii); | ||||
foreach ($prefixes as $prefix) { | |||||
if (isset($ids[$prefix.$part])) { | if (isset($ids[$prefix.$part])) { | ||||
$result[$prefix.$part][] = $commit; | $result[$prefix.$part][] = $commit; | ||||
} | } | ||||
if (isset($ids[$part])) { | } | ||||
$result[$part][] = $commit; | |||||
} | } | ||||
} | } | ||||
} | } | ||||
} | } | ||||
if ($result) { | |||||
foreach ($result as $identifier => $matching_commits) { | foreach ($result as $identifier => $matching_commits) { | ||||
if (count($matching_commits) == 1) { | if (count($matching_commits) == 1) { | ||||
$result[$identifier] = head($matching_commits); | $result[$identifier] = head($matching_commits); | ||||
} else { | } else { | ||||
// This reference is ambiguous (it matches more than one commit) so | // This reference is ambiguous (it matches more than one commit) so | ||||
// don't link it. | // don't link it. | ||||
unset($result[$identifier]); | unset($result[$identifier]); | ||||
} | } | ||||
} | } | ||||
$this->identifierMap += $result; | $this->identifierMap += $result; | ||||
} | } | ||||
return $commits; | return $commits; | ||||
} | } | ||||
protected function didFilterPage(array $commits) { | protected function didFilterPage(array $commits) { | ||||
if ($this->needCommitData) { | if ($this->needCommitData) { | ||||
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Lines | private function buildWhereClause(AphrontDatabaseConnection $conn_r) { | ||||
if ($this->identifiers !== null) { | if ($this->identifiers !== null) { | ||||
$min_unqualified = PhabricatorRepository::MINIMUM_UNQUALIFIED_HASH; | $min_unqualified = PhabricatorRepository::MINIMUM_UNQUALIFIED_HASH; | ||||
$min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; | $min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; | ||||
$refs = array(); | $refs = array(); | ||||
$bare = array(); | $bare = array(); | ||||
foreach ($this->identifiers as $identifier) { | foreach ($this->identifiers as $identifier) { | ||||
$matches = null; | $matches = null; | ||||
preg_match('/^(?:r([A-Z]+))?(.*)$/', $identifier, $matches); | preg_match('/^(?:[rR]([A-Z]+:?|[0-9]+:))?(.*)$/', | ||||
$repo = nonempty($matches[1], null); | $identifier, $matches); | ||||
$identifier = nonempty($matches[2], null); | $repo = nonempty(rtrim($matches[1], ':'), null); | ||||
epriestley: Prefer `$commit_identifier`, for consistency.
This is actually a bug with lint, which should… | |||||
$commit_identifier = nonempty($matches[2], null); | |||||
if ($repo === null) { | if ($repo === null) { | ||||
if ($this->defaultRepository) { | if ($this->defaultRepository) { | ||||
$repo = $this->defaultRepository->getCallsign(); | $repo = $this->defaultRepository->getCallsign(); | ||||
} | } | ||||
} | } | ||||
if ($repo === null) { | if ($repo === null) { | ||||
if (strlen($identifier) < $min_unqualified) { | if (strlen($commit_identifier) < $min_unqualified) { | ||||
continue; | continue; | ||||
} | } | ||||
$bare[] = $identifier; | $bare[] = $commit_identifier; | ||||
} else { | } else { | ||||
$refs[] = array( | $refs[] = array( | ||||
'callsign' => $repo, | 'callsign' => $repo, | ||||
'identifier' => $identifier, | 'identifier' => $commit_identifier, | ||||
); | ); | ||||
} | } | ||||
} | } | ||||
$sql = array(); | $sql = array(); | ||||
foreach ($bare as $identifier) { | foreach ($bare as $identifier) { | ||||
$sql[] = qsprintf( | $sql[] = qsprintf( | ||||
$conn_r, | $conn_r, | ||||
'(commit.commitIdentifier LIKE %> AND '. | '(commit.commitIdentifier LIKE %> AND '. | ||||
'LENGTH(commit.commitIdentifier) = 40)', | 'LENGTH(commit.commitIdentifier) = 40)', | ||||
$identifier); | $identifier); | ||||
} | } | ||||
if ($refs) { | if ($refs) { | ||||
$callsigns = ipull($refs, 'callsign'); | $callsigns = ipull($refs, 'callsign'); | ||||
$repos = id(new PhabricatorRepositoryQuery()) | $repos = id(new PhabricatorRepositoryQuery()) | ||||
->setViewer($this->getViewer()) | ->setViewer($this->getViewer()) | ||||
->withCallsigns($callsigns) | ->withIdentifiers($callsigns); | ||||
->execute(); | $repos->execute(); | ||||
$repos = mpull($repos, null, 'getCallsign'); | |||||
$repos = $repos->getIdentifierMap(); | |||||
Not Done Inline ActionsConsider ctype_digit() instead of is_numeric because values like "0.0", "1e5", and "-3" satisfy is_numeric(). (I think it doesn't matter in this case, but it's nice to do the stricter check when we have stricter expectations about the input.) epriestley: Consider `ctype_digit()` instead of `is_numeric` because values like "0.0", "1e5", and "-3"… | |||||
foreach ($refs as $key => $ref) { | foreach ($refs as $key => $ref) { | ||||
$repo = idx($repos, $ref['callsign']); | $repo = idx($repos, $ref['callsign']); | ||||
Not Done Inline ActionsWe should check for if ($callsigns) and if ($ids) before issuing these queries. epriestley: We should check for `if ($callsigns)` and `if ($ids)` before issuing these queries. | |||||
if (!$repo) { | if (!$repo) { | ||||
continue; | continue; | ||||
} | } | ||||
if ($repo->isSVN()) { | if ($repo->isSVN()) { | ||||
if (!ctype_digit($ref['identifier'])) { | if (!ctype_digit($ref['identifier'])) { | ||||
continue; | continue; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 175 Lines • Show Last 20 Lines |
Prefer $commit_identifier, for consistency.
This is actually a bug with lint, which should have caught this automatically. D11087 should have a fix for it.