NEW: Add Hierarchy::prepopulate_numchildren_cache()

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\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 @@ public function numHistoricalChildren()
      */
     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 @@ public function numChildren($cache = true)
 
         // 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 @@ public function showingCMSTree()
      *
      * @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 @@ public function getBreadcrumbs($separator = ' &raquo; ')
     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);
+    }
+}