ENHANCEMENT Cache canCreate

2017-11-30 15:56:16 +13:00 · 2017-11-30 15:56:16 +13:00 · 8ee5d9f5ce
parent 5d879e5b41
commit 8ee5d9f5ce
6 changed files with 327 additions and 70 deletions
--- a/_config/cache.yml
+++ b/_config/cache.yml
@ -8,3 +8,8 @@ SilverStripe\Core\Injector\Injector:
    factory: SilverStripe\Core\Cache\CacheFactory
    constructor:
      namespace: "CMSMain_SiteTreeHints"
+  Psr\SimpleCache\CacheInterface.SiteTree_CreatableChildren:
+    factory: SilverStripe\Core\Cache\CacheFactory
+    constructor:
+      namespace: "SiteTree_CreatableChildren"
+
--- a/_config/permissions.yml
+++ b/_config/permissions.yml
@ -6,7 +6,6 @@ SilverStripe\Core\Injector\Injector:
    class: SilverStripe\Security\InheritedPermissions
    constructor:
      BaseClass: SilverStripe\CMS\Model\SiteTree
-      CacheService: '%$Psr\SimpleCache\CacheInterface.InheritedPermissions'
    properties:
      DefaultPermissions: '%$SilverStripe\SiteConfig\SiteConfigPagePermissions'
      GlobalEditPermissions:
--- a/code/Controllers/CMSMain.php
+++ b/code/Controllers/CMSMain.php
@ -25,8 +25,6 @@ use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Environment;
 use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Core\Manifest\ModuleResource;
-use SilverStripe\Core\Manifest\ModuleResourceLoader;
 use SilverStripe\Forms\DateField;
 use SilverStripe\Forms\DropdownField;
 use SilverStripe\Forms\FieldGroup;
@ -53,7 +51,6 @@ use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBHTMLText;
 use SilverStripe\ORM\HiddenClass;
-use SilverStripe\ORM\Hierarchy;
 use SilverStripe\ORM\Hierarchy\MarkedSet;
 use SilverStripe\ORM\SS_List;
 use SilverStripe\ORM\ValidationResult;
@ -69,6 +66,8 @@ use SilverStripe\Versioned\ChangeSetItem;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\Requirements;
+use SilverStripe\Core\Flushable;
+use SilverStripe\Core\Cache\MemberCacheFlusher;
 use Translatable;

 /**
@ -81,7 +80,7 @@ use Translatable;
 *
 * @mixin LeftAndMainPageIconsExtension
 */
