NEW: Add Hierarchy::prepopulate_numchildren_cache() (#8380)

* NEW: Add Hierarchy::prepopulate_numchildren_cache()
API: Hierarchy::stageChildren() customisations must be applied to the base class and not include record-specific behaviour.

Adds the ability to prepopulate the cache for Hierarchy::numChildren()
in a batch.

Note that this optimisation means that stageChildren() is not called on
each record in order to calculate numChildren(). This means that the
structure of the stageChildren() query must be the same for all records
and the behaviour cannot be customised only for a subclass of the base
data class. For example, apply your customisations to SiteTree and not
a subclass.

This is an useful part of optimising the query count on tree generation.
See https://github.com/silverstripe/silverstripe-framework/issues/8379

* NEW: Add Hierarchy::prepopulateTreeDataCache()

This provides a more extensible way of preopulating caches for optimised
tree generation.

Fixes https://github.com/silverstripe/silverstripe-framework/issues/8391

docs/en/04_Changelogs/4.3.0.md

@@ -4,6 +4,7 @@
 
  - `DataList::column()` now returns all values and not just "distinct" values from a column as per the API docs
  - `DataList`, `ArrayList` and `UnsavedRalationList` all have `columnUnique()` method for fetching distinct column values
+ - Take care with `stageChildren()` overrides. `Hierarchy::numChildren() ` results will only make use of `stageChildren()` customisations that are applied to the base class and don't include record-specific behaviour.
 
 ## Upgrading {#upgrading}
 

src/ORM/Hierarchy/Hierarchy.php

@@ -10,7 +10,10 @@ use SilverStripe\ORM\ValidationResult;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataExtension;
+use SilverStripe\ORM\DB;
 use SilverStripe\Versioned\Versioned;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Core\Convert;
 use Exception;
 
 /**
@@ -71,6 +74,15 @@ class Hierarchy extends DataExtension
      */
     private static $hide_from_cms_tree = array();
 
+    /**
+     * Used to enable or disable the prepopulation of the numchildren cache.
+     * Defaults to true.
+     *
+     * @config
+     * @var boolean
+     */
+    private static $prepopulate_numchildren_cache = true;
+
     /**
      * Prevent virtual page virtualising these fields
      *
@@ -79,9 +91,17 @@ class Hierarchy extends DataExtension
      */
     private static $non_virtual_fields = [
         '_cache_children',
-        '_cache_numChildren',
     ];
 
+    /**
+     * A cache used by numChildren().
+     * Clear through {@link flushCache()}.
+     * version (int)0 means not on this stage.
+     *
+     * @var array
+     */
+    protected static $cache_numChildren = [];
+
     public static function get_extra_config($class, $extension, $args)
     {
         return array(
@@ -271,11 +291,18 @@ class Hierarchy extends DataExtension
      */
     public function numChildren($cache = true)
     {
-        // Load if caching
+
+        $baseClass = $this->owner->baseClass();
+        $cacheType = 'numChildren';
+        $id = $this->owner->ID;
+
+        // cached call
         if ($cache) {
-            $numChildren = $this->owner->_cache_numChildren;
-            if (isset($numChildren)) {
-                return $numChildren;
+            if (isset(self::$cache_numChildren[$baseClass][$cacheType][$id])) {
+                return self::$cache_numChildren[$baseClass][$cacheType][$id];
+            } elseif (isset(self::$cache_numChildren[$baseClass][$cacheType]['_complete'])) {
+                // If the cache is complete and we didn't find our ID in the cache, it means this object is childless.
+                return 0;
             }
         }
 
@@ -284,11 +311,89 @@ class Hierarchy extends DataExtension
 
         // Save if caching
         if ($cache) {
-            $this->owner->_cache_numChildren = $numChildren;
+            self::$cache_numChildren[$baseClass][$cacheType][$id] = $numChildren;
         }
+
         return $numChildren;
     }
 
+    /**
+     * Pre-populate any appropriate caches prior to rendering a tree.
+     * This is used to allow for the efficient rendering of tree views, notably in the CMS.
+     * In the cace of Hierarchy, it caches numChildren values. Other extensions can provide an
+     * onPrepopulateTreeDataCache(DataList $recordList = null, array $options) methods to hook
+     * into this event as well.
+     *
+     * @param DataList|array $recordList The list of records to prepopulate caches for. Null for all records.
+     * @param array $options A map of hints about what should be cached. "numChildrenMethod" and
+     *                       "childrenMethod" are allowed keys.
+     */
+    public function prepopulateTreeDataCache($recordList = null, array $options = [])
+    {
+        if (empty($options['numChildrenMethod']) || $options['numChildrenMethod'] === 'numChildren') {
+            $idList = is_array($recordList) ? $recordList :
+                ($recordList instanceof DataList ? $recordList->column('ID') : null);
+            self::prepopulate_numchildren_cache($this->owner->baseClass(), $idList);
+        }
+
+        $this->owner->extend('onPrepopulateTreeDataCache', $recordList, $options);
+    }
+
+    /**
+     * Pre-populate the cache for Versioned::get_versionnumber_by_stage() for
+     * a list of record IDs, for more efficient database querying.  If $idList
+     * is null, then every record will be pre-cached.
+     *
+     * @param string $class
+     * @param string $stage
+     * @param array $idList
+     */
+    public static function prepopulate_numchildren_cache($baseClass, $idList = null)
+    {
+        if (!Config::inst()->get(static::class, 'prepopulate_numchildren_cache')) {
+            return;
+        }
+
+        /** @var Versioned|DataObject $singleton */
+        $dummyObject = DataObject::singleton($baseClass);
+        $baseTable = $dummyObject->baseTable();
+
+        $idColumn = Convert::symbol2sql("{$baseTable}.ID");
+
+        // Get the stageChildren() result of a dummy object and break down into a generic query
+        $query = $dummyObject->stageChildren(true, true)->dataQuery()->query();
+
+        // optional ID-list filter
+        if ($idList) {
+            // Validate the ID list
+            foreach ($idList as $id) {
+                if (!is_numeric($id)) {
+                    user_error(
+                        "Bad ID passed to Versioned::prepopulate_numchildren_cache() in \$idList: " . $id,
+                        E_USER_ERROR
+                    );
+                }
+            }
+            $query->addWhere(['"ParentID" IN (' . DB::placeholders($idList) . ')' => $idList]);
+        }
+
+        $query->setOrderBy(null);
+
+        $query->setSelect([
+            '"ParentID"',
+            "COUNT(DISTINCT $idColumn) AS \"NumChildren\"",
+        ]);
+        $query->setGroupBy([Convert::symbol2sql("ParentID")]);
+
+        $numChildren = $query->execute()->map();
+        self::$cache_numChildren[$baseClass]['numChildren'] = $numChildren;
+        if (!$idList) {
+            // If all objects are being cached, mark this cache as complete
+            // to avoid counting children of childless object.
+            self::$cache_numChildren[$baseClass]['numChildren']['_complete'] = true;
+        }
+    }
+
     /**
      * Checks if we're on a controller where we should filter. ie. Are we loading the SiteTree?
      *
@@ -309,16 +414,28 @@ class Hierarchy extends DataExtension
      *
      * @param bool $showAll Include all of the elements, even those not shown in the menus. Only applicable when
      *                      extension is applied to {@link SiteTree}.
+     * @param bool $skipParentIDFilter Set to true to supress the ParentID and ID where statements.
      * @return DataList
      */
-    public function stageChildren($showAll = false)
+    public function stageChildren($showAll = false, $skipParentIDFilter = false)
     {
         $hideFromHierarchy = $this->owner->config()->hide_from_hierarchy;
         $hideFromCMSTree = $this->owner->config()->hide_from_cms_tree;
         $baseClass = $this->owner->baseClass();
-        $staged = DataObject::get($baseClass)
-                ->filter('ParentID', (int)$this->owner->ID)
-                ->exclude('ID', (int)$this->owner->ID);
+        $baseTable = $this->owner->baseTable();
+        $staged = DataObject::get($baseClass)->where(sprintf(
+            '%s.%s <> %s.%s',
+            Convert::symbol2sql($baseTable),
+            Convert::symbol2sql("ParentID"),
+            Convert::symbol2sql($baseTable),
+            Convert::symbol2sql("ID")
+        ));
+
+        if (!$skipParentIDFilter) {
+            // There's no filtering by ID if we don't have an ID.
+            $staged = $staged->filter('ParentID', (int)$this->owner->ID);
+        }
+
         if ($hideFromHierarchy) {
             $staged = $staged->exclude('ClassName', $hideFromHierarchy);
         }
@@ -439,6 +556,6 @@ class Hierarchy extends DataExtension
     public function flushCache()
     {
         $this->owner->_cache_children = null;
-        $this->owner->_cache_numChildren = null;
+        self::$cache_numChildren = [];
     }
 }

tests/php/ORM/HierarchyCachingTest.php

@@ -0,0 +1,149 @@
+<?php
+
+namespace SilverStripe\Versioned\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\Hierarchy\Hierarchy;
+use SilverStripe\Versioned\Versioned;
+use SilverStripe\ORM\Tests\HierarchyTest\TestObject;
+use SilverStripe\ORM\Tests\HierarchyTest\HideTestObject;
+use SilverStripe\ORM\Tests\HierarchyTest\HideTestSubObject;
+
+/**
+ * @internal Only test the right values are returned, not that the cache is actually used.
+ */
+class HierachyCacheTest extends SapphireTest
+{
+
+    protected static $fixture_file = 'HierarchyTest.yml';
+
+    protected static $extra_dataobjects = array(
+        TestObject::class,
+        HideTestObject::class,
+        HideTestSubObject::class,
+    );
+
+    public function setUp()
+    {
+        parent::setUp();
+        TestObject::singleton()->flushCache();
+    }
+
+    public static function setUpBeforeClass()
+    {
+        parent::setUpBeforeClass();
+        HideTestObject::config()->update(
+            'hide_from_hierarchy',
+            [ HideTestSubObject::class ]
+        );
+    }
+
+    public function cacheNumChildrenDataProvider()
+    {
+        return [
+            [TestObject::class, 'obj1', false, 0, 'childless object should have a numChildren of 0'],
+            [TestObject::class, 'obj1', true, 0, 'childless object should have a numChildren of 0 when cache'],
+            [TestObject::class, 'obj2', false, 2, 'Root object numChildren should count direct children'],
+            [TestObject::class, 'obj2', true, 2, 'Root object numChildren should count direct children when cache'],
+            [TestObject::class, 'obj3a', false, 2, 'Sub object numChildren should count direct children'],
+            [TestObject::class, 'obj3a', true, 2, 'Sub object numChildren should count direct children when cache'],
+            [TestObject::class, 'obj3d', false, 0, 'Childess Sub object numChildren should be 0'],
+            [TestObject::class, 'obj3d', true, 0, 'Childess Sub object numChildren should be 0 when cache'],
+            [HideTestObject::class, 'obj4', false, 1, 'Hidden object should not be included in count'],
+            [HideTestObject::class, 'obj4', true, 1, 'Hidden object should not be included in couunt when cache']
+        ];
+    }
+
+
+    /**
+     * @dataProvider cacheNumChildrenDataProvider
+     */
+    public function testNumChildrenCache($className, $identifier, $cache, $expected, $message)
+    {
+        $node = $this->objFromFixture($className, $identifier);
+
+        $actual = $node->numChildren($cache);
+
+        $this->assertEquals($expected, $actual, $message);
+
+        if ($cache) {
+            // When caching is eanbled, try re-accessing the numChildren value to make sure it doesn't change.
+            $actual = $node->numChildren($cache);
+            $this->assertEquals($expected, $actual, $message);
+        }
+    }
+
+    public function prepopulateCacheNumChildrenDataProvider()
+    {
+        return [
+            [
+                TestObject::class, [],
+                'obj1', false, 0, 'childless object should have a numChildren of 0'
+            ],
+            [
+                TestObject::class, [],
+               'obj1', true, 0, 'childless object should have a numChildren of 0 when cache'
+            ],
+            [
+                TestObject::class, [2],
+                'obj1', false, 0, 'childless object should have a numChildren of 0'
+            ],
+            [
+                TestObject::class, [2],
+                'obj1', true, 0, 'childless object should have a numChildren of 0 when cache'
+            ],
+            [
+                TestObject::class, [],
+                'obj2', false, 2, 'Root object numChildren should count direct children'
+            ],
+            [
+                TestObject::class, [],
+                'obj2', true, 2, 'Root object numChildren should count direct children when cache'
+            ],
+            [
+                TestObject::class, [2],
+                'obj2', false, 2, 'Root object numChildren should count direct children'
+            ],
+            [
+                TestObject::class, [2],
+                'obj2', true, 2, 'Root object numChildren should count direct children when cache'
+            ],
+            [
+                HideTestObject::class, [],
+                'obj4', false, 1, 'Hidden object should not be included in count'
+            ],
+            [
+                HideTestObject::class, [],
+                'obj4', true, 1, 'Hidden object should not be included in count when cache'
+            ],
+            [
+                HideTestObject::class, [2],
+                'obj4', false, 1, 'Hidden object should not be included in count'
+            ],
+            [
+                HideTestObject::class, [2],
+                'obj4', true, 1, 'Hidden object should not be included in count when cache'
+            ]
+        ];
+    }
+
+    /**
+     * @dataProvider prepopulateCacheNumChildrenDataProvider
+     */
+    public function testPrepopulatedNumChildrenCache(
+        $className,
+        $idList,
+        $identifier,
+        $cache,
+        $expected,
+        $message
+    ) {
+        DataObject::singleton($className)->prepopulateTreeDataCache($idList, ['numChildrenMethod' => 'numChildren']);
+        $node = $this->objFromFixture($className, $identifier);
+
+        $actual = $node->numChildren($cache);
+
+        $this->assertEquals($expected, $actual, $message);
+    }
+}