1 files changed, 116 insertions, 60 deletions

diff --git a/engine/lib/access.php b/engine/lib/access.php
index dba1e1ec6..de0693ea8 100644
--- a/engine/lib/access.php
+++ b/engine/lib/access.php
@@ -12,6 +12,26 @@
  */
 
 /**
+ * Return an ElggCache static variable cache for the access caches
+ *
+ * @staticvar ElggStaticVariableCache $access_cache
+ * @return \ElggStaticVariableCache
+ * @access private
+ */
+function _elgg_get_access_cache() {
+	/**
+	 * A default filestore cache using the dataroot.
+	 */
+	static $access_cache;
+
+	if (!$access_cache) {
+		$access_cache = new ElggStaticVariableCache('access');
+	}
+
+	return $access_cache;
+}
+
+/**
  * Return a string of access_ids for $user_id appropriate for inserting into an SQL IN clause.
  *
  * @uses get_access_array
@@ -29,10 +49,10 @@
  */
 function get_access_list($user_id = 0, $site_id = 0, $flush = false) {
 	global $CONFIG, $init_finished;
-	static $access_list;
-
-	if (!isset($access_list) || !$init_finished) {
-		$access_list = array();
+	$cache = _elgg_get_access_cache();
+	
+	if ($flush) {
+		$cache->clear();
 	}
 
 	if ($user_id == 0) {
@@ -45,13 +65,20 @@ function get_access_list($user_id = 0, $site_id = 0, $flush = false) {
 	$user_id = (int) $user_id;
 	$site_id = (int) $site_id;
 
-	if (isset($access_list[$user_id]) && $flush == false) {
-		return $access_list[$user_id];
-	}
+	$hash = $user_id . $site_id . 'get_access_list';
 
-	$access_list[$user_id] = "(" . implode(",", get_access_array($user_id, $site_id, $flush)) . ")";
+	if ($cache[$hash]) {
+		return $cache[$hash];
+	}
+	
+	$access_array = get_access_array($user_id, $site_id, $flush);
+	$access = "(" . implode(",", $access_array) . ")";
 
-	return $access_list[$user_id];
+	if ($init_finished) {
+		$cache[$hash] = $access;
+	}
+	
+	return $access;
 }
 
 /**
@@ -79,12 +106,10 @@ function get_access_list($user_id = 0, $site_id = 0, $flush = false) {
 function get_access_array($user_id = 0, $site_id = 0, $flush = false) {
 	global $CONFIG, $init_finished;
 
-	// @todo everything from the db is cached.
-	// this cache might be redundant. But db cache is flushed on every db write.
-	static $access_array;
+	$cache = _elgg_get_access_cache();
 
-	if (!isset($access_array) || (!isset($init_finished)) || (!$init_finished)) {
-		$access_array = array();
+	if ($flush) {
+		$cache->clear();
 	}
 
 	if ($user_id == 0) {
@@ -98,35 +123,41 @@ function get_access_array($user_id = 0, $site_id = 0, $flush = false) {
 	$user_id = (int) $user_id;
 	$site_id = (int) $site_id;
 
-	if (empty($access_array[$user_id]) || $flush == true) {
-		$tmp_access_array = array(ACCESS_PUBLIC);
+	$hash = $user_id . $site_id . 'get_access_array';
+
+	if ($cache[$hash]) {
+		$access_array = $cache[$hash];
+	} else {
+		$access_array = array(ACCESS_PUBLIC);
 
 		// The following can only return sensible data if the user is logged in.
 		if (elgg_is_logged_in()) {
-			$tmp_access_array[] = ACCESS_LOGGED_IN;
+			$access_array[] = ACCESS_LOGGED_IN;
 
 			// Get ACL memberships
 			$query = "SELECT am.access_collection_id"
 				. " FROM {$CONFIG->dbprefix}access_collection_membership am"
 				. " LEFT JOIN {$CONFIG->dbprefix}access_collections ag ON ag.id = am.access_collection_id"
-				. " WHERE am.user_guid = {$user_id} AND (ag.site_guid = {$site_id} OR ag.site_guid = 0)";
+				. " WHERE am.user_guid = $user_id AND (ag.site_guid = $site_id OR ag.site_guid = 0)";
 
-			if ($collections = get_data($query)) {
+			$collections = get_data($query);
+			if ($collections) {
 				foreach ($collections as $collection) {
 					if (!empty($collection->access_collection_id)) {
-						$tmp_access_array[] = (int)$collection->access_collection_id;
+						$access_array[] = (int)$collection->access_collection_id;
 					}
 				}
 			}
 
 			// Get ACLs owned.
 			$query = "SELECT ag.id FROM {$CONFIG->dbprefix}access_collections ag ";
-			$query .= "WHERE ag.owner_guid = {$user_id} AND (ag.site_guid = {$site_id} OR ag.site_guid = 0)";
+			$query .= "WHERE ag.owner_guid = $user_id AND (ag.site_guid = $site_id OR ag.site_guid = 0)";
 
-			if ($collections = get_data($query)) {
+			$collections = get_data($query);
+			if ($collections) {
 				foreach ($collections as $collection) {
 					if (!empty($collection->id)) {
-						$tmp_access_array[] = (int)$collection->id;
+						$access_array[] = (int)$collection->id;
 					}
 				}
 			}
@@ -134,21 +165,21 @@ function get_access_array($user_id = 0, $site_id = 0, $flush = false) {
 			$ignore_access = elgg_check_access_overrides($user_id);
 
 			if ($ignore_access == true) {
-				$tmp_access_array[] = ACCESS_PRIVATE;
+				$access_array[] = ACCESS_PRIVATE;
 			}
-
-			$access_array[$user_id] = $tmp_access_array;
-		} else {
-			// No user id logged in so we can only access public info
-			$tmp_return = $tmp_access_array;
 		}
 
-	} else {
-		$tmp_access_array = $access_array[$user_id];
+		if ($init_finished) {
+			$cache[$hash] = $access_array;
+		}
 	}
 
-	$options = array('user_id' => $user_id, 'site_id' => $site_id);
-	return elgg_trigger_plugin_hook('access:collections:read', 'user', $options, $tmp_access_array);
+	$options = array(
+		'user_id' => $user_id,
+		'site_id' => $site_id
+	);
+	
+	return elgg_trigger_plugin_hook('access:collections:read', 'user', $options, $access_array);
 }
 
 /**
@@ -394,9 +425,12 @@ function has_access_to_entity($entity, $user = null) {
  * @link http://docs.elgg.org/Access
  */
 function get_write_access_array($user_id = 0, $site_id = 0, $flush = false) {
-	global $CONFIG;
-	//@todo this is probably not needed since caching happens at the DB level.
-	static $access_array;
+	global $CONFIG, $init_finished;
+	$cache = _elgg_get_access_cache();
+
+	if ($flush) {
+		$cache->clear();
+	}
 
 	if ($user_id == 0) {
 		$user_id = elgg_get_logged_in_user_guid();
@@ -409,37 +443,41 @@ function get_write_access_array($user_id = 0, $site_id = 0, $flush = false) {
 	$user_id = (int) $user_id;
 	$site_id = (int) $site_id;
 
-	if (empty($access_array[$user_id]) || $flush == true) {
-		$query = "SELECT ag.* FROM {$CONFIG->dbprefix}access_collections ag ";
-		$query .= " WHERE (ag.site_guid = {$site_id} OR ag.site_guid = 0)";
-		$query .= " AND (ag.owner_guid = {$user_id})";
-		// ACCESS_PRIVATE through ACCESS_PUBLIC take 0 through 2
-		// @todo this AND clause is unnecessary because of id starts at 3 for table
-		$query .= " AND ag.id >= 3";
+	$hash = $user_id . $site_id . 'get_write_access_array';
 
-		$tmp_access_array = array(
+	if ($cache[$hash]) {
+		$access_array = $cache[$hash];
+	} else {
+		// @todo is there such a thing as public write access?
+		$access_array = array(
 			ACCESS_PRIVATE => elgg_echo("PRIVATE"),
 			ACCESS_FRIENDS => elgg_echo("access:friends:label"),
 			ACCESS_LOGGED_IN => elgg_echo("LOGGED_IN"),
 			ACCESS_PUBLIC => elgg_echo("PUBLIC")
 		);
+		
+		$query = "SELECT ag.* FROM {$CONFIG->dbprefix}access_collections ag ";
+		$query .= " WHERE (ag.site_guid = $site_id OR ag.site_guid = 0)";
+		$query .= " AND (ag.owner_guid = $user_id)";
+
 		$collections = get_data($query);
 		if ($collections) {
 			foreach ($collections as $collection) {
-				$tmp_access_array[$collection->id] = $collection->name;
+				$access_array[$collection->id] = $collection->name;
 			}
 		}
 
-		$access_array[$user_id] = $tmp_access_array;
-	} else {
-		$tmp_access_array = $access_array[$user_id];
+		if ($init_finished) {
+			$cache[$hash] = $access_array;
+		}
 	}
 
-	$options = array('user_id' => $user_id, 'site_id' => $site_id);
-	$tmp_access_array = elgg_trigger_plugin_hook('access:collections:write', 'user',
-		$options, $tmp_access_array);
-
-	return $tmp_access_array;
+	$options = array(
+		'user_id' => $user_id,
+		'site_id' => $site_id
+	);
+	return elgg_trigger_plugin_hook('access:collections:write', 'user',
+		$options, $access_array);
 }
 
 /**
@@ -469,7 +507,7 @@ function can_edit_access_collection($collection_id, $user_guid = null) {
 		return false;
 	}
 
-	$write_access = get_write_access_array($user->getGUID(), null, true);
+	$write_access = get_write_access_array($user->getGUID(), 0, true);
 
 	// don't ignore access when checking users.
 	if ($user_guid) {
@@ -553,8 +591,6 @@ function create_access_collection($name, $owner_guid = 0, $site_guid = 0) {
  * @see remove_user_from_access_collection()
  */
 function update_access_collection($collection_id, $members) {
-	global $CONFIG;
-
 	$acl = get_access_collection($collection_id);
 
 	if (!$acl) {
@@ -671,8 +707,10 @@ function add_user_to_access_collection($user_guid, $collection_id) {
 		return false;
 	}
 
+	// if someone tries to insert the same data twice, we do a no-op on duplicate key
 	$q = "INSERT INTO {$CONFIG->dbprefix}access_collection_membership
-			SET access_collection_id = {$collection_id}, user_guid = {$user_guid}";
+			SET access_collection_id = $collection_id, user_guid = $user_guid
+			ON DUPLICATE KEY UPDATE user_guid = user_guid";
 	$result = insert_data($q);
 
 	return $result !== false;
@@ -868,6 +906,8 @@ function get_readable_access_level($entity_access_id) {
  * @tip Use this to access entities in automated scripts
  * when no user is logged in.
  *
+ * @note This clears the access cache.
+ *
  * @warning This will not show disabled entities.
  * Use {@link access_show_hidden_entities()} to access disabled entities.
  *
@@ -879,6 +919,8 @@ function get_readable_access_level($entity_access_id) {
  * @see elgg_get_ignore_access()
  */
 function elgg_set_ignore_access($ignore = true) {
+	$cache = _elgg_get_access_cache();
+	$cache->clear();
 	$elgg_access = elgg_get_access_object();
 	return $elgg_access->setIgnoreAccess($ignore);
 }
@@ -944,7 +986,8 @@ function elgg_get_access_object() {
  *
  * @global bool $init_finished
  * @access private
- * @todo investigate why this is needed
+ * @todo This is required to tell the access system to start caching because
+ * calls are made while in ignore access mode and before the user is logged in.
  */
 $init_finished = false;
 
@@ -972,6 +1015,10 @@ function access_init() {
  *
  * Returns true to override the access system or null if no change is needed.
  *
+ * @param string $hook
+ * @param string $type
+ * @param bool $value
+ * @param array $params
  * @return true|null
  * @access private
  */
@@ -1004,16 +1051,25 @@ function elgg_override_permissions($hook, $type, $value, $params) {
 
 /**
  * Runs unit tests for the entities object.
+ *
+ * @param string $hook
+ * @param string $type
+ * @param array $value
+ * @param array $params
+ * @return array
+ *
  * @access private
  */
 function access_test($hook, $type, $value, $params) {
 	global $CONFIG;
+
 	$value[] = $CONFIG->path . 'engine/tests/api/access_collections.php';
 	return $value;
 }
 
-// This function will let us know when 'init' has finished
-elgg_register_event_handler('init', 'system', 'access_init', 9999);
+// Tell the access functions the system has booted, plugins are loaded,
+// and the user is logged in so it can start caching
+elgg_register_event_handler('ready', 'system', 'access_init');
 
 // For overrided permissions
 elgg_register_plugin_hook_handler('permissions_check', 'all', 'elgg_override_permissions');