From efa8855e030876fdc2ee6274caa8c0e526d1a460 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 10 Aug 2015 14:16:36 -0700
Subject: [PATCH] Read Differential changeset coverage information from
 Harbormaster

Summary: Ref T8096. This information has moved into Harbormaster.

Test Plan:
Pushed some test results in; see right margin:

{F698394}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8096

Differential Revision: https://secure.phabricator.com/D13850
---
 .../DifferentialChangesetViewController.php   | 106 ++++++++----------
 .../DifferentialHarbormasterField.php         |  19 +---
 .../differential/storage/DifferentialDiff.php |  16 +++
 3 files changed, 70 insertions(+), 71 deletions(-)

diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php
index 28a6e26b2c..5efb8561af 100644
--- a/src/applications/differential/controller/DifferentialChangesetViewController.php
+++ b/src/applications/differential/controller/DifferentialChangesetViewController.php
@@ -127,29 +127,32 @@ final class DifferentialChangesetViewController extends DifferentialController {
       $changeset = $choice;
     }
 
-    $coverage = null;
-    if ($right && $right->getDiffID()) {
-      $unit = id(new DifferentialDiffProperty())->loadOneWhere(
-        'diffID = %d AND name = %s',
-        $right->getDiffID(),
-        'arc:unit');
-
-      if ($unit) {
-        $coverage = array();
-        foreach ($unit->getData() as $result) {
-          $result_coverage = idx($result, 'coverage');
-          if (!$result_coverage) {
-            continue;
-          }
-          $file_coverage = idx($result_coverage, $right->getFileName());
-          if (!$file_coverage) {
-            continue;
-          }
-          $coverage[] = $file_coverage;
-        }
-
-        $coverage = ArcanistUnitTestResult::mergeCoverage($coverage);
+    if ($left_new || $right_new) {
+      $diff_map = array();
+      if ($left) {
+        $diff_map[] = $left->getDiff();
       }
+      if ($right) {
+        $diff_map[] = $right->getDiff();
+      }
+      $diff_map = mpull($diff_map, null, 'getPHID');
+
+      $buildables = id(new HarbormasterBuildableQuery())
+        ->setViewer($viewer)
+        ->withBuildablePHIDs(array_keys($diff_map))
+        ->withManualBuildables(false)
+        ->needBuilds(true)
+        ->needTargets(true)
+        ->execute();
+      $buildables = mpull($buildables, null, 'getBuildablePHID');
+      foreach ($diff_map as $diff_phid => $changeset_diff) {
+        $changeset_diff->attachBuildable(idx($buildables, $diff_phid));
+      }
+    }
+
+    $coverage = null;
+    if ($right_new) {
+      $coverage = $this->loadCoverage($right);
     }
 
     $spec = $request->getStr('range');
@@ -203,29 +206,6 @@ final class DifferentialChangesetViewController extends DifferentialController {
       $inlines = array();
     }
 
-    if ($left_new || $right_new) {
-      $diff_map = array();
-      if ($left) {
-        $diff_map[] = $left->getDiff();
-      }
-      if ($right) {
-        $diff_map[] = $right->getDiff();
-      }
-      $diff_map = mpull($diff_map, null, 'getPHID');
-
-      $buildables = id(new HarbormasterBuildableQuery())
-        ->setViewer($viewer)
-        ->withBuildablePHIDs(array_keys($diff_map))
-        ->withManualBuildables(false)
-        ->needBuilds(true)
-        ->needTargets(true)
-        ->execute();
-      $buildables = mpull($buildables, null, 'getBuildablePHID');
-      foreach ($diff_map as $diff_phid => $changeset_diff) {
-        $changeset_diff->attachBuildable(idx($buildables, $diff_phid));
-      }
-    }
-
     if ($left_new) {
       $inlines = array_merge(
         $inlines,
@@ -381,18 +361,7 @@ final class DifferentialChangesetViewController extends DifferentialController {
   private function buildLintInlineComments($changeset) {
     $diff = $changeset->getDiff();
 
-    $buildable = $diff->getBuildable();
-    if (!$buildable) {
-      return array();
-    }
-
-    $target_phids = array();
-    foreach ($buildable->getBuilds() as $build) {
-      foreach ($build->getBuildTargets() as $target) {
-        $target_phids[] = $target->getPHID();
-      }
-    }
-
+    $target_phids = $diff->getBuildTargetPHIDs();
     if (!$target_phids) {
       return array();
     }
@@ -425,4 +394,27 @@ final class DifferentialChangesetViewController extends DifferentialController {
     return $inlines;
   }
 
+  private function loadCoverage(DifferentialChangeset $changeset) {
+    $target_phids = $changeset->getDiff()->getBuildTargetPHIDs();
+    if (!$target_phids) {
+      return array();
+    }
+
+    $unit = id(new HarbormasterBuildUnitMessage())->loadAllWhere(
+      'buildTargetPHID IN (%Ls)',
+      $target_phids);
+
+    $coverage = array();
+    foreach ($unit as $message) {
+      $test_coverage = $message->getProperty('coverage', array());
+      $coverage_data = idx($test_coverage, $changeset->getFileName());
+      if (!strlen($coverage_data)) {
+        continue;
+      }
+      $coverage[] = $coverage_data;
+    }
+
+    return ArcanistUnitTestResult::mergeCoverage($coverage);
+  }
+
 }
diff --git a/src/applications/differential/customfield/DifferentialHarbormasterField.php b/src/applications/differential/customfield/DifferentialHarbormasterField.php
index f84bb74f29..c9b573bda2 100644
--- a/src/applications/differential/customfield/DifferentialHarbormasterField.php
+++ b/src/applications/differential/customfield/DifferentialHarbormasterField.php
@@ -29,20 +29,11 @@ abstract class DifferentialHarbormasterField
       $diff->attachProperty($key, idx($properties, $key));
     }
 
-    $messages = array();
-
-    $buildable = $diff->getBuildable();
-    if ($buildable) {
-      $target_phids = array();
-      foreach ($buildable->getBuilds() as $build) {
-        foreach ($build->getBuildTargets() as $target) {
-          $target_phids[] = $target->getPHID();
-        }
-      }
-
-      if ($target_phids) {
-        $messages = $this->loadHarbormasterTargetMessages($target_phids);
-      }
+    $target_phids = $diff->getBuildTargetPHIDs();
+    if ($target_phids) {
+      $messages = $this->loadHarbormasterTargetMessages($target_phids);
+    } else {
+      $messages = array();
     }
 
     if (!$messages) {
diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php
index 4a0e63d5a7..e30a401306 100644
--- a/src/applications/differential/storage/DifferentialDiff.php
+++ b/src/applications/differential/storage/DifferentialDiff.php
@@ -331,6 +331,22 @@ final class DifferentialDiff
     return $this->assertAttached($this->buildable);
   }
 
+  public function getBuildTargetPHIDs() {
+    $buildable = $this->getBuildable();
+
+    if (!$buildable) {
+      return array();
+    }
+
+    $target_phids = array();
+    foreach ($buildable->getBuilds() as $build) {
+      foreach ($build->getBuildTargets() as $target) {
+        $target_phids[] = $target->getPHID();
+      }
+    }
+
+    return $target_phids;
+  }
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */