Move generateDiffusionURI() into PhabricatorRepository

Summary: Ref T4245. This further reduces the reliance on callsigns in Diffusion.

Test Plan:
  - Pretty reasonable test coverage already exists.
  - Browsed repository list, browse view, history view, content view, change view, commit view, tag view, branch view of repositories.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

Differential Revision: https://secure.phabricator.com/D14937

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',
+            'action' => 'browse',
-            'repository'  => $repository,
+            'branch' => $repository->getDefaultBranch(),
-            'branch'    => $repository->getDefaultBranch(),
+            'commit' => $id,
-            '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(),
           ));

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.
    *

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);
+      $actual = $repository->generateURI($input);
-      $this->assertEqual(
+      $this->assertEqual($expect, (string)$actual);
-        $expect,
-        (string)$actual);
     }
   }
 

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',

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',
           ));
 

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();