mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-29 08:50:58 +01:00
Allow dashboard panels to detect rendering cycles and arrest stack overflows
Summary: Ref T4986. Ref T4983. Panels will soon be able to contain other panels, either via Remarkup (`{W1}`) or maybe through new types of meta-panels. Allow panels to detect that they are being rendered very deeply and/or within themselves. Test Plan: Faked some errors, got failed panel renders. Since panels can't //really// contain other panels yet, this doesn't really have an impact. Reviewers: btrahan Reviewed By: btrahan Subscribers: chad, epriestley Maniphest Tasks: T4983, T4986 Differential Revision: https://secure.phabricator.com/D9140
This commit is contained in:
parent
e5e95352c7
commit
63acd90cef
6 changed files with 96 additions and 32 deletions
|
@ -52,7 +52,7 @@ return array(
|
||||||
'rsrc/css/application/conpherence/widget-pane.css' => 'bf275a6c',
|
'rsrc/css/application/conpherence/widget-pane.css' => 'bf275a6c',
|
||||||
'rsrc/css/application/contentsource/content-source-view.css' => '4b8b05d4',
|
'rsrc/css/application/contentsource/content-source-view.css' => '4b8b05d4',
|
||||||
'rsrc/css/application/countdown/timer.css' => '86b7b0a0',
|
'rsrc/css/application/countdown/timer.css' => '86b7b0a0',
|
||||||
'rsrc/css/application/dashboard/dashboard.css' => '5b532b7b',
|
'rsrc/css/application/dashboard/dashboard.css' => '2b41640b',
|
||||||
'rsrc/css/application/diff/inline-comment-summary.css' => '8cfd34e8',
|
'rsrc/css/application/diff/inline-comment-summary.css' => '8cfd34e8',
|
||||||
'rsrc/css/application/differential/add-comment.css' => 'c478bcaa',
|
'rsrc/css/application/differential/add-comment.css' => 'c478bcaa',
|
||||||
'rsrc/css/application/differential/changeset-view.css' => '1570a1ff',
|
'rsrc/css/application/differential/changeset-view.css' => '1570a1ff',
|
||||||
|
@ -358,8 +358,8 @@ return array(
|
||||||
'rsrc/js/application/conpherence/behavior-pontificate.js' => '53f6f2dd',
|
'rsrc/js/application/conpherence/behavior-pontificate.js' => '53f6f2dd',
|
||||||
'rsrc/js/application/conpherence/behavior-widget-pane.js' => '40b1ff90',
|
'rsrc/js/application/conpherence/behavior-widget-pane.js' => '40b1ff90',
|
||||||
'rsrc/js/application/countdown/timer.js' => '889c96f3',
|
'rsrc/js/application/countdown/timer.js' => '889c96f3',
|
||||||
'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '4398eabb',
|
'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => 'fd965b41',
|
||||||
'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'aa3f313b',
|
'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'fa187a68',
|
||||||
'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2441746',
|
'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2441746',
|
||||||
'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => '533a187b',
|
'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => '533a187b',
|
||||||
'rsrc/js/application/differential/behavior-comment-jump.js' => '71755c79',
|
'rsrc/js/application/differential/behavior-comment-jump.js' => '71755c79',
|
||||||
|
@ -553,8 +553,8 @@ return array(
|
||||||
'javelin-behavior-conpherence-widget-pane' => '40b1ff90',
|
'javelin-behavior-conpherence-widget-pane' => '40b1ff90',
|
||||||
'javelin-behavior-countdown-timer' => '889c96f3',
|
'javelin-behavior-countdown-timer' => '889c96f3',
|
||||||
'javelin-behavior-dark-console' => 'e9fdb5e5',
|
'javelin-behavior-dark-console' => 'e9fdb5e5',
|
||||||
'javelin-behavior-dashboard-async-panel' => '4398eabb',
|
'javelin-behavior-dashboard-async-panel' => 'fd965b41',
|
||||||
'javelin-behavior-dashboard-move-panels' => 'aa3f313b',
|
'javelin-behavior-dashboard-move-panels' => 'fa187a68',
|
||||||
'javelin-behavior-device' => '03d6ed07',
|
'javelin-behavior-device' => '03d6ed07',
|
||||||
'javelin-behavior-differential-add-reviewers-and-ccs' => '533a187b',
|
'javelin-behavior-differential-add-reviewers-and-ccs' => '533a187b',
|
||||||
'javelin-behavior-differential-comment-jump' => '71755c79',
|
'javelin-behavior-differential-comment-jump' => '71755c79',
|
||||||
|
@ -701,7 +701,7 @@ return array(
|
||||||
'phabricator-core-css' => '40151074',
|
'phabricator-core-css' => '40151074',
|
||||||
'phabricator-countdown-css' => '86b7b0a0',
|
'phabricator-countdown-css' => '86b7b0a0',
|
||||||
'phabricator-crumbs-view-css' => '6a23399c',
|
'phabricator-crumbs-view-css' => '6a23399c',
|
||||||
'phabricator-dashboard-css' => '5b532b7b',
|
'phabricator-dashboard-css' => '2b41640b',
|
||||||
'phabricator-drag-and-drop-file-upload' => 'ae6abfba',
|
'phabricator-drag-and-drop-file-upload' => 'ae6abfba',
|
||||||
'phabricator-draggable-list' => '1681c4d4',
|
'phabricator-draggable-list' => '1681c4d4',
|
||||||
'phabricator-fatal-config-template-css' => '25d446d6',
|
'phabricator-fatal-config-template-css' => '25d446d6',
|
||||||
|
@ -1133,12 +1133,6 @@ return array(
|
||||||
8 => 'phuix-action-list-view',
|
8 => 'phuix-action-list-view',
|
||||||
9 => 'phuix-action-view',
|
9 => 'phuix-action-view',
|
||||||
),
|
),
|
||||||
'4398eabb' =>
|
|
||||||
array(
|
|
||||||
0 => 'javelin-behavior',
|
|
||||||
1 => 'javelin-dom',
|
|
||||||
2 => 'javelin-workflow',
|
|
||||||
),
|
|
||||||
'441f2137' =>
|
'441f2137' =>
|
||||||
array(
|
array(
|
||||||
0 => 'javelin-behavior',
|
0 => 'javelin-behavior',
|
||||||
|
@ -1270,11 +1264,6 @@ return array(
|
||||||
2 => 'javelin-util',
|
2 => 'javelin-util',
|
||||||
3 => 'phabricator-shaped-request',
|
3 => 'phabricator-shaped-request',
|
||||||
),
|
),
|
||||||
'7319e029' =>
|
|
||||||
array(
|
|
||||||
0 => 'javelin-behavior',
|
|
||||||
1 => 'javelin-dom',
|
|
||||||
),
|
|
||||||
'62e18640' =>
|
'62e18640' =>
|
||||||
array(
|
array(
|
||||||
0 => 'javelin-install',
|
0 => 'javelin-install',
|
||||||
|
@ -1329,6 +1318,11 @@ return array(
|
||||||
1 => 'javelin-stratcom',
|
1 => 'javelin-stratcom',
|
||||||
2 => 'javelin-dom',
|
2 => 'javelin-dom',
|
||||||
),
|
),
|
||||||
|
'7319e029' =>
|
||||||
|
array(
|
||||||
|
0 => 'javelin-behavior',
|
||||||
|
1 => 'javelin-dom',
|
||||||
|
),
|
||||||
'76f4ebed' =>
|
'76f4ebed' =>
|
||||||
array(
|
array(
|
||||||
0 => 'javelin-install',
|
0 => 'javelin-install',
|
||||||
|
@ -1598,15 +1592,6 @@ return array(
|
||||||
1 => 'javelin-stratcom',
|
1 => 'javelin-stratcom',
|
||||||
2 => 'javelin-dom',
|
2 => 'javelin-dom',
|
||||||
),
|
),
|
||||||
'aa3f313b' =>
|
|
||||||
array(
|
|
||||||
0 => 'javelin-behavior',
|
|
||||||
1 => 'javelin-dom',
|
|
||||||
2 => 'javelin-util',
|
|
||||||
3 => 'javelin-stratcom',
|
|
||||||
4 => 'javelin-workflow',
|
|
||||||
5 => 'phabricator-draggable-list',
|
|
||||||
),
|
|
||||||
'ad7a69ca' =>
|
'ad7a69ca' =>
|
||||||
array(
|
array(
|
||||||
0 => 'javelin-install',
|
0 => 'javelin-install',
|
||||||
|
@ -2034,11 +2019,26 @@ return array(
|
||||||
4 => 'javelin-stratcom',
|
4 => 'javelin-stratcom',
|
||||||
5 => 'phabricator-shaped-request',
|
5 => 'phabricator-shaped-request',
|
||||||
),
|
),
|
||||||
|
'fa187a68' =>
|
||||||
|
array(
|
||||||
|
0 => 'javelin-behavior',
|
||||||
|
1 => 'javelin-dom',
|
||||||
|
2 => 'javelin-util',
|
||||||
|
3 => 'javelin-stratcom',
|
||||||
|
4 => 'javelin-workflow',
|
||||||
|
5 => 'phabricator-draggable-list',
|
||||||
|
),
|
||||||
'fbbce3bf' =>
|
'fbbce3bf' =>
|
||||||
array(
|
array(
|
||||||
0 => 'phabricator-busy',
|
0 => 'phabricator-busy',
|
||||||
1 => 'javelin-behavior',
|
1 => 'javelin-behavior',
|
||||||
),
|
),
|
||||||
|
'fd965b41' =>
|
||||||
|
array(
|
||||||
|
0 => 'javelin-behavior',
|
||||||
|
1 => 'javelin-dom',
|
||||||
|
2 => 'javelin-workflow',
|
||||||
|
),
|
||||||
'fe2e0ba4' =>
|
'fe2e0ba4' =>
|
||||||
array(
|
array(
|
||||||
0 => 'javelin-behavior',
|
0 => 'javelin-behavior',
|
||||||
|
|
|
@ -21,9 +21,22 @@ final class PhabricatorDashboardPanelRenderController
|
||||||
return new Aphront404Response();
|
return new Aphront404Response();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($request->isAjax()) {
|
||||||
|
$parent_phids = $request->getStrList('parentPanelPHIDs', null);
|
||||||
|
if ($parent_phids === null) {
|
||||||
|
throw new Exception(
|
||||||
|
pht(
|
||||||
|
'Required parameter `parentPanelPHIDs` is not present in '.
|
||||||
|
'request.'));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
$parent_phids = array();
|
||||||
|
}
|
||||||
|
|
||||||
$rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine())
|
$rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine())
|
||||||
->setViewer($viewer)
|
->setViewer($viewer)
|
||||||
->setPanel($panel)
|
->setPanel($panel)
|
||||||
|
->setParentPanelPHIDs($parent_phids)
|
||||||
->renderPanel();
|
->renderPanel();
|
||||||
|
|
||||||
if ($request->isAjax()) {
|
if ($request->isAjax()) {
|
||||||
|
|
|
@ -41,6 +41,7 @@ final class PhabricatorDashboardPanelViewController
|
||||||
$rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine())
|
$rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine())
|
||||||
->setViewer($viewer)
|
->setViewer($viewer)
|
||||||
->setPanel($panel)
|
->setPanel($panel)
|
||||||
|
->setParentPanelPHIDs(array())
|
||||||
->renderPanel();
|
->renderPanel();
|
||||||
|
|
||||||
return $this->buildApplicationPage(
|
return $this->buildApplicationPage(
|
||||||
|
|
|
@ -5,6 +5,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
|
||||||
private $panel;
|
private $panel;
|
||||||
private $viewer;
|
private $viewer;
|
||||||
private $enableAsyncRendering;
|
private $enableAsyncRendering;
|
||||||
|
private $parentPanelPHIDs;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Allow the engine to render the panel via Ajax.
|
* Allow the engine to render the panel via Ajax.
|
||||||
|
@ -14,6 +15,11 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
|
||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setParentPanelPHIDs(array $parents) {
|
||||||
|
$this->parentPanelPHIDs = $parents;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
public function setViewer(PhabricatorUser $viewer) {
|
public function setViewer(PhabricatorUser $viewer) {
|
||||||
$this->viewer = $viewer;
|
$this->viewer = $viewer;
|
||||||
return $this;
|
return $this;
|
||||||
|
@ -44,13 +50,15 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
|
||||||
$panel->getPanelType()));
|
$panel->getPanelType()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
$this->detectRenderingCycle($panel);
|
||||||
|
|
||||||
if ($this->enableAsyncRendering) {
|
if ($this->enableAsyncRendering) {
|
||||||
if ($panel_type->shouldRenderAsync()) {
|
if ($panel_type->shouldRenderAsync()) {
|
||||||
return $this->renderAsyncPanel($panel);
|
return $this->renderAsyncPanel($panel);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
|
||||||
return $panel_type->renderPanel($viewer, $panel);
|
return $panel_type->renderPanel($viewer, $panel);
|
||||||
} catch (Exception $ex) {
|
} catch (Exception $ex) {
|
||||||
return $this->renderErrorPanel(
|
return $this->renderErrorPanel(
|
||||||
|
@ -75,6 +83,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
|
||||||
'dashboard-async-panel',
|
'dashboard-async-panel',
|
||||||
array(
|
array(
|
||||||
'panelID' => $panel_id,
|
'panelID' => $panel_id,
|
||||||
|
'parentPanelPHIDs' => $this->parentPanelPHIDs,
|
||||||
'uri' => '/dashboard/panel/render/'.$panel->getID().'/',
|
'uri' => '/dashboard/panel/render/'.$panel->getID().'/',
|
||||||
));
|
));
|
||||||
|
|
||||||
|
@ -87,4 +96,43 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
|
||||||
->appendChild(pht('Loading...'));
|
->appendChild(pht('Loading...'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Detect graph cycles in panels, and deeply nested panels.
|
||||||
|
*
|
||||||
|
* This method throws if the current rendering stack is too deep or contains
|
||||||
|
* a cycle. This can happen if you embed layout panels inside each other,
|
||||||
|
* build a big stack of panels, or embed a panel in remarkup inside another
|
||||||
|
* panel. Generally, all of this stuff is ridiculous and we just want to
|
||||||
|
* shut it down.
|
||||||
|
*
|
||||||
|
* @param PhabricatorDashboardPanel Panel being rendered.
|
||||||
|
* @return void
|
||||||
|
*/
|
||||||
|
private function detectRenderingCycle(PhabricatorDashboardPanel $panel) {
|
||||||
|
if ($this->parentPanelPHIDs === null) {
|
||||||
|
throw new Exception(
|
||||||
|
pht(
|
||||||
|
'You must call setParentPanelPHIDs() before rendering panels.'));
|
||||||
|
}
|
||||||
|
|
||||||
|
$max_depth = 4;
|
||||||
|
if (count($this->parentPanelPHIDs) >= $max_depth) {
|
||||||
|
throw new Exception(
|
||||||
|
pht(
|
||||||
|
'To render more than %s levels of panels nested inside other '.
|
||||||
|
'panels, purchase a subscription to Phabricator Gold.',
|
||||||
|
new PhutilNumber($max_depth)));
|
||||||
|
}
|
||||||
|
|
||||||
|
if (in_array($panel->getPHID(), $this->parentPanelPHIDs)) {
|
||||||
|
throw new Exception(
|
||||||
|
pht(
|
||||||
|
'You awake in a twisting maze of mirrors, all alike. '.
|
||||||
|
'You are likely to be eaten by a graph cycle. '.
|
||||||
|
'Should you escape alive, you resolve to be more careful about '.
|
||||||
|
'putting dashboard panels inside themselves.'));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -43,6 +43,7 @@ final class PhabricatorDashboardRenderingEngine extends Phobject {
|
||||||
->setViewer($viewer)
|
->setViewer($viewer)
|
||||||
->setPanel($panel)
|
->setPanel($panel)
|
||||||
->setEnableAsyncRendering(true)
|
->setEnableAsyncRendering(true)
|
||||||
|
->setParentPanelPHIDs(array())
|
||||||
->renderPanel();
|
->renderPanel();
|
||||||
}
|
}
|
||||||
$column_class = $layout_config->getColumnClass(
|
$column_class = $layout_config->getColumnClass(
|
||||||
|
|
|
@ -10,6 +10,7 @@ JX.behavior('dashboard-async-panel', function(config) {
|
||||||
panel.style.opacity = '0.5';
|
panel.style.opacity = '0.5';
|
||||||
|
|
||||||
new JX.Workflow(config.uri)
|
new JX.Workflow(config.uri)
|
||||||
|
.setData({parentPanelPHIDs: config.parentPanelPHIDs.join(',')})
|
||||||
.setHandler(function(r) {
|
.setHandler(function(r) {
|
||||||
JX.DOM.replace(panel, JX.$H(r.panelMarkup));
|
JX.DOM.replace(panel, JX.$H(r.panelMarkup));
|
||||||
})
|
})
|
||||||
|
|
Loading…
Reference in a new issue