Modernize most somewhat-weird Diffusion controllers

Summary: Ref T4245. This gets everything else except serving HTTP requests (complicated) and lint (quite weird).

Test Plan:
  - Viewed a diff.
  - Viewed externals.
  - Viewed history table to see last modified.
  - Did path completion and validation in Owners.
  - Did tree path search in Diffusion.
  - Viewed a repository.
  - Created a new repository.
  - Looked up symbols.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

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

src/applications/diffusion/controller/DiffusionController.php

@@ -67,15 +67,12 @@ abstract class DiffusionController extends PhabricatorController {
     $request = $this->getRequest();
     $viewer = $this->getViewer();
 
-    $identifier = $request->getURIData('repositoryCallsign');
+    $identifier = $this->getRepositoryIdentifierFromRequest($request);
-    if (!strlen($identifier)) {
-      $identifier = (int)$request->getURIData('repositoryID');
-    }
 
     $params = $options + array(
       'repository' => $identifier,
       'user' => $viewer,
-      'blob' => $request->getURIData('dblob'),
+      'blob' => $this->getDiffusionBlobFromRequest($request),
       'commit' => $request->getURIData('commit'),
       'path' => $request->getURIData('path'),
       'line' => $request->getURIData('line'),
@@ -94,6 +91,21 @@ abstract class DiffusionController extends PhabricatorController {
     return null;
   }
 
+  protected function getDiffusionBlobFromRequest(AphrontRequest $request) {
+    return $request->getURIData('dblob');
+  }
+
+  protected function getRepositoryIdentifierFromRequest(
+    AphrontRequest $request) {
+
+    $identifier = $request->getURIData('repositoryCallsign');
+    if (strlen($identifier)) {
+      return $identifier;
+    }
+
+    return (int)$request->getURIData('repositoryID');
+  }
+
   protected function processDiffusionRequest(AphrontRequest $request) {
     throw new PhutilMethodNotImplementedException();
   }

src/applications/diffusion/controller/DiffusionDiffController.php

@@ -6,27 +6,18 @@ final class DiffusionDiffController extends DiffusionController {
     return true;
   }
 
-  protected function shouldLoadDiffusionRequest() {
+  protected function getDiffusionBlobFromRequest(AphrontRequest $request) {
-    return false;
+    return $request->getStr('ref');
   }
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-    $data = $request->getURIMap();
+    $response = $this->loadDiffusionContext();
-    $data = $data + array(
+    if ($response) {
-      'dblob' => $this->getRequest()->getStr('ref'),
+      return $response;
-    );
-    try {
-      $drequest = DiffusionRequest::newFromAphrontRequestDictionary(
-        $data,
-        $request);
-    } catch (Exception $ex) {
-      return id(new Aphront404Response())
-        ->setRequest($request);
     }
-    $this->setDiffusionRequest($drequest);
 
-    $drequest = $this->getDiffusionRequest();
     $viewer = $this->getViewer();
+    $drequest = $this->getDiffusionRequest();
 
     if (!$request->isAjax()) {
 

src/applications/diffusion/controller/DiffusionExternalController.php

@@ -6,12 +6,7 @@ final class DiffusionExternalController extends DiffusionController {
     return true;
   }
 
-  protected function shouldLoadDiffusionRequest() {
+  public function handleRequest(AphrontRequest $request) {
-    return false;
-  }
-
-  protected function processDiffusionRequest(AphrontRequest $request) {
-
     $uri = $request->getStr('uri');
     $id  = $request->getStr('id');
 
@@ -64,10 +59,11 @@ final class DiffusionExternalController extends DiffusionController {
 
     if (empty($commits)) {
       $desc = null;
-      if ($uri) {
+      if (strlen($uri)) {
-        $desc = $uri.', at ';
+        $desc = pht('"%s", at "%s"', $uri, $id);
+      } else {
+        $desc = pht('"%s"', $id);
       }
-      $desc .= $id;
 
       $content = id(new PHUIInfoView())
         ->setTitle(pht('Unknown External'))
@@ -135,11 +131,13 @@ final class DiffusionExternalController extends DiffusionController {
       $content->setTable($table);
     }
 
-    return $this->buildApplicationPage(
+    $crumbs = $this->buildApplicationCrumbs();
-      $content,
+    $crumbs->addTextCrumb(pht('External'));
-      array(
+
-        'title' => pht('Unresolvable External'),
+    return $this->newPage()
-      ));
+      ->setTitle(pht('Unresolvable External'))
+      ->setCrumbs($crumbs)
+      ->appendChild($content);
   }
 
 }

src/applications/diffusion/controller/DiffusionLastModifiedController.php

@@ -6,9 +6,14 @@ final class DiffusionLastModifiedController extends DiffusionController {
     return true;
   }
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
+    $response = $this->loadDiffusionContext();
+    if ($response) {
+      return $response;
+    }
+
+    $viewer = $this->getViewer();
     $drequest = $this->getDiffusionRequest();
-    $viewer = $request->getUser();
 
     $paths = $request->getStr('paths');
     try {

src/applications/diffusion/controller/DiffusionPathCompleteController.php

@@ -2,21 +2,20 @@
 
 final class DiffusionPathCompleteController extends DiffusionController {
 
-  protected function shouldLoadDiffusionRequest() {
+  protected function getRepositoryIdentifierFromRequest(
-    return false;
+    AphrontRequest $request) {
+    return $request->getStr('repositoryPHID');
   }
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-
+    $response = $this->loadDiffusionContext();
-    $repository_phid = $request->getStr('repositoryPHID');
+    if ($response) {
-    $repository = id(new PhabricatorRepositoryQuery())
+      return $response;
-      ->setViewer($request->getUser())
-      ->withPHIDs(array($repository_phid))
-      ->executeOne();
-    if (!$repository) {
-      return new Aphront400Response();
     }
 
+    $viewer = $this->getViewer();
+    $drequest = $this->getDiffusionRequest();
+
     $query_path = $request->getStr('q');
     if (preg_match('@/$@', $query_path)) {
       $query_dir = $query_path;
@@ -25,19 +24,11 @@ final class DiffusionPathCompleteController extends DiffusionController {
     }
     $query_dir = ltrim($query_dir, '/');
 
-    $drequest = DiffusionRequest::newFromDictionary(
-      array(
-        'user' => $request->getUser(),
-        'repository' => $repository,
-        'path' => $query_dir,
-      ));
-    $this->setDiffusionRequest($drequest);
-
     $browse_results = DiffusionBrowseResultSet::newFromConduit(
       $this->callConduitWithDiffusionRequest(
         'diffusion.browsequery',
         array(
-          'path' => $drequest->getPath(),
+          'path' => $query_dir,
           'commit' => $drequest->getCommit(),
         )));
     $paths = $browse_results->getPaths();

src/applications/diffusion/controller/DiffusionPathTreeController.php

@@ -2,10 +2,16 @@
 
 final class DiffusionPathTreeController extends DiffusionController {
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-    $drequest = $this->getDiffusionRequest();
+    $response = $this->loadDiffusionContext();
+    if ($response) {
+      return $response;
+    }
 
-    if (!$drequest->getRepository()->canUsePathTree()) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
+    if (!$repository->canUsePathTree()) {
       return new Aphront404Response();
     }
 

src/applications/diffusion/controller/DiffusionPathValidateController.php

@@ -2,37 +2,29 @@
 
 final class DiffusionPathValidateController extends DiffusionController {
 
-  protected function shouldLoadDiffusionRequest() {
+  protected function getRepositoryIdentifierFromRequest(
-    return false;
+    AphrontRequest $request) {
+    return $request->getStr('repositoryPHID');
   }
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-
+    $response = $this->loadDiffusionContext();
-    $repository_phid = $request->getStr('repositoryPHID');
+    if ($response) {
-    $repository = id(new PhabricatorRepositoryQuery())
+      return $response;
-      ->setViewer($request->getUser())
-      ->withPHIDs(array($repository_phid))
-      ->executeOne();
-    if (!$repository) {
-      return new Aphront400Response();
     }
 
+    $viewer = $this->getViewer();
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
     $path = $request->getStr('path');
     $path = ltrim($path, '/');
 
-    $drequest = DiffusionRequest::newFromDictionary(
-      array(
-        'user' => $request->getUser(),
-        'repository' => $repository,
-        'path' => $path,
-      ));
-    $this->setDiffusionRequest($drequest);
-
     $browse_results = DiffusionBrowseResultSet::newFromConduit(
       $this->callConduitWithDiffusionRequest(
         'diffusion.browsequery',
         array(
-          'path' => $drequest->getPath(),
+          'path' => $path,
           'commit' => $drequest->getCommit(),
           'needValidityOnly' => true,
         )));

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -6,16 +6,19 @@ final class DiffusionRepositoryController extends DiffusionController {
     return true;
   }
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-    $viewer = $request->getUser();
+    $response = $this->loadDiffusionContext();
+    if ($response) {
+      return $response;
+    }
 
+    $viewer = $this->getViewer();
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
 
     $content = array();
 
     $crumbs = $this->buildCrumbs();
-    $content[] = $crumbs;
 
     $content[] = $this->buildPropertiesTable($drequest->getRepository());
 
@@ -73,11 +76,14 @@ final class DiffusionRepositoryController extends DiffusionController {
         ->setErrors(array($empty_message));
     }
 
-    return $this->buildApplicationPage(
+    return $this->newPage()
-      $content,
+      ->setTitle(
-      array(
+        array(
-        'title' => $drequest->getRepository()->getName(),
+          $repository->getName(),
-      ));
+          $repository->getDisplayName(),
+        ))
+      ->setCrumbs($crumbs)
+      ->appendChild($content);
   }
 
 

src/applications/diffusion/controller/DiffusionRepositoryNewController.php

@@ -2,8 +2,8 @@
 
 final class DiffusionRepositoryNewController extends DiffusionController {
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-    $viewer = $request->getUser();
+    $viewer = $this->getViewer();
 
     $this->requireApplicationCapability(
       DiffusionCreateRepositoriesCapability::CAPABILITY);
@@ -70,14 +70,10 @@ final class DiffusionRepositoryNewController extends DiffusionController {
       ->setHeaderText(pht('Create or Import Repository'))
       ->setForm($form);
 
-    return $this->buildApplicationPage(
+    return $this->newPage()
-      array(
+      ->setTitle(pht('New Repository'))
-        $crumbs,
+      ->setCrumbs($crumbs)
-        $form_box,
+      ->appendChild($form_box);
-      ),
-      array(
-        'title' => pht('New Repository'),
-      ));
   }
 
 }

src/applications/diffusion/controller/DiffusionSymbolController.php

@@ -2,15 +2,13 @@
 
 final class DiffusionSymbolController extends DiffusionController {
 
-  private $name;
+  public function handleRequest(AphrontRequest $request) {
-
+    $viewer = $this->getViewer();
-  protected function processDiffusionRequest(AphrontRequest $request) {
+    $name = $request->getURIData('name');
-    $user = $request->getUser();
-    $this->name = $request->getURIData('name');
 
     $query = id(new DiffusionSymbolQuery())
-      ->setViewer($user)
+      ->setViewer($viewer)
-      ->setName($this->name);
+      ->setName($name);
 
     if ($request->getStr('context')) {
       $query->setContext($request->getStr('context'));
@@ -48,9 +46,8 @@ final class DiffusionSymbolController extends DiffusionController {
     $symbols = $query->execute();
 
 
-
     $external_query = id(new DiffusionExternalSymbolQuery())
-      ->withNames(array($this->name));
+      ->withNames(array($name));
 
     if ($request->getStr('context')) {
       $external_query->withContexts(array($request->getStr('context')));
@@ -137,15 +134,17 @@ final class DiffusionSymbolController extends DiffusionController {
     $table->setNoDataString(
       pht('No matching symbol could be found in any indexed repository.'));
 
-    $panel = new PHUIObjectBoxView();
+    $panel = id(new PHUIObjectBoxView())
-    $panel->setHeaderText(pht('Similar Symbols'));
+      ->setHeaderText(pht('Similar Symbols'))
-    $panel->setTable($table);
+      ->setTable($table);
 
-    return $this->buildApplicationPage(
+    $crumbs = $this->buildApplicationCrumbs();
-      $panel,
+    $crumbs->addTextCrumb(pht('Find Symbol'));
-      array(
+
-        'title' => pht('Find Symbol'),
+    return $this->newPage()
-      ));
+      ->setTitle(pht('Find Symbol'))
+      ->setCrumbs($crumbs)
+      ->appendChild($panel);
   }
 
 }