Modernize Diffusion lint controllers

Summary: Ref T4245. On their best day these don't work all that well, but I'm pretty sure I didn't make anything worse.

Test Plan:
  - Viewed global lint.
  - Viewed lint for a repository.
  - Viewed lint details for a particular message.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4245

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

src/__phutil_library_map__.php

@@ -621,7 +621,6 @@ phutil_register_library_map(array(
     'DiffusionLastModifiedQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php',
     'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php',
     'DiffusionLintCountQuery' => 'applications/diffusion/query/DiffusionLintCountQuery.php',
-    'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php',
     'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php',
     'DiffusionLookSoonConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionLookSoonConduitAPIMethod.php',
     'DiffusionLowLevelCommitFieldsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php',
@@ -4585,7 +4584,6 @@ phutil_register_library_map(array(
     'DiffusionLastModifiedQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
     'DiffusionLintController' => 'DiffusionController',
     'DiffusionLintCountQuery' => 'PhabricatorQuery',
-    'DiffusionLintDetailsController' => 'DiffusionController',
     'DiffusionLintSaveRunner' => 'Phobject',
     'DiffusionLookSoonConduitAPIMethod' => 'DiffusionConduitAPIMethod',
     'DiffusionLowLevelCommitFieldsQuery' => 'DiffusionLowLevelQuery',

src/applications/diffusion/controller/DiffusionLintController.php

@@ -6,76 +6,106 @@ final class DiffusionLintController extends DiffusionController {
     return true;
   }
 
-  protected function processDiffusionRequest(AphrontRequest $request) {
+  public function handleRequest(AphrontRequest $request) {
-    $user = $request->getUser();
+    $viewer = $this->getViewer();
-    $drequest = $this->diffusionRequest;
 
-    if ($request->getStr('lint') !== null) {
+    if ($this->getRepositoryIdentifierFromRequest($request)) {
-      $controller = new DiffusionLintDetailsController();
+      $response = $this->loadDiffusionContext();
-      $controller->setDiffusionRequest($drequest);
+      if ($response) {
-      $controller->setCurrentApplication($this->getCurrentApplication());
+        return $response;
-      return $this->delegateToController($controller);
+      }
+
+      $drequest = $this->getDiffusionRequest();
+    } else {
+      $drequest = null;
+    }
+
+    $code = $request->getStr('lint');
+    if (strlen($code)) {
+      return $this->buildDetailsResponse();
     }
 
     $owners = array();
     if (!$drequest) {
       if (!$request->getArr('owner')) {
-        $owners = array($user->getPHID());
+        $owners = array($viewer->getPHID());
       } else {
         $owners = array(head($request->getArr('owner')));
       }
     }
 
-    $codes = $this->loadLintCodes($owners);
+    $codes = $this->loadLintCodes($drequest, $owners);
-
-    if ($codes && !$drequest) {
-      // TODO: Build some real Query classes for this stuff.
 
+    if ($codes) {
       $branches = id(new PhabricatorRepositoryBranch())->loadAllWhere(
         'id IN (%Ld)',
         array_unique(ipull($codes, 'branchID')));
+      $branches = mpull($branches, null, 'getID');
+    } else {
+      $branches = array();
+    }
 
+    if ($branches) {
       $repositories = id(new PhabricatorRepositoryQuery())
-        ->setViewer($user)
+        ->setViewer($viewer)
         ->withIDs(mpull($branches, 'getRepositoryID'))
         ->execute();
-
+      $repositories = mpull($repositories, null, 'getID');
-      $drequests = array();
+    } else {
-      foreach ($branches as $id => $branch) {
+      $repositories = array();
-        if (empty($repositories[$branch->getRepositoryID()])) {
-          continue;
-        }
-
-        $drequests[$id] = DiffusionRequest::newFromDictionary(array(
-          'user' => $user,
-          'repository' => $repositories[$branch->getRepositoryID()],
-          'branch' => $branch->getName(),
-        ));
-      }
     }
 
+
     $rows = array();
     $total = 0;
     foreach ($codes as $code) {
-      if (!$this->diffusionRequest) {
+      $branch = idx($branches, $code['branchID']);
-        $drequest = idx($drequests, $code['branchID']);
+      if (!$branch) {
+        continue;
       }
 
-      if (!$drequest) {
+      $repository = idx($repositories, $branch->getRepositoryID());
+      if (!$repository) {
         continue;
       }
 
       $total += $code['n'];
 
-      $href_lint = $drequest->generateURI(array(
+      if ($drequest) {
-        'action' => 'lint',
+        $href_lint = $drequest->generateURI(
-        'lint' => $code['code'],
+          array(
-      ));
+            'action' => 'lint',
-      $href_browse = $drequest->generateURI(array(
+            'lint' => $code['code'],
-        'action' => 'browse',
+          ));
-        'lint' => $code['code'],
+
-      ));
+        $href_browse = $drequest->generateURI(
-      $href_repo = $drequest->generateURI(array('action' => 'lint'));
+          array(
+            'action' => 'browse',
+            'lint' => $code['code'],
+          ));
+
+        $href_repo = $drequest->generateURI(
+          array(
+            'action' => 'lint',
+          ));
+      } else {
+        $href_lint = $repository->generateURI(
+          array(
+            'action' => 'lint',
+            'lint' => $code['code'],
+          ));
+
+        $href_browse = $repository->generateURI(
+          array(
+            'action' => 'browse',
+            'lint' => $code['code'],
+          ));
+
+        $href_repo = $repository->generateURI(
+          array(
+            'action' => 'lint',
+          ));
+      }
 
       $rows[] = array(
         phutil_tag('a', array('href' => $href_lint), $code['n']),
@@ -85,7 +115,7 @@ final class DiffusionLintController extends DiffusionController {
           array(
             'href' => $href_repo,
           ),
-          $drequest->getRepository()->getDisplayName()),
+          $repository->getDisplayName()),
         ArcanistLintSeverity::getStringForSeverity($code['maxSeverity']),
         $code['code'],
         $code['maxName'],
@@ -103,14 +133,14 @@ final class DiffusionLintController extends DiffusionController {
         pht('Name'),
         pht('Example'),
       ))
-      ->setColumnVisibility(array(true, true, !$this->diffusionRequest))
+      ->setColumnVisibility(array(true, true, !$drequest))
       ->setColumnClasses(array('n', 'n', '', '', 'pri', '', ''));
 
     $content = array();
 
-    if (!$this->diffusionRequest) {
+    if (!$drequest) {
       $form = id(new AphrontFormView())
-        ->setUser($user)
+        ->setUser($viewer)
         ->setMethod('GET')
         ->appendControl(
           id(new AphrontFormTokenizerControl())
@@ -137,18 +167,18 @@ final class DiffusionLintController extends DiffusionController {
         'view'   => 'lint',
       ));
 
-    if ($this->diffusionRequest) {
+    if ($drequest) {
       $title[] = $drequest->getRepository()->getDisplayName();
     } else {
       $crumbs->addTextCrumb(pht('All Lint'));
     }
 
-    if ($this->diffusionRequest) {
+    if ($drequest) {
       $branch = $drequest->loadBranch();
 
       $header = id(new PHUIHeaderView())
         ->setHeader($this->renderPathLinks($drequest, 'lint'))
-        ->setUser($user)
+        ->setUser($viewer)
         ->setPolicyObject($drequest->getRepository());
       $actions = $this->buildActionView($drequest);
       $properties = $this->buildPropertyView(
@@ -164,20 +194,17 @@ final class DiffusionLintController extends DiffusionController {
       $object_box = null;
     }
 
-
+    return $this->newPage()
-    return $this->buildApplicationPage(
+      ->setTitle($title)
-      array(
+      ->setCrumbs($crumbs)
-        $crumbs,
+      ->appendChild(
-        $object_box,
+        array(
-        $content,
+          $object_box,
-      ),
+          $content,
-      array(
+        ));
-        'title' => $title,
-      ));
   }
 
-  private function loadLintCodes(array $owner_phids) {
+  private function loadLintCodes($drequest, array $owner_phids) {
-    $drequest = $this->diffusionRequest;
     $conn = id(new PhabricatorRepository())->establishConnection('r');
     $where = array('1 = 1');
 
@@ -342,4 +369,146 @@ final class DiffusionLintController extends DiffusionController {
   }
 
 
+  private function buildDetailsResponse() {
+    $request = $this->getRequest();
+
+    $limit = 500;
+
+    $pager = id(new PHUIPagerView())
+      ->readFromRequest($request)
+      ->setPageSize($limit);
+
+    $offset = $pager->getOffset();
+
+    $drequest = $this->getDiffusionRequest();
+    $branch = $drequest->loadBranch();
+    $messages = $this->loadLintMessages($branch, $limit, $offset);
+    $is_dir = (substr('/'.$drequest->getPath(), -1) == '/');
+
+    $pager->setHasMorePages(count($messages) >= $limit);
+
+    $authors = $this->loadViewerHandles(ipull($messages, 'authorPHID'));
+
+    $rows = array();
+    foreach ($messages as $message) {
+      $path = phutil_tag(
+        'a',
+        array(
+          'href' => $drequest->generateURI(array(
+            'action' => 'lint',
+            'path' => $message['path'],
+          )),
+        ),
+        substr($message['path'], strlen($drequest->getPath()) + 1));
+
+      $line = phutil_tag(
+        'a',
+        array(
+          'href' => $drequest->generateURI(array(
+            'action' => 'browse',
+            'path' => $message['path'],
+            'line' => $message['line'],
+            'commit' => $branch->getLintCommit(),
+          )),
+        ),
+        $message['line']);
+
+      $author = $message['authorPHID'];
+      if ($author && $authors[$author]) {
+        $author = $authors[$author]->renderLink();
+      }
+
+      $rows[] = array(
+        $path,
+        $line,
+        $author,
+        ArcanistLintSeverity::getStringForSeverity($message['severity']),
+        $message['name'],
+        $message['description'],
+      );
+    }
+
+    $table = id(new AphrontTableView($rows))
+      ->setHeaders(array(
+        pht('Path'),
+        pht('Line'),
+        pht('Author'),
+        pht('Severity'),
+        pht('Name'),
+        pht('Description'),
+      ))
+      ->setColumnClasses(array('', 'n'))
+      ->setColumnVisibility(array($is_dir));
+
+    $content = array();
+
+    $content[] = id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Lint Details'))
+      ->setTable($table);
+
+    $crumbs = $this->buildCrumbs(
+      array(
+        'branch' => true,
+        'path'   => true,
+        'view'   => 'lint',
+      ));
+
+    $pager_box = $this->renderTablePagerBox($pager);
+
+    return $this->newPage()
+      ->setTitle(
+        array(
+          pht('Lint'),
+          $drequest->getRepository()->getDisplayName(),
+        ))
+      ->setCrumbs($crumbs)
+      ->appendChild(
+        array(
+          $content,
+          $pager_box,
+        ));
+  }
+
+  private function loadLintMessages(
+    PhabricatorRepositoryBranch $branch,
+    $limit,
+    $offset) {
+
+    $drequest = $this->getDiffusionRequest();
+    if (!$branch) {
+      return array();
+    }
+
+    $conn = $branch->establishConnection('r');
+
+    $where = array(
+      qsprintf($conn, 'branchID = %d', $branch->getID()),
+    );
+
+    if ($drequest->getPath() != '') {
+      $path = '/'.$drequest->getPath();
+      $is_dir = (substr($path, -1) == '/');
+      $where[] = ($is_dir
+        ? qsprintf($conn, 'path LIKE %>', $path)
+        : qsprintf($conn, 'path = %s', $path));
+    }
+
+    if ($drequest->getLint() != '') {
+      $where[] = qsprintf(
+        $conn,
+        'code = %s',
+        $drequest->getLint());
+    }
+
+    return queryfx_all(
+      $conn,
+      'SELECT *
+        FROM %T
+        WHERE %Q
+        ORDER BY path, code, line LIMIT %d OFFSET %d',
+      PhabricatorRepository::TABLE_LINTMESSAGE,
+      implode(' AND ', $where),
+      $limit,
+      $offset);
+  }
 }

src/applications/diffusion/controller/DiffusionLintDetailsController.php

@@ -1,143 +0,0 @@
-<?php
-
-final class DiffusionLintDetailsController extends DiffusionController {
-
-  protected function processDiffusionRequest(AphrontRequest $request) {
-    $limit = 500;
-    $offset = $request->getInt('offset', 0);
-
-    $drequest = $this->getDiffusionRequest();
-    $branch = $drequest->loadBranch();
-    $messages = $this->loadLintMessages($branch, $limit, $offset);
-    $is_dir = (substr('/'.$drequest->getPath(), -1) == '/');
-
-    $authors = $this->loadViewerHandles(ipull($messages, 'authorPHID'));
-
-    $rows = array();
-    foreach ($messages as $message) {
-      $path = phutil_tag(
-        'a',
-        array(
-          'href' => $drequest->generateURI(array(
-            'action' => 'lint',
-            'path' => $message['path'],
-          )),
-        ),
-        substr($message['path'], strlen($drequest->getPath()) + 1));
-
-      $line = phutil_tag(
-        'a',
-        array(
-          'href' => $drequest->generateURI(array(
-            'action' => 'browse',
-            'path' => $message['path'],
-            'line' => $message['line'],
-            'commit' => $branch->getLintCommit(),
-          )),
-        ),
-        $message['line']);
-
-      $author = $message['authorPHID'];
-      if ($author && $authors[$author]) {
-        $author = $authors[$author]->renderLink();
-      }
-
-      $rows[] = array(
-        $path,
-        $line,
-        $author,
-        ArcanistLintSeverity::getStringForSeverity($message['severity']),
-        $message['name'],
-        $message['description'],
-      );
-    }
-
-    $table = id(new AphrontTableView($rows))
-      ->setHeaders(array(
-        pht('Path'),
-        pht('Line'),
-        pht('Author'),
-        pht('Severity'),
-        pht('Name'),
-        pht('Description'),
-      ))
-      ->setColumnClasses(array('', 'n'))
-      ->setColumnVisibility(array($is_dir));
-
-    $content = array();
-
-    $pager = id(new PHUIPagerView())
-      ->setPageSize($limit)
-      ->setOffset($offset)
-      ->setHasMorePages(count($messages) >= $limit)
-      ->setURI($request->getRequestURI(), 'offset');
-
-    $content[] = id(new PHUIObjectBoxView())
-      ->setHeaderText(pht('Lint Details'))
-      ->setTable($table);
-
-    $crumbs = $this->buildCrumbs(
-      array(
-        'branch' => true,
-        'path'   => true,
-        'view'   => 'lint',
-      ));
-
-    return $this->buildApplicationPage(
-      array(
-        $crumbs,
-        $content,
-        $pager,
-      ),
-      array(
-        'title' => array(
-          pht('Lint'),
-          $drequest->getRepository()->getDisplayName(),
-        ),
-      ));
-  }
-
-  private function loadLintMessages(
-    PhabricatorRepositoryBranch $branch,
-    $limit,
-    $offset) {
-
-    $drequest = $this->getDiffusionRequest();
-    if (!$branch) {
-      return array();
-    }
-
-    $conn = $branch->establishConnection('r');
-
-    $where = array(
-      qsprintf($conn, 'branchID = %d', $branch->getID()),
-    );
-
-    if ($drequest->getPath() != '') {
-      $path = '/'.$drequest->getPath();
-      $is_dir = (substr($path, -1) == '/');
-      $where[] = ($is_dir
-        ? qsprintf($conn, 'path LIKE %>', $path)
-        : qsprintf($conn, 'path = %s', $path));
-    }
-
-    if ($drequest->getLint() != '') {
-      $where[] = qsprintf(
-        $conn,
-        'code = %s',
-        $drequest->getLint());
-    }
-
-    return queryfx_all(
-      $conn,
-      'SELECT *
-        FROM %T
-        WHERE %Q
-        ORDER BY path, code, line LIMIT %d OFFSET %d',
-      PhabricatorRepository::TABLE_LINTMESSAGE,
-      implode(' AND ', $where),
-      $limit,
-      $offset);
-  }
-
-}

src/applications/diffusion/request/DiffusionRequest.php

@@ -244,9 +244,10 @@ abstract class DiffusionRequest extends Phobject {
       $data = $blob + $data;
     }
 
-    $this->path            = idx($data, 'path');
+    $this->path = idx($data, 'path');
-    $this->line            = idx($data, 'line');
+    $this->line = idx($data, 'line');
     $this->initFromConduit = idx($data, 'initFromConduit', true);
+    $this->lint = idx($data, 'lint');
 
     $this->symbolicCommit = idx($data, 'commit');
     if ($this->supportsBranches()) {