Always pull extra browse information over Ajax, and batch some of the queries

Summary: This code is currently quite complicated because we pull history data inline for SVN files, and via ajax for everything else (SVN dirs, everything in Git and Hg). Always pull over ajax; batch some of the queries. Test Plan: {F34860} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley, vrana, aran Maniphest Tasks: T2683 Differential Revision: https://secure.phabricator.com/D5255
2024-09-20 01:08:50 +02:00 · 2014-05-10 15:57:16 -07:00 · 2014-05-10 15:57:16 -07:00 · e03deb7d4a
commit e03deb7d4a
parent df59f4b047
4 changed files with 119 additions and 120 deletions
--- a/src/applications/diffusion/controller/DiffusionLastModifiedController.php
+++ b/src/applications/diffusion/controller/DiffusionLastModifiedController.php
@ -12,8 +12,11 @@ final class DiffusionLastModifiedController extends DiffusionController {

    $paths = $request->getStr('paths');
    $paths = json_decode($paths, true);
+    if (!is_array($paths)) {
+      return new Aphront400Response();
+    }

-    $output = array();
+    $commits = array();
    foreach ($paths as $path) {
      $prequest = clone $drequest;
      $prequest->setPath($path);
@ -31,6 +34,8 @@ final class DiffusionLastModifiedController extends DiffusionController {
      $commit_data = PhabricatorRepositoryCommitData::newFromDictionary(
        $conduit_result['commitData']);

+      $commit->attachCommitData($commit_data);
+
      $phids = array();
      if ($commit_data) {
        if ($commit_data->getCommitDetail('authorPHID')) {
@ -41,18 +46,110 @@ final class DiffusionLastModifiedController extends DiffusionController {
        }
      }

-      $phids = array_keys($phids);
-      $handles = $this->loadViewerHandles($phids);
+      $commits[$path] = $commit;
+    }

-      $view = new DiffusionBrowseTableView();
-      $view->setUser($request->getUser());
-      $output[$path] = $view->renderLastModifiedColumns(
+    $phids = array_keys($phids);
+    $handles = $this->loadViewerHandles($phids);
+
+    $branch = $drequest->loadBranch();
+    if ($branch) {
+      $lint_query = id(new DiffusionLintCountQuery())
+        ->withBranchIDs(array($branch->getID()))
+        ->withPaths(array_keys($commits));
+
+      if ($drequest->getLint()) {
+        $lint_query->withCodes(array($drequest->getLint()));
+      }
+
+      $lint = $lint_query->execute();
+    } else {
+      $lint = array();
+    }
+
+    $output = array();
+    foreach ($commits as $path => $commit) {
+      $prequest = clone $drequest;
+      $prequest->setPath($path);
+
+      $output[$path] = $this->renderColumns(
        $prequest,
        $handles,
        $commit,
-        $commit_data);
+        idx($lint, $path));
    }

    return id(new AphrontAjaxResponse())->setContent($output);
  }
+
+  private function renderColumns(
+    DiffusionRequest $drequest,
+    array $handles,
+    PhabricatorRepositoryCommit $commit = null,
+    $lint = null) {
+    assert_instances_of($handles, 'PhabricatorObjectHandle');
+    $viewer = $this->getRequest()->getUser();
+
+    if ($commit) {
+      $epoch = $commit->getEpoch();
+      $modified = DiffusionView::linkCommit(
+        $drequest->getRepository(),
+        $commit->getCommitIdentifier());
+      $date = phabricator_date($epoch, $viewer);
+      $time = phabricator_time($epoch, $viewer);
+    } else {
+      $modified = '';
+      $date = '';
+      $time = '';
+    }
+
+    $data = $commit->getCommitData();
+    if ($data) {
+      $author_phid = $data->getCommitDetail('authorPHID');
+      if ($author_phid && isset($handles[$author_phid])) {
+        $author = $handles[$author_phid]->renderLink();
+      } else {
+        $author = DiffusionView::renderName($data->getAuthorName());
+      }
+
+      $committer = $data->getCommitDetail('committer');
+      if ($committer) {
+        $committer_phid = $data->getCommitDetail('committerPHID');
+        if ($committer_phid && isset($handles[$committer_phid])) {
+          $committer = $handles[$committer_phid]->renderLink();
+        } else {
+          $committer = DiffusionView::renderName($committer);
+        }
+        if ($author != $committer) {
+          $author = hsprintf('%s/%s', $author, $committer);
+        }
+      }
+
+      $details = AphrontTableView::renderSingleDisplayLine($data->getSummary());
+    } else {
+      $author = '';
+      $details = '';
+    }
+
+    $return = array(
+      'commit'    => $modified,
+      'date'      => $date,
+      'time'      => $time,
+      'author'    => $author,
+      'details'   => $details,
+    );
+
+    if ($lint !== null) {
+      $return['lint'] = phutil_tag(
+        'a',
+        array('href' => $drequest->generateURI(array(
+          'action' => 'lint',
+          'lint' => null,
+        ))),
+        number_format($lint));
+    }
+
+    return $return;
+  }
+
 }
--- a/src/applications/diffusion/view/DiffusionBrowseTableView.php
+++ b/src/applications/diffusion/view/DiffusionBrowseTableView.php
@ -17,96 +17,6 @@ final class DiffusionBrowseTableView extends DiffusionView {
    return $this;
  }

-  public function renderLastModifiedColumns(
-    DiffusionRequest $drequest,
-    array $handles,
-    PhabricatorRepositoryCommit $commit = null,
-    PhabricatorRepositoryCommitData $data = null) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-
-    if ($commit) {
-      $epoch = $commit->getEpoch();
-      $modified = DiffusionView::linkCommit(
-        $drequest->getRepository(),
-        $commit->getCommitIdentifier());
-      $date = phabricator_date($epoch, $this->user);
-      $time = phabricator_time($epoch, $this->user);
-    } else {
-      $modified = '';
-      $date = '';
-      $time = '';
-    }
-
-    if ($data) {
-      $author_phid = $data->getCommitDetail('authorPHID');
-      if ($author_phid && isset($handles[$author_phid])) {
-        $author = $handles[$author_phid]->renderLink();
-      } else {
-        $author = self::renderName($data->getAuthorName());
-      }
-
-      $committer = $data->getCommitDetail('committer');
-      if ($committer) {
-        $committer_phid = $data->getCommitDetail('committerPHID');
-        if ($committer_phid && isset($handles[$committer_phid])) {
-          $committer = $handles[$committer_phid]->renderLink();
-        } else {
-          $committer = self::renderName($committer);
-        }
-        if ($author != $committer) {
-          $author = hsprintf('%s/%s', $author, $committer);
-        }
-      }
-
-      $details = AphrontTableView::renderSingleDisplayLine($data->getSummary());
-    } else {
-      $author = '';
-      $details = '';
-    }
-
-    $return = array(
-      'commit'    => $modified,
-      'date'      => $date,
-      'time'      => $time,
-      'author'    => $author,
-      'details'   => $details,
-    );
-
-    $lint = self::loadLintMessagesCount($drequest);
-    if ($lint !== null) {
-      $return['lint'] = phutil_tag(
-        'a',
-        array('href' => $drequest->generateURI(array(
-          'action' => 'lint',
-          'lint' => null,
-        ))),
-        number_format($lint));
-    }
-
-    return $return;
-  }
-
-  private static function loadLintMessagesCount(DiffusionRequest $drequest) {
-    $path = $drequest->getPath();
-    $branch = $drequest->loadBranch();
-    if (!$branch) {
-      return null;
-    }
-
-    $query = id(new DiffusionLintCountQuery())
-      ->withBranchIDs(array($branch->getID()))
-      ->withPaths(array($path));
-
-    $code = $drequest->getLint();
-    if ($code) {
-      $query->withCodes(array($code));
-    }
-
-    $result = $query->execute();
-
-    return idx($result, $path);
-  }
-
  public function render() {
    $request = $this->getDiffusionRequest();
    $repository = $request->getRepository();
@ -152,29 +62,18 @@ final class DiffusionBrowseTableView extends DiffusionView {
          ));
      }

