From c458b50b856963a0f367d37e79bdeb4adf652478 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 29 Apr 2019 11:18:42 -0700 Subject: [PATCH] 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 | 4 + .../PhabricatorFactApplication.php | 4 +- .../fact/chart/PhabricatorChartDataset.php | 37 +++ .../fact/chart/PhabricatorChartFunction.php | 4 + ...PhabricatorChartFunctionArgumentParser.php | 4 + .../PhabricatorFactChartController.php | 214 +++++------------- .../fact/engine/PhabricatorChartEngine.php | 214 ++++++++++++++++++ .../fact/storage/PhabricatorFactChart.php | 48 +++- 8 files changed, 370 insertions(+), 159 deletions(-) create mode 100644 src/applications/fact/chart/PhabricatorChartDataset.php create mode 100644 src/applications/fact/engine/PhabricatorChartEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6777c69bd0..3ecee44659 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/fact/application/PhabricatorFactApplication.php b/src/applications/fact/application/PhabricatorFactApplication.php index 2b493b7bea..b3e0417754 100644 --- a/src/applications/fact/application/PhabricatorFactApplication.php +++ b/src/applications/fact/application/PhabricatorFactApplication.php @@ -30,7 +30,9 @@ final class PhabricatorFactApplication extends PhabricatorApplication { return array( '/fact/' => array( '' => 'PhabricatorFactHomeController', - '(?chart|draw)/' => 'PhabricatorFactChartController', + 'chart/' => 'PhabricatorFactChartController', + 'chart/(?P[^/]+)/(?:(?Pdraw)/)?' => + 'PhabricatorFactChartController', 'object/(?[^/]+)/' => 'PhabricatorFactObjectController', ), ); diff --git a/src/applications/fact/chart/PhabricatorChartDataset.php b/src/applications/fact/chart/PhabricatorChartDataset.php new file mode 100644 index 0000000000..3251808188 --- /dev/null +++ b/src/applications/fact/chart/PhabricatorChartDataset.php @@ -0,0 +1,37 @@ +function; + } + + public static function newFromDictionary(array $map) { + PhutilTypeSpec::checkMap( + $map, + array( + 'function' => 'list', + )); + + $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, + ); + } + +} diff --git a/src/applications/fact/chart/PhabricatorChartFunction.php b/src/applications/fact/chart/PhabricatorChartFunction.php index 414147da56..b4a66645ad 100644 --- a/src/applications/fact/chart/PhabricatorChartFunction.php +++ b/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; diff --git a/src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php b/src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php index 04342ed4cc..01fec105a3 100644 --- a/src/applications/fact/chart/PhabricatorChartFunctionArgumentParser.php +++ b/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); diff --git a/src/applications/fact/controller/PhabricatorFactChartController.php b/src/applications/fact/controller/PhabricatorFactChartController.php index 24d54a1cf6..cfda18e759 100644 --- a/src/applications/fact/controller/PhabricatorFactChartController.php +++ b/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); - } - - } diff --git a/src/applications/fact/engine/PhabricatorChartEngine.php b/src/applications/fact/engine/PhabricatorChartEngine.php new file mode 100644 index 0000000000..ef962e2173 --- /dev/null +++ b/src/applications/fact/engine/PhabricatorChartEngine.php @@ -0,0 +1,214 @@ +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); + } + +} diff --git a/src/applications/fact/storage/PhabricatorFactChart.php b/src/applications/fact/storage/PhabricatorFactChart.php index 7829010037..515b5f0a72 100644 --- a/src/applications/fact/storage/PhabricatorFactChart.php +++ b/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() {