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('/^(?:r([A-Z]+:?|[0-9]+:))?(.*)$/', $identifier, $matches); | ||||
| $repo = nonempty($matches[1], null); | $repo = nonempty(rtrim($matches[1], ':'), null); | ||||
| $identifier = nonempty($matches[2], null); | $commit_identifier = nonempty($matches[2], null); | ||||
epriestley: Prefer `$commit_identifier`, for consistency.
This is actually a bug with lint, which should… | |||||
| 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.