From 71e9fb96b56e5b7a52e5427453d6b58c19a81a16 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 25 May 2014 08:09:01 -0700
Subject: [PATCH] Move more hunk loads into DifferentialHunkQuery

Summary: Ref T5179. Ref T4045. I want to move all hunk loads into DifferentialHunkQuery so I can make it do magical things where hunks come from multiple places, handle non-utf8 encodings properly, handle compression, archive into Files, and so on.

Test Plan: Viewed some revisions. Called `differential.getrawdiff`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4045, T5179

Differential Revision: https://secure.phabricator.com/D9287
---
 src/__phutil_library_map__.php                |  6 +++-
 ...duitAPI_differential_getrawdiff_Method.php | 16 ++++-----
 .../DifferentialHunkParserTestCase.php        | 21 +++++++----
 .../query/DifferentialDiffQuery.php           | 35 +++++++++++++------
 .../render/DifferentialRawDiffRenderer.php    |  4 ---
 .../storage/DifferentialChangeset.php         | 28 ++++++++++++++-
 6 files changed, 76 insertions(+), 34 deletions(-)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 132149f3cc..93a3827bfb 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3007,7 +3007,11 @@ phutil_register_library_map(array(
     'DifferentialBranchField' => 'DifferentialCustomField',
     'DifferentialCapabilityDefaultView' => 'PhabricatorPolicyCapability',
     'DifferentialChangesSinceLastUpdateField' => 'DifferentialCustomField',
-    'DifferentialChangeset' => 'DifferentialDAO',
+    'DifferentialChangeset' =>
+    array(
+      0 => 'DifferentialDAO',
+      1 => 'PhabricatorPolicyInterface',
+    ),
     'DifferentialChangesetDetailView' => 'AphrontView',
     'DifferentialChangesetHTMLRenderer' => 'DifferentialChangesetRenderer',
     'DifferentialChangesetListView' => 'AphrontView',
diff --git a/src/applications/differential/conduit/ConduitAPI_differential_getrawdiff_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_getrawdiff_Method.php
index 188695c173..5970012eb1 100644
--- a/src/applications/differential/conduit/ConduitAPI_differential_getrawdiff_Method.php
+++ b/src/applications/differential/conduit/ConduitAPI_differential_getrawdiff_Method.php
@@ -31,23 +31,19 @@ final class ConduitAPI_differential_getrawdiff_Method
     $diff = id(new DifferentialDiffQuery())
       ->withIDs(array($diff_id))
       ->setViewer($viewer)
+      ->needChangesets(true)
       ->executeOne();
 
     if (!$diff) {
       throw new ConduitException('ERR_NOT_FOUND');
     }
 
-    $changesets = $diff->loadChangesets();
-    foreach ($changesets as $changeset) {
-      $changeset->attachHunks(
-        $changeset->loadHunks());
-    }
+    $renderer = id(new DifferentialRawDiffRenderer())
+      ->setChangesets($diff->getChangesets())
+      ->setViewer($viewer)
+      ->setFormat('git');
 
-    $renderer = new DifferentialRawDiffRenderer();
-    $renderer->setChangesets($changesets);
-    $renderer->setViewer($viewer);
-    $renderer->setFormat('git');
     return $renderer->buildPatch();
-
   }
+
 }
