Support ID-based repository URIs, and canonicalize repository URIs

Summary:
Ref T4245. Make `/diffusion/123/` work, but redirect the user to `/diffusion/XYZ/` if the repository has a callsign.

(Right now, every repository has a callsign, so this always redirects.)

Also redirect `/R123:abcdef` if the repository has a callsign.

Also also, move the Pull garbage collector somewhere more sensible.

Test Plan:
  - Added test coverage.
  - Visited `/diffusion/1/`, was redirected.
  - Visited `/diffusion/R1:abcdef`, was redirected.
  - Browsed Diffusion normally.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

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

src/__phutil_library_map__.php

@@ -689,7 +689,7 @@ phutil_register_library_map(array(
     'DiffusionPreCommitRefRepositoryHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefRepositoryHeraldField.php',
     'DiffusionPreCommitRefRepositoryProjectsHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefRepositoryProjectsHeraldField.php',
     'DiffusionPreCommitRefTypeHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefTypeHeraldField.php',
-    'DiffusionPullEventGarbageCollector' => 'applications/diffusion/DiffusionPullEventGarbageCollector.php',
+    'DiffusionPullEventGarbageCollector' => 'applications/diffusion/garbagecollector/DiffusionPullEventGarbageCollector.php',
     'DiffusionPushCapability' => 'applications/diffusion/capability/DiffusionPushCapability.php',
     'DiffusionPushEventViewController' => 'applications/diffusion/controller/DiffusionPushEventViewController.php',
     'DiffusionPushLogController' => 'applications/diffusion/controller/DiffusionPushLogController.php',

src/applications/diffusion/application/PhabricatorDiffusionApplication.php

@@ -64,7 +64,11 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication {
           '(?:query/(?P<queryKey>[^/]+)/)?' => 'DiffusionPushLogListController',
           'view/(?P<id>\d+)/' => 'DiffusionPushEventViewController',
         ),
-        '(?P<repositoryCallsign>[A-Z]+)/' => array(
+        '(?:'.
+          '(?P<repositoryCallsign>[A-Z]+)'.
+          '|'.
+          '(?P<repositoryID>[1-9]\d*)'.
+        ')/' => array(
           '' => 'DiffusionRepositoryController',
 
           'repository/(?P<dblob>.*)'    => 'DiffusionRepositoryController',
@@ -115,8 +119,9 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication {
         // catch-all for serving repositories over HTTP. We must accept
         // requests without the trailing "/" because SVN commands don't
         // necessarily include it.
-        '(?P<repositoryCallsign>[A-Z]+)(?:/.*)?' =>
+        '(?:(?P<repositoryCallsign>[A-Z]+)|(?P<repositoryID>[1-9]\d*))'.
-          'DiffusionRepositoryDefaultController',
+          '(?:/.*)?'
+          => 'DiffusionRepositoryDefaultController',
 
         'inline/' => array(
           'edit/(?P<phid>[^/]+)/' => 'DiffusionInlineCommentController',

src/applications/diffusion/controller/DiffusionController.php

@@ -64,6 +64,20 @@ abstract class DiffusionController extends PhabricatorController {
       return new Aphront404Response();
     }
 
+    // If the client is making a request like "/diffusion/1/...", but the
+    // repository has a different canonical path like "/diffusion/XYZ/...",
+    // redirect them to the canonical path.
+
+    $request_path = $request->getPath();
+    $repository = $drequest->getRepository();
+
+    $canonical_path = $repository->getCanonicalPath($request_path);
+    if ($canonical_path !== null) {
+      if ($canonical_path != $request_path) {
+        return id(new AphrontRedirectResponse())->setURI($canonical_path);
+      }
+    }
+
     $this->diffusionRequest = $drequest;
 
     return null;

src/applications/repository/storage/PhabricatorRepository.php

@@ -687,6 +687,55 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     return "/r{$callsign}{$identifier}";
   }
 
+  public function getCanonicalPath($request_path) {
+    $standard_pattern =
+      '(^'.
+        '(?P<prefix>/diffusion/)'.
+        '(?P<identifier>[^/]+)'.
+        '(?P<suffix>(?:/.*)?)'.
+      '\z)';
+
+    $matches = null;
+    if (preg_match($standard_pattern, $request_path, $matches)) {
+      $prefix = $matches['prefix'];
+
+      $callsign = $this->getCallsign();
+      if ($callsign) {
+        $identifier = $callsign;
+      } else {
+        $identifier = $this->getID();
+      }
+
+      $suffix = $matches['suffix'];
+      if (!strlen($suffix)) {
+        $suffix = '/';
+      }
+
+      return $prefix.$identifier.$suffix;
+    }
+
+    $commit_pattern =
+      '(^'.
+        '(?P<prefix>/)'.
+        '(?P<monogram>'.
+          '(?:'.
+            'r(?P<repositoryCallsign>[A-Z]+)'.
+            '|'.
+            'R(?P<repositoryID>[1-9]\d*):'.
+          ')'.
+          '(?P<commit>[a-f0-9]+)'.
+        ')'.
+      '\z)';
+
+    $matches = null;
+    if (preg_match($commit_pattern, $request_path, $matches)) {
+      $commit = $matches['commit'];
+      return $this->getCommitURI($commit);
+    }
+
+    return null;
+  }
+
   public function generateURI(array $params) {
     $req_branch = false;
     $req_commit = false;

src/applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php

@@ -9,6 +9,47 @@ final class PhabricatorRepositoryURITestCase
     );
   }
 
+  public function testRepositoryURICanonicalization() {
+    $repo = id(new PhabricatorRepository())
+      ->makeEphemeral()
+      ->setID(123);
+
+    $tests = array(
+      '/diffusion/123' => '/diffusion/123/',
+      '/diffusion/123/' => '/diffusion/123/',
+      '/diffusion/123/browse/master/' => '/diffusion/123/browse/master/',
+      '/kangaroo/' => null,
+    );
+
+    foreach ($tests as $input => $expect) {
+      $this->assertEqual(
+        $expect,
+        $repo->getCanonicalPath($input),
+        pht('Canonical Path (ID, No Callsign): %s', $input));
+    }
+
+    $repo->setCallsign('XYZ');
+
+    $tests = array(
+      '/diffusion/123' => '/diffusion/XYZ/',
+      '/diffusion/123/' => '/diffusion/XYZ/',
+      '/diffusion/123/browse/master/' => '/diffusion/XYZ/browse/master/',
+      '/diffusion/XYZ' => '/diffusion/XYZ/',
+      '/diffusion/XYZ/' => '/diffusion/XYZ/',
+      '/diffusion/XYZ/browse/master/' => '/diffusion/XYZ/browse/master/',
+      '/diffusion/ABC/' => '/diffusion/XYZ/',
+      '/kangaroo/' => null,
+      '/R1:abcdef' => '/rXYZabcdef',
+    );
+
+    foreach ($tests as $input => $expect) {
+      $this->assertEqual(
+        $expect,
+        $repo->getCanonicalPath($input),
+        pht('Canonical Path (ID, Callsign): %s', $input));
+    }
+  }
+
   public function testURIGeneration() {
     $svn = PhabricatorRepositoryType::REPOSITORY_TYPE_SVN;
     $git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;