Merge pull request #6792 from open-sausages/pulls/4.0/tree-dropdown-react

API major Hierarchy refactor

docs/en/04_Changelogs/4.0.0.md

@@ -16,7 +16,6 @@ guide developers in preparing existing 3.x code for compatibility with 4.0
   * [Filesystem API](#overview-filesystem)
   * [Template and Form API](#overview-template)
   * [i18n](#overview-i18n)
-  * [Cache](#overview-cache)
   * [Email and Mailer](#overview-mailer)
   * [SapphireTest](#overview-testing)
 * [Commit History](#commit-history)
@@ -1034,6 +1033,108 @@ One removed feature is the `Config::FIRST_SET` option. Either use uninherited co
 directly, or use the inherited config lookup. As falsey values now overwrite all parent class values, it is
 now generally safer to use the default inherited config, where in the past you would need to use `FIRST_SET`.
 
+#### Upgrading Cache API
+
+We have replaced the unsupported `Zend_Cache` library with [symfony/cache](https://github.com/symfony/cache).
+This also allowed us to remove SilverStripe's `Cache` API and use dependency injection with a standard
+[PSR-16](http://www.php-fig.org/psr/psr-16/) cache interface instead.
+
+Caches should be retrieved through `Injector` instead of `Cache::factory()`,
+and have a slightly different API (e.g. `set()` instead of `save()`).
+
+Before:
+
+
+    :::php
+    $cache = Cache::factory('myCache');
+     
+    // create a new item by trying to get it from the cache
+    $myValue = $cache->load('myCacheKey');
+    
+    // set a value and save it via the adapter
+    $cache->save(1234, 'myCacheKey');
+    
+    // retrieve the cache item
+    if (!$cache->load('myCacheKey')) {
+        // ... item does not exists in the cache
+    }
+    
+    // Remove a cache key
+    $cache->remove('myCacheKey');
+
+
+After:
+
+
+    :::php
+    use Psr\SimpleCache\CacheInterface;
+    $cache = Injector::inst()->get(CacheInterface::class . '.myCache');
+
+    // create a new item by trying to get it from the cache
+    $myValue = $cache->get('myCacheKey');
+    
+    // set a value and save it via the adapter
+    $cache->set('myCacheKey', 1234);
+    
+    // retrieve the cache item
+    if (!$cache->has('myCacheKey')) {
+        // ... item does not exists in the cache
+    }
+    
+    $cache->delete('myCacheKey');
+
+
+With the necessary minimal config in `_config/mycache.yml`
+
+
+    :::yml
+    ---
+    Name: mycache
+    ---
+    SilverStripe\Core\Injector\Injector:
+      Psr\SimpleCache\CacheInterface.myCache:
+        factory: SilverStripe\Core\Cache\CacheFactory
+        constructor:
+          namespace: 'mycache'
+
+
+##### Configuration Changes
+
+Caches are now configured through dependency injection services instead of PHP.
+See our ["Caching" docs](/developer-guides/performance/caching) for more details.
+ 
+Before (`mysite/_config.php`):
+
+    :::php
+    Cache::add_backend(
+        'primary_memcached', 
+        'Memcached',
+        array(
+            'servers' => array(
+                'host' => 'localhost', 
+                'port' => 11211, 
+            )
+        )
+    );
+    Cache::pick_backend('primary_memcached', 'any', 10);
+
+After (`mysite/_config/config.yml`):
+
+    :::yml
+    ---
+    After:
+      - '#corecache'
+    ---
+    SilverStripe\Core\Injector\Injector:
+      MemcachedClient:
+        class: 'Memcached'
+        calls:
+          - [ addServer, [ 'localhost', 11211 ] ]
+      SilverStripe\Core\Cache\CacheFactory:
+        class: 'SilverStripe\Core\Cache\MemcachedCacheFactory'
+        constructor:
+          client: '%$MemcachedClient
+
 
 ## <a name="api-changes"></a>API Changes
 
@@ -1108,6 +1209,7 @@ now generally safer to use the default inherited config, where in the past you w
 * Removed `CMSBatchAction_Delete`
 * Removed `CMSBatchAction_DeleteFromLive`
 * Removed `CMSMain.enabled_legacy_actions` config.
+* `CMSmain.getCMSTreeTitle` is now ignored on extensions. Use `updateCMSTreeTitle` in extensions instead.
 * Removed ability to run tests via web requests (`http://mydomain.com/dev/tests`), use the standard CLI
   command instead (`vendor/bin/phpunit`).
 * Removed `dev/jstests/` controller (no replacement)
@@ -1182,6 +1284,21 @@ A very small number of methods were chosen for deprecation, and will be removed
   * `BigSummary` is removed. Use `Summary` instead.
   * Most limit methods on `DBHTMLText` now plain text rather than attempt to manipulate the underlying HTML.
   * `FormField::Title` and `FormField::RightTitle` are now cast as plain text by default (but can be overridden).
+* `Hierarchy` class has had much of it's functionality refactored out into `MarkedSet`:
+  * `isMarked`
+  * `isTreeOpened`
+  * `isExpanded`
+  * `markByID`
+  * `markPartialTree`
+  * `markExpanded`
+  * `markUnexpanded`
+  * `markToExpose`
+  * `markClosed`
+  * `markOpened`
+  * `markedNodeIDs`
+  * `getChildrenAsUL` replaced with `renderChildren`, which now takes a template name.
+  * `markingFilterMatches` (and made protected)
+  * `markChildren` (and made protected)
 * Removed `DataList::applyFilterContext` private method
 * Search filter classes (e.g. `ExactMatchFilter`) are now registered with `Injector`
   via a new `DataListFilter.` prefix convention.
@@ -1302,6 +1419,12 @@ The below methods have been added or had their functionality updated to `DBDate`
 * Removed additional arguments from `DBMoney::getSymbol`. The result of this value is
   now localised based on the currency code assigned to the `DBMoney` instance
 * Removed `DBMoney::getAllowedCurrencies`. Apply validation to `MoneyField` instead.
+* `Hierarchy` has lots of removed api:
+  - `parentStack()` removed. Use `getAncestors()` instead
+  - `doAllChildrenIncludingDeleted()` removed. Use `AllChildrenIncludingDeleted()` instead.
+  - `naturalNext` removed.
+  - `naturalPrev` removed.
+  - `markingFinished` removed.
 
 ### <a name="overview-filesystem"></a>Filesystem API
 
@@ -1617,110 +1740,6 @@ New `TimeField` methods replace `getConfig()` / `setConfig()`
 * `i18n::get_language_code()` removed.
 * `i18n::get_common_locales()` removed.
 * `i18n.common_locales` config removed
-
-### <a name="overview-cache"></a>Cache API
-
-We have replaced the unsupported `Zend_Cache` library with [symfony/cache](https://github.com/symfony/cache).
-This also allowed us to remove SilverStripe's `Cache` API and use dependency injection with a standard
-[PSR-16](http://www.php-fig.org/psr/psr-16/) cache interface instead.
-
-#### Usage Changes
-
-Caches should be retrieved through `Injector` instead of `Cache::factory()`,
-and have a slightly different API (e.g. `set()` instead of `save()`).
-
-Before:
-
-
-    :::php
-    $cache = Cache::factory('myCache');
-     
-    // create a new item by trying to get it from the cache
-    $myValue = $cache->load('myCacheKey');
-    
-    // set a value and save it via the adapter
-    $cache->save(1234, 'myCacheKey');
-    
-    // retrieve the cache item
-    if (!$cache->load('myCacheKey')) {
-        // ... item does not exists in the cache
-    }
-    
-    // Remove a cache key
-    $cache->remove('myCacheKey');
-
-
-After:
-
-
-    :::php
-    use Psr\SimpleCache\CacheInterface;
-    $cache = Injector::inst()->get(CacheInterface::class . '.myCache');
-
-    // create a new item by trying to get it from the cache
-    $myValue = $cache->get('myCacheKey');
-    
-    // set a value and save it via the adapter
-    $cache->set('myCacheKey', 1234);
-    
-    // retrieve the cache item
-    if (!$cache->has('myCacheKey')) {
-        // ... item does not exists in the cache
-    }
-    
-    $cache->delete('myCacheKey');
-
-
-With the necessary minimal config in `_config/mycache.yml`
-
-
-    :::yml
-    ---
-    Name: mycache
-    ---
-    SilverStripe\Core\Injector\Injector:
-      Psr\SimpleCache\CacheInterface.myCache:
-        factory: SilverStripe\Core\Cache\CacheFactory
-        constructor:
-          namespace: 'mycache'
-
-
-#### Configuration Changes
-
-Caches are now configured through dependency injection services instead of PHP.
-See our ["Caching" docs](/developer-guides/performance/caching) for more details.
- 
-Before (`mysite/_config.php`):
-
-    :::php
-    Cache::add_backend(
-        'primary_memcached', 
-        'Memcached',
-        array(
-            'servers' => array(
-                'host' => 'localhost', 
-                'port' => 11211, 
-            )
-        )
-    );
-    Cache::pick_backend('primary_memcached', 'any', 10);
-
-After (`mysite/_config/config.yml`):
-
-    :::yml
-    ---
-    After:
-      - '#corecache'
-    ---
-    SilverStripe\Core\Injector\Injector:
-      MemcachedClient:
-        class: 'Memcached'
-        calls:
-          - [ addServer, [ 'localhost', 11211 ] ]
-      SilverStripe\Core\Cache\CacheFactory:
-        class: 'SilverStripe\Core\Cache\MemcachedCacheFactory'
-        constructor:
-          client: '%$MemcachedClient
           
 ### <a name="overview-mailer"></a>Email and Mailer
 

src/Forms/TreeDropdownField.php

@@ -2,12 +2,13 @@
 
 namespace SilverStripe\Forms;
 
+use SilverStripe\Assets\Folder;
 use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Convert;
-use SilverStripe\Core\Config\Config;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\Hierarchy\Hierarchy;
-use SilverStripe\View\Requirements;
+use SilverStripe\ORM\Hierarchy\MarkedSet;
 use SilverStripe\View\ViewableData;
 use Exception;
 use InvalidArgumentException;
@@ -54,7 +55,6 @@ use InvalidArgumentException;
  */
 class TreeDropdownField extends FormField
 {
-
     private static $url_handlers = array(
         '$Action!/$ID' => '$Action'
     );
@@ -64,24 +64,98 @@ class TreeDropdownField extends FormField
     );
 
     /**
-     * @ignore
+     * Class name for underlying object
+     *
+     * @var string
      */
-    protected $sourceObject, $keyField, $labelField, $filterCallback,
+    protected $sourceObject = null;
-        $disableCallback, $searchCallback, $baseID = 0;
+
     /**
-     * @var string default child method in Hierarchy->getChildrenAsUL
+     * Name of key field on underlying object
+     *
+     * @var string
+     */
+    protected $keyField = null;
+
+    /**
+     * Name of lavel field on underlying object
+     *
+     * @var string
+     */
+    protected $labelField = null;
+
+    /**
+     * Callback for filtering records
+     *
+     * @var callable
+     */
+    protected $filterCallback = null;
+
+    /**
+     * Callback for marking record as disabled
+     *
+     * @var callable
+     */
+    protected $disableCallback = null;
+
+    /**
+     * Callback for searching records. This callback takes the following arguments:
+     *  - sourceObject Object class to search
+     *  - labelField Label field
+     *  - search Search text
+     *
+     * @var callable
+     */
+    protected $searchCallback = null;
+
+    /**
+     * Filter for base record
+     *
+     * @var int
+     */
+    protected $baseID = 0;
+
+    /**
+     * Default child method in Hierarchy->getChildrenAsUL
+     *
+     * @var string
      */
     protected $childrenMethod = 'AllChildrenIncludingDeleted';
 
     /**
-     * @var string default child counting method in Hierarchy->getChildrenAsUL
+     * Default child counting method in Hierarchy->getChildrenAsUL
+     *
+     * @var string
      */
     protected $numChildrenMethod = 'numChildren';
 
     /**
-     * Used by field search to leave only the relevant entries
+     * Current string value for search text to filter on
+     *
+     * @var string
      */
-    protected $searchIds = null, $showSearch, $searchExpanded = array();
+    protected $search = null;
+
+    /**
+     * List of ids in current search result (keys are ids, values are true)
+     *
+     * @var array
+     */
+    protected $searchIds = [];
+
+    /**
+     * Determine if search should be shown
+     *
+     * @var bool
+     */
+    protected $showSearch = false;
+
+    /**
+     * List of ids which have their search expanded (keys are ids, values are true)
+     *
+     * @var array
+     */
+    protected $searchExpanded = [];
 
     /**
      * CAVEAT: for search to work properly $labelField must be a database field,
@@ -89,16 +163,7 @@ class TreeDropdownField extends FormField
      *
      * @param string $name the field name
      * @param string $title the field label
-     * @param string|array $sourceObject The object-type to list in the tree. This could
+     * @param string $sourceObject A DataObject class name with the {@link Hierarchy} extension.
-     * be one of the following:
-     * - A DataObject class name with the {@link Hierarchy} extension.
-     * - An array of key/value pairs, like a {@link DropdownField} source. In
-     *   this case, the field will act like show a flat list of tree items,
-     *   without any hierarchy. This is most useful in conjunction with
-     *   {@link TreeMultiselectField}, for presenting a set of checkboxes in
-     *   a compact view. Note, that all value strings must be XML encoded
-     *   safely prior to being passed in.
-     *
      * @param string $keyField to field on the source class to save as the
      *      field value (default ID).
      * @param string $labelField the field name to show as the human-readable
@@ -109,19 +174,24 @@ class TreeDropdownField extends FormField
     public function __construct(
         $name,
         $title = null,
-        $sourceObject = 'SilverStripe\\Security\\Group',
+        $sourceObject = null,
         $keyField = 'ID',
         $labelField = 'TreeTitle',
         $showSearch = true
     ) {
-
+        if (!is_a($sourceObject, DataObject::class, true)) {
+            throw new InvalidArgumentException("SourceObject must be a DataObject subclass");
+        }
+        if (!DataObject::has_extension($sourceObject, Hierarchy::class)) {
+            throw new InvalidArgumentException("SourceObject must have Hierarchy extension");
+        }
         $this->sourceObject = $sourceObject;
         $this->keyField     = $keyField;
         $this->labelField   = $labelField;
         $this->showSearch   = $showSearch;
 
-        //Extra settings for Folders
+        // Extra settings for Folders
-        if ($sourceObject == 'SilverStripe\\Assets\\Folder') {
+        if (strcasecmp($sourceObject, Folder::class) === 0) {
             $this->childrenMethod = 'ChildFolders';
             $this->numChildrenMethod = 'numChildFolders';
         }
@@ -194,6 +264,11 @@ class TreeDropdownField extends FormField
         return $this;
     }
 
+    /**
+     * Check if search is shown
+     *
+     * @return bool
+     */
     public function getShowSearch()
     {
         return $this->showSearch;
@@ -285,21 +360,11 @@ class TreeDropdownField extends FormField
      * Get the whole tree of a part of the tree via an AJAX request.
      *
      * @param HTTPRequest $request
-     * @return string
+     * @return HTTPResponse
      * @throws Exception
      */
     public function tree(HTTPRequest $request)
     {
-        // Array sourceObject is an explicit list of values - construct a "flat tree"
-        if (is_array($this->sourceObject)) {
-            $output = "<ul class=\"tree\">\n";
-            foreach ($this->sourceObject as $k => $v) {
-                $output .= '<li id="selector-' . $this->name . '-' . $k  . '"><a>' . $v . '</a>';
-            }
-            $output .= "</ul>";
-            return $output;
-        }
-
         // Regular source specification
         $isSubTree = false;
 
@@ -333,97 +398,61 @@ class TreeDropdownField extends FormField
             $this->populateIDs();
         }
 
+        // Create marking set
+        $markingSet = MarkedSet::create($obj, $this->childrenMethod, $this->numChildrenMethod, 30);
+
+        // Set filter on searched nodes
         if ($this->filterCallback || $this->search) {
-            $obj->setMarkingFilterFunction(array($this, "filterMarking"));
+            // Rely on filtering to limit tree
+            $markingSet->setMarkingFilterFunction(function ($node) {
+                return $this->filterMarking($node);
+            });
+            $markingSet->setLimitingEnabled(false);
         }
-        $obj->markPartialTree(
-            $nodeCountThreshold = 30,
-            $context = null,
-            $this->childrenMethod,
-            $this->numChildrenMethod
-        );
 
-        // allow to pass values to be selected within the ajax request
+        // Begin marking
-        if (isset($_REQUEST['forceValue']) || $this->value) {
+        $markingSet->markPartialTree();
-            $forceValue = ( isset($_REQUEST['forceValue']) ? $_REQUEST['forceValue'] : $this->value);
-            $values = preg_split('/,\s*/', $forceValue);
-            if ($values) {
-                foreach ($values as $value) {
-                    if (!$value || $value == 'unchanged') {
-                        continue;
-                    }
 
-                    $obj->markToExpose($this->objectForKey($value));
+        // Allow to pass values to be selected within the ajax request
+        $value = $request->requestVar('forceValue') ?: $this->value;
+        if ($value && ($values = preg_split('/,\s*/', $value))) {
+            foreach ($values as $value) {
+                if (!$value || $value == 'unchanged') {
+                    continue;
                 }
+
+                $markingSet->markToExpose($this->objectForKey($value));
             }
         }
 
-        $self = $this;
+        // Set title formatter
-        $titleFn = function (&$child) use (&$self) {
+        $customised = function (DataObject $child) use ($isSubTree) {
-            /** @var DataObject|Hierarchy $child */
+            return [
-            $keyField = $self->keyField;
+                'name' => $this->getName(),
-            $labelField = $self->labelField;
+                'id' => $child->obj($this->keyField),
-            return sprintf(
+                'title' => $child->obj($this->labelField),
-                '<li id="selector-%s-%s" data-id="%s" class="class-%s %s %s"><a rel="%d">%s</a>',
+                'disabled' => $this->nodeIsDisabled($child),
-                Convert::raw2xml($self->getName()),
+                'isSubTree' => $isSubTree
-                Convert::raw2xml($child->$keyField),
+            ];
-                Convert::raw2xml($child->$keyField),
-                Convert::raw2xml($child->class),
-                Convert::raw2xml($child->markingClasses($self->numChildrenMethod)),
-                ($self->nodeIsDisabled($child)) ? 'disabled' : '',
-                (int)$child->ID,
-                $child->obj($labelField)->forTemplate()
-            );
         };
 
-        // Limit the amount of nodes shown for performance reasons.
+        // Determine output format
-        // Skip the check if we're filtering the tree, since its not clear how many children will
+        if ($request->requestVar('format') === 'json') {
-        // match the filter criteria until they're queried (and matched up with previously marked nodes).
+            // Format JSON output
-        $nodeThresholdLeaf = Config::inst()->get('SilverStripe\\ORM\\Hierarchy\\Hierarchy', 'node_threshold_leaf');
+            $json = $markingSet
-        if ($nodeThresholdLeaf && !$this->filterCallback && !$this->search) {
+                ->getChildrenAsArray($customised);
-            $className = $this->sourceObject;
+            return HTTPResponse::create()
-            $nodeCountCallback = function ($parent, $numChildren) use ($className, $nodeThresholdLeaf) {
+                ->addHeader('Content-Type', 'application/json')
-                if ($className === 'SilverStripe\\CMS\\Model\\SiteTree'
+                ->setBody(json_encode($json));
-                    && $parent->ID
-                    && $numChildren > $nodeThresholdLeaf
-                ) {
-                    return sprintf(
-                        '<ul><li><span class="item">%s</span></li></ul>',
-                        _t('LeftAndMain.TooManyPages', 'Too many pages')
-                    );
-                }
-                return null;
-            };
         } else {
-            $nodeCountCallback = null;
+            // Return basic html
-        }
+            $html = $markingSet->renderChildren(
-
+                [self::class . '_HTML', 'type' => 'Includes'],
-        if ($isSubTree) {
+                $customised
-            $html = $obj->getChildrenAsUL(
-                "",
-                $titleFn,
-                null,
-                true,
-                $this->childrenMethod,
-                $this->numChildrenMethod,
-                true, // root call
-                null,
-                $nodeCountCallback
             );
-            return substr(trim($html), 4, -5);
+            return HTTPResponse::create()
-        } else {
+                ->addHeader('Content-Type', 'text/html')
-            $html = $obj->getChildrenAsUL(
+                ->setBody($html);
-                'class="tree"',
-                $titleFn,
-                null,
-                true,
-                $this->childrenMethod,
-                $this->numChildrenMethod,
-                true, // root call
-                null,
-                $nodeCountCallback
-            );
-            return $html;
         }
     }
 
@@ -432,7 +461,7 @@ class TreeDropdownField extends FormField
      * If a filter function has been set, that will be called. And if search text is set,
      * filter on that too. Return true if all applicable conditions are true, false otherwise.
      *
-     * @param mixed $node
+     * @param DataObject $node
      * @return bool
      */
     public function filterMarking($node)
@@ -440,7 +469,8 @@ class TreeDropdownField extends FormField
         if ($this->filterCallback && !call_user_func($this->filterCallback, $node)) {
             return false;
         }
-        if ($this->search != "") {
+
+        if ($this->search) {
             return isset($this->searchIds[$node->ID]) && $this->searchIds[$node->ID] ? true : false;
         }
 
@@ -591,11 +621,10 @@ class TreeDropdownField extends FormField
     public function performReadonlyTransformation()
     {
         /** @var TreeDropdownField_Readonly $copy */
-        $copy = $this->castedCopy('SilverStripe\\Forms\\TreeDropdownField_Readonly');
+        $copy = $this->castedCopy(TreeDropdownField_Readonly::class);
         $copy->setKeyField($this->keyField);
         $copy->setLabelField($this->labelField);
         $copy->setSourceObject($this->sourceObject);
-
         return $copy;
     }
 }

src/ORM/Hierarchy/Hierarchy.php

@@ -3,11 +3,9 @@
 namespace SilverStripe\ORM\Hierarchy;
 
 use SilverStripe\Admin\LeftAndMain;
-use SilverStripe\CMS\Model\SiteTree;
-use SilverStripe\Core\Config\Config;
 use SilverStripe\Control\Controller;
-use SilverStripe\Core\Resettable;
 use SilverStripe\ORM\DataList;
+use SilverStripe\ORM\SS_List;
 use SilverStripe\ORM\ValidationResult;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataObject;
@@ -20,18 +18,25 @@ use Exception;
  * obvious example of this is SiteTree.
  *
  * @property int $ParentID
- * @property DataObject $owner
+ * @property DataObject|Hierarchy $owner
  * @method DataObject Parent()
  */
-class Hierarchy extends DataExtension implements Resettable
+class Hierarchy extends DataExtension
 {
 
-    protected $markedNodes;
+    /**
+     * Cache for {@see numChildren()}
+     *
+     * @var int
+     */
+    protected $_cache_numChildren = null;
 
-    protected $markingFilter;
+    /**
-
+     * Cache for {@see Children()}
-    /** @var int */
+     *
-    protected $_cache_numChildren;
+     * @var SS_List
+     */
+    protected $_cache_children = null;
 
     /**
      * The lower bounds for the amount of nodes to mark. If set, the logic will expand nodes until it reaches at least
@@ -96,498 +101,41 @@ class Hierarchy extends DataExtension implements Resettable
     public function validate(ValidationResult $validationResult)
     {
         // The object is new, won't be looping.
-        if (!$this->owner->ID) {
+        /** @var DataObject|Hierarchy $owner */
+        $owner = $this->owner;
+        if (!$owner->ID) {
             return;
         }
         // The object has no parent, won't be looping.
-        if (!$this->owner->ParentID) {
+        if (!$owner->ParentID) {
             return;
         }
         // The parent has not changed, skip the check for performance reasons.
-        if (!$this->owner->isChanged('ParentID')) {
+        if (!$owner->isChanged('ParentID')) {
             return;
         }
 
         // Walk the hierarchy upwards until we reach the top, or until we reach the originating node again.
-        $node = $this->owner;
+        $node = $owner;
-        while ($node) {
+        while ($node && $node->ParentID) {
-            if ($node->ParentID==$this->owner->ID) {
+            if ((int)$node->ParentID === (int)$owner->ID) {
                 // Hierarchy is looping.
                 $validationResult->addError(
                     _t(
                         'Hierarchy.InfiniteLoopNotAllowed',
                         'Infinite loop found within the "{type}" hierarchy. Please change the parent to resolve this',
                         'First argument is the class that makes up the hierarchy.',
-                        array('type' => $this->owner->class)
+                        array('type' => $owner->class)
                     ),
                     'bad',
                     'INFINITE_LOOP'
                 );
                 break;
             }
-            $node = $node->ParentID ? $node->Parent() : null;
+            $node = $node->Parent();
-        }
-
-        // At this point the $validationResult contains the response.
-    }
-
-    /**
-     * Returns the children of this DataObject as an XHTML UL. This will be called recursively on each child, so if they
-     * have children they will be displayed as a UL inside a LI.
-     *
-     * @param string $attributes Attributes to add to the UL
-     * @param string|callable $titleEval PHP code to evaluate to start each child - this should include '<li>'
-     * @param string $extraArg Extra arguments that will be passed on to children, for if they overload this function
-     * @param bool $limitToMarked Display only marked children
-     * @param string $childrenMethod The name of the method used to get children from each object
-     * @param string $numChildrenMethod
-     * @param bool $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 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 = null,
-        $nodeCountCallback = null
-    ) {
-        if (!is_numeric($nodeCountThreshold)) {
-            $nodeCountThreshold = Config::inst()->get(__CLASS__, '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);
-        } else {
-            $children = null;
-            user_error(sprintf(
-                "Can't find the method '%s' on class '%s' for getting tree children",
-                $childrenMethod,
-                get_class($this->owner)
-            ), E_USER_ERROR);
-        }
-
-        $output = null;
-        if ($children) {
-            if ($attributes) {
-                $attributes = " $attributes";
-            }
-
-            $output = "<ul$attributes>\n";
-
-            foreach ($children as $child) {
-                if (!$limitToMarked || $child->isMarked()) {
-                    $foundAChild = true;
-                    if (is_callable($titleEval)) {
-                        $output .= $titleEval($child, $numChildrenMethod);
-                    } else {
-                        $output .= eval("return $titleEval;");
-                    }
-                    $output .= "\n";
-
-                    $numChildren = $child->$numChildrenMethod();
-
-                    if (// Always traverse into opened nodes (they might be exposed as parents of search results)
-                        $child->isExpanded()
-                        // Only traverse into children if we haven't reached the maximum node count already.
-                        // Otherwise, the remaining nodes are lazy loaded via ajax.
-                        && $child->isMarked()
-                    ) {
-                        // 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();
-                    }
-                    $output .= "</li>\n";
-                }
-            }
-
-            $output .= "</ul>\n";
-        }
-
-        if (isset($foundAChild) && $foundAChild) {
-            return $output;
-        }
-        return null;
-    }
-
-    /**
-     * Mark a segment of the tree, by calling mark().
-     *
-     * The method performs a breadth-first traversal until the number of nodes is more than minCount. This is used to
-     * get a limited number of tree nodes to show in the CMS initially.
-     *
-     * This method returns the number of nodes marked.  After this method is called other methods can check
-     * {@link isExpanded()} and {@link isMarked()} on individual nodes.
-     *
-     * @param int $nodeCountThreshold See {@link getChildrenAsUL()}
-     * @param mixed $context
-     * @param string $childrenMethod
-     * @param string $numChildrenMethod
-     * @return int The actual number of nodes marked.
-     */
-    public function markPartialTree(
-        $nodeCountThreshold = 30,
-        $context = null,
-        $childrenMethod = "AllChildrenIncludingDeleted",
-        $numChildrenMethod = "numChildren"
-    ) {
-        if (!is_numeric($nodeCountThreshold)) {
-            $nodeCountThreshold = 30;
-        }
-
-        $this->markedNodes = array($this->owner->ID => $this->owner);
-        $this->owner->markUnexpanded();
-
-        // foreach can't handle an ever-growing $nodes list
-        while (list($id, $node) = each($this->markedNodes)) {
-            $children = $this->markChildren($node, $context, $childrenMethod, $numChildrenMethod);
-            if ($nodeCountThreshold && sizeof($this->markedNodes) > $nodeCountThreshold) {
-                // Undo marking children as opened since they're lazy loaded
-                if ($children) {
-                    foreach ($children as $child) {
-                        $child->markClosed();
-                    }
-                }
-                break;
-            }
-        }
-        return sizeof($this->markedNodes);
-    }
-
-    /**
-     * Filter the marking to only those object with $node->$parameterName == $parameterValue
-     *
-     * @param string $parameterName  The parameter on each node to check when marking.
-     * @param mixed  $parameterValue The value the parameter must be to be marked.
-     */
-    public function setMarkingFilter($parameterName, $parameterValue)
-    {
-        $this->markingFilter = array(
-            "parameter" => $parameterName,
-            "value" => $parameterValue
-        );
-    }
-
-    /**
-     * Filter the marking to only those where the function returns true. The node in question will be passed to the
-     * function.
-     *
-     * @param string $funcName The name of the function to call
-     */
-    public function setMarkingFilterFunction($funcName)
-    {
-        $this->markingFilter = array(
-            "func" => $funcName,
-        );
-    }
-
-    /**
-     * Returns true if the marking filter matches on the given node.
-     *
-     * @param DataObject $node Node to check
-     * @return bool
-     */
-    public function markingFilterMatches($node)
-    {
-        if (!$this->markingFilter) {
-            return true;
-        }
-
-        if (isset($this->markingFilter['parameter']) && $parameterName = $this->markingFilter['parameter']) {
-            if (is_array($this->markingFilter['value'])) {
-                $ret = false;
-                foreach ($this->markingFilter['value'] as $value) {
-                    $ret = $ret||$node->$parameterName==$value;
-                    if ($ret == true) {
-                        break;
-                    }
-                }
-                return $ret;
-            } else {
-                return ($node->$parameterName == $this->markingFilter['value']);
-            }
-        } elseif ($func = $this->markingFilter['func']) {
-            return call_user_func($func, $node);
         }
     }
 
-    /**
-     * Mark all children of the given node that match the marking filter.
-     *
-     * @param DataObject $node              Parent node
-     * @param mixed      $context
-     * @param string     $childrenMethod    The name of the instance method to call to get the object's list of children
-     * @param string     $numChildrenMethod The name of the instance method to call to count the object's children
-     * @return DataList
-     */
-    public function markChildren(
-        $node,
-        $context = null,
-        $childrenMethod = "AllChildrenIncludingDeleted",
-        $numChildrenMethod = "numChildren"
-    ) {
-        if ($node->hasMethod($childrenMethod)) {
-            $children = $node->$childrenMethod($context);
-        } else {
-            $children = null;
-            user_error(sprintf(
-                "Can't find the method '%s' on class '%s' for getting tree children",
-                $childrenMethod,
-                get_class($node)
-            ), E_USER_ERROR);
-        }
-
-        $node->markExpanded();
-        if ($children) {
-            foreach ($children as $child) {
-                $markingMatches = $this->markingFilterMatches($child);
-                if ($markingMatches) {
-                    // Mark a child node as unexpanded if it has children and has not already been expanded
-                    if ($child->$numChildrenMethod() && !$child->isExpanded()) {
-                        $child->markUnexpanded();
-                    } else {
-                        $child->markExpanded();
-                    }
-                    $this->markedNodes[$child->ID] = $child;
-                }
-            }
-        }
-
-        return $children;
-    }
-
-    /**
-     * Ensure marked nodes that have children are also marked expanded. Call this after marking but before iterating
-     * over the tree.
-     *
-     * @param string $numChildrenMethod The name of the instance method to call to count the object's children
-     */
-    protected function markingFinished($numChildrenMethod = "numChildren")
-    {
-        // Mark childless nodes as expanded.
-        if ($this->markedNodes) {
-            foreach ($this->markedNodes as $id => $node) {
-                if (!$node->isExpanded() && !$node->$numChildrenMethod()) {
-                    $node->markExpanded();
-                }
-            }
-        }
-    }
-
-    /**
-     * Return CSS classes of 'unexpanded', 'closed', both, or neither, as well as a 'jstree-*' state depending on the
-     * marking of this DataObject.
-     *
-     * @param string $numChildrenMethod The name of the instance method to call to count the object's children
-     * @return string
-     */
-    public function markingClasses($numChildrenMethod = "numChildren")
-    {
-        $classes = '';
-        if (!$this->isExpanded()) {
-            $classes .= " unexpanded";
-        }
-
-        // Set jstree open state, or mark it as a leaf (closed) if there are no children
-        if (!$this->owner->$numChildrenMethod()) {
-            $classes .= " jstree-leaf closed";
-        } elseif ($this->isTreeOpened()) {
-            $classes .= " jstree-open";
-        } else {
-            $classes .= " jstree-closed closed";
-        }
-        return $classes;
-    }
-
-    /**
-     * Mark the children of the DataObject with the given ID.
-     *
-     * @param int  $id   ID of parent node
-     * @param bool $open If this is true, mark the parent node as opened
-     * @return bool
-     */
-    public function markById($id, $open = false)
-    {
-        if (isset($this->markedNodes[$id])) {
-            $this->markChildren($this->markedNodes[$id]);
-            if ($open) {
-                $this->markedNodes[$id]->markOpened();
-            }
-            return true;
-        } else {
-            return false;
-        }
-    }
-
-    /**
-     * Expose the given object in the tree, by marking this page and all it ancestors.
-     *
-     * @param DataObject $childObj
-     */
-    public function markToExpose($childObj)
-    {
-        if (is_object($childObj)) {
-            $stack = array_reverse($childObj->parentStack());
-            foreach ($stack as $stackItem) {
-                $this->markById($stackItem->ID, true);
-            }
-        }
-    }
-
-    /**
-     * Return the IDs of all the marked nodes.
-     *
-     * @return array
-     */
-    public function markedNodeIDs()
-    {
-        return array_keys($this->markedNodes);
-    }
-
-    /**
-     * Return an array of this page and its ancestors, ordered item -> root.
-     *
-     * @return SiteTree[]
-     */
-    public function parentStack()
-    {
-        $p = $this->owner;
-
-        while ($p) {
-            $stack[] = $p;
-            $p = $p->ParentID ? $p->Parent() : null;
-        }
-
-        return $stack;
-    }
-
-    /**
-     * Cache of DataObjects' marked statuses: [ClassName][ID] = bool
-     * @var array
-     */
-    protected static $marked = array();
-
-    /**
-     * Cache of DataObjects' expanded statuses: [ClassName][ID] = bool
-     * @var array
-     */
-    protected static $expanded = array();
-
-    /**
-     * Cache of DataObjects' opened statuses: [ClassName][ID] = bool
-     * @var array
-     */
-    protected static $treeOpened = array();
-
-    /**
-     * Mark this DataObject as expanded.
-     */
-    public function markExpanded()
-    {
-        self::$marked[$this->owner->baseClass()][$this->owner->ID] = true;
-        self::$expanded[$this->owner->baseClass()][$this->owner->ID] = true;
-    }
-
-    /**
-     * Mark this DataObject as unexpanded.
-     */
-    public function markUnexpanded()
-    {
-        self::$marked[$this->owner->baseClass()][$this->owner->ID] = true;
-        self::$expanded[$this->owner->baseClass()][$this->owner->ID] = false;
-    }
-
-    /**
-     * Mark this DataObject's tree as opened.
-     */
-    public function markOpened()
-    {
-        self::$marked[$this->owner->baseClass()][$this->owner->ID] = true;
-        self::$treeOpened[$this->owner->baseClass()][$this->owner->ID] = true;
-    }
-
-    /**
-     * Mark this DataObject's tree as closed.
-     */
-    public function markClosed()
-    {
-        if (isset(self::$treeOpened[$this->owner->baseClass()][$this->owner->ID])) {
-            unset(self::$treeOpened[$this->owner->baseClass()][$this->owner->ID]);
-        }
-    }
-
-    /**
-     * Check if this DataObject is marked.
-     *
-     * @return bool
-     */
-    public function isMarked()
-    {
-        $baseClass = $this->owner->baseClass();
-        $id = $this->owner->ID;
-        return isset(self::$marked[$baseClass][$id]) ? self::$marked[$baseClass][$id] : false;
-    }
-
-    /**
-     * Check if this DataObject is expanded.
-     *
-     * @return bool
-     */
-    public function isExpanded()
-    {
-        $baseClass = $this->owner->baseClass();
-        $id = $this->owner->ID;
-        return isset(self::$expanded[$baseClass][$id]) ? self::$expanded[$baseClass][$id] : false;
-    }
-
-    /**
-     * Check if this DataObject's tree is opened.
-     *
-     * @return bool
-     */
-    public function isTreeOpened()
-    {
-        $baseClass = $this->owner->baseClass();
-        $id = $this->owner->ID;
-        return isset(self::$treeOpened[$baseClass][$id]) ? self::$treeOpened[$baseClass][$id] : false;
-    }
 
     /**
      * Get a list of this DataObject's and all it's descendants IDs.
@@ -605,41 +153,39 @@ class Hierarchy extends DataExtension implements Resettable
      * Get a list of this DataObject's and all it's descendants ID, and put them in $idList.
      *
      * @param array $idList Array to put results in.
+     * @param DataObject|Hierarchy $node
      */
-    public function loadDescendantIDListInto(&$idList)
+    protected function loadDescendantIDListInto(&$idList, $node = null)
     {
-        if ($children = $this->AllChildren()) {
+        if (!$node) {
-            foreach ($children as $child) {
+            $node = $this->owner;
-                if (in_array($child->ID, $idList)) {
+        }
-                    continue;
+        $children = $node->AllChildren();
-                }
+        foreach ($children as $child) {
+            if (!in_array($child->ID, $idList)) {
                 $idList[] = $child->ID;
-                /** @var Hierarchy $ext */
+                $this->loadDescendantIDListInto($idList, $child);
-                $ext = $child->getExtensionInstance('SilverStripe\ORM\Hierarchy\Hierarchy');
-                $ext->setOwner($child);
-                $ext->loadDescendantIDListInto($idList);
-                $ext->clearOwner();
             }
         }
     }
 
     /**
-     * Get the children for this DataObject.
+     * Get the children for this DataObject filtered by canView()
      *
-     * @return DataList
+     * @return SS_List
      */
     public function Children()
     {
-        if (!(isset($this->_cache_children) && $this->_cache_children)) {
+        if ($this->_cache_children) {
-            $result = $this->owner->stageChildren(false);
+            return $this->_cache_children;
-            $children = array();
-            foreach ($result as $record) {
-                if ($record->canView()) {
-                    $children[] = $record;
-                }
-            }
-            $this->_cache_children = new ArrayList($children);
         }
+
+        $this->_cache_children = $this
+            ->owner
+            ->stageChildren(false)
+            ->filterByCallback(function (DataObject $record) {
+                return $record->canView();
+            });
         return $this->_cache_children;
     }
 
@@ -660,50 +206,24 @@ class Hierarchy extends DataExtension implements Resettable
      * - Modified children will be marked as "ModifiedOnStage"
      * - Everything else has "SameOnStage" set, as an indicator that this information has been looked up.
      *
-     * @param mixed $context
      * @return ArrayList
      */
-    public function AllChildrenIncludingDeleted($context = null)
+    public function AllChildrenIncludingDeleted()
     {
-        return $this->doAllChildrenIncludingDeleted($context);
+        $stageChildren = $this->owner->stageChildren(true);
-    }
 
-    /**
+        // Add live site content that doesn't exist on the stage site, if required.
-     * @see AllChildrenIncludingDeleted
+        if ($this->owner->hasExtension(Versioned::class)) {
-     *
+            // Next, go through the live children.  Only some of these will be listed
-     * @param mixed $context
+            $liveChildren = $this->owner->liveChildren(true, true);
-     * @return ArrayList
+            if ($liveChildren) {
-     */
+                $merged = new ArrayList();
-    public function doAllChildrenIncludingDeleted($context = null)
+                $merged->merge($stageChildren);
-    {
+                $merged->merge($liveChildren);
-        if (!$this->owner) {
+                $stageChildren = $merged;
-            user_error('Hierarchy::doAllChildrenIncludingDeleted() called without $this->owner');
-        }
-
-        $baseClass = $this->owner->baseClass();
-        if ($baseClass) {
-            $stageChildren = $this->owner->stageChildren(true);
-
-            // Add live site content that doesn't exist on the stage site, if required.
-            if ($this->owner->hasExtension(Versioned::class)) {
-                // Next, go through the live children.  Only some of these will be listed
-                $liveChildren = $this->owner->liveChildren(true, true);
-                if ($liveChildren) {
-                    $merged = new ArrayList();
-                    $merged->merge($stageChildren);
-                    $merged->merge($liveChildren);
-                    $stageChildren = $merged;
-                }
             }
-
-            $this->owner->extend("augmentAllChildrenIncludingDeleted", $stageChildren, $context);
-        } else {
-            user_error(
-                "Hierarchy::AllChildren() Couldn't determine base class for '{$this->owner->class}'",
-                E_USER_ERROR
-            );
         }
-
+        $this->owner->extend("augmentAllChildrenIncludingDeleted", $stageChildren);
         return $stageChildren;
     }
 
@@ -732,14 +252,9 @@ class Hierarchy extends DataExtension implements Resettable
      * Return the number of children that this page ever had, including pages that were deleted.
      *
      * @return int
-     * @throws Exception
      */
     public function numHistoricalChildren()
     {
-        if (!$this->owner->hasExtension(Versioned::class)) {
-            throw new Exception('Hierarchy->AllHistoricalChildren() only works with Versioned extension applied');
-        }
-
         return $this->AllHistoricalChildren()->count();
     }
 
@@ -752,15 +267,19 @@ class Hierarchy extends DataExtension implements Resettable
      */
     public function numChildren($cache = true)
     {
-        // Build the cache for this class if it doesn't exist.
+        // Load if caching
-        if (!$cache || !is_numeric($this->_cache_numChildren)) {
+        if ($cache && isset($this->_cache_numChildren)) {
-            // Hey, this is efficient now!
+            return $this->_cache_numChildren;
-            // We call stageChildren(), because Children() has canView() filtering
-            $this->_cache_numChildren = (int)$this->owner->stageChildren(true)->Count();
         }
 
-        // If theres no value in the cache, it just means that it doesn't have any children.
+        // We call stageChildren(), because Children() has canView() filtering
-        return $this->_cache_numChildren;
+        $children = (int)$this->owner->stageChildren(true)->Count();
+
+        // Save if caching
+        if ($cache) {
+            $this->_cache_numChildren = $children;
+        }
+        return $children;
     }
 
     /**
@@ -787,17 +306,16 @@ class Hierarchy extends DataExtension implements Resettable
      */
     public function stageChildren($showAll = false)
     {
-        $baseClass = $this->owner->baseClass();
+        $hideFromHierarchy = $this->owner->config()->hide_from_hierarchy;
-        $hide_from_hierarchy = $this->owner->config()->hide_from_hierarchy;
+        $hideFromCMSTree = $this->owner->config()->hide_from_cms_tree;
-        $hide_from_cms_tree = $this->owner->config()->hide_from_cms_tree;
+        $staged = DataObject::get($this->ownerBaseClass)
-        $staged = $baseClass::get()
                 ->filter('ParentID', (int)$this->owner->ID)
                 ->exclude('ID', (int)$this->owner->ID);
-        if ($hide_from_hierarchy) {
+        if ($hideFromHierarchy) {
-            $staged = $staged->exclude('ClassName', $hide_from_hierarchy);
+            $staged = $staged->exclude('ClassName', $hideFromHierarchy);
         }
-        if ($hide_from_cms_tree && $this->showingCMSTree()) {
+        if ($hideFromCMSTree && $this->showingCMSTree()) {
-            $staged = $staged->exclude('ClassName', $hide_from_cms_tree);
+            $staged = $staged->exclude('ClassName', $hideFromCMSTree);
         }
         if (!$showAll && DataObject::getSchema()->fieldSpec($this->owner, 'ShowInMenus')) {
             $staged = $staged->filter('ShowInMenus', 1);
@@ -821,21 +339,20 @@ class Hierarchy extends DataExtension implements Resettable
             throw new Exception('Hierarchy->liveChildren() only works with Versioned extension applied');
         }
 
-        $baseClass = $this->owner->baseClass();
+        $hideFromHierarchy = $this->owner->config()->hide_from_hierarchy;
-        $hide_from_hierarchy = $this->owner->config()->hide_from_hierarchy;
+        $hideFromCMSTree = $this->owner->config()->hide_from_cms_tree;
-        $hide_from_cms_tree = $this->owner->config()->hide_from_cms_tree;
+        $children = DataObject::get($this->owner->baseClass())
-        $children = $baseClass::get()
             ->filter('ParentID', (int)$this->owner->ID)
             ->exclude('ID', (int)$this->owner->ID)
             ->setDataQueryParam(array(
                 'Versioned.mode' => $onlyDeletedFromStage ? 'stage_unique' : 'stage',
                 'Versioned.stage' => 'Live'
             ));
-        if ($hide_from_hierarchy) {
+        if ($hideFromHierarchy) {
-            $children = $children->exclude('ClassName', $hide_from_hierarchy);
+            $children = $children->exclude('ClassName', $hideFromHierarchy);
         }
-        if ($hide_from_cms_tree && $this->showingCMSTree()) {
+        if ($hideFromCMSTree && $this->showingCMSTree()) {
-            $children = $children->exclude('ClassName', $hide_from_cms_tree);
+            $children = $children->exclude('ClassName', $hideFromCMSTree);
         }
         if (!$showAll && DataObject::getSchema()->fieldSpec($this->owner, 'ShowInMenus')) {
             $children = $children->filter('ShowInMenus', 1);
@@ -857,23 +374,27 @@ class Hierarchy extends DataExtension implements Resettable
         if (empty($parentID)) {
             return null;
         }
-        $idSQL = $this->owner->getSchema()->sqlColumnForField($this->owner, 'ID');
+        $idSQL = $this->owner->getSchema()->sqlColumnForField($this->ownerBaseClass, 'ID');
-        return DataObject::get_one($this->owner->class, array(
+        return DataObject::get_one($this->ownerBaseClass, array(
             array($idSQL => $parentID),
             $filter
         ));
     }
 
     /**
-     * Return all the parents of this class in a set ordered from the lowest to highest parent.
+     * Return all the parents of this class in a set ordered from the closest to furtherest parent.
      *
+     * @param bool $includeSelf
      * @return ArrayList
      */
-    public function getAncestors()
+    public function getAncestors($includeSelf = false)
     {
         $ancestors = new ArrayList();
-        $object    = $this->owner;
+        $object = $this->owner;
 
+        if ($includeSelf) {
+            $ancestors->push($object);
+        }
         while ($object = $object->getParent()) {
             $ancestors->push($object);
         }
@@ -891,81 +412,14 @@ class Hierarchy extends DataExtension implements Resettable
     {
         $crumbs = array();
         $ancestors = array_reverse($this->owner->getAncestors()->toArray());
+        /** @var DataObject $ancestor */
         foreach ($ancestors as $ancestor) {
-            $crumbs[] = $ancestor->Title;
+            $crumbs[] = $ancestor->getTitle();
         }
-        $crumbs[] = $this->owner->Title;
+        $crumbs[] = $this->owner->getTitle();
         return implode($separator, $crumbs);
     }
 
-    /**
-     * Get the next node in the tree of the type. If there is no instance of the className descended from this node,
-     * then search the parents.
-     *
-     * @todo Write!
-     *
-     * @param string     $className Class name of the node to find
-     * @param DataObject $afterNode Used for recursive calls to this function
-     * @return DataObject
-     */
-    public function naturalPrev($className, $afterNode = null)
-    {
-        return null;
-    }
-
-    /**
-     * Get the next node in the tree of the type. If there is no instance of the className descended from this node,
-     * then search the parents.
-     * @param string     $className Class name of the node to find.
-     * @param string|int $root      ID/ClassName of the node to limit the search to
-     * @param DataObject $afterNode Used for recursive calls to this function
-     * @return DataObject
-     */
-    public function naturalNext($className = null, $root = 0, $afterNode = null)
-    {
-        // If this node is not the node we are searching from, then we can possibly return this node as a solution
-        if ($afterNode && $afterNode->ID != $this->owner->ID) {
-            if (!$className || ($className && $this->owner->class == $className)) {
-                return $this->owner;
-            }
-        }
-
-        $nextNode = null;
-        $baseClass = $this->owner->baseClass();
-
-        $children = $baseClass::get()
-            ->filter('ParentID', (int)$this->owner->ID)
-            ->sort('"Sort"', 'ASC');
-        if ($afterNode) {
-            $children = $children->filter('Sort:GreaterThan', $afterNode->Sort);
-        }
-
-        // Try all the siblings of this node after the given node
-        /*if( $siblings = DataObject::get( $this->owner->baseClass(),
-		"\"ParentID\"={$this->owner->ParentID}" . ( $afterNode ) ? "\"Sort\"
-		> {$afterNode->Sort}" : "" , '\"Sort\" ASC' ) ) $searchNodes->merge( $siblings );*/
-
-        if ($children) {
-            foreach ($children as $node) {
-                if ($nextNode = $node->naturalNext($className, $node->ID, $this->owner)) {
-                    break;
-                }
-            }
-
-            if ($nextNode) {
-                return $nextNode;
-            }
-        }
-
-        // if this is not an instance of the root class or has the root id, search the parent
-        if (!(is_numeric($root) && $root == $this->owner->ID || $root == $this->owner->class)
-                && ($parent = $this->owner->Parent())) {
-            return $parent->naturalNext($className, $root, $this->owner);
-        }
-
-        return null;
-    }
-
     /**
      * Flush all Hierarchy caches:
      * - Children (instance)
@@ -978,21 +432,5 @@ class Hierarchy extends DataExtension implements Resettable
     {
         $this->_cache_children = null;
         $this->_cache_numChildren = null;
-        self::$marked = array();
-        self::$expanded = array();
-        self::$treeOpened = array();
-    }
-
-    /**
-     * Reset global Hierarchy caches:
-     * - Marked
-     * - Expanded
-     * - TreeOpened
-     */
-    public static function reset()
-    {
-        self::$marked = array();
-        self::$expanded = array();
-        self::$treeOpened = array();
     }
 }

src/ORM/Hierarchy/MarkedSet.php

@@ -0,0 +1,812 @@
+<?php
+
+namespace SilverStripe\ORM\Hierarchy;
+
+use InvalidArgumentException;
+use LogicException;
+use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\ORM\ArrayList;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\FieldType\DBField;
+use SilverStripe\ORM\SS_List;
+use SilverStripe\View\ArrayData;
+
+/**
+ * Contains a set of hierarchical objects generated from a marking compilation run.
+ *
+ * A set of nodes can be "marked" for later export, in order to prevent having to
+ * export the entire contents of a potentially huge tree.
+ */
+class MarkedSet
+{
+    use Injectable;
+
+    /**
+     * Marked nodes for a given subtree. The first item in this list
+     * is the root object of the subtree.
+     *
+     * A marked item is an item in a tree which will be included in
+     * a resulting tree.
+     *
+     * @var array Map of [itemID => itemInstance]
+     */
+    protected $markedNodes;
+
+    /**
+     * Optional filter callback for filtering nodes to mark
+     *
+     * Array with keys:
+     *  - parameter
+     *  - value
+     *  - func
+     *
+     * @var array
+     * @temp made public
+     */
+    public $markingFilter;
+
+    /**
+     * @var DataObject
+     */
+    protected $rootNode = null;
+
+    /**
+     * Method to use for getting children. Defaults to 'AllChildrenIncludingDeleted'
+     *
+     * @var string
+     */
+    protected $childrenMethod = null;
+
+    /**
+     * Method to use for counting children. Defaults to `numChildren`
+     *
+     * @var string
+     */
+    protected $numChildrenMethod = null;
+
+    /**
+     * Minimum number of nodes to iterate over before stopping recursion
+     *
+     * @var int
+     */
+    protected $nodeCountThreshold = null;
+
+    /**
+     * Max number of nodes to return from a single children collection
+     *
+     * @var int
+     */
+    protected $maxChildNodes;
+
+    /**
+     * Enable limiting
+     *
+     * @var bool
+     */
+    protected $enableLimiting = true;
+
+    /**
+     * Create an empty set with the given class
+     *
+     * @param DataObject $rootNode Root node for this set. To collect the entire tree,
+     * pass in a singelton object.
+     * @param string $childrenMethod Override children method
+     * @param string $numChildrenMethod Override children counting method
+     * @param int $nodeCountThreshold Minimum threshold for number nodes to mark
+     * @param int $maxChildNodes Maximum threshold for number of child nodes to include
+     */
+    public function __construct(
+        DataObject $rootNode,
+        $childrenMethod = null,
+        $numChildrenMethod = null,
+        $nodeCountThreshold = null,
+        $maxChildNodes = null
+    ) {
+        if (! $rootNode::has_extension(Hierarchy::class)) {
+            throw new InvalidArgumentException(
+                get_class($rootNode) . " does not have the Hierarchy extension"
+            );
+        }
+        $this->rootNode = $rootNode;
+        if ($childrenMethod) {
+            $this->setChildrenMethod($childrenMethod);
+        }
+        if ($numChildrenMethod) {
+            $this->setNumChildrenMethod($numChildrenMethod);
+        }
+        if ($nodeCountThreshold) {
+            $this->setNodeCountThreshold($nodeCountThreshold);
+        }
+        if ($maxChildNodes) {
+            $this->setMaxChildNodes($maxChildNodes);
+        }
+    }
+
+    /**
+     * Get total number of nodes to get. This acts as a soft lower-bounds for
+     * number of nodes to search until found.
+     * Defaults to value of node_threshold_total of hierarchy class.
+     *
+     * @return int
+     */
+    public function getNodeCountThreshold()
+    {
+        return $this->nodeCountThreshold
+            ?: $this->rootNode->config()->get('node_threshold_total');
+    }
+
+    /**
+     * Max number of nodes that can be physically rendered at any level.
+     * Acts as a hard upper bound, after which nodes will be trimmed for
+     * performance reasons.
+     *
+     * @return int
+     */
+    public function getMaxChildNodes()
+    {
+        return $this->maxChildNodes
+            ?: $this->rootNode->config()->get('node_threshold_leaf');
+    }
+
+    /**
+     * Set hard limit of number of nodes to get for this level
+     *
+     * @param int $count
+     * @return $this
+     */
+    public function setMaxChildNodes($count)
+    {
+        $this->maxChildNodes = $count;
+        return $this;
+    }
+
+    /**
+     * Set max node count
+     *
+     * @param int $total
+     * @return $this
+     */
+    public function setNodeCountThreshold($total)
+    {
+        $this->nodeCountThreshold = $total;
+        return $this;
+    }
+
+    /**
+     * Get method to use for getting children
+     *
+     * @return string
+     */
+    public function getChildrenMethod()
+    {
+        return $this->childrenMethod ?: 'AllChildrenIncludingDeleted';
+    }
+
+    /**
+     * Get children from this node
+     *
+     * @param DataObject $node
+     * @return SS_List
+     */
+    protected function getChildren(DataObject $node)
+    {
+        $method = $this->getChildrenMethod();
+        return $node->$method() ?: ArrayList::create();
+    }
+
+    /**
+     * Set method to use for getting children
+     *
+     * @param string $method
+     * @throws InvalidArgumentException
+     * @return $this
+     */
+    public function setChildrenMethod($method)
+    {
+        // Check method is valid
+        if (!$this->rootNode->hasMethod($method)) {
+            throw new InvalidArgumentException(sprintf(
+                "Can't find the method '%s' on class '%s' for getting tree children",
+                $method,
+                get_class($this->rootNode)
+            ));
+        }
+        $this->childrenMethod = $method;
+        return $this;
+    }
+
+    /**
+     * Get method name for num children
+     *
+     * @return string
+     */
+    public function getNumChildrenMethod()
+    {
+        return $this->numChildrenMethod ?: 'numChildren';
+    }
+
+    /**
+     * Count children
+     *
+     * @param DataObject $node
+     * @return int
+     */
+    protected function getNumChildren(DataObject $node)
+    {
+        $method = $this->getNumChildrenMethod();
+        return (int)$node->$method();
+    }
+
+    /**
+     * Set method name to get num children
+     *
+     * @param string $method
+     * @return $this
+     */
+    public function setNumChildrenMethod($method)
+    {
+        // Check method is valid
+        if (!$this->rootNode->hasMethod($method)) {
+            throw new InvalidArgumentException(sprintf(
+                "Can't find the method '%s' on class '%s' for counting tree children",
+                $method,
+                get_class($this->rootNode)
+            ));
+        }
+        $this->numChildrenMethod = $method;
+        return $this;
+    }
+
+    /**
+     * Returns the children of this DataObject as an XHTML UL. This will be called recursively on each child, so if they
+     * have children they will be displayed as a UL inside a LI.
+     *
+     * @param string $template Template for items in the list
+     * @param array|callable $context Additional arguments to add to template when rendering
+     * due to excessive line length. If callable, this will be executed with the current node dataobject
+     * @return string
+     */
+    public function renderChildren(
+        $template = null,
+        $context = []
+    ) {
+        // Default to HTML template
+        if (!$template) {
+            $template = [
+                'type' => 'Includes',
+                self::class . '_HTML'
+            ];
+        }
+        $tree = $this->getSubtree($this->rootNode, 0);
+        $node = $this->renderSubtree($tree, $template, $context);
+        return (string)$node->getField('SubTree');
+    }
+
+    /**
+     * Get child data formatted as JSON
+     *
+     * @param callable $serialiseEval A callback that takes a DataObject as a single parameter,
+     * and should return an array containing a simple array representation. This result will
+     * replace the 'node' property at each point in the tree.
+     * @return array
+     */
+    public function getChildrenAsArray($serialiseEval = null)
+    {
+        if (!$serialiseEval) {
+            $serialiseEval = function ($data) {
+                /** @var DataObject $node */
+                $node = $data['node'];
+                return [
+                    'id' => $node->ID,
+                    'title' => $node->getTitle()
+                ];
+            };
+        }
+
+        $tree = $this->getSubtree($this->rootNode, 0);
+
+        return $this->getSubtreeAsArray($tree, $serialiseEval);
+    }
+
+    /**
+     * Render a node in the tree with the given template
+     *
+     * @param array $data array data for current node
+     * @param string|array $template Template to use
+     * @param array|callable $context Additional arguments to add to template when rendering
+     * due to excessive line length. If callable, this will be executed with the current node dataobject
+     * @return ArrayData Viewable object representing the root node. use getField('SubTree') to get HTML
+     */
+    protected function renderSubtree($data, $template, $context = [])
+    {
+        // Render children
+        $childNodes = new ArrayList();
+        foreach ($data['children'] as $child) {
+            $childData = $this->renderSubtree($child, $template, $context);
+            $childNodes->push($childData);
+        }
+
+        // Build parent node
+        $parentNode = new ArrayData($data);
+        $parentNode->setField('children', $childNodes); // Replace raw array with template-friendly list
+        $parentNode->setField('markingClasses', $this->markingClasses($data['node']));
+
+        // Evaluate custom context
+        if (is_callable($context)) {
+            $context = call_user_func($context, $data['node']);
+        }
+        if ($context) {
+            foreach ($context as $key => $value) {
+                $parentNode->setField($key, $value);
+            }
+        }
+
+        // Render
+        $subtree = $parentNode->renderWith($template);
+        $parentNode->setField('SubTree', $subtree);
+        return $parentNode;
+    }
+
+    /**
+     * Return sub-tree as json array
+     *
+     * @param array $data
+     * @param callable $serialiseEval A callback that takes a DataObject as a single parameter,
+     * and should return an array containing a simple array representation. This result will
+     * replace the 'node' property at each point in the tree.
+     * @return mixed|string
+     */
+    protected function getSubtreeAsArray($data, $serialiseEval)
+    {
+        $output = $data;
+
+        // Serialise node
+        $output['node'] = $serialiseEval($data['node']);
+
+        // Force serialisation of DBField instances
+        if (is_array($output['node'])) {
+            foreach ($output['node'] as $key => $value) {
+                if ($value instanceof DBField) {
+                    $output['node'][$key] = $value->getSchemaValue();
+                }
+            }
+        } elseif ($output['node'] instanceof DBField) {
+            $output['node'] = $output['node']->getSchemaValue();
+        }
+
+        // Replace children with serialised elements
+        $output['children'] = [];
+        foreach ($data['children'] as $child) {
+            $output['children'][] = $this->getSubtreeAsArray($child, $serialiseEval);
+        }
+        return $output;
+    }
+
+    /**
+     * Get tree data for node
+     *
+     * @param DataObject $node
+     * @param int $depth
+     * @return array|string
+     */
+    protected function getSubtree($node, $depth = 0)
+    {
+        // Check if this node is limited due to child count
+        $numChildren = $this->getNumChildren($node);
+        $limited = $this->isNodeLimited($node, $numChildren);
+
+        // Build root rode
+        $expanded = $this->isExpanded($node);
+        $opened = $this->isTreeOpened($node);
+        $output = [
+            'node' => $node,
+            'marked' => $this->isMarked($node),
+            'expanded' => $expanded,
+            'opened' => $opened,
+            'depth' => $depth,
+            'count' => $numChildren, // Count of DB children
+            'limited' => $limited, // Flag whether 'items' has been limited
+            'children' => [], // Children to return in this request
+        ];
+
+        // Don't iterate children if past limit
+        // or not expanded (requires subsequent request to get)
+        if ($limited || !$expanded) {
+            return $output;
+        }
+
+        // Get children
+        $children = $this->getChildren($node);
+        foreach ($children as $child) {
+            // Recurse
+            if ($this->isMarked($child)) {
+                $output['children'][] = $this->getSubtree($child, $depth + 1);
+            }
+        }
+        return $output;
+    }
+
+    /**
+     * Mark a segment of the tree, by calling mark().
+     *
+     * The method performs a breadth-first traversal until the number of nodes is more than minCount. This is used to
+     * get a limited number of tree nodes to show in the CMS initially.
+     *
+     * This method returns the number of nodes marked.  After this method is called other methods can check
+     * {@link isExpanded()} and {@link isMarked()} on individual nodes.
+     *
+     * @return $this
+     */
+    public function markPartialTree()
+    {
+        $nodeCountThreshold = $this->getNodeCountThreshold();
+
+        // Add root node, not-expanded by default
+        /** @var DataObject|Hierarchy $rootNode */
+        $rootNode = $this->rootNode;
+        $this->clearMarks();
+        $this->markUnexpanded($rootNode);
+
+        // Build markedNodes for this subtree until we reach the threshold
+        // foreach can't handle an ever-growing $nodes list
+        while (list(, $node) = each($this->markedNodes)) {
+            $children = $this->markChildren($node);
+            if ($nodeCountThreshold && sizeof($this->markedNodes) > $nodeCountThreshold) {
+                // Undo marking children as opened since they're lazy loaded
+                /** @var DataObject|Hierarchy $child */
+                foreach ($children as $child) {
+                    $this->markClosed($child);
+                }
+                break;
+            }
+        }
+        return $this;
+    }
+
+    /**
+     * Filter the marking to only those object with $node->$parameterName == $parameterValue
+     *
+     * @param string $parameterName  The parameter on each node to check when marking.
+     * @param mixed  $parameterValue The value the parameter must be to be marked.
+     * @return $this
+     */
+    public function setMarkingFilter($parameterName, $parameterValue)
+    {
+        $this->markingFilter = array(
+            "parameter" => $parameterName,
+            "value" => $parameterValue
+        );
+        return $this;
+    }
+
+    /**
+     * Filter the marking to only those where the function returns true. The node in question will be passed to the
+     * function.
+     *
+     * @param callable $callback Callback to filter
+     * @return $this
+     */
+    public function setMarkingFilterFunction($callback)
+    {
+        $this->markingFilter = array(
+            "func" => $callback,
+        );
+        return $this;
+    }
+
+    /**
+     * Returns true if the marking filter matches on the given node.
+     *
+     * @param DataObject $node Node to check
+     * @return bool
+     */
+    protected function markingFilterMatches(DataObject $node)
+    {
+        if (!$this->markingFilter) {
+            return true;
+        }
+
+        // Func callback filter
+        if (isset($this->markingFilter['func'])) {
+            $func = $this->markingFilter['func'];
+            return call_user_func($func, $node);
+        }
+
+        // Check object property filter
+        if (isset($this->markingFilter['parameter'])) {
+            $parameterName = $this->markingFilter['parameter'];
+            $value = $this->markingFilter['value'];
+
+            if (is_array($value)) {
+                return in_array($node->$parameterName, $value);
+            } else {
+                return $node->$parameterName == $value;
+            }
+        }
+
+        throw new LogicException("Invalid marking filter");
+    }
+
+    /**
+     * Mark all children of the given node that match the marking filter.
+     *
+     * @param DataObject $node Parent node
+     * @return array List of children marked by this operation
+     */
+    protected function markChildren(DataObject $node)
+    {
+        $this->markExpanded($node);
+
+        // If too many children leave closed
+        if ($this->isNodeLimited($node)) {
+            // Limited nodes are always expanded
+            $this->markClosed($node);
+            return [];
+        }
+
+        // Iterate children if not limited
+        $children = $this->getChildren($node);
+        if (!$children) {
+            return [];
+        }
+
+        // Mark all children
+        $markedChildren = [];
+        foreach ($children as $child) {
+            $markingMatches = $this->markingFilterMatches($child);
+            if (!$markingMatches) {
+                continue;
+            }
+            // Mark a child node as unexpanded if it has children and has not already been expanded
+            if ($this->getNumChildren($child) > 0 && !$this->isExpanded($child)) {
+                $this->markUnexpanded($child);
+            } else {
+                $this->markExpanded($child);
+            }
+
+            $markedChildren[] = $child;
+        }
+        return $markedChildren;
+    }
+
+    /**
+     * Return CSS classes of 'unexpanded', 'closed', both, or neither, as well as a 'jstree-*' state depending on the
+     * marking of this DataObject.
+     *
+     * @param DataObject $node
+     * @return string
+     */
+    protected function markingClasses($node)
+    {
+        $classes = [];
+        if (!$this->isExpanded($node)) {
+            $classes[] = 'unexpanded';
+        }
+
+        // Set jstree open state, or mark it as a leaf (closed) if there are no children
+        if (!$this->getNumChildren($node)) {
+            // No children
+            $classes[] = "jstree-leaf closed";
+        } elseif ($this->isTreeOpened($node)) {
+            // Open with children
+            $classes[] = "jstree-open";
+        } else {
+            // Closed with children
+            $classes[] = "jstree-closed closed";
+        }
+        return implode(' ', $classes);
+    }
+
+    /**
+     * Mark the children of the DataObject with the given ID.
+     *
+     * @param int  $id   ID of parent node
+     * @param bool $open If this is true, mark the parent node as opened
+     * @return bool
+     */
+    public function markById($id, $open = false)
+    {
+        if (isset($this->markedNodes[$id])) {
+            $this->markChildren($this->markedNodes[$id]);
+            if ($open) {
+                $this->markOpened($this->markedNodes[$id]);
+            }
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    /**
+     * Expose the given object in the tree, by marking this page and all it ancestors.
+     *
+     * @param DataObject|Hierarchy $childObj
+     * @return $this
+     */
+    public function markToExpose(DataObject $childObj)
+    {
+        if (!$childObj) {
+            return $this;
+        }
+        $stack = $childObj->getAncestors(true)->reverse();
+        foreach ($stack as $stackItem) {
+            $this->markById($stackItem->ID, true);
+        }
+        return $this;
+    }
+
+    /**
+     * Return the IDs of all the marked nodes.
+     *
+     * @refactor called from CMSMain
+     * @return array
+     */
+    public function markedNodeIDs()
+    {
+        return array_keys($this->markedNodes);
+    }
+
+    /**
+     * Cache of DataObjects' expanded statuses: [ClassName][ID] = bool
+     * @var array
+     */
+    protected $expanded = [];
+
+    /**
+     * Cache of DataObjects' opened statuses: [ID] = bool
+     * @var array
+     */
+    protected $treeOpened = [];
+
+    /**
+     * Reset marked nodes
+     */
+    public function clearMarks()
+    {
+        $this->markedNodes = [];
+        $this->expanded = [];
+        $this->treeOpened = [];
+    }
+
+    /**
+     * Mark this DataObject as expanded.
+     *
+     * @param DataObject $node
+     * @return $this
+     */
+    public function markExpanded(DataObject $node)
+    {
+        $id = $node->ID ?: 0;
+        $this->markedNodes[$id] = $node;
+        $this->expanded[$id] = true;
+        return $this;
+    }
+
+    /**
+     * Mark this DataObject as unexpanded.
+     *
+     * @param DataObject $node
+     * @return $this
+     */
+    public function markUnexpanded(DataObject $node)
+    {
+        $id = $node->ID ?: 0;
+        $this->markedNodes[$id] = $node;
+        unset($this->expanded[$id]);
+        return $this;
+    }
+
+    /**
+     * Mark this DataObject's tree as opened.
+     *
+     * @param DataObject $node
+     * @return $this
+     */
+    public function markOpened(DataObject $node)
+    {
+        $id = $node->ID ?: 0;
+        $this->markedNodes[$id] = $node;
+        $this->treeOpened[$id] = true;
+        return $this;
+    }
+
+    /**
+     * Mark this DataObject's tree as closed.
+     *
+     * @param DataObject $node
+     * @return $this
+     */
+    public function markClosed(DataObject $node)
+    {
+        $id = $node->ID ?: 0;
+        $this->markedNodes[$id] = $node;
+        unset($this->treeOpened[$id]);
+        return $this;
+    }
+
+    /**
+     * Check if this DataObject is marked.
+     *
+     * @param DataObject $node
+     * @return bool
+     */
+    public function isMarked(DataObject $node)
+    {
+        $id = $node->ID ?: 0;
+        return !empty($this->markedNodes[$id]);
+    }
+
+    /**
+     * Check if this DataObject is expanded.
+     * An expanded object has had it's children iterated through.
+     *
+     * @param DataObject $node
+     * @return bool
+     */
+    public function isExpanded(DataObject $node)
+    {
+        $id = $node->ID ?: 0;
+        return !empty($this->expanded[$id]);
+    }
+
+    /**
+     * Check if this DataObject's tree is opened.
+     * This is an expanded node which also should have children visually shown.
+     *
+     * @param DataObject $node
+     * @return bool
+     */
+    public function isTreeOpened(DataObject $node)
+    {
+        $id = $node->ID ?: 0;
+        return !empty($this->treeOpened[$id]);
+    }
+
+    /**
+     * Check if this node has too many children
+     *
+     * @param DataObject|Hierarchy $node
+     * @param int $count Children count (if already calculated)
+     * @return bool
+     */
+    protected function isNodeLimited(DataObject $node, $count = null)
+    {
+        // Singleton root node isn't limited
+        if (!$node->ID) {
+            return false;
+        }
+
+        // Check if limiting is enabled first
+        if (!$this->getLimitingEnabled()) {
+            return false;
+        }
+
+        // Count children for this node and compare to max
+        if (!isset($count)) {
+            $count = $this->getNumChildren($node);
+        }
+        return $count > $this->getMaxChildNodes();
+    }
+
+    /**
+     * Toggle limiting on or off
+     *
+     * @param bool $enabled
+     * @return $this
+     */
+    public function setLimitingEnabled($enabled)
+    {
+        $this->enableLimiting = $enabled;
+        return $this;
+    }
+
+    /**
+     * Check if limiting is enabled
+     *
+     * @return bool
+     */
+    public function getLimitingEnabled()
+    {
+        return $this->enableLimiting;
+    }
+}

templates/SilverStripe/Forms/Includes/TreeDropdownField_HTML.ss

@@ -0,0 +1,28 @@
+<% if $depth == '0' && not $isSubTree %>
+    <ul class="tree">
+<% else_if $depth > 0 %>
+    <% if $limited || $children %>
+        <ul>
+    <% end_if %>
+<% end_if %>
+
+<% if $limited %>
+    <li><%t SilverStripe\\ORM\\Hierarchy.LIMITED_TITLE 'Too many children ({count}}' count=$count %></li>
+<% else_if $children %>
+    <% loop $children %>
+        <li id="selector-{$name}-{$id}" data-id="{$id}"
+            class="class-{$node.ClassName} {$markingClasses} <% if $disabled %>disabled<% end_if %>"
+        >
+            <a rel="$node.ID">{$title}</a>
+            $SubTree
+        </li>
+    <% end_loop %>
+<% end_if %>
+
+<% if $depth == '0' && not $isSubTree %>
+    </ul>
+<% else_if $depth > 0 %>
+    <% if $limited || $children %>
+        </ul>
+    <% end_if %>
+<% end_if %>

templates/SilverStripe/ORM/Hierarchy/Includes/MarkedSet_HTML.ss

@@ -0,0 +1,9 @@
+<% if $children || $limited %>
+<ul>
+    <% if $limited %>
+        <li><%t SilverStripe\\ORM\\Hierarchy.LIMITED_TITLE 'Too many children ({count}}' count=$count %></li>
+    <% else_if $children %>
+        <% loop $children %><li>$node.Title.XML $SubTree</li><% end_loop %>
+    <% end_if %>
+</ul>
+<% end_if %>

tests/php/Forms/TreeDropdownFieldTest.php

@@ -21,7 +21,8 @@ class TreeDropdownFieldTest extends SapphireTest
 
         // case insensitive search against keyword 'sub' for folders
         $request = new HTTPRequest('GET', 'url', array('search'=>'sub'));
-        $tree = $field->tree($request);
+        $response = $field->tree($request);
+        $tree = $response->getBody();
 
         $folder1 = $this->objFromFixture(Folder::class, 'folder1');
         $folder1Subfolder1 = $this->objFromFixture(Folder::class, 'folder1-subfolder1');
@@ -57,7 +58,8 @@ class TreeDropdownFieldTest extends SapphireTest
 
         // case insensitive search against keyword 'sub' for files
         $request = new HTTPRequest('GET', 'url', array('search'=>'sub'));
-        $tree = $field->tree($request);
+        $response = $field->tree($request);
+        $tree = $response->getBody();
 
         $parser = new CSSContentParser($tree);
 

tests/php/ORM/HierarchyTest.php

@@ -4,8 +4,6 @@ namespace SilverStripe\ORM\Tests;
 
 use SilverStripe\ORM\ValidationException;
 use SilverStripe\Versioned\Versioned;
-use SilverStripe\ORM\DataObject;
-use SilverStripe\Dev\CSSContentParser;
 use SilverStripe\Dev\SapphireTest;
 
 class HierarchyTest extends SapphireTest
@@ -42,11 +40,13 @@ class HierarchyTest extends SapphireTest
      */
     public function testPreventLoop()
     {
-        $this->setExpectedException(
+        $this->expectException(ValidationException::class);
-            ValidationException::class,
+        $this->expectExceptionMessage(sprintf(
-            sprintf('Infinite loop found within the "%s" hierarchy', HierarchyTest\TestObject::class)
+            'Infinite loop found within the "%s" hierarchy',
-        );
+            HierarchyTest\TestObject::class
+        ));
 
+        /** @var HierarchyTest\TestObject $obj2 */
         $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
         $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
 
@@ -67,13 +67,14 @@ class HierarchyTest extends SapphireTest
         // Check that obj1-3 appear at the top level of the AllHistoricalChildren tree
         $this->assertEquals(
             array("Obj 1", "Obj 2", "Obj 3"),
-            singleton(HierarchyTest\TestObject::class)->AllHistoricalChildren()->column('Title')
+            HierarchyTest\TestObject::singleton()->AllHistoricalChildren()->column('Title')
         );
 
         // Check numHistoricalChildren
-        $this->assertEquals(3, singleton(HierarchyTest\TestObject::class)->numHistoricalChildren());
+        $this->assertEquals(3, HierarchyTest\TestObject::singleton()->numHistoricalChildren());
 
         // Check that both obj 2 children are returned
+        /** @var HierarchyTest\TestObject $obj2 */
         $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
         $this->assertEquals(
             array("Obj 2a", "Obj 2b"),
@@ -85,7 +86,11 @@ class HierarchyTest extends SapphireTest
 
 
         // Obj 3 has been deleted; let's bring it back from the grave
-        $obj3 = Versioned::get_including_deleted(HierarchyTest\TestObject::class, "\"Title\" = 'Obj 3'")->First();
+        /** @var HierarchyTest\TestObject $obj3 */
+        $obj3 = Versioned::get_including_deleted(
+            HierarchyTest\TestObject::class,
+            "\"Title\" = 'Obj 3'"
+        )->First();
 
         // Check that all obj 3 children are returned
         $this->assertEquals(
@@ -97,46 +102,30 @@ class HierarchyTest extends SapphireTest
         $this->assertEquals(4, $obj3->numHistoricalChildren());
     }
 
-    /**
-     * Test that you can call Hierarchy::markExpanded/Unexpanded/Open() on a obj, and that
-     * calling Hierarchy::isMarked() on a different instance of that object will return true.
-     */
-    public function testItemMarkingIsntRestrictedToSpecificInstance()
-    {
-        // Mark a few objs
-        $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2')->markExpanded();
-        $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a')->markExpanded();
-        $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b')->markExpanded();
-        $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3')->markUnexpanded();
-
-        // Query some objs in a different context and check their m
-        $objs = DataObject::get(HierarchyTest\TestObject::class, '', '"ID" ASC');
-        $marked = $expanded = array();
-        foreach ($objs as $obj) {
-            if ($obj->isMarked()) {
-                $marked[] = $obj->Title;
-            }
-            if ($obj->isExpanded()) {
-                $expanded[] = $obj->Title;
-            }
-        }
-
-        $this->assertEquals(array('Obj 2', 'Obj 3', 'Obj 2a', 'Obj 2b'), $marked);
-        $this->assertEquals(array('Obj 2', 'Obj 2a', 'Obj 2b'), $expanded);
-    }
-
     public function testNumChildren()
     {
-        $this->assertEquals($this->objFromFixture(HierarchyTest\TestObject::class, 'obj1')->numChildren(), 0);
+        /** @var HierarchyTest\TestObject $obj1 */
-        $this->assertEquals($this->objFromFixture(HierarchyTest\TestObject::class, 'obj2')->numChildren(), 2);
-        $this->assertEquals($this->objFromFixture(HierarchyTest\TestObject::class, 'obj3')->numChildren(), 4);
-        $this->assertEquals($this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a')->numChildren(), 2);
-        $this->assertEquals($this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b')->numChildren(), 0);
-        $this->assertEquals($this->objFromFixture(HierarchyTest\TestObject::class, 'obj3a')->numChildren(), 2);
-        $this->assertEquals($this->objFromFixture(HierarchyTest\TestObject::class, 'obj3d')->numChildren(), 0);
-
         $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
-        $this->assertEquals($obj1->numChildren(), 0);
+        /** @var HierarchyTest\TestObject $obj2 */
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        /** @var HierarchyTest\TestObject $obj3 */
+        $obj3 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3');
+        /** @var HierarchyTest\TestObject $obj2a */
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        /** @var HierarchyTest\TestObject $obj2b */
+        $obj2b = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b');
+        /** @var HierarchyTest\TestObject $obj3a */
+        $obj3a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3a');
+        /** @var HierarchyTest\TestObject $obj3b */
+        $obj3b = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3d');
+
+        $this->assertEquals(0, $obj1->numChildren());
+        $this->assertEquals(2, $obj2->numChildren());
+        $this->assertEquals(4, $obj3->numChildren());
+        $this->assertEquals(2, $obj2a->numChildren());
+        $this->assertEquals(0, $obj2b->numChildren());
+        $this->assertEquals(2, $obj3a->numChildren());
+        $this->assertEquals(0, $obj3b->numChildren());
         $obj1Child1 = new HierarchyTest\TestObject();
         $obj1Child1->ParentID = $obj1->ID;
         $obj1Child1->write();
@@ -158,7 +147,9 @@ class HierarchyTest extends SapphireTest
 
     public function testLoadDescendantIDListIntoArray()
     {
+        /** @var HierarchyTest\TestObject $obj2 */
         $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        /** @var HierarchyTest\TestObject $obj2a */
         $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
         $obj2b = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b');
         $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
@@ -184,9 +175,13 @@ class HierarchyTest extends SapphireTest
      */
     public function testLiveChildrenOnlyDeletedFromStage()
     {
+        /** @var HierarchyTest\TestObject $obj1 */
         $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
+        /** @var HierarchyTest\TestObject $obj2 */
         $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        /** @var HierarchyTest\TestObject $obj2a */
         $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        /** @var HierarchyTest\TestObject $obj2b */
         $obj2b = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b');
 
         // Get a published set of objects for our fixture
@@ -212,9 +207,11 @@ class HierarchyTest extends SapphireTest
 
     public function testBreadcrumbs()
     {
+        /** @var HierarchyTest\TestObject $obj1 */
         $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        /** @var HierarchyTest\TestObject $obj2a */
         $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        /** @var HierarchyTest\TestObject $obj2aa */
         $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
 
         $this->assertEquals('Obj 1', $obj1->getBreadcrumbs());
@@ -222,402 +219,9 @@ class HierarchyTest extends SapphireTest
         $this->assertEquals('Obj 2 » Obj 2a » Obj 2aa', $obj2aa->getBreadcrumbs());
     }
 
-    /**
-     * @covers \SilverStripe\ORM\Hierarchy\Hierarchy::markChildren()
-     */
-    public function testMarkChildrenDoesntUnmarkPreviouslyMarked()
-    {
-        $obj3 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3');
-        $obj3aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3aa');
-        $obj3ba = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3ba');
-        $obj3ca = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3ca');
-
-        $obj3->markPartialTree();
-        $obj3->markToExpose($obj3aa);
-        $obj3->markToExpose($obj3ba);
-        $obj3->markToExpose($obj3ca);
-
-        $expected = <<<EOT
-<ul>
-<li>Obj 3a
-<ul>
-<li>Obj 3aa
-</li>
-<li>Obj 3ab
-</li>
-</ul>
-</li>
-<li>Obj 3b
-<ul>
-<li>Obj 3ba
-</li>
-<li>Obj 3bb
-</li>
-</ul>
-</li>
-<li>Obj 3c
-<ul>
-<li>Obj 3c
-</li>
-</ul>
-</li>
-<li>Obj 3d
-</li>
-</ul>
-
-EOT;
-
-        $this->assertSame($expected, $obj3->getChildrenAsUL());
-    }
-
-    public function testGetChildrenAsUL()
-    {
-        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
-
-        $nodeCountThreshold = 30;
-
-        $root = new HierarchyTest\TestObject();
-        $root->markPartialTree($nodeCountThreshold);
-        $html = $root->getChildrenAsUL(
-            "",
-            '"<li id=\"" . $child->ID . "\">" . $child->Title',
-            null,
-            false,
-            "AllChildrenIncludingDeleted",
-            "numChildren",
-            true,  // rootCall
-            $nodeCountThreshold
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2),
-            'Contains root elements'
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2, $obj2a),
-            'Contains child elements (in correct nesting)'
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2, $obj2a, $obj2aa),
-            'Contains grandchild elements (in correct nesting)'
-        );
-    }
-
-    public function testGetChildrenAsULMinNodeCount()
-    {
-        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-
-        // Set low enough that it should be fulfilled by root only elements
-        $nodeCountThreshold = 3;
-
-        $root = new HierarchyTest\TestObject();
-        $root->markPartialTree($nodeCountThreshold);
-        $html = $root->getChildrenAsUL(
-            "",
-            '"<li id=\"" . $child->ID . "\">" . $child->Title',
-            null,
-            false,
-            "AllChildrenIncludingDeleted",
-            "numChildren",
-            true,
-            $nodeCountThreshold
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj1),
-            'Contains root elements'
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2),
-            'Contains root elements'
-        );
-        $this->assertTreeNotContains(
-            $html,
-            array($obj2, $obj2a),
-            'Does not contains child elements because they exceed minNodeCount'
-        );
-    }
-
-    public function testGetChildrenAsULMinNodeCountWithMarkToExpose()
-    {
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
-
-        // Set low enough that it should be fulfilled by root only elements
-        $nodeCountThreshold = 3;
-
-        $root = new HierarchyTest\TestObject();
-        $root->markPartialTree($nodeCountThreshold);
-
-        // Mark certain node which should be included regardless of minNodeCount restrictions
-        $root->markToExpose($obj2aa);
-
-        $html = $root->getChildrenAsUL(
-            "",
-            '"<li id=\"" . $child->ID . "\">" . $child->Title',
-            null,
-            false,
-            "AllChildrenIncludingDeleted",
-            "numChildren",
-            true,
-            $nodeCountThreshold
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2),
-            'Contains root elements'
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2, $obj2a, $obj2aa),
-            'Does contain marked children nodes regardless of configured threshold'
-        );
-    }
-
-    public function testGetChildrenAsULMinNodeCountWithFilters()
-    {
-        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
-
-        // Set low enough that it should fit all search matches without lazy loading
-        $nodeCountThreshold = 3;
-
-        $root = new HierarchyTest\TestObject();
-
-        // Includes nodes by filter regardless of minNodeCount restrictions
-        $root->setMarkingFilterFunction(
-            function ($record) use ($obj2, $obj2a, $obj2aa) {
-                // Results need to include parent hierarchy, even if we just want to
-                // match the innermost node.
-                return in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID));
-            }
-        );
-        $root->markPartialTree($nodeCountThreshold);
-
-        $html = $root->getChildrenAsUL(
-            "",
-            '"<li id=\"" . $child->ID . "\">" . $child->Title',
-            null,
-            true, // limit to marked
-            "AllChildrenIncludingDeleted",
-            "numChildren",
-            true,
-            $nodeCountThreshold
-        );
-        $this->assertTreeNotContains(
-            $html,
-            array($obj1),
-            'Does not contain root elements which dont match the filter'
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2, $obj2a, $obj2aa),
-            'Contains non-root elements which match the filter'
-        );
-    }
-
-    public function testGetChildrenAsULHardLimitsNodes()
-    {
-        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
-
-        // Set low enough that it should fit all search matches without lazy loading
-        $nodeCountThreshold = 3;
-
-        $root = new HierarchyTest\TestObject();
-
-        // Includes nodes by filter regardless of minNodeCount restrictions
-        $root->setMarkingFilterFunction(
-            function ($record) use ($obj2, $obj2a, $obj2aa) {
-                // Results need to include parent hierarchy, even if we just want to
-                // match the innermost node.
-                return in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID));
-            }
-        );
-        $root->markPartialTree($nodeCountThreshold);
-
-        $html = $root->getChildrenAsUL(
-            "",
-            '"<li id=\"" . $child->ID . "\">" . $child->Title',
-            null,
-            true, // limit to marked
-            "AllChildrenIncludingDeleted",
-            "numChildren",
-            true,
-            $nodeCountThreshold
-        );
-        $this->assertTreeNotContains(
-            $html,
-            array($obj1),
-            'Does not contain root elements which dont match the filter'
-        );
-        $this->assertTreeContains(
-            $html,
-            array($obj2, $obj2a, $obj2aa),
-            'Contains non-root elements which match the filter'
-        );
-    }
-
-    public function testGetChildrenAsULNodeThresholdLeaf()
-    {
-        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-        $obj3 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3');
-        $obj3a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3a');
-
-        $nodeCountThreshold = 99;
-
-        $root = new HierarchyTest\TestObject();
-        $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'
-        );
-    }
-
-    /**
-     * This test checks that deleted ('archived') child pages don't set a css class on the parent
-     * node that makes it look like it has children
-     */
-    public function testGetChildrenAsULNodeDeletedOnLive()
-    {
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
-        $obj2ab = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b');
-
-        // delete all children under obj2
-        $obj2a->delete();
-        $obj2aa->delete();
-        $obj2ab->delete();
-        // Don't pre-load all children
-        $nodeCountThreshold = 1;
-
-        $childrenMethod = 'AllChildren';
-        $numChildrenMethod = 'numChildren';
-
-        $root = new HierarchyTest\TestObject();
-        $root->markPartialTree($nodeCountThreshold, null, $childrenMethod, $numChildrenMethod);
-
-        // As in LeftAndMain::getSiteTreeFor() but simpler and more to the point for testing purposes
-        $titleFn = function (&$child, $numChildrenMethod = "") {
-            return '<li class="' . $child->markingClasses($numChildrenMethod).
-                '" id="' . $child->ID . '">"' . $child->Title;
-        };
-
-        $html = $root->getChildrenAsUL(
-            "",
-            $titleFn,
-            null,
-            true, // limit to marked
-            $childrenMethod,
-            $numChildrenMethod,
-            true,
-            $nodeCountThreshold
-        );
-
-        // Get the class attribute from the $obj2 node in the sitetree, class 'jstree-leaf' means it's a leaf node
-        $nodeClass = $this->getNodeClassFromTree($html, $obj2);
-        $this->assertEquals('jstree-leaf closed', $nodeClass, 'object2 should not have children in the sitetree');
-    }
-
-    /**
-     * This test checks that deleted ('archived') child pages _do_ set a css class on the parent
-     * node that makes it look like it has children when getting all children including deleted
-     */
-    public function testGetChildrenAsULNodeDeletedOnStage()
-    {
-        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
-        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
-        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
-        $obj2ab = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b');
-
-        // delete all children under obj2
-        $obj2a->delete();
-        $obj2aa->delete();
-        $obj2ab->delete();
-        // Don't pre-load all children
-        $nodeCountThreshold = 1;
-
-        $childrenMethod = 'AllChildrenIncludingDeleted';
-        $numChildrenMethod = 'numHistoricalChildren';
-
-        $root = new HierarchyTest\TestObject();
-        $root->markPartialTree($nodeCountThreshold, null, $childrenMethod, $numChildrenMethod);
-
-        // As in LeftAndMain::getSiteTreeFor() but simpler and more to the point for testing purposes
-        $titleFn = function (&$child, $numChildrenMethod = "") {
-            return '<li class="' . $child->markingClasses($numChildrenMethod).
-                '" id="' . $child->ID . '">"' . $child->Title;
-        };
-
-        $html = $root->getChildrenAsUL(
-            "",
-            $titleFn,
-            null,
-            true, // limit to marked
-            $childrenMethod,
-            $numChildrenMethod,
-            true,
-            $nodeCountThreshold
-        );
-
-        // Get the class attribute from the $obj2 node in the sitetree
-        $nodeClass = $this->getNodeClassFromTree($html, $obj2);
-        // Object2 can now be expanded
-        $this->assertEquals('unexpanded jstree-closed closed', $nodeClass, 'obj2 should have children in the sitetree');
-    }
-
     public function testNoHideFromHeirarchy()
     {
+        /** @var HierarchyTest\HideTestObject $obj4 */
         $obj4 = $this->objFromFixture(HierarchyTest\HideTestObject::class, 'obj4');
         $obj4->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
 
@@ -632,10 +236,9 @@ EOT;
     {
         HierarchyTest\HideTestObject::config()->update(
             'hide_from_hierarchy',
-            [
+            [ HierarchyTest\HideTestSubObject::class ]
-            HierarchyTest\HideTestSubObject::class,
-            ]
         );
+        /** @var HierarchyTest\HideTestObject $obj4 */
         $obj4 = $this->objFromFixture(HierarchyTest\HideTestObject::class, 'obj4');
         $obj4->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
 
@@ -645,63 +248,11 @@ EOT;
             ->filter('ParentID', (int)$obj4->ID)
             ->exclude('ID', (int)$obj4->ID);
 
+        /** @var HierarchyTest\HideTestObject $child */
         foreach ($children as $child) {
             $child->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
         }
         $this->assertEquals($obj4->stageChildren()->Count(), 1);
         $this->assertEquals($obj4->liveChildren()->Count(), 1);
     }
-
-    /**
-     * @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);
-    }
-
-    /**
-     * Get the HTML class attribute from a node in the sitetree
-     *
-     * @param  $html
-     * @param  $node
-     * @return string
-     */
-    protected function getNodeClassFromTree($html, $node)
-    {
-        $parser = new CSSContentParser($html);
-        $xpath = '//ul/li[@id="' . $node->ID . '"]';
-        $object = $parser->getByXpath($xpath);
-
-        foreach ($object[0]->attributes() as $key => $attr) {
-            if ($key == 'class') {
-                return (string)$attr;
-            }
-        }
-        return '';
-    }
 }

tests/php/ORM/HierarchyTest/MarkedSetTest_HTML.ss

@@ -0,0 +1,9 @@
+<% if $children || $limited %>
+<ul>
+    <% if $limited %>
+        <li data-id="{$node.ID}"><span class="exceeded">Exceeded!</span></li>
+    <% else_if $children %>
+        <% loop $children %><li data-id="{$node.ID}" class="$markingClasses">$node.Title.XML $SubTree</li><% end_loop %>
+    <% end_if %>
+</ul>
+<% end_if %>

tests/php/ORM/MarkedSetTest.php

@@ -0,0 +1,444 @@
+<?php
+
+namespace SilverStripe\ORM\Tests;
+
+use DOMDocument;
+use SilverStripe\Dev\CSSContentParser;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\Hierarchy\MarkedSet;
+use SilverStripe\Versioned\Versioned;
+
+/**
+ * Test set of marked Hierarchy-extended DataObjects
+ */
+class MarkedSetTest extends SapphireTest
+{
+    protected static $fixture_file = 'HierarchyTest.yml';
+
+    protected static $extra_dataobjects = array(
+        HierarchyTest\TestObject::class,
+        HierarchyTest\HideTestObject::class,
+        HierarchyTest\HideTestSubObject::class,
+    );
+
+    protected static function getExtraDataObjects()
+    {
+        // Prevent setup breaking if versioned module absent
+        if (class_exists(Versioned::class)) {
+            return parent::getExtraDataObjects();
+        }
+        return [];
+    }
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        // Note: Soft support for versioned module optionality
+        if (!class_exists(Versioned::class)) {
+            $this->markTestSkipped('MarkedSetTest requires the Versioned extension');
+        }
+    }
+
+
+    /**
+     * Test that you can call MarkedSet::markExpanded/Unexpanded/Open() on a obj, and that
+     * calling MarkedSet::isMarked() on a different instance of that object will return true.
+     */
+    public function testItemMarkingIsntRestrictedToSpecificInstance()
+    {
+        // Build new object
+        $set = new MarkedSet(HierarchyTest\TestObject::singleton());
+
+        // Mark a few objs
+        $set->markExpanded($this->objFromFixture(HierarchyTest\TestObject::class, 'obj2'));
+        $set->markExpanded($this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a'));
+        $set->markExpanded($this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b'));
+        $set->markUnexpanded($this->objFromFixture(HierarchyTest\TestObject::class, 'obj3'));
+
+        // Query some objs in a different context and check their m
+        $objs = DataObject::get(HierarchyTest\TestObject::class, '', '"ID" ASC');
+        $marked = $expanded = array();
+        foreach ($objs as $obj) {
+            if ($set->isMarked($obj)) {
+                $marked[] = $obj->Title;
+            }
+            if ($set->isExpanded($obj)) {
+                $expanded[] = $obj->Title;
+            }
+        }
+        $this->assertEquals(array('Obj 2', 'Obj 3', 'Obj 2a', 'Obj 2b'), $marked);
+        $this->assertEquals(array('Obj 2', 'Obj 2a', 'Obj 2b'), $expanded);
+    }
+
+    /**
+     * @covers \SilverStripe\ORM\Hierarchy\MarkedSet::markChildren()
+     */
+    public function testMarkChildrenDoesntUnmarkPreviouslyMarked()
+    {
+        $obj3 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3');
+        $obj3aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3aa');
+        $obj3ba = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3ba');
+        $obj3ca = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3ca');
+
+        $set = new MarkedSet($obj3);
+        $set->markPartialTree();
+        $set->markToExpose($obj3aa);
+        $set->markToExpose($obj3ba);
+        $set->markToExpose($obj3ca);
+
+        $expected = <<<EOT
+<ul>
+    <li>Obj 3a
+        <ul>
+            <li>Obj 3aa
+            </li>
+            <li>Obj 3ab
+            </li>
+        </ul>
+    </li>
+    <li>Obj 3b
+        <ul>
+            <li>Obj 3ba
+            </li>
+            <li>Obj 3bb
+            </li>
+        </ul>
+    </li>
+    <li>Obj 3c
+        <ul>
+            <li>Obj 3c
+            </li>
+        </ul>
+    </li>
+    <li>Obj 3d
+    </li>
+</ul>
+
+EOT;
+
+        $this->assertHTMLSame($expected, $set->renderChildren());
+    }
+
+    public function testGetChildrenCustomTemplate()
+    {
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
+
+        // Render marked tree
+        $set = new MarkedSet(HierarchyTest\TestObject::singleton(), 'AllChildrenIncludingDeleted', 'numChildren', 30);
+        $set->markPartialTree();
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        $this->assertTreeContains(
+            $html,
+            array($obj2),
+            'Contains root elements'
+        );
+        $this->assertTreeContains(
+            $html,
+            array($obj2, $obj2a),
+            'Contains child elements (in correct nesting)'
+        );
+        $this->assertTreeContains(
+            $html,
+            array($obj2, $obj2a, $obj2aa),
+            'Contains grandchild elements (in correct nesting)'
+        );
+    }
+
+    public function testGetChildrenAsULMinNodeCount()
+    {
+        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+
+        // Render marked tree
+        $set = new MarkedSet(HierarchyTest\TestObject::singleton(), 'AllChildrenIncludingDeleted', 'numChildren');
+        $set->setNodeCountThreshold(3); // Set low enough that it should be fulfilled by root only elements
+        $set->markPartialTree();
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        $this->assertTreeContains(
+            $html,
+            array($obj1),
+            'Contains root elements'
+        );
+        $this->assertTreeContains(
+            $html,
+            array($obj2),
+            'Contains root elements'
+        );
+        $this->assertTreeNotContains(
+            $html,
+            array($obj2, $obj2a),
+            'Does not contains child elements because they exceed minNodeCount'
+        );
+    }
+
+    public function testGetChildrenAsULMinNodeCountWithMarkToExpose()
+    {
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
+
+        // Render marked tree
+        $set = new MarkedSet(HierarchyTest\TestObject::singleton(), 'AllChildrenIncludingDeleted', 'numChildren');
+        $set->setNodeCountThreshold(3); // Set low enough that it should be fulfilled by root only elements
+        $set->markPartialTree();
+        // Mark certain node which should be included regardless of minNodeCount restrictions
+        $set->markToExpose($obj2aa);
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        $this->assertTreeContains(
+            $html,
+            array($obj2),
+            'Contains root elements'
+        );
+        $this->assertTreeContains(
+            $html,
+            array($obj2, $obj2a, $obj2aa),
+            'Does contain marked children nodes regardless of configured threshold'
+        );
+    }
+
+    public function testGetChildrenAsULMinNodeCountWithFilters()
+    {
+        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
+
+        // Render marked tree
+        $set = new MarkedSet(HierarchyTest\TestObject::singleton(), 'AllChildrenIncludingDeleted', 'numChildren');
+        $set->setNodeCountThreshold(3); // Set low enough that it should be fulfilled by root only elements
+        // Includes nodes by filter regardless of minNodeCount restrictions
+        $set->setMarkingFilterFunction(
+            function ($record) use ($obj2, $obj2a, $obj2aa) {
+                // Results need to include parent hierarchy, even if we just want to
+                // match the innermost node.
+                return in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID));
+            }
+        );
+        $set->markPartialTree();
+        // Mark certain node which should be included regardless of minNodeCount restrictions
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        $this->assertTreeNotContains(
+            $html,
+            array($obj1),
+            'Does not contain root elements which dont match the filter'
+        );
+        $this->assertTreeContains(
+            $html,
+            array($obj2, $obj2a, $obj2aa),
+            'Contains non-root elements which match the filter'
+        );
+    }
+
+    public function testGetChildrenAsULHardLimitsNodes()
+    {
+        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
+
+        // Render marked tree
+        $set = new MarkedSet(HierarchyTest\TestObject::singleton(), 'AllChildrenIncludingDeleted', 'numChildren');
+        $set->setNodeCountThreshold(3); // Set low enough that it should miss out one node
+
+        // Includes nodes by filter regardless of minNodeCount restrictions
+        $set->setMarkingFilterFunction(
+            function ($record) use ($obj2, $obj2a, $obj2aa) {
+                // Results need to include parent hierarchy, even if we just want to
+                // match the innermost node.
+                return in_array($record->ID, array($obj2->ID, $obj2a->ID, $obj2aa->ID));
+            }
+        );
+        $set->markPartialTree();
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        $this->assertTreeNotContains(
+            $html,
+            array($obj1, $obj2aa),
+            'Does not contain root elements which dont match the filter or are limited'
+        );
+        $this->assertTreeContains(
+            $html,
+            array($obj2, $obj2a),
+            'Contains non-root elements which match the filter'
+        );
+    }
+
+    public function testGetChildrenAsULNodeThresholdLeaf()
+    {
+        $obj1 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj1');
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        $obj3 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3');
+        $obj3a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj3a');
+
+        // Render marked tree
+        $set = new MarkedSet(HierarchyTest\TestObject::singleton(), 'AllChildrenIncludingDeleted', 'numChildren');
+        $set->setNodeCountThreshold(99);
+        $set->setMaxChildNodes(2); // Force certain children to exceed limits
+        $set->markPartialTree();
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        $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'
+        );
+    }
+
+    /**
+     * This test checks that deleted ('archived') child pages don't set a css class on the parent
+     * node that makes it look like it has children
+     */
+    public function testGetChildrenAsULNodeDeletedOnLive()
+    {
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
+        $obj2ab = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b');
+
+        // delete all children under obj2
+        $obj2a->delete();
+        $obj2aa->delete();
+        $obj2ab->delete();
+
+        $set = new MarkedSet(
+            HierarchyTest\TestObject::singleton(),
+            'AllChildren',
+            'numChildren'
+        );
+        // Don't pre-load all children
+        $set->setNodeCountThreshold(1);
+        $set->markPartialTree();
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        // Get the class attribute from the $obj2 node in the sitetree, class 'jstree-leaf' means it's a leaf node
+        $nodeClass = $this->getNodeClassFromTree($html, $obj2);
+        $this->assertEquals('jstree-leaf closed', $nodeClass, 'object2 should not have children in the sitetree');
+    }
+
+    /**
+     * This test checks that deleted ('archived') child pages _do_ set a css class on the parent
+     * node that makes it look like it has children when getting all children including deleted
+     */
+    public function testGetChildrenAsULNodeDeletedOnStage()
+    {
+        $obj2 = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2');
+        $obj2a = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2a');
+        $obj2aa = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2aa');
+        $obj2ab = $this->objFromFixture(HierarchyTest\TestObject::class, 'obj2b');
+
+        // delete all children under obj2
+        $obj2a->delete();
+        $obj2aa->delete();
+        $obj2ab->delete();
+
+        $set = new MarkedSet(
+            HierarchyTest\TestObject::singleton(),
+            'AllChildrenIncludingDeleted',
+            'numHistoricalChildren'
+        );
+        // Don't pre-load all children
+        $set->setNodeCountThreshold(1);
+        $set->markPartialTree();
+        $template = __DIR__ . '/HierarchyTest/MarkedSetTest_HTML.ss';
+        $html = $set->renderChildren($template);
+
+        // Get the class attribute from the $obj2 node in the sitetree
+        $nodeClass = $this->getNodeClassFromTree($html, $obj2);
+        // Object2 can now be expanded
+        $this->assertEquals('unexpanded jstree-closed closed', $nodeClass, 'obj2 should have children in the sitetree');
+    }
+
+
+    /**
+     * @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[@data-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[@data-id="' . $node->ID . '"]';
+        }
+        $match = $parser->getByXpath($xpath);
+        self::assertThat((bool)$match, self::isFalse(), $message);
+    }
+
+    /**
+     * Get the HTML class attribute from a node in the sitetree
+     *
+     * @param string$html
+     * @param DataObject $node
+     * @return string
+     */
+    protected function getNodeClassFromTree($html, $node)
+    {
+        $parser = new CSSContentParser($html);
+        $xpath = '//ul/li[@data-id="' . $node->ID . '"]';
+        $object = $parser->getByXpath($xpath);
+
+        foreach ($object[0]->attributes() as $key => $attr) {
+            if ($key == 'class') {
+                return (string)$attr;
+            }
+        }
+        return '';
+    }
+
+    protected function assertHTMLSame($expected, $actual, $message = '')
+    {
+        // Trim each line, strip empty lines
+        $expected = implode("\n", array_filter(array_map('trim', explode("\n", $expected))));
+        $actual = implode("\n", array_filter(array_map('trim', explode("\n", $actual))));
+        $this->assertXmlStringEqualsXmlString($expected, $actual, $message);
+    }
+}