Skip to content

Commit

Permalink
NEW Enforce max node counts to avoid excessive resource usage
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@chillu
chillu authored and Sam Minnee committed Apr 8, 2013
1 parent 88d77db commit 01f46d0
Show file tree
Hide file tree
Showing 9 changed files with 257 additions and 103 deletions.
56 changes: 47 additions & 9 deletions admin/code/LeftAndMain.php
View file
Expand Up @@ -779,15 +779,53 @@ public function getSiteTreeFor($className, $rootID = null, $childrenMethod = nul
$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) {
Expand Down
3 changes: 3 additions & 0 deletions admin/css/screen.css
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions admin/scss/_tree.scss
View file
Expand Up @@ -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;
Expand All @@ -441,6 +454,7 @@
margin-right: 6px;
margin-top: -1px;
@include border-radius(2px, 2px);

&.modified, &.addedtodraft {
color: #7E7470;
border: 1px solid #C9B800;
Expand Down
5 changes: 4 additions & 1 deletion docs/en/changelogs/3.1.0.md
View file
Expand Up @@ -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 changes: 13 additions & 2 deletions docs/en/reference/sitetree.md
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
44 changes: 42 additions & 2 deletions forms/TreeDropdownField.php
View file
Expand Up @@ -269,10 +269,50 @@ public function tree(SS_HTTPRequest $request) {
. $this->keyField . '\" class=\"class-$child->class"'
. ' . $child->markingClasses() . "\"><a rel=\"$child->ID\">" . $child->' . $this->labelField . ' . "</a>"';

// 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 {
$nodeCountCallback = null;
}

if($isSubTree) {
return substr(trim($obj->getChildrenAsUL('', $eval, null, true, $this->childrenMethod)), 4, -5);
$html = $obj->getChildrenAsUL(
"",
$eval,
null,
true,
$this->childrenMethod,
'numChildren',
true, // root call
null,
$nodeCountCallback
);
return substr(trim($html), 4, -5);
} else {
return $obj->getChildrenAsUL('class="tree"', $eval, null, true, $this->childrenMethod);
$html = $obj->getChildrenAsUL(
'class="tree"',
$eval,
null,
true,
$this->childrenMethod,
'numChildren',
true, // root call
null,
$nodeCountCallback
);
return $html;
}
}

Expand Down
57 changes: 46 additions & 11 deletions model/Hierarchy.php
View file
Expand Up @@ -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) {
}
Expand Down Expand Up @@ -74,21 +96,29 @@ public function validate(ValidationResult $validationResult) {
* @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);
Expand All @@ -110,7 +140,6 @@ public function getChildrenAsUL($attributes = "", $titleEval = '"<li>" . $child-
$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)
Expand All @@ -119,10 +148,16 @@ public function getChildrenAsUL($attributes = "", $titleEval = '"<li>" . $child-
// 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();
}
Expand Down

0 comments on commit 01f46d0

Please sign in to comment.