Fix all remaining weird Diffusion request processing

Summary: Ref T4245. This is the last of it, and covers the clone/push stuff.

Test Plan:
  - Cloned git.
  - Pushed git.
  - Cloned mercurial.
  - Pushed mercurial.
  - Visited a `blah.git` URL in my browser just because; got redirected to a human-facing UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

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

src/applications/diffusion/application/PhabricatorDiffusionApplication.php

@@ -64,7 +64,7 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication {
           '(?:query/(?P<queryKey>[^/]+)/)?' => 'DiffusionPushLogListController',
           'view/(?P<id>\d+)/' => 'DiffusionPushEventViewController',
         ),
-        '(?P<repositoryCallsign>(?P<callsign>[A-Z]+))/' => array(
+        '(?P<repositoryCallsign>[A-Z]+)/' => array(
           '' => 'DiffusionRepositoryController',
 
           'repository/(?P<dblob>.*)'    => 'DiffusionRepositoryController',
@@ -115,7 +115,8 @@ 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<callsign>[A-Z]+)(/|$).*' => 'DiffusionRepositoryDefaultController',
+        '(?P<repositoryCallsign>[A-Z]+)(?:/.*)?' =>
+          'DiffusionRepositoryDefaultController',
 
         'inline/' => array(
           'edit/(?P<phid>[^/]+)/' => 'DiffusionInlineCommentController',

src/applications/diffusion/controller/DiffusionController.php

@@ -2,56 +2,34 @@
 
 abstract class DiffusionController extends PhabricatorController {
 
-  protected $diffusionRequest;
+  private $diffusionRequest;
-
-  public function setDiffusionRequest(DiffusionRequest $request) {
-    $this->diffusionRequest = $request;
-    return $this;
-  }
 
   protected function getDiffusionRequest() {
     if (!$this->diffusionRequest) {
-      throw new Exception(pht('No Diffusion request object!'));
+      throw new PhutilInvalidStateException('loadDiffusionContext');
     }
     return $this->diffusionRequest;
   }
 
+  protected function hasDiffusionRequest() {
+    return (bool)$this->diffusionRequest;
+  }
+
   public function willBeginExecution() {
     $request = $this->getRequest();
 
     // Check if this is a VCS request, e.g. from "git clone", "hg clone", or
     // "svn checkout". If it is, we jump off into repository serving code to
     // process the request.
-    if (DiffusionServeController::isVCSRequest($request)) {
+
-      $serve_controller = id(new DiffusionServeController())
+    $serve_controller = new DiffusionServeController();
-        ->setCurrentApplication($this->getCurrentApplication());
+    if ($serve_controller->isVCSRequest($request)) {
       return $this->delegateToController($serve_controller);
     }
 
     return parent::willBeginExecution();
   }
 
-  protected function shouldLoadDiffusionRequest() {
-    return true;
-  }
-
-  public function handleRequest(AphrontRequest $request) {
-    if ($request->getURIData('callsign') &&
-        $this->shouldLoadDiffusionRequest()) {
-      try {
-      $drequest = DiffusionRequest::newFromAphrontRequestDictionary(
-        $request->getURIMap(),
-        $request);
-      } catch (Exception $ex) {
-        return id(new Aphront404Response())
-          ->setRequest($request);
-      }
-      $this->setDiffusionRequest($drequest);
-    }
-
-    return $this->processDiffusionRequest($request);
-  }
-
   protected function loadDiffusionContextForEdit() {
     return $this->loadContext(
       array(
@@ -103,11 +81,12 @@ abstract class DiffusionController extends PhabricatorController {
       return $identifier;
     }
 
-    return (int)$request->getURIData('repositoryID');
+    $id = $request->getURIData('repositoryID');
-  }
+    if (strlen($id)) {
+      return (int)$id;
+    }
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+    return null;
-    throw new PhutilMethodNotImplementedException();
   }
 
   public function buildCrumbs(array $spec = array()) {

src/applications/diffusion/controller/DiffusionRepositoryEditController.php

@@ -6,8 +6,9 @@ abstract class DiffusionRepositoryEditController
   protected function buildApplicationCrumbs($is_main = false) {
     $crumbs = parent::buildApplicationCrumbs();
 
-    if ($this->diffusionRequest) {
+    if ($this->hasDiffusionRequest()) {
-      $repository = $this->getDiffusionRequest()->getRepository();
+      $drequest = $this->getDiffusionRequest();
+      $repository = $drequest->getRepository();
       $repo_uri = $repository->getURI();
       $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/');
 

src/applications/diffusion/controller/DiffusionServeController.php

@@ -2,12 +2,9 @@
 
 final class DiffusionServeController extends DiffusionController {
 
-  protected function shouldLoadDiffusionRequest() {
+  public function isVCSRequest(AphrontRequest $request) {
-    return false;
+    $identifier = $this->getRepositoryIdentifierFromRequest($request);
-  }
+    if ($identifier === null) {
-
-  public static function isVCSRequest(AphrontRequest $request) {
-    if (!self::getCallsign($request)) {
       return null;
     }
 
@@ -47,20 +44,8 @@ final class DiffusionServeController extends DiffusionController {
     return $vcs;
   }
 
-  private static function getCallsign(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-    $uri = $request->getRequestURI();
+    $identifier = $this->getRepositoryIdentifierFromRequest($request);
-
-    $regex = '@^/diffusion/(?P<callsign>[A-Z]+)(/|$)@';
-    $matches = null;
-    if (!preg_match($regex, (string)$uri, $matches)) {
-      return null;
-    }
-
-    return $matches['callsign'];
-  }
-
-  protected function processDiffusionRequest(AphrontRequest $request) {
-    $callsign = self::getCallsign($request);
 
     // If authentication credentials have been provided, try to find a user
     // that actually matches those credentials.
@@ -99,7 +84,7 @@ final class DiffusionServeController extends DiffusionController {
     try {
       $repository = id(new PhabricatorRepositoryQuery())
         ->setViewer($viewer)
-        ->withCallsigns(array($callsign))
+        ->withIdentifiers(array($identifier))
         ->executeOne();
       if (!$repository) {
         return new PhabricatorVCSResponse(

src/applications/diffusion/request/DiffusionRequest.php

@@ -117,43 +117,6 @@ abstract class DiffusionRequest extends Phobject {
     return $object;
   }
 
-
-  /**
-   * Create a new request from an Aphront request dictionary. This is an
-   * internal method that you generally should not call directly; instead,
-   * call @{method:newFromDictionary}.
-   *
-   * @param   map                 Map of Aphront request data.
-   * @return  DiffusionRequest    New request object.
-   * @task new
-   */
-  final public static function newFromAphrontRequestDictionary(
-    array $data,
-    AphrontRequest $request) {
-
-    $identifier = phutil_unescape_uri_path_component(idx($data, 'callsign'));
-    $object = self::newFromIdentifier($identifier, $request->getUser());
-
-    $use_branches = $object->supportsBranches();
-
-    if (isset($data['dblob'])) {
-      $parsed = self::parseRequestBlob(idx($data, 'dblob'), $use_branches);
-    } else {
-      $parsed = array(
-        'commit' => idx($data, 'commit'),
-        'path' => idx($data, 'path'),
-        'line' => idx($data, 'line'),
-        'branch' => idx($data, 'branch'),
-      );
-    }
-
-    $object->setUser($request->getUser());
-    $object->initializeFromDictionary($parsed);
-    $object->lint = $request->getStr('lint');
-    return $object;
-  }
-
-
   /**
    * Internal.
    *