ENHACEMENT: Improved caching of permissions for improved CMS perfromance for non-admins (merged from r102278)

code/model/SiteTree.php

@@ -866,11 +866,6 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		$extended = $this->extendedCan('canDelete', $memberID);
 		if($extended !== null) return $extended;
 				
-		// Check cache (the can_edit_multiple call below will also do this, but this is quicker)
-		if(isset(self::$cache_permissions['delete'][$this->ID])) {
-			return self::$cache_permissions['delete'][$this->ID];
-		}
-		
 		// Regular canEdit logic is handled by can_edit_multiple
 		$results = self::can_delete_multiple(array($this->ID), $memberID);
 		
@@ -942,11 +937,6 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		if($extended !== null) return $extended;
 
 		if($this->ID) {
-			// Check cache (the can_edit_multiple call below will also do this, but this is quicker)
-			if(isset(self::$cache_permissions['CanEditType'][$this->ID])) {
-				return self::$cache_permissions['CanEditType'][$this->ID];
-			}
-		
 			// Regular canEdit logic is handled by can_edit_multiple
 			$results = self::can_edit_multiple(array($this->ID), $memberID);
 
@@ -1029,16 +1019,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		$batchCallback=explode('::', $batchCallback);
 		
 		if(is_callable($batchCallback)) {
-			$permissionValues = call_user_func($batchCallback, $ids, 
+			call_user_func($batchCallback, $ids, Member::currentUserID(), false);
-				Member::currentUserID(), false);
-				
-			if(!isset(self::$cache_permissions[$permission])) {
-				self::$cache_permissions[$permission] = array();
-			}
-			
-			self::$cache_permissions[$permission] = $permissionValues 
-				+ self::$cache_permissions[$permission];
-			
 		} else {
 			user_error("SiteTree::prepopuplate_permission_cache can't calculate '$permission' "
 				. "with callback '$batchCallback'", E_USER_WARNING);
@@ -1069,7 +1050,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		
 		// This is the name used on the permission cache
 		// converts something like 'CanEditType' to 'edit'.
-		$cacheKey = strtolower(substr($typeField, 3, -4));
+		$cacheKey = strtolower(substr($typeField, 3, -4)) . "-$memberID";
 
 		// Default result: nothing editable
 		$result = array_fill_keys($ids, false);
@@ -1165,14 +1146,11 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		}
 
 		if(isset($combinedStageResult)) {
-			// Cache results
+			// Cache the results
-			// TODO - Caching permissions is breaking unit tests. One possible issue
+ 			if(empty(self::$cache_permissions[$cacheKey])) self::$cache_permissions[$cacheKey] = array();
-			// is the cache needs to be flushed when permission on a page is changed,
+ 			self::$cache_permissions[$cacheKey] = $combinedStageResult + self::$cache_permissions[$cacheKey];
-			// but this only solved some of the failing unit tests. Disabled for now.
+ 
-			/*foreach($combinedStageResult as $id => $val) {
+  		return $combinedStageResult;
-				self::$cache_permissions[$typeField][$id] = $val;
-			}*/
-			return $combinedStageResult;
 		} else {
 			return array();
 		}
@@ -1197,15 +1175,15 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 	 */
 	static function can_delete_multiple($ids, $memberID, $useCached = true) {
 		$deletable = array();
-		
 		$result = array_fill_keys($ids, false);
+		$cacheKey = "delete-$memberID";
 		
 		// Look in the cache for values
-		if($useCached && isset(self::$cache_permissions['delete'])) {
+		if($useCached && isset(self::$cache_permissions[$cacheKey])) {
-			$cachedValues = array_intersect_key(self::$cache_permissions['delete'], $result);
+			$cachedValues = array_intersect_key(self::$cache_permissions[$cacheKey], $result);
 			
 			// If we can't find everything in the cache, then look up the remainder separately
-			$uncachedValues = array_diff_key($result, self::$cache_permissions['delete']);
+			$uncachedValues = array_diff_key($result, self::$cache_permissions[$cacheKey]);
 			if($uncachedValues) {
 				$cachedValues = self::can_delete_multiple(array_keys($uncachedValues), $memberID, false)
 					+ $cachedValues;
@@ -1254,6 +1232,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		return array_fill_keys($deletable, true) + array_fill_keys($ids, false);
 	}
 		
+
 	/**
 	 * Collate selected descendants of this page.
 	 *
@@ -2707,6 +2686,10 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 		self::$cache_permissions = array();
 	}
 	
+	static function on_db_reset() {
+		self::$cache_permissions = array();
+	}
+
 }
 
 ?>

tests/model/SiteTreeTest.php

@@ -504,6 +504,8 @@ class SiteTreeTest extends SapphireTest {
 		$page->CanEditType = 'OnlyTheseUsers';
 		$page->EditorGroups()->add($this->idFromFixture('Group', 'editors'));
 		$page->write();
+		// Clear permission cache
+		SiteTree::on_db_reset();
 		
 		// Confirm that Member.editor can now edit the page
 		$this->objFromFixture('Member','editor')->logIn();