From 525673126221b3779c8ec63131035edd96cb22c6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 30 Jan 2013 12:01:49 -0800
Subject: [PATCH] Don't show changes for commits which affect more than 1,000
 files

Summary: @nh, does this do something reasonable on merges? We can refine the behavior ('click to show all 92 million files'), but I want to make sure it's at least feasible before we pursue it.

Test Plan: Set 1000 to "3" and looked at a change which touched 6 files.

Reviewers: nh, vrana, zjwsoft

Reviewed By: nh

CC: aran

Differential Revision: https://secure.phabricator.com/D4730
---
 .../controller/DiffusionCommitController.php  | 20 ++++++-
 .../pathchange/DiffusionPathChangeQuery.php   | 56 ++++++++++++++-----
 2 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index 1c796e51b3..69f96d2537 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -113,10 +113,18 @@ final class DiffusionCommitController extends DiffusionController {
     $content[] = $this->buildAuditTable($commit, $audit_requests);
     $content[] = $this->buildComments($commit);
 
+    $hard_limit = 1000;
+
     $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest(
       $drequest);
+    $change_query->setLimit($hard_limit + 1);
     $changes = $change_query->loadChanges();
 
+    $was_limited = (count($changes) > $hard_limit);
+    if ($was_limited) {
+      $changes = array_slice($changes, 0, $hard_limit);
+    }
+
     $content[] = $this->buildMergesTable($commit);
 
     $owners_paths = array();
@@ -171,11 +179,20 @@ final class DiffusionCommitController extends DiffusionController {
         "This commit hasn't been fully parsed yet (or doesn't affect any ".
         "paths).");
       $content[] = $no_changes;
+    } else if ($was_limited) {
+      $huge_commit = new AphrontErrorView();
+      $huge_commit->setSeverity(AphrontErrorView::SEVERITY_WARNING);
+      $huge_commit->setTitle(pht('Enormous Commit'));
+      $huge_commit->appendChild(
+        pht(
+          'This commit is enormous, and affects more than %d files. '.
+          'Changes are not shown.',
+          $hard_limit));
+      $content[] = $huge_commit;
     } else {
       $change_panel = new AphrontPanelView();
       $change_panel->setHeader("Changes (".number_format($count).")");
       $change_panel->setID('toc');
-
       if ($count > self::CHANGES_LIMIT) {
         $show_all_button = phutil_render_tag(
           'a',
@@ -307,7 +324,6 @@ final class DiffusionCommitController extends DiffusionController {
       'commit' => true,
     ));
 
-
     if ($changesets) {
       $nav = id(new DifferentialChangesetFileTreeSideNavBuilder())
         ->setAnchorName('top')
diff --git a/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php b/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php
index c7b53980ed..c661e55226 100644
--- a/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php
+++ b/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php
@@ -3,6 +3,16 @@
 final class DiffusionPathChangeQuery {
 
   private $request;
+  private $limit;
+
+  public function setLimit($limit) {
+    $this->limit = $limit;
+    return $this;
+  }
+
+  public function getLimit() {
+    return $this->limit;
+  }
 
   final private function __construct() {
     // <private>
@@ -31,20 +41,36 @@ final class DiffusionPathChangeQuery {
 
     $commit = $drequest->loadCommit();
 
+    $conn_r = $repository->establishConnection('r');
+
+    $limit = '';
+    if ($this->limit) {
+      $limit = qsprintf(
+        $conn_r,
+        'LIMIT %d',
+        $this->limit + 1);
+    }
+
     $raw_changes = queryfx_all(
-      $repository->establishConnection('r'),
+      $conn_r,
       'SELECT c.*, p.path pathName, t.path targetPathName,
           i.commitIdentifier targetCommitIdentifier
         FROM %T c
           LEFT JOIN %T p ON c.pathID = p.id
           LEFT JOIN %T t ON c.targetPathID = t.id
           LEFT JOIN %T i ON c.targetCommitID = i.id
-        WHERE c.commitID = %d AND isDirect = 1',
+        WHERE c.commitID = %d AND isDirect = 1 %Q',
       PhabricatorRepository::TABLE_PATHCHANGE,
       PhabricatorRepository::TABLE_PATH,
       PhabricatorRepository::TABLE_PATH,
       $commit->getTableName(),
-      $commit->getID());
+      $commit->getID(),
+      $limit);
+
+    $limited = $this->limit && (count($raw_changes) > $this->limit);
+    if ($limited) {
+      $raw_changes = array_slice($raw_changes, 0, $this->limit);
+    }
 
     $changes = array();
 
@@ -68,20 +94,22 @@ final class DiffusionPathChangeQuery {
       $changes[$id] = $change;
     }
 
-    // Deduce the away paths by examining all the changes.
+    // Deduce the away paths by examining all the changes, if we loaded them
+    // all.
 
-    $away = array();
-    foreach ($changes as $change) {
-      if ($change->getTargetPath()) {
-        $away[$change->getTargetPath()][] = $change->getPath();
+    if (!$limited) {
+      $away = array();
+      foreach ($changes as $change) {
+        if ($change->getTargetPath()) {
+          $away[$change->getTargetPath()][] = $change->getPath();
+        }
+      }
+      foreach ($changes as $change) {
+        if (isset($away[$change->getPath()])) {
+          $change->setAwayPaths($away[$change->getPath()]);
+        }
       }
     }
-    foreach ($changes as $change) {
-      if (isset($away[$change->getPath()])) {
-        $change->setAwayPaths($away[$change->getPath()]);
-      }
-    }
-
 
     return $changes;
   }