-class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionProvider
+class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionProvider, Flushable, MemberCacheFlusher
 {
    /**
     * Unique ID for page icons CSS block
@ -162,6 +161,15 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
        'SiteTreeAsUL' => 'HTMLFragment',
    );

+    private static $dependencies = [
+        'HintsCache' => '%$' . CacheInterface::class . '.CMSMain_SiteTreeHints',
+    ];
+
+    /**
+     * @var CacheInterface
+     */
+    protected $hintsCache;
+
    protected function init()
    {
        // set reading lang
@ -381,6 +389,33 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
        return 'edit';
    }

+    /**
+     * @param CacheInterface $cache
+     * @return $this
+     */
+    public function setHintsCache(CacheInterface $cache)
+    {
+        $this->hintsCache = $cache;
+
+        return $this;
+    }
+
+    /**
+     * @return CacheInterface $cache
+     */
+    public function getHintsCache()
+    {
+        return $this->hintsCache;
+    }
+
+    /**
+     * Clears all dependent cache backends
+     */
+    public function clearCache()
+    {
+        $this->getHintsCache()->clear();
+    }
+
    public function LinkWithSearch($link)
    {
        // Whitelist to avoid side effects
@ -977,67 +1012,65 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
    public function SiteTreeHints()
    {
        $classes = SiteTree::page_type_classes();
-
-        $cacheCanCreate = array();
-        foreach ($classes as $class) {
-            $cacheCanCreate[$class] = singleton($class)->canCreate();
-        }
-
-        // Generate basic cache key. Too complex to encompass all variations
-        $cache = Injector::inst()->get(CacheInterface::class . '.CMSMain_SiteTreeHints');
-        $cacheKey = md5(implode('_', array(Security::getCurrentUser()->ID, implode(',', $cacheCanCreate), implode(',', $classes))));
-        if ($this->getRequest()->getVar('flush')) {
-            $cache->clear();
-        }
+        $memberID = Security::getCurrentUser() ? Security::getCurrentUser()->ID : 0;
+        $cache = $this->getHintsCache();
+        $cacheKey = $this->generateHintsCacheKey($memberID);
        $json = $cache->get($cacheKey);
-        if (!$json) {
-            $def['Root'] = array();
-            $def['Root']['disallowedChildren'] = array();

-            // Contains all possible classes to support UI controls listing them all,
-            // such as the "add page here" context menu.
-            $def['All'] = array();
+        if ($json) return $json;

-            // Identify disallows and set globals
-            foreach ($classes as $class) {
-                $obj = singleton($class);
-                if ($obj instanceof HiddenClass) {
-                    continue;
-                }
+        $canCreate = [];
+        foreach ($classes as $class) {
+            $canCreate[$class] = singleton($class)->canCreate();
+        }

-                // Name item
-                $def['All'][$class] = array(
-                    'title' => $obj->i18n_singular_name()
-                );
+        $def['Root'] = [];
+        $def['Root']['disallowedChildren'] = [];

-                // Check if can be created at the root
-                $needsPerm = $obj->config()->get('need_permission');
-                if (!$obj->config()->get('can_be_root')
-                    || (!array_key_exists($class, $cacheCanCreate) || !$cacheCanCreate[$class])
-                    || ($needsPerm && !$this->can($needsPerm))
-                ) {
-                    $def['Root']['disallowedChildren'][] = $class;
-                }
+        // Contains all possible classes to support UI controls listing them all,
+        // such as the "add page here" context menu.
+        $def['All'] = [];

-                // Hint data specific to the class
-                $def[$class] = array();
-
-                $defaultChild = $obj->defaultChild();
-                if ($defaultChild !== 'Page' && $defaultChild !== null) {
-                    $def[$class]['defaultChild'] = $defaultChild;
-                }
-
-                $defaultParent = $obj->defaultParent();
-                if ($defaultParent !== 1 && $defaultParent !== null) {
-                    $def[$class]['defaultParent'] = $defaultParent;
-                }
+        // Identify disallows and set globals
+        foreach ($classes as $class) {
+            $obj = singleton($class);
+            if ($obj instanceof HiddenClass) {
+                continue;
            }

-            $this->extend('updateSiteTreeHints', $def);
+            // Name item
+            $def['All'][$class] = [
+                'title' => $obj->i18n_singular_name()
+            ];

-            $json = Convert::raw2json($def);
-            $cache->set($cacheKey, $json);
+            // Check if can be created at the root
+            $needsPerm = $obj->config()->get('need_permission');
+            if (!$obj->config()->get('can_be_root')
+                || (!array_key_exists($class, $canCreate) || !$canCreate[$class])
+                || ($needsPerm && !$this->can($needsPerm))
+            ) {
+                $def['Root']['disallowedChildren'][] = $class;
+            }
+
+            // Hint data specific to the class
+            $def[$class] = [];
+
+            $defaultChild = $obj->defaultChild();
+            if ($defaultChild !== 'Page' && $defaultChild !== null) {
+                $def[$class]['defaultChild'] = $defaultChild;
+            }
+
+            $defaultParent = $obj->defaultParent();
+            if ($defaultParent !== 1 && $defaultParent !== null) {
+                $def[$class]['defaultParent'] = $defaultParent;
+            }
        }
+
+        $this->extend('updateSiteTreeHints', $def);
+
+        $json = Convert::raw2json($def);
+        $cache->set($cacheKey, $json);
+
        return $json;
    }

@ -2174,4 +2207,37 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr
        $this->extend('updateCMSTreeTitle', $rootTitle);
        return $rootTitle;
    }
+
+    protected function generateHintsCacheKey($memberID)
+    {
+        return md5($memberID);
+    }
+
+    /**
+     * Clear the cache on ?flush
+     */
+    public static function flush()
+    {
+        static::singleton()->clearCache();
+    }
+
+    /**
+     * Flush the hints cache for a specific member
+     *
+     * @param null $memberIDs
+     */
+    public function flushMemberCache($memberIDs = null)
+    {
+        $cache = $this->getHintsCache();
+
+        if (!$memberIDs) {
+            $cache->clear();
+        } else if (is_array($memberIDs)) {
+            foreach($memberIDs as $memberID) {
+                $key = $this->generateHintsCacheKey($memberID);
+                $cache->delete($key);
+            }
+        }
+
+    }
 }
--- a/code/Model/SiteTree.php
+++ b/code/Model/SiteTree.php
@ -3,6 +3,7 @@
 namespace SilverStripe\CMS\Model;

 use Page;
+use Psr\SimpleCache\CacheInterface;
 use SilverStripe\CampaignAdmin\AddToCampaignHandler_FormAction;
 use SilverStripe\CMS\Controllers\CMSPageEditController;
 use SilverStripe\CMS\Controllers\ContentController;
@ -16,6 +17,7 @@ use SilverStripe\Control\RequestHandler;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Convert;
+use SilverStripe\Core\Flushable;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Core\Manifest\ModuleResource;
 use SilverStripe\Core\Manifest\ModuleResourceLoader;
@ -30,7 +32,6 @@ use SilverStripe\Forms\FormAction;
 use SilverStripe\Forms\GridField\GridField;
 use SilverStripe\Forms\GridField\GridFieldDataColumns;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
-use SilverStripe\Forms\ListboxField;
 use SilverStripe\Forms\LiteralField;
 use SilverStripe\Forms\OptionsetField;
 use SilverStripe\Forms\Tab;
@ -66,6 +67,7 @@ use SilverStripe\View\HTML;
 use SilverStripe\View\Parsers\ShortcodeParser;
 use SilverStripe\View\Parsers\URLSegmentFilter;
 use SilverStripe\View\SSViewer;
+use SilverStripe\Core\Cache\MemberCacheFlusher;
 use Subsite;

 /**
@ -100,7 +102,7 @@ use Subsite;
 * @mixin SiteTreeLinkTracking
 * @mixin InheritedPermissionsExtension
 */
-class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvider, CMSPreviewable, Resettable
+class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvider, CMSPreviewable, Resettable, Flushable, MemberCacheFlusher
 {

    /**
@ -343,6 +345,18 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
     */
    private static $base_description = 'Generic content page';

+    /**
+     * @var array
+     */
+    private static $dependencies = [
+        'creatableChildrenCache' => '%$' . CacheInterface::class . '.SiteTree_CreatableChildren'
+    ];
+
+    /**
+     * @var CacheInterface
+     */
+    protected $creatableChildrenCache;
+
    /**
     * Fetches the {@link SiteTree} object that maps to a link.
     *
@ -904,6 +918,25 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
        return null;
    }

+    /**
+     * @param CacheInterface $cache
+     * @return $this
+     */
+    public function setCreatableChildrenCache(CacheInterface $cache)
+    {
+        $this->creatableChildrenCache = $cache;
+
+        return $this;
+    }
+
+    /**
+     * @return CacheInterface $cache
+     */
+    public function getCreatableChildrenCache()
+    {
+        return $this->creatableChildrenCache;
+    }
+
    /**
     * Return a string of the form "parent - page" or "grandparent - parent - page" using page titles
     *
@ -1508,6 +1541,24 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
        $this->_cache_statusFlags = null;
    }

+    /**
+     * Flushes the member specific cache for creatable children
+     * @param null $memberIDs
+     */
+    public function flushMemberCache($memberIDs = null)
+    {
+        $cache = static::singleton()->getCreatableChildrenCache();
+
+        if ($memberIDs) {
+            $cache->clear();
+        } else if (is_array($memberIDs)) {
+            foreach ($memberIDs as $memberID) {
+                $key = $this->generateChildrenCacheKey($memberID);
+                $cache->delete($key);
+            }
+        }
+    }
+
    public function validate()
    {
        $result = parent::validate();
@ -2528,6 +2579,32 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
        return $allowedChildren;
    }

+    /**
+     * Gets a list of the page types that can be created under this specific page
+     *
+     * @return array
+     */
+    public function creatableChildren()
+    {
+        // Build the list of candidate children
+        $cache = static::singleton()->getCreatableChildrenCache();
+        $cacheKey = $this->generateChildrenCacheKey(Security::getCurrentUser() ? Security::getCurrentUser()->ID : 0);
+        $children = $cache->get($cacheKey, []);
+        if (!$children || !isset($children[$this->ID])) {
+            $children[$this->ID] = [];
+            $candidates = static::page_type_classes();
+            foreach ($candidates as $childClass) {
+                $child = singleton($childClass);
+                if ($child->canCreate(null, ['Parent' => $this])) {
+                    $children[$this->ID][$childClass] = $child->i18n_singular_name();
+                }
+            }
+            $cache->set($cacheKey, $children);
+        }
+
+        return $children[$this->ID];
+    }
+
    /**
     * Returns the class name of the default class for children of this page.
     *
@ -2644,18 +2721,7 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
     */
    public function getTreeTitle()
    {
-        // Build the list of candidate children
-        $children = array();
-        $candidates = static::page_type_classes();
-        foreach ($this->allowedChildren() as $childClass) {
-            if (!in_array($childClass, $candidates)) {
-                continue;
-            }
-            $child = singleton($childClass);
-            if ($child->canCreate(null, array('Parent' => $this))) {
-                $children[$childClass] = $child->i18n_singular_name();
-            }
-        }
+        $children = $this->creatableChildren();
        $flags = $this->getStatusFlags();
        $treeTitle = sprintf(
            "<span class=\"jstree-pageicon page-icon class-%s\"></span><span class=\"item\" data-allowedchildren=\"%s\">%s</span>",
@ -2956,6 +3022,15 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
        }
    }

+    /**
+     * Clear the creatableChildren cache on flush
+     */
+    public static function flush()
+    {
+        Injector::inst()->get(CacheInterface::class . '.SiteTree_CreatableChildren')
+            ->clear();
+    }
+
    /**
     * Update dependant pages
     */
@ -2973,4 +3048,9 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
            }
        }
    }
