From aaf1320b02cfc14fa718f310512a19be3dbad02e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 14 Apr 2014 12:06:20 -0700
Subject: [PATCH] Simplify Herald logic for loading Differential changes

Summary: Ref T4045. These three methods are fairly copy-pastey. Provide a more formal DifferentialHunk API for querying various types of line ranges.

Test Plan: Used test console to verify that "added content", "removed content", and "changed content" rules still produce the same data.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4045

Differential Revision: https://secure.phabricator.com/D8764
---
 .../differential/storage/DifferentialHunk.php | 24 ++++++++
 .../HeraldDifferentialRevisionAdapter.php     | 55 ++++---------------
 2 files changed, 34 insertions(+), 45 deletions(-)

diff --git a/src/applications/differential/storage/DifferentialHunk.php b/src/applications/differential/storage/DifferentialHunk.php
index 8efc8da499..3ffb11b9b0 100644
--- a/src/applications/differential/storage/DifferentialHunk.php
+++ b/src/applications/differential/storage/DifferentialHunk.php
@@ -9,6 +9,10 @@ final class DifferentialHunk extends DifferentialDAO {
   protected $newOffset;
   protected $newLen;
 
+  const FLAG_LINES_ADDED     = 1;
+  const FLAG_LINES_REMOVED   = 2;
+  const FLAG_LINES_STABLE    = 4;
+
   public function getAddedLines() {
     return $this->makeContent($include = '+');
   }
@@ -29,6 +33,26 @@ final class DifferentialHunk extends DifferentialDAO {
     return implode('', $this->makeContent($include = '-+'));
   }
 
+  public function getContentWithMask($mask) {
+    $include = array();
+
+    if (($mask & self::FLAG_LINES_ADDED)) {
+      $include[] = '+';
+    }
+
+    if (($mask & self::FLAG_LINES_REMOVED)) {
+      $include[] = '-';
+    }
+
+    if (($mask & self::FLAG_LINES_STABLE)) {
+      $include[] = ' ';
+    }
+
+    $include = implode('', $include);
+
+    return implode('', $this->makeContent($include));
+  }
+
   final private function makeContent($include) {
     $results = array();
     $lines = explode("\n", $this->changes);
diff --git a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
index 52f22f91a0..1815d43be1 100644
--- a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
+++ b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
@@ -204,56 +204,21 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
   }
 
   protected function loadContentDictionary() {
-    $changesets = $this->loadChangesets();
-
-    $hunks = array();
-    if ($changesets) {
-      $hunks = id(new DifferentialHunk())->loadAllWhere(
-        'changesetID in (%Ld)',
-        mpull($changesets, 'getID'));
-    }
-
-    $dict = array();
-    $hunks = mgroup($hunks, 'getChangesetID');
-    $changesets = mpull($changesets, null, 'getID');
-    foreach ($changesets as $id => $changeset) {
-      $path = $this->getAbsoluteRepositoryPathForChangeset($changeset);
-      $content = array();
-      foreach (idx($hunks, $id, array()) as $hunk) {
-        $content[] = $hunk->makeChanges();
-      }
-      $dict[$path] = implode("\n", $content);
-    }
-
-    return $dict;
+    $add_lines = DifferentialHunk::FLAG_LINES_ADDED;
+    $rem_lines = DifferentialHunk::FLAG_LINES_REMOVED;
+    $mask = ($add_lines | $rem_lines);
+    return $this->loadContentWithMask($mask);
   }
 
   protected function loadAddedContentDictionary() {
-    $changesets = $this->loadChangesets();
-
-    $hunks = array();
-    if ($changesets) {
-      $hunks = id(new DifferentialHunk())->loadAllWhere(
-        'changesetID in (%Ld)',
-        mpull($changesets, 'getID'));
-    }
-
-    $dict = array();
-    $hunks = mgroup($hunks, 'getChangesetID');
-    $changesets = mpull($changesets, null, 'getID');
-    foreach ($changesets as $id => $changeset) {
-      $path = $this->getAbsoluteRepositoryPathForChangeset($changeset);
-      $content = array();
-      foreach (idx($hunks, $id, array()) as $hunk) {
-        $content[] = implode('', $hunk->getAddedLines());
-      }
-      $dict[$path] = implode("\n", $content);
-    }
-
-    return $dict;
+    return $this->loadContentWithMask(DifferentialHunk::FLAG_LINES_ADDED);
   }
 
   protected function loadRemovedContentDictionary() {
+    return $this->loadContentWithMask(DifferentialHunk::FLAG_LINES_REMOVED);
+  }
+
+  private function loadContentWithMask($mask) {
     $changesets = $this->loadChangesets();
 
     $hunks = array();
@@ -270,7 +235,7 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
       $path = $this->getAbsoluteRepositoryPathForChangeset($changeset);
       $content = array();
       foreach (idx($hunks, $id, array()) as $hunk) {
-        $content[] = implode('', $hunk->getRemovedLines());
+        $content[] = $hunk->getContentWithMask($mask);
       }
       $dict[$path] = implode("\n", $content);
     }