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 @@ -183,6 +183,9 @@ ->withIDs($repository_ids) ->execute(); + $min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; + $result = array(); + foreach ($commits as $key => $commit) { $repo = idx($repos, $commit->getRepositoryID()); if ($repo) { @@ -190,36 +193,40 @@ } else { unset($commits[$key]); } - } - - if ($this->identifiers !== null) { - $ids = array_fuse($this->identifiers); - $min_qualified = PhabricatorRepository::MINIMUM_QUALIFIED_HASH; - $result = array(); - foreach ($commits as $commit) { - $prefix = 'r'.$commit->getRepository()->getCallsign(); + // Build the identifierMap + if ($this->identifiers !== null) { + $ids = array_fuse($this->identifiers); + $prefixes = array( + 'r'.$commit->getRepository()->getCallsign(), + 'r'.$commit->getRepository()->getCallsign().':', + 'r'.$commit->getRepository()->getID().':', + '', // No prefix is valid too and will only match the commitIdentifier + ); $suffix = $commit->getCommitIdentifier(); if ($commit->getRepository()->isSVN()) { - if (isset($ids[$prefix.$suffix])) { - $result[$prefix.$suffix][] = $commit; + foreach ($prefixes as $prefix) { + if (isset($ids[$prefix.$suffix])) { + $result[$prefix.$suffix][] = $commit; + } } } else { // This awkward construction is so we can link the commits up in O(N) // time instead of O(N^2). for ($ii = $min_qualified; $ii <= strlen($suffix); $ii++) { $part = substr($suffix, 0, $ii); - if (isset($ids[$prefix.$part])) { - $result[$prefix.$part][] = $commit; - } - if (isset($ids[$part])) { - $result[$part][] = $commit; + foreach ($prefixes as $prefix) { + if (isset($ids[$prefix.$part])) { + $result[$prefix.$part][] = $commit; + } } } } } + } + if ($result) { foreach ($result as $identifier => $matching_commits) { if (count($matching_commits) == 1) { $result[$identifier] = head($matching_commits); @@ -229,7 +236,6 @@ unset($result[$identifier]); } } - $this->identifierMap += $result; } @@ -327,9 +333,9 @@ $bare = array(); foreach ($this->identifiers as $identifier) { $matches = null; - preg_match('/^(?:r([A-Z]+))?(.*)$/', $identifier, $matches); - $repo = nonempty($matches[1], null); - $identifier = nonempty($matches[2], null); + preg_match('/^(?:r([A-Z]+:?|[0-9]+:))?(.*)$/', $identifier, $matches); + $repo = nonempty(rtrim($matches[1], ':'), null); + $commitIdentifier = nonempty($matches[2], null); if ($repo === null) { if ($this->defaultRepository) { @@ -338,14 +344,14 @@ } if ($repo === null) { - if (strlen($identifier) < $min_unqualified) { + if (strlen($commitIdentifier) < $min_unqualified) { continue; } - $bare[] = $identifier; + $bare[] = $commitIdentifier; } else { $refs[] = array( 'callsign' => $repo, - 'identifier' => $identifier, + 'identifier' => $commitIdentifier, ); } } @@ -362,14 +368,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'; }