diff --git a/src/applications/diffusion/controller/DiffusionExternalController.php b/src/applications/diffusion/controller/DiffusionExternalController.php index 8faa9275d1..b62ee94e27 100644 --- a/src/applications/diffusion/controller/DiffusionExternalController.php +++ b/src/applications/diffusion/controller/DiffusionExternalController.php @@ -45,13 +45,13 @@ final class DiffusionExternalController extends DiffusionController { if ($best_match) { $repository = $repositories[$best_match]; - $redirect = DiffusionRequest::generateDiffusionURI( + $redirect = $repository->generateURI( array( - 'action' => 'browse', - 'repository' => $repository, - 'branch' => $repository->getDefaultBranch(), - 'commit' => $id, + 'action' => 'browse', + 'branch' => $repository->getDefaultBranch(), + 'commit' => $id, )); + return id(new AphrontRedirectResponse())->setURI($redirect); } } @@ -83,10 +83,9 @@ final class DiffusionExternalController extends DiffusionController { } else if (count($commits) == 1) { $commit = head($commits); $repo = $repositories[$commit->getRepositoryID()]; - $redirect = DiffusionRequest::generateDiffusionURI( + $redirect = $repo->generateURI( array( 'action' => 'browse', - 'repository' => $repo, 'branch' => $repo->getDefaultBranch(), 'commit' => $commit->getCommitIdentifier(), )); @@ -96,10 +95,9 @@ final class DiffusionExternalController extends DiffusionController { $rows = array(); foreach ($commits as $commit) { $repo = $repositories[$commit->getRepositoryID()]; - $href = DiffusionRequest::generateDiffusionURI( + $href = $repo->generateURI( array( 'action' => 'browse', - 'repository' => $repo, 'branch' => $repo->getDefaultBranch(), 'commit' => $commit->getCommitIdentifier(), )); diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php index 9df5efc2c8..5f10617105 100644 --- a/src/applications/diffusion/request/DiffusionRequest.php +++ b/src/applications/diffusion/request/DiffusionRequest.php @@ -448,15 +448,6 @@ abstract class DiffusionRequest extends Phobject { /* -( Managing Diffusion URIs )-------------------------------------------- */ - /** - * Generate a Diffusion URI using this request to provide defaults. See - * @{method:generateDiffusionURI} for details. This method is the same, but - * preserves the request parameters if they are not overridden. - * - * @param map See @{method:generateDiffusionURI}. - * @return PhutilURI Generated URI. - * @task uri - */ public function generateURI(array $params) { if (empty($params['stable'])) { $default_commit = $this->getSymbolicCommit(); @@ -465,181 +456,21 @@ abstract class DiffusionRequest extends Phobject { } $defaults = array( - 'repository' => $this->getRepository(), 'path' => $this->getPath(), 'branch' => $this->getBranch(), 'commit' => $default_commit, 'lint' => idx($params, 'lint', $this->getLint()), ); + foreach ($defaults as $key => $val) { if (!isset($params[$key])) { // Overwrite NULL. $params[$key] = $val; } } - return self::generateDiffusionURI($params); + + return $this->getRepository()->generateURI($params); } - - /** - * Generate a Diffusion URI from a parameter map. Applies the correct encoding - * and formatting to the URI. Parameters are: - * - * - `action` One of `history`, `browse`, `change`, `lastmodified`, - * `branch`, `tags`, `branches`, or `revision-ref`. The action specified - * by the URI. - * - `repository` Repository. - * - `callsign` Repository callsign. - * - `branch` Optional if action is not `branch`, branch name. - * - `path` Optional, path to file. - * - `commit` Optional, commit identifier. - * - `line` Optional, line range. - * - `lint` Optional, lint code. - * - `params` Optional, query parameters. - * - * The function generates the specified URI and returns it. - * - * @param map See documentation. - * @return PhutilURI Generated URI. - * @task uri - */ - public static function generateDiffusionURI(array $params) { - $action = idx($params, 'action'); - - $repository = idx($params, 'repository'); - - if ($repository) { - $callsign = $repository->getCallsign(); - } else { - $callsign = idx($params, 'callsign'); - } - - $path = idx($params, 'path'); - $branch = idx($params, 'branch'); - $commit = idx($params, 'commit'); - $line = idx($params, 'line'); - - if (strlen($callsign)) { - $callsign = phutil_escape_uri_path_component($callsign).'/'; - } - - if (strlen($branch)) { - $branch = phutil_escape_uri_path_component($branch).'/'; - } - - if (strlen($path)) { - $path = ltrim($path, '/'); - $path = str_replace(array(';', '$'), array(';;', '$$'), $path); - $path = phutil_escape_uri($path); - } - - $path = "{$branch}{$path}"; - - if (strlen($commit)) { - $commit = str_replace('$', '$$', $commit); - $commit = ';'.phutil_escape_uri($commit); - } - - if (strlen($line)) { - $line = '$'.phutil_escape_uri($line); - } - - $req_callsign = false; - $req_branch = false; - $req_commit = false; - - switch ($action) { - case 'history': - case 'browse': - case 'change': - case 'lastmodified': - case 'tags': - case 'branches': - case 'lint': - case 'refs': - $req_callsign = true; - break; - case 'branch': - $req_callsign = true; - $req_branch = true; - break; - case 'commit': - $req_callsign = true; - $req_commit = true; - break; - } - - if ($req_callsign && !strlen($callsign)) { - throw new Exception( - pht( - "Diffusion URI action '%s' requires callsign!", - $action)); - } - - if ($req_commit && !strlen($commit)) { - throw new Exception( - pht( - "Diffusion URI action '%s' requires commit!", - $action)); - } - - switch ($action) { - case 'change': - case 'history': - case 'browse': - case 'lastmodified': - case 'tags': - case 'branches': - case 'lint': - case 'pathtree': - case 'refs': - $uri = "/diffusion/{$callsign}{$action}/{$path}{$commit}{$line}"; - break; - case 'branch': - if (strlen($path)) { - $uri = "/diffusion/{$callsign}repository/{$path}"; - } else { - $uri = "/diffusion/{$callsign}"; - } - break; - case 'external': - $commit = ltrim($commit, ';'); - $uri = "/diffusion/external/{$commit}/"; - break; - case 'rendering-ref': - // This isn't a real URI per se, it's passed as a query parameter to - // the ajax changeset stuff but then we parse it back out as though - // it came from a URI. - $uri = rawurldecode("{$path}{$commit}"); - break; - case 'commit': - $commit = ltrim($commit, ';'); - $callsign = rtrim($callsign, '/'); - $uri = "/r{$callsign}{$commit}"; - break; - default: - throw new Exception(pht("Unknown Diffusion URI action '%s'!", $action)); - } - - if ($action == 'rendering-ref') { - return $uri; - } - - $uri = new PhutilURI($uri); - - if (isset($params['lint'])) { - $params['params'] = idx($params, 'params', array()) + array( - 'lint' => $params['lint'], - ); - } - - if (idx($params, 'params')) { - $uri->setQueryParams($params['params']); - } - - return $uri; - } - - /** * Internal. Public only for unit tests. * diff --git a/src/applications/diffusion/request/__tests__/DiffusionURITestCase.php b/src/applications/diffusion/request/__tests__/DiffusionURITestCase.php index 43a2bd3404..b7899de9cd 100644 --- a/src/applications/diffusion/request/__tests__/DiffusionURITestCase.php +++ b/src/applications/diffusion/request/__tests__/DiffusionURITestCase.php @@ -86,10 +86,15 @@ final class DiffusionURITestCase extends PhutilTestCase { } public function testURIGeneration() { + $actor = PhabricatorUser::getOmnipotentUser(); + + $repository = PhabricatorRepository::initializeNewRepository($actor) + ->setCallsign('A') + ->makeEphemeral(); + $map = array( '/diffusion/A/browse/branch/path.ext;abc$1' => array( 'action' => 'browse', - 'callsign' => 'A', 'branch' => 'branch', 'path' => 'path.ext', 'commit' => 'abc', @@ -97,24 +102,20 @@ final class DiffusionURITestCase extends PhutilTestCase { ), '/diffusion/A/browse/a%252Fb/path.ext' => array( 'action' => 'browse', - 'callsign' => 'A', 'branch' => 'a/b', 'path' => 'path.ext', ), '/diffusion/A/browse/%2B/%20%21' => array( 'action' => 'browse', - 'callsign' => 'A', 'path' => '+/ !', ), '/diffusion/A/browse/money/%24%24100$2' => array( 'action' => 'browse', - 'callsign' => 'A', 'path' => 'money/$100', 'line' => '2', ), '/diffusion/A/browse/path/to/file.ext?view=things' => array( 'action' => 'browse', - 'callsign' => 'A', 'path' => 'path/to/file.ext', 'params' => array( 'view' => 'things', @@ -122,7 +123,6 @@ final class DiffusionURITestCase extends PhutilTestCase { ), '/diffusion/A/repository/master/' => array( 'action' => 'branch', - 'callsign' => 'A', 'branch' => 'master', ), 'path/to/file.ext;abc' => array( @@ -132,7 +132,6 @@ final class DiffusionURITestCase extends PhutilTestCase { ), '/diffusion/A/browse/branch/path.ext$3-5%2C7-12%2C14' => array( 'action' => 'browse', - 'callsign' => 'A', 'branch' => 'branch', 'path' => 'path.ext', 'line' => '3-5,7-12,14', @@ -140,10 +139,8 @@ final class DiffusionURITestCase extends PhutilTestCase { ); foreach ($map as $expect => $input) { - $actual = DiffusionRequest::generateDiffusionURI($input); - $this->assertEqual( - $expect, - (string)$actual); + $actual = $repository->generateURI($input); + $this->assertEqual($expect, (string)$actual); } } diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index f87a4bb59b..7565da6287 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -268,9 +268,8 @@ final class PhabricatorOwnersDetailController if (!$repo) { continue; } - $href = DiffusionRequest::generateDiffusionURI( + $href = $repo->generateURI( array( - 'repository' => $repo, 'branch' => $repo->getDefaultBranch(), 'path' => $path->getPath(), 'action' => 'browse', diff --git a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php index 1e15228630..0587e314b7 100644 --- a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php +++ b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php @@ -175,9 +175,8 @@ final class PhabricatorRepositorySearchEngine $size = $repository->getCommitCount(); if ($size) { - $history_uri = DiffusionRequest::generateDiffusionURI( + $history_uri = $repository->generateURI( array( - 'repository' => $repository, 'action' => 'history', )); diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 6996571b94..97b9684d0b 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -610,6 +610,141 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return "/r{$callsign}{$identifier}"; } + public function generateURI(array $params) { + $req_branch = false; + $req_commit = false; + + $action = idx($params, 'action'); + switch ($action) { + case 'history': + case 'browse': + case 'change': + case 'lastmodified': + case 'tags': + case 'branches': + case 'lint': + case 'pathtree': + case 'refs': + break; + case 'branch': + $req_branch = true; + break; + case 'commit': + case 'rendering-ref': + $req_commit = true; + break; + default: + throw new Exception( + pht( + 'Action "%s" is not a valid repository URI action.', + $action)); + } + + $path = idx($params, 'path'); + $branch = idx($params, 'branch'); + $commit = idx($params, 'commit'); + $line = idx($params, 'line'); + + if ($req_commit && !strlen($commit)) { + throw new Exception( + pht( + 'Diffusion URI action "%s" requires commit!', + $action)); + } + + if ($req_branch && !strlen($branch)) { + throw new Exception( + pht( + 'Diffusion URI action "%s" requires branch!', + $action)); + } + + if ($action === 'commit') { + return $this->getCommitURI($commit); + } + + + $identifier = $this->getID(); + + $callsign = $this->getCallsign(); + if ($callsign !== null) { + $identifier = $callsign; + } + + if (strlen($identifier)) { + $identifier = phutil_escape_uri_path_component($identifier); + } + + if (strlen($path)) { + $path = ltrim($path, '/'); + $path = str_replace(array(';', '$'), array(';;', '$$'), $path); + $path = phutil_escape_uri($path); + } + + if (strlen($branch)) { + $branch = phutil_escape_uri_path_component($branch); + $path = "{$branch}/{$path}"; + } + + if (strlen($commit)) { + $commit = str_replace('$', '$$', $commit); + $commit = ';'.phutil_escape_uri($commit); + } + + if (strlen($line)) { + $line = '$'.phutil_escape_uri($line); + } + + switch ($action) { + case 'change': + case 'history': + case 'browse': + case 'lastmodified': + case 'tags': + case 'branches': + case 'lint': + case 'pathtree': + case 'refs': + $uri = "/diffusion/{$identifier}/{$action}/{$path}{$commit}{$line}"; + break; + case 'branch': + if (strlen($path)) { + $uri = "/diffusion/{$identifier}/repository/{$path}"; + } else { + $uri = "/diffusion/{$identifier}/"; + } + break; + case 'external': + $commit = ltrim($commit, ';'); + $uri = "/diffusion/external/{$commit}/"; + break; + case 'rendering-ref': + // This isn't a real URI per se, it's passed as a query parameter to + // the ajax changeset stuff but then we parse it back out as though + // it came from a URI. + $uri = rawurldecode("{$path}{$commit}"); + break; + } + + if ($action == 'rendering-ref') { + return $uri; + } + + $uri = new PhutilURI($uri); + + if (isset($params['lint'])) { + $params['params'] = idx($params, 'params', array()) + array( + 'lint' => $params['lint'], + ); + } + + if (idx($params, 'params')) { + $uri->setQueryParams($params['params']); + } + + return $uri; + } + public function getNormalizedPath() { $uri = (string)$this->getCloneURIObject();