From 080e132aa740a4f36582ff7adeb1d1550d6a3bbf Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 16 Sep 2019 11:19:24 -0700 Subject: [PATCH] Track chart datapoints from their sources and provide a tabular view of chart data Summary: Depends on D20815. Ref T13279. Give datapoints "refs", which allow us to figure out where particular datapoints came from even after the point is transformed by functions. For now, show the raw points in a table below the chart. Test Plan: Viewed chart data, saw reasonable-looking numbers. Subscribers: yelirekim Maniphest Tasks: T13279 Differential Revision: https://secure.phabricator.com/D20816 --- src/__phutil_library_map__.php | 2 +- .../PhabricatorAccumulateChartFunction.php | 10 +- .../fact/chart/PhabricatorChartDataset.php | 31 ++++ .../fact/chart/PhabricatorChartFunction.php | 2 + .../chart/PhabricatorComposeChartFunction.php | 18 +++ .../chart/PhabricatorFactChartFunction.php | 68 ++++++++- .../PhabricatorHigherOrderChartFunction.php | 34 +++++ .../chart/PhabricatorPureChartFunction.php | 12 +- .../PhabricatorFactChartController.php | 12 +- .../PhabricatorChartRenderingEngine.php | 142 +++++++++++++++++- 10 files changed, 315 insertions(+), 16 deletions(-) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e8c0392427..470a6c69fe 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -8301,7 +8301,7 @@ phutil_register_library_map(array( 'PhabricatorAccessLog' => 'Phobject', 'PhabricatorAccessLogConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorAccessibilitySetting' => 'PhabricatorSelectSetting', - 'PhabricatorAccumulateChartFunction' => 'PhabricatorChartFunction', + 'PhabricatorAccumulateChartFunction' => 'PhabricatorHigherOrderChartFunction', 'PhabricatorActionListView' => 'AphrontTagView', 'PhabricatorActionView' => 'AphrontView', 'PhabricatorActivitySettingsPanel' => 'PhabricatorSettingsPanel', diff --git a/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php b/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php index 6ffbb85da9..63570ef234 100644 --- a/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php +++ b/src/applications/fact/chart/PhabricatorAccumulateChartFunction.php @@ -1,7 +1,7 @@ getArgument('x')->getDomain(); - } - - public function newInputValues(PhabricatorChartDataQuery $query) { - return $this->getArgument('x')->newInputValues($query); - } - public function evaluateFunction(array $xv) { // First, we're going to accumulate the underlying function. Then // we'll map the inputs through the accumulation. diff --git a/src/applications/fact/chart/PhabricatorChartDataset.php b/src/applications/fact/chart/PhabricatorChartDataset.php index 9faf02b740..a794834229 100644 --- a/src/applications/fact/chart/PhabricatorChartDataset.php +++ b/src/applications/fact/chart/PhabricatorChartDataset.php @@ -75,4 +75,35 @@ abstract class PhabricatorChartDataset PhabricatorChartDataQuery $data_query); + final public function getTabularDisplayData( + PhabricatorChartDataQuery $data_query) { + $results = array(); + + $functions = $this->getFunctions(); + foreach ($functions as $function) { + $datapoints = $function->newDatapoints($data_query); + + $refs = $function->getDataRefs(ipull($datapoints, 'x')); + + foreach ($datapoints as $key => $point) { + $x = $point['x']; + + if (isset($refs[$x])) { + $xrefs = $refs[$x]; + } else { + $xrefs = array(); + } + + $datapoints[$key]['refs'] = $xrefs; + } + + $results[] = array( + 'data' => $datapoints, + ); + } + + return id(new PhabricatorChartDisplayData()) + ->setWireData($results); + } + } diff --git a/src/applications/fact/chart/PhabricatorChartFunction.php b/src/applications/fact/chart/PhabricatorChartFunction.php index ac7ab64650..ef5c5641bf 100644 --- a/src/applications/fact/chart/PhabricatorChartFunction.php +++ b/src/applications/fact/chart/PhabricatorChartFunction.php @@ -180,6 +180,8 @@ abstract class PhabricatorChartFunction } abstract public function evaluateFunction(array $xv); + abstract public function getDataRefs(array $xv); + abstract public function loadRefs(array $refs); public function getDomain() { return null; diff --git a/src/applications/fact/chart/PhabricatorComposeChartFunction.php b/src/applications/fact/chart/PhabricatorComposeChartFunction.php index f6148ceae9..e69455b3ff 100644 --- a/src/applications/fact/chart/PhabricatorComposeChartFunction.php +++ b/src/applications/fact/chart/PhabricatorComposeChartFunction.php @@ -70,4 +70,22 @@ final class PhabricatorComposeChartFunction return $yv; } + public function getDataRefs(array $xv) { + // TODO: This is not entirely correct. The correct implementation would + // map "x -> y" at each stage of composition and pull and aggregate all + // the datapoint refs. In practice, we currently never compose functions + // with a data function somewhere in the middle, so just grabbing the first + // result is close enough. + + // In the future, we may: notably, "x -> shift(-1 month) -> ..." to + // generate a month-over-month overlay is a sensible operation which will + // source data from the middle of a function composition. + + foreach ($this->getFunctionArguments() as $function) { + return $function->getDataRefs($xv); + } + + return array(); + } + } diff --git a/src/applications/fact/chart/PhabricatorFactChartFunction.php b/src/applications/fact/chart/PhabricatorFactChartFunction.php index dbb886dd3e..0e940d644d 100644 --- a/src/applications/fact/chart/PhabricatorFactChartFunction.php +++ b/src/applications/fact/chart/PhabricatorFactChartFunction.php @@ -7,6 +7,7 @@ final class PhabricatorFactChartFunction private $fact; private $map; + private $refs; protected function newArguments() { $key_argument = $this->newArgument() @@ -51,13 +52,15 @@ final class PhabricatorFactChartFunction $data = queryfx_all( $conn, - 'SELECT value, epoch FROM %T WHERE %LA ORDER BY epoch ASC', + 'SELECT id, value, epoch FROM %T WHERE %LA ORDER BY epoch ASC', $table_name, $where); $map = array(); + $refs = array(); if ($data) { foreach ($data as $row) { + $ref = (string)$row['id']; $value = (int)$row['value']; $epoch = (int)$row['epoch']; @@ -66,10 +69,17 @@ final class PhabricatorFactChartFunction } $map[$epoch] += $value; + + if (!isset($refs[$epoch])) { + $refs[$epoch] = array(); + } + + $refs[$epoch][] = $ref; } } $this->map = $map; + $this->refs = $refs; } public function getDomain() { @@ -99,4 +109,60 @@ final class PhabricatorFactChartFunction return $yv; } + public function getDataRefs(array $xv) { + return array_select_keys($this->refs, $xv); + } + + public function loadRefs(array $refs) { + $fact = $this->fact; + + $datapoint_table = $fact->newDatapoint(); + $conn = $datapoint_table->establishConnection('r'); + + $dimension_table = new PhabricatorFactObjectDimension(); + + $where = array(); + + $where[] = qsprintf( + $conn, + 'p.id IN (%Ld)', + $refs); + + + $rows = queryfx_all( + $conn, + 'SELECT + p.id id, + p.value, + od.objectPHID objectPHID, + dd.objectPHID dimensionPHID + FROM %R p + LEFT JOIN %R od ON od.id = p.objectID + LEFT JOIN %R dd ON dd.id = p.dimensionID + WHERE %LA', + $datapoint_table, + $dimension_table, + $dimension_table, + $where); + $rows = ipull($rows, null, 'id'); + + $results = array(); + + foreach ($refs as $ref) { + if (!isset($rows[$ref])) { + continue; + } + + $row = $rows[$ref]; + + $results[$ref] = array( + 'objectPHID' => $row['objectPHID'], + 'dimensionPHID' => $row['dimensionPHID'], + 'value' => (float)$row['value'], + ); + } + + return $results; + } + } diff --git a/src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php b/src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php index ab160bd10f..7124603388 100644 --- a/src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php +++ b/src/applications/fact/chart/PhabricatorHigherOrderChartFunction.php @@ -32,4 +32,38 @@ abstract class PhabricatorHigherOrderChartFunction return array_keys($map); } + public function getDataRefs(array $xv) { + $refs = array(); + + foreach ($this->getFunctionArguments() as $function) { + $function_refs = $function->getDataRefs($xv); + + $function_refs = array_select_keys($function_refs, $xv); + if (!$function_refs) { + continue; + } + + foreach ($function_refs as $x => $ref_list) { + if (!isset($refs[$x])) { + $refs[$x] = array(); + } + foreach ($ref_list as $ref) { + $refs[$x][] = $ref; + } + } + } + + return $refs; + } + + public function loadRefs(array $refs) { + $results = array(); + + foreach ($this->getFunctionArguments() as $function) { + $results += $function->loadRefs($refs); + } + + return $results; + } + } diff --git a/src/applications/fact/chart/PhabricatorPureChartFunction.php b/src/applications/fact/chart/PhabricatorPureChartFunction.php index aa1213f8c4..74c748c274 100644 --- a/src/applications/fact/chart/PhabricatorPureChartFunction.php +++ b/src/applications/fact/chart/PhabricatorPureChartFunction.php @@ -1,4 +1,14 @@ newChartView(); - return $this->newChartResponse($chart_view); + $tabular_view = $engine->newTabularView(); + + return $this->newChartResponse($chart_view, $tabular_view); } - private function newChartResponse($chart_view) { + private function newChartResponse($chart_view, $tabular_view) { $box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Chart')) ->appendChild($chart_view); @@ -50,7 +52,11 @@ final class PhabricatorFactChartController extends PhabricatorFactController { return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) - ->appendChild($box); + ->appendChild( + array( + $box, + $tabular_view, + )); } private function newDemoChart() { diff --git a/src/applications/fact/engine/PhabricatorChartRenderingEngine.php b/src/applications/fact/engine/PhabricatorChartRenderingEngine.php index b328241ea6..c691fe1abf 100644 --- a/src/applications/fact/engine/PhabricatorChartRenderingEngine.php +++ b/src/applications/fact/engine/PhabricatorChartRenderingEngine.php @@ -109,7 +109,143 @@ final class PhabricatorChartRenderingEngine return $chart_view; } + public function newTabularView() { + $viewer = $this->getViewer(); + $tabular_data = $this->newTabularData(); + + $ref_keys = array(); + foreach ($tabular_data['datasets'] as $tabular_dataset) { + foreach ($tabular_dataset as $function) { + foreach ($function['data'] as $point) { + foreach ($point['refs'] as $ref) { + $ref_keys[$ref] = $ref; + } + } + } + } + + $chart = $this->getStoredChart(); + + $ref_map = array(); + foreach ($chart->getDatasets() as $dataset) { + foreach ($dataset->getFunctions() as $function) { + // If we aren't looking for anything else, bail out. + if (!$ref_keys) { + break 2; + } + + $function_refs = $function->loadRefs($ref_keys); + + $ref_map += $function_refs; + + // Remove the ref keys that we found data for from the list of keys + // we are looking for. If any function gives us data for a given ref, + // that's satisfactory. + foreach ($function_refs as $ref_key => $ref_data) { + unset($ref_keys[$ref_key]); + } + } + } + + $phids = array(); + foreach ($ref_map as $ref => $ref_data) { + if (isset($ref_data['objectPHID'])) { + $phids[] = $ref_data['objectPHID']; + } + } + + $handles = $viewer->loadHandles($phids); + + $tabular_view = array(); + foreach ($tabular_data['datasets'] as $tabular_data) { + foreach ($tabular_data as $function) { + $rows = array(); + foreach ($function['data'] as $point) { + $ref_views = array(); + + $xv = date('Y-m-d h:i:s', $point['x']); + $yv = $point['y']; + + $point_refs = array(); + foreach ($point['refs'] as $ref) { + if (!isset($ref_map[$ref])) { + continue; + } + $point_refs[$ref] = $ref_map[$ref]; + } + + if (!$point_refs) { + $rows[] = array( + $xv, + $yv, + ); + } else { + foreach ($point_refs as $ref => $ref_data) { + $ref_value = $ref_data['value']; + $ref_link = $handles[$ref_data['objectPHID']] + ->renderLink(); + + $view_uri = urisprintf( + '/fact/object/%s/', + $ref_data['objectPHID']); + + $ref_button = id(new PHUIButtonView()) + ->setIcon('fa-table') + ->setTag('a') + ->setColor('grey') + ->setHref($view_uri) + ->setText(pht('View Data')); + + $rows[] = array( + $xv, + $yv, + $ref_value, + $ref_link, + $ref_button, + ); + + $xv = null; + $yv = null; + } + } + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('X'), + pht('Y'), + pht('Raw'), + pht('Refs'), + null, + )) + ->setColumnClasses( + array( + 'n', + 'n', + 'n', + 'wide', + null, + )); + + $tabular_view[] = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Function')) + ->setTable($table); + } + } + + return $tabular_view; + } + public function newChartData() { + return $this->newWireData(false); + } + + public function newTabularData() { + return $this->newWireData(true); + } + + private function newWireData($is_tabular) { $chart = $this->getStoredChart(); $chart_key = $chart->getChartKey(); @@ -151,7 +287,11 @@ final class PhabricatorChartRenderingEngine $wire_datasets = array(); $ranges = array(); foreach ($datasets as $dataset) { - $display_data = $dataset->getChartDisplayData($data_query); + if ($is_tabular) { + $display_data = $dataset->getTabularDisplayData($data_query); + } else { + $display_data = $dataset->getChartDisplayData($data_query); + } $ranges[] = $display_data->getRange(); $wire_datasets[] = $display_data->getWireData();