Consolidate burndown logic into a "BurndownChartEngine"

Summary:
Ref T13279. For now, we need to render burndowns from both Maniphest (legacy) and Projects (new prototype).

Consolidate this logic into a "BurndownChartEngine". I plan to expand this to work a bit like a "SearchEngine", and serve as a UI layer on top of the raw chart features.

The old "ChartEngine" is now "ChartRenderingEngine".

Test Plan:
  - Viewed burndowns ("burnups") in Maniphest.
  - Viewed burndowns in Projects.
  - Saw the same chart.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: yelirekim

Maniphest Tasks: T13279

Differential Revision: https://secure.phabricator.com/D20496

src/__phutil_library_map__.php

@@ -2668,6 +2668,7 @@ phutil_register_library_map(array(
     'PhabricatorChartFunction' => 'applications/fact/chart/PhabricatorChartFunction.php',
     'PhabricatorChartFunctionArgument' => 'applications/fact/chart/PhabricatorChartFunctionArgument.php',
     'PhabricatorChartFunctionArgumentParser' => 'applications/fact/chart/PhabricatorChartFunctionArgumentParser.php',
+    'PhabricatorChartRenderingEngine' => 'applications/fact/engine/PhabricatorChartRenderingEngine.php',
     'PhabricatorChatLogApplication' => 'applications/chatlog/application/PhabricatorChatLogApplication.php',
     'PhabricatorChatLogChannel' => 'applications/chatlog/storage/PhabricatorChatLogChannel.php',
     'PhabricatorChatLogChannelListController' => 'applications/chatlog/controller/PhabricatorChatLogChannelListController.php',
@@ -4142,6 +4143,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectBoardReorderController' => 'applications/project/controller/PhabricatorProjectBoardReorderController.php',
     'PhabricatorProjectBoardViewController' => 'applications/project/controller/PhabricatorProjectBoardViewController.php',
     'PhabricatorProjectBuiltinsExample' => 'applications/uiexample/examples/PhabricatorProjectBuiltinsExample.php',
+    'PhabricatorProjectBurndownChartEngine' => 'applications/project/chart/PhabricatorProjectBurndownChartEngine.php',
     'PhabricatorProjectCardView' => 'applications/project/view/PhabricatorProjectCardView.php',
     'PhabricatorProjectColorTransaction' => 'applications/project/xaction/PhabricatorProjectColorTransaction.php',
     'PhabricatorProjectColorsConfigType' => 'applications/project/config/PhabricatorProjectColorsConfigType.php',
@@ -8680,6 +8682,7 @@ phutil_register_library_map(array(
     'PhabricatorChartFunction' => 'Phobject',
     'PhabricatorChartFunctionArgument' => 'Phobject',
     'PhabricatorChartFunctionArgumentParser' => 'Phobject',
+    'PhabricatorChartRenderingEngine' => 'Phobject',
     'PhabricatorChatLogApplication' => 'PhabricatorApplication',
     'PhabricatorChatLogChannel' => array(
       'PhabricatorChatLogDAO',
@@ -10383,6 +10386,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectBoardReorderController' => 'PhabricatorProjectBoardController',
     'PhabricatorProjectBoardViewController' => 'PhabricatorProjectBoardController',
     'PhabricatorProjectBuiltinsExample' => 'PhabricatorUIExample',
+    'PhabricatorProjectBurndownChartEngine' => 'PhabricatorChartEngine',
     'PhabricatorProjectCardView' => 'AphrontTagView',
     'PhabricatorProjectColorTransaction' => 'PhabricatorProjectTransactionType',
     'PhabricatorProjectColorsConfigType' => 'PhabricatorJSONConfigType',

src/applications/dashboard/paneltype/PhabricatorDashboardChartPanelType.php

@@ -37,7 +37,7 @@ final class PhabricatorDashboardChartPanelType
     PhabricatorDashboardPanel $panel,
     PhabricatorDashboardPanelRenderingEngine $engine) {
 
-    $engine = id(new PhabricatorChartEngine())
+    $engine = id(new PhabricatorChartRenderingEngine())
       ->setViewer($viewer);
 
     $chart = $engine->loadChart($panel->getProperty('chartKey'));
@@ -55,7 +55,7 @@ final class PhabricatorDashboardChartPanelType
     PHUIHeaderView $header) {
 
     $key = $panel->getProperty('chartKey');
-    $uri = PhabricatorChartEngine::getChartURI($key);
+    $uri = PhabricatorChartRenderingEngine::getChartURI($key);
 
     $icon = id(new PHUIIconView())
       ->setIcon('fa-area-chart');

src/applications/fact/controller/PhabricatorFactChartController.php

@@ -10,7 +10,7 @@ final class PhabricatorFactChartController extends PhabricatorFactController {
       return $this->newDemoChart();
     }
 
-    $engine = id(new PhabricatorChartEngine())
+    $engine = id(new PhabricatorChartRenderingEngine())
       ->setViewer($viewer);
 
     $chart = $engine->loadChart($chart_key);
@@ -95,7 +95,7 @@ final class PhabricatorFactChartController extends PhabricatorFactController {
     $chart = id(new PhabricatorFactChart())
       ->setDatasets($datasets);
 
-    $engine = id(new PhabricatorChartEngine())
+    $engine = id(new PhabricatorChartRenderingEngine())
       ->setViewer($viewer)
       ->setChart($chart);
 

src/applications/fact/engine/PhabricatorChartEngine.php

@@ -1,232 +1,48 @@
 <?php
 
-final class PhabricatorChartEngine
+abstract class PhabricatorChartEngine
   extends Phobject {
 
   private $viewer;
-  private $chart;
-  private $storedChart;
 
-  public function setViewer(PhabricatorUser $viewer) {
+  final public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
     return $this;
   }
 
-  public function getViewer() {
+  final public function getViewer() {
     return $this->viewer;
   }
 
-  public function setChart(PhabricatorFactChart $chart) {
-    $this->chart = $chart;
-    return $this;
+  final public function getChartEngineKey() {
+    return $this->getPhobjectClassConstant('CHARTENGINEKEY', 32);
   }
 
-  public function getChart() {
-    return $this->chart;
+  abstract protected function newChart();
+
+  final public function buildChart() {
+    $viewer = $this->getViewer();
+
+    $chart = $this->newChart();
+
+    $rendering_engine = id(new PhabricatorChartRenderingEngine())
+      ->setViewer($viewer)
+      ->setChart($chart);
+
+    return $rendering_engine->getStoredChart();
   }
 
-  public function loadChart($chart_key) {
-    $chart = id(new PhabricatorFactChart())->loadOneWhere(
-      'chartKey = %s',
-      $chart_key);
+  final public function buildChartPanel() {
+    $chart = $this->buildChart();
 
-    if ($chart) {
-      $this->setChart($chart);
-    }
+    $panel_type = id(new PhabricatorDashboardChartPanelType())
+      ->getPanelTypeKey();
 
-    return $chart;
-  }
+    $chart_panel = id(new PhabricatorDashboardPanel())
+      ->setPanelType($panel_type)
+      ->setProperty('chartKey', $chart->getChartKey());
 
-  public static function getChartURI($chart_key) {
-    return id(new PhabricatorFactChart())
-      ->setChartKey($chart_key)
-      ->getURI();
-  }
-
-  public function getStoredChart() {
-    if (!$this->storedChart) {
-      $chart = $this->getChart();
-      $chart_key = $chart->getChartKey();
-      if (!$chart_key) {
-        $chart_key = $chart->newChartKey();
-
-        $stored_chart = id(new PhabricatorFactChart())->loadOneWhere(
-          'chartKey = %s',
-          $chart_key);
-        if ($stored_chart) {
-          $chart = $stored_chart;
-        } else {
-          $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
-
-          try {
-            $chart->save();
-          } catch (AphrontDuplicateKeyQueryException $ex) {
-            $chart = id(new PhabricatorFactChart())->loadOneWhere(
-              'chartKey = %s',
-              $chart_key);
-            if (!$chart) {
-              throw new Exception(
-                pht(
-                  'Failed to load chart with key "%s" after key collision. '.
-                  'This should not be possible.',
-                  $chart_key));
-            }
-          }
-
-          unset($unguarded);
-        }
-        $this->setChart($chart);
-      }
-
-      $this->storedChart = $chart;
-    }
-
-    return $this->storedChart;
-  }
-
-  public function newChartView() {
-    $chart = $this->getStoredChart();
-    $chart_key = $chart->getChartKey();
-
-    $chart_node_id = celerity_generate_unique_node_id();
-
-    $chart_view = phutil_tag(
-      'div',
-      array(
-        'id' => $chart_node_id,
-        'style' => 'background: #ffffff; '.
-                   'height: 480px; ',
-      ),
-      '');
-
-    $data_uri = urisprintf('/fact/chart/%s/draw/', $chart_key);
-
-    Javelin::initBehavior(
-      'line-chart',
-      array(
-        'chartNodeID' => $chart_node_id,
-        'dataURI' => (string)$data_uri,
-      ));
-
-    return $chart_view;
-  }
-
-  public function newChartData() {
-    $chart = $this->getStoredChart();
-    $chart_key = $chart->getChartKey();
-
-    $datasets = $chart->getDatasets();
-
-    $functions = array();
-    foreach ($datasets as $dataset) {
-      $functions[] = $dataset->getFunction();
-    }
-
-    $subfunctions = array();
-    foreach ($functions as $function) {
-      foreach ($function->getSubfunctions() as $subfunction) {
-        $subfunctions[] = $subfunction;
-      }
-    }
-
-    foreach ($subfunctions as $subfunction) {
-      $subfunction->loadData();
-    }
-
-    list($domain_min, $domain_max) = $this->getDomain($functions);
-
-    $axis = id(new PhabricatorChartAxis())
-      ->setMinimumValue($domain_min)
-      ->setMaximumValue($domain_max);
-
-    $data_query = id(new PhabricatorChartDataQuery())
-      ->setMinimumValue($domain_min)
-      ->setMaximumValue($domain_max)
-      ->setLimit(2000);
-
-    $datasets = array();
-    foreach ($functions as $function) {
-      $points = $function->newDatapoints($data_query);
-
-      $x = array();
-      $y = array();
-
-      foreach ($points as $point) {
-        $x[] = $point['x'];
-        $y[] = $point['y'];
-      }
-
-      $datasets[] = array(
-        'x' => $x,
-        'y' => $y,
-        'color' => '#ff00ff',
-      );
-    }
-
-
-    $y_min = 0;
-    $y_max = 0;
-    foreach ($datasets as $dataset) {
-      if (!$dataset['y']) {
-        continue;
-      }
-
-      $y_min = min($y_min, min($dataset['y']));
-      $y_max = max($y_max, max($dataset['y']));
-    }
-
-    $chart_data = array(
-      'datasets' => $datasets,
-      'xMin' => $domain_min,
-      'xMax' => $domain_max,
-      'yMin' => $y_min,
-      'yMax' => $y_max,
-    );
-
-    return $chart_data;
-  }
-
-  private function getDomain(array $functions) {
-    $domain_min_list = null;
-    $domain_max_list = null;
-
-    foreach ($functions as $function) {
-      $domain = $function->getDomain();
-
-      list($function_min, $function_max) = $domain;
-
-      if ($function_min !== null) {
-        $domain_min_list[] = $function_min;
-      }
-
-      if ($function_max !== null) {
-        $domain_max_list[] = $function_max;
-      }
-    }
-
-    $domain_min = null;
-    $domain_max = null;
-
-    if ($domain_min_list) {
-      $domain_min = min($domain_min_list);
-    }
-
-    if ($domain_max_list) {
-      $domain_max = max($domain_max_list);
-    }
-
-    // If we don't have any domain data from the actual functions, pick a
-    // plausible domain automatically.
-
-    if ($domain_max === null) {
-      $domain_max = PhabricatorTime::getNow();
-    }
-
-    if ($domain_min === null) {
-      $domain_min = $domain_max - phutil_units('365 days in seconds');
-    }
-
-    return array($domain_min, $domain_max);
+    return $chart_panel;
   }
 
 }

src/applications/fact/engine/PhabricatorChartRenderingEngine.php

@@ -0,0 +1,232 @@
+<?php
+
+final class PhabricatorChartRenderingEngine
+  extends Phobject {
+
+  private $viewer;
+  private $chart;
+  private $storedChart;
+
+  public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  public function getViewer() {
+    return $this->viewer;
+  }
+
+  public function setChart(PhabricatorFactChart $chart) {
+    $this->chart = $chart;
+    return $this;
+  }
+
+  public function getChart() {
+    return $this->chart;
+  }
+
+  public function loadChart($chart_key) {
+    $chart = id(new PhabricatorFactChart())->loadOneWhere(
+      'chartKey = %s',
+      $chart_key);
+
+    if ($chart) {
+      $this->setChart($chart);
+    }
+
+    return $chart;
+  }
+
+  public static function getChartURI($chart_key) {
+    return id(new PhabricatorFactChart())
+      ->setChartKey($chart_key)
+      ->getURI();
+  }
+
+  public function getStoredChart() {
+    if (!$this->storedChart) {
+      $chart = $this->getChart();
+      $chart_key = $chart->getChartKey();
+      if (!$chart_key) {
+        $chart_key = $chart->newChartKey();
+
+        $stored_chart = id(new PhabricatorFactChart())->loadOneWhere(
+          'chartKey = %s',
+          $chart_key);
+        if ($stored_chart) {
+          $chart = $stored_chart;
+        } else {
+          $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+
+          try {
+            $chart->save();
+          } catch (AphrontDuplicateKeyQueryException $ex) {
+            $chart = id(new PhabricatorFactChart())->loadOneWhere(
+              'chartKey = %s',
+              $chart_key);
+            if (!$chart) {
+              throw new Exception(
+                pht(
+                  'Failed to load chart with key "%s" after key collision. '.
+                  'This should not be possible.',
+                  $chart_key));
+            }
+          }
+
+          unset($unguarded);
+        }
+        $this->setChart($chart);
+      }
+
+      $this->storedChart = $chart;
+    }
+
+    return $this->storedChart;
+  }
+
+  public function newChartView() {
+    $chart = $this->getStoredChart();
+    $chart_key = $chart->getChartKey();
+
+    $chart_node_id = celerity_generate_unique_node_id();
+
+    $chart_view = phutil_tag(
+      'div',
+      array(
+        'id' => $chart_node_id,
+        'style' => 'background: #ffffff; '.
+                   'height: 480px; ',
+      ),
+      '');
+
+    $data_uri = urisprintf('/fact/chart/%s/draw/', $chart_key);
+
+    Javelin::initBehavior(
+      'line-chart',
+      array(
+        'chartNodeID' => $chart_node_id,
+        'dataURI' => (string)$data_uri,
+      ));
+
+    return $chart_view;
+  }
+
+  public function newChartData() {
+    $chart = $this->getStoredChart();
+    $chart_key = $chart->getChartKey();
+
+    $datasets = $chart->getDatasets();
+
+    $functions = array();
+    foreach ($datasets as $dataset) {
+      $functions[] = $dataset->getFunction();
+    }
+
+    $subfunctions = array();
+    foreach ($functions as $function) {
+      foreach ($function->getSubfunctions() as $subfunction) {
+        $subfunctions[] = $subfunction;
+      }
+    }
+
+    foreach ($subfunctions as $subfunction) {
+      $subfunction->loadData();
+    }
+
+    list($domain_min, $domain_max) = $this->getDomain($functions);
+
+    $axis = id(new PhabricatorChartAxis())
+      ->setMinimumValue($domain_min)
+      ->setMaximumValue($domain_max);
+
+    $data_query = id(new PhabricatorChartDataQuery())
+      ->setMinimumValue($domain_min)
+      ->setMaximumValue($domain_max)
+      ->setLimit(2000);
+
+    $datasets = array();
+    foreach ($functions as $function) {
+      $points = $function->newDatapoints($data_query);
+
+      $x = array();
+      $y = array();
+
+      foreach ($points as $point) {
+        $x[] = $point['x'];
+        $y[] = $point['y'];
+      }
+
+      $datasets[] = array(
+        'x' => $x,
+        'y' => $y,
+        'color' => '#ff00ff',
+      );
+    }
+
+
+    $y_min = 0;
+    $y_max = 0;
+    foreach ($datasets as $dataset) {
+      if (!$dataset['y']) {
+        continue;
+      }
+
+      $y_min = min($y_min, min($dataset['y']));
+      $y_max = max($y_max, max($dataset['y']));
+    }
+
+    $chart_data = array(
+      'datasets' => $datasets,
+      'xMin' => $domain_min,
+      'xMax' => $domain_max,
+      'yMin' => $y_min,
+      'yMax' => $y_max,
+    );
+
+    return $chart_data;
+  }
+
+  private function getDomain(array $functions) {
+    $domain_min_list = null;
+    $domain_max_list = null;
+
+    foreach ($functions as $function) {
+      $domain = $function->getDomain();
+
+      list($function_min, $function_max) = $domain;
+
+      if ($function_min !== null) {
+        $domain_min_list[] = $function_min;
+      }
+
+      if ($function_max !== null) {
+        $domain_max_list[] = $function_max;
+      }
+    }
+
+    $domain_min = null;
+    $domain_max = null;
+
+    if ($domain_min_list) {
+      $domain_min = min($domain_min_list);
+    }
+
+    if ($domain_max_list) {
+      $domain_max = max($domain_max_list);
+    }
+
+    // If we don't have any domain data from the actual functions, pick a
+    // plausible domain automatically.
+
+    if ($domain_max === null) {
+      $domain_max = PhabricatorTime::getNow();
+    }
+
+    if ($domain_min === null) {
+      $domain_min = $domain_max - phutil_units('365 days in seconds');
+    }
+
+    return array($domain_min, $domain_max);
+  }
+
+}

src/applications/maniphest/controller/ManiphestReportController.php

@@ -381,53 +381,20 @@ final class ManiphestReportController extends ManiphestController {
     list($burn_x, $burn_y) = $this->buildSeries($data);
 
     if ($project_phid) {
-      $argv = array(
-        'sum',
-        array(
-          'accumulate',
-          array('fact', 'tasks.open-count.create.project', $project_phid),
-        ),
-        array(
-          'accumulate',
-          array('fact', 'tasks.open-count.status.project', $project_phid),
-        ),
-        array(
-          'accumulate',
-          array('fact', 'tasks.open-count.assign.project', $project_phid),
-        ),
-      );
+      $projects = id(new PhabricatorProjectQuery())
+        ->setViewer($viewer)
+        ->withPHIDs(array($project_phid))
+        ->execute();
     } else {
-      $argv = array(
-        'sum',
-        array('accumulate', array('fact', 'tasks.open-count.create')),
-        array('accumulate', array('fact', 'tasks.open-count.status')),
-      );
+      $projects = array();
     }
 
-    $function = id(new PhabricatorComposeChartFunction())
-      ->setArguments(array($argv));
-
-    $datasets = array(
-      id(new PhabricatorChartDataset())
-        ->setFunction($function),
-    );
-
-    $chart = id(new PhabricatorFactChart())
-      ->setDatasets($datasets);
-
-    $engine = id(new PhabricatorChartEngine())
+    $panel = id(new PhabricatorProjectBurndownChartEngine())
       ->setViewer($viewer)
-      ->setChart($chart);
+      ->setProjects($projects)
+      ->buildChartPanel();
 
-    $chart = $engine->getStoredChart();
-
-    $panel_type = id(new PhabricatorDashboardChartPanelType())
-      ->getPanelTypeKey();
-
-    $chart_panel = id(new PhabricatorDashboardPanel())
-      ->setPanelType($panel_type)
-      ->setName(pht('Burnup Rate'))
-      ->setProperty('chartKey', $chart->getChartKey());
+    $chart_panel = $panel->setName(pht('Burnup Rate'));
 
     $chart_view = id(new PhabricatorDashboardPanelRenderingEngine())
       ->setViewer($viewer)

src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php

@@ -0,0 +1,71 @@
+<?php
+
+final class PhabricatorProjectBurndownChartEngine
+  extends PhabricatorChartEngine {
+
+  const CHARTENGINEKEY = 'project.burndown';
+
+  private $projects;
+
+  public function setProjects(array $projects) {
+    assert_instances_of($projects, 'PhabricatorProject');
+
+    $this->projects = $projects;
+
+    return $this;
+  }
+
+  public function getProjects() {
+    return $this->projects;
+  }
+
+  protected function newChart() {
+    if ($this->projects !== null) {
+      $project_phids = mpull($this->projects, 'getPHID');
+    } else {
+      $project_phids = null;
+    }
+
+    $argvs = array();
+    if ($project_phids) {
+      foreach ($project_phids as $project_phid) {
+        $argvs[] = array(
+          'sum',
+          array(
+            'accumulate',
+            array('fact', 'tasks.open-count.create.project', $project_phid),
+          ),
+          array(
+            'accumulate',
+            array('fact', 'tasks.open-count.status.project', $project_phid),
+          ),
+          array(
+            'accumulate',
+            array('fact', 'tasks.open-count.assign.project', $project_phid),
+          ),
+        );
+      }
+    } else {
+      $argvs[] = array(
+        'sum',
+        array('accumulate', array('fact', 'tasks.open-count.create')),
+        array('accumulate', array('fact', 'tasks.open-count.status')),
+      );
+    }
+
+    $datasets = array();
+    foreach ($argvs as $argv) {
+      $function = id(new PhabricatorComposeChartFunction())
+        ->setArguments(array($argv));
+
+      $datasets[] = id(new PhabricatorChartDataset())
+        ->setFunction($function);
+    }
+
+    $chart = id(new PhabricatorFactChart())
+      ->setDatasets($datasets);
+
+    return $chart;
+  }
+
+}

src/applications/project/controller/PhabricatorProjectReportsController.php

@@ -31,48 +31,12 @@ final class PhabricatorProjectReportsController
     $crumbs->addTextCrumb(pht('Reports'));
     $crumbs->setBorder(true);
 
-    $project_phid = $project->getPHID();
-
-    $argv = array(
-      'sum',
-      array(
-        'accumulate',
-        array('fact', 'tasks.open-count.create.project', $project_phid),
-      ),
-      array(
-        'accumulate',
-        array('fact', 'tasks.open-count.status.project', $project_phid),
-      ),
-      array(
-        'accumulate',
-        array('fact', 'tasks.open-count.assign.project', $project_phid),
-      ),
-    );
-
-    $function = id(new PhabricatorComposeChartFunction())
-      ->setArguments(array($argv));
-
-    $datasets = array(
-      id(new PhabricatorChartDataset())
-        ->setFunction($function),
-    );
-
-    $chart = id(new PhabricatorFactChart())
-      ->setDatasets($datasets);
-
-    $engine = id(new PhabricatorChartEngine())
+    $chart_panel = id(new PhabricatorProjectBurndownChartEngine())
       ->setViewer($viewer)
-      ->setChart($chart);
+      ->setProjects(array($project))
+      ->buildChartPanel();
 
-    $chart = $engine->getStoredChart();
-
-    $panel_type = id(new PhabricatorDashboardChartPanelType())
-      ->getPanelTypeKey();
-
-    $chart_panel = id(new PhabricatorDashboardPanel())
-      ->setPanelType($panel_type)
-      ->setName(pht('%s: Burndown', $project->getName()))
-      ->setProperty('chartKey', $chart->getChartKey());
+    $chart_panel->setName(pht('%s: Burndown', $project->getName()));
 
     $chart_view = id(new PhabricatorDashboardPanelRenderingEngine())
       ->setViewer($viewer)