Render charts from storage instead of just one ad-hoc hard-coded chart

Summary:
Ref T13279. This changes the chart controller:

  - if we have no arguments, build a demo chart and redirect to it;
  - otherwise, load the specified chart from storage and render it.

This mostly prepares for "Chart" panels on dashboards.

Test Plan: Visited `/fact/chart/`, got redirected to a chart from storage.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: yelirekim

Maniphest Tasks: T13279

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

src/__phutil_library_map__.php

@@ -2663,6 +2663,8 @@ phutil_register_library_map(array(
     'PhabricatorChangesetResponse' => 'infrastructure/diff/PhabricatorChangesetResponse.php',
     'PhabricatorChartAxis' => 'applications/fact/chart/PhabricatorChartAxis.php',
     'PhabricatorChartDataQuery' => 'applications/fact/chart/PhabricatorChartDataQuery.php',
+    'PhabricatorChartDataset' => 'applications/fact/chart/PhabricatorChartDataset.php',
+    'PhabricatorChartEngine' => 'applications/fact/engine/PhabricatorChartEngine.php',
     'PhabricatorChartFunction' => 'applications/fact/chart/PhabricatorChartFunction.php',
     'PhabricatorChartFunctionArgument' => 'applications/fact/chart/PhabricatorChartFunctionArgument.php',
     'PhabricatorChartFunctionArgumentParser' => 'applications/fact/chart/PhabricatorChartFunctionArgumentParser.php',
@@ -8669,6 +8671,8 @@ phutil_register_library_map(array(
     'PhabricatorChangesetResponse' => 'AphrontProxyResponse',
     'PhabricatorChartAxis' => 'Phobject',
     'PhabricatorChartDataQuery' => 'Phobject',
+    'PhabricatorChartDataset' => 'Phobject',
+    'PhabricatorChartEngine' => 'Phobject',
     'PhabricatorChartFunction' => 'Phobject',
     'PhabricatorChartFunctionArgument' => 'Phobject',
     'PhabricatorChartFunctionArgumentParser' => 'Phobject',

src/applications/fact/application/PhabricatorFactApplication.php

@@ -30,7 +30,9 @@ final class PhabricatorFactApplication extends PhabricatorApplication {
     return array(
       '/fact/' => array(
         '' => 'PhabricatorFactHomeController',
-        '(?<mode>chart|draw)/' => 'PhabricatorFactChartController',
+        'chart/' => 'PhabricatorFactChartController',
+        'chart/(?P<chartKey>[^/]+)/(?:(?P<mode>draw)/)?' =>
+          'PhabricatorFactChartController',
         'object/(?<phid>[^/]+)/' => 'PhabricatorFactObjectController',
       ),
     );

src/applications/fact/chart/PhabricatorChartDataset.php

@@ -0,0 +1,37 @@
+<?php
+
+final class PhabricatorChartDataset
+  extends Phobject {
+
+  private $function;
+
+  public function getFunction() {
+    return $this->function;
+  }
+
+  public static function newFromDictionary(array $map) {
+    PhutilTypeSpec::checkMap(
+      $map,
+      array(
+        'function' => 'list<wild>',
+      ));
+
+    $dataset = new self();
+
+    $dataset->function = id(new PhabricatorComposeChartFunction())
+      ->setArguments(array($map['function']));
+
+    return $dataset;
+  }
+
+  public function toDictionary() {
+    // Since we wrap the raw value in a "compose(...)", when deserializing,
+    // we need to unwrap it when serializing.
+    $function_raw = head($this->getFunction()->toDictionary());
+
+    return array(
+      'function' => $function_raw,
+    );
+  }
+
+}

src/applications/fact/chart/PhabricatorChartFunction.php

@@ -43,6 +43,10 @@ abstract class PhabricatorChartFunction
     return $this;
   }
 
+  public function toDictionary() {
+    return $this->getArgumentParser()->getRawArguments();
+  }
+
   public function getSubfunctions() {
     $result = array();
     $result[] = $this;

src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php

@@ -103,6 +103,10 @@ final class PhabricatorChartFunctionArgumentParser
     return array_values($this->argumentMap);
   }
 
+  public function getRawArguments() {
+    return $this->rawArguments;
+  }
+
   public function parseArguments() {
     $have_count = count($this->rawArguments);
     $want_count = count($this->argumentMap);

src/applications/fact/controller/PhabricatorFactChartController.php

@@ -5,13 +5,60 @@ final class PhabricatorFactChartController extends PhabricatorFactController {
   public function handleRequest(AphrontRequest $request) {
     $viewer = $request->getViewer();
 
+    $chart_key = $request->getURIData('chartKey');
+    if ($chart_key === null) {
+      return $this->newDemoChart();
+    }
+
+    $chart = id(new PhabricatorFactChart())->loadOneWhere(
+      'chartKey = %s',
+      $chart_key);
+    if (!$chart) {
+      return new Aphront404Response();
+    }
+
+    $engine = id(new PhabricatorChartEngine())
+      ->setViewer($viewer)
+      ->setChart($chart);
+
     // When drawing a chart, we send down a placeholder piece of HTML first,
     // then fetch the data via async request. Determine if we're drawing
     // the structure or actually pulling the data.
     $mode = $request->getURIData('mode');
-    $is_chart_mode = ($mode === 'chart');
     $is_draw_mode = ($mode === 'draw');
 
+    // TODO: For now, always pull the data. We'll throw it away if we're just
+    // drawing the frame, but this makes errors easier to debug.
+    $chart_data = $engine->newChartData();
+
+    if ($is_draw_mode) {
+      return id(new AphrontAjaxResponse())->setContent($chart_data);
+    }
+
+    $chart_view = $engine->newChartView();
+    return $this->newChartResponse($chart_view);
+  }
+
+  private function newChartResponse($chart_view) {
+    $box = id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Chart'))
+      ->appendChild($chart_view);
+
+    $crumbs = $this->buildApplicationCrumbs()
+      ->addTextCrumb(pht('Chart'))
+      ->setBorder(true);
+
+    $title = pht('Chart');
+
+    return $this->newPage()
+      ->setTitle($title)
+      ->setCrumbs($crumbs)
+      ->appendChild($box);
+  }
+
+  private function newDemoChart() {
+    $viewer = $this->getViewer();
+
     $argvs = array();
 
     $argvs[] = array('fact', 'tasks.count.create');
@@ -40,165 +87,24 @@ final class PhabricatorFactChartController extends PhabricatorFactController {
       array('shift', 800),
     );
 
-    $functions = array();
-    foreach ($argvs as $argv) {
-      $functions[] = id(new PhabricatorComposeChartFunction())
-        ->setArguments(array($argv));
-    }
-
-    $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',
-      );
+    foreach ($argvs as $argv) {
+      $datasets[] = PhabricatorChartDataset::newFromDictionary(
+        array(
+          'function' => $argv,
+        ));
     }
 
+    $chart = id(new PhabricatorFactChart())
+      ->setDatasets($datasets);
 
-    $y_min = 0;
-    $y_max = 0;
-    foreach ($datasets as $dataset) {
-      if (!$dataset['y']) {
-        continue;
-      }
+    $engine = id(new PhabricatorChartEngine())
+      ->setViewer($viewer)
+      ->setChart($chart);
 
-      $y_min = min($y_min, min($dataset['y']));
-      $y_max = max($y_max, max($dataset['y']));
-    }
+    $chart = $engine->getStoredChart();
 
-    $chart_data = array(
-      'datasets' => $datasets,
-      'xMin' => $domain_min,
-      'xMax' => $domain_max,
-      'yMin' => $y_min,
-      'yMax' => $y_max,
-    );
-
-    // TODO: Move this back up, it's just down here for now to make
-    // debugging easier so the main page throws a more visible exception when
-    // something goes wrong.
-    if ($is_chart_mode) {
-      return $this->newChartResponse();
-    }
-
-    return id(new AphrontAjaxResponse())->setContent($chart_data);
+    return id(new AphrontRedirectResponse())->setURI($chart->getURI());
   }
 
-  private function newChartResponse() {
-    $request = $this->getRequest();
-    $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 = $request->getRequestURI();
-    $data_uri->setPath('/fact/draw/');
-
-    Javelin::initBehavior(
-      'line-chart',
-      array(
-        'chartNodeID' => $chart_node_id,
-        'dataURI' => (string)$data_uri,
-      ));
-
-    $box = id(new PHUIObjectBoxView())
-      ->setHeaderText(pht('Chart'))
-      ->appendChild($chart_view);
-
-    $crumbs = $this->buildApplicationCrumbs()
-      ->addTextCrumb(pht('Chart'))
-      ->setBorder(true);
-
-    $title = pht('Chart');
-
-    return $this->newPage()
-      ->setTitle($title)
-      ->setCrumbs($crumbs)
-      ->appendChild($box);
-
-  }
-
-  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/fact/engine/PhabricatorChartEngine.php

@@ -0,0 +1,214 @@
+<?php
+
+final class PhabricatorChartEngine
+  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 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/fact/storage/PhabricatorFactChart.php

@@ -7,6 +7,8 @@ final class PhabricatorFactChart
   protected $chartKey;
   protected $chartParameters = array();
 
+  private $datasets;
+
   protected function getConfiguration() {
     return array(
       self::CONFIG_SERIALIZATION => array(
@@ -33,6 +35,12 @@ final class PhabricatorFactChart
     return idx($this->chartParameters, $key, $default);
   }
 
+  public function newChartKey() {
+    $digest = serialize($this->chartParameters);
+    $digest = PhabricatorHash::digestForIndex($digest);
+    return $digest;
+  }
+
   public function save() {
     if ($this->getID()) {
       throw new Exception(
@@ -41,14 +49,46 @@ final class PhabricatorFactChart
           'overwrite an existing chart configuration.'));
     }
 
-    $digest = serialize($this->chartParameters);
-    $digest = PhabricatorHash::digestForIndex($digest);
-
-    $this->chartKey = $digest;
+    $this->chartKey = $this->newChartKey();
 
     return parent::save();
   }
 
+  public function setDatasets(array $datasets) {
+    assert_instances_of($datasets, 'PhabricatorChartDataset');
+
+    $dataset_list = array();
+    foreach ($datasets as $dataset) {
+      $dataset_list[] = $dataset->toDictionary();
+    }
+
+    $this->setChartParameter('datasets', $dataset_list);
+    $this->datasets = null;
+
+    return $this;
+  }
+
+  public function getDatasets() {
+    if ($this->datasets === null) {
+      $this->datasets = $this->newDatasets();
+    }
+    return $this->datasets;
+  }
+
+  private function newDatasets() {
+    $datasets = $this->getChartParameter('datasets', array());
+
+    foreach ($datasets as $key => $dataset) {
+      $datasets[$key] = PhabricatorChartDataset::newFromDictionary($dataset);
+    }
+
+    return $datasets;
+  }
+
+  public function getURI() {
+    return urisprintf('/fact/chart/%s/', $this->getChartKey());
+  }
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
   public function getCapabilities() {