diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -552,6 +552,7 @@ 'DiffusionRefNotFoundException' => 'applications/diffusion/exception/DiffusionRefNotFoundException.php', 'DiffusionRefsQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php', 'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/DiffusionRenameHistoryQuery.php', + 'DiffusionRepositoryByIDRemarkupRule' => 'applications/diffusion/remarkup/DiffusionRepositoryByIDRemarkupRule.php', 'DiffusionRepositoryController' => 'applications/diffusion/controller/DiffusionRepositoryController.php', 'DiffusionRepositoryCreateController' => 'applications/diffusion/controller/DiffusionRepositoryCreateController.php', 'DiffusionRepositoryDatasource' => 'applications/diffusion/typeahead/DiffusionRepositoryDatasource.php', @@ -3594,6 +3595,7 @@ 'DiffusionReadmeView' => 'DiffusionView', 'DiffusionRefNotFoundException' => 'Exception', 'DiffusionRefsQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', + 'DiffusionRepositoryByIDRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'DiffusionRepositoryController' => 'DiffusionController', 'DiffusionRepositoryCreateController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryDatasource' => 'PhabricatorTypeaheadDatasource', diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -40,8 +40,9 @@ public function getRemarkupRules() { return array( - new DiffusionRepositoryRemarkupRule(), new DiffusionCommitRemarkupRule(), + new DiffusionRepositoryRemarkupRule(), + new DiffusionRepositoryByIDRemarkupRule(), ); } 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,10 @@ $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('/^(?:[rR]([A-Z]+:?|[0-9]+:))?(.*)$/', + $identifier, $matches); + $repo = nonempty(rtrim($matches[1], ':'), null); + $commit_identifier = nonempty($matches[2], null); if ($repo === null) { if ($this->defaultRepository) { @@ -338,14 +345,14 @@ } if ($repo === null) { - if (strlen($identifier) < $min_unqualified) { + if (strlen($commit_identifier) < $min_unqualified) { continue; } - $bare[] = $identifier; + $bare[] = $commit_identifier; } else { $refs[] = array( 'callsign' => $repo, - 'identifier' => $identifier, + 'identifier' => $commit_identifier, ); } } @@ -362,14 +369,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/DiffusionRepositoryByIDRemarkupRule.php copy from src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php copy to src/applications/diffusion/remarkup/DiffusionRepositoryByIDRemarkupRule.php --- a/src/applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php +++ b/src/applications/diffusion/remarkup/DiffusionRepositoryByIDRemarkupRule.php @@ -1,25 +1,29 @@ 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/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 @@ -11,15 +11,19 @@ return '[A-Z]+'; } + public function getPriority() { + return 460.0; + } + 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/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php b/src/applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php --- a/src/applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php +++ b/src/applications/diffusion/remarkup/__tests__/DiffusionCommitRemarkupRuleTestCase.php @@ -48,6 +48,79 @@ ), ), ), + '{rP:1234 key=value}' => array( + 'embed' => array( + array( + 'offset' => 1, + 'id' => 'rP:1234', + 'tail' => ' key=value', + ), + ), + 'ref' => array( + array( + 'offset' => 1, + 'id' => 'rP:1234', + ), + ), + ), + '{R123:1234 key=value}' => array( + 'embed' => array( + array( + 'offset' => 1, + 'id' => 'R123:1234', + 'tail' => ' key=value', + ), + ), + 'ref' => array( + array( + 'offset' => 1, + 'id' => 'R123:1234', + ), + ), + ), + '{rP:12f3f6d3a9ef9c7731051815846810cb3c4cd248}' => array( + 'embed' => array( + array( + 'offset' => 1, + 'id' => 'rP:12f3f6d3a9ef9c7731051815846810cb3c4cd248', + ), + ), + 'ref' => array( + array( + 'offset' => 1, + 'id' => 'rP:12f3f6d3a9ef9c7731051815846810cb3c4cd248', + ), + ), + ), + '{R123:12f3f6d3a9ef9c7731051815846810cb3c4cd248}' => array( + 'embed' => array( + array( + 'offset' => 1, + 'id' => 'R123:12f3f6d3a9ef9c7731051815846810cb3c4cd248', + ), + ), + 'ref' => array( + array( + 'offset' => 1, + 'id' => 'R123:12f3f6d3a9ef9c7731051815846810cb3c4cd248', + ), + ), + ), + '{R123:12f3f6d3a9ef9c7731051815846810cb3c4cd248, key=value}' => array( + 'embed' => array( + array( + 'offset' => 1, + 'id' => 'R123:12f3f6d3a9ef9c7731051815846810cb3c4cd248', + 'tail' => ', key=value', + ), + ), + 'ref' => array( + array( + 'offset' => 1, + 'id' => 'R123:12f3f6d3a9ef9c7731051815846810cb3c4cd248', + ), + ), + ), ); foreach ($cases as $input => $expect) { 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]+:?|R[0-9]+:)[1-9]\d*'. '|'. - 'r[A-Z]+[a-f0-9]{'.$min_qualified.',40}'. + '(?:r[A-Z]+:?|R[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,35 @@ } public function canLoadNamedObject($name) { - return preg_match('/^r[A-Z]+$/', $name); + return preg_match('/^r[A-Z]+|R[0-9]+$/', $name); } public function loadNamedObjects( PhabricatorObjectQuery $query, array $names) { + $results = array(); $id_map = array(); - foreach ($names as $name) { + foreach ($names as $key => $name) { $id = substr($name, 1); $id_map[$id][] = $name; + $names[$key] = substr($name, 1); } - $objects = id(new PhabricatorRepositoryQuery()) + $query = id(new PhabricatorRepositoryQuery()) ->setViewer($query->getViewer()) - ->withCallsigns(array_keys($id_map)) - ->execute(); + ->withIdentifiers($names); - $results = array(); - foreach ($objects as $object) { - $callsign = $object->getCallsign(); - foreach (idx($id_map, $callsign, array()) as $name) { - $results[$name] = $object; + if ($query->execute()) { + $objects = $query->getIdentifierMap(); + foreach ($objects as $key => $object) { + foreach (idx($id_map, $key, array()) as $name) + $results[$name] = $object; } + return $results; + } else { + return array(); } - - return $results; } } 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,12 @@ private $remoteURIs; private $anyProjectPHIDs; + private $numericIdentifiers; + private $callsignIdentifiers; + private $phidIdentifiers; + + private $identifierMap; + const STATUS_OPEN = 'status-open'; const STATUS_CLOSED = 'status-closed'; const STATUS_ALL = 'status-all'; @@ -47,6 +53,27 @@ return $this; } + public function withIdentifiers(array $identifiers) { + $ids = array(); $callsigns = array(); $phids = array(); + foreach ($identifiers as $identifier) { + if (ctype_digit($identifier)) { + $ids[$identifier] = $identifier; + } else { + $repository_type = PhabricatorRepositoryRepositoryPHIDType::TYPECONST; + if (phid_get_type($identifier) === $repository_type) { + $phids[$identifier] = $identifier; + } else { + $callsigns[$identifier] = $identifier; + } + } + } + + $this->numericIdentifiers = $ids; + $this->callsignIdentifiers = $callsigns; + $this->phidIdentifiers = $phids; + return $this; + } + public function withStatus($status) { $this->status = $status; return $this; @@ -102,10 +129,22 @@ 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'); + if ($this->identifierMap === null) { + $this->identifierMap = array(); + } + $data = queryfx_all( $conn_r, 'SELECT * FROM %T r %Q %Q %Q %Q', @@ -202,6 +241,35 @@ } } + // Build the identifierMap + if ($this->numericIdentifiers) { + foreach ($this->numericIdentifiers as $id) { + if (isset($repositories[$id])) { + $this->identifierMap[$id] = $repositories[$id]; + } + } + } + + if ($this->callsignIdentifiers) { + $repository_callsigns = mpull($repositories, null, 'getCallsign'); + + foreach ($this->callsignIdentifiers as $callsign) { + if (isset($repository_callsigns[$callsign])) { + $this->identifierMap[$callsign] = $repository_callsigns[$callsign]; + } + } + } + + if ($this->phidIdentifiers) { + $repository_phids = mpull($repositories, null, 'getPHID'); + + foreach ($this->phidIdentifiers as $phid) { + if (isset($repository_phids[$phid])) { + $this->identifierMap[$phid] = $repository_phids[$phid]; + } + } + } + return $repositories; } @@ -387,6 +455,35 @@ $this->callsigns); } + if ($this->numericIdentifiers || + $this->callsignIdentifiers || + $this->phidIdentifiers) { + $identifier_clause = array(); + + if ($this->numericIdentifiers) { + $identifier_clause[] = qsprintf( + $conn_r, + 'r.id IN (%Ld)', + $this->numericIdentifiers); + } + + if ($this->callsignIdentifiers) { + $identifier_clause[] = qsprintf( + $conn_r, + 'r.callsign IN (%Ls)', + $this->callsignIdentifiers); + } + + if ($this->phidIdentifiers) { + $identifier_clause[] = qsprintf( + $conn_r, + 'r.phid IN (%Ls)', + $this->phidIdentifiers); + } + + $where = array('('.implode(' OR ', $identifier_clause).')'); + } + if ($this->types) { $where[] = qsprintf( $conn_r, @@ -420,7 +517,6 @@ return $this->formatWhereClause($where); } - public function getQueryApplicationClass() { return 'PhabricatorDiffusionApplication'; }