diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -199,12 +199,19 @@ $result = array(); foreach ($commits as $commit) { $prefix = 'r'.$commit->getRepository()->getCallsign(); + $prefix_id = 'r'.$commit->getRepository()->getID().':'; $suffix = $commit->getCommitIdentifier(); if ($commit->getRepository()->isSVN()) { if (isset($ids[$prefix.$suffix])) { $result[$prefix.$suffix][] = $commit; } + if (isset($ids[$prefix.':'.$suffix])) { + $result[$prefix.':'.$suffix][] = $commit; + } + if (isset($ids[$prefix_id.$suffix])) { + $result[$prefix_id.$suffix][] = $commit; + } } else { // This awkward construction is so we can link the commits up in O(N) // time instead of O(N^2). @@ -213,6 +220,12 @@ if (isset($ids[$prefix.$part])) { $result[$prefix.$part][] = $commit; } + if (isset($ids[$prefix.':'.$part])) { + $result[$prefix.':'.$part][] = $commit; + } + if (isset($ids[$prefix_id.$part])) { + $result[$prefix_id.$part][] = $commit; + } if (isset($ids[$part])) { $result[$part][] = $commit; } @@ -327,8 +340,9 @@ $bare = array(); foreach ($this->identifiers as $identifier) { $matches = null; - preg_match('/^(?:r([A-Z]+))?(.*)$/', $identifier, $matches); - $repo = nonempty($matches[1], null); + // FIXME: Replace with CommitPHIDType::getCommitObjectNamePattern ?? + preg_match('/^(?:r([A-Z]+:?|[0-9]+:))?(.*)$/', $identifier, $matches); + $repo = nonempty(rtrim($matches[1], ':'), null); $identifier = nonempty($matches[2], null); if ($repo === null) { @@ -362,14 +376,17 @@ if ($refs) { $callsigns = ipull($refs, 'callsign'); + $repos = id(new PhabricatorRepositoryQuery()) ->setViewer($this->getViewer()) - ->withCallsigns($callsigns) - ->execute(); - $repos = mpull($repos, null, 'getCallsign'); + ->withIdentifiers($callsigns); + $repos->execute(); + + $repos = $repos->getIdentifierMap(); foreach ($refs as $key => $ref) { $repo = idx($repos, $ref['callsign']); + if (!$repo) { continue; } diff --git a/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php b/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php --- a/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php +++ b/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php @@ -22,7 +22,6 @@ ->withIdentifiers($ids); $query->execute(); - return $query->getIdentifierMap(); } diff --git a/src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php b/src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php --- a/src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php +++ b/src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php @@ -8,18 +8,18 @@ } protected function getObjectIDPattern() { - return '[A-Z]+'; + return '[A-Z0-9]+'; } protected function loadObjects(array $ids) { $viewer = $this->getEngine()->getConfig('viewer'); - $repositories = id(new PhabricatorRepositoryQuery()) + $repos = id(new PhabricatorRepositoryQuery()) ->setViewer($viewer) - ->withCallsigns($ids) - ->execute(); + ->withIdentifiers($ids); - return mpull($repositories, null, 'getCallsign'); + $repos->execute(); + return $repos->getIdentifierMap(); } } diff --git a/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php b/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php --- a/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php +++ b/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php @@ -54,9 +54,9 @@ $min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; return - 'r[A-Z]+[1-9]\d*'. + 'r([A-Z]+:?|[0-9]+:)[1-9]\d*'. '|'. - 'r[A-Z]+[a-f0-9]{'.$min_qualified.',40}'. + 'r([A-Z]+:?|[0-9]+:)[a-f0-9]{'.$min_qualified.',40}'. '|'. '[a-f0-9]{'.$min_unqualified.',40}'; } diff --git a/src/applications/repository/phid/PhabricatorRepositoryRepositoryPHIDType.php b/src/applications/repository/phid/PhabricatorRepositoryRepositoryPHIDType.php --- a/src/applications/repository/phid/PhabricatorRepositoryRepositoryPHIDType.php +++ b/src/applications/repository/phid/PhabricatorRepositoryRepositoryPHIDType.php @@ -44,33 +44,19 @@ } public function canLoadNamedObject($name) { - return preg_match('/^r[A-Z]+$/', $name); + return preg_match('/^r[A-Z0-9]+$/', $name); } public function loadNamedObjects( PhabricatorObjectQuery $query, array $names) { - $id_map = array(); - foreach ($names as $name) { - $id = substr($name, 1); - $id_map[$id][] = $name; - } - - $objects = id(new PhabricatorRepositoryQuery()) + $results = id(new PhabricatorRepositoryQuery()) ->setViewer($query->getViewer()) - ->withCallsigns(array_keys($id_map)) - ->execute(); - - $results = array(); - foreach ($objects as $object) { - $callsign = $object->getCallsign(); - foreach (idx($id_map, $callsign, array()) as $name) { - $results[$name] = $object; - } - } + ->withIdentifiers($names); + $results->execute(); - return $results; + return $results->getIdentifierMap(); } } diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php --- a/src/applications/repository/query/PhabricatorRepositoryQuery.php +++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php @@ -12,6 +12,9 @@ private $remoteURIs; private $anyProjectPHIDs; + private $identifierMap; + private $allowMultipleIDTypes = false; + const STATUS_OPEN = 'status-open'; const STATUS_CLOSED = 'status-closed'; const STATUS_ALL = 'status-all'; @@ -47,6 +50,27 @@ return $this; } + public function withIdentifiers(array $identifiers) { + $this->allowMultipleIDTypes = true; + $ids = array(); $callsigns = array(); + foreach ($identifiers as $identifier) { + // Strip the leading lowercase 'r' in the repo identifier if present + if ($identifier[0] == 'r') { + $identifier = substr($identifier, 1); + } + + if (ctype_digit($identifier)) { + $ids[$identifier] = $identifier; + } else { + // FIXME: Find out how to identify PHIDs and handle them here too + $callsigns[$identifier] = $identifier; + } + } + $this->ids = $ids; + $this->callsigns = $callsigns; + return $this; + } + public function withStatus($status) { $this->status = $status; return $this; @@ -102,6 +126,14 @@ return $this; } + public function getIdentifierMap() { + if ($this->identifierMap === null) { + throw new Exception( + 'You must execute() the query before accessing the identifier map.'); + } + return $this->identifierMap; + } + protected function loadPage() { $table = new PhabricatorRepository(); $conn_r = $table->establishConnection('r'); @@ -202,6 +234,37 @@ } } + // Build the identifierMap + $this->identifierMap = array(); + + if ($this->ids) { + foreach ($this->ids as $id) { + if (isset($repositories[$id])) { + $this->identifierMap[$id] = $repositories[$id]; + } + } + } + + if ($this->callsigns) { + $repository_callsigns = mpull($repositories, null, 'getCallsign'); + + foreach ($this->callsigns as $callsign) { + if (isset($repository_callsigns[$callsign])) { + $this->identifierMap[$callsign] = $repository_callsigns[$callsign]; + } + } + } + + if ($this->phids) { + $repository_phids = mpull($repositories, null, 'getPHID'); + + foreach ($this->phids as $phid) { + if (isset($repository_phids[$phid])) { + $this->identifierMap[$phid] = $repository_phids[$phid]; + } + } + } + return $repositories; } @@ -387,6 +450,12 @@ $this->callsigns); } + if ($this->allowMultipleIDTypes) { + // Connect all the existing where conditions with an OR + // Make sure not to insert any other where conditions above this! + $where = array('('.implode(' OR ', $where).')'); + } + if ($this->types) { $where[] = qsprintf( $conn_r, @@ -420,7 +489,6 @@ return $this->formatWhereClause($where); } - public function getQueryApplicationClass() { return 'PhabricatorDiffusionApplication'; }