-      $commit = $path->getLastModifiedCommit();
-      if ($commit) {
-        $drequest = clone $request;
-        $drequest->setPath($request->getPath().$path->getPath().$dir_slash);
-        $dict = $this->renderLastModifiedColumns(
-          $drequest,
-          $this->handles,
-          $commit,
-          $path->getLastCommitData());
-      } else {
-        $dict = array(
-          'lint'      => celerity_generate_unique_node_id(),
-          'commit'    => celerity_generate_unique_node_id(),
-          'date'      => celerity_generate_unique_node_id(),
-          'time'      => celerity_generate_unique_node_id(),
-          'author'    => celerity_generate_unique_node_id(),
-          'details'   => celerity_generate_unique_node_id(),
-        );
+      $dict = array(
+        'lint'      => celerity_generate_unique_node_id(),
+        'commit'    => celerity_generate_unique_node_id(),
+        'date'      => celerity_generate_unique_node_id(),
+        'time'      => celerity_generate_unique_node_id(),
+        'author'    => celerity_generate_unique_node_id(),
+        'details'   => celerity_generate_unique_node_id(),
+      );

-        $need_pull[$base_path.$path->getPath().$dir_slash] = $dict;
-        foreach ($dict as $k => $uniq) {
-          $dict[$k] = phutil_tag('span', array('id' => $uniq), '');
-        }
+      $need_pull[$base_path.$path->getPath().$dir_slash] = $dict;
+      foreach ($dict as $k => $uniq) {
+        $dict[$k] = phutil_tag('span', array('id' => $uniq), '');
      }

      $editor_button = '';
--- a/src/applications/diffusion/view/DiffusionView.php
+++ b/src/applications/diffusion/view/DiffusionView.php
@ -144,7 +144,7 @@ abstract class DiffusionView extends AphrontView {
      "D{$id}");
  }

-  final protected static function renderName($name) {
+  final public static function renderName($name) {
    $email = new PhutilEmailAddress($name);
    if ($email->getDisplayName() && $email->getDomainName()) {
      Javelin::initBehavior('phabricator-tooltips', array());
--- a/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
+++ b/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js
@ -13,6 +13,9 @@ JX.behavior('diffusion-pull-lastmodified', function(config) {
    .setHandler(function(r) {
      for (var k in r) {
        for (var l in r[k]) {
+          if (!config.map[k][l]) {
+            continue;
+          }
          JX.DOM.setContent(JX.$(config.map[k][l]), JX.$H(r[k][l]));
        }
      }