tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

NEW Enforce max node counts to avoid excessive resource usage

Browse Source 
Rendering potentially 1000s of nodes can exceed the CPU and memory constraints
of a normal PHP process, as well as the rendering capabilities of browsers.
Set a hard maximum for the renderable nodes, deferring to a "show as list" action
in the main CMS tree. For TreeDropdownField, we don't have the list fallback option,
so ask the user to search for the node title instead.

Also makes both the "node_threshold_total" and "node_threshold_leaf" values configurable
This commit is contained in:
Ingo Schommer 2013-03-19 22:26:48 +01:00 committed by Sam Minnee
parent 88d77db9e0
commit 01f46d039f
9 changed files with 258 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
admin/code/LeftAndMain.php
View File

@ -779,15 +779,53 @@ class LeftAndMain extends Controller implements PermissionProvider {
$link = Controller::join_links($recordController->Link("show"), $child->ID);
return LeftAndMain_TreeNode::create($child, $link, $controller->isCurrentPage($child))->forTemplate();
};
$html = $obj->getChildrenAsUL(
"",
$titleFn,
singleton('CMSPagesController'),
true,
$childrenMethod,
$numChildrenMethod,
$nodeCountThreshold
);
// Limit the amount of nodes shown for performance reasons.
// Skip the check if we're filtering the tree, since its not clear how many children will
// match the filter criteria until they're queried (and matched up with previously marked nodes).
$nodeThresholdLeaf = Config::inst()->get('Hierarchy', 'node_threshold_leaf');
if($nodeThresholdLeaf && !$filterFunction) {
$nodeCountCallback = function($parent, $numChildren) use($controller, $className, $nodeThresholdLeaf) {
if($className == 'SiteTree' && $parent->ID && $numChildren > $nodeThresholdLeaf) {
return sprintf(
'<ul><li class="readonly"><span class="item">'
. '%s (<a href="%s" class="cms-panel-link" data-pjax-target="Content">%s</a>)'
. '</span></li></ul>',
_t('LeftAndMain.TooManyPages', 'Too many pages'),
Controller::join_links(
$controller->LinkWithSearch($controller->Link()), '
?view=list&ParentID=' . $parent->ID
),
_t(
'LeftAndMain.ShowAsList',
'show as list',
'Show large amount of pages in list instead of tree view'
)
);
}
};
} else {
$nodeCountCallback = null;
}
// If the amount of pages exceeds the node thresholds set, use the callback
if($obj->ParentID && $nodeCountCallback) {
$html = $nodeCountCallback($obj, $obj->$numChildrenMethod());
}
// Otherwise return the actual tree (which might still filter leaf thresholds on children)
if(!$html) {
$html = $obj->getChildrenAsUL(
"",
$titleFn,
singleton('CMSPagesController'),
true,
$childrenMethod,
$numChildrenMethod,
$nodeCountThreshold,
$nodeCountCallback
);
}
// Wrap the root if needs be.
if(!$rootID) {

3
admin/css/screen.css
View File

@ -784,6 +784,9 @@ form.import-form label.left { width: 250px; }
.tree-holder.jstree-apple li.Root > a .jstree-icon, .cms-tree.jstree-apple li.Root > a .jstree-icon { background-position: -56px -36px; }
.tree-holder.jstree-apple li.deletedonlive .text, .cms-tree.jstree-apple li.deletedonlive .text { text-decoration: line-through; }
.tree-holder.jstree-apple li.jstree-checked > a, .tree-holder.jstree-apple li.jstree-checked > a:link, .cms-tree.jstree-apple li.jstree-checked > a, .cms-tree.jstree-apple li.jstree-checked > a:link { background-color: #efe999; }
.tree-holder.jstree-apple li.readonly, .cms-tree.jstree-apple li.readonly { color: #aaaaaa; padding-left: 18px; }
.tree-holder.jstree-apple li.readonly a, .tree-holder.jstree-apple li.readonly a:link, .cms-tree.jstree-apple li.readonly a, .cms-tree.jstree-apple li.readonly a:link { margin: 0; padding: 0; }
.tree-holder.jstree-apple li.readonly .jstree-icon, .cms-tree.jstree-apple li.readonly .jstree-icon { display: none; }
.tree-holder.jstree-apple a, .tree-holder.jstree-apple a:link, .cms-tree.jstree-apple a, .cms-tree.jstree-apple a:link { color: #0073c1; padding: 3px 6px 3px 3px; border: none; display: inline-block; margin-right: 5px; }
.tree-holder.jstree-apple ins, .cms-tree.jstree-apple ins { background-color: transparent; background-image: url(../images/sitetree_ss_default_icons.png); }
.tree-holder.jstree-apple span.badge, .cms-tree.jstree-apple span.badge { clear: both; text-transform: uppercase; display: inline-block; padding: 0px 3px; font-size: 0.75em; line-height: 1em; margin-left: 3px; margin-right: 6px; margin-top: -1px; -webkit-border-radius: 2px 2px; -moz-border-radius: 2px / 2px; border-radius: 2px / 2px; }

14
admin/scss/_tree.scss
View File

@ -418,6 +418,19 @@
background-color: $color-cms-batchactions-menu-selected-background;
}
}
&.readonly {
color: $color-text-disabled;
padding-left: 18px;
// Don't show drag icons or required spacing
a, a:link {
margin: 0;
padding: 0;
}
.jstree-icon {
display: none;
}
}
}
a, a:link {
color: $color-text-blue-link;
@ -441,6 +454,7 @@
margin-right: 6px;
margin-top: -1px;
@include border-radius(2px, 2px);
&.modified, &.addedtodraft {
color: #7E7470;
border: 1px solid #C9B800;

5
docs/en/changelogs/3.1.0.md
View File

@ -443,4 +443,7 @@ you can enable those warnings and future-proof your code already.
`<% if "Some<String" == "Some>Other>String" %>...<% end_if %>`
This change was necessary in order to support inequality operators in comparisons in templates
This change was necessary in order to support inequality operators in comparisons in templates
* Hard limit displayed pages in the CMS tree to `500`, and the number of direct children to `250`,
to avoid excessive resource usage. Configure through `Hierarchy.node_threshold_total` and `
Hierarchy.node_threshold_leaf`. Set to `0` to show tree unrestricted.

15
docs/en/reference/sitetree.md
View File

@ -74,7 +74,7 @@ Hierarchy operations (defined on `[api:Hierarchy]`:
* `$page->AllHistoricalChildren()`: Return all the children this page had, including pages that were deleted from both stage & live.
* `$page->AllChildrenIncludingDeleted()`: Return all children, including those that have been deleted but are still in live.
## Limiting Hierarchy
## Allowed Children, Default Child and Root-Level
By default, any page type can be the child of any other page type.
However, there are static properties that can be
@ -115,9 +115,20 @@ level.
Note that there is no allowed_parents` control. To set this, you will need to specify the `allowed_children` of all other page types to exclude the page type in question.
## Permission Control
## Tree Limitations
SilverStripe limits the amount of initially rendered nodes in order to avoid
processing delays, usually to a couple of dozen. The value can be configured
through `[api:Hierarchy::$node_threshold_total]`.
If a website has thousands of pages, the tree UI metaphor can become an inefficient way
to manage them. The CMS has an alternative "list view" for this purpose, which allows
sorting and paging through large numbers of pages in a tabular view.
To avoid exceeding performance constraints of both the server and browser,
SilverStripe places hard limits on the amount of rendered pages in
a specific tree leaf, typically a couple of hundred pages.
The value can be configured through `[api:Hierarchy::$node_threshold_leaf]`.
## Tree Display (Description, Icons and Badges)

46
forms/TreeDropdownField.php
View File

@ -269,10 +269,50 @@ class TreeDropdownField extends FormField {
. $this->keyField . '\" class=\"class-$child->class"'
. ' . $child->markingClasses() . "\"><a rel=\"$child->ID\">" . $child->' . $this->labelField . ' . "</a>"';
if($isSubTree) {
return substr(trim($obj->getChildrenAsUL('', $eval, null, true, $this->childrenMethod)), 4, -5);
// Limit the amount of nodes shown for performance reasons.
// Skip the check if we're filtering the tree, since its not clear how many children will
// match the filter criteria until they're queried (and matched up with previously marked nodes).
$nodeThresholdLeaf = Config::inst()->get('Hierarchy', 'node_threshold_leaf');
if($nodeThresholdLeaf && !$this->filterCallback && !$this->search) {
$className = $this->sourceObject;
$nodeCountCallback = function($parent, $numChildren) use($className, $nodeThresholdLeaf) {
if($className == 'SiteTree' && $parent->ID && $numChildren > $nodeThresholdLeaf) {
return sprintf(
'<ul><li><span class="item">%s</span></li></ul>',
_t('LeftAndMain.TooManyPages', 'Too many pages')
);
}
};
} else {
return $obj->getChildrenAsUL('class="tree"', $eval, null, true, $this->childrenMethod);
$nodeCountCallback = null;
}
if($isSubTree) {
$html = $obj->getChildrenAsUL(
"",
$eval,
null,
true,
$this->childrenMethod,
'numChildren',
true, // root call
null,
$nodeCountCallback
);
return substr(trim($html), 4, -5);
} else {
$html = $obj->getChildrenAsUL(
'class="tree"',
$eval,
null,
true,
$this->childrenMethod,
'numChildren',
true, // root call
null,
$nodeCountCallback
);
return $html;
}
}

57
model/Hierarchy.php
View File

@ -15,6 +15,28 @@ class Hierarchy extends DataExtension {
* @var Int
*/
protected $_cache_numChildren;
/**
* @config
* @var integer The lower bounds for the amount of nodes to mark. If set, the logic will expand
* nodes until it reaches at least this number, and then stops. Root nodes will always
* show regardless of this settting. Further nodes can be lazy-loaded via ajax.
* This isn't a hard limit. Example: On a value of 10, with 20 root nodes, each having
* 30 children, the actual node count will be 50 (all root nodes plus first expanded child).
*/
private static $node_threshold_total = 50;
/**
* @config
* @var integer Limit on the maximum children a specific node can display.
* Serves as a hard limit to avoid exceeding available server resources
* in generating the tree, and browser resources in rendering it.
* Nodes with children exceeding this value typically won't display
* any children, although this is configurable through the $nodeCountCallback
* parameter in {@link getChildrenAsUL()}. "Root" nodes will always show
* all children, regardless of this setting.
*/
private static $node_threshold_leaf = 250;
public function augmentSQL(SQLQuery &$query) {
}
@ -74,21 +96,29 @@ class Hierarchy extends DataExtension {
* @param string $childrenMethod The name of the method used to get children from each object
* @param boolean $rootCall Set to true for this first call, and then to false for calls inside the recursion. You
* should not change this.
* @param int $nodeCountThreshold The lower bounds for the amount of nodes to mark. If set, the logic will expand
* nodes until it eaches at least this number, and then stops. Root nodes will always
* show regardless of this settting. Further nodes can be lazy-loaded via ajax.
* This isn't a hard limit. Example: On a value of 10, with 20 root nodes, each having
* 30 children, the actual node count will be 50 (all root nodes plus first expanded child).
* @param int $nodeCountThreshold See {@link self::$node_threshold_total}
* @param callable $nodeCountCallback Called with the node count, which gives the callback an opportunity
* to intercept the query. Useful e.g. to avoid excessive children listings (Arguments: $parent, $numChildren)
*
* @return string
*/
public function getChildrenAsUL($attributes = "", $titleEval = '"<li>" . $child->Title', $extraArg = null,
$limitToMarked = false, $childrenMethod = "AllChildrenIncludingDeleted",
$numChildrenMethod = "numChildren", $rootCall = true, $nodeCountThreshold = 30) {
$numChildrenMethod = "numChildren", $rootCall = true, $nodeCountThreshold = null, $nodeCountCallback = null) {
if(!is_numeric($nodeCountThreshold)) {
$nodeCountThreshold = Config::inst()->get('Hierarchy', 'node_threshold_total');
}
if($limitToMarked && $rootCall) {
$this->markingFinished($numChildrenMethod);
}
if($nodeCountCallback) {
$nodeCountWarning = $nodeCountCallback($this->owner, $this->owner->$numChildrenMethod());
if($nodeCountWarning) return $nodeCountWarning;
}
if($this->owner->hasMethod($childrenMethod)) {
$children = $this->owner->$childrenMethod($extraArg);
@ -110,7 +140,6 @@ class Hierarchy extends DataExtension {
$output .= (is_callable($titleEval)) ? $titleEval($child) : eval("return $titleEval;");
$output .= "\n";
$numChildren = $child->$numChildrenMethod();
if(
// Always traverse into opened nodes (they might be exposed as parents of search results)
@ -119,10 +148,16 @@ class Hierarchy extends DataExtension {
// Otherwise, the remaining nodes are lazy loaded via ajax.
&& $child->isMarked()
) {
$output .= $child->getChildrenAsUL("", $titleEval, $extraArg, $limitToMarked, $childrenMethod,
$numChildrenMethod, false, $nodeCountThreshold);
}
elseif($child->isTreeOpened()) {
// Additionally check if node count requirements are met
$nodeCountWarning = $nodeCountCallback ? $nodeCountCallback($child, $numChildren) : null;
if($nodeCountWarning) {
$output .= $nodeCountWarning;
$child->markClosed();
} else {
$output .= $child->getChildrenAsUL("", $titleEval, $extraArg, $limitToMarked, $childrenMethod,
$numChildrenMethod, false, $nodeCountThreshold);
}
} elseif($child->isTreeOpened()) {
// Since we're not loading children, don't mark it as open either
$child->markClosed();
}

163
tests/model/HierarchyTest.php
View File

@ -60,11 +60,11 @@ class HierarchyTest extends SapphireTest {
$obj3 = Versioned::get_including_deleted("HierarchyTest_Object", "\"Title\" = 'Obj 3'")->First();
// Check that both obj 3 children are returned
$this->assertEquals(array("Obj 3a", "Obj 3b"),
$this->assertEquals(array("Obj 3a", "Obj 3b", "Obj 3c"),
$obj3->AllHistoricalChildren()->column('Title'));
// Check numHistoricalChildren
$this->assertEquals(2, $obj3->numHistoricalChildren());
$this->assertEquals(3, $obj3->numHistoricalChildren());
}
@ -94,7 +94,7 @@ class HierarchyTest extends SapphireTest {
public function testNumChildren() {
$this->assertEquals($this->objFromFixture('HierarchyTest_Object', 'obj1')->numChildren(), 0);
$this->assertEquals($this->objFromFixture('HierarchyTest_Object', 'obj2')->numChildren(), 2);
$this->assertEquals($this->objFromFixture('HierarchyTest_Object', 'obj3')->numChildren(), 2);
$this->assertEquals($this->objFromFixture('HierarchyTest_Object', 'obj3')->numChildren(), 3);
$this->assertEquals($this->objFromFixture('HierarchyTest_Object', 'obj2a')->numChildren(), 2);
$this->assertEquals($this->objFromFixture('HierarchyTest_Object', 'obj2b')->numChildren(), 0);
$this->assertEquals($this->objFromFixture('HierarchyTest_Object', 'obj3a')->numChildren(), 2);
@ -200,29 +200,13 @@ class HierarchyTest extends SapphireTest {
true, // rootCall
$nodeCountThreshold
);
$parser = new CSSContentParser($html);
$node2 = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]'
);
$this->assertTrue(
(bool)$node2,
$this->assertTreeContains($html, array($obj2),
'Contains root elements'
);
$node2a = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]' .
'/ul/li[@id="' . $obj2a->ID . '"]'
);
$this->assertTrue(
(bool)$node2a,
$this->assertTreeContains($html, array($obj2, $obj2a),
'Contains child elements (in correct nesting)'
);
$node2aa = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]' .
'/ul/li[@id="' . $obj2a->ID . '"]' .
'/ul/li[@id="' . $obj2aa->ID . '"]'
);
$this->assertTrue(
(bool)$node2aa,
$this->assertTreeContains($html, array($obj2, $obj2a, $obj2aa),
'Contains grandchild elements (in correct nesting)'
);
}
@ -247,27 +231,13 @@ class HierarchyTest extends SapphireTest {
true,
$nodeCountThreshold
);
$parser = new CSSContentParser($html);
$node1 = $parser->getByXpath(
'//ul/li[@id="' . $obj1->ID . '"]'
);
$this->assertTrue(
(bool)$node1,
$this->assertTreeContains($html, array($obj1),
'Contains root elements'
);
$node2 = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]'
);
$this->assertTrue(
(bool)$node2,
$this->assertTreeContains($html, array($obj2),
'Contains root elements'
);
$node2a = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]' .
'/ul/li[@id="' . $obj2a->ID . '"]'
);
$this->assertFalse(
(bool)$node2a,
$this->assertTreeNotContains($html, array($obj2, $obj2a),
'Does not contains child elements because they exceed minNodeCount'
);
}
@ -296,20 +266,12 @@ class HierarchyTest extends SapphireTest {
true,
$nodeCountThreshold
);
$parser = new CSSContentParser($html);
$node2 = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]'
);
$this->assertTrue(
(bool)$node2,
$this->assertTreeContains($html, array($obj2),
'Contains root elements'
);
$node2aa = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]' .
'/ul/li[@id="' . $obj2a->ID . '"]' .
'/ul/li[@id="' . $obj2aa->ID . '"]'
$this->assertTreeContains($html, array($obj2, $obj2a, $obj2aa),
'Does contain marked children nodes regardless of configured threshold'
);
$this->assertTrue((bool)$node2aa);
}
public function testGetChildrenAsULMinNodeCountWithFilters() {
@ -343,21 +305,10 @@ class HierarchyTest extends SapphireTest {
true,
$nodeCountThreshold
);
$parser = new CSSContentParser($html);
$node1 = $parser->getByXpath(
'//ul/li[@id="' . $obj1->ID . '"]'
);
$this->assertFalse(
(bool)$node1,
$this->assertTreeNotContains($html, array($obj1),
'Does not contain root elements which dont match the filter'
);
$node2aa = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]' .
'/ul/li[@id="' . $obj2a->ID . '"]' .
'/ul/li[@id="' . $obj2aa->ID . '"]'
);
$this->assertTrue(
(bool)$node2aa,
$this->assertTreeContains($html, array($obj2, $obj2a, $obj2aa),
'Contains non-root elements which match the filter'
);
}
@ -377,8 +328,6 @@ class HierarchyTest extends SapphireTest {
$root->setMarkingFilterFunction(function($record) use($obj2, $obj2a, $obj2aa) {
// Results need to include parent hierarchy, even if we just want to
// match the innermost node.
// var_dump($record->Title);
// var_dump(in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID)));
return in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID));
});
$root->markPartialTree($nodeCountThreshold);
@ -393,25 +342,83 @@ class HierarchyTest extends SapphireTest {
true,
$nodeCountThreshold
);
$parser = new CSSContentParser($html);
$node1 = $parser->getByXpath(
'//ul/li[@id="' . $obj1->ID . '"]'
);
$this->assertFalse(
(bool)$node1,
$this->assertTreeNotContains($html, array($obj1),
'Does not contain root elements which dont match the filter'
);
$node2aa = $parser->getByXpath(
'//ul/li[@id="' . $obj2->ID . '"]' .
'/ul/li[@id="' . $obj2a->ID . '"]' .
'/ul/li[@id="' . $obj2aa->ID . '"]'
);
$this->assertTrue(
(bool)$node2aa,
$this->assertTreeContains($html, array($obj2, $obj2a, $obj2aa),
'Contains non-root elements which match the filter'
);
}
public function testGetChildrenAsULNodeThresholdLeaf() {
$obj1 = $this->objFromFixture('HierarchyTest_Object', 'obj1');
$obj2 = $this->objFromFixture('HierarchyTest_Object', 'obj2');
$obj2a = $this->objFromFixture('HierarchyTest_Object', 'obj2a');
$obj3 = $this->objFromFixture('HierarchyTest_Object', 'obj3');
$obj3a = $this->objFromFixture('HierarchyTest_Object', 'obj3a');
$nodeCountThreshold = 99;
$root = new HierarchyTest_Object();
$root->markPartialTree($nodeCountThreshold);
$nodeCountCallback = function($parent, $numChildren) {
// Set low enough that it the fixture structure should exceed it
if($parent->ID && $numChildren > 2) {
return '<span class="exceeded">Exceeded!</span>';
}
};
$html = $root->getChildrenAsUL(
"",
'"<li id=\"" . $child->ID . "\">" . $child->Title',
null,
true, // limit to marked
"AllChildrenIncludingDeleted",
"numChildren",
true,
$nodeCountThreshold,
$nodeCountCallback
);
$this->assertTreeContains($html, array($obj1),
'Does contain root elements regardless of count'
);
$this->assertTreeContains($html, array($obj3),
'Does contain root elements regardless of count'
);
$this->assertTreeContains($html, array($obj2, $obj2a),
'Contains children which do not exceed threshold'
);
$this->assertTreeNotContains($html, array($obj3, $obj3a),
'Does not contain children which exceed threshold'
);
}
/**
* @param String $html [description]
* @param array $nodes Breadcrumb path as array
* @param String $message
*/
protected function assertTreeContains($html, $nodes, $message = null) {
$parser = new CSSContentParser($html);
$xpath = '/';
foreach($nodes as $node) $xpath .= '/ul/li[@id="' . $node->ID . '"]';
$match = $parser->getByXpath($xpath);
self::assertThat((bool)$match, self::isTrue(), $message);
}
/**
* @param String $html [description]
* @param array $nodes Breadcrumb path as array
* @param String $message
*/
protected function assertTreeNotContains($html, $nodes, $message = null) {
$parser = new CSSContentParser($html);
$xpath = '/';
foreach($nodes as $node) $xpath .= '/ul/li[@id="' . $node->ID . '"]';
$match = $parser->getByXpath($xpath);
self::assertThat((bool)$match, self::isFalse(), $message);
}
}
class HierarchyTest_Object extends DataObject implements TestOnly {

3
tests/model/HierarchyTest.yml
View File

@ -17,6 +17,9 @@ HierarchyTest_Object:
obj3b:
Parent: =>HierarchyTest_Object.obj3
Title: Obj 3b
obj3c:
Parent: =>HierarchyTest_Object.obj3
Title: Obj 3c
obj2aa:
Parent: =>HierarchyTest_Object.obj2a
Title: Obj 2aa