+
+    protected function generateChildrenCacheKey($memberID)
+    {
+        return md5($memberID . '_' . __CLASS__);
+    }
 }
--- a/tests/php/Controllers/CMSMainTest.php
+++ b/tests/php/Controllers/CMSMainTest.php
@ -577,4 +577,57 @@ class CMSMainTest extends FunctionalTest
        $this->assertEquals(CMSMainTest_ClassB::class, $newPage->ClassName);
        $this->assertEquals('Class A', $newPage->Title);
    }
+
+    public function testSiteTreeHintsCache()
+    {
+        $cms = CMSMain::create();
+        $user = $this->objFromFixture(Member::class, 'rootedituser');
+        Security::setCurrentUser($user);
+        $pageClass = array_values(SiteTree::page_type_classes())[0];
+        $mockPageMissesCache = $this->getMockBuilder($pageClass)
+            ->setMethods(['canCreate'])
+            ->getMock();
+        $mockPageMissesCache
+            ->expects($this->exactly(3))
+            ->method('canCreate');
+
+        $mockPageHitsCache = $this->getMockBuilder($pageClass)
+            ->setMethods(['canCreate'])
+            ->getMock();
+        $mockPageHitsCache
+            ->expects($this->never())
+            ->method('canCreate');
+
+
+        // Initially, cache misses (1)
+        Injector::inst()->registerService($mockPageMissesCache, $pageClass);
+        $hints = $cms->SiteTreeHints();
+        $this->assertNotNull($hints);
+
+        // Now it hits
+        Injector::inst()->registerService($mockPageHitsCache, $pageClass);
+        $hints = $cms->SiteTreeHints();
+        $this->assertNotNull($hints);
+
+
+        // Mutating member record invalidates cache. Misses (2)
+        $user->FirstName = 'changed';
+        $user->write();
+        Injector::inst()->registerService($mockPageMissesCache, $pageClass);
+        $hints = $cms->SiteTreeHints();
+        $this->assertNotNull($hints);
+
+        // Now it hits again
+        Injector::inst()->registerService($mockPageHitsCache, $pageClass);
+        $hints = $cms->SiteTreeHints();
+        $this->assertNotNull($hints);
+
+        // Different user. Misses. (3)
+        $user = $this->objFromFixture(Member::class, 'allcmssectionsuser');
+        Security::setCurrentUser($user);
+        Injector::inst()->registerService($mockPageMissesCache, $pageClass);
+        $hints = $cms->SiteTreeHints();
+        $this->assertNotNull($hints);
+
+    }
 }