diff --git a/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php b/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php
index dae9fb48ac..664a77f3ac 100644
--- a/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php
+++ b/src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php
@@ -1,22 +1,29 @@
 <?php
 
 final class DifferentialHunkParserTestCase extends PhabricatorTestCase {
+
   private function createComment() {
     $comment = new DifferentialInlineComment();
     return $comment;
   }
 
+  private function createHunk(
+    $old_offset,
+    $old_len,
+    $new_offset,
+    $new_len,
+    $changes) {
 
+    $hunk = id(new DifferentialHunk())
+      ->setOldOffset($old_offset)
+      ->setOldLen($old_len)
+      ->setNewOffset($new_offset)
+      ->setNewLen($new_len)
+      ->setChanges($changes);
 
-  private function createHunk($oldOffset, $oldLen, $newOffset, $newLen, $changes) {
-    $hunk = new DifferentialHunk();
-    $hunk->setOldOffset($oldOffset);
-    $hunk->setOldLen($oldLen);
-    $hunk->setNewOffset($newOffset);
-    $hunk->setNewLen($newLen);
-    $hunk->setChanges($changes);
     return $hunk;
   }
+
   // Returns a change that consists of a single hunk, starting at line 1.
   private function createSingleChange($old_lines, $new_lines, $changes) {
     return array(
diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php
index e32b641e36..53e9b7782e 100644
--- a/src/applications/differential/query/DifferentialDiffQuery.php
+++ b/src/applications/differential/query/DifferentialDiffQuery.php
@@ -76,26 +76,39 @@ final class DifferentialDiffQuery
     }
 
 
-    if ($this->needChangesets) {
-      $this->loadChangesets($diffs);
+    if ($diffs && $this->needChangesets) {
+      $diffs = $this->loadChangesets($diffs);
     }
 
-    if ($this->needArcanistProjects) {
-      $this->loadArcanistProjects($diffs);
+    if ($diffs && $this->needArcanistProjects) {
+      $diffs = $this->loadArcanistProjects($diffs);
     }
 
     return $diffs;
   }
 
   private function loadChangesets(array $diffs) {
-    foreach ($diffs as $diff) {
-      $diff->attachChangesets(
-        $diff->loadRelatives(new DifferentialChangeset(), 'diffID'));
-      foreach ($diff->getChangesets() as $changeset) {
-        $changeset->attachHunks(
-          $changeset->loadRelatives(new DifferentialHunk(), 'changesetID'));
-      }
+    $diff_ids = mpull($diffs, 'getID');
+
+    $changesets = id(new DifferentialChangeset())->loadAllWhere(
+      'diffID IN (%Ld)',
+      $diff_ids);
+
+    if ($changesets) {
+      id(new DifferentialHunkQuery())
+        ->setViewer($this->getViewer())
+        ->setParentQuery($this)
+        ->withChangesets($changesets)
+        ->needAttachToChangesets(true)
+        ->execute();
     }
+
+    $changeset_groups = mgroup($changesets, 'getDiffID');
+    foreach ($diffs as $diff) {
+      $diff_changesets = idx($changeset_groups, $diff->getID(), array());
+      $diff->attachChangesets($diff_changesets);
+    }
+
     return $diffs;
   }
 
diff --git a/src/applications/differential/render/DifferentialRawDiffRenderer.php b/src/applications/differential/render/DifferentialRawDiffRenderer.php
index e8a1d09a60..65303e55d2 100644
--- a/src/applications/differential/render/DifferentialRawDiffRenderer.php
+++ b/src/applications/differential/render/DifferentialRawDiffRenderer.php
@@ -37,10 +37,6 @@ final class DifferentialRawDiffRenderer {
   public function buildPatch() {
     $diff = new DifferentialDiff();
     $diff->attachChangesets($this->getChangesets());
-    foreach ($diff->getChangesets() as $changeset) {
-      $changeset->attachHunks(
-        $changeset->loadRelatives(new DifferentialHunk(), 'changesetID'));
-    }
 
     $raw_changes = $diff->buildChangesList();
     $changes = array();
diff --git a/src/applications/differential/storage/DifferentialChangeset.php b/src/applications/differential/storage/DifferentialChangeset.php
index bd6ce67af6..53632c1234 100644
--- a/src/applications/differential/storage/DifferentialChangeset.php
+++ b/src/applications/differential/storage/DifferentialChangeset.php
@@ -1,6 +1,7 @@
 <?php
 
-final class DifferentialChangeset extends DifferentialDAO {
+final class DifferentialChangeset extends DifferentialDAO
+  implements PhabricatorPolicyInterface {
 
   protected $diffID;
   protected $oldFile;
@@ -16,6 +17,7 @@ final class DifferentialChangeset extends DifferentialDAO {
 
   private $unsavedHunks = array();
   private $hunks = self::ATTACHABLE;
+  private $diff = self::ATTACHABLE;
 
   const TABLE_CACHE = 'differential_changeset_parse_cache';
 
@@ -172,4 +174,28 @@ final class DifferentialChangeset extends DifferentialDAO {
     return false;
   }
 
+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return array(
+      PhabricatorPolicyCapability::CAN_VIEW,
+    );
+  }
+
+  public function getPolicy($capability) {
+    // TODO: For now, these are never queried directly through the policy
+    // framework. Fix that up.
+    return PhabricatorPolicies::getMostOpenPolicy();
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return false;
+  }
+
+  public function describeAutomaticCapability($capability) {
+    return null;
+  }
+
 }