From 8ee5d9f5ceddd4124feae69af9caf5a0f9b019f5 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Thu, 30 Nov 2017 15:56:16 +1300 Subject: [PATCH] ENHANCEMENT Cache canCreate --- _config/cache.yml | 5 + _config/permissions.yml | 1 - code/Controllers/CMSMain.php | 176 ++++++++++++++++++-------- code/Model/SiteTree.php | 108 ++++++++++++++-- tests/php/Controllers/CMSMainTest.php | 53 ++++++++ tests/php/Model/SiteTreeTest.php | 54 ++++++++ 6 files changed, 327 insertions(+), 70 deletions(-) diff --git a/_config/cache.yml b/_config/cache.yml index bbb93411..469fc35b 100644 --- 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" + diff --git a/_config/permissions.yml b/_config/permissions.yml index a81d2bcc..fcae58ee 100644 --- 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: diff --git a/code/Controllers/CMSMain.php b/code/Controllers/CMSMain.php index 84ca5255..e07ed442 100644 --- 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); + } + } + + } } diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 8e639bfd..ef858ad7 100755 --- 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( "%s", @@ -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__); + } } diff --git a/tests/php/Controllers/CMSMainTest.php b/tests/php/Controllers/CMSMainTest.php index 83d59872..6014d778 100644 --- 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); + + } } diff --git a/tests/php/Model/SiteTreeTest.php b/tests/php/Model/SiteTreeTest.php index ec6e2151..2a74b1a1 100644 --- 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); + + } }