--- a/tests/php/Model/SiteTreeTest.php
+++ b/tests/php/Model/SiteTreeTest.php
@ -26,6 +26,7 @@ use SilverStripe\Versioned\Versioned;
 use SilverStripe\View\Parsers\Diff;
 use SilverStripe\View\Parsers\ShortcodeParser;
 use SilverStripe\View\Parsers\URLSegmentFilter;
+use SilverStripe\Core\Injector\Injector;
 use LogicException;

 class SiteTreeTest extends SapphireTest
@ -1506,4 +1507,57 @@ class SiteTreeTest extends SapphireTest
        $class = new SiteTreeTest_LegacyControllerName;
        $this->assertEquals(SiteTreeTest_LegacyControllerName_Controller::class, $class->getControllerName());
    }
+
+    public function testTreeTitleCache()
+    {
+        $siteTree = SiteTree::create();
+        $user = $this->objFromFixture(Member::class, 'allsections');
+        Security::setCurrentUser($user);
+        $pageClass = array_values(SiteTree::page_type_classes())[0];
+
+        $mockPageMissesCache = $this->getMockBuilder($pageClass)
+            ->setMethods(['canCreate'])
+            ->getMock();
+        $mockPageMissesCache
+            ->expects($this->exactly(3))
+            ->method('canCreate');
+
+        $mockPageHitsCache = $this->getMockBuilder($pageClass)
+            ->setMethods(['canCreate'])
+            ->getMock();
+        $mockPageHitsCache
+            ->expects($this->never())
+            ->method('canCreate');
+
+        // Initially, cache misses (1)
+        Injector::inst()->registerService($mockPageMissesCache, $pageClass);
+        $title = $siteTree->getTreeTitle();
+        $this->assertNotNull($title);
+
+        // Now it hits
+        Injector::inst()->registerService($mockPageHitsCache, $pageClass);
+        $title = $siteTree->getTreeTitle();
+        $this->assertNotNull($title);
+
+
+        // Mutating member record invalidates cache. Misses (2)
+        $user->FirstName = 'changed';
+        $user->write();
+        Injector::inst()->registerService($mockPageMissesCache, $pageClass);
+        $title = $siteTree->getTreeTitle();
+        $this->assertNotNull($title);
+
+        // Now it hits again
+        Injector::inst()->registerService($mockPageHitsCache, $pageClass);
+        $title = $siteTree->getTreeTitle();
+        $this->assertNotNull($title);
+
+        // Different user. Misses. (3)
+        $user = $this->objFromFixture(Member::class, 'editor');
+        Security::setCurrentUser($user);
+        Injector::inst()->registerService($mockPageMissesCache, $pageClass);
+        $title = $siteTree->getTreeTitle();
+        $this->assertNotNull($title);
+
+    }
 }